StreetComplete
StreetComplete copied to clipboard
Multiplatform OSM API client
Requires #5408 to be done first.
The OSM API is a HTTP REST API that communicates chiefly in XML. This must be ported to Kotlin multiplatform.
Currently, the Java library de.westnordost.osmapi
is used for any communication with the OSM API 0.6, i.e. uploading map data, notes and gpx tracks, downloading map data, notes, user data, etc.
The dependency to osmapi
must be replaced with an implementation based on
-
ideally the http client from #5408 and
-
a multiplatform XML parser / writer, which is stable, well-tested and ideally (semi-)official / well-maintained and ideally but not necessarily compatible with
kotlinx-serialization
. This is also important for other XML parsing in the app, i.e. #5369 -
using
kotlinx-datetime
to parse timestamps (already a dependency)
See interfaces TracksApi
, MapDataApi
and NotesApi
XML Parser / Writer requirements
-
multiplatform support for at least Java/Android and iOS
-
stable, well-tested and ideally (semi-)official / well-maintained
xmlutil
There is currently one xml serializer compatible with kotlinx-serialization: io.github.pdvrieze.xmlutil
. It is also mentioned in the Ktor docs, i.e. is the XML serializer/deserializer used in Ktor.
I didn't find any other multiplatform XML parsers/writers.
I would like to pick this up. I will ping back after some basic implementation.
This is blocked by #5408, i.e. that should be done first.
Because, the thinking is the following:
-
Both the access to the OSM API and the other APIs we are accessing should be done with the same library in order to both reduce the number of dependencies and to reduce the amount of brain-work to familiarize oneself with a library
-
It would be possible to write a full kotlin multiplatform or iOS replacement of the Java library de.westnordost.osmapi instead, in a separate repository. de.westnordost.osmapi covers the whole of the OSM API. But re-implementing that library would be considerably more work than just implementing the (few) OSM API calls we actually use, plus, more continuous maintenance work, because the OSM API has been extended over the years, so the expectation is that if a kotlin multplatform or even an iOS version of that library is created, it will be maintained equally well. (And if it is not maintained, then maybe we don't even want to use it in STreetComplete, because we have had bad experiences with using libraries that were badly maintained or were stopped being maintained)
My intention is to build an equivalent of de.westnordost.osmapi so that it can be used in multiplatform. Using Ktor client seems a viable option and that can be extended for other API calls within the project. I understand that the maintenance will take its toll but I beleive Ktor is here to stay. Only caveat we found was with the deserialization of xml. Though ktor provides a library, its only JVM compatible. With io.github.pdvrieze.xmlutil, I think it makes a better sense. Let me know if we have decided on HTTP Client
Cool! Indeed. Yes, xmlutil should work. The only http client that is multiplatform also for native is Ktor Client, as far as I know, so the decision is to use Ktor. OkHttp 5 is only multiplatform jvm+js.
@westnordost we started off simple in an independent repository. I gave you read access. We can move it to any other repo once its in a decent shape
I can have a look at it when it is public, if you like, though bear in mind that I have zero experience with Ktor Client so far. The OSM API on the other hand I know inside-out.
We have come to a decent shape on the api. We may need help in how to publish it for using in other projects. This may be a bit tricky.
Not sure what you mean.
Java/Kotlin FOSS libraries are generally published on maven central via sonatype. If you are not sure what to write into the build gradle, I recently published a multiplatform library too, check https://github.com/westnordost/osm-opening-hours/blob/master/build.gradle.kts If I remember correctly, you need an account at sonatype to be able to publish artifacts, you'll need to read a guide for that because it is too long ago that I did that.
If it is regarding the license: Any open source license would do, permissive or LGPL, even GPLv3 would be fine.
The code is almost ready to be made public. @westnordost need advice on the package name though. right now its com.example.osmapi
. What do you suggest?
in.susrisha.osmapi ?
Or, com.
El 16 de febrero de 2024 11:36:44 CET, Naresh Kumar D @.***> escribió:
The code is almost ready to be made public. @westnordost need advice on the package name though. right now its
com.example.osmapi
. What do you suggest?-- Reply to this email directly or view it on GitHub: https://github.com/streetcomplete/StreetComplete/issues/5410#issuecomment-1948134443 You are receiving this because you were mentioned.
Message ID: @.***>
Its hosted under vindago org. So, we will name it com.vindago.osmapi Thanks for the suggestion
In that case, in.vindago.osmapi would be more correct (as you don't own vindago.com)
El 16 de febrero de 2024 12:30:24 CET, Naresh Kumar D @.***> escribió:
Its hosted under vindago org. So, we will name it com.vindago.osmapi Thanks for the suggestion
-- Reply to this email directly or view it on GitHub: https://github.com/streetcomplete/StreetComplete/issues/5410#issuecomment-1948222717 You are receiving this because you were mentioned.
Message ID: @.***>
@westnordost here is the public link to the API library. its not complete yet but its a start. https://github.com/vindagollc/osm-api/ Do have a look at the dev branch
Ah, cool, interesting! This is actually a fork/port of the Java library, so this clears the license question, it must be licensed under the identical license as the java library. Would you add the license information, too?
I can have a more thorough look through it later. Do you prefer I create issues for everything I find, or rather one issue in which I bundle the "review". Or some other method? It would be nice if there was a way where I could easily comment on and make suggestions on single lines. Maybe make a PR from master to a new empty branch that does not contain anything yet, so everything is a diff?
From a cursory look, it looks quite complete already! I haven't looked at the code in detail, and there seem to be still some open TODOs / WIP, but the overall structure looks good! That must have been quite a lot of work!
Some thoughts:
-
there are still a few example files around, e.g. Greeting.kt. Also, the OsmConnection class has some hardcoded basic authentication in there
-
the tests look incomplete. If you basically copied the Java library, couldn't you reuse (& convert to kotlin) the tests from the Java library instead of implementing them from scratch? If I remember correctly, the test coverage of the Java library has been very good
-
I see despite using kotlinx-serialization / io.github.pdvrieze.xmlutil you parse (and generate?) the XML response from the API by hand. With kotlinx-serialization it is not necessary (nor recommended, as far as I know) to do parsing by hand. You can simply annotate the data model how it should be (de)serialized. See https://kotlinlang.org/docs/serialization.html#serialize-and-deserialize-json for JSON, I would expect it to be somewhat similar for XML. Did you not know this, or is there some reason why you chose to parse it by hand? (Using this annotation-based approach not only reduced the XML parsing code, it should then of course reduce or eliminate the need for tests concerning the XML parsing)
-
Most data classes should probably rather be
data class
es. It is uncommon to haveopen class
es. -
I created the Java library almost ten years ago, in Java. There are a few things I would have done differently today, plus, since Kotlin is another language which has other structures and best practices and you use another library for communication with the OSM Api, it would make sense to do a few things differently architecture-wise. From the top of my head (i.e. incomplete) and without thoroughly thinking about it - so take it with a grain of salt:
- OsmConnection used to hold/bundle the information necessary and the common logic necessary to make calls to the OSM API. I think with ktor http one would basically use HttpClient instead, because on the http client, you can already set the default user agent, default timeout etc. For common logic, static internal functions seem to be the better option.
- The "Handler" classes is a callback-based API. In Kotlin, especially with coroutines, one would use
Flow
s for that instead. Flows is an interesting topic, it's a bit of a read, but they are very interesting and helpful. The Ktor HttpClient being deep in the Kotlin world, I would expect that there is already something that facilitates this - That LatLon is an interface... I don't remember why I did that, probably because there used to be a
Fixed1e7LatLon
implementation to "save space" before I noticed that it was really slow. I think LatLon could just be a data class. - I am not sure if I would have created all those custom exception classes for different HTTP status codes if I reimplemented the library now. It has been a lot of work to maintain to keep up to date with current behavior of the OSM API as new status codes were introduced / documented. I expect it to continue to be an (unnecessary) effort
-
The port/fork looks very similar to the osmapi java library architecture-wise. I wonder if it would be an option to make the KMP version, once finished, the next major version of the java library to reduce maintenance effort overall(?) (no guarantees, not sure how much this library would differ from the original compatibility/migration-wise