From c8764eda8426c00b741b522042f2680c012acd58 Mon Sep 17 00:00:00 2001 From: Ruslan Date: Mon, 20 May 2019 19:42:45 +0300 Subject: [PATCH] Fix Rubicon bidder for multi-format impression processing (#902) --- adapters/rubicon/rubicon.go | 22 +++++- adapters/rubicon/rubicon_test.go | 116 +++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/adapters/rubicon/rubicon.go b/adapters/rubicon/rubicon.go index efd63d831c5..678912ca8a7 100644 --- a/adapters/rubicon/rubicon.go +++ b/adapters/rubicon/rubicon.go @@ -612,11 +612,13 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapters. request.Device = &deviceCopy } - if thisImp.Video != nil { + isVideo := isVideo(thisImp) + if isVideo { videoCopy := *thisImp.Video videoExt := rubiconVideoExt{Skip: rubiconExt.Video.Skip, SkipDelay: rubiconExt.Video.SkipDelay, RP: rubiconVideoExtRP{SizeID: rubiconExt.Video.VideoSizeID}} videoCopy.Ext, err = json.Marshal(&videoExt) thisImp.Video = &videoCopy + thisImp.Banner = nil } else { primarySizeID, altSizeIDs, err := parseRubiconSizes(thisImp.Banner.Format) if err != nil { @@ -631,6 +633,7 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapters. continue } thisImp.Banner = &bannerCopy + thisImp.Video = nil } siteExt := rubiconSiteExt{RP: rubiconSiteExtRP{SiteID: rubiconExt.SiteId}} @@ -673,6 +676,20 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapters. return requestData, errs } +func isVideo(imp openrtb.Imp) bool { + video := imp.Video + if video != nil { + // Do any other media types exist? Or check required video fields. + return imp.Banner == nil || isFullyPopulatedVideo(video) + } + return false +} + +func isFullyPopulatedVideo(video *openrtb.Video) bool { + // These are just recommended video fields for XAPI + return video.MIMEs != nil && video.Protocols != nil && video.MaxDuration != 0 && video.Linearity != 0 && video.API != nil +} + func (a *RubiconAdapter) MakeBids(internalRequest *openrtb.BidRequest, externalRequest *adapters.RequestData, response *adapters.ResponseData) (*adapters.BidderResponse, []error) { if response.StatusCode == http.StatusNoContent { return nil, nil @@ -706,7 +723,8 @@ func (a *RubiconAdapter) MakeBids(internalRequest *openrtb.BidRequest, externalR bidType := openrtb_ext.BidTypeBanner - if bidReq.Imp[0].Video != nil { + isVideo := isVideo(bidReq.Imp[0]) + if isVideo { bidType = openrtb_ext.BidTypeVideo } diff --git a/adapters/rubicon/rubicon_test.go b/adapters/rubicon/rubicon_test.go index 31ecea55eca..5b2752aa9e2 100644 --- a/adapters/rubicon/rubicon_test.go +++ b/adapters/rubicon/rubicon_test.go @@ -1134,6 +1134,122 @@ func TestOpenRTBRequest(t *testing.T) { } } +func TestOpenRTBRequestWithBannerImpEvenIfImpHasVideo(t *testing.T) { + SIZE_ID := getTestSizes() + bidder := new(RubiconAdapter) + + request := &openrtb.BidRequest{ + ID: "test-request-id", + Imp: []openrtb.Imp{{ + ID: "test-imp-id", + Banner: &openrtb.Banner{ + Format: []openrtb.Format{ + SIZE_ID[15], + SIZE_ID[10], + }, + }, + Video: &openrtb.Video{ + W: 640, + H: 360, + MIMEs: []string{"video/mp4"}, + }, + Ext: json.RawMessage(`{"bidder": { + "zoneId": 8394, + "siteId": 283282, + "accountId": 7891, + "inventory": {"key1" : "val1"}, + "visitor": {"key2" : "val2"} + }}`), + }}, + } + + reqs, errs := bidder.MakeRequests(request) + + if len(errs) > 0 { + t.Errorf("Got unexpected errors while building HTTP requests: %v", errs) + } + if len(reqs) != 1 { + t.Fatalf("Unexpected number of HTTP requests. Got %d. Expected %d", len(reqs), 1) + } + + rubiconReq := &openrtb.BidRequest{} + if err := json.Unmarshal(reqs[0].Body, rubiconReq); err != nil { + t.Errorf("Unexpected error while decoding request: %s", err) + } + + if len(rubiconReq.Imp) != 1 { + t.Fatalf("Unexpected number of request impressions. Got %d. Expected %d", len(rubiconReq.Imp), 1) + } + + if rubiconReq.Imp[0].Video != nil { + t.Error("Unexpected video object in request impression") + } + + if rubiconReq.Imp[0].Banner == nil { + t.Error("Banner object must be in request impression") + } +} + +func TestOpenRTBRequestWithVideoImpEvenIfImpHasBannerButAllRequiredVideoFields(t *testing.T) { + SIZE_ID := getTestSizes() + bidder := new(RubiconAdapter) + + request := &openrtb.BidRequest{ + ID: "test-request-id", + Imp: []openrtb.Imp{{ + ID: "test-imp-id", + Banner: &openrtb.Banner{ + Format: []openrtb.Format{ + SIZE_ID[15], + SIZE_ID[10], + }, + }, + Video: &openrtb.Video{ + W: 640, + H: 360, + MIMEs: []string{"video/mp4"}, + Protocols: []openrtb.Protocol{openrtb.ProtocolVAST10}, + MaxDuration: 30, + Linearity: 1, + API: []openrtb.APIFramework{}, + }, + Ext: json.RawMessage(`{"bidder": { + "zoneId": 8394, + "siteId": 283282, + "accountId": 7891, + "inventory": {"key1" : "val1"}, + "visitor": {"key2" : "val2"} + }}`), + }}, + } + + reqs, errs := bidder.MakeRequests(request) + + if len(errs) > 0 { + t.Errorf("Got unexpected errors while building HTTP requests: %v", errs) + } + if len(reqs) != 1 { + t.Fatalf("Unexpected number of HTTP requests. Got %d. Expected %d", len(reqs), 1) + } + + rubiconReq := &openrtb.BidRequest{} + if err := json.Unmarshal(reqs[0].Body, rubiconReq); err != nil { + t.Errorf("Unexpected error while decoding request: %s", err) + } + + if len(rubiconReq.Imp) != 1 { + t.Fatalf("Unexpected number of request impressions. Got %d. Expected %d", len(rubiconReq.Imp), 1) + } + + if rubiconReq.Imp[0].Banner != nil { + t.Error("Unexpected banner object in request impression") + } + + if rubiconReq.Imp[0].Video == nil { + t.Error("Video object must be in request impression") + } +} + func TestOpenRTBEmptyResponse(t *testing.T) { httpResp := &adapters.ResponseData{ StatusCode: http.StatusNoContent,