Skip to content

fix Ranges.insert and add test cases

Harpo requested to merge hr/fix-range-insert into master

I discovered a bug in the implementation of Ranges.insert. This bug surfaces in the case such as insert 10 then insert 5. The case where only a single range currently exists in the list of ranges and a non adjacent smaller value is inserted. In the standard publishing case this does not arise. It does happen in the case where you download a message say logical clock 100 for a system, and then later download a message non sequentially such as logical clock 50. I believe because Grayjay does not actually synchronize all messages for another system for something like a following feed this should not actually surface to users. What it does mean is that if events such as usernames are persisted for other users the persisted process state (which tracks the ranges known for each process locally) is corrupted. However the ranges are only used for bulk synchronization. If it does matter the fix would be to scrub each ProcessState and cause the client to assume it does not have a set of messages for other users, and synchronize (or manually compute the expected values by looking at the event store) them again. As part of this fix I ported over the unit tests from polycentric-core for Ranges.insert.

Edited by Harpo

Merge request reports