Page MenuHomePhabricator
Differential D13464

[lib] Check ID conflicts when processing operations
ClosedPublic
Actions

Authored by tomek on Wed, Sep 25, 10:13 AM.
Tags
None
Referenced Files
F2840569: D13464.diff
Sun, Sep 29, 6:23 AM
Unknown Object (File)
Fri, Sep 27, 11:38 PM
Unknown Object (File)
Fri, Sep 27, 10:56 AM
Unknown Object (File)
Fri, Sep 27, 10:47 AM
Unknown Object (File)
Fri, Sep 27, 3:29 AM
Unknown Object (File)
Wed, Sep 25, 12:22 PM
Unknown Object (File)
Wed, Sep 25, 12:21 PM
Unknown Object (File)
Wed, Sep 25, 12:04 PM
Subscribers
ashoat

Details

Reviewers
kamil
will
Commits
rCOMM082447a2d740: [lib] Check ID conflicts when processing operations
Summary

We should check whether an entity with the same ID exists and discard an operation in a case of conflict

https://linear.app/comm/issue/ENG-9190/alternative-solution-update-canbeprocessed-functions

Test Plan

Tested some scenarios:

  1. Repeated text message with the same ID
  2. Creating sidebars with
    1. the same thread ID
    2. the same sidebar source message ID
    3. the same create sidebar message ID
  3. Repeated changing thread settings with the same message ID prefix

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Wed, Sep 25, 10:13 AM
Herald added a subscriber: ashoat. · View Herald TranscriptWed, Sep 25, 10:13 AM
Harbormaster completed remote builds in B31847: Diff 44557.Wed, Sep 25, 10:31 AM
tomek requested review of this revision.Wed, Sep 25, 10:31 AM
tomek updated this revision to Diff 44585.Thu, Sep 26, 12:33 AM

Check message ID conflict for all the operations

tomek updated this revision to Diff 44588.Thu, Sep 26, 12:50 AM

Validate change thread settings operation

Harbormaster completed remote builds in B31867: Diff 44585.Thu, Sep 26, 12:57 AM
tomek updated this revision to Diff 44589.Thu, Sep 26, 12:58 AM

Check create sidebar message conflicts

Harbormaster completed remote builds in B31870: Diff 44588.Thu, Sep 26, 1:07 AM
Harbormaster completed remote builds in B31871: Diff 44589.Thu, Sep 26, 1:15 AM
tomek edited the test plan for this revision. (Show Details)Thu, Sep 26, 2:26 AM
tomek added a child revision: D13483: [lib] Validate DM operations.Thu, Sep 26, 3:14 AM
kamil accepted this revision.Thu, Sep 26, 7:25 AM
kamil added inline comments.
lib/shared/dm-ops/process-dm-ops.js
150–161 ↗(On Diff #44589)

Do you think we can merge these two and use Promise.all?

This revision is now accepted and ready to land.Thu, Sep 26, 7:25 AM
tomek updated this revision to Diff 44621.Fri, Sep 27, 3:21 AM

Use Promise.all

Harbormaster completed remote builds in B31896: Diff 44621.Fri, Sep 27, 3:38 AM
Closed by commit rCOMM082447a2d740: [lib] Check ID conflicts when processing operations (authored by tomek). · Explain WhyFri, Sep 27, 3:54 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMM082447a2d740: [lib] Check ID conflicts when processing operations.

Revision Contents
Changeset List

Diff 44625

View Options

lib/shared/dm-ops/change-thread-settings-spec.js

Loading...
View Options

lib/shared/dm-ops/create-entry-spec.js

Loading...
View Options

lib/shared/dm-ops/create-sidebar-spec.js

Loading...
View Options

lib/shared/dm-ops/create-thread-spec.js

Loading...
View Options

lib/shared/dm-ops/dm-op-spec.js

Loading...
View Options

lib/shared/dm-ops/dm-op-utils.js

Loading...
View Options

lib/shared/dm-ops/process-dm-ops.js

Loading...
View Options

lib/shared/dm-ops/send-edit-message-spec.js

Loading...
View Options

lib/shared/dm-ops/send-multimedia-message-spec.js

Loading...
View Options

lib/shared/dm-ops/send-reaction-message-spec.js

Loading...
View Options

lib/shared/dm-ops/send-text-message-spec.js

Loading...