Support for easily finding duplicates (#355)

* Add support for easily finding duplicate definitions accross modules via a unit test in the app. * Fix linting errors * Delete unused import * Creating and using duplicatedDeepLinkEntries and allDeepLinkEntries as toplevel functions for better extensibility Co-authored-by: Andreas Rossbacher <[email protected]>
airbnb · Oct 29, 2022 · 5a14e69 · 5a14e69
1 parent 64ecbf3
commit 5a14e69
Show file tree

Hide file tree

Showing 5 changed files with 156 additions and 37 deletions.
diff --git a/deeplinkdispatch-base/src/main/java/com/airbnb/deeplinkdispatch/DeepLinkEntry.kt b/deeplinkdispatch-base/src/main/java/com/airbnb/deeplinkdispatch/DeepLinkEntry.kt
@@ -38,13 +38,64 @@ data class DeepLinkMatchResult(
             "parameters: $parameterMap"
     }
 
+    /**
+     * Whatever template has the first placeholder (and then configurable path segment) is the less
+     * concrete one.
+     * Because if they would have been all in the same index those elements would have been on the
+     * same level and in the same "list" of elements we compare in order.
+     * In this case the one with the more concete element would have won and the same is true here.
+     */
+    override fun compareTo(other: DeepLinkMatchResult): Int {
+        return this.deeplinkEntry.compareTo(other.deeplinkEntry)
+    }
+}
+
+sealed class DeepLinkEntry(open val uriTemplate: String, open val className: String) :
+    Comparable<DeepLinkEntry> {
+
+    data class ActivityDeeplinkEntry(
+        override val uriTemplate: String,
+        override val className: String
+    ) : DeepLinkEntry(uriTemplate, className)
+
+    data class MethodDeeplinkEntry(
+        override val uriTemplate: String,
+        override val className: String,
+        val method: String
+    ) : DeepLinkEntry(uriTemplate, className)
+
+    data class HandlerDeepLinkEntry(
+        override val uriTemplate: String,
+        override val className: String
+    ) : DeepLinkEntry(uriTemplate, className)
+
+    val clazz: Class<*> by lazy {
+        try {
+            Class.forName(className)
+        } catch (e: ClassNotFoundException) {
+            throw IllegalStateException(
+                "Deeplink class $className not found. If you are using Proguard" +
+                    "/R8/Dexguard please consult README.md for correct configuration.",
+                e
+            )
+        }
+    }
+
+    companion object {
+        private val placeholderRegex = "\\{.*?\\}".toRegex()
+    }
+
+    private val uriTemplateWithoutPlaceholders: String by lazy {
+        uriTemplate.replace(placeholderRegex, "")
+    }
+
     private val firstConfigurablePathSegmentIndex: Int by lazy {
-        deeplinkEntry.uriTemplate.indexOf(
+        uriTemplate.indexOf(
             configurablePathSegmentPrefixChar
         )
     }
     private val firstPlaceholderIndex: Int by lazy {
-        deeplinkEntry.uriTemplate.indexOf(
+        uriTemplate.indexOf(
             componentParamPrefixChar
         )
     }
@@ -64,55 +115,26 @@ data class DeepLinkMatchResult(
         }
     }
 
+    fun templatesMatchesSameUrls(other: DeepLinkEntry) = uriTemplateWithoutPlaceholders == other.uriTemplateWithoutPlaceholders
+
     /**
      * Whatever template has the first placeholder (and then configurable path segment) is the less
      * concrete one.
      * Because if they would have been all in the same index those elements would have been on the
      * same level and in the same "list" of elements we compare in order.
      * In this case the one with the more concete element would have won and the same is true here.
      */
-    override fun compareTo(other: DeepLinkMatchResult): Int {
+    override fun compareTo(other: DeepLinkEntry): Int {
         return when {
             this.firstNonConcreteIndex < other.firstNonConcreteIndex -> -1
             this.firstNonConcreteIndex == other.firstNonConcreteIndex -> {
-                if (this.firstNonConcreteIndex == -1 || deeplinkEntry.uriTemplate[firstNonConcreteIndex] == other.deeplinkEntry.uriTemplate[firstNonConcreteIndex]) {
+                if (this.firstNonConcreteIndex == -1 || uriTemplate[firstNonConcreteIndex] == other.uriTemplate[firstNonConcreteIndex]) {
                     0
-                } else if (deeplinkEntry.uriTemplate[firstNonConcreteIndex] == configurablePathSegmentPrefixChar) {
+                } else if (uriTemplate[firstNonConcreteIndex] == configurablePathSegmentPrefixChar) {
                     -1
                 } else 1
             }
             else -> 1
         }
     }
 }
-
-sealed class DeepLinkEntry(open val uriTemplate: String, open val className: String) {
-
-    data class ActivityDeeplinkEntry(
-        override val uriTemplate: String,
-        override val className: String
-    ) : DeepLinkEntry(uriTemplate, className)
-
-    data class MethodDeeplinkEntry(
-        override val uriTemplate: String,
-        override val className: String,
-        val method: String
-    ) : DeepLinkEntry(uriTemplate, className)
-
-    data class HandlerDeepLinkEntry(
-        override val uriTemplate: String,
-        override val className: String
-    ) : DeepLinkEntry(uriTemplate, className)
-
-    val clazz: Class<*> by lazy {
-        try {
-            Class.forName(className)
-        } catch (e: ClassNotFoundException) {
-            throw IllegalStateException(
-                "Deeplink class $className not found. If you are using Proguard" +
-                    "/R8/Dexguard please consult README.md for correct configuration.",
-                e
-            )
-        }
-    }
-}
diff --git a/deeplinkdispatch/src/main/java/com/airbnb/deeplinkdispatch/BaseDeepLinkDelegate.kt b/deeplinkdispatch/src/main/java/com/airbnb/deeplinkdispatch/BaseDeepLinkDelegate.kt
@@ -580,10 +580,37 @@ open class BaseDeepLinkDelegate @JvmOverloads constructor(
      * DeepLinkDelegate.
      */
     val allDeepLinkEntries by lazy {
-        registries.flatMap { it.getAllEntries() }
+        registries.allDeepLinkEntries()
+    }
+
+    /**
+     * Get a map of all DeepLinkEntries and its duplicates, DeepLinkEntry objects that
+     * might be slightly different but will map to the same URL during app operation.
+     */
+    val duplicatedDeepLinkEntries: Map<DeepLinkEntry, List<DeepLinkEntry>> by lazy {
+        registries.duplicatedDeepLinkEntries()
     }
 
     companion object {
         protected const val TAG = "DeepLinkDelegate"
     }
 }
+
+/**
+ * Get a map of all DeepLinkEntries and its duplicates, DeepLinkEntry objects that
+ * might be slightly different but will map to the same URL during app operation.
+ */
+fun List<BaseRegistry>.duplicatedDeepLinkEntries(): Map<DeepLinkEntry, List<DeepLinkEntry>> {
+    val allDeepLinkEntries = this.allDeepLinkEntries()
+    return allDeepLinkEntries.mapNotNull { deepLinkEntry ->
+        allDeepLinkEntries.filter { other ->
+            // Map every DeepLinkEntry to a list of the ones that matches the same URLs (minus itself)
+            deepLinkEntry !== other && deepLinkEntry.templatesMatchesSameUrls(
+                other
+            )
+        }.takeIf { it.isNotEmpty() }?.let { deepLinkEntry to it }
+    }.toMap()
+}
+
+fun List<BaseRegistry>.allDeepLinkEntries(): List<DeepLinkEntry> =
+    this.flatMap { it.getAllEntries() }
diff --git a/sample-library/src/main/java/com/airbnb/deeplinkdispatch/sample/library/LibraryActivity.java b/sample-library/src/main/java/com/airbnb/deeplinkdispatch/sample/library/LibraryActivity.java
@@ -27,6 +27,15 @@ public class LibraryActivity extends AppCompatActivity {
     }
   }
 
+  /**
+   * This will not create an error during index creation but could be found by writing a test
+   * There is another example method in the main sample apps `MainActivity`
+   */
+  @DeepLink("dld://host/intent/{geh}")
+  static Intent sampleDuplicatedUrlWithDifferentPlaceholderNameInLib(Context context) {
+    return null;
+  }
+
   /**
    * This method is a more concrete match for the URI dld://host/somePathOne/somePathTwo/somePathThree
    * to a annotated method in `sample` that is annotated with

diff --git a/sample/src/main/java/com/airbnb/deeplinkdispatch/sample/MainActivity.kt b/sample/src/main/java/com/airbnb/deeplinkdispatch/sample/MainActivity.kt
@@ -245,6 +245,24 @@ object MainActivityDeeplinks {
         return null
     }
 
+    /**
+     * This will not create an error during index creation but could be found by writing a test
+     */
+    @DeepLink("dld://host/intent/{abc}")
+    @JvmStatic
+    fun sampleDuplicatedUrlWithDifferentPlaceholderName1(context: Context?): Intent? {
+        return null
+    }
+
+    /**
+     * This will not create an error during index creation but could be found by writing a test
+     */
+    @DeepLink("dld://host/intent/{def}")
+    @JvmStatic
+    fun sampleDuplicatedUrlWithDifferentPlaceholderName2(context: Context?): Intent? {
+        return null
+    }
+
     /**
      * This method is a less concrete match for the URI
      * dld://host/somePathOne/somePathTwo/somePathThree to a annotated method in `sample-library`

diff --git a/sample/src/test/java/com/airbnb/deeplinkdispatch/sample/AppDeepLinkDelegateTest.kt b/sample/src/test/java/com/airbnb/deeplinkdispatch/sample/AppDeepLinkDelegateTest.kt
@@ -0,0 +1,43 @@
+package com.airbnb.deeplinkdispatch.sample
+
+import com.airbnb.deeplinkdispatch.sample.benchmarkable.BenchmarkDeepLinkModuleRegistry
+import com.airbnb.deeplinkdispatch.sample.kaptlibrary.KaptLibraryDeepLinkModuleRegistry
+import com.airbnb.deeplinkdispatch.sample.library.LibraryDeepLinkModuleRegistry
+import org.hamcrest.MatcherAssert
+import org.hamcrest.core.Is
+import org.hamcrest.core.IsEqual
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+@Config(sdk = [21], manifest = "../sample/src/main/AndroidManifest.xml")
+@RunWith(RobolectricTestRunner::class)
+class AppDeepLinkDelegateTest {
+
+    // Demo test to find duplicate URLs across all modules
+    @Test
+    fun testDuplicatedEntries() {
+        val configurablePlaceholdersMap = mapOf(
+            "configPathOne" to "somePathThree",
+            "configurable-path-segment-one" to "",
+            "configurable-path-segment" to "",
+            "configurable-path-segment-two" to "",
+            "configPathOne" to "somePathOne"
+        )
+        val deepLinkDelegate = DeepLinkDelegate(
+            SampleModuleRegistry(),
+            LibraryDeepLinkModuleRegistry(),
+            BenchmarkDeepLinkModuleRegistry(),
+            KaptLibraryDeepLinkModuleRegistry(),
+            configurablePlaceholdersMap
+        )
+        MatcherAssert.assertThat(
+            deepLinkDelegate.duplicatedDeepLinkEntries.size,
+            IsEqual.equalTo(3)
+        )
+        MatcherAssert.assertThat(deepLinkDelegate.allDeepLinkEntries.any { it.uriTemplate == "dld://host/intent/{abc}" }, Is.`is`(true))
+        MatcherAssert.assertThat(deepLinkDelegate.allDeepLinkEntries.any { it.uriTemplate == "dld://host/intent/{def}" }, Is.`is`(true))
+        MatcherAssert.assertThat(deepLinkDelegate.allDeepLinkEntries.any { it.uriTemplate == "dld://host/intent/{geh}" }, Is.`is`(true))
+    }
+}