Sync horrors from 2012

gistfile1.txt

19:50 <@rnewman> well, this might be a problem.
19:50 <@rnewman>  0:01.90 Sync.Engine.Bookmarks DEBUG   Mapped: Bookmarks Toolbar,fTaggy tag,fake-guid-1,false
19:50 <@rnewman>  0:01.90 Sync.Engine.Bookmarks DEBUG   Finding mapping: , fBookmarks Toolbar
19:50 <@rnewman>  0:01.90 Sync.Engine.Bookmarks DEBUG   Mapped dupe: toolbar
19:50 <@rnewman>  0:01.90 Sync.Engine.Bookmarks DEBUG   zzzzzzzzzzzz mapped to toolbar 0:01.90 Sync.Engine.Bookmarks    DEBUG   Local item toolbar is a duplicate for incoming item zzzzzzzzzzzz 0:01.90 Sync.Store.Bookmarks   DEBUG   Number of rows matching GUID toolbar: 0
19:50 <@rnewman> trivial test
19:50 <@rnewman> create a folder on a client somewhere
19:50 <@rnewman> name it "Bookmarks Toolbar"
19:51 <@rnewman> any desktop client that applies that record is gonna have a bad time.
19:51  * rnewman tries it for real
20:34 <@rnewman> shit, this is a really complicated bug
20:34 <@rnewman> I think what happens is this
20:34 <@rnewman> you get a record titled, say, "Bookmarks Toolbar" on Android, with a parent named ""
20:35 <@rnewman> this would reconcile with desktop's Bookmarks Toolbar
20:35 <@rnewman> except! it will only reconcile if it is *not* marked with hasDupe: true
20:35 <@rnewman> desktop sets hasDupe when it uploads the record
20:35 <@rnewman> Android does not
20:35 <@rnewman> so you get that record on your phone, then get node reassigned
20:35 <@rnewman> the phone uploads the record, desktop downloads it, and wipes out your local toolbar
20:35 <@rnewman> because the phone does not set hasDupe
20:36 <@rnewman> mconnor: when I say our bookmark code on desktop needs to be rewritten, this is the kind of edge case I'm talking about :D
20:36 <@rnewman> we have total data annihiliation hiding in accidental orderings of function invocation on desktop
20:37 <@rnewman> that aren't captured in any spec
20:37 <@rnewman> and aren't covered by any tests
20:37 <@mconnor> rnewman: didn't you and philikon rewrite this once already? :P
20:37 <@rnewman> yeah, so now desktop syncs fairly flawlessly with other desktops :P
20:38 <@rnewman> I wonder what happens if I upload a record "", parent ""
20:38 <@rnewman> that might nuke the places root
20:39 <@rnewman> perhaps I should eat something first
20:39 <@rnewman> god, I feel like such a programmer right now
20:39 <@rnewman> food, then more tests
20:42 <@rnewman> oh, christ, this is probably worse than I thought; if hasDupe is set, we don't find dupes
20:42 <@rnewman> which will lead to... duplicate folders
20:42 <@rnewman> if someone tries to clean up dupes, they'll get... more dupes
20:42 <@rnewman> the more you restore from backups etc. to try to fix things, the worse it'll get
20:43 <@rnewman> why do I feel like I can probably blame anant and thunder for this?
20:43 <@rnewman> so fragile
01:37 <@rnewman> oh, the horror
01:38 <@rnewman> this bug doesn't happen in the real world. the only reason why it does not is that places doesn't use "toolbar" as the GUID for the real toolbar root
01:38 <@rnewman> we try to change it, and fail
01:49 <@rnewman> alright, that's enough for one evening
01:50 <@rnewman> I'm betting there's a bug hiding somewhere in that bookmark reconciling code
01:50 <@rnewman> hasDupe + _guidMap are incredibly hacky
02:30 <@rnewman> bed fail.