StreetComplete icon indicating copy to clipboard operation
StreetComplete copied to clipboard

Multiple Notes created in the same place for the same poi

Open kmpoppe opened this issue 1 year ago • 28 comments

For unknown reasons SC seems to create multiple notes in the same place (coordinates) and for the same poi. Examples:

How to Reproduce unknown

Expected Behavior Before uploading any note, SC could check the MapNotes API with the bounding box reduced to the point that it wants to create the note and warn about a potential duplicate when, after checking the contents of the note, the text would be created for the same node/way/relation as the existing one.

Not only would that reduce the number of open notes in general but also reduce the amount of work contributors have to do checking the same errors over and over again.

Versions affected 45.1, 45.2+ (I believe I saw that also with 50-ish versions but I don't remember the Note IDs)

kmpoppe avatar Mar 01 '23 20:03 kmpoppe

Code is here: https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt#L61-L90

The note can only be uploaded but at the same time not marked as synced (= will be a candidate for upload again) if

  • The Notes API returns an error on upload
  • any listener of uploadedChangeListener throws an exception. Following who listens who, there are in the end only three listeners for that event:
    1. AnswersCounterFragment updates its UI (update count)
    2. UndoButtonFragment updates its UI (enable/disable button)
    3. UploadButtonFragment updates its UI (update count)

The first is out of our control. Regarding the second:

There is potential for that in case the upload happens in the background, there is no UI and there will be some sort of memory leak / race condition. However:

  • all three fragments stop listening for updates on onStop
  • but even if that does not owrk, the updating of the UI happens in the viewLifecycleScope. So, guaranteed always on the UI thread and guaranteed to be not executed if the view lifecycle is over (view has been removed), AFAIK.

So, unless we assume that the view lifecycle management in Android is somewhat bugged (on race conditions or so), I do not see a possibility for this here either.

westnordost avatar Mar 02 '23 13:03 westnordost

Is there a reason keep the position of uploadedChangeListener?.onUploaded(NOTE, edit.position)? If it could be moved to the end, then after a crash the note would not be uploaded again. Though there is some viewLifeCycle code in noteEditsController.markSynced, which might crash the same way before the note is stored locally (noteController.put), leading to the original quest re-appearing.

So, unless we assume that the view lifecycle management in Android is somewhat bugged...

I sometimes get SCEE error reports that hint towards lifecycle issues that should actually be impossible. E.g. Can't access the Fragment View's LifecycleOwner when getView() is null i.e., before onCreateView() or after onDestroyView() in MainFragment.OnUpdated. But the listener is unregistered before states that may lead to the view being destroyed... if the lifecycle callbacks are reliable. Since such things happen only to a few users (but for them more frequently), I assume it's an device / vendor specifc issue with bugged lifecycle management.

Helium314 avatar Mar 02 '23 16:03 Helium314

Maybe another symptom of https://github.com/streetcomplete/StreetComplete/issues/4258 (Being immediately asked the same quest after solving it) ?

matkoniecz avatar Mar 02 '23 16:03 matkoniecz

Maybe another symptom of #4258 (Being immediately asked the same quest after solving it) ?

I've seen symptoms of #4258 and the like, but never for notes. Maybe because I always survey off line and manually upload when I'm back home?

rhhsm avatar Mar 02 '23 17:03 rhhsm

https://www.openstreetmap.org/note/3483111 https://www.openstreetmap.org/note/3483112 may be a hint

https://www.openstreetmap.org/note/3483111 was without a working photo before I closed it

matkoniecz avatar Mar 04 '23 03:03 matkoniecz

Code is here: https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt#L61-L90

The note can only be uploaded but at the same time not marked as synced (= will be a candidate for upload again) if

Is it possible for an app to die for unrelated reasons after upload and before sync? For example phone powering down because it run out of battery? Or app crashing with unrelated exception? Or Android killing app because it run out of memory?

And that causing reuploads?

matkoniecz avatar Mar 04 '23 05:03 matkoniecz

Yes, that is possible. That's always possible. But the timeframe is so small that I don't think it is likely.

westnordost avatar Mar 04 '23 10:03 westnordost

https://www.openstreetmap.org/note/3483111 https://www.openstreetmap.org/note/3483112 may be a hint https://www.openstreetmap.org/note/3483111 was without a working photo before I closed it

What is interesting in that example is that while the rest of the note text and position is the same, the attached pictures have different names: https://www.openstreetmap.org/note/3483111 => https://westnordost.de/p/124774.jpg (but no photo uploaded) https://www.openstreetmap.org/note/3483112 => https://westnordost.de/p/124775.jpg (has uploaded working photo)

Does that help narrow down where it might be dying? (as it is in that path where it generates new URLs for picture, creates and uploads OSM Note containing that picture URL, but before it successfully completes upload of picture to westnordost.de server)

mnalis avatar Mar 04 '23 17:03 mnalis

ref: https://community.openstreetmap.org/t/potential-note-duplicates-worldwide/9517

mnalis avatar Mar 06 '23 12:03 mnalis

I recently had weird crashes when testing with a lot of phone rotating and changing "display size" (dpi). There were two crashes after clicking ok button, like

java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState
	at androidx.fragment.app.FragmentManager.checkStateLoss(FragmentManager.java:1610)
	at androidx.fragment.app.FragmentManager.enqueueAction(FragmentManager.java:1650)
	at androidx.fragment.app.FragmentManager.popBackStack(FragmentManager.java:795)
	at de.westnordost.streetcomplete.screens.main.MainFragment.closeBottomSheet(MainFragment.kt:915)
	at de.westnordost.streetcomplete.screens.main.MainFragment.onEdited(MainFragment.kt:449)
	at de.westnordost.streetcomplete.quests.AbstractOsmQuestForm.solve(AbstractOsmQuestForm.kt:303)

In both cases it looked like a full crash (app disappeared), but when opening again the quest form was still open. Clicking ok now worked, and resulted in the quest being answered a second time. (onEdited is called after the edit is already in the database).

Now obviously this was a quest (the other time it happened for an overlay), but the call to closeBottomSheet also happens when creating notes, and could lead to creation of duplicate notes. Tough I guess the user is able to re-position the note after the crash, and this happened in none of the cases above.

Does that help narrow down where it might be dying?

This fits with the suspicion that the crash happens during uploadedChangeListener?.onUploaded(NOTE, edit.position), which is the only place where a crash would lead to the note being uploaded, but not marked as uploaded in the database.

Helium314 avatar Mar 07 '23 16:03 Helium314

Maybe that's an old MainFragment that should have been removed but is still around due to a memory leak?

(Could find this out by logging the class names of all listeners on crash. There should only be one MainFragment and one AbstractOsmQuestForm)

westnordost avatar Mar 07 '23 17:03 westnordost

https://www.openstreetmap.org/note/3550651 https://www.openstreetmap.org/note/3550652 https://www.openstreetmap.org/note/3550654 - duplication by SC, upload at the same time, different content

Seems to hint toward "Maybe another symptom of https://github.com/streetcomplete/StreetComplete/issues/4258 (Being immediately asked the same quest after solving it)"

matkoniecz avatar Mar 08 '23 13:03 matkoniecz

Seems to hint toward "Maybe another symptom of #4258 (Being immediately asked the same quest after solving it)"

I'm not sure if these by yours truly show that or another bug: 3521192 3521195 Same content but a few minutes between the timestamps (are they creation or upload timestamps)? Not sequential note IDs either, which I assume means slightly different upload times.

peternewman avatar Apr 26 '23 22:04 peternewman

are they creation or upload timestamps

In my experience (but I've been known to be wrong before :smile:) there is no creation timestamp, things get timestamps only at the moment when they get successfully uploaded. (which would explain non-sequential IDs too, as they too get assigned only on upload).

@peternewman would you maybe remember if you had auto-upload enabled when you created those notes?

mnalis avatar Apr 27 '23 00:04 mnalis

@peternewman would you maybe remember if you had auto-upload enabled when you created those notes?

Probably auto but only on Wi-Fi as that’s my normal setting.

peternewman avatar Apr 27 '23 22:04 peternewman

Maybe this issue affects nodes too, see https://github.com/Helium314/SCEE/issues/445, is there any way to find node duplicates? Though this may also be completely unrelated and a SCEE-only bug.

Helium314 avatar Aug 11 '23 08:08 Helium314

is there any way to find node duplicates?

duplicate geometry osmose check seems to do it: https://osmose.openstreetmap.fr/en/map/#zoom=9&lat=47.502&lon=1.991&item=1230%2C9xxx&level=1

possible alternatives

There was this one, but it is gone: https://wiki.openstreetmap.org/wiki/Duplicate_nodes_map - perhaps some of other QA Tools picked it up?

This Atlas check seems to do that too: https://github.com/osmlab/atlas-checks/blob/dev/docs/checks/duplicateNodeCheck.md

There is this, but for smaller areas only: https://github.com/Jkkarr2015/OpenStreetMapDuplicateNodeFinder

mnalis avatar Aug 11 '23 15:08 mnalis

Possible part of the problem could be quests not getting resolved, even if the user did answer, answering it twice and then getting two uploads? See 3906016 & 3906018

K

kmpoppe avatar Oct 02 '23 05:10 kmpoppe

https://www.openstreetmap.org/note/3080883 is interesting case - images were missing (not activated on server?), also before I closed note as duplicate

while repeated https://www.openstreetmap.org/note/3080884 has this images uploaded

matkoniecz avatar Oct 04 '23 10:10 matkoniecz

I haven't seen this before: 3968442 is anonymous, but 3968441 appears to be the 'correct' one (same location, node and text in note).

arrival-spring avatar Nov 01 '23 18:11 arrival-spring

Another case of duplicate notes created: 4179529 and then 12 days later 4197891

When I originally sent the note I was at the airport with spotty WiFi connection. Note duplicate was uploaded when I opened StreetComplete to take another note.

starsep avatar Apr 20 '24 15:04 starsep

What happened here?

https://www.openstreetmap.org/note/4306320, https://www.openstreetmap.org/note/4306321 & https://www.openstreetmap.org/note/4308022, https://www.openstreetmap.org/note/4308023

These are from different users for the exact same places (albeit with different photos) but created within 10/11 seconds frm one another. @westnordost could you check from your server logs, if these users uploaded the pictures to your space from the same IP (which could mean they are two distinct people going out mapping together)?

kmpoppe avatar Jun 27 '24 20:06 kmpoppe

There are no logs.

westnordost avatar Jun 27 '24 20:06 westnordost

It's funny though, the pictures also have been taken a few seconds apart. I'd guess that they were walking together and solving quests, but didn't know that there is a team mode, so they were both solving the same quests.

westnordost avatar Jun 27 '24 20:06 westnordost

Another case of duplicate notes created: 4179529 and then 12 days later 4197891 When I originally sent the note I was at the airport with spotty WiFi connection. Note duplicate was uploaded when I opened StreetComplete to take another note.

Yeah, in my experience duplicates also happens mostly with slow/flakey Internet connection... I would guess SC sends Note create request, notes gets created, but "OK" response from OSM API HTTP server never reaches SC, so SC assumes the note didn't successfully upload, and then tries to create it again at a later time.


Perhaps StreetComplete could verify if the note (with the same text!) already exists on that same location (bbox exact at the notes coordinates) before trying to create note? E.g. for notes quoted in the example above:

https://api.openstreetmap.org/api/0.6/notes/search?display_name=starsep&bbox=6.1033897,46.2359573,6.1033897,46.2359573&closed=-1

would've returned previous note, so StreetComplete (after comparing text are the same, in order to avoid dropping new/different additional note comments etc.) could just delete the note which it was trying to create again, as it would have confirmation that the note was already successfully uploaded.

(that is assuming eventual pictures are already uploaded before the Note gets created in OSM)

Which also got me wondering - do we have the same issue with "create POI"-type Overlays? The same situation might happen there too (create node request sent and processed, but OK response never received)?

mnalis avatar Jun 28 '24 13:06 mnalis

Just a quick one ... 4337559 & 4337695 where OP even mentions that the internet is patchy, the note got added twice with the first one having it's pictures 404'ing. I wonder how that happens? Are photo ids not returned by the server only when it uploaded a picture successfully?

kmpoppe avatar Jul 28 '24 14:07 kmpoppe

The photos have been uploaded successfully, otherwise SC would not know the URLs to the photos.

But on posting the note, SC did get an error reply (or timeout) from the OSM Notes API, which is why it tries later to upload the note.

The pictures in the first note are 404ing because the photos are only copied to their final location on the server when they are "activated". They are activated by telling the server to activate all photos referenced in a certain OSM note. This is a abuse protection so that the photo hosting is bound to OSM notes and cannot be used as some kind of free image hosting service.

westnordost avatar Jul 28 '24 19:07 westnordost

Ah, that sounds like a really clever solution, thanks for the explanation!

kmpoppe avatar Jul 28 '24 19:07 kmpoppe