Skip to content

Commit

Permalink
Fix: Aliases Of Bidders With UserSync Supports Declared Only (#3850)
Browse files Browse the repository at this point in the history
  • Loading branch information
SyntaxNode authored Aug 8, 2024
1 parent 211f13a commit ce331a7
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 6 deletions.
17 changes: 16 additions & 1 deletion config/bidderinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
101 changes: 101 additions & 0 deletions config/bidderinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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"}}}
Expand Down
6 changes: 1 addition & 5 deletions usersync/syncersbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit ce331a7

Please sign in to comment.