Create an extension, improve the parsing algorithm, add robustness and unit test it

vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt

@@ -20,7 +20,6 @@ import android.net.Uri
 import android.view.View
 import im.vector.app.core.platform.VectorViewModelAction
 import im.vector.app.features.call.conference.ConferenceEvent
-import im.vector.app.features.location.LocationData
 import org.matrix.android.sdk.api.session.content.ContentAttachmentData
 import org.matrix.android.sdk.api.session.room.model.message.MessageStickerContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageWithAttachmentContent
@@ -90,6 +89,7 @@ sealed class RoomDetailAction : VectorViewModelAction {
     data class EnsureNativeWidgetAllowed(val widget: Widget,
                                          val userJustAccepted: Boolean,
                                          val grantedEvents: RoomDetailViewEvents) : RoomDetailAction()
+
     data class UpdateJoinJitsiCallStatus(val conferenceEvent: ConferenceEvent) : RoomDetailAction()
 
     data class OpenOrCreateDm(val userId: String) : RoomDetailAction()

vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt

@@ -171,8 +171,8 @@ import im.vector.app.features.html.EventHtmlRenderer
 import im.vector.app.features.html.PillImageSpan
 import im.vector.app.features.html.PillsPostProcessor
 import im.vector.app.features.invite.VectorInviteView
-import im.vector.app.features.location.LocationData
 import im.vector.app.features.location.LocationSharingMode
+import im.vector.app.features.location.toLocationData
 import im.vector.app.features.media.ImageContentRenderer
 import im.vector.app.features.media.VideoContentRenderer
 import im.vector.app.features.notifications.NotificationDrawerManager
@@ -613,15 +613,12 @@ class RoomDetailFragment @Inject constructor(
     }
 
     private fun handleShowLocationPreview(locationContent: MessageLocationContent, senderId: String) {
-                // TODO Create a helper
-        val geoUri = locationContent.getBestGeoUri()
-        val locationData = LocationData.create(geoUri)
         navigator
                 .openLocationSharing(
                         context = requireContext(),
                         roomId = roomDetailArgs.roomId,
                         mode = LocationSharingMode.PREVIEW,
-                        initialLocationData = locationData,
+                        initialLocationData = locationContent.toLocationData(),
                         locationOwnerId = senderId
                 )
     }
@@ -1948,7 +1945,7 @@ class RoomDetailFragment @Inject constructor(
         when (action.messageContent) {
             is MessageTextContent           -> shareText(requireContext(), action.messageContent.body)
             is MessageLocationContent       -> {
-                LocationData.create(action.messageContent.getBestGeoUri())?.let {
+                action.messageContent.toLocationData()?.let {
                     openLocation(requireActivity(), it.latitude, it.longitude)
                 }
             }

vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewEvents.kt

@@ -20,7 +20,6 @@ import android.net.Uri
 import android.view.View
 import im.vector.app.core.platform.VectorViewEvents
 import im.vector.app.features.call.webrtc.WebRtcCall
-import im.vector.app.features.location.LocationData
 import org.matrix.android.sdk.api.session.widgets.model.Widget
 import org.matrix.android.sdk.api.util.MatrixItem
 import org.matrix.android.sdk.internal.crypto.model.event.WithHeldCode

vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt

@@ -53,7 +53,6 @@ import im.vector.app.features.home.room.detail.sticker.StickerPickerActionHandle
 import im.vector.app.features.home.room.detail.timeline.factory.TimelineFactory
 import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever
 import im.vector.app.features.home.room.typing.TypingHelper
-import im.vector.app.features.location.LocationData
 import im.vector.app.features.powerlevel.PowerLevelsFlowFactory
 import im.vector.app.features.session.coroutineScope
 import im.vector.app.features.settings.VectorDataStore

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt

@@ -40,8 +40,8 @@ import im.vector.app.features.home.room.detail.timeline.tools.createLinkMovement
 import im.vector.app.features.home.room.detail.timeline.tools.linkify
 import im.vector.app.features.html.SpanUtils
 import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
-import im.vector.app.features.location.LocationData
 import im.vector.app.features.location.UrlMapProvider
+import im.vector.app.features.location.toLocationData
 import im.vector.app.features.media.ImageContentRenderer
 import im.vector.lib.core.utils.epoxy.charsequence.toEpoxyCharSequence
 import org.matrix.android.sdk.api.extensions.orFalse
@@ -79,7 +79,7 @@ class MessageActionsEpoxyController @Inject constructor(
         val bindingOptions = spanUtils.getBindingOptions(body)
         val locationUrl = state.timelineEvent()?.root?.getClearContent()
                 ?.toModel<MessageLocationContent>(catchError = true)
-                ?.let { LocationData.create(it.getBestGeoUri()) }
+                ?.toLocationData()
                 ?.let { urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, 1200, 800) }
 
         bottomSheetMessagePreviewItem {

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt

@@ -72,8 +72,8 @@ import im.vector.app.features.html.PillsPostProcessor
 import im.vector.app.features.html.SpanUtils
 import im.vector.app.features.html.VectorHtmlCompressor
 import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
-import im.vector.app.features.location.LocationData
 import im.vector.app.features.location.UrlMapProvider
+import im.vector.app.features.location.toLocationData
 import im.vector.app.features.media.ImageContentRenderer
 import im.vector.app.features.media.VideoContentRenderer
 import im.vector.app.features.settings.VectorPreferences
@@ -200,13 +200,10 @@ class MessageItemFactory @Inject constructor(
                                   informationData: MessageInformationData,
                                   highlight: Boolean,
                                   attributes: AbsMessageItem.Attributes): MessageLocationItem? {
-        val geoUri = locationContent.getBestGeoUri()
-        val locationData = LocationData.create(geoUri)
-
         val width = resources.displayMetrics.widthPixels - dimensionConverter.dpToPx(60)
         val height = dimensionConverter.dpToPx(200)
 
-        val locationUrl = locationData?.let {
+        val locationUrl = locationContent.toLocationData()?.let {
             urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, width, height)
         }
 

vector/src/main/java/im/vector/app/features/location/LocationData.kt

@@ -17,41 +17,44 @@
 package im.vector.app.features.location
 
 import android.os.Parcelable
+import androidx.annotation.VisibleForTesting
 import kotlinx.parcelize.Parcelize
+import org.matrix.android.sdk.api.session.room.model.message.MessageLocationContent
 
 @Parcelize
 data class LocationData(
         val latitude: Double,
         val longitude: Double,
         val uncertainty: Double?
-) : Parcelable {
+) : Parcelable
 
-    companion object {
-
-        /**
-         * Creates location data from geo uri
-         * @param geoUri geo:latitude,longitude;uncertainty
-         * @return location data or null if geo uri is not valid
-         */
-        fun create(geoUri: String): LocationData? {
-            val geoParts = geoUri
-                    .split(":")
-                    .takeIf { it.firstOrNull() == "geo" }
-                    ?.getOrNull(1)
-                    ?.split(",")
-
-            val latitude = geoParts?.firstOrNull()
-            val geoTailParts = geoParts?.getOrNull(1)?.split(";")
-            val longitude = geoTailParts?.firstOrNull()
-            val uncertainty = geoTailParts?.getOrNull(1)?.replace("u=", "")
-
-            return if (latitude != null && longitude != null) {
-                LocationData(
-                        latitude = latitude.toDouble(),
-                        longitude = longitude.toDouble(),
-                        uncertainty = uncertainty?.toDouble()
-                )
-            } else null
-        }
-    }
+/**
+ * Creates location data from a LocationContent
+ * "geo:40.05,29.24;30" -> LocationData(40.05, 29.24, 30)
+ * @return location data or null if geo uri is not valid
+ */
+fun MessageLocationContent.toLocationData(): LocationData? {
+    return parseGeo(getBestGeoUri())
 }
+
+@VisibleForTesting
+fun parseGeo(geo: String): LocationData? {
+    val geoParts = geo
+            .split(":")
+            .takeIf { it.firstOrNull() == "geo" }
+            ?.getOrNull(1)
+            ?.split(";") ?: return null
+
+    val gpsParts = geoParts.getOrNull(0)?.split(",") ?: return null
+    val lat = gpsParts.getOrNull(0)?.toDoubleOrNull() ?: return null
+    val lng = gpsParts.getOrNull(1)?.toDoubleOrNull() ?: return null
+
+    val uncertainty = geoParts.getOrNull(1)?.replace("u=", "")?.toDoubleOrNull()
+
+    return LocationData(
+            latitude = lat,
+            longitude = lng,
+            uncertainty = uncertainty
+    )
+}
+

vector/src/test/java/im/vector/app/features/location/LocationDataTest.kt

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.location
+
+import org.amshove.kluent.shouldBeEqualTo
+import org.amshove.kluent.shouldBeNull
+import org.junit.Test
+
+class LocationDataTest {
+    @Test
+    fun validCases() {
+        parseGeo("geo:12.34,56.78;13.56") shouldBeEqualTo
+                LocationData(latitude = 12.34, longitude = 56.78, uncertainty = 13.56)
+        parseGeo("geo:12.34,56.78") shouldBeEqualTo
+                LocationData(latitude = 12.34, longitude = 56.78, uncertainty = null)
+        // Error is ignored in case of invalid uncertainty
+        parseGeo("geo:12.34,56.78;13.5z6") shouldBeEqualTo
+                LocationData(latitude = 12.34, longitude = 56.78, uncertainty = null)
+        parseGeo("geo:12.34,56.78;13. 56") shouldBeEqualTo
+                LocationData(latitude = 12.34, longitude = 56.78, uncertainty = null)
+        // Space are ignored (trim)
+        parseGeo("geo: 12.34,56.78;13.56") shouldBeEqualTo
+                LocationData(latitude = 12.34, longitude = 56.78, uncertainty = 13.56)
+        parseGeo("geo:12.34,56.78; 13.56") shouldBeEqualTo
+                LocationData(latitude = 12.34, longitude = 56.78, uncertainty = 13.56)
+    }
+
+    @Test
+    fun invalidCases() {
+        parseGeo("").shouldBeNull()
+        parseGeo("geo").shouldBeNull()
+        parseGeo("geo:").shouldBeNull()
+        parseGeo("geo:12.34").shouldBeNull()
+        parseGeo("geo:12.34;13.56").shouldBeNull()
+        parseGeo("gea:12.34,56.78;13.56").shouldBeNull()
+        parseGeo("geo:12.x34,56.78;13.56").shouldBeNull()
+        parseGeo("geo:12.34,56.7y8;13.56").shouldBeNull()
+        // Spaces are not ignored if inside the numbers
+        parseGeo("geo:12.3 4,56.78;13.56").shouldBeNull()
+        parseGeo("geo:12.34,56.7 8;13.56").shouldBeNull()
+        // Or in the protocol part
+        parseGeo(" geo:12.34,56.78;13.56").shouldBeNull()
+        parseGeo("ge o:12.34,56.78;13.56").shouldBeNull()
+        parseGeo("geo :12.34,56.78;13.56").shouldBeNull()
+    }
+}