GDPR: pass geo based on special feature 1 opt-in #2122
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -217,8 +217,8 @@ func TestDefaults(t *testing.T) { | |
| VendorExceptions: []openrtb_ext.BidderName{}, | ||
| VendorExceptionMap: map[openrtb_ext.BidderName]struct{}{}, | ||
| }, | ||
| SpecialFeature1: TCF2SpecialFeature{ | ||
| Enforce: true, | ||
| VendorExceptions: []openrtb_ext.BidderName{}, | ||
| VendorExceptionMap: map[openrtb_ext.BidderName]struct{}{}, | ||
| }, | ||
|
|
@@ -272,7 +272,7 @@ gdpr: | |
| purpose10: | ||
| enforce_vendors: false | ||
| vendor_exceptions: ["foo10"] | ||
| special_feature1: | ||
| vendor_exceptions: ["fooSP1"] | ||
| ccpa: | ||
| enforce: true | ||
|
|
@@ -540,8 +540,8 @@ func TestFullConfig(t *testing.T) { | |
| VendorExceptions: []openrtb_ext.BidderName{openrtb_ext.BidderName("foo10")}, | ||
| VendorExceptionMap: map[openrtb_ext.BidderName]struct{}{openrtb_ext.BidderName("foo10"): {}}, | ||
| }, | ||
| SpecialFeature1: TCF2SpecialFeature{ | ||
| Enforce: true, // true by default | ||
| VendorExceptions: []openrtb_ext.BidderName{openrtb_ext.BidderName("fooSP1")}, | ||
| VendorExceptionMap: map[openrtb_ext.BidderName]struct{}{openrtb_ext.BidderName("fooSP1"): {}}, | ||
| }, | ||
|
|
@@ -765,6 +765,91 @@ func TestMigrateConfigPurposeOneTreatment(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func TestMigrateConfigSpecialFeature1(t *testing.T) { | ||
| oldSpecialFeature1Config := []byte(` | ||
| gdpr: | ||
| tcf2: | ||
| special_purpose1: | ||
| enabled: true | ||
| vendor_exceptions: ["appnexus"] | ||
| `) | ||
| newSpecialFeature1Config := []byte(` | ||
| gdpr: | ||
| tcf2: | ||
| special_feature1: | ||
| enforce: true | ||
| vendor_exceptions: ["appnexus"] | ||
| `) | ||
| oldAndNewSpecialFeature1Config := []byte(` | ||
| gdpr: | ||
| tcf2: | ||
| special_purpose1: | ||
| enabled: false | ||
| vendor_exceptions: ["appnexus"] | ||
| special_feature1: | ||
| enforce: true | ||
| vendor_exceptions: ["rubicon"] | ||
| `) | ||
|
|
||
| tests := []struct { | ||
| description string | ||
| config []byte | ||
| wantSpecialFeature1Enforce bool | ||
| wantSpecialFeature1VendorExceptions []string | ||
| }{ | ||
| { | ||
| description: "New config and old config not set", | ||
| config: []byte{}, | ||
| }, | ||
| { | ||
| description: "New config not set, old config set", | ||
| config: oldSpecialFeature1Config, | ||
| wantSpecialFeature1Enforce: true, | ||
| wantSpecialFeature1VendorExceptions: []string{"appnexus"}, | ||
| }, | ||
| { | ||
| description: "New config set, old config not set", | ||
| config: newSpecialFeature1Config, | ||
| wantSpecialFeature1Enforce: true, | ||
| wantSpecialFeature1VendorExceptions: []string{"appnexus"}, | ||
| }, | ||
| { | ||
| description: "New config and old config set", | ||
| config: oldAndNewSpecialFeature1Config, | ||
| wantSpecialFeature1Enforce: true, | ||
| wantSpecialFeature1VendorExceptions: []string{"rubicon"}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| v := viper.New() | ||
| v.SetConfigType("yaml") | ||
| v.ReadConfig(bytes.NewBuffer(tt.config)) | ||
|
|
||
| migrateConfigSpecialFeature1(v) | ||
|
|
||
| if len(tt.config) > 0 { | ||
| assert.Equal(t, tt.wantSpecialFeature1Enforce, v.Get("gdpr.tcf2.special_feature1.enforce").(bool), tt.description) | ||
| assert.Equal(t, tt.wantSpecialFeature1VendorExceptions, v.GetStringSlice("gdpr.tcf2.special_feature1.vendor_exceptions"), tt.description) | ||
| } else { | ||
| assert.Nil(t, v.Get("gdpr.tcf2.special_feature1.enforce"), tt.description) | ||
| assert.Nil(t, v.Get("gdpr.tcf2.special_feature1.vendor_exceptions"), tt.description) | ||
| } | ||
|
There was a problem hiding this comment. I find this to be a very comprehensive test case but: can we make one last assertion in the resulting Or even better, a Another option is to probably add an There was a problem hiding this comment. @guscarreon and I spoke offline about this. My line of thinking was that the coverage was there across three unit tests but it was hard to see: I'm hesitant to make use of another function in the package to perform assertions in a pure unit test so instead of calling |
||
|
|
||
| var c Configuration | ||
| err := v.Unmarshal(&c) | ||
| assert.NoError(t, err, tt.description) | ||
| assert.Equal(t, tt.wantSpecialFeature1Enforce, c.GDPR.TCF2.SpecialFeature1.Enforce, tt.description) | ||
|
|
||
| // convert expected vendor exceptions to type BidderName | ||
| expectedVendorExceptions := make([]openrtb_ext.BidderName, 0, 0) | ||
| for _, ve := range tt.wantSpecialFeature1VendorExceptions { | ||
| expectedVendorExceptions = append(expectedVendorExceptions, openrtb_ext.BidderName(ve)) | ||
| } | ||
| assert.ElementsMatch(t, expectedVendorExceptions, c.GDPR.TCF2.SpecialFeature1.VendorExceptions, tt.description) | ||
| } | ||
| } | ||
|
|
||
| func TestMigrateConfigTCF2PurposeEnabledFlags(t *testing.T) { | ||
| trueStr := "true" | ||
| falseStr := "false" | ||
|
|
@@ -1291,3 +1376,58 @@ func doTimeoutTest(t *testing.T, expected int, requested int, max uint64, def ui | |
| limited := cfg.LimitAuctionTimeout(time.Duration(requested) * time.Millisecond) | ||
| assert.Equal(t, limited, expectedDuration, "Expected %dms timeout, got %dms", expectedDuration, limited/time.Millisecond) | ||
| } | ||
|
|
||
| func TestSpecialFeature1VendorExceptionMap(t *testing.T) { | ||
| baseConfig := []byte(` | ||
| gdpr: | ||
| default_value: 0 | ||
| tcf2: | ||
| special_feature1: | ||
| vendor_exceptions: `) | ||
|
|
||
| tests := []struct { | ||
| description string | ||
| configVendorExceptions []byte | ||
| wantVendorExceptions []openrtb_ext.BidderName | ||
| wantVendorExceptionsMap map[openrtb_ext.BidderName]struct{} | ||
| }{ | ||
| { | ||
| description: "nil vendor exceptions", | ||
| configVendorExceptions: []byte(`null`), | ||
| wantVendorExceptions: []openrtb_ext.BidderName{}, | ||
| wantVendorExceptionsMap: map[openrtb_ext.BidderName]struct{}{}, | ||
| }, | ||
| { | ||
| description: "no vendor exceptions", | ||
| configVendorExceptions: []byte(`[]`), | ||
| wantVendorExceptions: []openrtb_ext.BidderName{}, | ||
| wantVendorExceptionsMap: map[openrtb_ext.BidderName]struct{}{}, | ||
| }, | ||
| { | ||
| description: "one vendor exception", | ||
| configVendorExceptions: []byte(`["vendor1"]`), | ||
| wantVendorExceptions: []openrtb_ext.BidderName{openrtb_ext.BidderName("vendor1")}, | ||
| wantVendorExceptionsMap: map[openrtb_ext.BidderName]struct{}{openrtb_ext.BidderName("vendor1"): {}}, | ||
| }, | ||
| { | ||
| description: "many exceptions", | ||
| configVendorExceptions: []byte(`["vendor1","vendor2"]`), | ||
| wantVendorExceptions: []openrtb_ext.BidderName{openrtb_ext.BidderName("vendor1"), openrtb_ext.BidderName("vendor2")}, | ||
| wantVendorExceptionsMap: map[openrtb_ext.BidderName]struct{}{openrtb_ext.BidderName("vendor1"): {}, openrtb_ext.BidderName("vendor2"): {}}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| config := append(baseConfig, tt.configVendorExceptions...) | ||
|
|
||
| v := viper.New() | ||
| SetupViper(v, "") | ||
| v.SetConfigType("yaml") | ||
| v.ReadConfig(bytes.NewBuffer(config)) | ||
| cfg, err := New(v) | ||
| assert.NoError(t, err, "Setting up config error", tt.description) | ||
|
|
||
| assert.Equal(t, tt.wantVendorExceptions, cfg.GDPR.TCF2.SpecialFeature1.VendorExceptions, tt.description) | ||
| assert.Equal(t, tt.wantVendorExceptionsMap, cfg.GDPR.TCF2.SpecialFeature1.VendorExceptionMap, tt.description) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. We were using the wrong terminology?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not just a matter of the wrong terminology. We are looking at the wrong field in the GVL. The go-gdpr package provides a
vendor.SpecialPurposemethod that checks if a special purpose is defined in the GVLspecialPurposesfield for a vendor. We are currently calling this method but that isn't correct. Instead we should be looking at GVLspecialFeaturesfor a vendor. I have a go-gdpr PR out that is currently under review that will add a new method that allows us to access the correct field for a given vendor.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'a concerning. What's the difference between a special purpose and a special feature? The TCF folks could've done better in the naming there.