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

Rubicon: Pass PBS host info to XAPI #3903

Merged
merged 3 commits into from
Sep 23, 2024
Merged

Rubicon: Pass PBS host info to XAPI #3903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5374e60
Added pbs host info for XAPI.
And1sS Sep 4, 2024
38d3d59
Fixed json tests.
And1sS Sep 4, 2024
06f32e8
Added unit test.
And1sS Sep 4, 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
12 changes: 10 additions & 2 deletions adapters/rubicon/rubicon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package rubicon
import (
"encoding/json"
"fmt"
"github.com/prebid/prebid-server/v2/version"
"net/http"
"net/url"
"strconv"
Expand All @@ -25,6 +26,7 @@ var bannerExtContent = []byte(`{"rp":{"mime":"text/html"}}`)

type RubiconAdapter struct {
URI string
externalURI string
XAPIUsername string
XAPIPassword string
}
Expand Down Expand Up @@ -219,6 +221,7 @@ func Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server co

bidder := &RubiconAdapter{
URI: uri,
externalURI: server.ExternalUrl,
XAPIUsername: config.XAPI.Username,
XAPIPassword: config.XAPI.Password,
}
Expand Down Expand Up @@ -271,7 +274,7 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ada
rubiconRequest := *request
for imp, bidderExt := range impsToExtMap {
rubiconExt := bidderExt.Bidder
target, err := updateImpRpTargetWithFpdAttributes(bidderExt, rubiconExt, *imp, request.Site, request.App)
target, err := a.updateImpRpTarget(bidderExt, rubiconExt, *imp, request.Site, request.App)
if err != nil {
errs = append(errs, err)
continue
Expand Down Expand Up @@ -650,7 +653,7 @@ func resolveBidFloor(bidFloor float64, bidFloorCur string, reqInfo *adapters.Ext
return bidFloor, nil
}

func updateImpRpTargetWithFpdAttributes(extImp rubiconExtImpBidder, extImpRubicon openrtb_ext.ExtImpRubicon,
func (a *RubiconAdapter) updateImpRpTarget(extImp rubiconExtImpBidder, extImpRubicon openrtb_ext.ExtImpRubicon,
imp openrtb2.Imp, site *openrtb2.Site, app *openrtb2.App) (json.RawMessage, error) {

existingTarget, _, _, err := jsonparser.Get(imp.Ext, "rp", "target")
Expand Down Expand Up @@ -743,6 +746,11 @@ func updateImpRpTargetWithFpdAttributes(extImp rubiconExtImpBidder, extImpRubico
if len(extImpRubicon.Keywords) > 0 {
addStringArrayAttribute(extImpRubicon.Keywords, target, "keywords")
}

target["pbs_login"] = a.XAPIUsername
target["pbs_version"] = version.Ver
target["pbs_url"] = a.externalURI

updatedTarget, err := json.Marshal(target)
if err != nil {
return nil, err
Expand Down
69 changes: 68 additions & 1 deletion adapters/rubicon/rubicon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,73 @@ func TestOpenRTBFirstPartyDataPopulating(t *testing.T) {
}
}

func TestPbsHostInfoPopulating(t *testing.T) {
bidder := RubiconAdapter{
URI: "url",
externalURI: "externalUrl",
XAPIUsername: "username",
XAPIPassword: "password",
}

request := &openrtb2.BidRequest{
ID: "test-request-id",
Imp: []openrtb2.Imp{{
ID: "test-imp-id",
Banner: &openrtb2.Banner{
Format: []openrtb2.Format{
{W: 300, H: 250},
},
},
Ext: json.RawMessage(`{
"bidder": {
"zoneId": 8394,
"siteId": 283282,
"accountId": 7891,
"inventory": {"key1" : "val1"},
"visitor": {"key2" : "val2"}
}
}`),
}},
App: &openrtb2.App{
ID: "com.test",
Name: "testApp",
},
}

reqs, _ := bidder.MakeRequests(request, &adapters.ExtraRequestInfo{})

rubiconReq := &openrtb2.BidRequest{}
if err := json.Unmarshal(reqs[0].Body, rubiconReq); err != nil {
t.Fatalf("Unexpected error while decoding request: %s", err)
}

var rpImpExt rubiconImpExt
if err := json.Unmarshal(rubiconReq.Imp[0].Ext, &rpImpExt); err != nil {
t.Fatalf("Error unmarshalling imp.ext: %s", err)
}

var pbsLogin string
pbsLogin, err := jsonparser.GetString(rpImpExt.RP.Target, "pbs_login")
if err != nil {
t.Fatal("Error extracting pbs_login")
}
assert.Equal(t, pbsLogin, "username", "Unexpected pbs_login value")

var pbsVersion string
pbsVersion, err = jsonparser.GetString(rpImpExt.RP.Target, "pbs_version")
if err != nil {
t.Fatal("Error extracting pbs_version")
}
assert.Equal(t, pbsVersion, "", "Unexpected pbs_version value")
Copy link
Member Author

Choose a reason for hiding this comment

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

Any ideas on how to actually test version?
In pbs Java we just mock it. But it seems like here nobody tests it.
I can set version.Ver to test value at the beginning of test and defer its reset, but it is a dirty hack.

Copy link
Collaborator

@bsardo bsardo Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure there is any value in adding any additional test logic here, nor do I see an easy way to fake it. The version is just a string and is populated during the build process. Checking that it is empty seems sufficient to me since it will predictably always be an empty string in your unit tests.
We already have test coverage for the version package including the Ver field in endpoints/version_test.go so we can feel confident that Ver is set correctly on startup assuming the build is triggered with the version as a parameter.


var pbsUrl string
pbsUrl, err = jsonparser.GetString(rpImpExt.RP.Target, "pbs_url")
if err != nil {
t.Fatal("Error extracting pbs_url")
}
assert.Equal(t, pbsUrl, "externalUrl", "Unexpected pbs_url value")
}

func TestOpenRTBRequestWithBadvOverflowed(t *testing.T) {
bidder := new(RubiconAdapter)

Expand Down Expand Up @@ -990,7 +1057,7 @@ func TestOpenRTBResponseOverridePriceFromCorrespondingImp(t *testing.T) {
"siteId": 68780,
"zoneId": 327642,
"debug": {
"cpmoverride" : 20
"cpmoverride" : 20
}
}}`),
}},
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/25-26-transition-period.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/app-imp-fpd.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,10 @@
"sectioncat": [
"sectionCat1",
"sectionCat2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
10 changes: 8 additions & 2 deletions adapters/rubicon/rubicontest/exemplary/bidonmultiformat.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@
],
"search": [
"someSearch"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down Expand Up @@ -213,7 +216,10 @@
],
"search": [
"someSearch"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/flexible-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,10 @@
"sectioncat": [
"sectionCat1",
"sectionCat2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/hardcode-secure.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/non-bidonmultiformat.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@
],
"search": [
"someSearch"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/simple-banner.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/simple-native.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/simple-video.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/site-imp-fpd.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,10 @@
"sectionCat1",
"sectionCat2"
],
"dfp_ad_unit_code": "adSlotFromData"
"dfp_ad_unit_code": "adSlotFromData",
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/exemplary/user-fpd.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,10 @@
"search": [
"someSearch"
],
"dfp_ad_unit_code": "someAdSlot"
"dfp_ad_unit_code": "someAdSlot",
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/supplemental/no-site-content-data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
5 changes: 4 additions & 1 deletion adapters/rubicon/rubicontest/supplemental/no-site-content.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,10 @@
"pagecat": [
"val1",
"val2"
]
],
"pbs_login": "xuser",
"pbs_url": "http://hosturl.com",
"pbs_version": ""
},
"track": {
"mint": "",
Expand Down
Loading