changesApplied is rejecting edits to objects that currently have no tags

[matkoniecz]

Continuation of https://github.com/westnordost/StreetComplete/commit/a439af64bc5c60048d28eeeda076b794e951cdb2

(1) https://github.com/westnordost/StreetComplete/blob/ed6ab1426f5c26dc4779c2c84118483a4a84a6eb/app/src/main/java/de/westnordost/streetcomplete/data/osm/osmquest/SingleOsmElementTagChangesUploader.kt#L114 rejects edits done on objects currently without tags

(2) val copy = this.copy() is creating objects where replacing copy.tags with empty hash table is impossible

(3) mutilating https://github.com/westnordost/StreetComplete/blob/ed6ab1426f5c26dc4779c2c84118483a4a84a6eb/app/src/main/java/de/westnordost/streetcomplete/ktx/Element.kt#L8 works but is awful

    var tags = tags?.let { HashMap(it) }

    if (tags == null) {
        tags = HashMap<String, String>()
    }

At least https://www.openstreetmap.org/node/4103244148 finally got edited confirming that I found where bug happens.

(4) slightly less awful mutilating copy that still turn copy into something else would probably work (I have not tested it as it is still unusuably awful anyway)

    val tags = if(tags == null) {
        HashMap<String, String>()
    } else {
        HashMap(tags)
    }

(5) rename copy to make clear that it not only copies, but initialized tags to empty if unset? (still feels like a hack)

(6) probably proper way to change it would be to create all elements with empty HashMap<String, String>() - I just tried to make PR doing this but I was unable to track down where it happens. From searching from code I guess that it may be UpdateElementsHandler ( https://github.com/westnordost/StreetComplete/blob/ed6ab1426f5c26dc4779c2c84118483a4a84a6eb/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UpdateElementsHandler.kt ), but I would need to test it

It seems that migrating elements in database is not necessary, as currently in SC no quest fetches tagless elements.

[westnordost]

Why did you post this as a bug? It is clearly a part of the kerb pull request, isn't it? Or are you seeking advice?

[westnordost]

I think the cleanest solution would be to change in the osmapi library that Element::getTags() can actually never be null. I implemented it like that initially so that not every single element has an empty HashMap because this would make for quite some additional data to be stored.. Hm.

Well, I just noticed that it is possible to create a HashMap with an initial capacity of 0. That would be a solution. new HashMap(0)

[matkoniecz]

Or are you seeking advice?

Yes, I forgot about automated label (it is not a bug a it is not affecting any currently existing quest).

I think the cleanest solution would be to change in the osmapi library

So solution would be too make PR for https://github.com/westnordost/osmapi and bump version there. What is the proper way of testing PR version of osmapi, to test whatever it works as expected before submitting a PR there?

I am looking at https://docs.gradle.org/current/userguide/dependency_management.html#declaring-repositories and while I hoped for ability to specify git repo + commit as source of dependency (to test not-yet-released version). I see only file-based dependency as the closest solution and I wonder whatever I missed a proper solution.

[westnordost]

So solution would be too make PR for https://github.com/westnordost/osmapi and bump version there. What is the proper way of testing PR version of osmapi, to test whatever it works as expected before submitting a PR there?

Yes. There is a test framework with test cases in osmapi. You can simply add tests

[westnordost]

I am looking at https://docs.gradle.org/current/userguide/dependency_management.html#declaring-repositories and while I hoped for ability to specify git repo + commit as source of dependency (to test not-yet-released version). I see only file-based dependency as the closest solution and I wonder whatever I missed a proper solution.

For testing in StreetComplete, you can publish a new version of osmapi to local maven repository (~/.m2/) your publish gradle task IIRC. gradle wil always look in the local maven repository for dependencies first.

[matkoniecz]

gradle publishToMavenLocal after adding apply plugin: 'maven-publish' to build.gradle completed without errors.

It also causes this task to appear on ./gradlew tasks, though running it is not resulting in anything.

After looking at docs it appears that I need to build something

./gradlew clean build publishToMavenLocal is also not really working, as build fails with Execution failed for task ':libs:all:signArchives'. path may not be null or empty string. path=''

It seems to complain about missing data in gradle.properties, I will look into it tomorrow evening.

[matkoniecz]

Also, closing issue as there is no real need to keep it open.