From 84e1d034acf1a6f0d7297db2cfc90b00c2583741 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Wed, 25 Mar 2020 09:45:18 -0400 Subject: [PATCH] Fixes for PR comments --- gdpr/gdpr.go | 11 ++++++--- gdpr/impl.go | 4 +-- gdpr/impl_test.go | 42 ++++++++++++++++---------------- gdpr/vendorlist-fetching.go | 16 ++++++------ gdpr/vendorlist-fetching_test.go | 4 +-- 5 files changed, 41 insertions(+), 36 deletions(-) diff --git a/gdpr/gdpr.go b/gdpr/gdpr.go index ab562c5614b..a6b64203a95 100644 --- a/gdpr/gdpr.go +++ b/gdpr/gdpr.go @@ -26,6 +26,11 @@ type Permissions interface { PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) } +const ( + tCF1 uint8 = 1 + tCF2 uint8 = 2 +) + // NewPermissions gets an instance of the Permissions for use elsewhere in the project. func NewPermissions(ctx context.Context, cfg config.GDPR, vendorIDs map[openrtb_ext.BidderName]uint16, client *http.Client) Permissions { // If the host doesn't buy into the IAB GDPR consent framework, then save some cycles and let all syncs happen. @@ -36,9 +41,9 @@ func NewPermissions(ctx context.Context, cfg config.GDPR, vendorIDs map[openrtb_ return &permissionsImpl{ cfg: cfg, vendorIDs: vendorIDs, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - newVendorListFetcher(ctx, cfg, client, vendorListURLMaker, 1), - newVendorListFetcher(ctx, cfg, client, vendorListURLMaker, 2)}, + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: newVendorListFetcher(ctx, cfg, client, vendorListURLMaker, tCF1), + tCF2: newVendorListFetcher(ctx, cfg, client, vendorListURLMaker, tCF2)}, } } diff --git a/gdpr/impl.go b/gdpr/impl.go index fbd3f19b54d..615c3a090c9 100644 --- a/gdpr/impl.go +++ b/gdpr/impl.go @@ -20,7 +20,7 @@ import ( type permissionsImpl struct { cfg config.GDPR vendorIDs map[openrtb_ext.BidderName]uint16 - fetchVendorList []func(ctx context.Context, id uint16) (vendorlist.VendorList, error) + fetchVendorList map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error) } func (p *permissionsImpl) HostCookiesAllowed(ctx context.Context, consent string) (bool, error) { @@ -122,7 +122,7 @@ func (p *permissionsImpl) parseVendor(ctx context.Context, vendorID uint16, cons if version < 1 || version > 2 { return } - vendorList, err := p.fetchVendorList[version-1](ctx, parsedConsent.VendorListVersion()) + vendorList, err := p.fetchVendorList[version](ctx, parsedConsent.VendorListVersion()) if err != nil { return } diff --git a/gdpr/impl_test.go b/gdpr/impl_test.go index a8c920b3580..8b89577d6c8 100644 --- a/gdpr/impl_test.go +++ b/gdpr/impl_test.go @@ -19,9 +19,9 @@ func TestNoConsentButAllowByDefault(t *testing.T) { UsersyncIfAmbiguous: true, }, vendorIDs: nil, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - failedListFetcher, - failedListFetcher, + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: failedListFetcher, + tCF2: failedListFetcher, }, } allowSync, err := perms.BidderSyncAllowed(context.Background(), openrtb_ext.BidderAppnexus, "") @@ -39,9 +39,9 @@ func TestNoConsentAndRejectByDefault(t *testing.T) { UsersyncIfAmbiguous: false, }, vendorIDs: nil, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - failedListFetcher, - failedListFetcher, + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: failedListFetcher, + tCF2: failedListFetcher, }, } allowSync, err := perms.BidderSyncAllowed(context.Background(), openrtb_ext.BidderAppnexus, "") @@ -69,11 +69,11 @@ func TestAllowedSyncs(t *testing.T) { openrtb_ext.BidderAppnexus: 2, openrtb_ext.BidderPubmatic: 3, }, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - listFetcher(map[uint16]vendorlist.VendorList{ + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), - listFetcher(map[uint16]vendorlist.VendorList{ + tCF2: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), }, @@ -105,11 +105,11 @@ func TestProhibitedPurposes(t *testing.T) { openrtb_ext.BidderAppnexus: 2, openrtb_ext.BidderPubmatic: 3, }, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - listFetcher(map[uint16]vendorlist.VendorList{ + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), - listFetcher(map[uint16]vendorlist.VendorList{ + tCF2: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), }, @@ -141,11 +141,11 @@ func TestProhibitedVendors(t *testing.T) { openrtb_ext.BidderAppnexus: 2, openrtb_ext.BidderPubmatic: 3, }, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - listFetcher(map[uint16]vendorlist.VendorList{ + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), - listFetcher(map[uint16]vendorlist.VendorList{ + tCF2: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), }, @@ -165,9 +165,9 @@ func TestMalformedConsent(t *testing.T) { cfg: config.GDPR{ HostVendorID: 2, }, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - listFetcher(nil), - listFetcher(nil), + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: listFetcher(nil), + tCF2: listFetcher(nil), }, } @@ -193,11 +193,11 @@ func TestAllowPersonalInfo(t *testing.T) { openrtb_ext.BidderAppnexus: 2, openrtb_ext.BidderPubmatic: 3, }, - fetchVendorList: []func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ - listFetcher(map[uint16]vendorlist.VendorList{ + fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){ + tCF1: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), - listFetcher(map[uint16]vendorlist.VendorList{ + tCF2: listFetcher(map[uint16]vendorlist.VendorList{ 1: parseVendorListData(t, vendorListData), }), }, diff --git a/gdpr/vendorlist-fetching.go b/gdpr/vendorlist-fetching.go index 9078b8420da..987622a6a8a 100644 --- a/gdpr/vendorlist-fetching.go +++ b/gdpr/vendorlist-fetching.go @@ -26,7 +26,7 @@ type saveVendors func(uint16, api.VendorList) // // Nothing in this file is exported. Public APIs can be found in gdpr.go -func newVendorListFetcher(initCtx context.Context, cfg config.GDPR, client *http.Client, urlMaker func(uint16, int) string, TCFVer int) func(ctx context.Context, id uint16) (vendorlist.VendorList, error) { +func newVendorListFetcher(initCtx context.Context, cfg config.GDPR, client *http.Client, urlMaker func(uint16, uint8) string, TCFVer uint8) func(ctx context.Context, id uint16) (vendorlist.VendorList, error) { // These save and load functions can be used to store & retrieve lists from our cache. save, load := newVendorListCache() @@ -51,18 +51,18 @@ func newVendorListFetcher(initCtx context.Context, cfg config.GDPR, client *http } // populateCache saves all the known versions of the vendor list for future use. -func populateCache(ctx context.Context, client *http.Client, urlMaker func(uint16, int) string, saver saveVendors, TCFVer int) { - latestVersion := saveOne(ctx, client, urlMaker(0, 1), saver, TCFVer) +func populateCache(ctx context.Context, client *http.Client, urlMaker func(uint16, uint8) string, saver saveVendors, TCFVer uint8) { + latestVersion := saveOne(ctx, client, urlMaker(0, TCFVer), saver, TCFVer) for i := uint16(1); i < latestVersion; i++ { - saveOne(ctx, client, urlMaker(i, 1), saver, TCFVer) + saveOne(ctx, client, urlMaker(i, TCFVer), saver, TCFVer) } } // Make a URL which can be used to fetch a given version of the Global Vendor List. If the version is 0, // this will fetch the latest version. -func vendorListURLMaker(version uint16, tCFVersion int) string { - if tCFVersion == 2 { +func vendorListURLMaker(version uint16, TCFVer uint8) string { + if TCFVer == 2 { if version == 0 { return "https://vendorlist.consensu.org/v2/vendor-list.json" } @@ -79,7 +79,7 @@ func vendorListURLMaker(version uint16, tCFVersion int) string { // The goal here is to update quickly when new versions of the VendorList are released, but not wreck // server performance if a bad CMP starts sending us malformed consent strings that advertize a version // that doesn't exist yet. -func newOccasionalSaver(timeout time.Duration, TCFVer int) func(ctx context.Context, client *http.Client, url string, saver saveVendors) { +func newOccasionalSaver(timeout time.Duration, TCFVer uint8) func(ctx context.Context, client *http.Client, url string, saver saveVendors) { lastSaved := &atomic.Value{} lastSaved.Store(time.Time{}) @@ -94,7 +94,7 @@ func newOccasionalSaver(timeout time.Duration, TCFVer int) func(ctx context.Cont } } -func saveOne(ctx context.Context, client *http.Client, url string, saver saveVendors, cTFVer int) uint16 { +func saveOne(ctx context.Context, client *http.Client, url string, saver saveVendors, cTFVer uint8) uint16 { req, err := http.NewRequest("GET", url, nil) if err != nil { glog.Errorf("Failed to build GET %s request. Cookie syncs may be affected: %v", url, err) diff --git a/gdpr/vendorlist-fetching_test.go b/gdpr/vendorlist-fetching_test.go index 97de82e5c38..8197fa263bc 100644 --- a/gdpr/vendorlist-fetching_test.go +++ b/gdpr/vendorlist-fetching_test.go @@ -203,9 +203,9 @@ func mockVendorListData(t *testing.T, version uint16, vendors map[uint16]*purpos return string(data) } -func testURLMaker(server *httptest.Server) func(uint16, int) string { +func testURLMaker(server *httptest.Server) func(uint16, uint8) string { url := server.URL - return func(version uint16, TCFVer int) string { + return func(version uint16, TCFVer uint8) string { return url + "?version=" + strconv.Itoa(int(version)) } }