Major Description
Symptoms
https://github.com/iNPUTmice/jmap/issues/47#issuecomment-816353104 tests by Daniel Gultsch on the LTTRS led to James stucking itself in the email changelog, failing to resynchronise itself:
[
"Email/changes",
{
"accountId": "cf2e474f942d8ea3192028d2e37d5b08b3ddd36fb986d2ad6a19d66277a981c4",
"oldState": "4887d9c1-9707-11eb-b57c-1b93c9e59cb7",
"newState": "4887d9c1-9707-11eb-b57c-1b93c9e59cb7",
"hasMoreChanges": true,
"created": [],
"updated": [],
"destroyed": []
},
"3"
]
This is an issue, as a client will not be able to receive changes past that one, essentially breaking the synchronisation logic.
Explanation
I did easily succeeded to reproduce that behaviour by having the number of items in a single state change exceeding the limit:
@Test
default void test() {
EmailChangeRepository repository = emailChangeRepository();
MessageId messageId = generateNewMessageId();
State state = generateNewState();
EmailChange oldState = EmailChange.builder()
.accountId(ACCOUNT_ID)
.state(state)
.date(DATE.minusHours(1))
.isDelegated(false)
.created(messageId)
.build();
final State newState = generateNewState();
EmailChange change = EmailChange.builder()
.accountId(ACCOUNT_ID)
.state(newState)
.date(DATE)
.isDelegated(false)
.updated(messageId, generateNewMessageId(), generateNewMessageId(), generateNewMessageId(), generateNewMessageId(), generateNewMessageId())
.build();
repository.save(oldState).block();
repository.save(change).block();
System.out.println(repository.getSinceState(ACCOUNT_ID, oldState.getState(), Optional.empty()).block());
}
lead to:
EmailChanges{newState=State{value=ce31b717-edff-4a27-bb28-5482a541c1e0}, hasMoreChanges=true, created=[], updated=[], destroyed=[]}
Translation: the default maximum number of changes (5...) is exceeded by a single entry of the changelog.
The fix?
What to do from here:
- Specify the maxChanges property to override the (stupid) James default
- We should increase this default to IMO at leat 256, we should rely on injections to have a (convenient) lower value in our tests
- We should avoid stucking our selves like that... => we should return canNotCalculateChanges to let it explicitly know to the client.
