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

Account-level override for the list of EEA countries #3678 #4118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
10 changes: 10 additions & 0 deletions config/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type Account struct {
DefaultBidLimit int `mapstructure:"default_bid_limit" json:"default_bid_limit"`
BidAdjustments *openrtb_ext.ExtRequestPrebidBidAdjustments `mapstructure:"bidadjustments" json:"bidadjustments"`
Privacy AccountPrivacy `mapstructure:"privacy" json:"privacy"`
EEACountries []string `mapstructure:"eea_countries" json:"eea_countries"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it would be better to include this within the AccountGDPR object since it's only used for GDPR enforcement and to match the host configuration of .privacy.gdpr. eea_countries.

}

// CookieSync represents the account-level defaults for the cookie sync endpoint.
Expand Down Expand Up @@ -381,3 +382,12 @@ func (ip *IPv4) Validate(errs []error) []error {
}
return errs
}

func (a *Account) ValidateEEACountries(errs []error) []error {
Copy link
Contributor

Choose a reason for hiding this comment

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

This validation method is not called.

for _, country := range a.EEACountries {
if len(country) != 2 { // Check if country code is valid
errs = append(errs, fmt.Errorf("invalid EEA country code: %s", country))
}
}
return errs
}
15 changes: 12 additions & 3 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,17 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog

recordImpMetrics(r.BidRequestWrapper, e.me)

// Retrieve host and account-level EEA countries config
eeaCountries := SelectEEACountries(e.privacyConfig.GDPR.EEACountries, r.Account.EEACountries)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: The comment is not necessary. The inputs are clearly host and account level, but if you'd like to make that association clearer consider a revised method name.


// Create a map for efficient lookup
eeaCountriesMap := make(map[string]struct{})
Copy link
Contributor

Choose a reason for hiding this comment

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

The performance benefit of a map only comes into play when the map is used more than once. As it is now, this map is only referenced once in parseGDPRDefaultValue. This forces the processing of every element with the overhead of map creation. It would be more efficient to use a linear search in this instance.

Eventually, this map can live in the account config object when that caching layer is improved. Doesn't make sense there right now though.

for _, country := range eeaCountries {
eeaCountriesMap[strings.ToUpper(country)] = struct{}{}
}

// Make our best guess if GDPR applies
gdprDefaultValue := e.parseGDPRDefaultValue(r.BidRequestWrapper)
gdprDefaultValue := e.parseGDPRDefaultValue(r.BidRequestWrapper, eeaCountriesMap)
gdprSignal, err := getGDPR(r.BidRequestWrapper)
if err != nil {
return nil, err
Expand Down Expand Up @@ -571,7 +580,7 @@ func buildMultiBidMap(prebid *openrtb_ext.ExtRequestPrebid) map[string]openrtb_e
return multiBidMap
}

func (e *exchange) parseGDPRDefaultValue(r *openrtb_ext.RequestWrapper) gdpr.Signal {
func (e *exchange) parseGDPRDefaultValue(r *openrtb_ext.RequestWrapper, eeaCountriesMap map[string]struct{}) gdpr.Signal {
gdprDefaultValue := e.gdprDefaultValue

var geo *openrtb2.Geo
Expand All @@ -584,7 +593,7 @@ func (e *exchange) parseGDPRDefaultValue(r *openrtb_ext.RequestWrapper) gdpr.Sig
if geo != nil {
// If we have a country set, and it is on the list, we assume GDPR applies if not set on the request.
// Otherwise we assume it does not apply as long as it appears "valid" (is 3 characters long).
if _, found := e.privacyConfig.GDPR.EEACountriesMap[strings.ToUpper(geo.Country)]; found {
if _, found := eeaCountriesMap[strings.ToUpper(geo.Country)]; found {
gdprDefaultValue = gdpr.SignalYes
} else if len(geo.Country) == 3 {
// The country field is formatted properly as a three character country code
Expand Down
12 changes: 12 additions & 0 deletions exchange/gdpr.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,15 @@ func enforceGDPR(signal gdpr.Signal, defaultValue gdpr.Signal, channelEnabled bo
gdprApplies := signal == gdpr.SignalYes || (signal == gdpr.SignalAmbiguous && defaultValue == gdpr.SignalYes)
return gdprApplies && channelEnabled
}

// SelectEEACountries selects the EEA countries based on host and account configurations.
// Account-level configuration takes precedence over the host-level configuration.
func SelectEEACountries(hostEEACountries []string, accountEEACountries []string) []string {
// If account-level configuration is provided, it takes precedence.
if len(accountEEACountries) > 0 {
return accountEEACountries
}

// Otherwise, return the host-level configuration.
return hostEEACountries
}
Loading