596424 - Gloda._initMyIdentities which initializes concept of 'me' does not normalize (lowercase) e-mail addresses

Status: REOPENED

(MailNews Core :: Database, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

This can break the 'to me'/'from me' logic in the faceted search UI.  This happens because the normal e-mail => identity logic in Gloda.getOrCreateMailIdentities does lowercase things and that is what is used for all e-mail indexing.

The fix for this can be easily made to update the database without requiring it to be blown away.

The basic strategy to correct the problem is twofold:

1) Detect e-mail addresses that are not already all lowercase.  If we have such an e-mail address, check for an Identity that exists with the mixed case variant.  If it exists, delete it.

2) Ensure correct identity/contact relationships for all identity lookups, making sure to update the database records and prune no-longer-relevant contacts.  We can expect that in all cases where the bug triggered that we will have identities with their own separate contacts.  We were already sorta-doing this by clobbering the in-memory representations to achieve this status, but it never hit the disk and could leave orphaned contacts that could result in confusing global search autocomplete cases that would have no results.

Unit tests required to verify correct upgrade procedure of existing databases:

a1) Prior to calling the me-init logic, have an account with a mixed-case email with a corresponding 'me' contact and associated mixed-case identity.  Have an already extant lowercase identity and contact not associated with the 'me' contact.  Run the me-init logic.  Verify that the in-memory 'me' identity is continuing to use the same 'me' contact and that it only has the lower-case 'me' identity now associated.  Flush cache (or otherwise avoid cache), verify that state is reflected on disk.  Verify that the mixed-case identity has been deleted and that the contact that owned the lowercase identity has likewise been deleted.

a2) In that same test, have poked some popularity values onto things and verify 'conservation of popularity' to make sure that we did not throw away any popularity scores... those can be important.

Unit tests required to verify correct handling of stuff going forward:

b) Have an account with mixed-case email prior to 'me' init.  Post 'me' init ensure that we only have an identity with the lowercased e-mail address and no mixed-cased identity.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 kill mixed case identities, normify down to a single contact

I think this resolves the problem and fixes existing unhappy databases.

This removes some code from glodaTestHelper to create an identity that I don't believe was working right because of a missing function call (so we ended up just adding useless expandos.)

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/e965fd4adf4e

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

backed out of comm-central:
http://hg.mozilla.org/comm-central/rev/5bda22574605
http://hg.mozilla.org/comm-central/rev/f0b8ffc1da8d

The linux opt build for this experienced a test failure on momo-vm-12 but then was happy again on a subsequent build.  Something that I'm assuming is deterministic would appear to not be...

The log bits look like so:

TEST-INFO | (xpcshell/head.js) | test 2 pending
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 155] null == null
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 163] null == null
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 164] null == null
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 177] 0 == 0
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 180] 2 == 2
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 182] 1 != 5
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js | [test_mixed_case_me_email_migration : 191] 1 == 1
*******************************************
Generator explosion!
Unhappiness at: /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js:190
Because: TypeError: GlodaDatastore.getIdentity("email", postMeContact._identities[i].value) is null
Stack:
  test_mixed_case_me_email_migration()@/buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js:190
  _async_driver()@../../../../resources/asyncTestUtils.js:155
  ()@/buildbot/linux-comm-central-check/build/mozilla/testing/xpcshell/head.js:284
  
**** Async Generator Stack source functions:
  test_mixed_case_me_email_migration
  _async_test_runner
*********
-- Exception object --
+ message (string) 'GlodaDatastore.getIdentity("email", postMeContact._identities[i].value) is null'
+ fileName (string) '/buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js'
+ lineNumber (number) 190
+ stack (string) 299 chars
+ name (string) 'TypeError'
*
-- Stack Trace --
test_mixed_case_me_email_migration()@/buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/mailnews/db/gloda/test/unit/test_me_logic.js:190
_async_driver()@../../../../resources/asyncTestUtils.js:155
()@/buildbot/linux-comm-central-check/build/mozilla/testing/xpcshell/head.js:284
TEST-UNEXPECTED-FAIL | ../../../../resources/asyncTestUtils.js | Generator explosion. ex: TypeError: GlodaDatastore.getIdentity("email", postMeContact._identities[i].value) is null async stack test_mixed_case_me_email_migration,_async_test_runner - See following stack:
JS frame :: /buildbot/linux-comm-central-check/build/mozilla/testing/xpcshell/head.js :: do_throw :: line 317
JS frame :: ../../../../resources/logHelper.js :: mark_failure :: line 554
JS frame :: ../../../../resources/asyncTestUtils.js :: _async_driver :: line 175
JS frame :: /buildbot/linux-comm-central-check/build/mozilla/testing/xpcshell/head.js :: anonymous :: line 284
TEST-INFO | (xpcshell/head.js) | exiting test
TEST-INFO | (xpcshell/head.js) | test 2 finished

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 475411 [details] [diff] [review]
v1 kill mixed case identities, normify down to a single contact

I guess this needs a new patch.