From e4b2458b49f6f0c811ac7824115c085422bb1672 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 11 Mar 2020 12:32:30 -0700 Subject: [PATCH 1/3] If Device.UA is not present in request body, init it with user-agent from request header if it's present --- endpoints/openrtb2/video_auction.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index feb8de193e7..d39bcb6f4d7 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -140,6 +140,14 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re //create full open rtb req from full video request mergeData(videoBidReq, bidReq) + //if Device.UA is not present in request body, init it with user-agent from request header if it's present + if videoBidReq.Device.UA == "" { + userAgent := r.Header.Get("User-Agent") + if userAgent != "" { + videoBidReq.Device.UA = userAgent + } + } + initialPodNumber := len(videoBidReq.PodConfig.Pods) if len(podErrors) > 0 { //remove incorrect pods From 96c1ccb75a7b08e4501b032eeccac4c6b8cc73b8 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 11 Mar 2020 14:07:12 -0700 Subject: [PATCH 2/3] Moved User-Agent handler to parseVideoRequest func and added unit test --- ...o_valid_sample_with_device_user_agent.json | 80 +++++++++++++++++++ ...alid_sample_without_device_user_agent.json | 63 +++++++++++++++ endpoints/openrtb2/video_auction.go | 20 ++--- endpoints/openrtb2/video_auction_test.go | 75 +++++++++++++++++ 4 files changed, 228 insertions(+), 10 deletions(-) create mode 100644 endpoints/openrtb2/sample-requests/video/video_valid_sample_with_device_user_agent.json create mode 100644 endpoints/openrtb2/sample-requests/video/video_valid_sample_without_device_user_agent.json diff --git a/endpoints/openrtb2/sample-requests/video/video_valid_sample_with_device_user_agent.json b/endpoints/openrtb2/sample-requests/video/video_valid_sample_with_device_user_agent.json new file mode 100644 index 00000000000..68c3f4e1c15 --- /dev/null +++ b/endpoints/openrtb2/sample-requests/video/video_valid_sample_with_device_user_agent.json @@ -0,0 +1,80 @@ + +{ + "accountid": "555888777", + "podconfig": { + "durationrangesec": [ + 30 + ], + "requireexactduration": true, + "pods": [ + { + "podid": 1, + "adpoddurationsec": 180, + "configid": "fba10607-0c12-43d1-ad07-b8a513bc75d6" + }, + { + "podid": 2, + "adpoddurationsec": 150, + "configid": "8b452b41-2681-4a20-9086-6f16ffad7773" + } + ] + }, + "site": { + "page": "prebid.com" + }, + "user": { + "buyeruids": { + "appnexus": "unique_id_an", + "rubicon": "unique_id_rubi" + }, + "gdpr": { + "consentrequired": false, + "consentstring": "something" + }, + "yob": 1991, + "gender": "F", + "keywords": "Hotels, Travelling" + }, + "device": { + "ua": "TestHeaderSample", + "ip": "123.145.167.10", + "devicetype": 1, + "dnt": 33, + "ifa": "AA000DFE74168477C70D291f574D344790E0BB11", + "lmt": 44, + "os": "mac os", + "w": 640, + "h": 480, + "didsha1": "didsha1", + "didmd5": "didmd5", + "dpidsha1": "dpidsha1", + "dpidmd5": "dpidmd5", + "macsha1": "macsha1", + "macmd5": "macmd5" + }, + "includebrandcategory":{ + "primaryadserver": 1, + "publisher": "" + }, + "video": { + "w": 640, + "h": 480, + "mimes": [ + "video/mp4" + ], + "protocols": [ + 2,3,5,6 + ] + }, + "content": { + "episode": 6, + "title": "episodeName", + "series": "TvName", + "season": "season3", + "len": 900, + "livestream": 0 + }, + "cacheconfig": { + "ttl": 42 + } +} diff --git a/endpoints/openrtb2/sample-requests/video/video_valid_sample_without_device_user_agent.json b/endpoints/openrtb2/sample-requests/video/video_valid_sample_without_device_user_agent.json new file mode 100644 index 00000000000..e040a5625ba --- /dev/null +++ b/endpoints/openrtb2/sample-requests/video/video_valid_sample_without_device_user_agent.json @@ -0,0 +1,63 @@ + +{ + "accountid": "555888777", + "podconfig": { + "durationrangesec": [ + 30 + ], + "requireexactduration": true, + "pods": [ + { + "podid": 1, + "adpoddurationsec": 180, + "configid": "fba10607-0c12-43d1-ad07-b8a513bc75d6" + }, + { + "podid": 2, + "adpoddurationsec": 150, + "configid": "8b452b41-2681-4a20-9086-6f16ffad7773" + } + ] + }, + "site": { + "page": "prebid.com" + }, + "user": { + "buyeruids": { + "appnexus": "unique_id_an", + "rubicon": "unique_id_rubi" + }, + "gdpr": { + "consentrequired": false, + "consentstring": "something" + }, + "yob": 1991, + "gender": "F", + "keywords": "Hotels, Travelling" + }, + "includebrandcategory":{ + "primaryadserver": 1, + "publisher": "" + }, + "video": { + "w": 640, + "h": 480, + "mimes": [ + "video/mp4" + ], + "protocols": [ + 2,3,5,6 + ] + }, + "content": { + "episode": 6, + "title": "episodeName", + "series": "TvName", + "season": "season3", + "len": 900, + "livestream": 0 + }, + "cacheconfig": { + "ttl": 42 + } +} diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index d39bcb6f4d7..be3429efa2b 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -120,7 +120,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re } } //unmarshal and validate combined result - videoBidReq, errL, podErrors := deps.parseVideoRequest(resolvedRequest) + videoBidReq, errL, podErrors := deps.parseVideoRequest(resolvedRequest, r.Header) if len(errL) > 0 { handleError(&labels, w, errL, &vo) return @@ -140,14 +140,6 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re //create full open rtb req from full video request mergeData(videoBidReq, bidReq) - //if Device.UA is not present in request body, init it with user-agent from request header if it's present - if videoBidReq.Device.UA == "" { - userAgent := r.Header.Get("User-Agent") - if userAgent != "" { - videoBidReq.Device.UA = userAgent - } - } - initialPodNumber := len(videoBidReq.PodConfig.Pods) if len(podErrors) > 0 { //remove incorrect pods @@ -564,7 +556,7 @@ func createBidExtension(videoRequest *openrtb_ext.BidRequestVideo) ([]byte, erro return reqJSON, nil } -func (deps *endpointDeps) parseVideoRequest(request []byte) (req *openrtb_ext.BidRequestVideo, errs []error, podErrors []PodError) { +func (deps *endpointDeps) parseVideoRequest(request []byte, headers http.Header) (req *openrtb_ext.BidRequestVideo, errs []error, podErrors []PodError) { req = &openrtb_ext.BidRequestVideo{} if err := json.Unmarshal(request, &req); err != nil { @@ -572,6 +564,14 @@ func (deps *endpointDeps) parseVideoRequest(request []byte) (req *openrtb_ext.Bi return } + //if Device.UA is not present in request body, init it with user-agent from request header if it's present + if req.Device.UA == "" { + userAgent := headers.Get("User-Agent") + if userAgent != "" { + req.Device.UA = userAgent + } + } + errL, podErrors := deps.validateVideoRequest(req) if len(errL) > 0 { errs = append(errs, errL...) diff --git a/endpoints/openrtb2/video_auction_test.go b/endpoints/openrtb2/video_auction_test.go index dfe2a6a50b8..a5ad62c9fa8 100644 --- a/endpoints/openrtb2/video_auction_test.go +++ b/endpoints/openrtb2/video_auction_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "io/ioutil" + "net/http" "net/http/httptest" "strings" "testing" @@ -745,6 +746,80 @@ func TestHandleErrorMetrics(t *testing.T) { assert.Equal(t, "request missing required field: PodConfig.Pods", mod.videoObjects[0].Errors[1].Error(), "Second error in AnalyticsObject should have message regarding Pods") } +func TestParseVideoRequestWithUserAgentAndHeader(t *testing.T) { + ex := &mockExchangeVideo{} + reqData, err := ioutil.ReadFile("sample-requests/video/video_valid_sample_with_device_user_agent.json") + if err != nil { + t.Fatalf("Failed to fetch a valid request: %v", err) + } + + headers := http.Header{} + headers.Add("User-Agent", "TestHeader") + + deps := mockDeps(t, ex) + req, valErr, podErr := deps.parseVideoRequest(reqData, headers) + + assert.Equal(t, "TestHeaderSample", req.Device.UA, "Header should be taken from original request") + assert.Equal(t, []error(nil), valErr, "No validation errors should be returned") + assert.Equal(t, make([]PodError, 0), podErr, "No pod errors should be returned") + +} + +func TestParseVideoRequestWithUserAgentAndEmptyHeader(t *testing.T) { + ex := &mockExchangeVideo{} + reqData, err := ioutil.ReadFile("sample-requests/video/video_valid_sample_with_device_user_agent.json") + if err != nil { + t.Fatalf("Failed to fetch a valid request: %v", err) + } + + headers := http.Header{} + + deps := mockDeps(t, ex) + req, valErr, podErr := deps.parseVideoRequest(reqData, headers) + + assert.Equal(t, "TestHeaderSample", req.Device.UA, "Header should be taken from original request") + assert.Equal(t, []error(nil), valErr, "No validation errors should be returned") + assert.Equal(t, make([]PodError, 0), podErr, "No pod errors should be returned") + +} + +func TestParseVideoRequestWithoutUserAgentWithHeader(t *testing.T) { + ex := &mockExchangeVideo{} + reqData, err := ioutil.ReadFile("sample-requests/video/video_valid_sample_without_device_user_agent.json") + if err != nil { + t.Fatalf("Failed to fetch a valid request: %v", err) + } + + headers := http.Header{} + headers.Add("User-Agent", "TestHeader") + + deps := mockDeps(t, ex) + req, valErr, podErr := deps.parseVideoRequest(reqData, headers) + + assert.Equal(t, "TestHeader", req.Device.UA, "Device.ua should be taken from request header") + assert.Equal(t, []error(nil), valErr, "No validation errors should be returned") + assert.Equal(t, make([]PodError, 0), podErr, "No pod errors should be returned") + +} + +func TestParseVideoRequestWithoutUserAgentAndEmptyHeader(t *testing.T) { + ex := &mockExchangeVideo{} + reqData, err := ioutil.ReadFile("sample-requests/video/video_valid_sample_without_device_user_agent.json") + if err != nil { + t.Fatalf("Failed to fetch a valid request: %v", err) + } + + headers := http.Header{} + + deps := mockDeps(t, ex) + req, valErr, podErr := deps.parseVideoRequest(reqData, headers) + + assert.Equal(t, "", req.Device.UA, "Device.ua should be empty") + assert.Equal(t, []error(nil), valErr, "No validation errors should be returned") + assert.Equal(t, make([]PodError, 0), podErr, "No pod errors should be returned") + +} + func mockDepsWithMetrics(t *testing.T, ex *mockExchangeVideo) (*endpointDeps, *pbsmetrics.Metrics, *mockAnalyticsModule) { theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{}) mockModule := &mockAnalyticsModule{} From 98949713596546c9b7f2b2868ab4b7e7208c2388 Mon Sep 17 00:00:00 2001 From: Veronika Solovei Date: Wed, 11 Mar 2020 14:31:03 -0700 Subject: [PATCH 3/3] Minor clean up --- endpoints/openrtb2/video_auction.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index be3429efa2b..2a8663959a6 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -566,10 +566,7 @@ func (deps *endpointDeps) parseVideoRequest(request []byte, headers http.Header) //if Device.UA is not present in request body, init it with user-agent from request header if it's present if req.Device.UA == "" { - userAgent := headers.Get("User-Agent") - if userAgent != "" { - req.Device.UA = userAgent - } + req.Device.UA = headers.Get("User-Agent") } errL, podErrors := deps.validateVideoRequest(req)