From e92f00287fe3a26bc1599edc6656fb1e91392731 Mon Sep 17 00:00:00 2001 From: pm-avinash-kapre <112699665+AvinashKapre@users.noreply.github.com> Date: Tue, 20 Feb 2024 12:13:11 +0530 Subject: [PATCH 1/3] OTT-1637: Fix- correct NBR for inapp request (#716) --- modules/pubmatic/openwrap/entrypointhook.go | 33 ++-- .../pubmatic/openwrap/entrypointhook_test.go | 146 ++++++++++++++++++ modules/pubmatic/openwrap/models/constants.go | 29 ++-- modules/pubmatic/vastunwrap/entryhook.go | 2 +- 4 files changed, 182 insertions(+), 28 deletions(-) diff --git a/modules/pubmatic/openwrap/entrypointhook.go b/modules/pubmatic/openwrap/entrypointhook.go index bfef6288f55..51d493f0b0f 100644 --- a/modules/pubmatic/openwrap/entrypointhook.go +++ b/modules/pubmatic/openwrap/entrypointhook.go @@ -53,16 +53,16 @@ func (m OpenWrap) handleEntrypointHook( if queryParams.Get("sshb") == "1" { return result, nil } - - requestExtWrapper, err = GetRequestWrapper(payload, result) endpoint = GetEndpoint(payload.Request.URL.Path, source) if endpoint == models.EndpointHybrid { rCtx.Endpoint = models.EndpointHybrid return result, nil } + // init default for all modules result.Reject = true + requestExtWrapper, err = GetRequestWrapper(payload, result, endpoint) if err != nil { result.NbrCode = nbr.InvalidRequestWrapperExtension result.Errors = append(result.Errors, err.Error()) @@ -134,18 +134,25 @@ func (m OpenWrap) handleEntrypointHook( result.Reject = false return result, nil } -func GetRequestWrapper(payload hookstage.EntrypointPayload, result hookstage.HookResult[hookstage.EntrypointPayload]) (models.RequestExtWrapper, error) { - requestExtWrapper, err := models.GetRequestExtWrapper(payload.Body, "ext", "wrapper") - if err != nil { + +func GetRequestWrapper(payload hookstage.EntrypointPayload, result hookstage.HookResult[hookstage.EntrypointPayload], endpoint string) (models.RequestExtWrapper, error) { + var requestExtWrapper models.RequestExtWrapper + var err error + switch endpoint { + case models.EndpintInappVideo: + requestExtWrapper, err = v25.ConvertVideoToAuctionRequest(payload, &result) + case models.EndpointAMP: + requestExtWrapper, err = models.GetQueryParamRequestExtWrapper(payload.Request) + case models.EndpointV25: + fallthrough + case models.EndpointVideo, models.EndpointVAST, models.EndpointJson: + requestExtWrapper, err = models.GetRequestExtWrapper(payload.Body, "ext", "wrapper") + case models.EndpointWebS2S: + fallthrough + default: requestExtWrapper, err = models.GetRequestExtWrapper(payload.Body) - if err != nil { - requestExtWrapper, err = models.GetQueryParamRequestExtWrapper(payload.Request) - //update this code once we fully support modular code for all endpoint - if err != nil { - requestExtWrapper, err = v25.ConvertVideoToAuctionRequest(payload, &result) - } - } } + return requestExtWrapper, err } @@ -165,7 +172,7 @@ func GetEndpoint(path, source string) string { case OpenWrapV25: return models.EndpointV25 case OpenWrapV25Video: - return models.EndpointVideo + return models.EndpintInappVideo case OpenWrapAmp: return models.EndpointAMP case OpenWrapOpenRTBVideo: diff --git a/modules/pubmatic/openwrap/entrypointhook_test.go b/modules/pubmatic/openwrap/entrypointhook_test.go index bfb48f3826b..aee2e5af9bd 100644 --- a/modules/pubmatic/openwrap/entrypointhook_test.go +++ b/modules/pubmatic/openwrap/entrypointhook_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/golang/mock/gomock" + "github.com/prebid/prebid-server/hooks/hookexecution" "github.com/prebid/prebid-server/hooks/hookstage" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/cache" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/config" @@ -529,3 +530,148 @@ func TestOpenWrap_handleEntrypointHook(t *testing.T) { }) } } + +func TestGetRequestWrapper(t *testing.T) { + type args struct { + payload hookstage.EntrypointPayload + result hookstage.HookResult[hookstage.EntrypointPayload] + endpoint string + } + tests := []struct { + name string + args args + want models.RequestExtWrapper + wantErr bool + }{ + { + name: "EndpointWebS2S", + args: args{ + payload: hookstage.EntrypointPayload{ + Body: []byte(`{"ext":{"wrapper":{"profileid":5890,"versionid":1},"prebid":{"bidderparams":{"pubmatic":{"wrapper":{"profileid":100,"versionid":2}}}}}}`), + }, + result: hookstage.HookResult[hookstage.EntrypointPayload]{}, + endpoint: models.EndpointWebS2S, + }, + want: models.RequestExtWrapper{ + SSAuctionFlag: -1, + ProfileId: 100, + VersionId: 2, + }, + wantErr: false, + }, + { + name: "EndpointV25", + args: args{ + payload: hookstage.EntrypointPayload{ + Body: []byte(`{"ext":{"wrapper":{"profileid":5890,"versionid":1},"prebid":{"bidderparams":{"pubmatic":{"wrapper":{"profileid":100,"versionid":2}}}}}}`), + }, + result: hookstage.HookResult[hookstage.EntrypointPayload]{}, + endpoint: models.EndpointV25, + }, + want: models.RequestExtWrapper{ + SSAuctionFlag: -1, + ProfileId: 5890, + VersionId: 1, + }, + }, + //TODO: Add test cases for other endpoints after migration(AMP, Video, VAST, JSON, InappVideo) + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetRequestWrapper(tt.args.payload, tt.args.result, tt.args.endpoint) + assert.Equal(t, err != nil, tt.wantErr) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestGetEndpoint(t *testing.T) { + type args struct { + path string + source string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "EndpointAuction Activation", + args: args{ + path: hookexecution.EndpointAuction, + source: "pbjs", + }, + want: models.EndpointWebS2S, + }, + { + name: "EndpointAuction inapp", + args: args{ + path: hookexecution.EndpointAuction, + source: "owsdk", + }, + want: models.EndpointV25, + }, + { + name: "EndpointAuction default", + args: args{ + path: hookexecution.EndpointAuction, + }, + want: models.EndpointHybrid, + }, + { + name: "OpenWrapAuction", + args: args{ + path: OpenWrapAuction, + }, + want: models.EndpointHybrid, + }, + { + name: "OpenWrapV25", + args: args{ + path: OpenWrapV25, + }, + want: models.EndpointV25, + }, + { + name: "OpenWrapV25Video", + args: args{ + path: OpenWrapV25Video, + }, + want: models.EndpintInappVideo, + }, + { + name: "OpenWrapAmp", + args: args{ + path: OpenWrapAmp, + }, + want: models.EndpointAMP, + }, + { + name: "OpenWrapOpenRTBVideo", + args: args{ + path: OpenWrapOpenRTBVideo, + }, + want: models.EndpointVideo, + }, + { + name: "OpenWrapVAST", + args: args{ + path: OpenWrapVAST, + }, + want: models.EndpointVAST, + }, + { + name: "OpenWrapJSON", + args: args{ + path: OpenWrapJSON, + }, + want: models.EndpointJson, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := GetEndpoint(tt.args.path, tt.args.source) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/modules/pubmatic/openwrap/models/constants.go b/modules/pubmatic/openwrap/models/constants.go index d68fd411ecc..78be2effa15 100755 --- a/modules/pubmatic/openwrap/models/constants.go +++ b/modules/pubmatic/openwrap/models/constants.go @@ -423,20 +423,21 @@ const ( ) const ( - EndpointV25 = "v25" - EndpointAMP = "amp" - EndpointVideo = "video" - EndpointJson = "json" - EndpointORTB = "ortb" - EndpointVAST = "vast" - EndpointWebS2S = "webs2s" - EndPointCTV = "ctv" - EndpointHybrid = "hybrid" - Openwrap = "openwrap" - ImpTypeBanner = "banner" - ImpTypeVideo = "video" - ContentTypeSite = "site" - ContentTypeApp = "app" + EndpointV25 = "v25" + EndpointAMP = "amp" + EndpintInappVideo = "inappvideo" + EndpointVideo = "video" + EndpointJson = "json" + EndpointORTB = "ortb" + EndpointVAST = "vast" + EndpointWebS2S = "webs2s" + EndPointCTV = "ctv" + EndpointHybrid = "hybrid" + Openwrap = "openwrap" + ImpTypeBanner = "banner" + ImpTypeVideo = "video" + ContentTypeSite = "site" + ContentTypeApp = "app" ) const ( diff --git a/modules/pubmatic/vastunwrap/entryhook.go b/modules/pubmatic/vastunwrap/entryhook.go index 7eeb5707f4f..52cf86435e5 100644 --- a/modules/pubmatic/vastunwrap/entryhook.go +++ b/modules/pubmatic/vastunwrap/entryhook.go @@ -40,7 +40,7 @@ func handleEntrypointHook( } } else { endpoint := openwrap.GetEndpoint(payload.Request.URL.Path, source) - requestExtWrapper, _ := openwrap.GetRequestWrapper(payload, result) + requestExtWrapper, _ := openwrap.GetRequestWrapper(payload, result, endpoint) vastRequestContext = models.RequestCtx{ ProfileID: requestExtWrapper.ProfileId, DisplayID: requestExtWrapper.VersionId, From 3ab68248124c977228999dcba4d0c55404036e9b Mon Sep 17 00:00:00 2001 From: ashishshinde-pubm <109787960+ashishshinde-pubm@users.noreply.github.com> Date: Tue, 20 Feb 2024 16:43:30 +0530 Subject: [PATCH 2/3] OTT-1632: OW Bug Fix owlogger in case of invalid-configuration (#713) Co-authored-by: Avinash Kapre --- .../pubmatic/openwrap/beforevalidationhook.go | 4 + .../openwrap/beforevalidationhook_test.go | 294 +++++++++++++++++- modules/pubmatic/openwrap/logger_test.go | 145 +++++++++ 3 files changed, 442 insertions(+), 1 deletion(-) create mode 100644 modules/pubmatic/openwrap/logger_test.go diff --git a/modules/pubmatic/openwrap/beforevalidationhook.go b/modules/pubmatic/openwrap/beforevalidationhook.go index cd0b02d7f60..6e5faa38033 100644 --- a/modules/pubmatic/openwrap/beforevalidationhook.go +++ b/modules/pubmatic/openwrap/beforevalidationhook.go @@ -114,6 +114,7 @@ func (m OpenWrap) handleBeforeValidationHook( err = errors.New("failed to get profile data: received empty data") } result.Errors = append(result.Errors, err.Error()) + rCtx.ImpBidCtx = getDefaultImpBidCtx(*payload.BidRequest) // for wrapper logger sz m.metricEngine.RecordPublisherInvalidProfileRequests(rCtx.Endpoint, rCtx.PubIDStr, rCtx.ProfileIDStr) m.metricEngine.RecordPublisherInvalidProfileImpressions(rCtx.PubIDStr, rCtx.ProfileIDStr, len(payload.BidRequest.Imp)) return result, err @@ -128,6 +129,7 @@ func (m OpenWrap) handleBeforeValidationHook( result.NbrCode = nbr.InvalidPlatform err = errors.New("failed to get platform data") result.Errors = append(result.Errors, err.Error()) + rCtx.ImpBidCtx = getDefaultImpBidCtx(*payload.BidRequest) // for wrapper logger sz m.metricEngine.RecordPublisherInvalidProfileRequests(rCtx.Endpoint, rCtx.PubIDStr, rCtx.ProfileIDStr) m.metricEngine.RecordPublisherInvalidProfileImpressions(rCtx.PubIDStr, rCtx.ProfileIDStr, len(payload.BidRequest.Imp)) return result, err @@ -195,6 +197,7 @@ func (m OpenWrap) handleBeforeValidationHook( result.NbrCode = nbr.InternalError err = errors.New("failed to parse imp.ext: " + imp.ID) result.Errors = append(result.Errors, err.Error()) + rCtx.ImpBidCtx = map[string]models.ImpCtx{} // do not create "s" object in owlogger return result, err } } @@ -205,6 +208,7 @@ func (m OpenWrap) handleBeforeValidationHook( result.NbrCode = nbr.InvalidImpressionTagID err = errors.New("tagid missing for imp: " + imp.ID) result.Errors = append(result.Errors, err.Error()) + rCtx.ImpBidCtx = map[string]models.ImpCtx{} // do not create "s" object in owlogger return result, err } diff --git a/modules/pubmatic/openwrap/beforevalidationhook_test.go b/modules/pubmatic/openwrap/beforevalidationhook_test.go index 5acfee84083..9a947eccbd4 100644 --- a/modules/pubmatic/openwrap/beforevalidationhook_test.go +++ b/modules/pubmatic/openwrap/beforevalidationhook_test.go @@ -2481,7 +2481,6 @@ func TestOpenWrap_handleBeforeValidationHook(t *testing.T) { "adunit@700x900": "1232433543534543", }, }) - //prometheus metrics mockEngine.EXPECT().RecordPublisherProfileRequests("5890", "1234") mockEngine.EXPECT().RecordPublisherRequests(rctx.Endpoint, "5890", rctx.Platform) @@ -2978,6 +2977,299 @@ func TestUserAgent_handleBeforeValidationHook(t *testing.T) { } } +func TestImpBidCtx_handleBeforeValidationHook(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockCache := mock_cache.NewMockCache(ctrl) + mockEngine := mock_metrics.NewMockMetricsEngine(ctrl) + type fields struct { + cfg config.Config + cache cache.Cache + metricEngine metrics.MetricsEngine + } + type args struct { + ctx context.Context + moduleCtx hookstage.ModuleInvocationContext + payload hookstage.BeforeValidationRequestPayload + bidrequest json.RawMessage + } + type want struct { + rctx *models.RequestCtx + error bool + } + tests := []struct { + name string + fields fields + args args + want want + setup func() + }{ + { + name: "default_impctx_if_getProfileData_fails", + args: args{ + ctx: context.Background(), + moduleCtx: hookstage.ModuleInvocationContext{ + ModuleContext: hookstage.ModuleContext{ + "rctx": rctx, + }, + }, + bidrequest: json.RawMessage(`{"id":"123-456-789","imp":[{"id":"123","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"tagid":"adunit","ext":{"wrapper":{"div":"div"},"bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}}],"site":{"domain":"test.com","page":"www.test.com","publisher":{"id":"5890"}},"device":{"ua":"Mozilla/5.0(X11;Linuxx86_64)AppleWebKit/537.36(KHTML,likeGecko)Chrome/52.0.2743.82Safari/537.36","ip":"123.145.167.10"},"user":{"id":"119208432","buyeruid":"1rwe432","yob":1980,"gender":"F","geo":{"country":"US","region":"CA","metro":"90001","city":"Alamo"}},"wseat":["Wseat_0","Wseat_1"],"bseat":["Bseat_0","Bseat_1"],"cur":["cur_0","cur_1"],"wlang":["Wlang_0","Wlang_1"],"bcat":["bcat_0","bcat_1"],"badv":["badv_0","badv_1"],"bapp":["bapp_0","bapp_1"],"source":{"ext":{"omidpn":"MyIntegrationPartner","omidpv":"7.1"}},"ext":{"prebid":{},"wrapper":{"test":123,"profileid":123,"versionid":1,"wiid":"test_display_wiid"}}}`), + }, + fields: fields{ + cache: mockCache, + metricEngine: mockEngine, + }, + setup: func() { + mockCache.EXPECT().GetPartnerConfigMap(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(map[int]map[string]string{ + 2: { + models.PARTNER_ID: "2", + models.PREBID_PARTNER_NAME: "appnexus", + models.BidderCode: "appnexus", + models.SERVER_SIDE_FLAG: "1", + models.KEY_GEN_PATTERN: "_AU_@_W_x_H_", + models.TIMEOUT: "200", + }, + -1: { + models.PLATFORM_KEY: models.PLATFORM_APP, + models.DisplayVersionID: "1", + }, + }, errors.New("test")) + mockEngine.EXPECT().RecordPublisherProfileRequests("5890", "1234") + mockEngine.EXPECT().RecordBadRequests(rctx.Endpoint, getPubmaticErrorCode(nbr.InvalidProfileConfiguration)) + mockEngine.EXPECT().RecordNobidErrPrebidServerRequests("5890", nbr.InvalidProfileConfiguration) + mockEngine.EXPECT().RecordPublisherInvalidProfileRequests(rctx.Endpoint, "5890", rctx.ProfileIDStr) + mockEngine.EXPECT().RecordPublisherInvalidProfileImpressions("5890", rctx.ProfileIDStr, gomock.Any()) + }, + want: want{ + rctx: &models.RequestCtx{ + ImpBidCtx: map[string]models.ImpCtx{ + "123": { + IncomingSlots: []string{ + "640x480v", + }, + SlotName: "adunit", + AdUnitName: "adunit", + }, + }, + }, + error: true, + }, + }, + { + name: "default_impctx_if_platform_is_missing", + args: args{ + ctx: context.Background(), + moduleCtx: hookstage.ModuleInvocationContext{ + ModuleContext: hookstage.ModuleContext{ + "rctx": rctx, + }, + }, + bidrequest: json.RawMessage(`{"id":"123-456-789","imp":[{"id":"123","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"tagid":"adunit","ext":{"wrapper":{"div":"div"},"bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}}],"site":{"domain":"test.com","page":"www.test.com","publisher":{"id":"5890"}},"device":{"ua":"Mozilla/5.0(X11;Linuxx86_64)AppleWebKit/537.36(KHTML,likeGecko)Chrome/52.0.2743.82Safari/537.36","ip":"123.145.167.10"},"user":{"id":"119208432","buyeruid":"1rwe432","yob":1980,"gender":"F","geo":{"country":"US","region":"CA","metro":"90001","city":"Alamo"}},"wseat":["Wseat_0","Wseat_1"],"bseat":["Bseat_0","Bseat_1"],"cur":["cur_0","cur_1"],"wlang":["Wlang_0","Wlang_1"],"bcat":["bcat_0","bcat_1"],"badv":["badv_0","badv_1"],"bapp":["bapp_0","bapp_1"],"source":{"ext":{"omidpn":"MyIntegrationPartner","omidpv":"7.1"}},"ext":{"prebid":{},"wrapper":{"test":123,"profileid":123,"versionid":1,"wiid":"test_display_wiid"}}}`), + }, + fields: fields{ + cache: mockCache, + metricEngine: mockEngine, + }, + setup: func() { + mockCache.EXPECT().GetPartnerConfigMap(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(map[int]map[string]string{ + 2: { + models.PARTNER_ID: "2", + models.PREBID_PARTNER_NAME: "appnexus", + models.BidderCode: "appnexus", + models.SERVER_SIDE_FLAG: "1", + models.KEY_GEN_PATTERN: "_AU_@_W_x_H_", + models.TIMEOUT: "200", + }, + -1: { + models.DisplayVersionID: "1", + }, + }, nil) + //prometheus metrics + mockEngine.EXPECT().RecordPublisherProfileRequests("5890", "1234") + mockEngine.EXPECT().RecordBadRequests(rctx.Endpoint, getPubmaticErrorCode(nbr.InvalidPlatform)) + mockEngine.EXPECT().RecordNobidErrPrebidServerRequests("5890", nbr.InvalidPlatform) + mockEngine.EXPECT().RecordPublisherInvalidProfileRequests(rctx.Endpoint, "5890", rctx.ProfileIDStr) + mockEngine.EXPECT().RecordPublisherInvalidProfileImpressions("5890", rctx.ProfileIDStr, gomock.Any()) + }, + want: want{ + rctx: &models.RequestCtx{ + ImpBidCtx: map[string]models.ImpCtx{ + "123": { + IncomingSlots: []string{ + "640x480v", + }, + SlotName: "adunit", + AdUnitName: "adunit", + }, + }, + }, + error: true, + }, + }, + { + name: "default_impctx_if_all_partners_throttled", + args: args{ + ctx: context.Background(), + moduleCtx: hookstage.ModuleInvocationContext{ + ModuleContext: hookstage.ModuleContext{ + "rctx": rctx, + }, + }, + bidrequest: json.RawMessage(`{"id":"123-456-789","imp":[{"id":"123","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"tagid":"adunit","ext":{"wrapper":{"div":"div"},"bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}}],"site":{"domain":"test.com","page":"www.test.com","publisher":{"id":"5890"}},"device":{"ua":"Mozilla/5.0(X11;Linuxx86_64)AppleWebKit/537.36(KHTML,likeGecko)Chrome/52.0.2743.82Safari/537.36","ip":"123.145.167.10"},"user":{"id":"119208432","buyeruid":"1rwe432","yob":1980,"gender":"F","geo":{"country":"US","region":"CA","metro":"90001","city":"Alamo"}},"wseat":["Wseat_0","Wseat_1"],"bseat":["Bseat_0","Bseat_1"],"cur":["cur_0","cur_1"],"wlang":["Wlang_0","Wlang_1"],"bcat":["bcat_0","bcat_1"],"badv":["badv_0","badv_1"],"bapp":["bapp_0","bapp_1"],"source":{"ext":{"omidpn":"MyIntegrationPartner","omidpv":"7.1"}},"ext":{"prebid":{},"wrapper":{"test":123,"profileid":123,"versionid":1,"wiid":"test_display_wiid"}}}`), + }, + fields: fields{ + cache: mockCache, + metricEngine: mockEngine, + }, + setup: func() { + mockCache.EXPECT().GetPartnerConfigMap(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(map[int]map[string]string{ + 2: { + models.PARTNER_ID: "2", + models.PREBID_PARTNER_NAME: "appnexus", + models.BidderCode: "appnexus", + models.SERVER_SIDE_FLAG: "1", + models.KEY_GEN_PATTERN: "_AU_@_W_x_H_", + models.TIMEOUT: "200", + models.THROTTLE: "0", + }, + -1: { + models.DisplayVersionID: "1", + models.PLATFORM_KEY: models.PLATFORM_APP, + }, + }, nil) + //prometheus metrics + mockEngine.EXPECT().RecordPublisherProfileRequests("5890", "1234") + mockEngine.EXPECT().RecordBadRequests(rctx.Endpoint, getPubmaticErrorCode(nbr.AllPartnerThrottled)) + mockEngine.EXPECT().RecordNobidErrPrebidServerRequests("5890", nbr.AllPartnerThrottled) + mockEngine.EXPECT().RecordPublisherRequests(rctx.Endpoint, "5890", rctx.Platform) + }, + want: want{ + error: false, + rctx: &models.RequestCtx{ + ImpBidCtx: map[string]models.ImpCtx{ + "123": { + IncomingSlots: []string{ + "640x480v", + }, + SlotName: "adunit", + AdUnitName: "adunit", + }, + }, + }, + }, + }, + { + name: "empty_impctx_if_TagID_not_present_in_imp", + args: args{ + ctx: context.Background(), + moduleCtx: hookstage.ModuleInvocationContext{ + ModuleContext: hookstage.ModuleContext{ + "rctx": rctx, + }, + }, + bidrequest: json.RawMessage(`{"id":"123-456-789","imp":[{"id":"123","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"ext":{"wrapper":{"div":"div"},"bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}},{"id":"456","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"ext":{"wrapper":{"div":"div"},"bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}}],"site":{"domain":"test.com","page":"www.test.com","publisher":{"id":"5890"}},"device":{"ua":"Mozilla/5.0(X11;Linuxx86_64)AppleWebKit/537.36(KHTML,likeGecko)Chrome/52.0.2743.82Safari/537.36","ip":"123.145.167.10"},"user":{"id":"119208432"},"source":{"ext":{"omidpn":"MyIntegrationPartner","omidpv":"7.1"}},"ext":{"prebid":{},"wrapper":{"test":123,"profileid":123,"versionid":1,"wiid":"test_display_wiid"}}}`), + }, + fields: fields{ + cache: mockCache, + metricEngine: mockEngine, + }, + setup: func() { + mockCache.EXPECT().GetPartnerConfigMap(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(map[int]map[string]string{ + 2: { + models.PARTNER_ID: "2", + models.PREBID_PARTNER_NAME: "appnexus", + models.BidderCode: "appnexus", + models.SERVER_SIDE_FLAG: "1", + models.KEY_GEN_PATTERN: "_AU_@_W_x_H_", + models.TIMEOUT: "200", + }, + -1: { + models.DisplayVersionID: "1", + models.PLATFORM_KEY: models.PLATFORM_APP, + }, + }, nil) + mockCache.EXPECT().GetAdunitConfigFromCache(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&adunitconfig.AdUnitConfig{}) + //prometheus metrics + mockEngine.EXPECT().RecordPublisherProfileRequests("5890", "1234") + mockEngine.EXPECT().RecordBadRequests(rctx.Endpoint, getPubmaticErrorCode(nbr.InvalidImpressionTagID)) + mockEngine.EXPECT().RecordNobidErrPrebidServerRequests("5890", nbr.InvalidImpressionTagID) + mockEngine.EXPECT().RecordPublisherRequests(rctx.Endpoint, "5890", rctx.Platform) + }, + want: want{ + rctx: &models.RequestCtx{ + ImpBidCtx: map[string]models.ImpCtx{}, + }, + error: true, + }, + }, + { + name: "empty_impctx_if_imp_ext_parse_fails", + args: args{ + ctx: context.Background(), + moduleCtx: hookstage.ModuleInvocationContext{ + ModuleContext: hookstage.ModuleContext{ + "rctx": rctx, + }, + }, + bidrequest: json.RawMessage(`{"id":"123-456-789","imp":[{"id":"123","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"ext":{"wrapper":"invalid","bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}},{"id":"456","video":{"mimes":["video/mp4","video/mpeg"],"w":640,"h":480},"ext":{"wrapper":{"div":"div"},"bidder":{"pubmatic":{"keywords":[{"key":"pmzoneid","value":["val1","val2"]}]}},"prebid":{}}}],"site":{"domain":"test.com","page":"www.test.com","publisher":{"id":"5890"}},"device":{"ua":"Mozilla/5.0(X11;Linuxx86_64)AppleWebKit/537.36(KHTML,likeGecko)Chrome/52.0.2743.82Safari/537.36","ip":"123.145.167.10"},"user":{"id":"119208432"},"source":{"ext":{"omidpn":"MyIntegrationPartner","omidpv":"7.1"}},"ext":{"prebid":{},"wrapper":{"test":123,"profileid":123,"versionid":1,"wiid":"test_display_wiid"}}}`), + }, + fields: fields{ + cache: mockCache, + metricEngine: mockEngine, + }, + setup: func() { + mockCache.EXPECT().GetPartnerConfigMap(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(map[int]map[string]string{ + 2: { + models.PARTNER_ID: "2", + models.PREBID_PARTNER_NAME: "appnexus", + models.BidderCode: "appnexus", + models.SERVER_SIDE_FLAG: "1", + models.KEY_GEN_PATTERN: "_AU_@_W_x_H_", + models.TIMEOUT: "200", + }, + -1: { + models.DisplayVersionID: "1", + models.PLATFORM_KEY: models.PLATFORM_APP, + }, + }, nil) + mockCache.EXPECT().GetAdunitConfigFromCache(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&adunitconfig.AdUnitConfig{}) + //prometheus metrics + mockEngine.EXPECT().RecordPublisherProfileRequests("5890", "1234") + mockEngine.EXPECT().RecordBadRequests(rctx.Endpoint, getPubmaticErrorCode(nbr.InternalError)) + mockEngine.EXPECT().RecordNobidErrPrebidServerRequests("5890", nbr.InternalError) + mockEngine.EXPECT().RecordPublisherRequests(rctx.Endpoint, "5890", rctx.Platform) + }, + want: want{ + rctx: &models.RequestCtx{ + ImpBidCtx: map[string]models.ImpCtx{}, + }, + error: true, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { + tt.setup() + } + adapters.InitBidders("./static/bidder-params/") + m := OpenWrap{ + cfg: tt.fields.cfg, + cache: tt.fields.cache, + metricEngine: tt.fields.metricEngine, + } + tt.args.payload.BidRequest = &openrtb2.BidRequest{} + json.Unmarshal(tt.args.bidrequest, tt.args.payload.BidRequest) + + _, err := m.handleBeforeValidationHook(tt.args.ctx, tt.args.moduleCtx, tt.args.payload) + assert.Equal(t, tt.want.error, err != nil, "mismatched error") + iRctx := tt.args.moduleCtx.ModuleContext["rctx"] + gotRctx := iRctx.(models.RequestCtx) + assert.Equal(t, tt.want.rctx.ImpBidCtx, gotRctx.ImpBidCtx, "mismatched rctx.ImpBidCtx") + }) + } +} + func TestGetSlotName(t *testing.T) { type args struct { tagId string diff --git a/modules/pubmatic/openwrap/logger_test.go b/modules/pubmatic/openwrap/logger_test.go new file mode 100644 index 00000000000..cad1a487354 --- /dev/null +++ b/modules/pubmatic/openwrap/logger_test.go @@ -0,0 +1,145 @@ +package openwrap + +import ( + "testing" + + "github.com/prebid/openrtb/v19/openrtb2" + "github.com/prebid/prebid-server/util/ptrutil" + "github.com/stretchr/testify/assert" +) + +func Test_getIncomingSlots(t *testing.T) { + type args struct { + imp openrtb2.Imp + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "only_native_slot", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Native: &openrtb2.Native{ + Request: `{"ver":"1.2"}`, + }, + }, + }, + want: []string{"1x1"}, + }, + { + name: "native_with_other_slots_then_do_not_consider_native", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Native: &openrtb2.Native{ + Request: `{"ver":"1.2"}`, + }, + Banner: &openrtb2.Banner{ + W: ptrutil.ToPtr[int64](300), + H: ptrutil.ToPtr[int64](250), + }, + }, + }, + want: []string{"300x250"}, + }, + { + name: "only_banner_slot", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Banner: &openrtb2.Banner{ + W: ptrutil.ToPtr[int64](300), + H: ptrutil.ToPtr[int64](250), + }, + }, + }, + want: []string{"300x250"}, + }, + { + name: "banner_with_format", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Banner: &openrtb2.Banner{ + W: ptrutil.ToPtr[int64](300), + H: ptrutil.ToPtr[int64](250), + Format: []openrtb2.Format{ + { + W: 400, + H: 300, + }, + }, + }, + }, + }, + want: []string{"300x250", "400x300"}, + }, + { + name: "only_video_slot", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Video: &openrtb2.Video{ + W: 300, + H: 250, + }, + }, + }, + want: []string{"300x250v"}, + }, + { + name: "all_slots", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Native: &openrtb2.Native{ + Request: `{"ver":"1.2"}`, + }, + Banner: &openrtb2.Banner{ + W: ptrutil.ToPtr[int64](300), + H: ptrutil.ToPtr[int64](250), + Format: []openrtb2.Format{ + { + W: 400, + H: 300, + }, + }, + }, + Video: &openrtb2.Video{ + W: 300, + H: 250, + }, + }, + }, + want: []string{"300x250", "400x300", "300x250v"}, + }, + { + name: "duplicate_slot", + args: args{ + imp: openrtb2.Imp{ + ID: "1", + Banner: &openrtb2.Banner{ + W: ptrutil.ToPtr[int64](300), + H: ptrutil.ToPtr[int64](250), + Format: []openrtb2.Format{ + { + W: 300, + H: 250, + }, + }, + }, + }, + }, + want: []string{"300x250"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + slots := getIncomingSlots(tt.args.imp) + assert.ElementsMatch(t, tt.want, slots, "mismatched slots") + }) + } +} From 0ae5992d67e6007a810a4d1840d8254c611ca379 Mon Sep 17 00:00:00 2001 From: ashishshinde-pubm <109787960+ashishshinde-pubm@users.noreply.github.com> Date: Tue, 20 Feb 2024 16:56:04 +0530 Subject: [PATCH 3/3] OTT-1633: OW Bug Fix: Do not send null device.ext.atts (#712) --- analytics/pubmatic/logger_test.go | 10 +- analytics/pubmatic/record.go | 8 +- analytics/pubmatic/record_test.go | 16 +- modules/pubmatic/openwrap/device.go | 57 +++-- modules/pubmatic/openwrap/device_test.go | 201 +++++++++--------- .../endpoints/legacy/openrtb/v25/video.go | 6 +- modules/pubmatic/openwrap/models/device.go | 99 ++++++++- modules/pubmatic/openwrap/models/tracker.go | 4 +- modules/pubmatic/openwrap/tracker/create.go | 2 +- .../pubmatic/openwrap/tracker/create_test.go | 12 +- modules/pubmatic/openwrap/tracker/tracker.go | 2 +- .../pubmatic/openwrap/tracker/tracker_test.go | 11 +- 12 files changed, 270 insertions(+), 158 deletions(-) diff --git a/analytics/pubmatic/logger_test.go b/analytics/pubmatic/logger_test.go index fe0be1d4278..61457307e6a 100644 --- a/analytics/pubmatic/logger_test.go +++ b/analytics/pubmatic/logger_test.go @@ -3568,11 +3568,11 @@ func TestGetLogAuctionObjectAsURL(t *testing.T) { rCtx: &models.RequestCtx{ PubID: 5890, DeviceCtx: models.DeviceCtx{ - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusRestricted), - }, - }, + Ext: func() *models.ExtDevice { + extDevice := models.ExtDevice{} + extDevice.UnmarshalJSON([]byte(`{"atts":1}`)) + return &extDevice + }(), }, }, logInfo: true, diff --git a/analytics/pubmatic/record.go b/analytics/pubmatic/record.go index 91f162434ac..6e2782602f5 100644 --- a/analytics/pubmatic/record.go +++ b/analytics/pubmatic/record.go @@ -51,9 +51,9 @@ type record struct { // Device struct for storing device information type Device struct { - Platform models.DevicePlatform `json:"plt,omitempty"` - IFAType *models.DeviceIFAType `json:"ifty,omitempty"` //OTT-416, adding device.ext.ifa_type - ATTS *openrtb_ext.IOSAppTrackingStatus `json:"atts,omitempty"` //device.ext.atts + Platform models.DevicePlatform `json:"plt,omitempty"` + IFAType *models.DeviceIFAType `json:"ifty,omitempty"` //OTT-416, adding device.ext.ifa_type + ATTS *float64 `json:"atts,omitempty"` //device.ext.atts } /* @@ -199,7 +199,7 @@ func (wlog *WloggerRecord) logDeviceObject(dvc *models.DeviceCtx) { wlog.Device.Platform = dvc.Platform wlog.Device.IFAType = dvc.IFATypeID if dvc.Ext != nil { - wlog.record.Device.ATTS = dvc.Ext.ATTS + wlog.record.Device.ATTS, _ = dvc.Ext.GetAtts() } } diff --git a/analytics/pubmatic/record_test.go b/analytics/pubmatic/record_test.go index a8b8142f959..f1bfb2eb141 100644 --- a/analytics/pubmatic/record_test.go +++ b/analytics/pubmatic/record_test.go @@ -415,9 +415,7 @@ func TestLogDeviceObject(t *testing.T) { dvc: &models.DeviceCtx{ Platform: models.DevicePlatformDesktop, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeDPID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{}, - }, + Ext: &models.ExtDevice{}, }, }, want: Device{ @@ -431,17 +429,17 @@ func TestLogDeviceObject(t *testing.T) { dvc: &models.DeviceCtx{ Platform: models.DevicePlatformDesktop, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeDPID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusNotDetermined), - }, - }, + Ext: func() *models.ExtDevice { + extDevice := models.ExtDevice{} + extDevice.UnmarshalJSON([]byte(`{"atts":0}`)) + return &extDevice + }(), }, }, want: Device{ Platform: models.DevicePlatformDesktop, IFAType: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeDPID]), - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusNotDetermined), + ATTS: ptrutil.ToPtr(float64(openrtb_ext.IOSAppTrackingStatusNotDetermined)), }, }, } diff --git a/modules/pubmatic/openwrap/device.go b/modules/pubmatic/openwrap/device.go index 6b608c23451..1e5ae197851 100644 --- a/modules/pubmatic/openwrap/device.go +++ b/modules/pubmatic/openwrap/device.go @@ -1,7 +1,6 @@ package openwrap import ( - "encoding/json" "strings" "github.com/prebid/openrtb/v19/openrtb2" @@ -21,7 +20,7 @@ func populateDeviceContext(dvc *models.DeviceCtx, device *openrtb2.Device) { //unmarshal device ext var deviceExt models.ExtDevice - if err := json.Unmarshal(device.Ext, &deviceExt); err != nil { + if err := deviceExt.UnmarshalJSON(device.Ext); err != nil { return } dvc.Ext = &deviceExt @@ -36,29 +35,45 @@ func updateDeviceIFADetails(dvc *models.DeviceCtx) { } deviceExt := dvc.Ext - deviceExt.IFAType = strings.TrimSpace(deviceExt.IFAType) - deviceExt.SessionID = strings.TrimSpace(deviceExt.SessionID) - - //refactor below condition - if deviceExt.IFAType != "" { - if dvc.DeviceIFA != "" { - if _, ok := models.DeviceIFATypeID[strings.ToLower(deviceExt.IFAType)]; !ok { - deviceExt.IFAType = "" - } - } else if deviceExt.SessionID != "" { - dvc.DeviceIFA = deviceExt.SessionID - deviceExt.IFAType = models.DeviceIFATypeSESSIONID - } else { - deviceExt.IFAType = "" + extIFATypeStr, _ := deviceExt.GetIFAType() + extSessionIDStr, _ := deviceExt.GetSessionID() + + if extIFATypeStr == "" { + if extSessionIDStr == "" { + deviceExt.DeleteIFAType() + deviceExt.DeleteSessionID() + return + } + dvc.DeviceIFA = extSessionIDStr + extIFATypeStr = models.DeviceIFATypeSESSIONID + } + if dvc.DeviceIFA != "" { + if _, ok := models.DeviceIFATypeID[strings.ToLower(extIFATypeStr)]; !ok { + extIFATypeStr = "" } - } else if deviceExt.SessionID != "" { - dvc.DeviceIFA = deviceExt.SessionID - deviceExt.IFAType = models.DeviceIFATypeSESSIONID + } else if extSessionIDStr != "" { + dvc.DeviceIFA = extSessionIDStr + extIFATypeStr = models.DeviceIFATypeSESSIONID + + } else { + extIFATypeStr = "" } - if ifaTypeID, ok := models.DeviceIFATypeID[strings.ToLower(deviceExt.IFAType)]; ok { + if ifaTypeID, ok := models.DeviceIFATypeID[strings.ToLower(extIFATypeStr)]; ok { dvc.IFATypeID = &ifaTypeID } + + if extIFATypeStr == "" { + deviceExt.DeleteIFAType() + } else { + deviceExt.SetIFAType(extIFATypeStr) + } + + if extSessionIDStr == "" { + deviceExt.DeleteSessionID() + } else { + deviceExt.SetSessionID(extSessionIDStr) + } } func amendDeviceObject(device *openrtb2.Device, dvc *models.DeviceCtx) { @@ -73,6 +88,6 @@ func amendDeviceObject(device *openrtb2.Device, dvc *models.DeviceCtx) { //update device extension if dvc.Ext != nil { - device.Ext, _ = json.Marshal(dvc.Ext) + device.Ext, _ = dvc.Ext.MarshalJSON() } } diff --git a/modules/pubmatic/openwrap/device_test.go b/modules/pubmatic/openwrap/device_test.go index 633f8d9333e..9cbfd4877ff 100644 --- a/modules/pubmatic/openwrap/device_test.go +++ b/modules/pubmatic/openwrap/device_test.go @@ -7,7 +7,6 @@ import ( "github.com/prebid/openrtb/v19/openrtb2" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" - "github.com/prebid/prebid-server/openrtb_ext" "github.com/prebid/prebid-server/util/ptrutil" "github.com/stretchr/testify/assert" ) @@ -68,7 +67,11 @@ func TestPopulateDeviceExt(t *testing.T) { want: want{ deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, - Ext: &models.ExtDevice{}, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.UnmarshalJSON([]byte(`{"anykey": "anyval"}`)) + return deviceExt + }(), }, }, }, @@ -82,6 +85,7 @@ func TestPopulateDeviceExt(t *testing.T) { want: want{ deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, + Ext: models.NewExtDevice(), }, }, }, @@ -97,7 +101,7 @@ func TestPopulateDeviceExt(t *testing.T) { /* removed_invalid_ifatype */ deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, - Ext: &models.ExtDevice{}, + Ext: models.NewExtDevice(), }, }, }, @@ -113,11 +117,11 @@ func TestPopulateDeviceExt(t *testing.T) { deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeDPID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: `DpId`, - }, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetIFAType("DpId") + return deviceExt + }(), }, }, }, @@ -133,11 +137,11 @@ func TestPopulateDeviceExt(t *testing.T) { deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeSESSIONID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: `sessionid`, - }, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetIFAType("sessionid") + return deviceExt + }(), }, }, }, @@ -150,7 +154,7 @@ func TestPopulateDeviceExt(t *testing.T) { }, want: want{ deviceCtx: models.DeviceCtx{ - Ext: &models.ExtDevice{}, + Ext: models.NewExtDevice(), }, }, }, @@ -165,6 +169,11 @@ func TestPopulateDeviceExt(t *testing.T) { want: want{ deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.UnmarshalJSON([]byte(`{"atts":"invalid_value"}`)) + return deviceExt + }(), }, }, }, @@ -177,11 +186,11 @@ func TestPopulateDeviceExt(t *testing.T) { }, want: want{ deviceCtx: models.DeviceCtx{ - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusRestricted), - }, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.UnmarshalJSON([]byte(`{"atts":1}`)) + return deviceExt + }(), }, }, }, @@ -197,12 +206,12 @@ func TestPopulateDeviceExt(t *testing.T) { deviceCtx: models.DeviceCtx{ DeviceIFA: `test_ifa`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeSESSIONID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: `sessionid`, - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusRestricted), - }, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.UnmarshalJSON([]byte(`{"atts":1}`)) + deviceExt.SetIFAType("sessionid") + return deviceExt + }(), }, }, }, @@ -259,15 +268,15 @@ func TestUpdateDeviceIFADetails(t *testing.T) { name: `ifa_type_present_ifa_missing`, args: args{ dvc: &models.DeviceCtx{ - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeDPID, - }, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(models.DeviceIFATypeDPID) + return ext + }(), }, }, want: &models.DeviceCtx{ - Ext: &models.ExtDevice{}, + Ext: models.NewExtDevice(), }, }, { @@ -275,18 +284,16 @@ func TestUpdateDeviceIFADetails(t *testing.T) { args: args{ dvc: &models.DeviceCtx{ DeviceIFA: `sample_ifa_value`, - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: `wrong_ifa_type`, - }, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType("wrong_ifa_type") + return ext + }(), }, }, want: &models.DeviceCtx{ DeviceIFA: `sample_ifa_value`, - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{}, - }, + Ext: models.NewExtDevice(), }, }, { @@ -294,21 +301,21 @@ func TestUpdateDeviceIFADetails(t *testing.T) { args: args{ dvc: &models.DeviceCtx{ DeviceIFA: `sample_ifa_value`, - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeDPID, - }, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(models.DeviceIFATypeDPID) + return ext + }(), }, }, want: &models.DeviceCtx{ DeviceIFA: `sample_ifa_value`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeDPID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeDPID, - }, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(models.DeviceIFATypeDPID) + return ext + }(), }, }, { @@ -316,61 +323,59 @@ func TestUpdateDeviceIFADetails(t *testing.T) { args: args{ dvc: &models.DeviceCtx{ DeviceIFA: `sample_ifa_value`, - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: strings.ToUpper(models.DeviceIFATypeDPID), - }, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(strings.ToUpper(models.DeviceIFATypeDPID)) + return ext + }(), }, }, want: &models.DeviceCtx{ DeviceIFA: `sample_ifa_value`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeDPID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: strings.ToUpper(models.DeviceIFATypeDPID), - }, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(strings.ToUpper(models.DeviceIFATypeDPID)) + return ext + }(), }, }, { name: `ifa_type_present_session_id_present`, args: args{ dvc: &models.DeviceCtx{ - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeDPID, - }, - SessionID: `sample_session_id`, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(strings.ToUpper(models.DeviceIFATypeDPID)) + ext.SetSessionID(`sample_session_id`) + return ext + }(), }, }, want: &models.DeviceCtx{ DeviceIFA: `sample_session_id`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeSESSIONID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeSESSIONID, - }, - SessionID: `sample_session_id`, - }, + Ext: func() *models.ExtDevice { + ext := &models.ExtDevice{} + ext.SetIFAType(models.DeviceIFATypeSESSIONID) + ext.SetSessionID(`sample_session_id`) + return ext + }(), }, }, { name: `ifa_type_present_session_id_missing`, args: args{ dvc: &models.DeviceCtx{ - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeDPID, - }, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetIFAType(models.DeviceIFATypeDPID) + return deviceExt + }(), }, }, want: &models.DeviceCtx{ - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{}, - }, + Ext: models.NewExtDevice(), }, }, { @@ -378,21 +383,22 @@ func TestUpdateDeviceIFADetails(t *testing.T) { args: args{ dvc: &models.DeviceCtx{ DeviceIFA: `existing_ifa_id`, - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{}, - SessionID: `sample_session_id`, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetSessionID(`sample_session_id`) + return deviceExt + }(), }, }, want: &models.DeviceCtx{ DeviceIFA: `sample_session_id`, IFATypeID: ptrutil.ToPtr(models.DeviceIFATypeID[models.DeviceIFATypeSESSIONID]), - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - IFAType: models.DeviceIFATypeSESSIONID, - }, - SessionID: `sample_session_id`, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetSessionID(`sample_session_id`) + deviceExt.SetIFAType(models.DeviceIFATypeSESSIONID) + return deviceExt + }(), }, }, // TODO: Add test cases. @@ -460,9 +466,11 @@ func TestAmendDeviceObject(t *testing.T) { }, dvc: &models.DeviceCtx{ DeviceIFA: `new_ifa`, - Ext: &models.ExtDevice{ - SessionID: `sample_session`, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetSessionID("sample_session") + return deviceExt + }(), }, }, want: &openrtb2.Device{ @@ -481,9 +489,11 @@ func TestAmendDeviceObject(t *testing.T) { }, dvc: &models.DeviceCtx{ DeviceIFA: `new_ifa`, - Ext: &models.ExtDevice{ - SessionID: `sample_session`, - }, + Ext: func() *models.ExtDevice { + deviceExt := &models.ExtDevice{} + deviceExt.SetSessionID("sample_session") + return deviceExt + }(), }, }, want: &openrtb2.Device{ @@ -497,6 +507,7 @@ func TestAmendDeviceObject(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { amendDeviceObject(tt.args.device, tt.args.dvc) + assert.Equal(t, tt.args.device, tt.want, "mismatched device object") }) } } diff --git a/modules/pubmatic/openwrap/endpoints/legacy/openrtb/v25/video.go b/modules/pubmatic/openwrap/endpoints/legacy/openrtb/v25/video.go index 17dd0d81b2f..c0015b20743 100644 --- a/modules/pubmatic/openwrap/endpoints/legacy/openrtb/v25/video.go +++ b/modules/pubmatic/openwrap/endpoints/legacy/openrtb/v25/video.go @@ -217,13 +217,11 @@ func ConvertVideoToAuctionRequest(payload hookstage.EntrypointPayload, result *h var deviceExt models.ExtDevice if session_id := GetValueFromRequest(values, redirectQueryParams, models.DeviceExtSessionID); session_id != nil { - deviceExt.SessionID = GetString(session_id) + deviceExt.SetSessionID(GetString(session_id)) } if ifaType := GetValueFromRequest(values, redirectQueryParams, models.DeviceExtIfaType); ifaType != nil { - deviceExt.ExtDevice = openrtb_ext.ExtDevice{ - IFAType: GetString(ifaType), - } + deviceExt.SetIFAType(GetString(ifaType)) } bidRequest.Device.Ext, _ = json.Marshal(deviceExt) } else { diff --git a/modules/pubmatic/openwrap/models/device.go b/modules/pubmatic/openwrap/models/device.go index 109c1a467f7..a6955241e5d 100644 --- a/modules/pubmatic/openwrap/models/device.go +++ b/modules/pubmatic/openwrap/models/device.go @@ -1,6 +1,9 @@ package models -import "github.com/prebid/prebid-server/openrtb_ext" +import ( + "encoding/json" + "strings" +) const ( //Device.DeviceType values as per OpenRTB-API-Specification-Version-2-5 @@ -56,8 +59,96 @@ const ( DeviceIFATypeSESSIONID = "sessionid" ) +// device.ext related keys +const ( + ExtDeviceIFAType = "ifa_type" + ExtDeviceSessionID = "session_id" + ExtDeviceAtts = "atts" +) + +// ExtDevice will store device.ext parameters type ExtDevice struct { - openrtb_ext.ExtDevice - SessionID string `json:"session_id,omitempty"` - IDFV string `json:"idfv,omitempty"` + data map[string]any +} + +func NewExtDevice() *ExtDevice { + return &ExtDevice{ + data: make(map[string]any), + } +} + +func (e *ExtDevice) UnmarshalJSON(data []byte) error { + var intermediate map[string]interface{} + if err := json.Unmarshal(data, &intermediate); err != nil { + return err + } + e.data = intermediate + return nil +} + +func (e *ExtDevice) MarshalJSON() ([]byte, error) { + return json.Marshal(e.data) +} + +func (e *ExtDevice) getStringValue(key string) (value string, found bool) { + if e.data == nil { + return value, found + } + val, found := e.data[key] + if !found { + return "", found + } + value, found = val.(string) + return strings.TrimSpace(value), found +} + +func (e *ExtDevice) GetIFAType() (value string, found bool) { + return e.getStringValue(ExtDeviceIFAType) +} + +func (e *ExtDevice) GetSessionID() (value string, found bool) { + return e.getStringValue(ExtDeviceSessionID) +} + +func (e *ExtDevice) getFloatValue(key string) (value float64, found bool) { + if e.data == nil { + return value, found + } + val, found := e.data[key] + if !found { + return 0, found + } + value, found = val.(float64) + return value, found +} + +func (e *ExtDevice) GetAtts() (value *float64, found bool) { + val, ok := e.getFloatValue(ExtDeviceAtts) + if !ok { + return nil, ok + } + return &val, ok +} + +func (e *ExtDevice) setStringValue(key, value string) { + if e.data == nil { + e.data = make(map[string]any) + } + e.data[key] = value +} + +func (e *ExtDevice) SetIFAType(ifaType string) { + e.setStringValue(ExtDeviceIFAType, ifaType) +} + +func (e *ExtDevice) SetSessionID(sessionID string) { + e.setStringValue(ExtDeviceSessionID, sessionID) +} + +func (e *ExtDevice) DeleteIFAType() { + delete(e.data, ExtDeviceIFAType) +} + +func (e *ExtDevice) DeleteSessionID() { + delete(e.data, ExtDeviceSessionID) } diff --git a/modules/pubmatic/openwrap/models/tracker.go b/modules/pubmatic/openwrap/models/tracker.go index cbf73f63120..0b007471663 100644 --- a/modules/pubmatic/openwrap/models/tracker.go +++ b/modules/pubmatic/openwrap/models/tracker.go @@ -1,7 +1,5 @@ package models -import "github.com/prebid/prebid-server/openrtb_ext" - // OWTracker vast video parameters to be injected type OWTracker struct { Tracker Tracker @@ -39,7 +37,7 @@ type Tracker struct { FloorSource *int FloorType int CustomDimensions string - ATTS *openrtb_ext.IOSAppTrackingStatus + ATTS *float64 LoggerData LoggerData // need this in logger to avoid duplicate computation ImpID string `json:"-"` diff --git a/modules/pubmatic/openwrap/tracker/create.go b/modules/pubmatic/openwrap/tracker/create.go index a2a84f4aaaf..7d79ecb52f9 100644 --- a/modules/pubmatic/openwrap/tracker/create.go +++ b/modules/pubmatic/openwrap/tracker/create.go @@ -87,7 +87,7 @@ func createTrackers(rctx models.RequestCtx, trackers map[string]models.OWTracker ) if rctx.DeviceCtx.Ext != nil { - tracker.ATTS = rctx.DeviceCtx.Ext.ATTS + tracker.ATTS, _ = rctx.DeviceCtx.Ext.GetAtts() } if impCtx, ok := rctx.ImpBidCtx[bid.ImpID]; ok { diff --git a/modules/pubmatic/openwrap/tracker/create_test.go b/modules/pubmatic/openwrap/tracker/create_test.go index f5f38ec92d6..d47edc3f349 100644 --- a/modules/pubmatic/openwrap/tracker/create_test.go +++ b/modules/pubmatic/openwrap/tracker/create_test.go @@ -110,11 +110,11 @@ func Test_createTrackers(t *testing.T) { rctx: func() models.RequestCtx { testRctx := rctx testRctx.StartTime = startTime - testRctx.DeviceCtx.Ext = &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusRestricted), - }, - } + testRctx.DeviceCtx.Ext = func() *models.ExtDevice { + extDevice := models.ExtDevice{} + extDevice.UnmarshalJSON([]byte(`{"atts":1}`)) + return &extDevice + }() return testRctx }(), bidResponse: &openrtb2.BidResponse{ @@ -176,7 +176,7 @@ func Test_createTrackers(t *testing.T) { KGPSV: "adunit-1@250x300", }, CustomDimensions: "author=henry", - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusRestricted), + ATTS: ptrutil.ToPtr(float64(openrtb_ext.IOSAppTrackingStatusRestricted)), }, TrackerURL: "https:?adv=domain.com&af=banner&aps=0&atts=1&au=adunit-1&bc=pubmatic&bidid=bidID-1&cds=author%3Dhenry&di=deal-id-1&dur=20&eg=8.7&en=8.7&frv=4.4&ft=0&fv=6.4&iid=loggerIID&kgpv=adunit-1%40250x300&orig=publisher.com&origbidid=bidID-1&pdvid=1&pid=1234&plt=5&pn=prebidBidderCode&psz=250x300&pubid=5890&purl=abc.com&sl=1&slot=impID-1_adunit-1&ss=1&ssai=mediatailor&tgid=1&tst=" + strconv.FormatInt(startTime, 10), Price: 8.7, diff --git a/modules/pubmatic/openwrap/tracker/tracker.go b/modules/pubmatic/openwrap/tracker/tracker.go index 54e7d34c0e6..1ea29ffe0c1 100644 --- a/modules/pubmatic/openwrap/tracker/tracker.go +++ b/modules/pubmatic/openwrap/tracker/tracker.go @@ -27,7 +27,7 @@ func GetTrackerInfo(rCtx models.RequestCtx, responseExt openrtb_ext.ExtBidRespon } if rCtx.DeviceCtx.Ext != nil { - tracker.ATTS = rCtx.DeviceCtx.Ext.ATTS + tracker.ATTS, _ = rCtx.DeviceCtx.Ext.GetAtts() } constructedURLString := constructTrackerURL(rCtx, tracker) diff --git a/modules/pubmatic/openwrap/tracker/tracker_test.go b/modules/pubmatic/openwrap/tracker/tracker_test.go index 25a36119177..e6e78fe0a32 100644 --- a/modules/pubmatic/openwrap/tracker/tracker_test.go +++ b/modules/pubmatic/openwrap/tracker/tracker_test.go @@ -1,6 +1,7 @@ package tracker import ( + "encoding/json" "strconv" "testing" "time" @@ -38,11 +39,11 @@ func TestGetTrackerInfo(t *testing.T) { ABTestConfigApplied: 1, DeviceCtx: models.DeviceCtx{ Platform: models.DevicePlatformMobileAppAndroid, - Ext: &models.ExtDevice{ - ExtDevice: openrtb_ext.ExtDevice{ - ATTS: ptrutil.ToPtr(openrtb_ext.IOSAppTrackingStatusRestricted), - }, - }, + Ext: func() *models.ExtDevice { + extDevice := models.ExtDevice{} + json.Unmarshal([]byte(`{"atts":1}`), &extDevice) + return &extDevice + }(), }, }, responseExt: openrtb_ext.ExtBidResponse{},