Improve performance on upload: Only fetch newest element from API if necessary

[westnordost]

Current Behavior

On upload of edits to OSM data,

1. first the app downloads the newest version of the element the edit refers to,
2. then applies the edit to it locally
3. and then uploads the edited element

This means that for every singular edit, two OSM API calls are made.

The Issue

While this makes the implementation easier, the first call the OSM API is avoidable in 99% of cases: If the OSM element hasn't been edited in the meantime since the the app downloaded it, the app could just apply the edit to the locally persisted OSM element and upload that.

Only if the OSM API responds with a HTTP 409 Conflict error, we know that the upload needs to fetch the newest version of that element, apply the edit and try again once.

This improvement not only makes the upload faster, it also avoids unnecessary internet traffic.

Challenges

• oftentimes, multiple edits are made on the same element in a row (e.g. add building type, add housenumber, add building levels). An implementation should keep this in mind - it should not run into HTTP 409 conflicts in cases where the edit "in the meantime" has just been done by the app itself
• the OSM API returns a HTTP 409 conflict error also in case one tries to upload a change in a changeset that as already been closed. So, this case needs to be handled separately

[Helium314]

Some crude sketch:

Current upload (in ElementEditUploader):

• download element
• create updates (or throw if element was changed too much)
• try uploading changes
• ConflictException if changeset was closed or element was changed between download and upload
• on ConflictException, create a new changeset and upload again

Improved upload:

• get element from MapDataController (in ElementEditsUploader)
• forward to ElementEditUploader and create updates from this element
• try uploading changes
• ConflictException if changeset was closed or if element changed since initial download
• on ConflictException, download the element
  • if downloaded element is unchanged, create a new changeset and upload again
  • else create changes with downloaded elements and upload again (or throw if element was changed too much)
    • if this upload results in yet another ConflictException, create a new changeset and upload again

The changes are already persisted to database after each upload, so I think the first Challenge actually is not an issue.

If what I wrote is reasonable, I'll give it a try.

[westnordost]

That sounds reasonable

if downloaded element is unchanged, create a new changeset and upload again

requires that all Elements implement equals correctly and I am not sure if it is a given that the timestamp will be the same(-> Is the locally stored element being updated after upload?)

[Helium314]

Thanks for the hint with the timestamp! Looks like after uploading the first change (and persisting the changed element), the local timestamp differs from timestamp of the element (re-)downloaded after the upload. This is probably because local timestamp is more precise, containing millisecond information instead of always ending with 000.

So how to solve this? Override equals to ignore small timestamp differences? Add some equalsExceptTimestamp? Use less precise timestamps in createUpdates? Something else?

[westnordost]

Actually, you do not need to compare the elements. It should be enough to compare the version attribute, no?

[Helium314]

It should be. But I'm not sure whether this is necessarily true, as I don't know enough about how OSM elements may be changed.

So if you think it's fine, I'll do a simple version check.

[westnordost]

Every time anything is changed on an element, the version number is increased. However, I am not sure how it is done locally (-> Is the locally stored element being updated after upload?)

[Helium314]

Yes it is, see https://github.com/streetcomplete/StreetComplete/blob/a8d915e022ddf1146b54a3a47ef262eec7baa586/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt#L59 This is where I found that after uploading the edit, the only difference between downloaded element and local element is the timestamp.

Anyway, I think a simple way of checking everything except timestamp is using remoteElement?.copy(timestampEdited = 0) == localElement?.copy(timestampEdited = 0) which is not much longer than remoteElement?.version == localElement?.version

[westnordost]

Anyway, I think a simple way of checking everything except timestamp is using remoteElement?.copy(timestampEdited = 0) == localElement?.copy(timestampEdited = 0) which is not much longer than remoteElement?.version == localElement?.version

Well, it is both longer and more difficult to understand.