From ce331a7d67fb26a09a4c3a15ec02fc75a2935c76 Mon Sep 17 00:00:00 2001 From: Scott Kay Date: Thu, 8 Aug 2024 14:28:25 -0400 Subject: [PATCH] Fix: Aliases Of Bidders With UserSync Supports Declared Only (#3850) --- config/bidderinfo.go | 17 ++++++- config/bidderinfo_test.go | 101 +++++++++++++++++++++++++++++++++++++ usersync/syncersbuilder.go | 6 +-- 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/config/bidderinfo.go b/config/bidderinfo.go index 6e38fc0c1db..6352728de5c 100644 --- a/config/bidderinfo.go +++ b/config/bidderinfo.go @@ -201,6 +201,21 @@ func (bi BidderInfo) IsEnabled() bool { return !bi.Disabled } +// Defined returns true if at least one field exists, except for the supports field. +func (s *Syncer) Defined() bool { + if s == nil { + return false + } + + return s.Key != "" || + s.IFrame != nil || + s.Redirect != nil || + s.ExternalURL != "" || + s.SupportCORS != nil || + s.FormatOverride != "" || + s.SkipWhen != nil +} + type InfoReader interface { Read() (map[string][]byte, error) } @@ -335,7 +350,7 @@ func processBidderAliases(aliasNillableFieldsByBidder map[string]aliasNillableFi if aliasBidderInfo.PlatformID == "" { aliasBidderInfo.PlatformID = parentBidderInfo.PlatformID } - if aliasBidderInfo.Syncer == nil && parentBidderInfo.Syncer != nil { + if aliasBidderInfo.Syncer == nil && parentBidderInfo.Syncer.Defined() { syncerKey := aliasBidderInfo.AliasOf if parentBidderInfo.Syncer.Key != "" { syncerKey = parentBidderInfo.Syncer.Key diff --git a/config/bidderinfo_test.go b/config/bidderinfo_test.go index 3305e34e88a..3837cfed815 100644 --- a/config/bidderinfo_test.go +++ b/config/bidderinfo_test.go @@ -8,6 +8,7 @@ import ( "gopkg.in/yaml.v3" "github.com/prebid/prebid-server/v2/openrtb_ext" + "github.com/prebid/prebid-server/v2/util/ptrutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -401,6 +402,15 @@ func TestProcessAliasBidderInfo(t *testing.T) { Key: "bidderA", } + parentWithSyncerSupports := parentWithoutSyncerKey + parentWithSyncerSupports.Syncer = &Syncer{ + Supports: []string{"iframe"}, + } + + aliasWithoutSyncer := parentWithoutSyncerKey + aliasWithoutSyncer.AliasOf = "bidderA" + aliasWithoutSyncer.Syncer = nil + testCases := []struct { description string aliasInfos map[string]aliasNillableFields @@ -428,6 +438,26 @@ func TestProcessAliasBidderInfo(t *testing.T) { expectedErr: nil, expectedBidderInfos: BidderInfos{"bidderA": parentWithSyncerKey, "bidderB": bidderB}, }, + { + description: "inherit all parent info in alias bidder, except for syncer is parent only defines supports", + aliasInfos: map[string]aliasNillableFields{ + "bidderB": { + Disabled: nil, + ModifyingVastXmlAllowed: nil, + Experiment: nil, + XAPI: nil, + }, + }, + bidderInfos: BidderInfos{ + "bidderA": parentWithSyncerSupports, + "bidderB": BidderInfo{ + AliasOf: "bidderA", + // all other fields should be inherited from parent bidder, except for syncer + }, + }, + expectedErr: nil, + expectedBidderInfos: BidderInfos{"bidderA": parentWithSyncerSupports, "bidderB": aliasWithoutSyncer}, + }, { description: "inherit all parent info in alias bidder, use parent name as syncer alias key", aliasInfos: map[string]aliasNillableFields{ @@ -1522,6 +1552,77 @@ func TestSyncerEndpointOverride(t *testing.T) { } } +func TestSyncerDefined(t *testing.T) { + testCases := []struct { + name string + givenSyncer *Syncer + expected bool + }{ + { + name: "nil", + givenSyncer: nil, + expected: false, + }, + { + name: "empty", + givenSyncer: &Syncer{}, + expected: false, + }, + { + name: "key-only", + givenSyncer: &Syncer{Key: "anyKey"}, + expected: true, + }, + { + name: "iframe-only", + givenSyncer: &Syncer{IFrame: &SyncerEndpoint{}}, + expected: true, + }, + { + name: "redirect-only", + givenSyncer: &Syncer{Redirect: &SyncerEndpoint{}}, + expected: true, + }, + { + name: "externalurl-only", + givenSyncer: &Syncer{ExternalURL: "anyURL"}, + expected: true, + }, + { + name: "supportscors-only", + givenSyncer: &Syncer{SupportCORS: ptrutil.ToPtr(false)}, + expected: true, + }, + { + name: "formatoverride-only", + givenSyncer: &Syncer{FormatOverride: "anyFormat"}, + expected: true, + }, + { + name: "skipwhen-only", + givenSyncer: &Syncer{SkipWhen: &SkipWhen{}}, + expected: true, + }, + { + name: "supports-only", + givenSyncer: &Syncer{Supports: []string{"anySupports"}}, + expected: false, + }, + { + name: "supports-with-other", + givenSyncer: &Syncer{Key: "anyKey", Supports: []string{"anySupports"}}, + expected: true, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + result := test.givenSyncer.Defined() + assert.Equal(t, test.expected, result) + }) + } +} + func TestApplyBidderInfoConfigSyncerOverrides(t *testing.T) { var ( givenFileSystem = BidderInfos{"a": {Syncer: &Syncer{Key: "original"}}} diff --git a/usersync/syncersbuilder.go b/usersync/syncersbuilder.go index 9c916b821b4..ed86ef02efd 100644 --- a/usersync/syncersbuilder.go +++ b/usersync/syncersbuilder.go @@ -84,13 +84,9 @@ func shouldCreateSyncer(cfg config.BidderInfo) bool { return false } - if cfg.Syncer == nil { - return false - } - // a syncer may provide just a Supports field to provide hints to the host. we should only try to create a syncer // if there is at least one non-Supports value populated. - return cfg.Syncer.Key != "" || cfg.Syncer.IFrame != nil || cfg.Syncer.Redirect != nil || cfg.Syncer.SupportCORS != nil + return cfg.Syncer.Defined() } func chooseSyncerConfig(biddersSyncerConfig []namedSyncerConfig) (namedSyncerConfig, error) {