From 1e1913181efc31611d844f6d17c917fc8d937a1b Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Mon, 13 Nov 2023 14:08:22 -0800 Subject: [PATCH 1/8] Add forceRespFormat func and test framewok --- config/bidderinfo.go | 4 ++++ endpoints/cookie_sync_test.go | 4 ++++ endpoints/setuid.go | 4 ++++ endpoints/setuid_test.go | 5 +++++ usersync/chooser_test.go | 5 +++++ usersync/syncer.go | 16 ++++++++++++++++ 6 files changed, 38 insertions(+) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index 45abc85227a..ddc5d1b8af7 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -124,6 +124,10 @@ type Syncer struct { // SupportCORS identifies if CORS is supported for the user syncing endpoints. SupportCORS *bool `yaml:"supportCors" mapstructure:"support_cors"` + + // TODO: Check what Java does if applicable + // TODO: Add description + ForceSyncType string `yaml:"forceSyncType" mapstructure:"force_sync_type"` } // SyncerEndpoint specifies the configuration of the URL returned by the /cookie_sync endpoint diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index 956123006ca..c57ad81e977 100644 --- a/endpoints/cookie_sync_test.go +++ b/endpoints/cookie_sync_test.go @@ -1996,6 +1996,10 @@ func (m *MockSyncer) GetSync(syncTypes []usersync.SyncType, privacyMacros macros return args.Get(0).(usersync.Sync), args.Error(1) } +func (m *MockSyncer) ForceResponseFormat() string { + return "" +} + type MockAnalyticsRunner struct { mock.Mock } diff --git a/endpoints/setuid.go b/endpoints/setuid.go index f9c55fc84de..1964ac9c42a 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -359,6 +359,10 @@ func isSyncerPriority(bidderNameFromSyncerQuery string, priorityGroups [][]strin // Returns either "b" (iframe), "i" (redirect), or an empty string "" (legacy behavior of an // empty response body with no content type). func getResponseFormat(query url.Values, syncer usersync.Syncer) (string, error) { + if syncer.ForceResponseFormat() != "" { + return syncer.ForceResponseFormat(), nil + } + format, formatProvided := query["f"] formatEmpty := len(format) == 0 || format[0] == "" diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index fe66379702c..18e31ee400a 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -1666,6 +1666,7 @@ func (g *fakePermsSetUID) AuctionActivitiesAllowed(ctx context.Context, bidderCo type fakeSyncer struct { key string defaultSyncType usersync.SyncType + forceSyncType string } func (s fakeSyncer) Key() string { @@ -1684,6 +1685,10 @@ func (s fakeSyncer) GetSync(syncTypes []usersync.SyncType, privacyMacros macros. return usersync.Sync{}, nil } +func (s fakeSyncer) ForceResponseFormat() string { + return s.forceSyncType +} + func ToHTTPCookie(cookie *usersync.Cookie) (*http.Cookie, error) { encoder := usersync.Base64Encoder{} encodedCookie, err := encoder.Encode(cookie) diff --git a/usersync/chooser_test.go b/usersync/chooser_test.go index e73122b784a..75ce5f46fa5 100644 --- a/usersync/chooser_test.go +++ b/usersync/chooser_test.go @@ -505,6 +505,7 @@ type fakeSyncer struct { key string supportsIFrame bool supportsRedirect bool + forceSyncType string } func (s fakeSyncer) Key() string { @@ -531,6 +532,10 @@ func (fakeSyncer) GetSync([]SyncType, macros.UserSyncPrivacy) (Sync, error) { return Sync{}, nil } +func (s fakeSyncer) ForceResponseFormat() string { + return s.forceSyncType +} + type fakePrivacy struct { gdprAllowsHostCookie bool gdprAllowsBidderSync bool diff --git a/usersync/syncer.go b/usersync/syncer.go index 3add47e5873..21ef430a3bd 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -35,6 +35,9 @@ type Syncer interface { // GetSync returns a user sync for the user's device to perform, or an error if the none of the // sync types are supported or if macro substitution fails. GetSync(syncTypes []SyncType, userSyncMacros macros.UserSyncPrivacy) (Sync, error) + + // TODO: Add Description + ForceResponseFormat() string } // Sync represents a user sync to be performed by the user's device. @@ -50,6 +53,7 @@ type standardSyncer struct { iframe *template.Template redirect *template.Template supportCORS bool + forceSyncType string } const ( @@ -72,6 +76,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st key: syncerConfig.Key, defaultSyncType: resolveDefaultSyncType(syncerConfig), supportCORS: syncerConfig.SupportCORS != nil && *syncerConfig.SupportCORS, + forceSyncType: syncerConfig.ForceSyncType, } if syncerConfig.IFrame != nil { @@ -266,3 +271,14 @@ func (s standardSyncer) chooseTemplate(syncType SyncType) *template.Template { return nil } } + +func (s standardSyncer) ForceResponseFormat() string { + switch s.forceSyncType { + case setuidSyncTypeIFrame: + return s.forceSyncType + case setuidSyncTypeRedirect: + return s.forceSyncType + default: + return "" + } +} From 3c082553bc3146a08915e9c2a459629963bb9a61 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 15 Nov 2023 09:23:18 -0800 Subject: [PATCH 2/8] Tests --- config/bidderinfo.go | 3 +-- endpoints/setuid_test.go | 42 +++++++++++++++++++++++++++++++++------ usersync/syncer.go | 2 +- usersync/syncer_test.go | 43 +++++++++++++++++++++++++++++++++++----- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index ddc5d1b8af7..7f438c762a0 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -125,8 +125,7 @@ type Syncer struct { // SupportCORS identifies if CORS is supported for the user syncing endpoints. SupportCORS *bool `yaml:"supportCors" mapstructure:"support_cors"` - // TODO: Check what Java does if applicable - // TODO: Add description + // ForceSyncType allows a bidder to override their callback type "b" for iframe, "i" for redirect ForceSyncType string `yaml:"forceSyncType" mapstructure:"force_sync_type"` } diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 18e31ee400a..ff903232a83 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -34,6 +34,7 @@ func TestSetUIDEndpoint(t *testing.T) { gdprAllowsHostCookies bool gdprReturnsError bool gdprMalformed bool + forceSyncType string expectedSyncs map[string]string expectedBody string expectedStatusCode int @@ -336,6 +337,17 @@ func TestSetUIDEndpoint(t *testing.T) { expectedStatusCode: http.StatusBadRequest, expectedBody: "invalid gpp_sid encoding, must be a csv list of integers", }, + { + uri: "/setuid?bidder=pubmatic&uid=123&f=b", + syncersBidderNameToKey: map[string]string{"pubmatic": "pubmatic"}, + existingSyncs: nil, + gdprAllowsHostCookies: true, + forceSyncType: "i", + expectedSyncs: map[string]string{"pubmatic": "123"}, + expectedStatusCode: http.StatusOK, + expectedHeaders: map[string]string{"Content-Length": "86", "Content-Type": "image/png"}, + description: "Set uid for valid bidder with iframe format, but it's overriden by forceType", + }, } analytics := analyticsBuild.New(&config.Analytics{}) @@ -343,7 +355,7 @@ func TestSetUIDEndpoint(t *testing.T) { for _, test := range testCases { response := doRequest(makeRequest(test.uri, test.existingSyncs), analytics, metrics, - test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil) + test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil, test.forceSyncType) assert.Equal(t, test.expectedStatusCode, response.Code, "Test Case: %s. /setuid returned unexpected error code", test.description) if test.expectedSyncs != nil { @@ -467,7 +479,7 @@ func TestSetUIDPriorityEjection(t *testing.T) { request.AddCookie(httpCookie) // Make Request to /setuid - response := doRequest(request, analytics, &metricsConf.NilMetricsEngine{}, syncersByBidder, true, false, false, false, test.givenMaxCookieSize, test.givenPriorityGroups) + response := doRequest(request, analytics, &metricsConf.NilMetricsEngine{}, syncersByBidder, true, false, false, false, test.givenMaxCookieSize, test.givenPriorityGroups, "") if test.expectedWarning != "" { assert.Equal(t, test.expectedWarning, response.Body.String(), test.description) @@ -1333,7 +1345,7 @@ func TestSetUIDEndpointMetrics(t *testing.T) { for _, v := range test.cookies { addCookie(req, v) } - response := doRequest(req, analyticsEngine, metricsEngine, test.syncersBidderNameToKey, test.gdprAllowsHostCookies, false, false, test.cfgAccountRequired, 0, nil) + response := doRequest(req, analyticsEngine, metricsEngine, test.syncersBidderNameToKey, test.gdprAllowsHostCookies, false, false, test.cfgAccountRequired, 0, nil, "") assert.Equal(t, test.expectedResponseCode, response.Code, test.description) analyticsEngine.AssertExpectations(t) @@ -1349,7 +1361,7 @@ func TestOptedOut(t *testing.T) { syncersBidderNameToKey := map[string]string{"pubmatic": "pubmatic"} analytics := analyticsBuild.New(&config.Analytics{}) metrics := &metricsConf.NilMetricsEngine{} - response := doRequest(request, analytics, metrics, syncersBidderNameToKey, true, false, false, false, 0, nil) + response := doRequest(request, analytics, metrics, syncersBidderNameToKey, true, false, false, false, 0, nil, "") assert.Equal(t, http.StatusUnauthorized, response.Code) } @@ -1445,6 +1457,24 @@ func TestGetResponseFormat(t *testing.T) { expectedFormat: "i", description: "parameter given is empty (by empty item), use default sync type redirect", }, + { + urlValues: url.Values{"f": []string{""}}, + syncer: fakeSyncer{key: "a", defaultSyncType: usersync.SyncTypeRedirect}, + expectedFormat: "i", + description: "parameter given is empty (by empty item), use default sync type redirect", + }, + { + urlValues: url.Values{"f": []string{"b"}}, + syncer: fakeSyncer{key: "a", forceSyncType: "i"}, + expectedFormat: "i", + description: "parameter given as `b`, but force sync type is opposite", + }, + { + urlValues: url.Values{"f": []string{"i"}}, + syncer: fakeSyncer{key: "a", forceSyncType: "b"}, + expectedFormat: "b", + description: "parameter given as `i`, but force sync type is opposite", + }, } for _, test := range testCases { @@ -1544,7 +1574,7 @@ func makeRequest(uri string, existingSyncs map[string]string) *http.Request { return request } -func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string) *httptest.ResponseRecorder { +func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string, forceSyncType string) *httptest.ResponseRecorder { cfg := config.Configuration{ AccountRequired: cfgAccountRequired, AccountDefaults: config.Account{}, @@ -1575,7 +1605,7 @@ func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.Me syncersByBidder := make(map[string]usersync.Syncer) for bidderName, syncerKey := range syncersBidderNameToKey { - syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame} + syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame, forceSyncType: forceSyncType} if priorityGroups == nil { cfg.UserSync.PriorityGroups = [][]string{{}} cfg.UserSync.PriorityGroups[0] = append(cfg.UserSync.PriorityGroups[0], bidderName) diff --git a/usersync/syncer.go b/usersync/syncer.go index 21ef430a3bd..740e160550e 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -36,7 +36,7 @@ type Syncer interface { // sync types are supported or if macro substitution fails. GetSync(syncTypes []SyncType, userSyncMacros macros.UserSyncPrivacy) (Sync, error) - // TODO: Add Description + // ForceResponseFromat returns the callback format as specified in a bidders config ForceResponseFormat() string } diff --git a/usersync/syncer_test.go b/usersync/syncer_test.go index 6aa3103211e..eadd1f6ce32 100644 --- a/usersync/syncer_test.go +++ b/usersync/syncer_test.go @@ -27,6 +27,7 @@ func TestNewSyncer(t *testing.T) { givenIFrameConfig *config.SyncerEndpoint givenRedirectConfig *config.SyncerEndpoint givenExternalURL string + givenForceType string expectedError string expectedDefault SyncType expectedIFrame string @@ -97,6 +98,7 @@ func TestNewSyncer(t *testing.T) { givenExternalURL: "http://syncer.com", givenIFrameConfig: iframeConfig, givenRedirectConfig: redirectConfig, + givenForceType: "i", expectedDefault: SyncTypeIFrame, expectedIFrame: "https://bidder.com/iframe?redirect=http%3A%2F%2Fsyncer.com%2Fhost", expectedRedirect: "https://bidder.com/redirect?redirect=http%3A%2F%2Fsyncer.com%2Fhost", @@ -105,11 +107,12 @@ func TestNewSyncer(t *testing.T) { for _, test := range testCases { syncerConfig := config.Syncer{ - Key: test.givenKey, - SupportCORS: &supportCORS, - IFrame: test.givenIFrameConfig, - Redirect: test.givenRedirectConfig, - ExternalURL: test.givenExternalURL, + Key: test.givenKey, + SupportCORS: &supportCORS, + IFrame: test.givenIFrameConfig, + Redirect: test.givenRedirectConfig, + ExternalURL: test.givenExternalURL, + ForceSyncType: test.givenForceType, } result, err := NewSyncer(hostConfig, syncerConfig, test.givenBidderName) @@ -120,6 +123,7 @@ func TestNewSyncer(t *testing.T) { result := result.(standardSyncer) assert.Equal(t, test.givenKey, result.key, test.description+":key") assert.Equal(t, supportCORS, result.supportCORS, test.description+":cors") + assert.Equal(t, test.givenForceType, result.forceSyncType, test.description+":cors") assert.Equal(t, test.expectedDefault, result.defaultSyncType, test.description+":default_sync") if test.expectedIFrame == "" { @@ -779,3 +783,32 @@ func TestSyncerChooseTemplate(t *testing.T) { assert.Equal(t, test.expectedTemplate, result, test.description) } } + +func TestSyncerForceResponseFormat(t *testing.T) { + testCases := []struct { + description string + givenForceType string + expectedForceType string + }{ + { + description: "IFrame", + givenForceType: "b", + expectedForceType: "b", + }, + { + description: "Redirect", + givenForceType: "i", + expectedForceType: "i", + }, + { + description: "Empty", + expectedForceType: "", + }, + } + + for _, test := range testCases { + syncer := standardSyncer{forceSyncType: test.givenForceType} + forceType := syncer.ForceResponseFormat() + assert.Equal(t, test.expectedForceType, forceType, test.description) + } +} From 3d94e16bef23967aa8ceef60bd351de27c83803e Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 29 Nov 2023 12:51:02 -0500 Subject: [PATCH 3/8] Update variable names to match java --- config/bidderinfo.go | 4 ++-- endpoints/setuid_test.go | 18 +++++++++--------- usersync/chooser_test.go | 4 ++-- usersync/syncer.go | 10 +++++----- usersync/syncer_test.go | 16 ++++++++-------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index c99db9cf0d9..58430644f04 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -125,8 +125,8 @@ type Syncer struct { // SupportCORS identifies if CORS is supported for the user syncing endpoints. SupportCORS *bool `yaml:"supportCors" mapstructure:"support_cors"` - // ForceSyncType allows a bidder to override their callback type "b" for iframe, "i" for redirect - ForceSyncType string `yaml:"forceSyncType" mapstructure:"force_sync_type"` + // FormatOverride allows a bidder to override their callback type "b" for iframe, "i" for redirect + FormatOverride string `yaml:"forceOverride" mapstructure:"force_override"` // SkipWhen allows bidders to specify when they don't want to sync SkipWhen *SkipWhen `yaml:"skipwhen" mapstructure:"skipwhen"` diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index ff903232a83..774b9a490d0 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -34,7 +34,7 @@ func TestSetUIDEndpoint(t *testing.T) { gdprAllowsHostCookies bool gdprReturnsError bool gdprMalformed bool - forceSyncType string + forceOverride string expectedSyncs map[string]string expectedBody string expectedStatusCode int @@ -342,7 +342,7 @@ func TestSetUIDEndpoint(t *testing.T) { syncersBidderNameToKey: map[string]string{"pubmatic": "pubmatic"}, existingSyncs: nil, gdprAllowsHostCookies: true, - forceSyncType: "i", + forceOverride: "i", expectedSyncs: map[string]string{"pubmatic": "123"}, expectedStatusCode: http.StatusOK, expectedHeaders: map[string]string{"Content-Length": "86", "Content-Type": "image/png"}, @@ -355,7 +355,7 @@ func TestSetUIDEndpoint(t *testing.T) { for _, test := range testCases { response := doRequest(makeRequest(test.uri, test.existingSyncs), analytics, metrics, - test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil, test.forceSyncType) + test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil, test.forceOverride) assert.Equal(t, test.expectedStatusCode, response.Code, "Test Case: %s. /setuid returned unexpected error code", test.description) if test.expectedSyncs != nil { @@ -1465,13 +1465,13 @@ func TestGetResponseFormat(t *testing.T) { }, { urlValues: url.Values{"f": []string{"b"}}, - syncer: fakeSyncer{key: "a", forceSyncType: "i"}, + syncer: fakeSyncer{key: "a", forceOverride: "i"}, expectedFormat: "i", description: "parameter given as `b`, but force sync type is opposite", }, { urlValues: url.Values{"f": []string{"i"}}, - syncer: fakeSyncer{key: "a", forceSyncType: "b"}, + syncer: fakeSyncer{key: "a", forceOverride: "b"}, expectedFormat: "b", description: "parameter given as `i`, but force sync type is opposite", }, @@ -1574,7 +1574,7 @@ func makeRequest(uri string, existingSyncs map[string]string) *http.Request { return request } -func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string, forceSyncType string) *httptest.ResponseRecorder { +func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string, forceOverride string) *httptest.ResponseRecorder { cfg := config.Configuration{ AccountRequired: cfgAccountRequired, AccountDefaults: config.Account{}, @@ -1605,7 +1605,7 @@ func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.Me syncersByBidder := make(map[string]usersync.Syncer) for bidderName, syncerKey := range syncersBidderNameToKey { - syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame, forceSyncType: forceSyncType} + syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame, forceOverride: forceOverride} if priorityGroups == nil { cfg.UserSync.PriorityGroups = [][]string{{}} cfg.UserSync.PriorityGroups[0] = append(cfg.UserSync.PriorityGroups[0], bidderName) @@ -1696,7 +1696,7 @@ func (g *fakePermsSetUID) AuctionActivitiesAllowed(ctx context.Context, bidderCo type fakeSyncer struct { key string defaultSyncType usersync.SyncType - forceSyncType string + forceOverride string } func (s fakeSyncer) Key() string { @@ -1716,7 +1716,7 @@ func (s fakeSyncer) GetSync(syncTypes []usersync.SyncType, privacyMacros macros. } func (s fakeSyncer) ForceResponseFormat() string { - return s.forceSyncType + return s.forceOverride } func ToHTTPCookie(cookie *usersync.Cookie) (*http.Cookie, error) { diff --git a/usersync/chooser_test.go b/usersync/chooser_test.go index 5bda103e077..ee06741e4b0 100644 --- a/usersync/chooser_test.go +++ b/usersync/chooser_test.go @@ -671,7 +671,7 @@ type fakeSyncer struct { key string supportsIFrame bool supportsRedirect bool - forceSyncType string + forceOverride string } func (s fakeSyncer) Key() string { @@ -699,7 +699,7 @@ func (fakeSyncer) GetSync([]SyncType, macros.UserSyncPrivacy) (Sync, error) { } func (s fakeSyncer) ForceResponseFormat() string { - return s.forceSyncType + return s.forceOverride } type fakePrivacy struct { diff --git a/usersync/syncer.go b/usersync/syncer.go index 740e160550e..56ca578c3d5 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -53,7 +53,7 @@ type standardSyncer struct { iframe *template.Template redirect *template.Template supportCORS bool - forceSyncType string + forceOverride string } const ( @@ -76,7 +76,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st key: syncerConfig.Key, defaultSyncType: resolveDefaultSyncType(syncerConfig), supportCORS: syncerConfig.SupportCORS != nil && *syncerConfig.SupportCORS, - forceSyncType: syncerConfig.ForceSyncType, + forceOverride: syncerConfig.FormatOverride, } if syncerConfig.IFrame != nil { @@ -273,11 +273,11 @@ func (s standardSyncer) chooseTemplate(syncType SyncType) *template.Template { } func (s standardSyncer) ForceResponseFormat() string { - switch s.forceSyncType { + switch s.forceOverride { case setuidSyncTypeIFrame: - return s.forceSyncType + return s.forceOverride case setuidSyncTypeRedirect: - return s.forceSyncType + return s.forceOverride default: return "" } diff --git a/usersync/syncer_test.go b/usersync/syncer_test.go index eadd1f6ce32..589378c6be0 100644 --- a/usersync/syncer_test.go +++ b/usersync/syncer_test.go @@ -107,12 +107,12 @@ func TestNewSyncer(t *testing.T) { for _, test := range testCases { syncerConfig := config.Syncer{ - Key: test.givenKey, - SupportCORS: &supportCORS, - IFrame: test.givenIFrameConfig, - Redirect: test.givenRedirectConfig, - ExternalURL: test.givenExternalURL, - ForceSyncType: test.givenForceType, + Key: test.givenKey, + SupportCORS: &supportCORS, + IFrame: test.givenIFrameConfig, + Redirect: test.givenRedirectConfig, + ExternalURL: test.givenExternalURL, + FormatOverride: test.givenForceType, } result, err := NewSyncer(hostConfig, syncerConfig, test.givenBidderName) @@ -123,7 +123,7 @@ func TestNewSyncer(t *testing.T) { result := result.(standardSyncer) assert.Equal(t, test.givenKey, result.key, test.description+":key") assert.Equal(t, supportCORS, result.supportCORS, test.description+":cors") - assert.Equal(t, test.givenForceType, result.forceSyncType, test.description+":cors") + assert.Equal(t, test.givenForceType, result.forceOverride, test.description+":cors") assert.Equal(t, test.expectedDefault, result.defaultSyncType, test.description+":default_sync") if test.expectedIFrame == "" { @@ -807,7 +807,7 @@ func TestSyncerForceResponseFormat(t *testing.T) { } for _, test := range testCases { - syncer := standardSyncer{forceSyncType: test.givenForceType} + syncer := standardSyncer{forceOverride: test.givenForceType} forceType := syncer.ForceResponseFormat() assert.Equal(t, test.expectedForceType, forceType, test.description) } From 7a9d0157124632e9a7f7f37bb91713e77aef87f7 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 29 Nov 2023 16:46:09 -0500 Subject: [PATCH 4/8] update naming --- config/bidderinfo.go | 2 +- endpoints/setuid_test.go | 24 ++++++++++++------------ usersync/chooser_test.go | 4 ++-- usersync/syncer.go | 10 +++++----- usersync/syncer_test.go | 15 ++++++--------- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index 58430644f04..7f3fa454a42 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -126,7 +126,7 @@ type Syncer struct { SupportCORS *bool `yaml:"supportCors" mapstructure:"support_cors"` // FormatOverride allows a bidder to override their callback type "b" for iframe, "i" for redirect - FormatOverride string `yaml:"forceOverride" mapstructure:"force_override"` + FormatOverride string `yaml:"formatOverride" mapstructure:"format_override"` // SkipWhen allows bidders to specify when they don't want to sync SkipWhen *SkipWhen `yaml:"skipwhen" mapstructure:"skipwhen"` diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 774b9a490d0..fd5c7d3dd97 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -34,7 +34,7 @@ func TestSetUIDEndpoint(t *testing.T) { gdprAllowsHostCookies bool gdprReturnsError bool gdprMalformed bool - forceOverride string + formatOverride string expectedSyncs map[string]string expectedBody string expectedStatusCode int @@ -342,11 +342,11 @@ func TestSetUIDEndpoint(t *testing.T) { syncersBidderNameToKey: map[string]string{"pubmatic": "pubmatic"}, existingSyncs: nil, gdprAllowsHostCookies: true, - forceOverride: "i", + formatOverride: "i", expectedSyncs: map[string]string{"pubmatic": "123"}, expectedStatusCode: http.StatusOK, expectedHeaders: map[string]string{"Content-Length": "86", "Content-Type": "image/png"}, - description: "Set uid for valid bidder with iframe format, but it's overriden by forceType", + description: "Set uid for valid bidder with iframe format, but it's overriden by formatOverride", }, } @@ -355,7 +355,7 @@ func TestSetUIDEndpoint(t *testing.T) { for _, test := range testCases { response := doRequest(makeRequest(test.uri, test.existingSyncs), analytics, metrics, - test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil, test.forceOverride) + test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil, test.formatOverride) assert.Equal(t, test.expectedStatusCode, response.Code, "Test Case: %s. /setuid returned unexpected error code", test.description) if test.expectedSyncs != nil { @@ -1465,15 +1465,15 @@ func TestGetResponseFormat(t *testing.T) { }, { urlValues: url.Values{"f": []string{"b"}}, - syncer: fakeSyncer{key: "a", forceOverride: "i"}, + syncer: fakeSyncer{key: "a", formatOverride: "i"}, expectedFormat: "i", - description: "parameter given as `b`, but force sync type is opposite", + description: "parameter given as `b`, but formatOverride is opposite", }, { urlValues: url.Values{"f": []string{"i"}}, - syncer: fakeSyncer{key: "a", forceOverride: "b"}, + syncer: fakeSyncer{key: "a", formatOverride: "b"}, expectedFormat: "b", - description: "parameter given as `i`, but force sync type is opposite", + description: "parameter given as `i`, but formatOverride is opposite", }, } @@ -1574,7 +1574,7 @@ func makeRequest(uri string, existingSyncs map[string]string) *http.Request { return request } -func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string, forceOverride string) *httptest.ResponseRecorder { +func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string, formatOverride string) *httptest.ResponseRecorder { cfg := config.Configuration{ AccountRequired: cfgAccountRequired, AccountDefaults: config.Account{}, @@ -1605,7 +1605,7 @@ func doRequest(req *http.Request, analytics analytics.Runner, metrics metrics.Me syncersByBidder := make(map[string]usersync.Syncer) for bidderName, syncerKey := range syncersBidderNameToKey { - syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame, forceOverride: forceOverride} + syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame, formatOverride: formatOverride} if priorityGroups == nil { cfg.UserSync.PriorityGroups = [][]string{{}} cfg.UserSync.PriorityGroups[0] = append(cfg.UserSync.PriorityGroups[0], bidderName) @@ -1696,7 +1696,7 @@ func (g *fakePermsSetUID) AuctionActivitiesAllowed(ctx context.Context, bidderCo type fakeSyncer struct { key string defaultSyncType usersync.SyncType - forceOverride string + formatOverride string } func (s fakeSyncer) Key() string { @@ -1716,7 +1716,7 @@ func (s fakeSyncer) GetSync(syncTypes []usersync.SyncType, privacyMacros macros. } func (s fakeSyncer) ForceResponseFormat() string { - return s.forceOverride + return s.formatOverride } func ToHTTPCookie(cookie *usersync.Cookie) (*http.Cookie, error) { diff --git a/usersync/chooser_test.go b/usersync/chooser_test.go index ee06741e4b0..e272e632ac1 100644 --- a/usersync/chooser_test.go +++ b/usersync/chooser_test.go @@ -671,7 +671,7 @@ type fakeSyncer struct { key string supportsIFrame bool supportsRedirect bool - forceOverride string + formatOverride string } func (s fakeSyncer) Key() string { @@ -699,7 +699,7 @@ func (fakeSyncer) GetSync([]SyncType, macros.UserSyncPrivacy) (Sync, error) { } func (s fakeSyncer) ForceResponseFormat() string { - return s.forceOverride + return s.formatOverride } type fakePrivacy struct { diff --git a/usersync/syncer.go b/usersync/syncer.go index 56ca578c3d5..b17950ac431 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -53,7 +53,7 @@ type standardSyncer struct { iframe *template.Template redirect *template.Template supportCORS bool - forceOverride string + formatOverride string } const ( @@ -76,7 +76,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st key: syncerConfig.Key, defaultSyncType: resolveDefaultSyncType(syncerConfig), supportCORS: syncerConfig.SupportCORS != nil && *syncerConfig.SupportCORS, - forceOverride: syncerConfig.FormatOverride, + formatOverride: syncerConfig.FormatOverride, } if syncerConfig.IFrame != nil { @@ -273,11 +273,11 @@ func (s standardSyncer) chooseTemplate(syncType SyncType) *template.Template { } func (s standardSyncer) ForceResponseFormat() string { - switch s.forceOverride { + switch s.formatOverride { case setuidSyncTypeIFrame: - return s.forceOverride + return s.formatOverride case setuidSyncTypeRedirect: - return s.forceOverride + return s.formatOverride default: return "" } diff --git a/usersync/syncer_test.go b/usersync/syncer_test.go index 589378c6be0..feec3bd6859 100644 --- a/usersync/syncer_test.go +++ b/usersync/syncer_test.go @@ -98,7 +98,6 @@ func TestNewSyncer(t *testing.T) { givenExternalURL: "http://syncer.com", givenIFrameConfig: iframeConfig, givenRedirectConfig: redirectConfig, - givenForceType: "i", expectedDefault: SyncTypeIFrame, expectedIFrame: "https://bidder.com/iframe?redirect=http%3A%2F%2Fsyncer.com%2Fhost", expectedRedirect: "https://bidder.com/redirect?redirect=http%3A%2F%2Fsyncer.com%2Fhost", @@ -107,12 +106,11 @@ func TestNewSyncer(t *testing.T) { for _, test := range testCases { syncerConfig := config.Syncer{ - Key: test.givenKey, - SupportCORS: &supportCORS, - IFrame: test.givenIFrameConfig, - Redirect: test.givenRedirectConfig, - ExternalURL: test.givenExternalURL, - FormatOverride: test.givenForceType, + Key: test.givenKey, + SupportCORS: &supportCORS, + IFrame: test.givenIFrameConfig, + Redirect: test.givenRedirectConfig, + ExternalURL: test.givenExternalURL, } result, err := NewSyncer(hostConfig, syncerConfig, test.givenBidderName) @@ -123,7 +121,6 @@ func TestNewSyncer(t *testing.T) { result := result.(standardSyncer) assert.Equal(t, test.givenKey, result.key, test.description+":key") assert.Equal(t, supportCORS, result.supportCORS, test.description+":cors") - assert.Equal(t, test.givenForceType, result.forceOverride, test.description+":cors") assert.Equal(t, test.expectedDefault, result.defaultSyncType, test.description+":default_sync") if test.expectedIFrame == "" { @@ -807,7 +804,7 @@ func TestSyncerForceResponseFormat(t *testing.T) { } for _, test := range testCases { - syncer := standardSyncer{forceOverride: test.givenForceType} + syncer := standardSyncer{formatOverride: test.givenForceType} forceType := syncer.ForceResponseFormat() assert.Equal(t, test.expectedForceType, forceType, test.description) } From 57a5f047e98db00dcb9db5edaa73d954f01b5915 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 3 Jan 2024 15:07:15 -0800 Subject: [PATCH 5/8] Forced Format impl updates --- endpoints/cookie_sync_test.go | 2 +- endpoints/setuid.go | 6 +----- endpoints/setuid_test.go | 29 +++++++++++++++++++++-------- usersync/chooser_test.go | 2 +- usersync/syncer.go | 28 ++++++++++++++++++---------- usersync/syncer_test.go | 4 ++-- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index b9baf50555c..b03659fe445 100644 --- a/endpoints/cookie_sync_test.go +++ b/endpoints/cookie_sync_test.go @@ -2094,7 +2094,7 @@ func (m *MockSyncer) Key() string { return args.String(0) } -func (m *MockSyncer) DefaultSyncType() usersync.SyncType { +func (m *MockSyncer) DefaultResponseFormat() usersync.SyncType { args := m.Called() return args.Get(0).(usersync.SyncType) } diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 7bf076b4cfe..0f4d36d35ac 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -358,15 +358,11 @@ func isSyncerPriority(bidderNameFromSyncerQuery string, priorityGroups [][]strin // Returns either "b" (iframe), "i" (redirect), or an empty string "" (legacy behavior of an // empty response body with no content type). func getResponseFormat(query url.Values, syncer usersync.Syncer) (string, error) { - if syncer.ForceResponseFormat() != "" { - return syncer.ForceResponseFormat(), nil - } - format, formatProvided := query["f"] formatEmpty := len(format) == 0 || format[0] == "" if !formatProvided || formatEmpty { - switch syncer.DefaultSyncType() { + switch syncer.DefaultResponseFormat() { case usersync.SyncTypeIFrame: return "b", nil case usersync.SyncTypeRedirect: diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 97364f24eb2..41ac58d77af 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -348,7 +348,7 @@ func TestSetUIDEndpoint(t *testing.T) { expectedBody: "invalid gpp_sid encoding, must be a csv list of integers", }, { - uri: "/setuid?bidder=pubmatic&uid=123&f=b", + uri: "/setuid?bidder=pubmatic&uid=123", syncersBidderNameToKey: map[string]string{"pubmatic": "pubmatic"}, existingSyncs: nil, gdprAllowsHostCookies: true, @@ -356,7 +356,7 @@ func TestSetUIDEndpoint(t *testing.T) { expectedSyncs: map[string]string{"pubmatic": "123"}, expectedStatusCode: http.StatusOK, expectedHeaders: map[string]string{"Content-Length": "86", "Content-Type": "image/png"}, - description: "Set uid for valid bidder with iframe format, but it's overriden by formatOverride", + description: "Format not provided in URL, but formatOverride is defined", }, } @@ -1474,16 +1474,22 @@ func TestGetResponseFormat(t *testing.T) { description: "parameter given is empty (by empty item), use default sync type redirect", }, { - urlValues: url.Values{"f": []string{"b"}}, + urlValues: url.Values{"f": []string{}}, syncer: fakeSyncer{key: "a", formatOverride: "i"}, expectedFormat: "i", - description: "parameter given as `b`, but formatOverride is opposite", + description: "format not provided, but formatOverride is defined, expect i", }, { - urlValues: url.Values{"f": []string{"i"}}, + urlValues: url.Values{"f": []string{}}, syncer: fakeSyncer{key: "a", formatOverride: "b"}, expectedFormat: "b", - description: "parameter given as `i`, but formatOverride is opposite", + description: "format not provided, but formatOverride is defined, expect b", + }, + { + urlValues: url.Values{"f": []string{}}, + syncer: fakeSyncer{key: "a", formatOverride: "b", defaultSyncType: usersync.SyncTypeRedirect}, + expectedFormat: "b", + description: "format not provided, default is defined but formatOverride is defined as well, expect b", }, } @@ -1713,8 +1719,15 @@ func (s fakeSyncer) Key() string { return s.key } -func (s fakeSyncer) DefaultSyncType() usersync.SyncType { - return s.defaultSyncType +func (s fakeSyncer) DefaultResponseFormat() usersync.SyncType { + switch s.formatOverride { + case "b": + return usersync.SyncTypeIFrame + case "i": + return usersync.SyncTypeRedirect + default: + return s.defaultSyncType + } } func (s fakeSyncer) SupportsType(syncTypes []usersync.SyncType) bool { diff --git a/usersync/chooser_test.go b/usersync/chooser_test.go index ba29f675717..513f158dfe3 100644 --- a/usersync/chooser_test.go +++ b/usersync/chooser_test.go @@ -724,7 +724,7 @@ func (s fakeSyncer) Key() string { return s.key } -func (s fakeSyncer) DefaultSyncType() SyncType { +func (s fakeSyncer) DefaultResponseFormat() SyncType { return SyncTypeIFrame } diff --git a/usersync/syncer.go b/usersync/syncer.go index b17950ac431..5afdc019d21 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -26,8 +26,8 @@ type Syncer interface { // necessarily, a one-to-one mapping with a bidder. Key() string - // DefaultSyncType is the default SyncType for this syncer. - DefaultSyncType() SyncType + // DefaultResponseFormat is the default SyncType for this syncer. + DefaultResponseFormat() SyncType // SupportsType returns true if the syncer supports at least one of the specified sync types. SupportsType(syncTypes []SyncType) bool @@ -35,9 +35,6 @@ type Syncer interface { // GetSync returns a user sync for the user's device to perform, or an error if the none of the // sync types are supported or if macro substitution fails. GetSync(syncTypes []SyncType, userSyncMacros macros.UserSyncPrivacy) (Sync, error) - - // ForceResponseFromat returns the callback format as specified in a bidders config - ForceResponseFormat() string } // Sync represents a user sync to be performed by the user's device. @@ -81,7 +78,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st if syncerConfig.IFrame != nil { var err error - syncer.iframe, err = buildTemplate(bidder, setuidSyncTypeIFrame, hostConfig, syncerConfig.ExternalURL, *syncerConfig.IFrame) + syncer.iframe, err = buildTemplate(bidder, setuidSyncTypeIFrame, hostConfig, syncerConfig.ExternalURL, *syncerConfig.IFrame, syncerConfig.FormatOverride) if err != nil { return nil, fmt.Errorf("iframe %v", err) } @@ -92,7 +89,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st if syncerConfig.Redirect != nil { var err error - syncer.redirect, err = buildTemplate(bidder, setuidSyncTypeRedirect, hostConfig, syncerConfig.ExternalURL, *syncerConfig.Redirect) + syncer.redirect, err = buildTemplate(bidder, setuidSyncTypeRedirect, hostConfig, syncerConfig.ExternalURL, *syncerConfig.Redirect, syncerConfig.FormatOverride) if err != nil { return nil, fmt.Errorf("redirect %v", err) } @@ -122,12 +119,16 @@ var ( macroRegex = regexp.MustCompile(`{{\s*\..*?\s*}}`) ) -func buildTemplate(bidderName, syncTypeValue string, hostConfig config.UserSync, syncerExternalURL string, syncerEndpoint config.SyncerEndpoint) (*template.Template, error) { +func buildTemplate(bidderName, syncTypeValue string, hostConfig config.UserSync, syncerExternalURL string, syncerEndpoint config.SyncerEndpoint, formatOverride string) (*template.Template, error) { redirectTemplate := syncerEndpoint.RedirectURL if redirectTemplate == "" { redirectTemplate = hostConfig.RedirectURL } + if formatOverride != "" { + syncTypeValue = formatOverride + } + externalURL := chooseExternalURL(syncerEndpoint.ExternalURL, syncerExternalURL, hostConfig.ExternalURL) redirectURL := macroRegexSyncerKey.ReplaceAllLiteralString(redirectTemplate, bidderName) @@ -194,8 +195,15 @@ func (s standardSyncer) Key() string { return s.key } -func (s standardSyncer) DefaultSyncType() SyncType { - return s.defaultSyncType +func (s standardSyncer) DefaultResponseFormat() SyncType { + switch s.formatOverride { + case setuidSyncTypeIFrame: + return SyncTypeIFrame + case setuidSyncTypeRedirect: + return SyncTypeRedirect + default: + return s.defaultSyncType + } } func (s standardSyncer) SupportsType(syncTypes []SyncType) bool { diff --git a/usersync/syncer_test.go b/usersync/syncer_test.go index feec3bd6859..70fbadf0277 100644 --- a/usersync/syncer_test.go +++ b/usersync/syncer_test.go @@ -323,7 +323,7 @@ func TestBuildTemplate(t *testing.T) { } for _, test := range testCases { - result, err := buildTemplate(key, syncTypeValue, hostConfig, test.givenSyncerExternalURL, test.givenSyncerEndpoint) + result, err := buildTemplate(key, syncTypeValue, hostConfig, test.givenSyncerExternalURL, test.givenSyncerEndpoint, "") if test.expectedError == "" { assert.NoError(t, err, test.description+":err") @@ -481,7 +481,7 @@ func TestSyncerKey(t *testing.T) { func TestSyncerDefaultSyncType(t *testing.T) { syncer := standardSyncer{defaultSyncType: SyncTypeRedirect} - assert.Equal(t, SyncTypeRedirect, syncer.DefaultSyncType()) + assert.Equal(t, SyncTypeRedirect, syncer.DefaultResponseFormat()) } func TestSyncerSupportsType(t *testing.T) { From 9dba7b16ef6cd6bcb8b689c3bbeee207bbea70b2 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Sun, 7 Jan 2024 11:05:07 -0800 Subject: [PATCH 6/8] Remove unused code, update tests --- config/bidderinfo.go | 9 ++++++ config/bidderinfo_test.go | 35 +++++++++++++++++++++++ usersync/syncer.go | 24 +++------------- usersync/syncer_test.go | 58 +++++++++++++++++++-------------------- 4 files changed, 77 insertions(+), 49 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index b48b6b01292..e60f439a30e 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -209,6 +209,11 @@ type InfoReaderFromDisk struct { Path string } +const ( + ResponseFormatIFrame = "b" // b = blank HTML response + ResponseFormatRedirect = "i" // i = image response +) + func (r InfoReaderFromDisk) Read() (map[string][]byte, error) { bidderConfigs, err := os.ReadDir(r.Path) if err != nil { @@ -565,6 +570,10 @@ func validateSyncer(bidderInfo BidderInfo) error { return nil } + if bidderInfo.Syncer.FormatOverride != ResponseFormatIFrame && bidderInfo.Syncer.FormatOverride != ResponseFormatRedirect && bidderInfo.Syncer.FormatOverride != "" { + return fmt.Errorf("syncer could not be created, invalid FormatOverride value: %s", bidderInfo.Syncer.FormatOverride) + } + for _, supports := range bidderInfo.Syncer.Supports { if !strings.EqualFold(supports, "iframe") && !strings.EqualFold(supports, "redirect") { return fmt.Errorf("syncer could not be created, invalid supported endpoint: %s", supports) diff --git a/config/bidderinfo_test.go b/config/bidderinfo_test.go index ee1b35d4994..6c965bb6d7c 100644 --- a/config/bidderinfo_test.go +++ b/config/bidderinfo_test.go @@ -593,6 +593,7 @@ func TestBidderInfoValidationPositive(t *testing.T) { URL: "http://bidderB.com/usersync", UserMacro: "UID", }, + FormatOverride: ResponseFormatRedirect, }, }, "bidderC": BidderInfo{ @@ -627,6 +628,9 @@ func TestBidderInfoValidationPositive(t *testing.T) { }, }, }, + Syncer: &Syncer{ + FormatOverride: ResponseFormatIFrame, + }, }, } errs := bidderInfos.validate(make([]error, 0)) @@ -1318,6 +1322,37 @@ func TestBidderInfoValidationNegative(t *testing.T) { errors.New("parent bidder: bidderC not found for an alias: bidderB"), }, }, + { + "Invalid format override value", + BidderInfos{ + "bidderB": BidderInfo{ + Endpoint: "http://bidderA.com/openrtb2", + Maintainer: &MaintainerInfo{ + Email: "maintainer@bidderA.com", + }, + Capabilities: &CapabilitiesInfo{ + App: &PlatformInfo{ + MediaTypes: []openrtb_ext.BidType{ + openrtb_ext.BidTypeBanner, + openrtb_ext.BidTypeNative, + }, + }, + Site: &PlatformInfo{ + MediaTypes: []openrtb_ext.BidType{ + openrtb_ext.BidTypeBanner, + openrtb_ext.BidTypeNative, + }, + }, + }, + Syncer: &Syncer{ + FormatOverride: "x", + }, + }, + }, + []error{ + errors.New("syncer could not be created, invalid FormatOverride value: x"), + }, + }, } for _, test := range testCases { diff --git a/usersync/syncer.go b/usersync/syncer.go index 5afdc019d21..cab274679be 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -53,11 +53,6 @@ type standardSyncer struct { formatOverride string } -const ( - setuidSyncTypeIFrame = "b" // b = blank HTML response - setuidSyncTypeRedirect = "i" // i = image response -) - // NewSyncer creates a new Syncer from the provided configuration, or return an error if macro substition // fails or an endpoint url is invalid. func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder string) (Syncer, error) { @@ -78,7 +73,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st if syncerConfig.IFrame != nil { var err error - syncer.iframe, err = buildTemplate(bidder, setuidSyncTypeIFrame, hostConfig, syncerConfig.ExternalURL, *syncerConfig.IFrame, syncerConfig.FormatOverride) + syncer.iframe, err = buildTemplate(bidder, config.ResponseFormatIFrame, hostConfig, syncerConfig.ExternalURL, *syncerConfig.IFrame, syncerConfig.FormatOverride) if err != nil { return nil, fmt.Errorf("iframe %v", err) } @@ -89,7 +84,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st if syncerConfig.Redirect != nil { var err error - syncer.redirect, err = buildTemplate(bidder, setuidSyncTypeRedirect, hostConfig, syncerConfig.ExternalURL, *syncerConfig.Redirect, syncerConfig.FormatOverride) + syncer.redirect, err = buildTemplate(bidder, config.ResponseFormatRedirect, hostConfig, syncerConfig.ExternalURL, *syncerConfig.Redirect, syncerConfig.FormatOverride) if err != nil { return nil, fmt.Errorf("redirect %v", err) } @@ -197,9 +192,9 @@ func (s standardSyncer) Key() string { func (s standardSyncer) DefaultResponseFormat() SyncType { switch s.formatOverride { - case setuidSyncTypeIFrame: + case config.ResponseFormatIFrame: return SyncTypeIFrame - case setuidSyncTypeRedirect: + case config.ResponseFormatRedirect: return SyncTypeRedirect default: return s.defaultSyncType @@ -279,14 +274,3 @@ func (s standardSyncer) chooseTemplate(syncType SyncType) *template.Template { return nil } } - -func (s standardSyncer) ForceResponseFormat() string { - switch s.formatOverride { - case setuidSyncTypeIFrame: - return s.formatOverride - case setuidSyncTypeRedirect: - return s.formatOverride - default: - return "" - } -} diff --git a/usersync/syncer_test.go b/usersync/syncer_test.go index 70fbadf0277..d7cc7c158d0 100644 --- a/usersync/syncer_test.go +++ b/usersync/syncer_test.go @@ -484,6 +484,35 @@ func TestSyncerDefaultSyncType(t *testing.T) { assert.Equal(t, SyncTypeRedirect, syncer.DefaultResponseFormat()) } +func TestSyncerDefaultResponseFormat(t *testing.T) { + testCases := []struct { + description string + givenSyncer standardSyncer + expectedSyncType SyncType + }{ + { + description: "IFrame", + givenSyncer: standardSyncer{formatOverride: config.ResponseFormatIFrame}, + expectedSyncType: SyncTypeIFrame, + }, + { + description: "Default with Redirect Override", + givenSyncer: standardSyncer{defaultSyncType: SyncTypeIFrame, formatOverride: config.ResponseFormatRedirect}, + expectedSyncType: SyncTypeRedirect, + }, + { + description: "Default with no override", + givenSyncer: standardSyncer{defaultSyncType: SyncTypeRedirect}, + expectedSyncType: SyncTypeRedirect, + }, + } + + for _, test := range testCases { + syncType := test.givenSyncer.DefaultResponseFormat() + assert.Equal(t, test.expectedSyncType, syncType, test.description) + } +} + func TestSyncerSupportsType(t *testing.T) { endpointTemplate := template.Must(template.New("test").Parse("iframe")) @@ -780,32 +809,3 @@ func TestSyncerChooseTemplate(t *testing.T) { assert.Equal(t, test.expectedTemplate, result, test.description) } } - -func TestSyncerForceResponseFormat(t *testing.T) { - testCases := []struct { - description string - givenForceType string - expectedForceType string - }{ - { - description: "IFrame", - givenForceType: "b", - expectedForceType: "b", - }, - { - description: "Redirect", - givenForceType: "i", - expectedForceType: "i", - }, - { - description: "Empty", - expectedForceType: "", - }, - } - - for _, test := range testCases { - syncer := standardSyncer{formatOverride: test.givenForceType} - forceType := syncer.ForceResponseFormat() - assert.Equal(t, test.expectedForceType, forceType, test.description) - } -} From 15d814d3886408a88dfb1f63ba71d882ba373557 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 9 Jan 2024 15:50:51 -0800 Subject: [PATCH 7/8] Remove unused code, change error msg --- config/bidderinfo.go | 2 +- config/bidderinfo_test.go | 2 +- endpoints/cookie_sync_test.go | 4 ---- endpoints/setuid_test.go | 4 ---- usersync/chooser_test.go | 4 ---- 5 files changed, 2 insertions(+), 14 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index e60f439a30e..1841ab76bf5 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -571,7 +571,7 @@ func validateSyncer(bidderInfo BidderInfo) error { } if bidderInfo.Syncer.FormatOverride != ResponseFormatIFrame && bidderInfo.Syncer.FormatOverride != ResponseFormatRedirect && bidderInfo.Syncer.FormatOverride != "" { - return fmt.Errorf("syncer could not be created, invalid FormatOverride value: %s", bidderInfo.Syncer.FormatOverride) + return fmt.Errorf("syncer could not be created, invalid format override value: %s", bidderInfo.Syncer.FormatOverride) } for _, supports := range bidderInfo.Syncer.Supports { diff --git a/config/bidderinfo_test.go b/config/bidderinfo_test.go index 6c965bb6d7c..b80e53764ef 100644 --- a/config/bidderinfo_test.go +++ b/config/bidderinfo_test.go @@ -1350,7 +1350,7 @@ func TestBidderInfoValidationNegative(t *testing.T) { }, }, []error{ - errors.New("syncer could not be created, invalid FormatOverride value: x"), + errors.New("syncer could not be created, invalid format override value: x"), }, }, } diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index b03659fe445..adfdcb22fab 100644 --- a/endpoints/cookie_sync_test.go +++ b/endpoints/cookie_sync_test.go @@ -2109,10 +2109,6 @@ func (m *MockSyncer) GetSync(syncTypes []usersync.SyncType, privacyMacros macros return args.Get(0).(usersync.Sync), args.Error(1) } -func (m *MockSyncer) ForceResponseFormat() string { - return "" -} - type MockAnalyticsRunner struct { mock.Mock } diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 41ac58d77af..ded528abeb3 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -1738,10 +1738,6 @@ func (s fakeSyncer) GetSync(syncTypes []usersync.SyncType, privacyMacros macros. return usersync.Sync{}, nil } -func (s fakeSyncer) ForceResponseFormat() string { - return s.formatOverride -} - func ToHTTPCookie(cookie *usersync.Cookie) (*http.Cookie, error) { encoder := usersync.Base64Encoder{} encodedCookie, err := encoder.Encode(cookie) diff --git a/usersync/chooser_test.go b/usersync/chooser_test.go index 513f158dfe3..604b5a6d07b 100644 --- a/usersync/chooser_test.go +++ b/usersync/chooser_test.go @@ -744,10 +744,6 @@ func (fakeSyncer) GetSync([]SyncType, macros.UserSyncPrivacy) (Sync, error) { return Sync{}, nil } -func (s fakeSyncer) ForceResponseFormat() string { - return s.formatOverride -} - type fakePrivacy struct { gdprAllowsHostCookie bool gdprAllowsBidderSync bool From 63b8c6ff2a16d4adf998ab33bb54c83dbc8e1aad Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 10 Jan 2024 15:16:42 -0800 Subject: [PATCH 8/8] constant name change --- config/bidderinfo.go | 6 +++--- config/bidderinfo_test.go | 4 ++-- usersync/syncer.go | 8 ++++---- usersync/syncer_test.go | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index 1841ab76bf5..d78f5722552 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -210,8 +210,8 @@ type InfoReaderFromDisk struct { } const ( - ResponseFormatIFrame = "b" // b = blank HTML response - ResponseFormatRedirect = "i" // i = image response + SyncResponseFormatIFrame = "b" // b = blank HTML response + SyncResponseFormatRedirect = "i" // i = image response ) func (r InfoReaderFromDisk) Read() (map[string][]byte, error) { @@ -570,7 +570,7 @@ func validateSyncer(bidderInfo BidderInfo) error { return nil } - if bidderInfo.Syncer.FormatOverride != ResponseFormatIFrame && bidderInfo.Syncer.FormatOverride != ResponseFormatRedirect && bidderInfo.Syncer.FormatOverride != "" { + if bidderInfo.Syncer.FormatOverride != SyncResponseFormatIFrame && bidderInfo.Syncer.FormatOverride != SyncResponseFormatRedirect && bidderInfo.Syncer.FormatOverride != "" { return fmt.Errorf("syncer could not be created, invalid format override value: %s", bidderInfo.Syncer.FormatOverride) } diff --git a/config/bidderinfo_test.go b/config/bidderinfo_test.go index b80e53764ef..3305e34e88a 100644 --- a/config/bidderinfo_test.go +++ b/config/bidderinfo_test.go @@ -593,7 +593,7 @@ func TestBidderInfoValidationPositive(t *testing.T) { URL: "http://bidderB.com/usersync", UserMacro: "UID", }, - FormatOverride: ResponseFormatRedirect, + FormatOverride: SyncResponseFormatRedirect, }, }, "bidderC": BidderInfo{ @@ -629,7 +629,7 @@ func TestBidderInfoValidationPositive(t *testing.T) { }, }, Syncer: &Syncer{ - FormatOverride: ResponseFormatIFrame, + FormatOverride: SyncResponseFormatIFrame, }, }, } diff --git a/usersync/syncer.go b/usersync/syncer.go index cab274679be..50985eca5be 100644 --- a/usersync/syncer.go +++ b/usersync/syncer.go @@ -73,7 +73,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st if syncerConfig.IFrame != nil { var err error - syncer.iframe, err = buildTemplate(bidder, config.ResponseFormatIFrame, hostConfig, syncerConfig.ExternalURL, *syncerConfig.IFrame, syncerConfig.FormatOverride) + syncer.iframe, err = buildTemplate(bidder, config.SyncResponseFormatIFrame, hostConfig, syncerConfig.ExternalURL, *syncerConfig.IFrame, syncerConfig.FormatOverride) if err != nil { return nil, fmt.Errorf("iframe %v", err) } @@ -84,7 +84,7 @@ func NewSyncer(hostConfig config.UserSync, syncerConfig config.Syncer, bidder st if syncerConfig.Redirect != nil { var err error - syncer.redirect, err = buildTemplate(bidder, config.ResponseFormatRedirect, hostConfig, syncerConfig.ExternalURL, *syncerConfig.Redirect, syncerConfig.FormatOverride) + syncer.redirect, err = buildTemplate(bidder, config.SyncResponseFormatRedirect, hostConfig, syncerConfig.ExternalURL, *syncerConfig.Redirect, syncerConfig.FormatOverride) if err != nil { return nil, fmt.Errorf("redirect %v", err) } @@ -192,9 +192,9 @@ func (s standardSyncer) Key() string { func (s standardSyncer) DefaultResponseFormat() SyncType { switch s.formatOverride { - case config.ResponseFormatIFrame: + case config.SyncResponseFormatIFrame: return SyncTypeIFrame - case config.ResponseFormatRedirect: + case config.SyncResponseFormatRedirect: return SyncTypeRedirect default: return s.defaultSyncType diff --git a/usersync/syncer_test.go b/usersync/syncer_test.go index d7cc7c158d0..c9c568e299a 100644 --- a/usersync/syncer_test.go +++ b/usersync/syncer_test.go @@ -492,12 +492,12 @@ func TestSyncerDefaultResponseFormat(t *testing.T) { }{ { description: "IFrame", - givenSyncer: standardSyncer{formatOverride: config.ResponseFormatIFrame}, + givenSyncer: standardSyncer{formatOverride: config.SyncResponseFormatIFrame}, expectedSyncType: SyncTypeIFrame, }, { description: "Default with Redirect Override", - givenSyncer: standardSyncer{defaultSyncType: SyncTypeIFrame, formatOverride: config.ResponseFormatRedirect}, + givenSyncer: standardSyncer{defaultSyncType: SyncTypeIFrame, formatOverride: config.SyncResponseFormatRedirect}, expectedSyncType: SyncTypeRedirect, }, {