From f1ce47036f7d49e0e6d3951d2ac2a798629349b5 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Mon, 17 Jul 2023 11:54:06 -0700 Subject: [PATCH 01/20] Priority Bidder Code --- config/config.go | 2 + config/usersync.go | 10 +- endpoints/cookie_sync.go | 2 +- endpoints/cookie_sync_test.go | 25 ++-- endpoints/setuid.go | 2 +- pbs/usersync.go | 2 +- usersync/cookie.go | 30 +--- usersync/cookie_test.go | 15 +- usersync/ejector.go | 102 +++++++++++++ usersync/ejector_test.go | 259 ++++++++++++++++++++++++++++++++++ 10 files changed, 402 insertions(+), 47 deletions(-) create mode 100644 usersync/ejector.go create mode 100644 usersync/ejector_test.go diff --git a/config/config.go b/config/config.go index 543d1afd912..340e291a315 100644 --- a/config/config.go +++ b/config/config.go @@ -951,6 +951,8 @@ func SetupViper(v *viper.Viper, filename string, bidderInfos BidderInfos) { v.SetDefault("event.timeout_ms", 1000) + v.SetDefault("user_sync.priority_groups", [][]string{}) + v.SetDefault("accounts.filesystem.enabled", false) v.SetDefault("accounts.filesystem.directorypath", "./stored_requests/data/by_id") v.SetDefault("accounts.in_memory_cache.type", "none") diff --git a/config/usersync.go b/config/usersync.go index 27badbcb815..d490853d08e 100644 --- a/config/usersync.go +++ b/config/usersync.go @@ -2,13 +2,13 @@ package config // UserSync specifies the static global user sync configuration. type UserSync struct { - Cooperative UserSyncCooperative `mapstructure:"coop_sync"` - ExternalURL string `mapstructure:"external_url"` - RedirectURL string `mapstructure:"redirect_url"` + Cooperative UserSyncCooperative `mapstructure:"coop_sync"` + ExternalURL string `mapstructure:"external_url"` + RedirectURL string `mapstructure:"redirect_url"` + PriorityGroups [][]string `mapstructure:"priority_groups"` } // UserSyncCooperative specifies the static global default cooperative cookie sync type UserSyncCooperative struct { - EnabledByDefault bool `mapstructure:"default"` - PriorityGroups [][]string `mapstructure:"priority_groups"` + EnabledByDefault bool `mapstructure:"default"` } diff --git a/endpoints/cookie_sync.go b/endpoints/cookie_sync.go index ef3c40e8815..6d0a6a79965 100644 --- a/endpoints/cookie_sync.go +++ b/endpoints/cookie_sync.go @@ -166,7 +166,7 @@ func (c *cookieSyncEndpoint) parseRequest(r *http.Request) (usersync.Request, pr Bidders: request.Bidders, Cooperative: usersync.Cooperative{ Enabled: (request.CooperativeSync != nil && *request.CooperativeSync) || (request.CooperativeSync == nil && c.config.UserSync.Cooperative.EnabledByDefault), - PriorityGroups: c.config.UserSync.Cooperative.PriorityGroups, + PriorityGroups: c.config.UserSync.PriorityGroups, }, Limit: request.Limit, Privacy: usersyncPrivacy{ diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index a69a2cdb819..ab88664695c 100644 --- a/endpoints/cookie_sync_test.go +++ b/endpoints/cookie_sync_test.go @@ -515,9 +515,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{ @@ -562,9 +562,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{ @@ -615,9 +615,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: true, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -641,9 +641,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -667,9 +667,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: true, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -693,9 +693,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -719,9 +719,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: true, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -745,9 +745,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -888,9 +888,8 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ - Cooperative: config.UserSyncCooperative{ - PriorityGroups: [][]string{{"a", "b", "c"}}, - }, + PriorityGroups: [][]string{{"a", "b", "c"}}, + Cooperative: config.UserSyncCooperative{}, }, expectedPrivacy: privacy.Policies{}, expectedRequest: usersync.Request{ @@ -918,9 +917,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, @@ -949,9 +948,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 905fa26b2f2..e0ec8848791 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -157,7 +157,7 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use setSiteCookie := siteCookieCheck(r.UserAgent()) // Write Cookie - encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder) + encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, SyncerKey: syncer.Key(), OldestEjector: usersync.OldestEjector{}}) if err != nil { w.WriteHeader(http.StatusBadRequest) metricsEngine.RecordSetUid(metrics.SetUidBadRequest) diff --git a/pbs/usersync.go b/pbs/usersync.go index 7b468cb039d..2b576788804 100644 --- a/pbs/usersync.go +++ b/pbs/usersync.go @@ -81,7 +81,7 @@ func (deps *UserSyncDeps) OptOut(w http.ResponseWriter, r *http.Request, _ httpr pc.SetOptOut(optout != "") // Write Cookie - encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder) + encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder, &usersync.OldestEjector{}) // TODO: Getting Non Priority Keys Here if err != nil { w.WriteHeader(http.StatusBadRequest) return diff --git a/usersync/cookie.go b/usersync/cookie.go index c0eb898c5ea..825d164c031 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "net/http" - "sort" "time" "github.com/prebid/prebid-server/config" @@ -58,10 +57,7 @@ func ReadCookie(r *http.Request, decoder Decoder, host *config.HostCookie) *Cook } // PrepareCookieForWrite ejects UIDs as long as the cookie is too full -func (cookie *Cookie) PrepareCookieForWrite(cfg *config.HostCookie, encoder Encoder) (string, error) { - uuidKeys := sortUIDs(cookie.uids) - - i := 0 +func (cookie *Cookie) PrepareCookieForWrite(cfg *config.HostCookie, encoder Encoder, ejector Ejector) (string, error) { for len(cookie.uids) > 0 { encodedCookie, err := encoder.Encode(cookie) if err != nil { @@ -82,10 +78,11 @@ func (cookie *Cookie) PrepareCookieForWrite(cfg *config.HostCookie, encoder Enco return encodedCookie, nil } - uidToDelete := uuidKeys[i] + uidToDelete, err := ejector.Choose(cookie.uids) + if err != nil { + return encodedCookie, err + } delete(cookie.uids, uidToDelete) - - i++ } return "", nil } @@ -132,23 +129,6 @@ func (cookie *Cookie) Sync(key string, uid string) error { return nil } -// sortUIDs is used to get a list of uids sorted from oldest to newest -// This list is used to eject oldest uids from the cookie -// This will be incorporated with a more complex ejection framework in a future PR -func sortUIDs(uids map[string]UIDEntry) []string { - if len(uids) > 0 { - uuidKeys := make([]string, 0, len(uids)) - for key := range uids { - uuidKeys = append(uuidKeys, key) - } - sort.SliceStable(uuidKeys, func(i, j int) bool { - return uids[uuidKeys[i]].Expires.Before(uids[uuidKeys[j]].Expires) - }) - return uuidKeys - } - return nil -} - // SyncHostCookie syncs the request cookie with the host cookie func SyncHostCookie(r *http.Request, requestCookie *Cookie, host *config.HostCookie) { if uid, _, _ := requestCookie.GetUID(host.Family); uid == "" && host.CookieName != "" { diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 5b86df54441..8e779346298 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -393,6 +393,19 @@ func TestPrepareCookieForWrite(t *testing.T) { optOut: false, } + ejector := &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"1"}, + {"2", "3"}, + {"4", "5", "6"}, + {"7"}, + }, + SyncerKey: "1", + OldestEjector: OldestEjector{ + nonPriorityKeys: []string{}, + }, + } + testCases := []struct { name string givenMaxCookieSize int @@ -447,7 +460,7 @@ func TestPrepareCookieForWrite(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - encodedCookie, err := cookieToSend.PrepareCookieForWrite(&config.HostCookie{MaxCookieSizeBytes: test.givenMaxCookieSize}, encoder) + encodedCookie, err := cookieToSend.PrepareCookieForWrite(&config.HostCookie{MaxCookieSizeBytes: test.givenMaxCookieSize}, encoder, ejector) assert.NoError(t, err) decodedCookie := decoder.Decode(encodedCookie) diff --git a/usersync/ejector.go b/usersync/ejector.go new file mode 100644 index 00000000000..f9cc1ac0817 --- /dev/null +++ b/usersync/ejector.go @@ -0,0 +1,102 @@ +package usersync + +import ( + "errors" + "math" + "time" +) + +type Ejector interface { + Choose(uids map[string]UIDEntry) (string, error) +} + +type OldestEjector struct { + nonPriorityKeys []string +} + +func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { + oldestElement := getOldestElement(o.nonPriorityKeys, uids) + + return oldestElement, nil +} + +type PriorityBidderEjector struct { + PriorityGroups [][]string + SyncerKey string + OldestEjector OldestEjector +} + +func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { + p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) + // There are non priority keys present, let's eject one of those + if len(p.OldestEjector.nonPriorityKeys) > 0 { + return p.OldestEjector.Choose(uids) + } + + // There are only priority keys left, check if the syncer is apart of the priority groups + if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { + // Eject Oldest Element from Lowest Priority + lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] + + oldestElement := getOldestElement(lowestPriorityGroup, uids) + + return oldestElement, nil + } + return "", errors.New("syncer key " + p.SyncerKey + " is not in priority groups") +} + +func isSyncerPriority(syncer string, priorityGroups [][]string) bool { + for _, group := range priorityGroups { + for _, bidder := range group { + if syncer == bidder { + return true + } + } + } + return false +} + +func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []string { + nonPriorityKeys := []string{} + + if len(priorityGroups) == 0 { + for key := range uids { + nonPriorityKeys = append(nonPriorityKeys, key) + } + return nonPriorityKeys + } + + for key := range uids { + for _, group := range priorityGroups { + isPriority := false + for _, bidder := range group { + if key == bidder { + isPriority = true + break + } + } + if !isPriority { + nonPriorityKeys = append(nonPriorityKeys, key) + } + } + } + return nonPriorityKeys +} + +func getOldestElement(list []string, uids map[string]UIDEntry) string { + var oldestElem string = "" + var oldestDate int64 = math.MaxInt64 + + for _, key := range list { + if value, ok := uids[key]; ok { + timeUntilExpiration := time.Until(value.Expires) + if timeUntilExpiration < time.Duration(oldestDate) { + oldestElem = key + oldestDate = int64(timeUntilExpiration) + } + } else { + continue + } + } + return oldestElem +} diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go new file mode 100644 index 00000000000..f365a693b1e --- /dev/null +++ b/usersync/ejector_test.go @@ -0,0 +1,259 @@ +package usersync + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestChooseElementToEject(t *testing.T) { + testCases := []struct { + name string + givenUids map[string]UIDEntry + givenEjector Ejector + expected string + expectedError error + }{ + { + name: "priority-ejector", + givenUids: map[string]UIDEntry{ + "newestElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "oldestElement": { + UID: "456", + Expires: time.Now(), + }, + }, + givenEjector: &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"adnxs"}, + {"oldestElement", "newerElement"}, + }, + SyncerKey: "adnxs", + }, + expected: "oldestElement", + }, + { + name: "priority-ejector-syncer-not-priority", + givenUids: map[string]UIDEntry{}, + givenEjector: &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"syncerKey1"}, + }, + SyncerKey: "adnxs", + }, + expectedError: errors.New("syncer key adnxs is not in priority groups"), + }, + { + name: "oldest-ejector", + givenUids: map[string]UIDEntry{ + "newestElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "oldestElement": { + UID: "456", + Expires: time.Now(), + }, + }, + givenEjector: &OldestEjector{ + []string{"newestElement", "oldestElement"}, + }, + expected: "oldestElement", + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + uidToDelete, err := test.givenEjector.Choose(test.givenUids) + if test.expectedError != nil { + assert.Equal(t, test.expectedError, err) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expected, uidToDelete) + } + }) + } +} + +func TestGetOldestElement(t *testing.T) { + testCases := []struct { + name string + givenUids map[string]UIDEntry + givenFilteredKeys []string + expected string + }{ + { + name: "basic-oldest-element", + givenUids: map[string]UIDEntry{ + "newestElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "oldestElement": { + UID: "456", + Expires: time.Now(), + }, + }, + givenFilteredKeys: []string{"newestElement", "oldestElement"}, + expected: "oldestElement", + }, + { + name: "no-filtered-keys", + givenUids: map[string]UIDEntry{ + "newestElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "oldestElement": { + UID: "456", + Expires: time.Now(), + }, + }, + givenFilteredKeys: []string{}, + expected: "", + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + oldestElement := getOldestElement(test.givenFilteredKeys, test.givenUids) + assert.Equal(t, test.expected, oldestElement) + }) + } +} + +func TestGetNonPriorityKeys(t *testing.T) { + testCases := []struct { + name string + givenUids map[string]UIDEntry + givenPriorityGroups [][]string + expected []string + }{ + { + name: "one-priority-group", + givenUids: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", + }, + }, + givenPriorityGroups: [][]string{ + {"syncerKey1"}, + }, + expected: []string{"syncerKey2", "syncerKey3"}, + }, + { + name: "no-priority-groups", + givenUids: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", + }, + }, + givenPriorityGroups: [][]string{}, + expected: []string{"syncerKey1", "syncerKey2", "syncerKey3"}, + }, + { + name: "no-given-uids", + givenUids: map[string]UIDEntry{}, + givenPriorityGroups: [][]string{ + {"syncerKey1"}, + }, + expected: []string{}, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + keys := getNonPriorityKeys(test.givenUids, test.givenPriorityGroups) + assert.Equal(t, true, containsSameElements(test.expected, keys)) + }) + } +} + +func TestIsSyncerPriority(t *testing.T) { + testCases := []struct { + name string + givenSyncerKey string + givenPriorityGroups [][]string + expected bool + }{ + { + name: "syncer-is-priority", + givenSyncerKey: "adnxs", + givenPriorityGroups: [][]string{ + {"adnxs"}, + {"2", "3"}, + }, + expected: true, + }, + { + name: "syncer-is-not-priority", + givenSyncerKey: "adnxs", + givenPriorityGroups: [][]string{ + {"1"}, + {"2", "3"}, + }, + expected: false, + }, + { + name: "no-syncer-given", + givenSyncerKey: "", + givenPriorityGroups: [][]string{ + {"1"}, + {"2", "3"}, + }, + expected: false, + }, + { + name: "no-priority-groups-given", + givenSyncerKey: "adnxs", + givenPriorityGroups: [][]string{}, + expected: false, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + isPriority := isSyncerPriority(test.givenSyncerKey, test.givenPriorityGroups) + assert.Equal(t, test.expected, isPriority) + }) + } +} + +func containsSameElements(x, y []string) bool { + if len(x) != len(y) { + return false + } + + countOfElements := make(map[string]int) + + for _, key := range x { + countOfElements[key]++ + } + for _, key := range y { + countOfElements[key]-- + } + + for _, count := range countOfElements { + if count != 0 { + return false + } + } + return true +} From de5e6180666c7a6801fe29727f280f61c64003ee Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 18 Jul 2023 18:39:20 -0700 Subject: [PATCH 02/20] Tweak Priority Ejector to update pri groups --- usersync/ejector.go | 38 ++++++++++++++++++++++++++++++-------- usersync/ejector_test.go | 22 +++++++++++++++++++++- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/usersync/ejector.go b/usersync/ejector.go index f9cc1ac0817..9961a3bd7e1 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -28,23 +28,46 @@ type PriorityBidderEjector struct { func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) - // There are non priority keys present, let's eject one of those + + // There are non priority keys present, eject one of those if len(p.OldestEjector.nonPriorityKeys) > 0 { return p.OldestEjector.Choose(uids) } // There are only priority keys left, check if the syncer is apart of the priority groups if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { - // Eject Oldest Element from Lowest Priority + // Eject Oldest Element from Lowest Priority Group and Update Priority Group lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] - oldestElement := getOldestElement(lowestPriorityGroup, uids) + updatedPriorityGroup := removeElementFromPriorityGroup(lowestPriorityGroup, oldestElement) + + if updatedPriorityGroup == nil { + p.PriorityGroups = p.PriorityGroups[:len(p.PriorityGroups)-1] + } else { + p.PriorityGroups[len(p.PriorityGroups)-1] = updatedPriorityGroup + } return oldestElement, nil } return "", errors.New("syncer key " + p.SyncerKey + " is not in priority groups") } +// updatePriorityGroup will remove the selected element from the given priority group list +func removeElementFromPriorityGroup(lowestGroup []string, oldestElement string) []string { + if len(lowestGroup) <= 1 { + return nil + } + var updatedPriorityGroup []string + + for index, elem := range lowestGroup { + if elem == oldestElement { + updatedPriorityGroup := append(lowestGroup[:index], lowestGroup[index+1:]...) + return updatedPriorityGroup + } + } + return updatedPriorityGroup +} + func isSyncerPriority(syncer string, priorityGroups [][]string) bool { for _, group := range priorityGroups { for _, bidder := range group { @@ -58,7 +81,6 @@ func isSyncerPriority(syncer string, priorityGroups [][]string) bool { func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []string { nonPriorityKeys := []string{} - if len(priorityGroups) == 0 { for key := range uids { nonPriorityKeys = append(nonPriorityKeys, key) @@ -67,17 +89,17 @@ func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []s } for key := range uids { + isPriority := false for _, group := range priorityGroups { - isPriority := false for _, bidder := range group { if key == bidder { isPriority = true break } } - if !isPriority { - nonPriorityKeys = append(nonPriorityKeys, key) - } + } + if !isPriority { + nonPriorityKeys = append(nonPriorityKeys, key) } } return nonPriorityKeys diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index f365a693b1e..6116528ba22 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -31,7 +31,7 @@ func TestChooseElementToEject(t *testing.T) { givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ {"adnxs"}, - {"oldestElement", "newerElement"}, + {"oldestElement", "newestElement"}, }, SyncerKey: "adnxs", }, @@ -65,6 +65,26 @@ func TestChooseElementToEject(t *testing.T) { }, expected: "oldestElement", }, + { + name: "priority-oldest-ejector-fallback", + givenUids: map[string]UIDEntry{ + "nonPriorityElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "priorityElement": { + UID: "456", + Expires: time.Now(), + }, + }, + givenEjector: &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"priorityElement"}, + }, + OldestEjector: OldestEjector{}, + }, + expected: "nonPriorityElement", + }, } for _, test := range testCases { From 73a9daf68fcb4240927c584786f27c0489b4ab97 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 19 Jul 2023 14:16:55 -0700 Subject: [PATCH 03/20] Fallback Ejector --- usersync/ejector.go | 36 +++++++++++++++++++++++++++++------- usersync/ejector_test.go | 1 + 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/usersync/ejector.go b/usersync/ejector.go index 9961a3bd7e1..101a0818c8c 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -3,6 +3,7 @@ package usersync import ( "errors" "math" + "math/rand" "time" ) @@ -12,35 +13,40 @@ type Ejector interface { type OldestEjector struct { nonPriorityKeys []string + FallbackEjector FallbackEjector } func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { oldestElement := getOldestElement(o.nonPriorityKeys, uids) + if oldestElement == "" { + return o.FallbackEjector.Choose(uids) + } return oldestElement, nil } type PriorityBidderEjector struct { - PriorityGroups [][]string - SyncerKey string - OldestEjector OldestEjector + PriorityGroups [][]string + SyncerKey string + OldestEjector OldestEjector + FallbackEjector FallbackEjector } func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) - - // There are non priority keys present, eject one of those if len(p.OldestEjector.nonPriorityKeys) > 0 { return p.OldestEjector.Choose(uids) } - // There are only priority keys left, check if the syncer is apart of the priority groups if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { // Eject Oldest Element from Lowest Priority Group and Update Priority Group lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] oldestElement := getOldestElement(lowestPriorityGroup, uids) - updatedPriorityGroup := removeElementFromPriorityGroup(lowestPriorityGroup, oldestElement) + if oldestElement == "" { + return p.FallbackEjector.Choose(uids) + } + updatedPriorityGroup := removeElementFromPriorityGroup(lowestPriorityGroup, oldestElement) if updatedPriorityGroup == nil { p.PriorityGroups = p.PriorityGroups[:len(p.PriorityGroups)-1] } else { @@ -122,3 +128,19 @@ func getOldestElement(list []string, uids map[string]UIDEntry) string { } return oldestElem } + +type FallbackEjector struct{} + +// Choose implemention for Fallback Ejector, selects a random uid to eject when the other two ejectors can't come up with a uid +func (f *FallbackEjector) Choose(uids map[string]UIDEntry) (string, error) { + keys := make([]string, len(uids)) + + i := 0 + for key := range uids { + keys[i] = key + i++ + } + + rand.Seed(time.Now().UnixNano()) + return keys[rand.Intn(len(keys))], nil +} diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index 6116528ba22..fe67bde1dc8 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -62,6 +62,7 @@ func TestChooseElementToEject(t *testing.T) { }, givenEjector: &OldestEjector{ []string{"newestElement", "oldestElement"}, + FallbackEjector{}, }, expected: "oldestElement", }, From b0feb46e51d253a895326d871967e594ef4854b9 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Mon, 24 Jul 2023 14:32:54 -0700 Subject: [PATCH 04/20] Opt out non prioity key support for oldest ejector --- pbs/usersync.go | 3 ++- router/router.go | 1 + usersync/ejector.go | 5 +++++ usersync/ejector_test.go | 8 ++++++++ 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pbs/usersync.go b/pbs/usersync.go index 2b576788804..6ea27218e6b 100644 --- a/pbs/usersync.go +++ b/pbs/usersync.go @@ -22,6 +22,7 @@ type UserSyncDeps struct { ExternalUrl string RecaptchaSecret string HostCookieConfig *config.HostCookie + PriorityGroups [][]string } // Struct for parsing json in google's response @@ -81,7 +82,7 @@ func (deps *UserSyncDeps) OptOut(w http.ResponseWriter, r *http.Request, _ httpr pc.SetOptOut(optout != "") // Write Cookie - encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder, &usersync.OldestEjector{}) // TODO: Getting Non Priority Keys Here + encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder, &usersync.OldestEjector{PriorityGroups: deps.PriorityGroups}) if err != nil { w.WriteHeader(http.StatusBadRequest) return diff --git a/router/router.go b/router/router.go index 8fc99589c22..dc7e785b358 100644 --- a/router/router.go +++ b/router/router.go @@ -264,6 +264,7 @@ func New(cfg *config.Configuration, rateConvertor *currency.RateConverter) (r *R HostCookieConfig: &(cfg.HostCookie), ExternalUrl: cfg.ExternalURL, RecaptchaSecret: cfg.RecaptchaSecret, + PriorityGroups: cfg.UserSync.PriorityGroups, } r.GET("/setuid", endpoints.NewSetUIDEndpoint(cfg, syncersByBidder, gdprPermsBuilder, tcf2CfgBuilder, pbsAnalytics, accounts, r.MetricsEngine)) diff --git a/usersync/ejector.go b/usersync/ejector.go index 101a0818c8c..8e90610503a 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -13,10 +13,15 @@ type Ejector interface { type OldestEjector struct { nonPriorityKeys []string + PriorityGroups [][]string FallbackEjector FallbackEjector } func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { + if len(o.nonPriorityKeys) == 0 { + o.nonPriorityKeys = getNonPriorityKeys(uids, o.PriorityGroups) + } + oldestElement := getOldestElement(o.nonPriorityKeys, uids) if oldestElement == "" { return o.FallbackEjector.Choose(uids) diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index fe67bde1dc8..d1a7f7ce419 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -62,6 +62,7 @@ func TestChooseElementToEject(t *testing.T) { }, givenEjector: &OldestEjector{ []string{"newestElement", "oldestElement"}, + [][]string{}, FallbackEjector{}, }, expected: "oldestElement", @@ -101,6 +102,13 @@ func TestChooseElementToEject(t *testing.T) { } } +// TODO: +func TestOldestEjector(t *testing.T) {} + +func TestPriorityEjector(t *testing.T) {} + +func TestFallbackEjector(t *testing.T) {} + func TestGetOldestElement(t *testing.T) { testCases := []struct { name string From f29fafa825fca16c0280e214070eaf20ee06863b Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 25 Jul 2023 15:23:58 -0700 Subject: [PATCH 05/20] Simplified removeElementFromPriorityGroup --- endpoints/setuid.go | 2 +- usersync/ejector.go | 27 +++++++++++++-------------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index e0ec8848791..87cd30e5ec8 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -157,7 +157,7 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use setSiteCookie := siteCookieCheck(r.UserAgent()) // Write Cookie - encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, SyncerKey: syncer.Key(), OldestEjector: usersync.OldestEjector{}}) + encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, SyncerKey: syncer.Key()}) if err != nil { w.WriteHeader(http.StatusBadRequest) metricsEngine.RecordSetUid(metrics.SetUidBadRequest) diff --git a/usersync/ejector.go b/usersync/ejector.go index 8e90610503a..6b73af4af14 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -38,6 +38,7 @@ type PriorityBidderEjector struct { } func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { + // If there are non priority keys left in the cookie.uids, we should eject one of those using the OldestEjector p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) if len(p.OldestEjector.nonPriorityKeys) > 0 { return p.OldestEjector.Choose(uids) @@ -46,17 +47,13 @@ func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { // Eject Oldest Element from Lowest Priority Group and Update Priority Group lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] + oldestElement := getOldestElement(lowestPriorityGroup, uids) if oldestElement == "" { return p.FallbackEjector.Choose(uids) } - updatedPriorityGroup := removeElementFromPriorityGroup(lowestPriorityGroup, oldestElement) - if updatedPriorityGroup == nil { - p.PriorityGroups = p.PriorityGroups[:len(p.PriorityGroups)-1] - } else { - p.PriorityGroups[len(p.PriorityGroups)-1] = updatedPriorityGroup - } + p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, oldestElement) return oldestElement, nil } @@ -64,19 +61,21 @@ func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) } // updatePriorityGroup will remove the selected element from the given priority group list -func removeElementFromPriorityGroup(lowestGroup []string, oldestElement string) []string { - if len(lowestGroup) <= 1 { - return nil +func removeElementFromPriorityGroup(priorityGroups [][]string, oldestElement string) [][]string { + lowestPriorityGroup := priorityGroups[len(priorityGroups)-1] + if len(lowestPriorityGroup) <= 1 { + priorityGroups = priorityGroups[:len(priorityGroups)-1] // Remove entire priority group since it will be empty + return priorityGroups } - var updatedPriorityGroup []string - for index, elem := range lowestGroup { + for index, elem := range lowestPriorityGroup { if elem == oldestElement { - updatedPriorityGroup := append(lowestGroup[:index], lowestGroup[index+1:]...) - return updatedPriorityGroup + updatedPriorityGroup := append(lowestPriorityGroup[:index], lowestPriorityGroup[index+1:]...) // Delete only that element from the group + priorityGroups[len(priorityGroups)-1] = updatedPriorityGroup + return priorityGroups } } - return updatedPriorityGroup + return priorityGroups } func isSyncerPriority(syncer string, priorityGroups [][]string) bool { From bb166890ef0e8e094b758c007b02b6a7930905e4 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 25 Jul 2023 16:09:57 -0700 Subject: [PATCH 06/20] Add ejector specific tests --- usersync/ejector.go | 56 ++++++++++++------------- usersync/ejector_test.go | 88 ++++++++++++++++++++++++++++++++-------- 2 files changed, 98 insertions(+), 46 deletions(-) diff --git a/usersync/ejector.go b/usersync/ejector.go index 6b73af4af14..7122527d5cc 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -17,6 +17,16 @@ type OldestEjector struct { FallbackEjector FallbackEjector } +type PriorityBidderEjector struct { + PriorityGroups [][]string + SyncerKey string + OldestEjector OldestEjector + FallbackEjector FallbackEjector +} + +type FallbackEjector struct{} + +// Choose method for oldest ejector will return the oldest non priority uid func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { if len(o.nonPriorityKeys) == 0 { o.nonPriorityKeys = getNonPriorityKeys(uids, o.PriorityGroups) @@ -30,22 +40,14 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { return oldestElement, nil } -type PriorityBidderEjector struct { - PriorityGroups [][]string - SyncerKey string - OldestEjector OldestEjector - FallbackEjector FallbackEjector -} - +// Choose method for priority ejector will return the oldest priority uid, otherwise it will call a different ejector method func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { - // If there are non priority keys left in the cookie.uids, we should eject one of those using the OldestEjector p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) if len(p.OldestEjector.nonPriorityKeys) > 0 { return p.OldestEjector.Choose(uids) } if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { - // Eject Oldest Element from Lowest Priority Group and Update Priority Group lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] oldestElement := getOldestElement(lowestPriorityGroup, uids) @@ -60,17 +62,31 @@ func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) return "", errors.New("syncer key " + p.SyncerKey + " is not in priority groups") } -// updatePriorityGroup will remove the selected element from the given priority group list +// Choose method for Fallback Ejector, selects a random uid to eject when the other two ejectors can't come up with a uid +func (f *FallbackEjector) Choose(uids map[string]UIDEntry) (string, error) { + keys := make([]string, len(uids)) + + i := 0 + for key := range uids { + keys[i] = key + i++ + } + + rand.Seed(time.Now().UnixNano()) + return keys[rand.Intn(len(keys))], nil +} + +// updatePriorityGroup will remove the selected element from the priority groups, and will remove the entire priority group if it's empty func removeElementFromPriorityGroup(priorityGroups [][]string, oldestElement string) [][]string { lowestPriorityGroup := priorityGroups[len(priorityGroups)-1] if len(lowestPriorityGroup) <= 1 { - priorityGroups = priorityGroups[:len(priorityGroups)-1] // Remove entire priority group since it will be empty + priorityGroups = priorityGroups[:len(priorityGroups)-1] return priorityGroups } for index, elem := range lowestPriorityGroup { if elem == oldestElement { - updatedPriorityGroup := append(lowestPriorityGroup[:index], lowestPriorityGroup[index+1:]...) // Delete only that element from the group + updatedPriorityGroup := append(lowestPriorityGroup[:index], lowestPriorityGroup[index+1:]...) priorityGroups[len(priorityGroups)-1] = updatedPriorityGroup return priorityGroups } @@ -132,19 +148,3 @@ func getOldestElement(list []string, uids map[string]UIDEntry) string { } return oldestElem } - -type FallbackEjector struct{} - -// Choose implemention for Fallback Ejector, selects a random uid to eject when the other two ejectors can't come up with a uid -func (f *FallbackEjector) Choose(uids map[string]UIDEntry) (string, error) { - keys := make([]string, len(uids)) - - i := 0 - for key := range uids { - keys[i] = key - i++ - } - - rand.Seed(time.Now().UnixNano()) - return keys[rand.Intn(len(keys))], nil -} diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index d1a7f7ce419..ed88af77773 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestChooseElementToEject(t *testing.T) { +func TestPriorityEjector(t *testing.T) { testCases := []struct { name string givenUids map[string]UIDEntry @@ -19,11 +19,11 @@ func TestChooseElementToEject(t *testing.T) { { name: "priority-ejector", givenUids: map[string]UIDEntry{ - "newestElement": { + "newestLowestPriorityElement": { UID: "123", Expires: time.Now().Add((90 * 24 * time.Hour)), }, - "oldestElement": { + "oldestLowestPriorityElement": { UID: "456", Expires: time.Now(), }, @@ -31,11 +31,11 @@ func TestChooseElementToEject(t *testing.T) { givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ {"adnxs"}, - {"oldestElement", "newestElement"}, + {"oldestLowestPriorityElement", "newestLowestPriorityElement"}, }, SyncerKey: "adnxs", }, - expected: "oldestElement", + expected: "oldestLowestPriorityElement", }, { name: "priority-ejector-syncer-not-priority", @@ -48,6 +48,49 @@ func TestChooseElementToEject(t *testing.T) { }, expectedError: errors.New("syncer key adnxs is not in priority groups"), }, + { + name: "priority-calls-oldest-ejector", + givenUids: map[string]UIDEntry{ + "nonPriorityElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "priorityElement": { + UID: "456", + Expires: time.Now(), + }, + }, + givenEjector: &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"priorityElement"}, + }, + OldestEjector: OldestEjector{}, + }, + expected: "nonPriorityElement", + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + uidToDelete, err := test.givenEjector.Choose(test.givenUids) + if test.expectedError != nil { + assert.Equal(t, test.expectedError, err) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expected, uidToDelete) + } + }) + } +} + +func TestOldestEjector(t *testing.T) { + testCases := []struct { + name string + givenUids map[string]UIDEntry + givenEjector Ejector + expected string + expectedError error + }{ { name: "oldest-ejector", givenUids: map[string]UIDEntry{ @@ -68,24 +111,23 @@ func TestChooseElementToEject(t *testing.T) { expected: "oldestElement", }, { - name: "priority-oldest-ejector-fallback", + name: "non-priority-keys-empty", givenUids: map[string]UIDEntry{ - "nonPriorityElement": { + "newestElement": { UID: "123", Expires: time.Now().Add((90 * 24 * time.Hour)), }, - "priorityElement": { + "oldestElement": { UID: "456", Expires: time.Now(), }, }, - givenEjector: &PriorityBidderEjector{ - PriorityGroups: [][]string{ - {"priorityElement"}, - }, - OldestEjector: OldestEjector{}, + givenEjector: &OldestEjector{ + []string{}, + [][]string{}, + FallbackEjector{}, }, - expected: "nonPriorityElement", + expected: "oldestElement", }, } @@ -102,12 +144,22 @@ func TestChooseElementToEject(t *testing.T) { } } -// TODO: -func TestOldestEjector(t *testing.T) {} +func TestFallbackEjector(t *testing.T) { + fallbackEjector := FallbackEjector{} -func TestPriorityEjector(t *testing.T) {} + uids := map[string]UIDEntry{ + "element1": {}, + "element2": {}, + "element3": {}, + } -func TestFallbackEjector(t *testing.T) {} + randomUIDChosen, err := fallbackEjector.Choose(uids) + assert.NoError(t, err) + + if _, ok := uids[randomUIDChosen]; !ok { + t.Errorf("Returned element not in original list") + } +} func TestGetOldestElement(t *testing.T) { testCases := []struct { From 6c9b7d1d1eeda3eb1de67fcdf5bd3eae2dd1491e Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 1 Aug 2023 17:35:19 -0700 Subject: [PATCH 07/20] Ejector refactor, remove fallback, add tests --- pbs/usersync.go | 2 +- usersync/ejector.go | 95 +++++++++++++++----------------- usersync/ejector_test.go | 115 ++++++++++++++++++++++++++------------- 3 files changed, 122 insertions(+), 90 deletions(-) diff --git a/pbs/usersync.go b/pbs/usersync.go index 6ea27218e6b..bfaef4cf6d4 100644 --- a/pbs/usersync.go +++ b/pbs/usersync.go @@ -82,7 +82,7 @@ func (deps *UserSyncDeps) OptOut(w http.ResponseWriter, r *http.Request, _ httpr pc.SetOptOut(optout != "") // Write Cookie - encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder, &usersync.OldestEjector{PriorityGroups: deps.PriorityGroups}) + encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder, &usersync.OldestEjector{PriorityGroups: deps.PriorityGroups, EjectPriority: false}) if err != nil { w.WriteHeader(http.StatusBadRequest) return diff --git a/usersync/ejector.go b/usersync/ejector.go index 7122527d5cc..038b7105116 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -3,7 +3,6 @@ package usersync import ( "errors" "math" - "math/rand" "time" ) @@ -14,68 +13,62 @@ type Ejector interface { type OldestEjector struct { nonPriorityKeys []string PriorityGroups [][]string - FallbackEjector FallbackEjector + EjectPriority bool } type PriorityBidderEjector struct { - PriorityGroups [][]string - SyncerKey string - OldestEjector OldestEjector - FallbackEjector FallbackEjector + PriorityGroups [][]string + SyncerKey string + OldestEjector OldestEjector } -type FallbackEjector struct{} - -// Choose method for oldest ejector will return the oldest non priority uid +// Choose method for oldest ejector will return the oldest uid after determing which set of elements to be choosing from func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { - if len(o.nonPriorityKeys) == 0 { - o.nonPriorityKeys = getNonPriorityKeys(uids, o.PriorityGroups) + var elements []string + + // Set which elements the ejector will be choosing from + if len(o.nonPriorityKeys) > 0 { + elements = o.nonPriorityKeys + } else if o.EjectPriority { + elements = o.PriorityGroups[len(o.PriorityGroups)-1] + } else { + elements = getNonPriorityKeys(uids, o.PriorityGroups) } - oldestElement := getOldestElement(o.nonPriorityKeys, uids) - if oldestElement == "" { - return o.FallbackEjector.Choose(uids) - } + uidToDelete := getOldestElement(elements, uids) - return oldestElement, nil + return uidToDelete, nil } -// Choose method for priority ejector will return the oldest priority uid, otherwise it will call a different ejector method +// Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) if len(p.OldestEjector.nonPriorityKeys) > 0 { + p.OldestEjector.EjectPriority = false return p.OldestEjector.Choose(uids) } if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] - oldestElement := getOldestElement(lowestPriorityGroup, uids) - if oldestElement == "" { - return p.FallbackEjector.Choose(uids) + if len(lowestPriorityGroup) == 1 { + uidToDelete := lowestPriorityGroup[0] + p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) + return uidToDelete, nil + } else { + p.OldestEjector.EjectPriority = true + p.OldestEjector.PriorityGroups = p.PriorityGroups + uidToDelete, err := p.OldestEjector.Choose(uids) + if err != nil { + return "", err + } + p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) + return uidToDelete, nil } - - p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, oldestElement) - - return oldestElement, nil } return "", errors.New("syncer key " + p.SyncerKey + " is not in priority groups") } -// Choose method for Fallback Ejector, selects a random uid to eject when the other two ejectors can't come up with a uid -func (f *FallbackEjector) Choose(uids map[string]UIDEntry) (string, error) { - keys := make([]string, len(uids)) - - i := 0 - for key := range uids { - keys[i] = key - i++ - } - - rand.Seed(time.Now().UnixNano()) - return keys[rand.Intn(len(keys))], nil -} - // updatePriorityGroup will remove the selected element from the priority groups, and will remove the entire priority group if it's empty func removeElementFromPriorityGroup(priorityGroups [][]string, oldestElement string) [][]string { lowestPriorityGroup := priorityGroups[len(priorityGroups)-1] @@ -106,6 +99,7 @@ func isSyncerPriority(syncer string, priorityGroups [][]string) bool { } func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []string { + // If no priority groups, then all keys in uids are non-priority nonPriorityKeys := []string{} if len(priorityGroups) == 0 { for key := range uids { @@ -114,28 +108,29 @@ func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []s return nonPriorityKeys } - for key := range uids { - isPriority := false - for _, group := range priorityGroups { - for _, bidder := range group { - if key == bidder { - isPriority = true - break - } - } + // Create map of keys that are a priority + isPriority := make(map[string]bool) + for _, group := range priorityGroups { + for _, bidder := range group { + isPriority[bidder] = true } - if !isPriority { + } + + // Loop over uids and compare the keys in this map to the keys in the isPriority map to find the non piority keys + for key := range uids { + if !isPriority[key] { nonPriorityKeys = append(nonPriorityKeys, key) } } + return nonPriorityKeys } -func getOldestElement(list []string, uids map[string]UIDEntry) string { +func getOldestElement(elements []string, uids map[string]UIDEntry) string { var oldestElem string = "" var oldestDate int64 = math.MaxInt64 - for _, key := range list { + for _, key := range elements { if value, ok := uids[key]; ok { timeUntilExpiration := time.Until(value.Expires) if timeUntilExpiration < time.Duration(oldestDate) { diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index ed88af77773..db019054c2f 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -17,56 +17,87 @@ func TestPriorityEjector(t *testing.T) { expectedError error }{ { - name: "priority-ejector", + name: "one-lowest-priority-element", givenUids: map[string]UIDEntry{ - "newestLowestPriorityElement": { + "higherPriority": { UID: "123", Expires: time.Now().Add((90 * 24 * time.Hour)), }, - "oldestLowestPriorityElement": { + "lowestPriority": { UID: "456", Expires: time.Now(), }, }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"adnxs"}, - {"oldestLowestPriorityElement", "newestLowestPriorityElement"}, + {"adnxs", "higherPriority"}, + {"lowestPriority"}, }, SyncerKey: "adnxs", }, - expected: "oldestLowestPriorityElement", + expected: "lowestPriority", }, { - name: "priority-ejector-syncer-not-priority", - givenUids: map[string]UIDEntry{}, + name: "multiple-uids-same-priority", + givenUids: map[string]UIDEntry{ + "newerButSamePriority": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "olderButSamePriority": { + UID: "456", + Expires: time.Now(), + }, + }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"syncerKey1"}, + {"adnxs"}, + {"newerButSamePriority", "olderButSamePriority"}, }, - SyncerKey: "adnxs", + SyncerKey: "adnxs", + OldestEjector: OldestEjector{}, }, - expectedError: errors.New("syncer key adnxs is not in priority groups"), + expected: "olderButSamePriority", }, { - name: "priority-calls-oldest-ejector", + name: "non-priority-uids-present", givenUids: map[string]UIDEntry{ - "nonPriorityElement": { + "higherPriority": { UID: "123", Expires: time.Now().Add((90 * 24 * time.Hour)), }, - "priorityElement": { + "lowestPriority": { + UID: "456", + Expires: time.Now(), + }, + "oldestNonPriority": { UID: "456", Expires: time.Now(), }, + "newestNonPriority": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"priorityElement"}, + {"adnxs", "higherPriority"}, + {"lowestPriority"}, }, - OldestEjector: OldestEjector{}, + SyncerKey: "adnxs", }, - expected: "nonPriorityElement", + expected: "oldestNonPriority", + }, + { + name: "priority-ejector-syncer-not-priority", + givenUids: map[string]UIDEntry{}, + givenEjector: &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"syncerKey1"}, + }, + SyncerKey: "adnxs", + }, + expectedError: errors.New("syncer key adnxs is not in priority groups"), }, } @@ -92,7 +123,7 @@ func TestOldestEjector(t *testing.T) { expectedError error }{ { - name: "oldest-ejector", + name: "non-priority-keys-present", givenUids: map[string]UIDEntry{ "newestElement": { UID: "123", @@ -106,12 +137,12 @@ func TestOldestEjector(t *testing.T) { givenEjector: &OldestEjector{ []string{"newestElement", "oldestElement"}, [][]string{}, - FallbackEjector{}, + false, }, expected: "oldestElement", }, { - name: "non-priority-keys-empty", + name: "eject-priority-true", givenUids: map[string]UIDEntry{ "newestElement": { UID: "123", @@ -124,11 +155,34 @@ func TestOldestEjector(t *testing.T) { }, givenEjector: &OldestEjector{ []string{}, - [][]string{}, - FallbackEjector{}, + [][]string{{"newestElement", "oldestElement"}}, + true, }, expected: "oldestElement", }, + { + name: "non-priority-keys-initially-empty", + givenUids: map[string]UIDEntry{ + "priorityElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "newestNonPriority": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "oldestNonPriority": { + UID: "456", + Expires: time.Now(), + }, + }, + givenEjector: &OldestEjector{ + []string{}, + [][]string{{"newestElement"}}, + false, + }, + expected: "oldestNonPriority", + }, } for _, test := range testCases { @@ -144,23 +198,6 @@ func TestOldestEjector(t *testing.T) { } } -func TestFallbackEjector(t *testing.T) { - fallbackEjector := FallbackEjector{} - - uids := map[string]UIDEntry{ - "element1": {}, - "element2": {}, - "element3": {}, - } - - randomUIDChosen, err := fallbackEjector.Choose(uids) - assert.NoError(t, err) - - if _, ok := uids[randomUIDChosen]; !ok { - t.Errorf("Returned element not in original list") - } -} - func TestGetOldestElement(t *testing.T) { testCases := []struct { name string From 0112ea7a10d6e373cf49ae4c4dacb9a53d6f41d5 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 2 Aug 2023 17:16:32 -0700 Subject: [PATCH 08/20] Fix validate merge fail --- endpoints/cookie_sync_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index d34bbeecde0..e301d457989 100644 --- a/endpoints/cookie_sync_test.go +++ b/endpoints/cookie_sync_test.go @@ -983,9 +983,9 @@ func TestCookieSyncParseRequest(t *testing.T) { givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"}, givenCCPAEnabled: true, givenConfig: config.UserSync{ + PriorityGroups: [][]string{{"a", "b", "c"}}, Cooperative: config.UserSyncCooperative{ EnabledByDefault: false, - PriorityGroups: [][]string{{"a", "b", "c"}}, }, }, expectedPrivacy: privacy.Policies{}, From 53c05f5bacf0a0e6b9bfa58308b00d2bb2926f18 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 3 Aug 2023 08:19:28 -0700 Subject: [PATCH 09/20] Address comments --- usersync/ejector.go | 4 +--- usersync/ejector_test.go | 24 +----------------------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/usersync/ejector.go b/usersync/ejector.go index 038b7105116..23697a0447b 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -127,7 +127,7 @@ func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []s } func getOldestElement(elements []string, uids map[string]UIDEntry) string { - var oldestElem string = "" + var oldestElem string var oldestDate int64 = math.MaxInt64 for _, key := range elements { @@ -137,8 +137,6 @@ func getOldestElement(elements []string, uids map[string]UIDEntry) string { oldestElem = key oldestDate = int64(timeUntilExpiration) } - } else { - continue } } return oldestElem diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index db019054c2f..f9e25deb3d8 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -299,7 +299,7 @@ func TestGetNonPriorityKeys(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { keys := getNonPriorityKeys(test.givenUids, test.givenPriorityGroups) - assert.Equal(t, true, containsSameElements(test.expected, keys)) + assert.Equal(t, true, assert.ElementsMatch(t, test.expected, keys)) }) } } @@ -353,25 +353,3 @@ func TestIsSyncerPriority(t *testing.T) { }) } } - -func containsSameElements(x, y []string) bool { - if len(x) != len(y) { - return false - } - - countOfElements := make(map[string]int) - - for _, key := range x { - countOfElements[key]++ - } - for _, key := range y { - countOfElements[key]-- - } - - for _, count := range countOfElements { - if count != 0 { - return false - } - } - return true -} From 489d34a6f7676c48fde61006333410752b60a4dd Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 3 Aug 2023 10:55:55 -0700 Subject: [PATCH 10/20] Update IsSyncerPriority, update Pri Ejector --- endpoints/setuid.go | 19 +++++++- endpoints/setuid_test.go | 93 ++++++++++++++++++++++++++++++++++++++++ usersync/cookie_test.go | 1 + usersync/ejector.go | 39 +++++++---------- usersync/ejector_test.go | 64 ++++----------------------- 5 files changed, 136 insertions(+), 80 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 3e491b24f70..66d07edbc64 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -164,8 +164,12 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use setSiteCookie := siteCookieCheck(r.UserAgent()) + // Priority Ejector Set Up + priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, SyncerKey: syncer.Key()} + priorityEjector.IsSyncerPriority = isSyncerPriority(syncer.Key(), cfg.UserSync.PriorityGroups, syncersByBidder) + // Write Cookie - encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, SyncerKey: syncer.Key()}) + encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, priorityEjector) if err != nil { w.WriteHeader(http.StatusBadRequest) metricsEngine.RecordSetUid(metrics.SetUidBadRequest) @@ -335,6 +339,19 @@ func getSyncer(query url.Values, syncersByBidder map[string]usersync.Syncer) (us return syncer, bidder, nil } +func isSyncerPriority(syncer string, priorityGroups [][]string, syncersByBidder map[string]usersync.Syncer) bool { + for _, group := range priorityGroups { + for _, bidder := range group { + if bidderSyncer, ok := syncersByBidder[bidder]; ok { + if syncer == bidderSyncer.Key() { + return true + } + } + } + } + return false +} + // getResponseFormat reads the format query parameter or falls back to the syncer's default. // Returns either "b" (iframe), "i" (redirect), or an empty string "" (legacy behavior of an // empty response body with no content type). diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 05030cc7cf7..8675a76ece4 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -1322,6 +1322,99 @@ func TestGetResponseFormat(t *testing.T) { } } +func TestIsSyncerPriority(t *testing.T) { + testCases := []struct { + name string + givenSyncerKey string + givenPriorityGroups [][]string + givenSyncersByBidder map[string]usersync.Syncer + expected bool + }{ + { + name: "syncer-is-priority-one-to-one", + givenSyncerKey: "prioritySyncer", + givenPriorityGroups: [][]string{ + {"prioritySyncer"}, + {"2", "3"}, + }, + givenSyncersByBidder: map[string]usersync.Syncer{ + "prioritySyncer": fakeSyncer{ + key: "prioritySyncer", + }, + }, + expected: true, + }, + { + name: "syncer-is-priority-not-one-to-one", + givenSyncerKey: "adnxs", + givenPriorityGroups: [][]string{ + {"appnexus"}, + {"2", "3"}, + }, + givenSyncersByBidder: map[string]usersync.Syncer{ + "appnexus": fakeSyncer{ + key: "adnxs", + }, + }, + expected: true, + }, + { + name: "syncer-is-not-priority", + givenSyncerKey: "notPrioritySyncer", + givenPriorityGroups: [][]string{ + {"1"}, + {"2", "3"}, + }, + givenSyncersByBidder: map[string]usersync.Syncer{ + "1": fakeSyncer{ + key: "1", + }, + "2": fakeSyncer{ + key: "2", + }, + "3": fakeSyncer{ + key: "3", + }, + }, + expected: false, + }, + { + name: "no-syncer-given", + givenSyncerKey: "", + givenPriorityGroups: [][]string{ + {"1"}, + {"2", "3"}, + }, + givenSyncersByBidder: map[string]usersync.Syncer{ + "1": fakeSyncer{ + key: "1", + }, + "2": fakeSyncer{ + key: "2", + }, + "3": fakeSyncer{ + key: "3", + }, + }, + expected: false, + }, + { + name: "no-priority-groups-given", + givenSyncerKey: "adnxs", + givenPriorityGroups: [][]string{}, + givenSyncersByBidder: map[string]usersync.Syncer{}, + expected: false, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + isPriority := isSyncerPriority(test.givenSyncerKey, test.givenPriorityGroups, test.givenSyncersByBidder) + assert.Equal(t, test.expected, isPriority) + }) + } +} + func assertHasSyncs(t *testing.T, testCase string, resp *httptest.ResponseRecorder, syncs map[string]string) { t.Helper() cookie := parseCookieString(t, resp) diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 8e779346298..563f60d9a8e 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -404,6 +404,7 @@ func TestPrepareCookieForWrite(t *testing.T) { OldestEjector: OldestEjector{ nonPriorityKeys: []string{}, }, + IsSyncerPriority: true, } testCases := []struct { diff --git a/usersync/ejector.go b/usersync/ejector.go index 23697a0447b..e80e33d42c8 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -17,9 +17,10 @@ type OldestEjector struct { } type PriorityBidderEjector struct { - PriorityGroups [][]string - SyncerKey string - OldestEjector OldestEjector + PriorityGroups [][]string + SyncerKey string + OldestEjector OldestEjector + IsSyncerPriority bool } // Choose method for oldest ejector will return the oldest uid after determing which set of elements to be choosing from @@ -43,28 +44,29 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { // Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) + if len(p.OldestEjector.nonPriorityKeys) > 0 { p.OldestEjector.EjectPriority = false return p.OldestEjector.Choose(uids) } - if isSyncerPriority(p.SyncerKey, p.PriorityGroups) { + if p.IsSyncerPriority { lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] if len(lowestPriorityGroup) == 1 { uidToDelete := lowestPriorityGroup[0] p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) return uidToDelete, nil - } else { - p.OldestEjector.EjectPriority = true - p.OldestEjector.PriorityGroups = p.PriorityGroups - uidToDelete, err := p.OldestEjector.Choose(uids) - if err != nil { - return "", err - } - p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) - return uidToDelete, nil } + + p.OldestEjector.EjectPriority = true + p.OldestEjector.PriorityGroups = p.PriorityGroups + uidToDelete, err := p.OldestEjector.Choose(uids) + if err != nil { + return "", err + } + p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) + return uidToDelete, nil } return "", errors.New("syncer key " + p.SyncerKey + " is not in priority groups") } @@ -87,17 +89,6 @@ func removeElementFromPriorityGroup(priorityGroups [][]string, oldestElement str return priorityGroups } -func isSyncerPriority(syncer string, priorityGroups [][]string) bool { - for _, group := range priorityGroups { - for _, bidder := range group { - if syncer == bidder { - return true - } - } - } - return false -} - func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []string { // If no priority groups, then all keys in uids are non-priority nonPriorityKeys := []string{} diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index f9e25deb3d8..6020ba01210 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -33,7 +33,8 @@ func TestPriorityEjector(t *testing.T) { {"adnxs", "higherPriority"}, {"lowestPriority"}, }, - SyncerKey: "adnxs", + SyncerKey: "adnxs", + IsSyncerPriority: true, }, expected: "lowestPriority", }, @@ -54,8 +55,9 @@ func TestPriorityEjector(t *testing.T) { {"adnxs"}, {"newerButSamePriority", "olderButSamePriority"}, }, - SyncerKey: "adnxs", - OldestEjector: OldestEjector{}, + SyncerKey: "adnxs", + IsSyncerPriority: true, + OldestEjector: OldestEjector{}, }, expected: "olderButSamePriority", }, @@ -84,7 +86,8 @@ func TestPriorityEjector(t *testing.T) { {"adnxs", "higherPriority"}, {"lowestPriority"}, }, - SyncerKey: "adnxs", + SyncerKey: "adnxs", + IsSyncerPriority: true, }, expected: "oldestNonPriority", }, @@ -95,7 +98,8 @@ func TestPriorityEjector(t *testing.T) { PriorityGroups: [][]string{ {"syncerKey1"}, }, - SyncerKey: "adnxs", + SyncerKey: "adnxs", + IsSyncerPriority: false, }, expectedError: errors.New("syncer key adnxs is not in priority groups"), }, @@ -303,53 +307,3 @@ func TestGetNonPriorityKeys(t *testing.T) { }) } } - -func TestIsSyncerPriority(t *testing.T) { - testCases := []struct { - name string - givenSyncerKey string - givenPriorityGroups [][]string - expected bool - }{ - { - name: "syncer-is-priority", - givenSyncerKey: "adnxs", - givenPriorityGroups: [][]string{ - {"adnxs"}, - {"2", "3"}, - }, - expected: true, - }, - { - name: "syncer-is-not-priority", - givenSyncerKey: "adnxs", - givenPriorityGroups: [][]string{ - {"1"}, - {"2", "3"}, - }, - expected: false, - }, - { - name: "no-syncer-given", - givenSyncerKey: "", - givenPriorityGroups: [][]string{ - {"1"}, - {"2", "3"}, - }, - expected: false, - }, - { - name: "no-priority-groups-given", - givenSyncerKey: "adnxs", - givenPriorityGroups: [][]string{}, - expected: false, - }, - } - - for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { - isPriority := isSyncerPriority(test.givenSyncerKey, test.givenPriorityGroups) - assert.Equal(t, test.expected, isPriority) - }) - } -} From b22b404dc15e63074b7cc45c39ef4f3a01f30032 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Fri, 11 Aug 2023 12:32:21 -0700 Subject: [PATCH 11/20] Update ejector logic --- endpoints/setuid.go | 11 +- endpoints/setuid_test.go | 17 +++ pbs/usersync.go | 2 +- usersync/cookie_test.go | 26 +++- usersync/ejector.go | 116 ++++++-------- usersync/ejector_test.go | 317 +++++++++++++++++++++++++-------------- 6 files changed, 303 insertions(+), 186 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 66d07edbc64..38e742f3040 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -165,8 +165,15 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use setSiteCookie := siteCookieCheck(r.UserAgent()) // Priority Ejector Set Up - priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, SyncerKey: syncer.Key()} - priorityEjector.IsSyncerPriority = isSyncerPriority(syncer.Key(), cfg.UserSync.PriorityGroups, syncersByBidder) + if !isSyncerPriority(syncer.Key(), cfg.UserSync.PriorityGroups, syncersByBidder) { + err = errors.New("syncer key " + syncer.Key() + " is not in priority groups") + w.WriteHeader(http.StatusBadRequest) + metricsEngine.RecordSetUid(metrics.SetUidBadRequest) + so.Errors = []error{err} + so.Status = http.StatusBadRequest + return + } + priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups} // Write Cookie encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, priorityEjector) diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 8675a76ece4..08cf4b618d1 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -306,6 +306,15 @@ func TestSetUIDEndpoint(t *testing.T) { expectedHeaders: map[string]string{"Content-Type": "text/html", "Content-Length": "0"}, description: "Set uid for valid bidder with valid account provided with invalid user sync activity", }, + { + uri: "/setuid?bidder=nonPriorityBidder&uid=123", + syncersBidderNameToKey: map[string]string{"nonPriorityBidder": "nonPrioritySyncer"}, + existingSyncs: nil, + gdprAllowsHostCookies: true, + expectedSyncs: nil, + expectedStatusCode: http.StatusBadRequest, + description: "Syncer is not a priority", + }, } analytics := analyticsConf.NewPBSAnalytics(&config.Analytics{}) @@ -1447,6 +1456,11 @@ func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metric "blocked_acct": true, }, AccountDefaults: config.Account{}, + UserSync: config.UserSync{ + PriorityGroups: [][]string{ + {}, + }, + }, } cfg.MarshalAccountDefaults() @@ -1469,6 +1483,9 @@ func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metric syncersByBidder := make(map[string]usersync.Syncer) for bidderName, syncerKey := range syncersBidderNameToKey { syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame} + if bidderName != "nonPriorityBidder" { + cfg.UserSync.PriorityGroups[0] = append(cfg.UserSync.PriorityGroups[0], bidderName) + } } fakeAccountsFetcher := FakeAccountsFetcher{AccountData: map[string]json.RawMessage{ diff --git a/pbs/usersync.go b/pbs/usersync.go index bfaef4cf6d4..a5b49f6db03 100644 --- a/pbs/usersync.go +++ b/pbs/usersync.go @@ -82,7 +82,7 @@ func (deps *UserSyncDeps) OptOut(w http.ResponseWriter, r *http.Request, _ httpr pc.SetOptOut(optout != "") // Write Cookie - encodedCookie, err := pc.PrepareCookieForWrite(deps.HostCookieConfig, encoder, &usersync.OldestEjector{PriorityGroups: deps.PriorityGroups, EjectPriority: false}) + encodedCookie, err := encoder.Encode(pc) if err != nil { w.WriteHeader(http.StatusBadRequest) return diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 563f60d9a8e..ab0fc4aa524 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -400,11 +400,29 @@ func TestPrepareCookieForWrite(t *testing.T) { {"4", "5", "6"}, {"7"}, }, - SyncerKey: "1", - OldestEjector: OldestEjector{ - nonPriorityKeys: []string{}, + syncersByBidder: map[string]Syncer{ + "1": fakeSyncer{ + key: "1", + }, + "2": fakeSyncer{ + key: "2", + }, + "3": fakeSyncer{ + key: "3", + }, + "4": fakeSyncer{ + key: "4", + }, + "5": fakeSyncer{ + key: "5", + }, + "6": fakeSyncer{ + key: "6", + }, + "mistmatchedBidder": fakeSyncer{ + key: "7", + }, }, - IsSyncerPriority: true, } testCases := []struct { diff --git a/usersync/ejector.go b/usersync/ejector.go index e80e33d42c8..ad4fc200c53 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -1,7 +1,6 @@ package usersync import ( - "errors" "math" "time" ) @@ -10,65 +9,51 @@ type Ejector interface { Choose(uids map[string]UIDEntry) (string, error) } -type OldestEjector struct { - nonPriorityKeys []string - PriorityGroups [][]string - EjectPriority bool -} +type OldestEjector struct{} type PriorityBidderEjector struct { - PriorityGroups [][]string - SyncerKey string - OldestEjector OldestEjector - IsSyncerPriority bool + PriorityGroups [][]string + syncersByBidder map[string]Syncer + OldestEjector OldestEjector } -// Choose method for oldest ejector will return the oldest uid after determing which set of elements to be choosing from +// Choose method for oldest ejector will return the oldest uid func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { - var elements []string - - // Set which elements the ejector will be choosing from - if len(o.nonPriorityKeys) > 0 { - elements = o.nonPriorityKeys - } else if o.EjectPriority { - elements = o.PriorityGroups[len(o.PriorityGroups)-1] - } else { - elements = getNonPriorityKeys(uids, o.PriorityGroups) + var oldestElement string + var oldestDate time.Duration = math.MaxInt64 + + for key, value := range uids { + timeUntilExpiration := time.Until(value.Expires) + if timeUntilExpiration < time.Duration(oldestDate) { + oldestElement = key + oldestDate = timeUntilExpiration + } } - - uidToDelete := getOldestElement(elements, uids) - - return uidToDelete, nil + return oldestElement, nil } // Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { - p.OldestEjector.nonPriorityKeys = getNonPriorityKeys(uids, p.PriorityGroups) - - if len(p.OldestEjector.nonPriorityKeys) > 0 { - p.OldestEjector.EjectPriority = false - return p.OldestEjector.Choose(uids) + nonPriortyUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) + if len(nonPriortyUids) > 0 { + return p.OldestEjector.Choose(nonPriortyUids) } - if p.IsSyncerPriority { - lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] + lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] - if len(lowestPriorityGroup) == 1 { - uidToDelete := lowestPriorityGroup[0] - p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) - return uidToDelete, nil - } - - p.OldestEjector.EjectPriority = true - p.OldestEjector.PriorityGroups = p.PriorityGroups - uidToDelete, err := p.OldestEjector.Choose(uids) - if err != nil { - return "", err - } + if len(lowestPriorityGroup) == 1 { + uidToDelete := lowestPriorityGroup[0] p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) return uidToDelete, nil } - return "", errors.New("syncer key " + p.SyncerKey + " is not in priority groups") + + lowestPriorityUids := getPriorityUids(lowestPriorityGroup, uids, p.syncersByBidder) + uidToDelete, err := p.OldestEjector.Choose(lowestPriorityUids) + if err != nil { + return "", err + } + p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) + return uidToDelete, nil } // updatePriorityGroup will remove the selected element from the priority groups, and will remove the entire priority group if it's empty @@ -89,46 +74,45 @@ func removeElementFromPriorityGroup(priorityGroups [][]string, oldestElement str return priorityGroups } -func getNonPriorityKeys(uids map[string]UIDEntry, priorityGroups [][]string) []string { +func getNonPriorityUids(uids map[string]UIDEntry, priorityGroups [][]string, syncersByBidder map[string]Syncer) map[string]UIDEntry { // If no priority groups, then all keys in uids are non-priority - nonPriorityKeys := []string{} if len(priorityGroups) == 0 { - for key := range uids { - nonPriorityKeys = append(nonPriorityKeys, key) - } - return nonPriorityKeys + return uids } // Create map of keys that are a priority isPriority := make(map[string]bool) for _, group := range priorityGroups { for _, bidder := range group { - isPriority[bidder] = true + if bidderSyncer, ok := syncersByBidder[bidder]; ok { + isPriority[bidderSyncer.Key()] = true + } } } - // Loop over uids and compare the keys in this map to the keys in the isPriority map to find the non piority keys - for key := range uids { - if !isPriority[key] { - nonPriorityKeys = append(nonPriorityKeys, key) + // Create a map for non-priority uids + nonPriorityUIDs := make(map[string]UIDEntry) + + // Loop over uids and populate the nonPriorityUIDs map with non-priority keys + for key, value := range uids { + if _, found := isPriority[key]; !found { + nonPriorityUIDs[key] = value } } - return nonPriorityKeys + return nonPriorityUIDs } -func getOldestElement(elements []string, uids map[string]UIDEntry) string { - var oldestElem string - var oldestDate int64 = math.MaxInt64 +func getPriorityUids(lowestPriorityGroup []string, uids map[string]UIDEntry, syncersByBidder map[string]Syncer) map[string]UIDEntry { + lowestPriorityUIDs := make(map[string]UIDEntry) - for _, key := range elements { - if value, ok := uids[key]; ok { - timeUntilExpiration := time.Until(value.Expires) - if timeUntilExpiration < time.Duration(oldestDate) { - oldestElem = key - oldestDate = int64(timeUntilExpiration) + // Loop over lowestPriorityGroup and populate the lowestPriorityUIDs map + for _, bidder := range lowestPriorityGroup { + if bidderSyncer, ok := syncersByBidder[bidder]; ok { + if uidEntry, exists := uids[bidderSyncer.Key()]; exists { + lowestPriorityUIDs[bidderSyncer.Key()] = uidEntry } } } - return oldestElem + return lowestPriorityUIDs } diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index 6020ba01210..1214dea05c3 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -1,7 +1,6 @@ package usersync import ( - "errors" "testing" "time" @@ -19,7 +18,7 @@ func TestPriorityEjector(t *testing.T) { { name: "one-lowest-priority-element", givenUids: map[string]UIDEntry{ - "higherPriority": { + "highestPrioritySyncer": { UID: "123", Expires: time.Now().Add((90 * 24 * time.Hour)), }, @@ -30,11 +29,17 @@ func TestPriorityEjector(t *testing.T) { }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"adnxs", "higherPriority"}, + {"highestPriorityBidder"}, {"lowestPriority"}, }, - SyncerKey: "adnxs", - IsSyncerPriority: true, + syncersByBidder: map[string]Syncer{ + "highestPriorityBidder": fakeSyncer{ + key: "highestPrioritySyncer", + }, + "lowestPriority": fakeSyncer{ + key: "lowestPriority", + }, + }, }, expected: "lowestPriority", }, @@ -52,12 +57,17 @@ func TestPriorityEjector(t *testing.T) { }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"adnxs"}, {"newerButSamePriority", "olderButSamePriority"}, }, - SyncerKey: "adnxs", - IsSyncerPriority: true, - OldestEjector: OldestEjector{}, + syncersByBidder: map[string]Syncer{ + "newerButSamePriority": fakeSyncer{ + key: "newerButSamePriority", + }, + "olderButSamePriority": fakeSyncer{ + key: "olderButSamePriority", + }, + }, + OldestEjector: OldestEjector{}, }, expected: "olderButSamePriority", }, @@ -83,25 +93,45 @@ func TestPriorityEjector(t *testing.T) { }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"adnxs", "higherPriority"}, + {"higherPriority"}, {"lowestPriority"}, }, - SyncerKey: "adnxs", - IsSyncerPriority: true, + syncersByBidder: map[string]Syncer{ + "higherPriority": fakeSyncer{ + key: "higherPriority", + }, + "lowestPriority": fakeSyncer{ + key: "lowestPriority", + }, + "oldestNonPriority": fakeSyncer{ + key: "oldestNonPriority", + }, + "newestNonPriority": fakeSyncer{ + key: "newestNonPriority", + }, + }, }, expected: "oldestNonPriority", }, { - name: "priority-ejector-syncer-not-priority", - givenUids: map[string]UIDEntry{}, + name: "one-priority-element", + givenUids: map[string]UIDEntry{ + "onlyPriorityElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + }, givenEjector: &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"syncerKey1"}, + {"onlyPriorityElement"}, + }, + syncersByBidder: map[string]Syncer{ + "onlyPriorityElement": fakeSyncer{ + key: "onlyPriorityElement", + }, }, - SyncerKey: "adnxs", - IsSyncerPriority: false, }, - expectedError: errors.New("syncer key adnxs is not in priority groups"), + expected: "onlyPriorityElement", }, } @@ -120,14 +150,12 @@ func TestPriorityEjector(t *testing.T) { func TestOldestEjector(t *testing.T) { testCases := []struct { - name string - givenUids map[string]UIDEntry - givenEjector Ejector - expected string - expectedError error + name string + givenUids map[string]UIDEntry + expected string }{ { - name: "non-priority-keys-present", + name: "multiple-elements", givenUids: map[string]UIDEntry{ "newestElement": { UID: "123", @@ -138,126 +166,158 @@ func TestOldestEjector(t *testing.T) { Expires: time.Now(), }, }, - givenEjector: &OldestEjector{ - []string{"newestElement", "oldestElement"}, - [][]string{}, - false, - }, expected: "oldestElement", }, { - name: "eject-priority-true", + name: "one-element", givenUids: map[string]UIDEntry{ - "newestElement": { + "onlyElement": { UID: "123", Expires: time.Now().Add((90 * 24 * time.Hour)), }, - "oldestElement": { - UID: "456", - Expires: time.Now(), - }, - }, - givenEjector: &OldestEjector{ - []string{}, - [][]string{{"newestElement", "oldestElement"}}, - true, }, - expected: "oldestElement", + expected: "onlyElement", }, { - name: "non-priority-keys-initially-empty", - givenUids: map[string]UIDEntry{ - "priorityElement": { - UID: "123", - Expires: time.Now().Add((90 * 24 * time.Hour)), - }, - "newestNonPriority": { - UID: "123", - Expires: time.Now().Add((90 * 24 * time.Hour)), - }, - "oldestNonPriority": { - UID: "456", - Expires: time.Now(), - }, - }, - givenEjector: &OldestEjector{ - []string{}, - [][]string{{"newestElement"}}, - false, - }, - expected: "oldestNonPriority", + name: "no-elements", + givenUids: map[string]UIDEntry{}, + expected: "", }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - uidToDelete, err := test.givenEjector.Choose(test.givenUids) - if test.expectedError != nil { - assert.Equal(t, test.expectedError, err) - } else { - assert.NoError(t, err) - assert.Equal(t, test.expected, uidToDelete) - } + ejector := OldestEjector{} + oldestElement, err := ejector.Choose(test.givenUids) + assert.NoError(t, err) + assert.Equal(t, test.expected, oldestElement) }) } } -func TestGetOldestElement(t *testing.T) { +func TestGetNonPriorityUids(t *testing.T) { + syncersByBidder := map[string]Syncer{ + "syncerKey1": fakeSyncer{ + key: "syncerKey1", + }, + "syncerKey2": fakeSyncer{ + key: "syncerKey2", + }, + "syncerKey3": fakeSyncer{ + key: "syncerKey3", + }, + } + testCases := []struct { - name string - givenUids map[string]UIDEntry - givenFilteredKeys []string - expected string + name string + givenUids map[string]UIDEntry + givenPriorityGroups [][]string + expected map[string]UIDEntry }{ { - name: "basic-oldest-element", + name: "one-priority-group", givenUids: map[string]UIDEntry{ - "newestElement": { - UID: "123", - Expires: time.Now().Add((90 * 24 * time.Hour)), + "syncerKey1": { + UID: "123", }, - "oldestElement": { - UID: "456", - Expires: time.Now(), + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", + }, + }, + givenPriorityGroups: [][]string{ + {"syncerKey1"}, + }, + expected: map[string]UIDEntry{ + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", }, }, - givenFilteredKeys: []string{"newestElement", "oldestElement"}, - expected: "oldestElement", }, { - name: "no-filtered-keys", + name: "multiple-priority-groups", givenUids: map[string]UIDEntry{ - "newestElement": { - UID: "123", - Expires: time.Now().Add((90 * 24 * time.Hour)), + "syncerKey1": { + UID: "123", }, - "oldestElement": { - UID: "456", - Expires: time.Now(), + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", + }, + }, + givenPriorityGroups: [][]string{ + {"syncerKey1"}, + {"syncerKey2"}, + }, + expected: map[string]UIDEntry{ + "syncerKey3": { + UID: "789", + }, + }, + }, + { + name: "no-priority-groups", + givenUids: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", + }, + }, + expected: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", }, }, - givenFilteredKeys: []string{}, - expected: "", }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - oldestElement := getOldestElement(test.givenFilteredKeys, test.givenUids) - assert.Equal(t, test.expected, oldestElement) + uids := getNonPriorityUids(test.givenUids, test.givenPriorityGroups, syncersByBidder) + assert.Equal(t, true, mapsEqual(test.expected, uids)) }) } } -func TestGetNonPriorityKeys(t *testing.T) { +func TestGetPriorityUids(t *testing.T) { + syncersByBidder := map[string]Syncer{ + "syncerKey1": fakeSyncer{ + key: "syncerKey1", + }, + "syncerKey2": fakeSyncer{ + key: "syncerKey2", + }, + "syncerKey3": fakeSyncer{ + key: "syncerKey3", + }, + } + testCases := []struct { - name string - givenUids map[string]UIDEntry - givenPriorityGroups [][]string - expected []string + name string + givenUids map[string]UIDEntry + givenLowestPriorityGroup []string + expected map[string]UIDEntry }{ { - name: "one-priority-group", + name: "one-priority-element", givenUids: map[string]UIDEntry{ "syncerKey1": { UID: "123", @@ -269,13 +329,15 @@ func TestGetNonPriorityKeys(t *testing.T) { UID: "789", }, }, - givenPriorityGroups: [][]string{ - {"syncerKey1"}, + givenLowestPriorityGroup: []string{"syncerKey1"}, + expected: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, }, - expected: []string{"syncerKey2", "syncerKey3"}, }, { - name: "no-priority-groups", + name: "multiple-priority-elements", givenUids: map[string]UIDEntry{ "syncerKey1": { UID: "123", @@ -287,23 +349,52 @@ func TestGetNonPriorityKeys(t *testing.T) { UID: "789", }, }, - givenPriorityGroups: [][]string{}, - expected: []string{"syncerKey1", "syncerKey2", "syncerKey3"}, + givenLowestPriorityGroup: []string{"syncerKey1", "syncerKey2"}, + expected: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, + "syncerKey2": { + UID: "456", + }, + }, }, { - name: "no-given-uids", - givenUids: map[string]UIDEntry{}, - givenPriorityGroups: [][]string{ - {"syncerKey1"}, + name: "no-priority-elements", + givenUids: map[string]UIDEntry{ + "syncerKey1": { + UID: "123", + }, + "syncerKey2": { + UID: "456", + }, + "syncerKey3": { + UID: "789", + }, }, - expected: []string{}, + givenLowestPriorityGroup: []string{}, + expected: map[string]UIDEntry{}, }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - keys := getNonPriorityKeys(test.givenUids, test.givenPriorityGroups) - assert.Equal(t, true, assert.ElementsMatch(t, test.expected, keys)) + uids := getPriorityUids(test.givenLowestPriorityGroup, test.givenUids, syncersByBidder) + assert.Equal(t, true, mapsEqual(test.expected, uids)) }) } } + +func mapsEqual(map1, map2 map[string]UIDEntry) bool { + if len(map1) != len(map2) { + return false + } + + for key, value1 := range map1 { + if value2, exists := map2[key]; !exists || value1 != value2 { + return false + } + } + + return true +} From 8060ba456ce808949a1e78f1485153e1ef015845 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 15 Aug 2023 10:42:38 -0700 Subject: [PATCH 12/20] Implement TieEjector logic --- endpoints/setuid.go | 2 +- usersync/cookie_test.go | 1 + usersync/ejector.go | 6 +++--- usersync/ejector_test.go | 3 ++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 38e742f3040..5d40607445f 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -173,7 +173,7 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use so.Status = http.StatusBadRequest return } - priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups} + priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, TieEjector: &usersync.OldestEjector{}} // Write Cookie encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, priorityEjector) diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index ab0fc4aa524..2f9ddfa246c 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -423,6 +423,7 @@ func TestPrepareCookieForWrite(t *testing.T) { key: "7", }, }, + TieEjector: &OldestEjector{}, } testCases := []struct { diff --git a/usersync/ejector.go b/usersync/ejector.go index ad4fc200c53..c591cc4c162 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -14,7 +14,7 @@ type OldestEjector struct{} type PriorityBidderEjector struct { PriorityGroups [][]string syncersByBidder map[string]Syncer - OldestEjector OldestEjector + TieEjector Ejector } // Choose method for oldest ejector will return the oldest uid @@ -36,7 +36,7 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { nonPriortyUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) if len(nonPriortyUids) > 0 { - return p.OldestEjector.Choose(nonPriortyUids) + return p.TieEjector.Choose(nonPriortyUids) } lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] @@ -48,7 +48,7 @@ func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) } lowestPriorityUids := getPriorityUids(lowestPriorityGroup, uids, p.syncersByBidder) - uidToDelete, err := p.OldestEjector.Choose(lowestPriorityUids) + uidToDelete, err := p.TieEjector.Choose(lowestPriorityUids) if err != nil { return "", err } diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index 1214dea05c3..8adb8312091 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -67,7 +67,7 @@ func TestPriorityEjector(t *testing.T) { key: "olderButSamePriority", }, }, - OldestEjector: OldestEjector{}, + TieEjector: &OldestEjector{}, }, expected: "olderButSamePriority", }, @@ -110,6 +110,7 @@ func TestPriorityEjector(t *testing.T) { key: "newestNonPriority", }, }, + TieEjector: &OldestEjector{}, }, expected: "oldestNonPriority", }, From 4891b4e0a1a9dc9e9ca034f1132d8b52914d9a32 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 15 Aug 2023 10:58:04 -0700 Subject: [PATCH 13/20] Minor tweaks --- usersync/ejector.go | 8 +++----- usersync/ejector_test.go | 19 +++---------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/usersync/ejector.go b/usersync/ejector.go index c591cc4c162..bce60892a69 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -1,7 +1,6 @@ package usersync import ( - "math" "time" ) @@ -20,13 +19,12 @@ type PriorityBidderEjector struct { // Choose method for oldest ejector will return the oldest uid func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { var oldestElement string - var oldestDate time.Duration = math.MaxInt64 + var oldestDate time.Time = time.Unix(1<<63-62135596801, 999999999) // Max value for time for key, value := range uids { - timeUntilExpiration := time.Until(value.Expires) - if timeUntilExpiration < time.Duration(oldestDate) { + if value.Expires.Before(oldestDate) { oldestElement = key - oldestDate = timeUntilExpiration + oldestDate = value.Expires } } return oldestElement, nil diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index 8adb8312091..c95b004a06d 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -1,6 +1,7 @@ package usersync import ( + "reflect" "testing" "time" @@ -293,7 +294,7 @@ func TestGetNonPriorityUids(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { uids := getNonPriorityUids(test.givenUids, test.givenPriorityGroups, syncersByBidder) - assert.Equal(t, true, mapsEqual(test.expected, uids)) + assert.Equal(t, true, reflect.DeepEqual(test.expected, uids)) }) } } @@ -381,21 +382,7 @@ func TestGetPriorityUids(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { uids := getPriorityUids(test.givenLowestPriorityGroup, test.givenUids, syncersByBidder) - assert.Equal(t, true, mapsEqual(test.expected, uids)) + assert.Equal(t, true, reflect.DeepEqual(test.expected, uids)) }) } } - -func mapsEqual(map1, map2 map[string]UIDEntry) bool { - if len(map1) != len(map2) { - return false - } - - for key, value1 := range map1 { - if value2, exists := map2[key]; !exists || value1 != value2 { - return false - } - } - - return true -} From bc8c72abe2e109dfe4939dc413a70e6060424bbe Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 17 Aug 2023 14:22:16 -0700 Subject: [PATCH 14/20] Update IsSyncerPriority logic --- endpoints/setuid.go | 17 +++------ endpoints/setuid_test.go | 77 ++++++++++------------------------------ usersync/cookie_test.go | 76 +++++++++++++++++++++++++++++---------- usersync/ejector.go | 12 +++++-- usersync/ejector_test.go | 34 ++++++++++++++++-- 5 files changed, 120 insertions(+), 96 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 5d40607445f..e2f277a0eb4 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -165,15 +165,8 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use setSiteCookie := siteCookieCheck(r.UserAgent()) // Priority Ejector Set Up - if !isSyncerPriority(syncer.Key(), cfg.UserSync.PriorityGroups, syncersByBidder) { - err = errors.New("syncer key " + syncer.Key() + " is not in priority groups") - w.WriteHeader(http.StatusBadRequest) - metricsEngine.RecordSetUid(metrics.SetUidBadRequest) - so.Errors = []error{err} - so.Status = http.StatusBadRequest - return - } priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, TieEjector: &usersync.OldestEjector{}} + priorityEjector.IsSyncerPriority = isSyncerPriority(bidderName, cfg.UserSync.PriorityGroups) // Write Cookie encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, priorityEjector) @@ -346,13 +339,11 @@ func getSyncer(query url.Values, syncersByBidder map[string]usersync.Syncer) (us return syncer, bidder, nil } -func isSyncerPriority(syncer string, priorityGroups [][]string, syncersByBidder map[string]usersync.Syncer) bool { +func isSyncerPriority(bidderNameFromSyncerQuery string, priorityGroups [][]string) bool { for _, group := range priorityGroups { for _, bidder := range group { - if bidderSyncer, ok := syncersByBidder[bidder]; ok { - if syncer == bidderSyncer.Key() { - return true - } + if bidderNameFromSyncerQuery == bidder { + return true } } } diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 08cf4b618d1..aa6f7eb20b5 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -1333,92 +1333,49 @@ func TestGetResponseFormat(t *testing.T) { func TestIsSyncerPriority(t *testing.T) { testCases := []struct { - name string - givenSyncerKey string - givenPriorityGroups [][]string - givenSyncersByBidder map[string]usersync.Syncer - expected bool + name string + givenBidderNameFromSyncerQuery string + givenPriorityGroups [][]string + expected bool }{ { - name: "syncer-is-priority-one-to-one", - givenSyncerKey: "prioritySyncer", + name: "bidder-name-is-priority", + givenBidderNameFromSyncerQuery: "priorityBidder", givenPriorityGroups: [][]string{ - {"prioritySyncer"}, + {"priorityBidder"}, {"2", "3"}, }, - givenSyncersByBidder: map[string]usersync.Syncer{ - "prioritySyncer": fakeSyncer{ - key: "prioritySyncer", - }, - }, - expected: true, - }, - { - name: "syncer-is-priority-not-one-to-one", - givenSyncerKey: "adnxs", - givenPriorityGroups: [][]string{ - {"appnexus"}, - {"2", "3"}, - }, - givenSyncersByBidder: map[string]usersync.Syncer{ - "appnexus": fakeSyncer{ - key: "adnxs", - }, - }, expected: true, }, { - name: "syncer-is-not-priority", - givenSyncerKey: "notPrioritySyncer", + name: "bidder-name-is-not-priority", + givenBidderNameFromSyncerQuery: "notPriorityBidderName", givenPriorityGroups: [][]string{ {"1"}, {"2", "3"}, }, - givenSyncersByBidder: map[string]usersync.Syncer{ - "1": fakeSyncer{ - key: "1", - }, - "2": fakeSyncer{ - key: "2", - }, - "3": fakeSyncer{ - key: "3", - }, - }, expected: false, }, { - name: "no-syncer-given", - givenSyncerKey: "", + name: "no-bidder-name-given", + givenBidderNameFromSyncerQuery: "", givenPriorityGroups: [][]string{ {"1"}, {"2", "3"}, }, - givenSyncersByBidder: map[string]usersync.Syncer{ - "1": fakeSyncer{ - key: "1", - }, - "2": fakeSyncer{ - key: "2", - }, - "3": fakeSyncer{ - key: "3", - }, - }, expected: false, }, { - name: "no-priority-groups-given", - givenSyncerKey: "adnxs", - givenPriorityGroups: [][]string{}, - givenSyncersByBidder: map[string]usersync.Syncer{}, - expected: false, + name: "no-priority-groups-given", + givenBidderNameFromSyncerQuery: "bidderName", + givenPriorityGroups: [][]string{}, + expected: false, }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - isPriority := isSyncerPriority(test.givenSyncerKey, test.givenPriorityGroups, test.givenSyncersByBidder) + isPriority := isSyncerPriority(test.givenBidderNameFromSyncerQuery, test.givenPriorityGroups) assert.Equal(t, test.expected, isPriority) }) } @@ -1485,6 +1442,8 @@ func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metric syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame} if bidderName != "nonPriorityBidder" { cfg.UserSync.PriorityGroups[0] = append(cfg.UserSync.PriorityGroups[0], bidderName) + } else { + cfg.HostCookie.MaxCookieSizeBytes = 100 } } diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 2f9ddfa246c..f49eac8a6a8 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -380,15 +380,23 @@ func TestWriteCookieUserAgent(t *testing.T) { func TestPrepareCookieForWrite(t *testing.T) { encoder := Base64Encoder{} decoder := Base64Decoder{} - cookieToSend := &Cookie{ + + mainCookie := &Cookie{ uids: map[string]UIDEntry{ "1": newTempId("1234567890123456789012345678901234567890123456", 7), - "7": newTempId("abcdefghijklmnopqrstuvwxy", 1), "2": newTempId("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 6), "3": newTempId("123456789012345678901234567896123456789012345678", 5), "4": newTempId("aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ", 4), "5": newTempId("12345678901234567890123456789012345678901234567890", 3), "6": newTempId("abcdefghij", 2), + "7": newTempId("abcdefghijklmnopqrstuvwxy", 1), + }, + optOut: false, + } + + errorCookie := &Cookie{ + uids: map[string]UIDEntry{ + "syncerNotPriority": newTempId("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 7), }, optOut: false, } @@ -429,39 +437,52 @@ func TestPrepareCookieForWrite(t *testing.T) { testCases := []struct { name string givenMaxCookieSize int + givenCookieToSend *Cookie + givenIsSyncerPriority bool expectedRemainingUidKeys []string + expectedError error }{ { - name: "no-uids-ejected", - givenMaxCookieSize: 2000, + name: "no-uids-ejected", + givenMaxCookieSize: 2000, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ "1", "2", "3", "4", "5", "6", "7", }, }, { - name: "no-uids-ejected-2", - givenMaxCookieSize: 0, + name: "no-uids-ejected-2", + givenMaxCookieSize: 0, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ "1", "2", "3", "4", "5", "6", "7", }, }, { - name: "one-uid-ejected", - givenMaxCookieSize: 900, + name: "one-uid-ejected", + givenMaxCookieSize: 900, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ "1", "2", "3", "4", "5", "6", }, }, { - name: "four-uids-ejected", - givenMaxCookieSize: 500, + name: "four-uids-ejected", + givenMaxCookieSize: 500, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ "1", "2", "3", }, }, { - name: "all-but-one-uids-ejected", - givenMaxCookieSize: 300, + name: "all-but-one-uids-ejected", + givenMaxCookieSize: 300, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ "1", }, @@ -469,26 +490,43 @@ func TestPrepareCookieForWrite(t *testing.T) { { name: "all-uids-ejected", givenMaxCookieSize: 100, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{}, }, { name: "invalid-max-size", givenMaxCookieSize: -100, + givenCookieToSend: mainCookie, + givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{}, }, + { + name: "syncer-is-not-priority", + givenMaxCookieSize: 100, + givenCookieToSend: errorCookie, + givenIsSyncerPriority: false, + expectedError: errors.New("syncer key is not a priority, and there are only priority elements left"), + }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - encodedCookie, err := cookieToSend.PrepareCookieForWrite(&config.HostCookie{MaxCookieSizeBytes: test.givenMaxCookieSize}, encoder, ejector) - assert.NoError(t, err) - decodedCookie := decoder.Decode(encodedCookie) + ejector.IsSyncerPriority = test.givenIsSyncerPriority + encodedCookie, err := test.givenCookieToSend.PrepareCookieForWrite(&config.HostCookie{MaxCookieSizeBytes: test.givenMaxCookieSize}, encoder, ejector) + + if test.expectedError != nil { + assert.Equal(t, test.expectedError, err) + } else { + assert.NoError(t, err) + decodedCookie := decoder.Decode(encodedCookie) - for _, key := range test.expectedRemainingUidKeys { - _, ok := decodedCookie.uids[key] - assert.Equal(t, true, ok) + for _, key := range test.expectedRemainingUidKeys { + _, ok := decodedCookie.uids[key] + assert.Equal(t, true, ok) + } + assert.Equal(t, len(decodedCookie.uids), len(test.expectedRemainingUidKeys)) } - assert.Equal(t, len(decodedCookie.uids), len(test.expectedRemainingUidKeys)) }) } } diff --git a/usersync/ejector.go b/usersync/ejector.go index bce60892a69..e61533a9ad2 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -1,6 +1,7 @@ package usersync import ( + "errors" "time" ) @@ -11,9 +12,10 @@ type Ejector interface { type OldestEjector struct{} type PriorityBidderEjector struct { - PriorityGroups [][]string - syncersByBidder map[string]Syncer - TieEjector Ejector + PriorityGroups [][]string + syncersByBidder map[string]Syncer + IsSyncerPriority bool + TieEjector Ejector } // Choose method for oldest ejector will return the oldest uid @@ -33,6 +35,10 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { // Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { nonPriortyUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) + if len(nonPriortyUids) == 1 && !p.IsSyncerPriority { + return "", errors.New("syncer key is not a priority, and there are only priority elements left") + } + if len(nonPriortyUids) > 0 { return p.TieEjector.Choose(nonPriortyUids) } diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index c95b004a06d..b58793b1ec2 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -1,6 +1,7 @@ package usersync import ( + "errors" "reflect" "testing" "time" @@ -41,6 +42,7 @@ func TestPriorityEjector(t *testing.T) { key: "lowestPriority", }, }, + IsSyncerPriority: true, }, expected: "lowestPriority", }, @@ -68,7 +70,8 @@ func TestPriorityEjector(t *testing.T) { key: "olderButSamePriority", }, }, - TieEjector: &OldestEjector{}, + IsSyncerPriority: true, + TieEjector: &OldestEjector{}, }, expected: "olderButSamePriority", }, @@ -111,7 +114,8 @@ func TestPriorityEjector(t *testing.T) { key: "newestNonPriority", }, }, - TieEjector: &OldestEjector{}, + IsSyncerPriority: true, + TieEjector: &OldestEjector{}, }, expected: "oldestNonPriority", }, @@ -132,9 +136,35 @@ func TestPriorityEjector(t *testing.T) { key: "onlyPriorityElement", }, }, + IsSyncerPriority: true, }, expected: "onlyPriorityElement", }, + { + name: "syncer-is-not-priority", + givenUids: map[string]UIDEntry{ + "onlyPriorityElement": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + "syncer": { + UID: "456", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + }, + givenEjector: &PriorityBidderEjector{ + PriorityGroups: [][]string{ + {"onlyPriorityElement"}, + }, + syncersByBidder: map[string]Syncer{ + "onlyPriorityElement": fakeSyncer{ + key: "onlyPriorityElement", + }, + }, + IsSyncerPriority: false, + }, + expectedError: errors.New("syncer key is not a priority, and there are only priority elements left"), + }, } for _, test := range testCases { From 7d801a8737e0127f682a2b6d25d43bd7e1df53e7 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 17 Aug 2023 14:53:28 -0700 Subject: [PATCH 15/20] One line update to pri ejector --- usersync/cookie_test.go | 14 +++++++------- usersync/ejector.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index f49eac8a6a8..24acb63602f 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -451,6 +451,13 @@ func TestPrepareCookieForWrite(t *testing.T) { "1", "2", "3", "4", "5", "6", "7", }, }, + { + name: "syncer-is-not-priority", + givenMaxCookieSize: 100, + givenCookieToSend: errorCookie, + givenIsSyncerPriority: false, + expectedError: errors.New("syncer key is not a priority, and there are only priority elements left"), + }, { name: "no-uids-ejected-2", givenMaxCookieSize: 0, @@ -501,13 +508,6 @@ func TestPrepareCookieForWrite(t *testing.T) { givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{}, }, - { - name: "syncer-is-not-priority", - givenMaxCookieSize: 100, - givenCookieToSend: errorCookie, - givenIsSyncerPriority: false, - expectedError: errors.New("syncer key is not a priority, and there are only priority elements left"), - }, } for _, test := range testCases { diff --git a/usersync/ejector.go b/usersync/ejector.go index e61533a9ad2..a46295c6a85 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -35,7 +35,7 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { // Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { nonPriortyUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) - if len(nonPriortyUids) == 1 && !p.IsSyncerPriority { + if len(nonPriortyUids) == 1 && !p.IsSyncerPriority && len(p.PriorityGroups) > 0 { return "", errors.New("syncer key is not a priority, and there are only priority elements left") } From 54dfaaeaca3ff9701d8bfef3b027e30cf34d07d6 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 17 Aug 2023 17:47:43 -0700 Subject: [PATCH 16/20] Clean up, add tests --- usersync/cookie_test.go | 16 ++++++++-------- usersync/ejector.go | 21 +++++++++++++-------- usersync/ejector_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 24acb63602f..250ed3508c2 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -451,6 +451,14 @@ func TestPrepareCookieForWrite(t *testing.T) { "1", "2", "3", "4", "5", "6", "7", }, }, + { + name: "invalid-max-size", + givenMaxCookieSize: -100, + givenCookieToSend: mainCookie, + expectedRemainingUidKeys: []string{ + "1", "2", "3", "4", "5", "6", "7", + }, + }, { name: "syncer-is-not-priority", givenMaxCookieSize: 100, @@ -498,14 +506,6 @@ func TestPrepareCookieForWrite(t *testing.T) { name: "all-uids-ejected", givenMaxCookieSize: 100, givenCookieToSend: mainCookie, - givenIsSyncerPriority: true, - expectedRemainingUidKeys: []string{}, - }, - { - name: "invalid-max-size", - givenMaxCookieSize: -100, - givenCookieToSend: mainCookie, - givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{}, }, } diff --git a/usersync/ejector.go b/usersync/ejector.go index a46295c6a85..a1fdda39cdb 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -34,17 +34,16 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { // Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { - nonPriortyUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) - if len(nonPriortyUids) == 1 && !p.IsSyncerPriority && len(p.PriorityGroups) > 0 { - return "", errors.New("syncer key is not a priority, and there are only priority elements left") + nonPriorityUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) + if err := p.checkSyncerPriority(nonPriorityUids); err != nil { + return "", err } - if len(nonPriortyUids) > 0 { - return p.TieEjector.Choose(nonPriortyUids) + if len(nonPriorityUids) > 0 { + return p.TieEjector.Choose(nonPriorityUids) } lowestPriorityGroup := p.PriorityGroups[len(p.PriorityGroups)-1] - if len(lowestPriorityGroup) == 1 { uidToDelete := lowestPriorityGroup[0] p.PriorityGroups = removeElementFromPriorityGroup(p.PriorityGroups, uidToDelete) @@ -64,8 +63,7 @@ func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) func removeElementFromPriorityGroup(priorityGroups [][]string, oldestElement string) [][]string { lowestPriorityGroup := priorityGroups[len(priorityGroups)-1] if len(lowestPriorityGroup) <= 1 { - priorityGroups = priorityGroups[:len(priorityGroups)-1] - return priorityGroups + return priorityGroups[:len(priorityGroups)-1] } for index, elem := range lowestPriorityGroup { @@ -120,3 +118,10 @@ func getPriorityUids(lowestPriorityGroup []string, uids map[string]UIDEntry, syn } return lowestPriorityUIDs } + +func (p *PriorityBidderEjector) checkSyncerPriority(nonPriorityUids map[string]UIDEntry) error { + if len(nonPriorityUids) == 1 && !p.IsSyncerPriority && len(p.PriorityGroups) > 0 { + return errors.New("syncer key is not a priority, and there are only priority elements left") + } + return nil +} diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index b58793b1ec2..4a5be8d9794 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -119,6 +119,32 @@ func TestPriorityEjector(t *testing.T) { }, expected: "oldestNonPriority", }, + { + name: "empty-priority-groups", + givenUids: map[string]UIDEntry{ + "oldestNonPriority": { + UID: "456", + Expires: time.Now(), + }, + "newestNonPriority": { + UID: "123", + Expires: time.Now().Add((90 * 24 * time.Hour)), + }, + }, + givenEjector: &PriorityBidderEjector{ + syncersByBidder: map[string]Syncer{ + "oldestNonPriority": fakeSyncer{ + key: "oldestNonPriority", + }, + "newestNonPriority": fakeSyncer{ + key: "newestNonPriority", + }, + }, + IsSyncerPriority: false, + TieEjector: &OldestEjector{}, + }, + expected: "oldestNonPriority", + }, { name: "one-priority-element", givenUids: map[string]UIDEntry{ From 8440f54b6c5e1089aeed173fd0d2437618b5f5db Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 24 Aug 2023 10:41:13 -0700 Subject: [PATCH 17/20] Sole element > MaxCookieSize edge case --- usersync/cookie.go | 2 ++ usersync/cookie_test.go | 36 +++++++++++++++++++----------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/usersync/cookie.go b/usersync/cookie.go index 825d164c031..94ada94ed75 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -76,6 +76,8 @@ func (cookie *Cookie) PrepareCookieForWrite(cfg *config.HostCookie, encoder Enco isCookieTooBig := cookieSize > cfg.MaxCookieSizeBytes && cfg.MaxCookieSizeBytes > 0 if !isCookieTooBig { return encodedCookie, nil + } else if len(cookie.uids) == 1 { + return "", errors.New("uid that's trying to be synced is bigger than MaxCookieSize") } uidToDelete, err := ejector.Choose(cookie.uids) diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 250ed3508c2..cb9f0d0bfbc 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -383,13 +383,13 @@ func TestPrepareCookieForWrite(t *testing.T) { mainCookie := &Cookie{ uids: map[string]UIDEntry{ - "1": newTempId("1234567890123456789012345678901234567890123456", 7), - "2": newTempId("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 6), - "3": newTempId("123456789012345678901234567896123456789012345678", 5), - "4": newTempId("aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ", 4), - "5": newTempId("12345678901234567890123456789012345678901234567890", 3), - "6": newTempId("abcdefghij", 2), - "7": newTempId("abcdefghijklmnopqrstuvwxy", 1), + "mainUID": newTempId("1234567890123456789012345678901234567890123456", 7), + "2": newTempId("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 6), + "3": newTempId("123456789012345678901234567896123456789012345678", 5), + "4": newTempId("aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ", 4), + "5": newTempId("12345678901234567890123456789012345678901234567890", 3), + "6": newTempId("abcdefghij", 2), + "7": newTempId("abcdefghijklmnopqrstuvwxy", 1), }, optOut: false, } @@ -397,20 +397,21 @@ func TestPrepareCookieForWrite(t *testing.T) { errorCookie := &Cookie{ uids: map[string]UIDEntry{ "syncerNotPriority": newTempId("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 7), + "2": newTempId("1234567890123456789012345678901234567890123456", 7), // Priority Element }, optOut: false, } ejector := &PriorityBidderEjector{ PriorityGroups: [][]string{ - {"1"}, + {"mainUID"}, {"2", "3"}, {"4", "5", "6"}, {"7"}, }, syncersByBidder: map[string]Syncer{ - "1": fakeSyncer{ - key: "1", + "mainUID": fakeSyncer{ + key: "mainUID", }, "2": fakeSyncer{ key: "2", @@ -448,7 +449,7 @@ func TestPrepareCookieForWrite(t *testing.T) { givenCookieToSend: mainCookie, givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ - "1", "2", "3", "4", "5", "6", "7", + "mainUID", "2", "3", "4", "5", "6", "7", }, }, { @@ -456,7 +457,7 @@ func TestPrepareCookieForWrite(t *testing.T) { givenMaxCookieSize: -100, givenCookieToSend: mainCookie, expectedRemainingUidKeys: []string{ - "1", "2", "3", "4", "5", "6", "7", + "mainUID", "2", "3", "4", "5", "6", "7", }, }, { @@ -472,7 +473,7 @@ func TestPrepareCookieForWrite(t *testing.T) { givenCookieToSend: mainCookie, givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ - "1", "2", "3", "4", "5", "6", "7", + "mainUID", "2", "3", "4", "5", "6", "7", }, }, { @@ -481,7 +482,7 @@ func TestPrepareCookieForWrite(t *testing.T) { givenCookieToSend: mainCookie, givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ - "1", "2", "3", "4", "5", "6", + "mainUID", "2", "3", "4", "5", "6", }, }, { @@ -490,7 +491,7 @@ func TestPrepareCookieForWrite(t *testing.T) { givenCookieToSend: mainCookie, givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ - "1", "2", "3", + "mainUID", "2", "3", }, }, { @@ -499,13 +500,14 @@ func TestPrepareCookieForWrite(t *testing.T) { givenCookieToSend: mainCookie, givenIsSyncerPriority: true, expectedRemainingUidKeys: []string{ - "1", + "mainUID", }, }, { - name: "all-uids-ejected", + name: "only-main-uid-left", givenMaxCookieSize: 100, givenCookieToSend: mainCookie, + expectedError: errors.New("uid that's trying to be synced is bigger than MaxCookieSize"), expectedRemainingUidKeys: []string{}, }, } From 2e84e37248c51da99c615a6f85bbb8603407a82d Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Tue, 29 Aug 2023 09:35:36 -0700 Subject: [PATCH 18/20] Pass syncerByBidder, add new test --- endpoints/setuid.go | 2 +- endpoints/setuid_test.go | 164 ++++++++++++++++++++++++++++++++++----- usersync/cookie_test.go | 2 +- usersync/ejector.go | 6 +- usersync/ejector_test.go | 12 +-- 5 files changed, 156 insertions(+), 30 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index e2f277a0eb4..72f82520bf9 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -165,7 +165,7 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use setSiteCookie := siteCookieCheck(r.UserAgent()) // Priority Ejector Set Up - priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, TieEjector: &usersync.OldestEjector{}} + priorityEjector := &usersync.PriorityBidderEjector{PriorityGroups: cfg.UserSync.PriorityGroups, TieEjector: &usersync.OldestEjector{}, SyncersByBidder: syncersByBidder} priorityEjector.IsSyncerPriority = isSyncerPriority(bidderName, cfg.UserSync.PriorityGroups) // Write Cookie diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index aa6f7eb20b5..925e4c9b41a 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "strings" "testing" "time" @@ -306,15 +307,6 @@ func TestSetUIDEndpoint(t *testing.T) { expectedHeaders: map[string]string{"Content-Type": "text/html", "Content-Length": "0"}, description: "Set uid for valid bidder with valid account provided with invalid user sync activity", }, - { - uri: "/setuid?bidder=nonPriorityBidder&uid=123", - syncersBidderNameToKey: map[string]string{"nonPriorityBidder": "nonPrioritySyncer"}, - existingSyncs: nil, - gdprAllowsHostCookies: true, - expectedSyncs: nil, - expectedStatusCode: http.StatusBadRequest, - description: "Syncer is not a priority", - }, } analytics := analyticsConf.NewPBSAnalytics(&config.Analytics{}) @@ -322,7 +314,7 @@ func TestSetUIDEndpoint(t *testing.T) { for _, test := range testCases { response := doRequest(makeRequest(test.uri, test.existingSyncs), analytics, metrics, - test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false) + test.syncersBidderNameToKey, test.gdprAllowsHostCookies, test.gdprReturnsError, test.gdprMalformed, false, 0, nil) assert.Equal(t, test.expectedStatusCode, response.Code, "Test Case: %s. /setuid returned unexpected error code", test.description) if test.expectedSyncs != nil { @@ -349,6 +341,126 @@ func TestSetUIDEndpoint(t *testing.T) { } } +func TestSetUIDPriorityEjection(t *testing.T) { + decoder := usersync.Base64Decoder{} + analytics := analyticsConf.NewPBSAnalytics(&config.Analytics{}) + syncersByBidder := map[string]string{ + "pubmatic": "pubmatic", + "syncer1": "syncer1", + "syncer2": "syncer2", + "syncer3": "syncer3", + "syncer4": "syncer4", + "mismatchedBidderName": "syncer5", + "syncerToEject": "syncerToEject", + } + + testCases := []struct { + description string + uri string + givenExistingSyncs []string + givenPriorityGroups [][]string + givenMaxCookieSize int + expectedStatusCode int + expectedSyncer string + expectedUID string + expectedNumOfRemainingElements int + }{ + { + description: "Cookie empty, expect bidder to be synced, no ejection", + uri: "/setuid?bidder=pubmatic&uid=123", + givenPriorityGroups: [][]string{}, + givenMaxCookieSize: 500, + expectedSyncer: "pubmatic", + expectedUID: "123", + expectedNumOfRemainingElements: 1, + expectedStatusCode: http.StatusOK, + }, + { + description: "Cookie full, no priority groups, one ejection", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, + givenPriorityGroups: [][]string{}, + givenMaxCookieSize: 500, + expectedUID: "123", + expectedSyncer: "pubmatic", + expectedNumOfRemainingElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "Cookie full, eject lowest priority element", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer2", "syncer3", "syncer4", "syncerToEject"}, + givenPriorityGroups: [][]string{{"pubmatic", "syncer2", "syncer3", "syncer4"}, {"syncerToEject"}}, + givenMaxCookieSize: 500, + expectedUID: "123", + expectedSyncer: "pubmatic", + expectedNumOfRemainingElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "Cookie full, all elements same priority, one ejection", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer5"}, + givenPriorityGroups: [][]string{{"pubmatic", "syncer1", "syncer2", "syncer3", "mismatchedBidderName"}}, + givenMaxCookieSize: 500, + expectedUID: "123", + expectedSyncer: "pubmatic", + expectedNumOfRemainingElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "There are only priority elements left, but the bidder being synced isn't one", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, + givenPriorityGroups: [][]string{{"syncer1", "syncer2", "syncer3", "syncer4"}}, + givenMaxCookieSize: 500, + expectedStatusCode: http.StatusBadRequest, + }, + } + for _, test := range testCases { + request := httptest.NewRequest("GET", test.uri, nil) + + // Cookie Set Up + cookie := usersync.NewCookie() + for _, key := range test.givenExistingSyncs { + cookie.Sync(key, "111") + } + httpCookie, err := ToHTTPCookie(cookie) + assert.NoError(t, err) + request.AddCookie(httpCookie) + + // Make Request to /setuid + response := doRequest(request, analytics, &metricsConf.NilMetricsEngine{}, syncersByBidder, true, false, false, false, test.givenMaxCookieSize, test.givenPriorityGroups) + + // Get Cookie From Header + var cookieHeader string + for k, v := range response.Result().Header { + if k == "Set-Cookie" { + cookieHeader = v[0] + } + } + encodedCookieValue := getUIDFromHeader(cookieHeader) + + // Check That Bidder On Request was Synced, it's UID matches, and that the right number of elements are present after ejection + decodedCookie := decoder.Decode(encodedCookieValue) + decodedCookieUIDs := decodedCookie.GetUIDs() + if test.expectedStatusCode == http.StatusOK { + uid, ok := decodedCookieUIDs[test.expectedSyncer] + assert.Equal(t, true, ok, test.description) + assert.Equal(t, test.expectedUID, uid, test.description) + assert.Equal(t, test.expectedNumOfRemainingElements, len(decodedCookieUIDs), test.description) + + // Specific test case handling where we eject the lowest priority element + if len(test.givenPriorityGroups) == 2 { + syncer := test.givenPriorityGroups[len(test.givenPriorityGroups)-1][0] + _, ok := decodedCookieUIDs[syncer] + assert.Equal(t, false, ok, test.description) + } + } + assert.Equal(t, test.expectedStatusCode, response.Result().StatusCode, test.description) + } +} + func TestParseSignalFromGPPSID(t *testing.T) { type testOutput struct { signal gdpr.Signal @@ -1204,7 +1316,7 @@ func TestSetUIDEndpointMetrics(t *testing.T) { for _, v := range test.cookies { addCookie(req, v) } - response := doRequest(req, analyticsEngine, metricsEngine, test.syncersBidderNameToKey, test.gdprAllowsHostCookies, false, false, test.cfgAccountRequired) + response := doRequest(req, analyticsEngine, metricsEngine, test.syncersBidderNameToKey, test.gdprAllowsHostCookies, false, false, test.cfgAccountRequired, 0, nil) assert.Equal(t, test.expectedResponseCode, response.Code, test.description) analyticsEngine.AssertExpectations(t) @@ -1220,7 +1332,7 @@ func TestOptedOut(t *testing.T) { syncersBidderNameToKey := map[string]string{"pubmatic": "pubmatic"} analytics := analyticsConf.NewPBSAnalytics(&config.Analytics{}) metrics := &metricsConf.NilMetricsEngine{} - response := doRequest(request, analytics, metrics, syncersBidderNameToKey, true, false, false, false) + response := doRequest(request, analytics, metrics, syncersBidderNameToKey, true, false, false, false, 0, nil) assert.Equal(t, http.StatusUnauthorized, response.Code) } @@ -1406,7 +1518,7 @@ func makeRequest(uri string, existingSyncs map[string]string) *http.Request { return request } -func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool) *httptest.ResponseRecorder { +func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string) *httptest.ResponseRecorder { cfg := config.Configuration{ AccountRequired: cfgAccountRequired, BlacklistedAcctMap: map[string]bool{ @@ -1414,9 +1526,10 @@ func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metric }, AccountDefaults: config.Account{}, UserSync: config.UserSync{ - PriorityGroups: [][]string{ - {}, - }, + PriorityGroups: priorityGroups, + }, + HostCookie: config.HostCookie{ + MaxCookieSizeBytes: maxCookieSize, }, } cfg.MarshalAccountDefaults() @@ -1440,10 +1553,9 @@ func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metric syncersByBidder := make(map[string]usersync.Syncer) for bidderName, syncerKey := range syncersBidderNameToKey { syncersByBidder[bidderName] = fakeSyncer{key: syncerKey, defaultSyncType: usersync.SyncTypeIFrame} - if bidderName != "nonPriorityBidder" { + if priorityGroups == nil { + cfg.UserSync.PriorityGroups = [][]string{{}} cfg.UserSync.PriorityGroups[0] = append(cfg.UserSync.PriorityGroups[0], bidderName) - } else { - cfg.HostCookie.MaxCookieSizeBytes = 100 } } @@ -1563,3 +1675,17 @@ func ToHTTPCookie(cookie *usersync.Cookie) (*http.Cookie, error) { Path: "/", }, nil } + +func getUIDFromHeader(setCookieHeader string) string { + cookies := strings.Split(setCookieHeader, ";") + for _, cookie := range cookies { + trimmedCookie := strings.TrimSpace(cookie) + if strings.HasPrefix(trimmedCookie, "uids=") { + parts := strings.SplitN(trimmedCookie, "=", 2) + if len(parts) == 2 { + return parts[1] + } + } + } + return "" +} diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index cb9f0d0bfbc..340069767b1 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -409,7 +409,7 @@ func TestPrepareCookieForWrite(t *testing.T) { {"4", "5", "6"}, {"7"}, }, - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "mainUID": fakeSyncer{ key: "mainUID", }, diff --git a/usersync/ejector.go b/usersync/ejector.go index a1fdda39cdb..84299f72c12 100644 --- a/usersync/ejector.go +++ b/usersync/ejector.go @@ -13,7 +13,7 @@ type OldestEjector struct{} type PriorityBidderEjector struct { PriorityGroups [][]string - syncersByBidder map[string]Syncer + SyncersByBidder map[string]Syncer IsSyncerPriority bool TieEjector Ejector } @@ -34,7 +34,7 @@ func (o *OldestEjector) Choose(uids map[string]UIDEntry) (string, error) { // Choose method for priority ejector will return the oldest lowest priority element func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) { - nonPriorityUids := getNonPriorityUids(uids, p.PriorityGroups, p.syncersByBidder) + nonPriorityUids := getNonPriorityUids(uids, p.PriorityGroups, p.SyncersByBidder) if err := p.checkSyncerPriority(nonPriorityUids); err != nil { return "", err } @@ -50,7 +50,7 @@ func (p *PriorityBidderEjector) Choose(uids map[string]UIDEntry) (string, error) return uidToDelete, nil } - lowestPriorityUids := getPriorityUids(lowestPriorityGroup, uids, p.syncersByBidder) + lowestPriorityUids := getPriorityUids(lowestPriorityGroup, uids, p.SyncersByBidder) uidToDelete, err := p.TieEjector.Choose(lowestPriorityUids) if err != nil { return "", err diff --git a/usersync/ejector_test.go b/usersync/ejector_test.go index 4a5be8d9794..d13e7484b40 100644 --- a/usersync/ejector_test.go +++ b/usersync/ejector_test.go @@ -34,7 +34,7 @@ func TestPriorityEjector(t *testing.T) { {"highestPriorityBidder"}, {"lowestPriority"}, }, - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "highestPriorityBidder": fakeSyncer{ key: "highestPrioritySyncer", }, @@ -62,7 +62,7 @@ func TestPriorityEjector(t *testing.T) { PriorityGroups: [][]string{ {"newerButSamePriority", "olderButSamePriority"}, }, - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "newerButSamePriority": fakeSyncer{ key: "newerButSamePriority", }, @@ -100,7 +100,7 @@ func TestPriorityEjector(t *testing.T) { {"higherPriority"}, {"lowestPriority"}, }, - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "higherPriority": fakeSyncer{ key: "higherPriority", }, @@ -132,7 +132,7 @@ func TestPriorityEjector(t *testing.T) { }, }, givenEjector: &PriorityBidderEjector{ - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "oldestNonPriority": fakeSyncer{ key: "oldestNonPriority", }, @@ -157,7 +157,7 @@ func TestPriorityEjector(t *testing.T) { PriorityGroups: [][]string{ {"onlyPriorityElement"}, }, - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "onlyPriorityElement": fakeSyncer{ key: "onlyPriorityElement", }, @@ -182,7 +182,7 @@ func TestPriorityEjector(t *testing.T) { PriorityGroups: [][]string{ {"onlyPriorityElement"}, }, - syncersByBidder: map[string]Syncer{ + SyncersByBidder: map[string]Syncer{ "onlyPriorityElement": fakeSyncer{ key: "onlyPriorityElement", }, From 10aebbc408b94f9960008cc000c3e8a8f1a0f635 Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Wed, 13 Sep 2023 14:00:15 -0700 Subject: [PATCH 19/20] Return unaltered cookie and 200 --- endpoints/cookie_sync.go | 1 + endpoints/setuid.go | 28 ++++++-- endpoints/setuid_test.go | 144 +++++++++++++++++++++------------------ 3 files changed, 103 insertions(+), 70 deletions(-) diff --git a/endpoints/cookie_sync.go b/endpoints/cookie_sync.go index a8583a72316..3c160529e94 100644 --- a/endpoints/cookie_sync.go +++ b/endpoints/cookie_sync.go @@ -39,6 +39,7 @@ var ( errCookieSyncAccountBlocked = errors.New("account is disabled, please reach out to the prebid server host") errCookieSyncAccountConfigMalformed = errors.New("account config is malformed and could not be read") errCookieSyncAccountInvalid = errors.New("account must be valid if provided, please reach out to the prebid server host") + errSyncerIsNotPriority = errors.New("syncer key is not a priority, and there are only priority elements left") ) var cookieSyncBidderFilterAllowAll = usersync.NewUniformBidderFilter(usersync.BidderFilterModeInclude) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 42fe3948377..0192d350d6a 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -57,6 +57,16 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use } usersync.SyncHostCookie(r, cookie, &cfg.HostCookie) + unalteredCookie, err := encoder.Encode(cookie) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(err.Error())) + metricsEngine.RecordSetUid(metrics.SetUidBadRequest) + so.Errors = []error{err} + so.Status = http.StatusBadRequest + return + } + query := r.URL.Query() syncer, bidderName, err := getSyncer(query, syncersByBidder) @@ -166,11 +176,15 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use // Write Cookie encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, priorityEjector) if err != nil { - w.WriteHeader(http.StatusBadRequest) - metricsEngine.RecordSetUid(metrics.SetUidBadRequest) - so.Errors = []error{err} - so.Status = http.StatusBadRequest - return + if err.Error() == errSyncerIsNotPriority.Error() { + encodedCookie = unalteredCookie + } else { + w.WriteHeader(http.StatusBadRequest) + metricsEngine.RecordSetUid(metrics.SetUidBadRequest) + so.Errors = []error{err} + so.Status = http.StatusBadRequest + return + } } usersync.WriteCookie(w, encodedCookie, &cfg.HostCookie, setSiteCookie) @@ -185,6 +199,10 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use w.Header().Add("Content-Length", "0") w.WriteHeader(http.StatusOK) } + // Add Warning to Response If Present + if err != nil && err.Error() == errSyncerIsNotPriority.Error() { + w.Write([]byte("Warning: " + err.Error() + " returning unaltered cookie")) + } }) } diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 925e4c9b41a..886c15df9c4 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -355,66 +355,73 @@ func TestSetUIDPriorityEjection(t *testing.T) { } testCases := []struct { - description string - uri string - givenExistingSyncs []string - givenPriorityGroups [][]string - givenMaxCookieSize int - expectedStatusCode int - expectedSyncer string - expectedUID string - expectedNumOfRemainingElements int + description string + uri string + givenExistingSyncs []string + givenPriorityGroups [][]string + givenMaxCookieSize int + expectedStatusCode int + expectedSyncer string + expectedUID string + expectedNumOfElements int }{ { - description: "Cookie empty, expect bidder to be synced, no ejection", - uri: "/setuid?bidder=pubmatic&uid=123", - givenPriorityGroups: [][]string{}, - givenMaxCookieSize: 500, - expectedSyncer: "pubmatic", - expectedUID: "123", - expectedNumOfRemainingElements: 1, - expectedStatusCode: http.StatusOK, - }, - { - description: "Cookie full, no priority groups, one ejection", - uri: "/setuid?bidder=pubmatic&uid=123", - givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, - givenPriorityGroups: [][]string{}, - givenMaxCookieSize: 500, - expectedUID: "123", - expectedSyncer: "pubmatic", - expectedNumOfRemainingElements: 4, - expectedStatusCode: http.StatusOK, - }, - { - description: "Cookie full, eject lowest priority element", - uri: "/setuid?bidder=pubmatic&uid=123", - givenExistingSyncs: []string{"syncer2", "syncer3", "syncer4", "syncerToEject"}, - givenPriorityGroups: [][]string{{"pubmatic", "syncer2", "syncer3", "syncer4"}, {"syncerToEject"}}, - givenMaxCookieSize: 500, - expectedUID: "123", - expectedSyncer: "pubmatic", - expectedNumOfRemainingElements: 4, - expectedStatusCode: http.StatusOK, - }, - { - description: "Cookie full, all elements same priority, one ejection", - uri: "/setuid?bidder=pubmatic&uid=123", - givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer5"}, - givenPriorityGroups: [][]string{{"pubmatic", "syncer1", "syncer2", "syncer3", "mismatchedBidderName"}}, - givenMaxCookieSize: 500, - expectedUID: "123", - expectedSyncer: "pubmatic", - expectedNumOfRemainingElements: 4, - expectedStatusCode: http.StatusOK, - }, - { - description: "There are only priority elements left, but the bidder being synced isn't one", - uri: "/setuid?bidder=pubmatic&uid=123", - givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, - givenPriorityGroups: [][]string{{"syncer1", "syncer2", "syncer3", "syncer4"}}, - givenMaxCookieSize: 500, - expectedStatusCode: http.StatusBadRequest, + description: "Cookie empty, expect bidder to be synced, no ejection", + uri: "/setuid?bidder=pubmatic&uid=123", + givenPriorityGroups: [][]string{}, + givenMaxCookieSize: 500, + expectedSyncer: "pubmatic", + expectedUID: "123", + expectedNumOfElements: 1, + expectedStatusCode: http.StatusOK, + }, + { + description: "Cookie full, no priority groups, one ejection", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, + givenPriorityGroups: [][]string{}, + givenMaxCookieSize: 500, + expectedUID: "123", + expectedSyncer: "pubmatic", + expectedNumOfElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "Cookie full, eject lowest priority element", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer2", "syncer3", "syncer4", "syncerToEject"}, + givenPriorityGroups: [][]string{{"pubmatic", "syncer2", "syncer3", "syncer4"}, {"syncerToEject"}}, + givenMaxCookieSize: 500, + expectedUID: "123", + expectedSyncer: "pubmatic", + expectedNumOfElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "Cookie full, all elements same priority, one ejection", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer5"}, + givenPriorityGroups: [][]string{{"pubmatic", "syncer1", "syncer2", "syncer3", "mismatchedBidderName"}}, + givenMaxCookieSize: 500, + expectedUID: "123", + expectedSyncer: "pubmatic", + expectedNumOfElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "There are only priority elements left, but the bidder being synced isn't one", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, + givenPriorityGroups: [][]string{{"syncer1", "syncer2", "syncer3", "syncer4"}}, + givenMaxCookieSize: 500, + expectedNumOfElements: 4, + expectedStatusCode: http.StatusOK, + }, + { + description: "Uid that's trying to be synced is bigger than MaxCookieSize", + uri: "/setuid?bidder=pubmatic&uid=123", + givenMaxCookieSize: 1, + expectedStatusCode: http.StatusBadRequest, }, } for _, test := range testCases { @@ -444,19 +451,26 @@ func TestSetUIDPriorityEjection(t *testing.T) { // Check That Bidder On Request was Synced, it's UID matches, and that the right number of elements are present after ejection decodedCookie := decoder.Decode(encodedCookieValue) decodedCookieUIDs := decodedCookie.GetUIDs() - if test.expectedStatusCode == http.StatusOK { - uid, ok := decodedCookieUIDs[test.expectedSyncer] - assert.Equal(t, true, ok, test.description) - assert.Equal(t, test.expectedUID, uid, test.description) - assert.Equal(t, test.expectedNumOfRemainingElements, len(decodedCookieUIDs), test.description) + + if test.expectedSyncer != "" { + assert.Equal(t, test.expectedUID, decodedCookieUIDs[test.expectedSyncer], test.description) + assert.Equal(t, test.expectedNumOfElements, len(decodedCookieUIDs), test.description) // Specific test case handling where we eject the lowest priority element if len(test.givenPriorityGroups) == 2 { syncer := test.givenPriorityGroups[len(test.givenPriorityGroups)-1][0] - _, ok := decodedCookieUIDs[syncer] - assert.Equal(t, false, ok, test.description) + _, syncerExists := decodedCookieUIDs[syncer] + assert.False(t, syncerExists, test.description) } + } else if len(test.givenExistingSyncs) > 0 { + // Check that the unaltered cookie was returned + for _, sync := range test.givenExistingSyncs { + _, syncExists := decodedCookieUIDs[sync] + assert.True(t, syncExists, test.description) + } + assert.Equal(t, test.expectedNumOfElements, len(decodedCookieUIDs), test.description) } + assert.Equal(t, test.expectedStatusCode, response.Result().StatusCode, test.description) } } From 48876a1b562bbfa4a36f5cad3c2c3da90b7e158f Mon Sep 17 00:00:00 2001 From: AlexBVolcy Date: Thu, 14 Sep 2023 13:52:37 -0700 Subject: [PATCH 20/20] Just return 200 and warning, cookie unchanged --- endpoints/setuid.go | 19 ++++------------ endpoints/setuid_test.go | 47 ++++++++++++++++++---------------------- 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 0192d350d6a..38abb7d9a09 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -57,16 +57,6 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use } usersync.SyncHostCookie(r, cookie, &cfg.HostCookie) - unalteredCookie, err := encoder.Encode(cookie) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) - metricsEngine.RecordSetUid(metrics.SetUidBadRequest) - so.Errors = []error{err} - so.Status = http.StatusBadRequest - return - } - query := r.URL.Query() syncer, bidderName, err := getSyncer(query, syncersByBidder) @@ -177,7 +167,10 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use encodedCookie, err := cookie.PrepareCookieForWrite(&cfg.HostCookie, encoder, priorityEjector) if err != nil { if err.Error() == errSyncerIsNotPriority.Error() { - encodedCookie = unalteredCookie + w.WriteHeader(http.StatusOK) + w.Write([]byte("Warning: " + err.Error() + ", cookie not updated")) + so.Status = http.StatusOK + return } else { w.WriteHeader(http.StatusBadRequest) metricsEngine.RecordSetUid(metrics.SetUidBadRequest) @@ -199,10 +192,6 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use w.Header().Add("Content-Length", "0") w.WriteHeader(http.StatusOK) } - // Add Warning to Response If Present - if err != nil && err.Error() == errSyncerIsNotPriority.Error() { - w.Write([]byte("Warning: " + err.Error() + " returning unaltered cookie")) - } }) } diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 886c15df9c4..df84d5c7040 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -364,6 +364,7 @@ func TestSetUIDPriorityEjection(t *testing.T) { expectedSyncer string expectedUID string expectedNumOfElements int + expectedWarning string }{ { description: "Cookie empty, expect bidder to be synced, no ejection", @@ -409,13 +410,13 @@ func TestSetUIDPriorityEjection(t *testing.T) { expectedStatusCode: http.StatusOK, }, { - description: "There are only priority elements left, but the bidder being synced isn't one", - uri: "/setuid?bidder=pubmatic&uid=123", - givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, - givenPriorityGroups: [][]string{{"syncer1", "syncer2", "syncer3", "syncer4"}}, - givenMaxCookieSize: 500, - expectedNumOfElements: 4, - expectedStatusCode: http.StatusOK, + description: "There are only priority elements left, but the bidder being synced isn't one", + uri: "/setuid?bidder=pubmatic&uid=123", + givenExistingSyncs: []string{"syncer1", "syncer2", "syncer3", "syncer4"}, + givenPriorityGroups: [][]string{{"syncer1", "syncer2", "syncer3", "syncer4"}}, + givenMaxCookieSize: 500, + expectedStatusCode: http.StatusOK, + expectedWarning: "Warning: syncer key is not a priority, and there are only priority elements left, cookie not updated", }, { description: "Uid that's trying to be synced is bigger than MaxCookieSize", @@ -439,20 +440,22 @@ func TestSetUIDPriorityEjection(t *testing.T) { // Make Request to /setuid response := doRequest(request, analytics, &metricsConf.NilMetricsEngine{}, syncersByBidder, true, false, false, false, test.givenMaxCookieSize, test.givenPriorityGroups) - // Get Cookie From Header - var cookieHeader string - for k, v := range response.Result().Header { - if k == "Set-Cookie" { - cookieHeader = v[0] + if test.expectedWarning != "" { + assert.Equal(t, test.expectedWarning, response.Body.String(), test.description) + } else if test.expectedSyncer != "" { + // Get Cookie From Header + var cookieHeader string + for k, v := range response.Result().Header { + if k == "Set-Cookie" { + cookieHeader = v[0] + } } - } - encodedCookieValue := getUIDFromHeader(cookieHeader) + encodedCookieValue := getUIDFromHeader(cookieHeader) - // Check That Bidder On Request was Synced, it's UID matches, and that the right number of elements are present after ejection - decodedCookie := decoder.Decode(encodedCookieValue) - decodedCookieUIDs := decodedCookie.GetUIDs() + // Check That Bidder On Request was Synced, it's UID matches, and that the right number of elements are present after ejection + decodedCookie := decoder.Decode(encodedCookieValue) + decodedCookieUIDs := decodedCookie.GetUIDs() - if test.expectedSyncer != "" { assert.Equal(t, test.expectedUID, decodedCookieUIDs[test.expectedSyncer], test.description) assert.Equal(t, test.expectedNumOfElements, len(decodedCookieUIDs), test.description) @@ -462,15 +465,7 @@ func TestSetUIDPriorityEjection(t *testing.T) { _, syncerExists := decodedCookieUIDs[syncer] assert.False(t, syncerExists, test.description) } - } else if len(test.givenExistingSyncs) > 0 { - // Check that the unaltered cookie was returned - for _, sync := range test.givenExistingSyncs { - _, syncExists := decodedCookieUIDs[sync] - assert.True(t, syncExists, test.description) - } - assert.Equal(t, test.expectedNumOfElements, len(decodedCookieUIDs), test.description) } - assert.Equal(t, test.expectedStatusCode, response.Result().StatusCode, test.description) } }