diff --git a/usersync/cookie.go b/usersync/cookie.go index 5732e6c4c31..51474afdd7c 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -226,10 +226,9 @@ func (cookie *Cookie) TrySync(key string, uid string) error { // This exists so that Cookie (which is public) can have private fields, and the rest of // the code doesn't have to worry about the cookie data storage format. type cookieJson struct { - LegacyUIDs map[string]string `json:"uids,omitempty"` - UIDs map[string]uidWithExpiry `json:"tempUIDs,omitempty"` - OptOut bool `json:"optout,omitempty"` - Birthday *time.Time `json:"bday,omitempty"` + UIDs map[string]uidWithExpiry `json:"tempUIDs,omitempty"` + OptOut bool `json:"optout,omitempty"` + Birthday *time.Time `json:"bday,omitempty"` } func (cookie *Cookie) MarshalJSON() ([]byte, error) { @@ -240,53 +239,31 @@ func (cookie *Cookie) MarshalJSON() ([]byte, error) { }) } -// UnmarshalJSON holds some transition code. -// -// "Legacy" cookies had UIDs *without* expiration dates, and recognized "0" as a legitimate UID for audienceNetwork. -// "Current" cookies always include UIDs with expiration dates, and never allow "0" for audienceNetwork. -// -// This Unmarshal method interprets both data formats, and does some conversions on legacy data to make it current. -// If you're seeing this message after March 2018, it's safe to assume that all the legacy cookies have been -// updated and remove the legacy logic. func (cookie *Cookie) UnmarshalJSON(b []byte) error { var cookieContract cookieJson - err := json.Unmarshal(b, &cookieContract) - if err == nil { - cookie.optOut = cookieContract.OptOut - cookie.birthday = cookieContract.Birthday - - if cookie.optOut { - cookie.uids = make(map[string]uidWithExpiry) - } else { - cookie.uids = cookieContract.UIDs - - if cookie.uids == nil { - cookie.uids = make(map[string]uidWithExpiry, len(cookieContract.LegacyUIDs)) - } + if err := json.Unmarshal(b, &cookieContract); err != nil { + return err + } - // Interpret "legacy" UIDs as having been expired already. - // This should cause us to re-sync, since it would be time for a new one. - for bidder, uid := range cookieContract.LegacyUIDs { - if _, ok := cookie.uids[bidder]; !ok { - cookie.uids[bidder] = uidWithExpiry{ - UID: uid, - Expires: time.Now().Add(-5 * time.Minute), - } - } - } + cookie.optOut = cookieContract.OptOut + cookie.birthday = cookieContract.Birthday - // Any "0" values from audienceNetwork really meant "no ID available." This happens if they've never - // logged into Facebook. However... once we know a user's ID, we stop trying to re-sync them until the - // expiration date has passed. - // - // Since users may log into facebook later, this is a bad strategy. - // Since "0" is a fake ID for this bidder, we'll just treat it like it doesn't exist. - if id, ok := cookie.uids[string(openrtb_ext.BidderAudienceNetwork)]; ok && id.UID == "0" { - delete(cookie.uids, string(openrtb_ext.BidderAudienceNetwork)) - } - } + if cookie.optOut { + cookie.uids = nil + } else { + cookie.uids = cookieContract.UIDs } - return err + + if cookie.uids == nil { + cookie.uids = make(map[string]uidWithExpiry) + } + + // Audience Network / Facebook Handling + if id, ok := cookie.uids[string(openrtb_ext.BidderAudienceNetwork)]; ok && id.UID == "0" { + delete(cookie.uids, string(openrtb_ext.BidderAudienceNetwork)) + } + + return nil } func timestamp() *time.Time { diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 4e87db5dd0a..8efa3b746b6 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -2,7 +2,6 @@ package usersync import ( "encoding/base64" - "encoding/json" "net/http" "net/http/httptest" "strings" @@ -15,7 +14,7 @@ import ( func TestOptOutCookie(t *testing.T) { cookie := &Cookie{ - uids: make(map[string]uidWithExpiry), + uids: map[string]uidWithExpiry{"appnexus": {UID: "test"}}, optOut: true, birthday: timestamp(), } @@ -260,32 +259,6 @@ func TestCookieReadWrite(t *testing.T) { assert.Len(t, received.uids, 2, "Sync Count") } -func TestPopulatedLegacyCookieRead(t *testing.T) { - legacyJson := `{"uids":{"adnxs":"123","audienceNetwork":"456"},"bday":"2017-08-03T21:04:52.629198911Z"}` - var cookie Cookie - json.Unmarshal([]byte(legacyJson), &cookie) - - if cookie.HasAnyLiveSyncs() { - t.Error("Expected 0 user syncs. Found at least 1.") - } - if cookie.HasLiveSync("adnxs") { - t.Errorf("Received cookie should act like it has no ID for adnxs.") - } - if cookie.HasLiveSync("audienceNetwork") { - t.Errorf("Received cookie should act like it has no ID for audienceNetwork.") - } -} - -func TestEmptyLegacyCookieRead(t *testing.T) { - legacyJson := `{"bday":"2017-08-29T18:54:18.393925772Z"}` - var cookie Cookie - json.Unmarshal([]byte(legacyJson), &cookie) - - if cookie.HasAnyLiveSyncs() { - t.Error("Expected 0 user syncs. Found at least 1.") - } -} - func TestNilCookie(t *testing.T) { var nilCookie *Cookie @@ -427,7 +400,11 @@ func ensureConsistency(t *testing.T, cookie *Cookie) { t.Error("The PBSCookie interface shouldn't let modifications happen if the user has opted out") } - assert.Equal(t, len(cookie.uids), len(copiedCookie.uids), "Incorrect sync count on reparsed cookie.") + if cookie.optOut { + assert.Equal(t, 0, len(copiedCookie.uids), "Incorrect sync count on reparsed cookie.") + } else { + assert.Equal(t, len(cookie.uids), len(copiedCookie.uids), "Incorrect sync count on reparsed cookie.") + } for family, uid := range copiedCookie.uids { if !cookie.HasLiveSync(family) {