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

ImproveDigital: Remove placementKey & addtlconsent parsing #3728

Merged
merged 1 commit into from
Oct 29, 2024
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
83 changes: 3 additions & 80 deletions adapters/improvedigital/improvedigital.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,9 @@ import (
)

const (
isRewardedInventory = "is_rewarded_inventory"
stateRewardedInventoryEnable = "1"
consentProvidersSettingsInputKey = "ConsentedProvidersSettings"
consentProvidersSettingsOutKey = "consented_providers_settings"
consentedProvidersKey = "consented_providers"
publisherEndpointParam = "{PublisherId}"
isRewardedInventory = "is_rewarded_inventory"
stateRewardedInventoryEnable = "1"
publisherEndpointParam = "{PublisherId}"
)

type ImprovedigitalAdapter struct {
Expand Down Expand Up @@ -75,17 +72,6 @@ func (a *ImprovedigitalAdapter) makeRequest(request openrtb2.BidRequest, imp ope

request.Imp = []openrtb2.Imp{imp}

userExtAddtlConsent, err := a.getAdditionalConsentProvidersUserExt(request)
if err != nil {
return nil, err
}

if len(userExtAddtlConsent) > 0 {
userCopy := *request.User
userCopy.Ext = userExtAddtlConsent
request.User = &userCopy
}

reqJSON, err := json.Marshal(request)
if err != nil {
return nil, err
Expand Down Expand Up @@ -255,69 +241,6 @@ func isMultiFormatImp(imp openrtb2.Imp) bool {
return formatCount > 1
}

// This method responsible to clone request and convert additional consent providers string to array when additional consent provider found
func (a *ImprovedigitalAdapter) getAdditionalConsentProvidersUserExt(request openrtb2.BidRequest) ([]byte, error) {
var cpStr string

// If user/user.ext not defined, no need to parse additional consent
if request.User == nil || request.User.Ext == nil {
return nil, nil
}

// Start validating additional consent
// Check key exist user.ext.ConsentedProvidersSettings
var userExtMap = make(map[string]json.RawMessage)
if err := json.Unmarshal(request.User.Ext, &userExtMap); err != nil {
return nil, err
}

cpsMapValue, cpsJSONFound := userExtMap[consentProvidersSettingsInputKey]
if !cpsJSONFound {
return nil, nil
}

// Check key exist user.ext.ConsentedProvidersSettings.consented_providers
var cpMap = make(map[string]json.RawMessage)
if err := json.Unmarshal(cpsMapValue, &cpMap); err != nil {
return nil, err
}

cpMapValue, cpJSONFound := cpMap[consentedProvidersKey]
if !cpJSONFound {
return nil, nil
}
// End validating additional consent

// Trim enclosing quotes after casting json.RawMessage to string
consentStr := strings.Trim((string)(cpMapValue), "\"")
// Split by ~ and take only the second string (if exists) as the consented providers spec
var consentStrParts = strings.Split(consentStr, "~")
if len(consentStrParts) < 2 {
return nil, nil
}
cpStr = strings.TrimSpace(consentStrParts[1])
if len(cpStr) == 0 {
return nil, nil
}

// Prepare consent providers string
cpStr = fmt.Sprintf("[%s]", strings.Replace(cpStr, ".", ",", -1))
cpMap[consentedProvidersKey] = json.RawMessage(cpStr)

cpJSON, err := json.Marshal(cpMap)
if err != nil {
return nil, err
}
userExtMap[consentProvidersSettingsOutKey] = cpJSON

extJson, err := json.Marshal(userExtMap)
if err != nil {
return nil, err
}

return extJson, nil
}

func getImpExtWithRewardedInventory(imp openrtb2.Imp) ([]byte, error) {
var ext = make(map[string]json.RawMessage)
if err := json.Unmarshal(imp.Ext, &ext); err != nil {
Expand Down

This file was deleted.

This file was deleted.

3 changes: 1 addition & 2 deletions adapters/improvedigital/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func TestInvalidParams(t *testing.T) {
var validParams = []string{
`{"placementId":13245}`,
`{"placementId":13245, "size": {"w":16, "h":9}}`,
`{"publisherId":13245, "placementKey": "slotA"}`,
`{"placementId":13245, "keyValues":{"target1":["foo"],"target2":["bar", "baz"]}}`,
}

Expand All @@ -56,5 +55,5 @@ var invalidParams = []string{
`{"placementId": "1"}`,
`{"size": true}`,
`{"placementId": true, "size":"1234567"}`,
`{"placementId":13245, "publisherId":13245, "placementKey": "slotA"}`,
`{"publisherId":13245, "placementKey": "slotA"}`,
}
21 changes: 8 additions & 13 deletions static/bidder-params/improvedigital.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,12 @@
"placementId": {
"type": "integer",
"minimum": 1,
"description": "An ID which identifies this placement of the impression"
"description": "The placement ID from Improve Digital"
},
"publisherId": {
"type": "integer",
"minimum": 1,
"description": "An ID which identifies publisher. Required when using a placementKey"
},
"placementKey": {
"type": "string",
"description": "An uniq name which identifies this placement of the impression. Must be used with publisherId"
"description": "The publisher ID from Improve Digital"
},
"keyValues": {
"type": "object",
Expand All @@ -32,13 +28,12 @@
"type": "integer"
}
},
"required": ["w", "h"],
"required": [
"w",
"h"
],
"description": "Placement size"
}
},
"oneOf": [{
"required": ["placementId"]
}, {
"required": ["publisherId", "placementKey"]
}]
}
"required": ["placementId"]
Copy link
Contributor

@SyntaxNode SyntaxNode Jun 25, 2024

Choose a reason for hiding this comment

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

This is a breaking change. Before this change, it's valid to provide a publisherId and placementKey combination. If publishers are satisfying the requirement this way, this change will break them when only a placementId is now accepted.

Can you provide a change window?

FYI @bretg @bsardo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbartek25 Can you advice on this please?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @SyntaxNode. Thank you for bringing this up. The change already happened. Improve Digital ad server removed support for placementKey already and our PBS Java adapter schema was updated accordingly long time ago: https://github.com/prebid/prebid-server-java/blob/master/src/main/resources/static/bidder-params/improvedigital.json so this change is safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @SyntaxNode. Do you need any further clarification/information to resolve your query? Otherwise, it will be really appreciated if you could expedite the approval & merger of this PR.

TIA.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looked back over the documentation history... the 'publisherKey' parameter has been gone for about 3 years. publisherId was added to the docs 2 years ago. So it appears that just the Go adapter had fallen behind.

However, I agree it could be a breaking change to a PBS-Go host company. The publisherId param should not be required nor the publisherKey field completley removed until a major PBS release prompts host companies to review their stored requests for the newly required parameter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 tagged as PBS 3.0 which we are tentatively planning to deliver in the next couple of months.

}
Loading