Made multiformat legacy imps become multiformat openrtb imps.

mercuryyy · Jul 9, 2018 · 95ffd46 · 95ffd46
1 parent df05e99
commit 95ffd46
Show file tree

Hide file tree

Showing 13 changed files with 43 additions and 103 deletions.
diff --git a/adapters/appnexus/appnexus.go b/adapters/appnexus/appnexus.go
@@ -74,7 +74,7 @@ type appnexusImpExt struct {
 
 func (a *AppNexusAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *pbs.PBSBidder) (pbs.PBSBidSlice, error) {
 	supportedMediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER, pbs.MEDIA_TYPE_VIDEO}
-	anReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), supportedMediaTypes, true)
+	anReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), supportedMediaTypes)
 
 	if err != nil {
 		return nil, err

diff --git a/adapters/audienceNetwork/facebook.go b/adapters/audienceNetwork/facebook.go
@@ -120,7 +120,7 @@ func (a *FacebookAdapter) callOne(ctx context.Context, reqJSON bytes.Buffer) (re
 
 func (a *FacebookAdapter) MakeOpenRtbBidRequest(req *pbs.PBSRequest, bidder *pbs.PBSBidder, placementId string, mtype pbs.MediaType, pubId string, unitInd int) (openrtb.BidRequest, error) {
 	// this method creates imps for all ad units for the bidder with a single media type
-	fbReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), []pbs.MediaType{mtype}, true)
+	fbReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), []pbs.MediaType{mtype})
 
 	if err != nil {
 		return openrtb.BidRequest{}, err

diff --git a/adapters/beachfront/beachfront.go b/adapters/beachfront/beachfront.go
@@ -2,13 +2,14 @@ package beachfront
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
+	"net/http"
+	"strings"
+
 	"github.com/mxmCherry/openrtb"
-	"github.com/pkg/errors"
 	"github.com/prebid/prebid-server/adapters"
 	"github.com/prebid/prebid-server/openrtb_ext"
-	"net/http"
-	"strings"
 )
 
 const Seat = "beachfront"

diff --git a/adapters/conversant/conversant.go b/adapters/conversant/conversant.go
@@ -44,7 +44,7 @@ type conversantParams struct {
 
 func (a *ConversantAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *pbs.PBSBidder) (pbs.PBSBidSlice, error) {
 	mediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER, pbs.MEDIA_TYPE_VIDEO}
-	cnvrReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes, true)
+	cnvrReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes)
 
 	if err != nil {
 		return nil, err

diff --git a/adapters/indexExchange/index.go b/adapters/indexExchange/index.go
@@ -41,7 +41,7 @@ func (a *IndexAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *pb
 		}
 	}
 	mediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER, pbs.MEDIA_TYPE_VIDEO}
-	indexReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes, true)
+	indexReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes)
 
 	if err != nil {
 		return nil, err

diff --git a/adapters/lifestreet/lifestreet.go b/adapters/lifestreet/lifestreet.go
@@ -84,7 +84,7 @@ func (a *LifestreetAdapter) callOne(ctx context.Context, req *pbs.PBSRequest, re
 }
 
 func (a *LifestreetAdapter) MakeOpenRtbBidRequest(req *pbs.PBSRequest, bidder *pbs.PBSBidder, slotTag string, mtype pbs.MediaType, unitInd int) (openrtb.BidRequest, error) {
-	lsReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), []pbs.MediaType{mtype}, true)
+	lsReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), []pbs.MediaType{mtype})
 
 	if err != nil {
 		return openrtb.BidRequest{}, err

diff --git a/adapters/openrtb_util.go b/adapters/openrtb_util.go
@@ -74,7 +74,7 @@ func makeVideo(unit pbs.PBSAdUnit) *openrtb.Video {
 //
 // Any objects pointed to by the returned BidRequest *must not be mutated*, or we will get race conditions.
 // The only exception is the Imp property, whose objects will be created new by this method and can be mutated freely.
-func MakeOpenRTBGeneric(req *pbs.PBSRequest, bidder *pbs.PBSBidder, bidderFamily string, allowedMediatypes []pbs.MediaType, singleMediaTypeImp bool) (openrtb.BidRequest, error) {
+func MakeOpenRTBGeneric(req *pbs.PBSRequest, bidder *pbs.PBSBidder, bidderFamily string, allowedMediatypes []pbs.MediaType) (openrtb.BidRequest, error) {
 	imps := make([]openrtb.Imp, 0, len(bidder.AdUnits)*len(allowedMediatypes))
 	for _, unit := range bidder.AdUnits {
 		if len(unit.Sizes) <= 0 {
@@ -85,47 +85,26 @@ func MakeOpenRTBGeneric(req *pbs.PBSRequest, bidder *pbs.PBSBidder, bidderFamily
 			continue
 		}
 
-		if singleMediaTypeImp {
-			for _, mType := range unitMediaTypes {
-				newImp := openrtb.Imp{
-					ID:     unit.Code,
-					Secure: &req.Secure,
-					Instl:  unit.Instl,
-				}
-				switch mType {
-				case pbs.MEDIA_TYPE_BANNER:
-					newImp.Banner = makeBanner(unit)
-				case pbs.MEDIA_TYPE_VIDEO:
-					video := makeVideo(unit)
-					if video == nil {
-						return openrtb.BidRequest{}, &BadInputError{
-							Message: "Invalid AdUnit: VIDEO media type with no video data",
-						}
+		newImp := openrtb.Imp{
+			ID:     unit.Code,
+			Secure: &req.Secure,
+			Instl:  unit.Instl,
+		}
+		for _, mType := range unitMediaTypes {
+			switch mType {
+			case pbs.MEDIA_TYPE_BANNER:
+				newImp.Banner = makeBanner(unit)
+			case pbs.MEDIA_TYPE_VIDEO:
+				newImp.Video = makeVideo(unit)
+				// It's strange to error here... but preserves legacy behavior in legacy code. See #603.
+				if newImp.Video == nil {
+					return openrtb.BidRequest{}, &BadInputError{
+						Message: "Invalid AdUnit: VIDEO media type with no video data",
 					}
-					newImp.Video = video
-				default:
-					// Error - unknown media type
-					continue
-				}
-				imps = append(imps, newImp)
-			}
-		} else {
-			newImp := openrtb.Imp{
-				ID:     unit.Code,
-				Secure: &req.Secure,
-				Instl:  unit.Instl,
-			}
-			for _, mType := range unitMediaTypes {
-				switch mType {
-				case pbs.MEDIA_TYPE_BANNER:
-					newImp.Banner = makeBanner(unit)
-				case pbs.MEDIA_TYPE_VIDEO:
-					newImp.Video = makeVideo(unit)
-				default:
-					// Error - unknown media type
-					continue
 				}
 			}
+		}
+		if newImp.Banner != nil || newImp.Video != nil {
 			imps = append(imps, newImp)
 		}
 	}

diff --git a/adapters/openrtb_util_test.go b/adapters/openrtb_util_test.go
@@ -52,7 +52,7 @@ func TestOpenRTB(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 
 	assert.Equal(t, err, nil)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
@@ -90,7 +90,7 @@ func TestOpenRTBVideo(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO})
 
 	assert.Equal(t, err, nil)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
@@ -119,7 +119,7 @@ func TestOpenRTBVideoNoVideoData(t *testing.T) {
 			},
 		},
 	}
-	_, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO}, true)
+	_, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO})
 
 	assert.NotEqual(t, err, nil)
 
@@ -161,7 +161,7 @@ func TestOpenRTBVideoFilteredOut(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	for i := 0; i < len(resp.Imp); i++ {
 		if resp.Imp[i].Video != nil {
@@ -196,7 +196,7 @@ func TestOpenRTBMultiMediaImp(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO, pbs.MEDIA_TYPE_BANNER}, false)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO, pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	assert.Equal(t, len(resp.Imp), 1)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
@@ -232,50 +232,14 @@ func TestOpenRTBMultiMediaImpFiltered(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, false)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	assert.Equal(t, len(resp.Imp), 1)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
 	assert.EqualValues(t, *resp.Imp[0].Banner.W, 10)
 	assert.EqualValues(t, resp.Imp[0].Video, (*openrtb.Video)(nil))
 }
 
-func TestOpenRTBSingleMediaImp(t *testing.T) {
-
-	pbReq := pbs.PBSRequest{}
-	pbBidder := pbs.PBSBidder{
-		BidderCode: "bannerCode",
-		AdUnits: []pbs.PBSAdUnit{
-			{
-				Code:       "unitCode",
-				MediaTypes: []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO, pbs.MEDIA_TYPE_BANNER},
-				Sizes: []openrtb.Format{
-					{
-						W: 10,
-						H: 12,
-					},
-				},
-				Video: pbs.PBSVideo{
-					Mimes:          []string{"video/mp4"},
-					Minduration:    15,
-					Maxduration:    30,
-					Startdelay:     5,
-					Skippable:      0,
-					PlaybackMethod: 1,
-				},
-			},
-		},
-	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_VIDEO, pbs.MEDIA_TYPE_BANNER}, true)
-	assert.Equal(t, err, nil)
-	assert.Equal(t, len(resp.Imp), 2)
-	assert.Equal(t, resp.Imp[0].ID, "unitCode")
-	assert.EqualValues(t, resp.Imp[0].Video.MaxDuration, 30)
-	assert.EqualValues(t, resp.Imp[0].Video.MinDuration, 15)
-	assert.Equal(t, resp.Imp[1].ID, "unitCode")
-	assert.EqualValues(t, *resp.Imp[1].Banner.W, 10)
-}
-
 func TestOpenRTBNoSize(t *testing.T) {
 
 	pbReq := pbs.PBSRequest{}
@@ -288,7 +252,7 @@ func TestOpenRTBNoSize(t *testing.T) {
 			},
 		},
 	}
-	_, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	_, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	if err == nil {
 		t.Errorf("Bids without impressions should not be allowed.")
 	}
@@ -335,7 +299,7 @@ func TestOpenRTBMobile(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
 	assert.EqualValues(t, *resp.Imp[0].Banner.W, 300)
@@ -371,7 +335,7 @@ func TestOpenRTBEmptyUser(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	assert.EqualValues(t, resp.User, &openrtb.User{})
 }
@@ -398,7 +362,7 @@ func TestOpenRTBUserWithCookie(t *testing.T) {
 		},
 	}
 	pbReq.Cookie = pbsCookie
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	assert.EqualValues(t, resp.User.BuyerUID, "abcde")
 }
@@ -495,7 +459,7 @@ func TestGDPR(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 
 	assert.Equal(t, err, nil)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
@@ -558,7 +522,7 @@ func TestGDPRMobile(t *testing.T) {
 			},
 		},
 	}
-	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}, true)
+	resp, err := MakeOpenRTBGeneric(&pbReq, &pbBidder, "test", []pbs.MediaType{pbs.MEDIA_TYPE_BANNER})
 	assert.Equal(t, err, nil)
 	assert.Equal(t, resp.Imp[0].ID, "unitCode")
 	assert.EqualValues(t, *resp.Imp[0].Banner.W, 300)

diff --git a/adapters/pubmatic/pubmatic.go b/adapters/pubmatic/pubmatic.go
@@ -46,7 +46,7 @@ func PrepareLogMessage(tID, pubId, adUnitId, bidID, details string, args ...inte
 
 func (a *PubmaticAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *pbs.PBSBidder) (pbs.PBSBidSlice, error) {
 	mediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER, pbs.MEDIA_TYPE_VIDEO}
-	pbReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes, true)
+	pbReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes)
 
 	if err != nil {
 		logf("[PUBMATIC] Failed to make ortb request for request id [%s] \n", pbReq.ID)

diff --git a/adapters/pulsepoint/pulsepoint.go b/adapters/pulsepoint/pulsepoint.go
@@ -40,7 +40,7 @@ type PulsepointParams struct {
 
 func (a *PulsePointAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *pbs.PBSBidder) (pbs.PBSBidSlice, error) {
 	mediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}
-	ppReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes, true)
+	ppReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), mediaTypes)
 
 	if err != nil {
 		return nil, err

diff --git a/adapters/rubicon/rubicon.go b/adapters/rubicon/rubicon.go
@@ -332,7 +332,7 @@ func (a *RubiconAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *
 	callOneObjects := make([]callOneObject, 0, len(bidder.AdUnits))
 	supportedMediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER, pbs.MEDIA_TYPE_VIDEO}
 
-	rubiReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), supportedMediaTypes, true)
+	rubiReq, err := adapters.MakeOpenRTBGeneric(req, bidder, a.Name(), supportedMediaTypes)
 	if err != nil {
 		return nil, err
 	}

diff --git a/adapters/rubicon/rubicon_test.go b/adapters/rubicon/rubicon_test.go
@@ -71,8 +71,6 @@ func DummyRubiconServer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	fmt.Println("Request", string(body))
-
 	var breq openrtb.BidRequest
 	err = json.Unmarshal(body, &breq)
 	if err != nil {
@@ -939,8 +937,6 @@ func CreatePrebidRequest(server *httptest.Server, t *testing.T) (an *RubiconAdap
 		t.Fatalf("Json encoding failed: %v", err)
 	}
 
-	fmt.Println("body", body)
-
 	req := httptest.NewRequest("POST", server.URL, body)
 	req.Header.Add("Referer", rubidata.page)
 	req.Header.Add("User-Agent", rubidata.deviceUA)

diff --git a/adapters/sovrn/sovrn.go b/adapters/sovrn/sovrn.go
@@ -41,7 +41,7 @@ func (s *SovrnAdapter) SkipNoCookies() bool {
 // Call send bid requests to sovrn and receive responses
 func (s *SovrnAdapter) Call(ctx context.Context, req *pbs.PBSRequest, bidder *pbs.PBSBidder) (pbs.PBSBidSlice, error) {
 	supportedMediaTypes := []pbs.MediaType{pbs.MEDIA_TYPE_BANNER}
-	sReq, err := adapters.MakeOpenRTBGeneric(req, bidder, s.FamilyName(), supportedMediaTypes, true)
+	sReq, err := adapters.MakeOpenRTBGeneric(req, bidder, s.FamilyName(), supportedMediaTypes)
 
 	if err != nil {
 		return nil, err