Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pubmatic: Forward displaymanager and displaymanagerver from app extension to pubmatic ssp #4000

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+425 −14
Open

Pubmatic: Forward displaymanager and displaymanagerver from app extension to pubmatic ssp #4000

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f5553e
Forward displaymanager and displaymanagerver from app extension to pu…
AvinashKapre Oct 20, 2024
38ea889
test cases
AvinashKapre Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions adapters/pubmatic/pubmatic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ func (a *PubmaticAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ad
extractWrapperExtFromImp := true
extractPubIDFromImp := true

displayManager, displayManagerVer := "", ""
if request.App != nil && request.App.Ext != nil {
displayManager, displayManagerVer = getDisplayManagerAndVer(request.App)
}

newReqExt, err := extractPubmaticExtFromRequest(request)
if err != nil {
return nil, []error{err}
Expand All @@ -99,7 +104,7 @@ func (a *PubmaticAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ad
}

for i := 0; i < len(request.Imp); i++ {
wrapperExtFromImp, pubIDFromImp, err := parseImpressionObject(&request.Imp[i], extractWrapperExtFromImp, extractPubIDFromImp)
wrapperExtFromImp, pubIDFromImp, err := parseImpressionObject(&request.Imp[i], extractWrapperExtFromImp, extractPubIDFromImp, displayManager, displayManagerVer)

// If the parsing is failed, remove imp and add the error.
if err != nil {
Expand Down Expand Up @@ -246,7 +251,7 @@ func assignBannerWidthAndHeight(banner *openrtb2.Banner, w, h int64) *openrtb2.B
}

// parseImpressionObject parse the imp to get it ready to send to pubmatic
func parseImpressionObject(imp *openrtb2.Imp, extractWrapperExtFromImp, extractPubIDFromImp bool) (*pubmaticWrapperExt, string, error) {
func parseImpressionObject(imp *openrtb2.Imp, extractWrapperExtFromImp, extractPubIDFromImp bool, displayManager, displayManagerVer string) (*pubmaticWrapperExt, string, error) {
var wrapExt *pubmaticWrapperExt
var pubID string

Expand All @@ -259,6 +264,12 @@ func parseImpressionObject(imp *openrtb2.Imp, extractWrapperExtFromImp, extractP
imp.Audio = nil
}

// Populate imp.displaymanager and imp.displaymanagerver if the SDK failed to do it.
if imp.DisplayManager == "" && imp.DisplayManagerVer == "" && displayManager != "" && displayManagerVer != "" {
imp.DisplayManager = displayManager
imp.DisplayManagerVer = displayManagerVer
}

var bidderExt ExtImpBidderPubmatic
if err := json.Unmarshal(imp.Ext, &bidderExt); err != nil {
return wrapExt, pubID, err
Expand Down Expand Up @@ -655,3 +666,19 @@ func Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server co
}
return bidder, nil
}

// getDisplayManagerAndVer returns the display manager and version from the request.app.ext or request.app.prebid.ext source and version
func getDisplayManagerAndVer(app *openrtb2.App) (string, string) {
if source, err := jsonparser.GetString(app.Ext, openrtb_ext.PrebidExtKey, "source"); err == nil && source != "" {
if version, err := jsonparser.GetString(app.Ext, openrtb_ext.PrebidExtKey, "version"); err == nil && version != "" {
return source, version
}
}

if source, err := jsonparser.GetString(app.Ext, "source"); err == nil && source != "" {
if version, err := jsonparser.GetString(app.Ext, "version"); err == nil && version != "" {
return source, version
}
}
return "", ""
}
224 changes: 212 additions & 12 deletions adapters/pubmatic/pubmatic_test.go
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coded unit tests are fine, but we look at the json test coverage since that includes additional assertions for memory safety. Please add json tests to cover this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the test cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SyntaxNode Can you please review the latest changes?

Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,22 @@ func TestParseImpressionObject(t *testing.T) {
imp *openrtb2.Imp
extractWrapperExtFromImp bool
extractPubIDFromImp bool
displayManager string
displayManagerVer string
}
type want struct {
bidfloor float64
impExt json.RawMessage
displayManager string
displayManagerVer string
}
tests := []struct {
name string
args args
expectedWrapperExt *pubmaticWrapperExt
expectedPublisherId string
want want
wantErr bool
expectedBidfloor float64
}{
{
name: "imp.bidfloor empty and kadfloor set",
Expand All @@ -97,7 +105,9 @@ func TestParseImpressionObject(t *testing.T) {
Ext: json.RawMessage(`{"bidder":{"kadfloor":"0.12"}}`),
},
},
expectedBidfloor: 0.12,
want: want{
bidfloor: 0.12,
},
},
{
name: "imp.bidfloor set and kadfloor empty",
Expand All @@ -108,7 +118,9 @@ func TestParseImpressionObject(t *testing.T) {
Ext: json.RawMessage(`{"bidder":{}}`),
},
},
expectedBidfloor: 0.12,
want: want{
bidfloor: 0.12,
},
},
{
name: "imp.bidfloor set and kadfloor invalid",
Expand All @@ -119,8 +131,9 @@ func TestParseImpressionObject(t *testing.T) {
Ext: json.RawMessage(`{"bidder":{"kadfloor":"aaa"}}`),
},
},
expectedBidfloor: 0.12,
},
want: want{
bidfloor: 0.12,
}},
{
name: "imp.bidfloor set and kadfloor set, higher imp.bidfloor",
args: args{
Expand All @@ -130,8 +143,9 @@ func TestParseImpressionObject(t *testing.T) {
Ext: json.RawMessage(`{"bidder":{"kadfloor":"0.11"}}`),
},
},
expectedBidfloor: 0.12,
},
want: want{
bidfloor: 0.12,
}},
{
name: "imp.bidfloor set and kadfloor set, higher kadfloor",
args: args{
Expand All @@ -141,8 +155,9 @@ func TestParseImpressionObject(t *testing.T) {
Ext: json.RawMessage(`{"bidder":{"kadfloor":"0.13"}}`),
},
},
expectedBidfloor: 0.13,
},
want: want{
bidfloor: 0.13,
}},
{
name: "kadfloor string set with whitespace",
args: args{
Expand All @@ -152,16 +167,72 @@ func TestParseImpressionObject(t *testing.T) {
Ext: json.RawMessage(`{"bidder":{"kadfloor":" \t 0.13 "}}`),
},
},
expectedBidfloor: 0.13,
want: want{
bidfloor: 0.13,
}},
{
name: "Populate imp.displaymanager and imp.displaymanagerver if both are empty in imp",
args: args{
imp: &openrtb2.Imp{
Video: &openrtb2.Video{},
Ext: json.RawMessage(`{"bidder":{"kadfloor":"0.12"}}`),
},
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
want: want{
bidfloor: 0.12,
impExt: json.RawMessage(nil),
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
},
{
name: "do not populate imp.displaymanager and imp.displaymanagerver in imp if only displaymanager or displaymanagerver is present in args",
args: args{
imp: &openrtb2.Imp{
Video: &openrtb2.Video{},
Ext: json.RawMessage(`{"bidder":{"kadfloor":"0.12"}}`),
DisplayManagerVer: "1.0.0",
},
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
want: want{
bidfloor: 0.12,
impExt: json.RawMessage(nil),
displayManagerVer: "1.0.0",
},
},
{
name: "do not populate imp.displaymanager and imp.displaymanagerver if already present in imp",
args: args{
imp: &openrtb2.Imp{
Video: &openrtb2.Video{},
Ext: json.RawMessage(`{"bidder":{"kadfloor":"0.12"}}`),
DisplayManager: "prebid-mobile",
DisplayManagerVer: "1.0.0",
},
displayManager: "prebid-android",
displayManagerVer: "2.0.0",
},
want: want{
bidfloor: 0.12,
impExt: json.RawMessage(nil),
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
receivedWrapperExt, receivedPublisherId, err := parseImpressionObject(tt.args.imp, tt.args.extractWrapperExtFromImp, tt.args.extractPubIDFromImp)
receivedWrapperExt, receivedPublisherId, err := parseImpressionObject(tt.args.imp, tt.args.extractWrapperExtFromImp, tt.args.extractPubIDFromImp, tt.args.displayManager, tt.args.displayManagerVer)
assert.Equal(t, tt.wantErr, err != nil)
assert.Equal(t, tt.expectedWrapperExt, receivedWrapperExt)
assert.Equal(t, tt.expectedPublisherId, receivedPublisherId)
assert.Equal(t, tt.expectedBidfloor, tt.args.imp.BidFloor)
assert.Equal(t, tt.want.bidfloor, tt.args.imp.BidFloor)
assert.Equal(t, tt.want.displayManager, tt.args.imp.DisplayManager)
assert.Equal(t, tt.want.displayManagerVer, tt.args.imp.DisplayManagerVer)
})
}
}
Expand Down Expand Up @@ -716,3 +787,132 @@ func TestGetMapFromJSON(t *testing.T) {
})
}
}

func TestGetDisplayManagerAndVer(t *testing.T) {
type args struct {
app *openrtb2.App
}
type want struct {
displayManager string
displayManagerVer string
}
tests := []struct {
name string
args args
want want
}{
{
name: "request app object is not nil but app.ext has no source and version",
args: args{

app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{}`),
},
},
want: want{
displayManager: "",
displayManagerVer: "",
},
},
{
name: "request app object is not nil and app.ext has source and version",
args: args{

app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"source":"prebid-mobile","version":"1.0.0"}`),
},
},
want: want{
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
},
{
name: "request app object is not nil and app.ext.prebid has source and version",
args: args{
app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"prebid":{"source":"prebid-mobile","version":"1.0.0"}}`),
},
},
want: want{
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
},
{
name: "request app object is not nil and app.ext has only version",
args: args{
app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"version":"1.0.0"}`),
},
},
want: want{
displayManager: "",
displayManagerVer: "",
},
},
{
name: "request app object is not nil and app.ext has only source",
args: args{
app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"source":"prebid-mobile"}`),
},
},
want: want{
displayManager: "",
displayManagerVer: "",
},
},
{
name: "request app object is not nil and app.ext have empty source but version is present",
args: args{
app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"source":"", "version":"1.0.0"}`),
},
},
want: want{
displayManager: "",
displayManagerVer: "",
},
},
{
name: "request app object is not nil and app.ext have empty version but source is present",
args: args{
app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"source":"prebid-mobile", "version":""}`),
},
},
want: want{
displayManager: "",
displayManagerVer: "",
},
},
{
name: "request app object is not nil and both app.ext and app.ext.prebid have source and version",
args: args{
app: &openrtb2.App{
Name: "AutoScout24",
Ext: json.RawMessage(`{"source":"prebid-mobile-android","version":"2.0.0","prebid":{"source":"prebid-mobile","version":"1.0.0"}}`),
},
},
want: want{
displayManager: "prebid-mobile",
displayManagerVer: "1.0.0",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
displayManager, displayManagerVer := getDisplayManagerAndVer(tt.args.app)
assert.Equal(t, tt.want.displayManager, displayManager)
assert.Equal(t, tt.want.displayManagerVer, displayManagerVer)
})
}
}
Loading