Fix ItemsMap doesn't match real object #1279
Conversation
Multiple reads after tasks have been fetched causes folder index to be misaligned. This fix forces fetched tasks to be queued, ensuring that task handlers finish committing their changes before running the next task handler.
|
@darkdread Welcome, and thank you for the proposed fix! It makes sense to me at first glance, and this bug surely is a rather critical show stopper. Since you seem to have nailed the bug down to the actual problem there, I'm wondering if you could add a test case that demonstrates the bug and that this implementation fixes it? That would make it both easier to review as well as bulletproof against future regressions. The testing setup is pretty straight-forward, and even though we're in the process of porting everything from Jaribu to Mocha/Chai (and TS), it's still fine to add new cases to the existing suites. |
raucao
changed the title
Updated inmemorystorage.ts getNodes to follow behavior of localStorage and indexeddb.
darkdread
force-pushed
the
bugfix/1187-ItemsMap_doesnt_match_real_object
branch
from
a94fc46
to
41e409d
Compare
|
Great! @darkdread Just to confirm with you: the first test case added is a missing case and supposed to be green on |
In order to simulate the bug, you have to emulate the cloning properties of localStorage and indexeddb for inmemorystorage. I believe the current getNodes implementation of inmemorystorage is a bug; it should behave the same way as its other counterparts. The first test case after applying the inmemorystorage fix should fail on master, while the second test case should pass. On a side note, should I open a bug for the fix I applied? |
|
Ok, thanks. Confirmed.
Yes, I think that's a good idea. |
|
FWIW, I'm not familiar with this part of the code to knowledgeably review it. Thanks, everyone, for your work! |
Multiple reads after tasks have been fetched causes folder index to be misaligned. This fix forces fetched tasks to be queued, ensuring that task handlers finish committing their changes before running the next task handler.
fixes #1187