Multiplatform JSON parsing (kotlinx.serialization) (#5442)

* Use kotlinx.serialization in TrafficFlowSegmentsApi * Use kotlinx.serialization in StatisticsParser I checked that API: - does not return trailing commas - returns false, not "false" * Use kotlinx.serialization in StreetCompleteImageUploader * Apply suggestions from code review
streetcomplete · Jan 13, 2024 · c14ab00 · c14ab00
1 parent 3795c26
commit c14ab00
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 101 deletions.
diff --git a/...t/java/de/westnordost/streetcomplete/quests/oneway_suspects/TrafficFlowSegmentsApiTest.kt b/...t/java/de/westnordost/streetcomplete/quests/oneway_suspects/TrafficFlowSegmentsApiTest.kt
@@ -20,12 +20,12 @@ class TrafficFlowSegmentsApiTest {
         val result = TrafficFlowSegmentsApi.parse("""
         {"segments":[
             {"wayId":1,"fromPosition":{"lon":1, "lat":2},"toPosition":{"lon":5, "lat":6}},
-            {"wayId":2,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}},
+            {"wayId":2,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}}
         ]}
         """.trimIndent())
         val expected = mapOf(
-            1L to listOf(TrafficFlowSegment(LatLon(2.0, 1.0), LatLon(6.0, 5.0))),
-            2L to listOf(TrafficFlowSegment(LatLon(4.0, 3.0), LatLon(8.0, 7.0)))
+            1L to listOf(TrafficFlowSegment(1L, LatLon(2.0, 1.0), LatLon(6.0, 5.0))),
+            2L to listOf(TrafficFlowSegment(2L, LatLon(4.0, 3.0), LatLon(8.0, 7.0)))
         )
         assertThat(result).containsAllEntriesOf(expected)
     }
@@ -34,12 +34,12 @@ class TrafficFlowSegmentsApiTest {
         val result = TrafficFlowSegmentsApi.parse("""
         {"segments":[
             {"wayId":1,"fromPosition":{"lon":1, "lat":2},"toPosition":{"lon":5, "lat":6}},
-            {"wayId":1,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}},
+            {"wayId":1,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}}
         ]}
         """.trimIndent())
         val expected = mapOf(1L to listOf(
-            TrafficFlowSegment(LatLon(2.0, 1.0), LatLon(6.0, 5.0)),
-            TrafficFlowSegment(LatLon(4.0, 3.0), LatLon(8.0, 7.0))
+            TrafficFlowSegment(1L, LatLon(2.0, 1.0), LatLon(6.0, 5.0)),
+            TrafficFlowSegment(1L, LatLon(4.0, 3.0), LatLon(8.0, 7.0))
         ))
         assertThat(result).containsAllEntriesOf(expected)
     }

diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploader.kt
@@ -2,18 +2,30 @@ package de.westnordost.streetcomplete.data.osmnotes
 
 import de.westnordost.streetcomplete.ApplicationConstants
 import de.westnordost.streetcomplete.data.download.ConnectionException
-import org.json.JSONException
-import org.json.JSONObject
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
+import kotlinx.serialization.SerializationException
+import kotlinx.serialization.json.Json
 import java.io.File
 import java.io.IOException
 import java.net.HttpURLConnection
 import java.net.URL
 import java.net.URLConnection
 
+
+@Serializable
+private data class PhotoUploadResponse(
+    @SerialName("future_url")
+    val futureUrl: String
+)
+
 /** Upload and activate a list of image paths to an instance of the
  * <a href="https://github.com/exploide/sc-photo-service">StreetComplete image hosting service</a>
  */
 class StreetCompleteImageUploader(private val baseUrl: String) {
+    private val json = Json {
+        ignoreUnknownKeys = true
+    }
 
     /** Upload list of images.
      *
@@ -43,10 +55,9 @@ class StreetCompleteImageUploader(private val baseUrl: String) {
                 if (status == HttpURLConnection.HTTP_OK) {
                     val response = connection.inputStream.bufferedReader().use { it.readText() }
                     try {
-                        val jsonResponse = JSONObject(response)
-                        val url = jsonResponse.getString("future_url")
-                        imageLinks.add(url)
-                    } catch (e: JSONException) {
+                        val parsedResponse = json.decodeFromString<PhotoUploadResponse>(response)
+                        imageLinks.add(parsedResponse.futureUrl)
+                    } catch (e: SerializationException) {
                         throw ImageUploadServerException("Upload Failed: Unexpected response \"$response\"")
                     }
                 } else {

diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsParser.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsParser.kt
@@ -2,70 +2,54 @@ package de.westnordost.streetcomplete.data.user.statistics
 
 import kotlinx.datetime.Instant
 import kotlinx.datetime.LocalDate
-import org.json.JSONObject
+import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.Json.Default.decodeFromString
 
-class StatisticsParser(private val typeAliases: List<Pair<String, String>>) {
-    fun parse(json: String): Statistics {
-        val obj = JSONObject(json)
-
-        val typesStatistics = parseEditTypeStatistics(obj.getJSONObject("questTypes"))
-        val countriesStatistics = parseCountriesStatistics(
-            obj.getJSONObject("countries"),
-            obj.getJSONObject("countryRanks")
-        )
-        val rank = obj.getInt("rank")
-        val daysActive = obj.getInt("daysActive")
-        val isAnalyzing = obj.getBoolean("isAnalyzing")
-        val lastUpdate = Instant.parse(obj.getString("lastUpdate"))
 
-        val currentWeekRank = obj.getInt("currentWeekRank")
-        val currentWeekTypesStatistics = parseEditTypeStatistics(obj.getJSONObject("currentWeekQuestTypes"))
-        val currentWeekCountriesStatistics = parseCountriesStatistics(
-            obj.getJSONObject("currentWeekCountries"),
-            obj.getJSONObject("currentWeekCountryRanks")
-        )
+@Serializable
+private data class StatisticsDTO(
+    val questTypes: Map<String, Int>,
+    val countries: Map<String, Int>,
+    val countryRanks: Map<String, Int>,
+    val rank: Int,
+    val currentWeekRank: Int,
+    val currentWeekQuestTypes: Map<String, Int>,
+    val currentWeekCountries: Map<String, Int>,
+    val currentWeekCountryRanks: Map<String, Int>,
+    val daysActive: Int,
+    val activeDatesRange: Int,
+    val activeDates: List<LocalDate>,
+    val lastUpdate: Instant,
+    val isAnalyzing: Boolean,
+)
 
-        val activeDatesRange = obj.getInt("activeDatesRange")
-        val activeDatesJson = obj.getJSONArray("activeDates")
-        val activeDates = ArrayList<LocalDate>(activeDatesJson.length())
-        for (i in 0 until activeDatesJson.length()) {
-            activeDates.add(LocalDate.parse(activeDatesJson.getString(i)))
-        }
-
-        return Statistics(
-            typesStatistics,
-            countriesStatistics,
-            rank,
-            daysActive,
-            currentWeekRank,
-            currentWeekTypesStatistics,
-            currentWeekCountriesStatistics,
-            activeDatesRange,
-            activeDates,
-            lastUpdate.toEpochMilliseconds(),
-            isAnalyzing,
-        )
-    }
-
-    private fun parseEditTypeStatistics(obj: JSONObject): List<EditTypeStatistics> {
-        val typesByName: MutableMap<String, Int> = mutableMapOf()
-        for (questTypeName in obj.keys()) {
-            typesByName[questTypeName] = obj.getInt(questTypeName)
+class StatisticsParser(private val typeAliases: List<Pair<String, String>>) {
+    fun parse(json: String): Statistics {
+        return with(decodeFromString<StatisticsDTO>(json)) {
+            Statistics(
+                types = parseEditTypeStatistics(questTypes),
+                countries = countries.map { (key, value) ->
+                    CountryStatistics(countryCode = key, count = value, rank = countryRanks[key])
+                }.sortedBy(CountryStatistics::countryCode),
+                rank = rank,
+                daysActive = daysActive,
+                currentWeekRank = currentWeekRank,
+                currentWeekTypes = parseEditTypeStatistics(currentWeekQuestTypes),
+                currentWeekCountries = currentWeekCountries.map { (key, value) ->
+                    CountryStatistics(countryCode = key, count = value, rank = currentWeekCountryRanks[key])
+                }.sortedBy(CountryStatistics::countryCode),
+                activeDatesRange = activeDatesRange,
+                activeDates = activeDates,
+                lastUpdate = lastUpdate.toEpochMilliseconds(),
+                isAnalyzing = isAnalyzing,
+            )
         }
-        mergeTypeAliases(typesByName)
-        return typesByName.map { EditTypeStatistics(it.key, it.value) }
     }
 
-    private fun parseCountriesStatistics(editsObj: JSONObject, ranksObj: JSONObject): List<CountryStatistics> {
-        val countries: MutableMap<String, Int> = mutableMapOf()
-        for (country in editsObj.keys()) {
-            countries[country] = editsObj.getInt(country)
-        }
-        val ranks: MutableMap<String, Int> = mutableMapOf()
-        for (country in ranksObj.keys()) {
-            ranks[country] = ranksObj.getInt(country)
-        }
-        return countries.map { CountryStatistics(it.key, it.value, ranks[it.key]) }
+    private fun parseEditTypeStatistics(input: Map<String, Int>): List<EditTypeStatistics> {
+        val result = input.toMutableMap()
+        mergeTypeAliases(result)
+        return result.map { EditTypeStatistics(it.key, it.value) }
     }
 
     private fun mergeTypeAliases(map: MutableMap<String, Int>) {

diff --git a/...main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegment.kt b/...main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegment.kt
@@ -1,5 +1,11 @@
 package de.westnordost.streetcomplete.quests.oneway_suspects.data
 
 import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
+import kotlinx.serialization.Serializable
 
-data class TrafficFlowSegment(val fromPosition: LatLon, val toPosition: LatLon)
+@Serializable
+data class TrafficFlowSegment(
+    val wayId: Long,
+    val fromPosition: LatLon,
+    val toPosition: LatLon,
+)
diff --git a/.../java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsApi.kt b/.../java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsApi.kt
@@ -1,10 +1,9 @@
 package de.westnordost.streetcomplete.quests.oneway_suspects.data
 
-import android.annotation.SuppressLint
 import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox
-import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
 import de.westnordost.streetcomplete.util.ktx.format
-import org.json.JSONObject
+import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.Json
 import java.net.URL
 
 /** Dao for using this API: https://github.com/ENT8R/oneway-data-api  */
@@ -24,28 +23,14 @@ class TrafficFlowSegmentsApi(private val apiUrl: String) {
     }
 
     companion object {
-        fun parse(json: String): Map<Long, List<TrafficFlowSegment>> {
-            val obj = JSONObject(json)
-            if (!obj.has("segments")) return mapOf()
-
-            val segments = obj.getJSONArray("segments")
-            @SuppressLint("UseSparseArrays")
-            val result = mutableMapOf<Long, MutableList<TrafficFlowSegment>>()
-
-            for (i in 0 until segments.length()) {
-                if (segments.isNull(i)) continue
-                val segment = segments.getJSONObject(i)
-                val wayId = segment.getLong("wayId")
-                result.getOrPut(wayId) { mutableListOf() }.add(
-                    TrafficFlowSegment(
-                        parseLatLon(segment.getJSONObject("fromPosition")),
-                        parseLatLon(segment.getJSONObject("toPosition"))
-                    )
-                )
-            }
-            return result
+        private val json = Json {
+            ignoreUnknownKeys = true
         }
 
-        private fun parseLatLon(pos: JSONObject) = LatLon(pos.getDouble("lat"), pos.getDouble("lon"))
+        @Serializable
+        data class TrafficFlowSegmentList(val segments: List<TrafficFlowSegment>)
+        fun parse(jsonString: String): Map<Long, List<TrafficFlowSegment>> {
+            return json.decodeFromString<TrafficFlowSegmentList>(jsonString).segments.groupBy { it.wayId }
+        }
     }
 }
diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsParserTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsParserTest.kt
@@ -46,25 +46,25 @@ class StatisticsParserTest {
             "questTypes": {
                 "TestQuestTypeA": "11",
                 "TestQuestTypeB": "4",
-                "TestQuestTypeCAlias": "45",
+                "TestQuestTypeCAlias": "45"
             },
             "countries": {
                 "DE": "8",
-                "US": "7",
+                "US": "7"
             },
             "countryRanks": {
-                "US": "123",
+                "US": "123"
             },
             "rank": "2345",
             "currentWeekRank": "3",
             "currentWeekQuestTypes": {
                 "TestQuestTypeA": "9",
                 "TestQuestTypeB": "99",
-                "TestQuestTypeCAlias": "999",
+                "TestQuestTypeCAlias": "999"
             },
             "currentWeekCountries": {
                 "IT": 4,
-                "AT": 5,
+                "AT": 5
             },
             "currentWeekCountryRanks": {
                 "AT": 666
@@ -73,7 +73,7 @@ class StatisticsParserTest {
             "activeDatesRange": "45",
             "activeDates": ["2011-08-07", "2012-12-09"],
             "lastUpdate": "2007-12-03T10:15:30+01:00",
-            "isAnalyzing": "false"
+            "isAnalyzing": false
         }
         """))
     }