Removed old 2.5 location usages #3923

VeronikaSolovei9 · 2024-09-16T21:13:43Z

No description provided.

bsardo · 2024-09-18T16:50:43Z

The functions getGDPR and getConsent in exchange/gdpr.go will need to be updated.

bsardo · 2024-09-18T16:42:23Z

adapters/algorix/algorix.go

I don't think we want to modify the adapters. Adapters may not support 2.6. We're updating core to work with 2.6 but will down convert to 2.5 for adapters that don't declare 2.6 support. Even if an adapter does declare 2.6 support in their YAML file, I think we should leave it to the bidder to update their adapter since they must also support 2.5 at this point in time. Thoughts?

Any opinions on how to handle those adapters that do declare 2.6 support?

Discussed offline. For now, we will only update adapters that have declared 2.6 support in their YAML. In the future, once this feature has been rolled out, any new adapter PRs that declare 2.6 support will also require a review of their adapter to ensure they are accessing only 2.6 fields.

Adapters reverted.

bsardo · 2024-09-18T16:45:32Z

analytics/agma/agma_module.go

@@ -174,15 +174,11 @@ func (l *AgmaLogger) extractPublisherAndSite(requestWrapper *openrtb_ext.Request
 }

 func (l *AgmaLogger) shouldTrackEvent(requestWrapper *openrtb_ext.RequestWrapper) (bool, string) {


This is an interesting case that could require us to support both 2.5 and 2.6. Analytics modules could be called before the up convert occurs if there is an error.

The upconvert happens right after the wrapper is created in all three endpoints. So if the module is operating on the openrtb struct, we can be confident that it has the 2.6 version. If it is operating on the byte array, it is getting whatever was sent to the endpoint, which could be anything.

We have requestWrapper *openrtb_ext.RequestWrapper as input to this function.
Do we still want to check old locations?

github-actions · 2024-09-19T02:27:47Z

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, 19e3a1b

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-19T21:30:00Z

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, 57d9a48

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-20T20:12:55Z

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, 3e02d6b

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

bsardo · 2024-09-23T18:01:30Z

We will need to update schainwriter.go to write to the 2.6 location instead of the 2.5.

github-actions · 2024-09-23T21:08: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, 1826c39

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

bsardo · 2024-09-24T15:15:17Z

exchange/gdpr_test.go

-			description: "User Ext Consent is not empty",
-			giveUser:    &openrtb2.User{Ext: json.RawMessage(`{"consent": "BOS2bx5OS2bx5ABABBAAABoAAAAAFA"}`)},
+			description: "User Consent is not empty",
+			giveUser:    &openrtb2.User{Consent: "BOS2bx5OS2bx5ABABBAAABoAAAAAFA"},
 			wantConsent: "BOS2bx5OS2bx5ABABBAAABoAAAAAFA",
 		},
 		{
 			description: "User Ext Consent is empty",


Change description name to "User consent is empty"

bsardo · 2024-09-24T15:24:36Z

schain/schainwriter.go

-	} else {
-		sourceCopy := *req.Source
-		req.Source = &sourceCopy


I think we still need this to avoid shared memory issues since Source is a pointer and we will only have made shallow copies of the request for each bidder.

bsardo · 2024-09-24T15:48:24Z

schain/schainwriter.go

-	schain := openrtb_ext.ExtRequestPrebidSChain{
-		SChain: *selectedSChain,
-	}


I think you're right that we need to delete this block, however, notice that we were copying the value of the selected schain, which I believe was being done to protect against shared memory issues. Following this logic, when we make the assignment below:
req.Source.SChain = selectedSChain
I think we should instead be copying the value of selectedSChain to req.Source.SChain.

I suggest the following approach to accomplish this as I think it is clearer than the alternative:

Change var selectedSChain *openrtb2.SupplyChain to var selectedSChain openrtb2.SupplyChain

Update the selectedSChain assignments so values are copied to it instead of memory addresses

Change req.Source.SChain = selectedSChain to req.Source.SChain = &selectedSChain

bsardo · 2024-09-24T16:07:01Z

schain/schainwriter_test.go

+							},
+						},
+					},
+					Ext: json.RawMessage(`{` + seller2SChain + `}`),


Maybe change the description of this test from:
Use schain for bidder in ext.prebid.schains; ensure other ext.source field values are retained
to:
Use schain for bidder in ext.prebid.schains; ensure other source field values are retained

And to reduce confusion, perhaps change:
Ext: json.RawMessage(`{` + seller2SChain + `}`),
to:
Ext: json.RawMessage(`{"some":"data"}`),

bsardo · 2024-09-24T16:19:48Z

schain/schainwriter_test.go

Can you update the test case:
description: "Use source schain -- no bidder schain or wildcard schain in nil ext.prebid.schains",
so source.schain is set and unmodified instead of source.ext.schain?

Update description or test case?

Oops sorry if this wasn't clear. I wasn't asking for you to update the description but rather was pointing out that I think we should update this particular test case so source.schain is set in givenRequest instead of source.ext.schain and that we confirm it is not modified.

Right! Sorry I confused myself :) Added.

bsardo · 2024-09-24T16:42:19Z

privacy/gdpr/consentwriter.go

@@ -19,21 +19,18 @@ func (c ConsentWriter) Write(req *openrtb2.BidRequest) error {
 	reqWrap := &openrtb_ext.RequestWrapper{BidRequest: req}


We shouldn't need to wrap the request if we aren't using exts.

bsardo · 2024-09-24T16:44:24Z

privacy/ccpa/consentwriter.go

This writer appears to only be used on the AMP endpoint and it is being instantiated prior to the upconvert. I assume we want to make these changes anyway so that we set the consent string in the 2.6 location rather than relying on the upconvert logic so that we can get rid of this 2.5 logic.

bsardo · 2024-09-24T16:44:45Z

privacy/ccpa/consentwriter.go

 		}
 	}

+	// do we need to rebuild req here?


We shouldn't need to wrap the request if we aren't using exts.

bsardo · 2024-09-24T16:45:17Z

privacy/ccpa/consentwriter.go

We shouldn't need to wrap the request on line 19 if we aren't using exts.

bsardo · 2024-09-24T16:57:27Z

macros/provider.go

-	userExt, err := reqWrapper.GetUserExt()
-	if err == nil && userExt != nil && userExt.GetConsent() != nil {
-		b.macros[MacroKeyConsent] = *userExt.GetConsent()
+	//is this happening after upconvert?
+	if reqWrapper.User != nil && len(reqWrapper.User.Consent) > 0 {
+		b.macros[MacroKeyConsent] = reqWrapper.User.Consent


I'm not sure. I can reach out to the event framework author to see what their intent is. Depending on their answer we will either need to look at both the 2.5 and 2.6 locations or just the 2.6 location.

github-actions · 2024-09-24T23:42:36Z

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, 17411df

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-25T18:25: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, a566161

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-25T20:30:52Z

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, de24c4d

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-27T21:37:37Z

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, c386b20

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-28T21:41:45Z

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, 79f94ef

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

github-actions · 2024-09-28T22:29: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, 10a3cd5

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

# Conflicts: # endpoints/openrtb2/amp_auction_test.go

github-actions · 2024-09-30T18:10:16Z

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, 8401e16

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:243:	getBidVideo		100.0%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:258:	getMediaTypeForImp	85.7%
github.com/prebid/prebid-server/v2/adapters/openx/openx.go:272:	Builder			100.0%
total:								(statements)		87.1%

VeronikaSolovei9 added 2 commits September 16, 2024 14:13

Removed old 2.5 location usages

0b2cd1e

Partial unit test fix

33d48ca

bsardo assigned bsardo and hhhjort Sep 17, 2024

bsardo reviewed Sep 18, 2024

View reviewed changes

Revert back adapters without 2.6 support

19e3a1b

Clean up

57d9a48

getGDPR and getConsent update

3e02d6b

bsardo added the do not port label Sep 23, 2024

sChainWriter update

1826c39

bsardo reviewed Sep 24, 2024

View reviewed changes

CR fixes

17411df

VeronikaSolovei9 marked this pull request as ready for review September 24, 2024 23:44

Unit tests fixes

a566161

Unit tests fixes

de24c4d

bsardo approved these changes Sep 25, 2024

View reviewed changes

Unit tests fixes

c386b20

Fixes of fixes

79f94ef

Unit tests fixes

10a3cd5

hhhjort approved these changes Sep 29, 2024

View reviewed changes

Merge branch 'ortb26' into remove_old_25_locations_usages

8401e16

# Conflicts: # endpoints/openrtb2/amp_auction_test.go

VeronikaSolovei9 merged commit fe57c0d into ortb26 Sep 30, 2024
2 checks passed

bsardo pushed a commit to bsardo/prebid-server that referenced this pull request Oct 10, 2024

Removed old 2.5 location usages (prebid#3923)

c65bd03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed old 2.5 location usages #3923

Removed old 2.5 location usages #3923

VeronikaSolovei9 commented Sep 16, 2024

bsardo commented Sep 18, 2024 •

edited

Loading

bsardo Sep 18, 2024

bsardo Sep 18, 2024

VeronikaSolovei9 Sep 19, 2024

bsardo Sep 18, 2024

hhhjort Sep 18, 2024

VeronikaSolovei9 Sep 20, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 20, 2024

bsardo commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

VeronikaSolovei9 Sep 24, 2024

bsardo Sep 25, 2024

VeronikaSolovei9 Sep 25, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

bsardo Sep 24, 2024

github-actions bot commented Sep 24, 2024

github-actions bot commented Sep 25, 2024

github-actions bot commented Sep 25, 2024

github-actions bot commented Sep 27, 2024

github-actions bot commented Sep 28, 2024

github-actions bot commented Sep 28, 2024

github-actions bot commented Sep 30, 2024

		@@ -174,15 +174,11 @@ func (l AgmaLogger) extractPublisherAndSite(requestWrapper openrtb_ext.Request
		}

		func (l AgmaLogger) shouldTrackEvent(requestWrapper openrtb_ext.RequestWrapper) (bool, string) {

		@@ -19,21 +19,18 @@ func (c ConsentWriter) Write(req *openrtb2.BidRequest) error {
		reqWrap := &openrtb_ext.RequestWrapper{BidRequest: req}

Removed old 2.5 location usages #3923

Removed old 2.5 location usages #3923

Conversation

VeronikaSolovei9 commented Sep 16, 2024

bsardo commented Sep 18, 2024 • edited Loading

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

Choose a reason for hiding this comment

github-actions bot commented Sep 19, 2024

Code coverage summary

openx

github-actions bot commented Sep 19, 2024

Code coverage summary

openx

github-actions bot commented Sep 20, 2024

Code coverage summary

openx

bsardo commented Sep 23, 2024

github-actions bot commented Sep 23, 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

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

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 Sep 24, 2024

Code coverage summary

openx

github-actions bot commented Sep 25, 2024

Code coverage summary

openx

github-actions bot commented Sep 25, 2024

Code coverage summary

openx

github-actions bot commented Sep 27, 2024

Code coverage summary

openx

github-actions bot commented Sep 28, 2024

Code coverage summary

openx

github-actions bot commented Sep 28, 2024

Code coverage summary

openx

github-actions bot commented Sep 30, 2024

Code coverage summary

openx

bsardo commented Sep 18, 2024 •

edited

Loading