OpenX: accept incoming string fields to support Prebid.js 9 #3668

laurb9 · 2024-05-07T19:29:10Z

accept incoming imp[].ext.customFloor as string or float, emit float in imp[].bidfloor
accept incoming imp[].ext.unit as string or integer, emit string in imp[].tagid
enable gzip compression

1,2 are to support the Prebid.js 9 changes introduced in prebid/Prebid.js#11458

Corresponding PBS-Java change: prebid/prebid-server-java#3178

github-actions · 2024-05-07T19:29:57Z

Code coverage summary

Note:

Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
Coverage summary encompasses all commits leading up to the latest one, 67c1018

openx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/openx/openx.go:34:	MakeRequests		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:72:	makeRequest		75.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:118:	preprocess		85.3%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:187:	MakeBids		87.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:246:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:260:	Builder			100.0%
total:								(statements)		86.6%

github-actions · 2024-05-09T04:33:07Z

Code coverage summary

Note:

Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
Coverage summary encompasses all commits leading up to the latest one, 370ff75

openx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/openx/openx.go:34:	MakeRequests		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:72:	makeRequest		75.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:118:	preprocess		85.3%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:187:	MakeBids		87.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:246:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:260:	Builder			100.0%
total:								(statements)		86.6%

SyntaxNode · 2024-05-09T15:07:57Z

adapters/openx/openxtest/supplemental/multi-imp.json

@@ -7,7 +7,7 @@
        "banner": {},
        "ext": {
          "bidder": {
-            "unit": "banner-unit-1",


Was this test case previously invalid? Doesn't look like it adhered to the unit json schema pattern.

I don't recall if these were valid at some point in the past, but they are not now.

SyntaxNode · 2024-05-09T15:08:35Z

adapters/openx/openxtest/supplemental/multi-imp.json

@@ -7,7 +7,7 @@
        "banner": {},
        "ext": {
          "bidder": {
-            "unit": "banner-unit-1",
+            "unit": "111",


Curious, why "111" instead of "1"? Is there a test involved with the length of the unit string?

No functional reason, just think it's easier to find and replace if needed.

SyntaxNode · 2024-05-09T15:12:42Z

adapters/openx/params_test.go

@@ -56,7 +59,8 @@ var invalidParams = []string{
 	`{"unit": "123", "delDomain": "foo.b"}`,
 	`{"unit": "123", "delDomain": "foo.barr"}`,
 	`{"unit": "123", "delDomain": ".bar"}`,
-	`{"unit": "123", "delDomain": "foo.bar", "customFloor": "0.1"}`,
+	`{"unit": "123", "delDomain": "foo.bar", "customFloor": ""}`,
+	`{"unit": "123", "delDomain": "foo.bar", "customFloor": "-0.1"}`,
 	`{"unit": "123", "delDomain": "foo.bar", "customFloor": -0.1}`,
 	`{"unit": "123", "delDomain": "foo.bar", "customParams": "foo: bar"}`,


I recommend adding customFloor tests invalid test cases as "1." and "1.0x" to add more test coverage of the regex.

github-actions · 2024-05-09T20:44:28Z

Code coverage summary

Note:

Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
Coverage summary encompasses all commits leading up to the latest one, 028d8db

openx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/openx/openx.go:34:	MakeRequests		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:72:	makeRequest		75.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:118:	preprocess		85.3%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:187:	MakeBids		87.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:246:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:260:	Builder			100.0%
total:								(statements)		86.6%

github-actions · 2024-05-14T17:27:13Z

Code coverage summary

Note:

Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
Coverage summary encompasses all commits leading up to the latest one, 21d557e

openx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/openx/openx.go:34:	MakeRequests		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:72:	makeRequest		75.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:118:	preprocess		85.3%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:187:	MakeBids		87.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:246:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:260:	Builder			100.0%
total:								(statements)		86.6%

laurb9 · 2024-06-04T16:21:54Z

Please let me know if there is anything else we should do

onkarvhanumante · 2024-06-11T06:33:46Z

endpoints/openrtb2/sample-requests/disabled/good/partial.json

          "openx": {
            "unit": "539439964",
            "delDomain": "se-demo-d.openx.net",
-            "customFloor": 0.1,
+            "customFloor": "0.5",
            "customParams": {"foo": "bar"}
          }              
        }


@laurb9 why was this file changed? Adapter change should be limited to Adapter package

This file contains an OpenX adapter request, and we thought it would be best if it were updated to the most recent request format.

could move this to openx specific exemplary test folder

I can undo this particular line change if it blocks the review, but moving it to the openx test folder does not add any new use cases there. Removing samples from openrtb2/sample-requests is outside the scope of this PR, aside from that openrtb2/sample-requests contains requests for other adapters as well, so the file looks consistent with the intended purpose of that directory, not as an adapter-specific request, but an usage example for certain pbs configurations.

onkarvhanumante · 2024-06-11T06:42:49Z

static/bidder-params/openx.json

      "description": "The minimum CPM price in USD.",
-      "minimum": 0


should add/update json tests

to have request where unit is string

to have request where unit is number

to have request where customFloor is string

to have request where customFloor is number

These parameter variations are in optional-params.json:
unit is string: https://github.com/prebid/prebid-server/pull/3668/files#diff-4214c2dc95f1c24891a70e424d9576ab40f10447745b95f68a877ca7145f1bd5R12
unit is number: https://github.com/prebid/prebid-server/pull/3668/files#diff-4214c2dc95f1c24891a70e424d9576ab40f10447745b95f68a877ca7145f1bd5R28
customFloor is string: https://github.com/prebid/prebid-server/pull/3668/files#diff-4214c2dc95f1c24891a70e424d9576ab40f10447745b95f68a877ca7145f1bd5R15
customFloor is number: https://github.com/prebid/prebid-server/pull/3668/files#diff-4214c2dc95f1c24891a70e424d9576ab40f10447745b95f68a877ca7145f1bd5R31

github-actions · 2024-06-18T18:07:40Z

Code coverage summary

Note:

Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
Coverage summary encompasses all commits leading up to the latest one, 2a84206

openx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/openx/openx.go:34:	MakeRequests		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:72:	makeRequest		75.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:118:	preprocess		85.3%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:187:	MakeBids		87.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:246:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:260:	Builder			100.0%
total:								(statements)		86.6%

onkarvhanumante · 2024-06-25T12:30:01Z

@laurb9 requesting to take a look at https://github.com/prebid/prebid-server/pull/3668/files#r1652722717

onkarvhanumante · 2024-07-01T08:49:49Z

openrtb_ext/imp_openx.go

+	Unit         json.Number            `json:"unit"`
 	Platform     string                 `json:"platform"`
 	DelDomain    string                 `json:"delDomain"`
-	CustomFloor  float64                `json:"customFloor"`
+	CustomFloor  json.Number            `json:"customFloor"`


@laurb9
could use StringInt data type for Unit, CustomFloor

prebid-server/util/jsonutil/stringInt.go

Lines 7 to 29 in a1f48b5

type StringInt int

func (st *StringInt) UnmarshalJSON(b []byte) error {

if len(b) == 0 {

return nil

}

if b[0] == '"' {

b = b[1 : len(b)-1]

}

if len(b) == 0 {

return nil

}

i, err := jsonparser.ParseInt(b)

if err != nil {

return err

}

*st = StringInt(i)

return nil

}

@laurb9 could use StringInt data type for Unit, CustomFloor

CustomFloor is a string|float emitted as float and Unit is a string|int emitted as a string.
I believe neither can make use of StringInt which is a int|string serialized to int.

onkarvhanumante

@laurb9 ptal at https://github.com/prebid/prebid-server/pull/3668/files#r1660697479

laurb9 · 2024-07-18T22:01:29Z

Just checking if there is anything else to be done here, this PR addresses breaking changes in Prebid.js 9

laurb9 added 2 commits May 7, 2024 19:17

OpenX: accept customFloor as string or float

47306c4

OpenX: Accept unit as integer or string, emit string

67c1018

laurb9 mentioned this pull request May 7, 2024

OpenX: accept incoming string fields to support Prebid.js 9 prebid/prebid-server-java#3178

Merged

OpenX: enable gzip compression

370ff75

SyntaxNode reviewed May 9, 2024

View reviewed changes

Add more test cases per feedback

028d8db

laurb9 requested a review from SyntaxNode May 10, 2024 21:09

gargcreation1992 assigned onkarvhanumante and Sonali-More-Xandr May 13, 2024

Merge branch 'prebid:master' into plat-1150

21d557e

onkarvhanumante reviewed Jun 11, 2024

View reviewed changes

Merge branch 'master' into plat-1150

2a84206

laurb9 requested a review from onkarvhanumante June 18, 2024 18:08

onkarvhanumante approved these changes Jun 28, 2024

View reviewed changes

onkarvhanumante reviewed Jul 1, 2024

View reviewed changes

onkarvhanumante requested changes Jul 1, 2024

View reviewed changes

laurb9 requested a review from onkarvhanumante July 2, 2024 17:07

onkarvhanumante approved these changes Jul 3, 2024

View reviewed changes

Sonali-More-Xandr approved these changes Jul 24, 2024

View reviewed changes

Sonali-More-Xandr merged commit f087eea into prebid:master Jul 24, 2024
5 checks passed

cross-repo-issue-creation bot mentioned this pull request Jul 24, 2024

Port PR from PBS-Go: OpenX: accept incoming string fields to support Prebid.js 9 prebid/prebid-server-java#3320

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenX: accept incoming string fields to support Prebid.js 9 #3668

OpenX: accept incoming string fields to support Prebid.js 9 #3668

laurb9 commented May 7, 2024 •

edited

Loading

github-actions bot commented May 7, 2024

github-actions bot commented May 9, 2024

SyntaxNode May 9, 2024

laurb9 May 9, 2024

SyntaxNode May 9, 2024

laurb9 May 9, 2024

SyntaxNode May 9, 2024

github-actions bot commented May 9, 2024

github-actions bot commented May 14, 2024

laurb9 commented Jun 4, 2024

onkarvhanumante Jun 11, 2024

laurb9 Jun 11, 2024

onkarvhanumante Jun 25, 2024

laurb9 Jun 25, 2024

onkarvhanumante Jun 11, 2024 •

edited

Loading

laurb9 Jun 18, 2024

github-actions bot commented Jun 18, 2024

onkarvhanumante commented Jun 25, 2024

onkarvhanumante Jul 1, 2024

laurb9 Jul 2, 2024 •

edited

Loading

onkarvhanumante left a comment

laurb9 commented Jul 18, 2024

	type StringInt int

	func (st *StringInt) UnmarshalJSON(b []byte) error {
	if len(b) == 0 {
	return nil
	}

	if b[0] == '"' {
	b = b[1 : len(b)-1]
	}

	if len(b) == 0 {
	return nil
	}

	i, err := jsonparser.ParseInt(b)
	if err != nil {
	return err
	}

	*st = StringInt(i)
	return nil
	}

OpenX: accept incoming string fields to support Prebid.js 9 #3668

OpenX: accept incoming string fields to support Prebid.js 9 #3668

Conversation

laurb9 commented May 7, 2024 • edited Loading

github-actions bot commented May 7, 2024

Code coverage summary

openx

github-actions bot commented May 9, 2024

Code coverage summary

openx

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 9, 2024

Code coverage summary

openx

github-actions bot commented May 14, 2024

Code coverage summary

openx

laurb9 commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onkarvhanumante Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 18, 2024

Code coverage summary

openx

onkarvhanumante commented Jun 25, 2024

Choose a reason for hiding this comment

laurb9 Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

onkarvhanumante left a comment

Choose a reason for hiding this comment

laurb9 commented Jul 18, 2024

laurb9 commented May 7, 2024 •

edited

Loading

onkarvhanumante Jun 11, 2024 •

edited

Loading

laurb9 Jul 2, 2024 •

edited

Loading