Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update entity conflict logic for additional case #805

Open
matthew-white opened this issue Nov 27, 2024 · 2 comments
Open

Update entity conflict logic for additional case #805

matthew-white opened this issue Nov 27, 2024 · 2 comments
Labels
backend Requires a change to the API server bug entities Multiple Encounter workflows

Comments

@matthew-white
Copy link
Member

matthew-white commented Nov 27, 2024

Right now, there are exactly three situations in which a new entity version is supposed to be marked as a conflict. Two of them are new as of v2024.3.

  1. The base version of an entity update is different from the current version on the server. This is the classic case of an entity conflict.
  2. An offline entity create was delayed, then was later applied as an update. In that case, the update should be marked as a soft conflict.
  3. An entity update was made on an offline branch, but there was another update made from outside the branch between the trunk version of the branch and when the new update from the branch was processed. In other words, the new version is not contiguous with its trunk version: there was an interleaving update. In that case, the new version from the branch is marked as a soft conflict. The most interesting example of this is when the new version immediately follows its base version (i.e., the previous update from the branch). If the new version doesn't immediately follow its base version (if the new version is not the subsequent version), that's already a conflict due to rule 1 above.

Note one important situation in which a new entity version is not marked as a conflict. If an offline update is force-applied from the backlog, that is not considered to be a conflict unless rule 3 applies. For example, let's say I create an entity, then make two offline updates to the entity. I submit both updates, but the first is delayed, so the second ends up being force-applied from the backlog and processed before the first. Assuming that there was no other update from another source, the second update should not be marked as a conflict. Its base version according to the server is v1, and it immediately follows it (1 ✅); it was not a create applied as an update (2 ✅); and it is contiguous with its trunk version (3 ✅). If the first update does eventually arrive, the first update will be marked as a conflict due to rule 1. However, the second update (the one processed first) is not considered to be a conflict.

This issue has to do with a related situation in which a new entity version is marked as a soft conflict when it is not intended to be. This situation came up as part of #698:

  • Create an entity (v1).
  • Download the entity to Collect, then create 4 offline updates to the entity. Don't send the updates yet.
  • Send the 2nd update first. Wait 5 days or force-process the update immediately. The update will be v2 on the server. It is not a conflict: its exact base version was not found, so it was applied on top of the latest version.
  • Now send the 1st update. It will be immediately applied, as its base version (v1) is on the server. The update will be v3 on the server. It is a conflict because there was another version between it and its base version.
  • Now send the 4th update. Wait 5 days or force-process the update immediately. The update will be v4 on the server. It is not a conflict, for the same reasons as v2.

In this example, even though v3 and v4 are from the same branch, and v3 is a conflict, we do not expect v4 to be a conflict. v2, v3, and v4 are all contiguous with the trunk version (v1): there's no conflict with some interweaving update from another source, as in the first example above. To summarize: if the versions from an offline branch were force-processed out of order, then even if all the versions are contiguous with their trunk version, it may be the case that an earlier version from the branch is a conflict while a later version is not.

According to the three rules above, v4 should not be marked as a conflict. However, right now, it is being marked as a conflict. That can be seen in the Backend test here.

@matthew-white matthew-white added bug backend Requires a change to the API server entities Multiple Encounter workflows labels Nov 27, 2024
@github-project-automation github-project-automation bot moved this to 🕒 backlog in ODK Central Nov 27, 2024
@matthew-white
Copy link
Member Author

matthew-white commented Nov 27, 2024

@ktuite and I explored this issue a little while reviewing getodk/central-backend#1187. We ended up suspecting that there was an issue with these lines of code:

add(version) {
  if (version.baseVersion === this.lastContiguousWithTrunk &&
    version.version === version.baseVersion + 1)
    this.lastContiguousWithTrunk = version.version;
}

When this method is called for v3 (the 1st update) in the scenario above, version.baseVersion will be 1, which differs from this.lastContiguousWithTrunk, which will be 2. In other words, add() considers the branch to have been interrupted. What's actually happening is that all the updates are contiguous with the trunk version, but the updates have been applied out of order. In other words, in the normal case where updates are applied in order, add() can tell whether a branch is interrupted, but in the case where an update is force-applied from the backlog, add() breaks down.

add() was originally written for Frontend and was adapted for Backend. If we patch it in Backend, we should probably also patch it in Frontend: there may be a problem there as well. In Frontend, I think lastContiguousWithTrunk is just used to show the accuracy warning ("In this case, the author’s view may not be accurate.").

@matthew-white
Copy link
Member Author

The example above involves four offline updates from the same branch, where two of the updates are force-processed at different times. However, I'm pretty sure that this issue will also come up in simpler cases. For example:

  • Create an entity (v1).
  • Download the entity to Collect, then create 3 offline updates to the entity. Don't send the updates yet.
  • Send the 3rd update first. Wait 5 days or force-process the update immediately. The update will be v2 on the server. It is not a conflict: its exact base version was not found, so it was applied on top of the latest version.
  • Now send the 1st update. It will be immediately applied, as its base version is on the server (v1). The update will be v3 on the server. It is a conflict because there was another version between it and its base version.
  • Now send the 2nd update. It will be immediately applied, as its base version is on the server (v3). The update will be v4 on the server. It should not be a conflict, as it immediately follows its base version and is contiguous with its trunk version. However, right now, it will be marked as a conflict.

I think this issue requires that:

  1. An offline update is force-processed from the backlog.
    • We need the updates from the branch to be contiguous with the trunk version, because otherwise it should be a conflict. Yet to create the issue, we also need the updates to be out of order, which is what force-processing achieves.
  2. A subsequent offline update from the same branch is processed where the base version exists on the server, but is not the latest version.
    • Since the base version exists on the server, this update would not be force-processed. But to create the issue, it would have to come in after force-processing has taken place for the offline update in step 1.
  3. A third update from the same branch is processed. It is this update that would have the incorrect conflict status. No matter what this update is — what order, whether it's force-processed or not — it will be marked as a conflict. But in order for it to be incorrectly marked as a conflict, its base version must be the latest version on the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Requires a change to the API server bug entities Multiple Encounter workflows
Projects
Status: 🕒 backlog
Development

No branches or pull requests

1 participant