Skip to content

Commit

Permalink
only inherit parent syncer key in alias syncer cfg
Browse files Browse the repository at this point in the history
As of now, in processBidderAliases if an alias has not defined syncer cfg then PBS was inheriting entire syncer cfg of parent.

Now, if a parent has syncer cfg with key, endpoints etc then alias was inheriting syncer entire parent cfg including key. This is incorrect because as per syncer rule, bidders can have same key but only one of them should define endpoints.

Therefore inheriting entire syncer cfg of parent resulted in validation error on PBS server startup. To fix this, commit updates processBidderAliases to only inherit parent key in alias syncer cfg.
  • Loading branch information
onkarvhanumante committed Sep 26, 2023
1 parent 2358273 commit dd12c58
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 16 deletions.
9 changes: 7 additions & 2 deletions config/bidderinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,13 @@ func processBidderAliases(aliasNillableFieldsByBidder map[string]aliasNillableFi
if aliasBidderInfo.PlatformID == "" {
aliasBidderInfo.PlatformID = parentBidderInfo.PlatformID
}
if aliasBidderInfo.Syncer == nil {
aliasBidderInfo.Syncer = parentBidderInfo.Syncer
if aliasBidderInfo.Syncer == nil && parentBidderInfo.Syncer != nil {
syncerKey := aliasBidderInfo.AliasOf
if parentBidderInfo.Syncer.Key != "" {
syncerKey = parentBidderInfo.Syncer.Key
}
syncer := Syncer{Key: syncerKey}
aliasBidderInfo.Syncer = &syncer
}
if aliasBidderInfo.UserSyncURL == "" {
aliasBidderInfo.UserSyncURL = parentBidderInfo.UserSyncURL
Expand Down
100 changes: 86 additions & 14 deletions config/bidderinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,6 @@ func TestProcessBidderInfo(t *testing.T) {
PlatformID: "123",
Syncer: &Syncer{
Key: "foo",
IFrame: &SyncerEndpoint{
URL: "https://foo.com/sync?mode=iframe&r={{.RedirectURL}}",
RedirectURL: "https://redirect/setuid/iframe",
ExternalURL: "https://iframe.host",
UserMacro: "UID",
},
},
UserSyncURL: "user-url",
XAPI: AdapterXAPI{
Expand All @@ -281,7 +275,7 @@ func TestProcessBidderInfo(t *testing.T) {
}

func TestProcessAliasBidderInfo(t *testing.T) {
parentBidderInfo := BidderInfo{
parentWithSyncerKey := BidderInfo{
AppSecret: "app-secret",
Capabilities: &CapabilitiesInfo{
App: &PlatformInfo{
Expand Down Expand Up @@ -377,8 +371,66 @@ func TestProcessAliasBidderInfo(t *testing.T) {
Tracker: "alias-tracker",
},
}
bidderB := parentBidderInfo
bidderB := parentWithSyncerKey
bidderB.AliasOf = "bidderA"
bidderB.Syncer = &Syncer{
Key: bidderB.Syncer.Key,
}

parentWithoutSyncerKey := BidderInfo{
AppSecret: "app-secret",
Capabilities: &CapabilitiesInfo{
App: &PlatformInfo{
MediaTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner, openrtb_ext.BidTypeVideo, openrtb_ext.BidTypeNative},
},
Site: &PlatformInfo{
MediaTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner, openrtb_ext.BidTypeVideo, openrtb_ext.BidTypeNative},
},
},
Debug: &DebugInfo{
Allow: true,
},
Disabled: false,
Endpoint: "https://endpoint.com",
EndpointCompression: "GZIP",
Experiment: BidderInfoExperiment{
AdsCert: BidderAdsCert{
Enabled: true,
},
},
ExtraAdapterInfo: "extra-info",
GVLVendorID: 42,
Maintainer: &MaintainerInfo{
Email: "some-email@domain.com",
},
ModifyingVastXmlAllowed: true,
OpenRTB: &OpenRTBInfo{
GPPSupported: true,
Version: "2.6",
},
PlatformID: "123",
Syncer: &Syncer{
IFrame: &SyncerEndpoint{
URL: "https://foo.com/sync?mode=iframe&r={{.RedirectURL}}",
RedirectURL: "https://redirect/setuid/iframe",
ExternalURL: "https://iframe.host",
UserMacro: "UID",
},
},
UserSyncURL: "user-url",
XAPI: AdapterXAPI{
Username: "uname",
Password: "pwd",
Tracker: "tracker",
},
}

bidderC := parentWithoutSyncerKey
bidderC.AliasOf = "bidderA"
bidderC.Syncer = &Syncer{
Key: "bidderA",
}

testCases := []struct {
description string
aliasInfos map[string]aliasNillableFields
Expand All @@ -387,7 +439,7 @@ func TestProcessAliasBidderInfo(t *testing.T) {
expectedErr error
}{
{
description: "inherit all parent info in alias bidder",
description: "inherit all parent info in alias bidder, use parent syncer key as syncer alias key",
aliasInfos: map[string]aliasNillableFields{
"bidderB": {
Disabled: nil,
Expand All @@ -397,14 +449,34 @@ func TestProcessAliasBidderInfo(t *testing.T) {
},
},
bidderInfos: BidderInfos{
"bidderA": parentBidderInfo,
"bidderA": parentWithSyncerKey,
"bidderB": BidderInfo{
AliasOf: "bidderA",
// all other fields should be inherited from parent bidder
},
},
expectedErr: nil,
expectedBidderInfos: BidderInfos{"bidderA": parentBidderInfo, "bidderB": bidderB},
expectedBidderInfos: BidderInfos{"bidderA": parentWithSyncerKey, "bidderB": bidderB},
},
{
description: "inherit all parent info in alias bidder, use parent name as syncer alias key",
aliasInfos: map[string]aliasNillableFields{
"bidderC": {
Disabled: nil,
ModifyingVastXmlAllowed: nil,
Experiment: nil,
XAPI: nil,
},
},
bidderInfos: BidderInfos{
"bidderA": parentWithoutSyncerKey,
"bidderC": BidderInfo{
AliasOf: "bidderA",
// all other fields should be inherited from parent bidder
},
},
expectedErr: nil,
expectedBidderInfos: BidderInfos{"bidderA": parentWithoutSyncerKey, "bidderC": bidderC},
},
{
description: "all bidder info specified for alias, do not inherit from parent bidder",
Expand All @@ -417,11 +489,11 @@ func TestProcessAliasBidderInfo(t *testing.T) {
},
},
bidderInfos: BidderInfos{
"bidderA": parentBidderInfo,
"bidderA": parentWithSyncerKey,
"bidderB": aliasBidderInfo,
},
expectedErr: nil,
expectedBidderInfos: BidderInfos{"bidderA": parentBidderInfo, "bidderB": aliasBidderInfo},
expectedBidderInfos: BidderInfos{"bidderA": parentWithSyncerKey, "bidderB": aliasBidderInfo},
},
{
description: "invalid alias",
Expand Down Expand Up @@ -449,7 +521,7 @@ func TestProcessAliasBidderInfo(t *testing.T) {
if test.expectedErr != nil {
assert.Equal(t, test.expectedErr, err)
} else {
assert.Equal(t, test.expectedBidderInfos, bidderInfos)
assert.Equal(t, test.expectedBidderInfos, bidderInfos, test.description)
}
}
}
Expand Down

0 comments on commit dd12c58

Please sign in to comment.