Conversation
|
Hi, can you restest this against v0.15.1? Also please make sure the JMAP query tests pass: $ cd stalwart/tests ; STORE=rocksdb cargo test jmap_tests -- --nocapture |
|
Sure, working on upgrading to 0.15 soon, but might be after the new year. |
Two bugs: 1. Returned position is incorrect (always 0, in my tests) 2. Negative anchor offsets are off by one
80ace69 to
98d31a8
Compare
| "N05779", "N04652", "N01534", "A00845", "N03409", "N03410", "N02061", "N02426", | ||
| "N00662", "N01205", | ||
| "T03614", "N05779", "N04652", "N01534", "A00845", "N03409", "N03410", "N02061", | ||
| "N02426", "N00662", // "N01205" anchor is next |
There was a problem hiding this comment.
I'm pretty sure this change shows the bug. From the spec:
anchorOffset: "Int" (default: 0)
The index of the first result to return relative to the index of the anchor, if an anchor is given. This MAY be negative. For example, "-1" means the Foo immediately preceding the anchor is the first result in the list returned (see below for more details).
| ID | Index |
|---|---|
| T03614 | -10 |
| N05779 | -9 |
| N04652 | -8 |
| N01534 | -7 |
| A00845 | -6 |
| N03409 | -5 |
| N03410 | -4 |
| N02061 | -3 |
| N02426 | -2 |
| N00662 | -1 |
| N01205 | anchor |
| }, | ||
| vec!["N01496"], | ||
| vec!["N01496"], | ||
| vec![ |
There was a problem hiding this comment.
This also is correct, again from the spec:
If an "anchor" argument is given, the anchor is looked for in the results after filtering and sorting. If found, the "anchorOffset" is then added to its index. If the resulting index is now negative, it is clamped to 0. This index is now used exactly as though it were supplied as the "position" argument. If the anchor is not found, the call is rejected with an "anchorNotFound" error.
The current behavior doesn't clamp to 0, it reduces the limit instead.
|
@mdecimus Updated - I spent a lot more time on this, my original PR had some issues. Both the new unit tests (that test position) and the existing integration tests are now working - the existing integration tests had a couple issues I've added inline comments for. |
New unit tests demonstrate the bugs if you run them before the change - returned position is wrong, and offsets cause the wrong items to be returned.
This is relevant to section 5.5 of the JMAP spec
I identified a couple bugs:
See inline comments for changes.