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.

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

PulsePoint: Marking cp/ct params to be either String or Int #3677

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions adapters/pulsepoint/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ func TestInvalidParams(t *testing.T) {
var validParams = []string{
`{"cp":1000, "ct": 2000}`,
`{"cp":1001, "ct": 2001}`,
`{"cp":"1000", "ct": "2000"}`,
`{"cp":"1000", "ct": 2000}`,
`{"cp":1000, "ct": "2000"}`,
`{"cp":1001, "ct": 2001, "cf": "1x1"}`,
}

Expand All @@ -52,8 +55,4 @@ var invalidParams = []string{
`{"ct":"1000"}`,
`{"cp":1000}`,
`{"ct":1000}`,
`{"cp":1000, "ct":"1000"}`,
`{"cp":1000, "ct": "abcd"}`,
`{"cp":"abcd", "ct": 1000}`,
`{"cp":"1000.2", "ct": "1000.1"}`,
}
31 changes: 28 additions & 3 deletions adapters/pulsepoint/pulsepoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package pulsepoint

import (
"encoding/json"
"errors"
"fmt"
"net/http"
"strconv"
Expand All @@ -10,6 +11,7 @@ import (
"github.com/prebid/prebid-server/v2/config"
"github.com/prebid/prebid-server/v2/errortypes"
"github.com/prebid/prebid-server/v2/openrtb_ext"
"github.com/prebid/prebid-server/v2/util/jsonutil"

"github.com/prebid/openrtb/v20/openrtb2"
)
Expand Down Expand Up @@ -49,11 +51,25 @@ func (a *PulsePointAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *
continue
}
// parse pubid and keep it for reference
if pubID == "" && pulsepointExt.PubID > 0 {
pubID = strconv.Itoa(pulsepointExt.PubID)
if pubID == "" {
pubID, err = parseParam("pubID", pulsepointExt.PubID)
if err != nil {
errs = append(errs, &errortypes.BadInput{
Message: err.Error(),
})
continue
}
}
// tag id to be sent
imp.TagID = strconv.Itoa(pulsepointExt.TagID)
var tagID string
tagID, err = parseParam("tagID", pulsepointExt.TagID)
if err != nil {
errs = append(errs, &errortypes.BadInput{
Message: err.Error(),
})
continue
}
imp.TagID = tagID
imps = append(imps, imp)
}

Expand Down Expand Up @@ -163,3 +179,12 @@ func getBidType(imp openrtb2.Imp) openrtb_ext.BidType {
}
return ""
}

func parseParam(paramName string, paramValue jsonutil.StringInt) (string, error) {
value := int(paramValue)
// verify we got a non-zero value
if value == 0 {
return "", errors.New("param not found - " + paramName)
}
return strconv.Itoa(value), nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
{
"mockBidRequest": {
"id": "request-id",
"site": {
"page": "http://publisher.com/index.html",
"publisher": {
"id": "123456789",
"name": "publisher.com"
}
},
"imp": [{
"id": "banner-1",
"banner": {
"w": 320,
"h": 50
},
"ext": {
"bidder": {
"cp": "1234",
"ct": "1001"
}
}
}]
},
"httpCalls": [{
"expectedRequest": {
"headers": {
"Content-Type": [
"application/json;charset=utf-8"
],
"Accept": [
"application/json"
]
},
"uri": "http://bidder.pulsepoint.com",
"body": {
"id": "request-id",
"site": {
"page": "http://publisher.com/index.html",
"publisher": {
"id": "1234",
"name": "publisher.com"
}
},
"imp": [{
"id": "banner-1",
"tagid": "1001",
"banner": {
"w": 320,
"h": 50
},
"ext": {
"bidder": {
"cp": "1234",
"ct": "1001"
}
}
}]
},
"impIDs":["banner-1"]
},
"mockResponse": {
"status": 200,
"body": {
"id": "response-id",
"seatbid": [{
"bid": [{
"id": "banner-1-bid",
"impid": "banner-1",
"price": 3.5,
"adm": "<div>Creative</div>",
"adomain": [
"advertiser.com"
],
"crid": "20",
"w": 300,
"h": 250
}],
"seat": "pulsepoint-seat"
}],
"cur": "USD"
}
}
}],
"expectedBidResponses": [{
"bids": [{
"bid": {
"id": "banner-1-bid",
"impid": "banner-1",
"price": 3.5,
"adm": "<div>Creative</div>",
"adomain": [
"advertiser.com"
],
"crid": "20",
"w": 300,
"h": 250
},
"type": "banner"
}]
}]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"mockBidRequest": {
"id": "request-id",
"site": {
"page": "http://publisher.com/index.html",
"publisher": {
"id": "123456789",
"name": "publisher.com"
}
},
"imp": [{
"id": "banner-1",
"banner": {
"w": 320,
"h": 50
},
"ext": {
"bidder": {
"cp": "1234"
}
}
}]
},
"httpCalls": [],
"expectedMakeRequestsErrors": [{
"value": "param not found - tagID",
"comparison": "literal"
}],
"expectedBidResponses": [],
"expectedMakeBidsErrors": []
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
"ext": {
"bidder": {
"cp": "1234",
"ct": "1001"
"ct": "ab1001"
}
}
}]
},
"httpCalls": [],
"expectedMakeRequestsErrors": [{
"value": "json: cannot unmarshal string into Go struct field ExtImpPulsePoint.cp of type int",
"value": "Value looks like Number/Boolean/None, but can't find its end: ',' or '}' symbol",
"comparison": "literal"
}],
"expectedBidResponses": [],
Expand Down
8 changes: 6 additions & 2 deletions openrtb_ext/imp_pulsepoint.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package openrtb_ext

import (
"github.com/prebid/prebid-server/v2/util/jsonutil"
)

// ExtImpPulsePoint defines the json spec for bidrequest.imp[i].ext.prebid.bidder.pulsepoint
// PubId/TagId are mandatory params

type ExtImpPulsePoint struct {
PubID int `json:"cp"`
TagID int `json:"ct"`
PubID jsonutil.StringInt `json:"cp"`
TagID jsonutil.StringInt `json:"ct"`
}
10 changes: 8 additions & 2 deletions static/bidder-params/pulsepoint.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@
"type": "object",
"properties": {
"cp": {
"type": "integer",
"type": [
"integer",
"string"
],
"description": "An ID which identifies the publisher selling the impression"
},
"ct": {
"type": "integer",
"type": [
"integer",
"string"
],
Comment on lines +8 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

@anand-venkatraman could you add JSON test where cp and ct are string

Copy link
Contributor

Choose a reason for hiding this comment

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

@anand-venkatraman requesting to address above comment ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for the delay, i have added a few tests cases to validParams under params_test.go file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@anand-venkatraman thanks for adding params test. Requesting to add Json framework tests as well for scenario where cp and ct are string in imp.ext

Copy link
Contributor Author

@anand-venkatraman anand-venkatraman Jun 12, 2024

Choose a reason for hiding this comment

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

addressed in commit 4539af1

onkarvhanumante marked this conversation as resolved.
Show resolved Hide resolved
"description": "An ID which identifies the ad slot being sold"
}
},
Expand Down
Loading