From 15f7f62ccda17b17494673aea6676d941e63e381 Mon Sep 17 00:00:00 2001
From: Akash Yadav <itsaky01@gmail.com>
Date: Fri, 28 Jun 2024 07:20:09 +0530
Subject: [PATCH] fix: make ApiVersionsRegistry thread-safe

---
 .../internal/versions/DefaultApiVersions.kt   | 15 ++--
 .../versions/DefaultApiVersionsRegistry.kt    | 89 +++++++++----------
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersions.kt b/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersions.kt
index 346c11221..1dbd3db74 100644
--- a/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersions.kt
+++ b/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersions.kt
@@ -19,14 +19,15 @@ package com.itsaky.androidide.xml.internal.versions
 
 import com.itsaky.androidide.xml.versions.ApiVersion
 import com.itsaky.androidide.xml.versions.ApiVersions
-import org.eclipse.jdt.core.Signature
-import java.util.concurrent.ConcurrentHashMap
 
-/** @author Akash Yadav */
+/**
+ * This implementation is not thread safe. Do not modify concurrently.
+ *
+ * @author Akash Yadav
+ */
 internal class DefaultApiVersions : ApiVersions {
 
-  val classes =
-    ConcurrentHashMap<String, Pair<ApiVersion?, ConcurrentHashMap<String, ApiVersion>>>()
+  val classes = HashMap<String, Pair<ApiVersion?, HashMap<String, ApiVersion>>>()
 
   private fun String.flatten() = replace('.', '/')
 
@@ -57,9 +58,9 @@ internal class DefaultApiVersions : ApiVersions {
   private fun computeClass(
     name: String,
     version: ApiVersion? = null
-  ): Pair<ApiVersion?, ConcurrentHashMap<String, ApiVersion>> {
+  ): Pair<ApiVersion?, HashMap<String, ApiVersion>> {
     return classes.computeIfAbsent(name.flatten()) {
-      version to ConcurrentHashMap()
+      version to hashMapOf()
     }
   }
 }
diff --git a/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersionsRegistry.kt b/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersionsRegistry.kt
index 3285d43dd..c23ecfcc1 100644
--- a/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersionsRegistry.kt
+++ b/xml/utils/src/main/java/com/itsaky/androidide/xml/internal/versions/DefaultApiVersionsRegistry.kt
@@ -33,7 +33,7 @@ import java.util.concurrent.ConcurrentHashMap
  */
 @AutoService(ApiVersionsRegistry::class)
 @VisibleForTesting
-class DefaultApiVersionsRegistry : ApiVersionsParser(), ApiVersionsRegistry {
+class DefaultApiVersionsRegistry : ApiVersionsRegistry {
 
   private val versions = ConcurrentHashMap<String, ApiVersions>()
 
@@ -43,10 +43,6 @@ class DefaultApiVersionsRegistry : ApiVersionsParser(), ApiVersionsRegistry {
 
   override var isLoggingEnabled: Boolean = true
 
-  private var _currentApiVersions: DefaultApiVersions? = null
-  private val currentApiVersions: DefaultApiVersions
-    get() = checkNotNull(_currentApiVersions)
-
   override fun forPlatformDir(platform: File): ApiVersions? {
     var version = versions[platform.path]
     if (version != null) {
@@ -69,50 +65,53 @@ class DefaultApiVersionsRegistry : ApiVersionsParser(), ApiVersionsRegistry {
     }
 
     return versionsFile.inputStream().buffered().use { inputStream ->
-      check(_currentApiVersions == null)
-      _currentApiVersions = DefaultApiVersions()
-      parse(inputStream)
-      currentApiVersions.also {
-        _currentApiVersions = null
-      }
+      // we do not implement the parsing logic in the registry itself for thread safety
+      val versions = DefaultApiVersions()
+      val parser = ApiVersionsParserInternal(versions)
+      parser.parse(inputStream)
+      versions
     }
   }
 
-  override fun isDuplicateClass(name: String): Boolean {
-    return currentApiVersions.containsClass(name)
-  }
-
-  override fun isDuplicateMember(className: String, memberName: String): Boolean {
-    return currentApiVersions.containsClassMember(className, memberName)
-  }
-
-  override fun consumeClassVersionInfo(name: String, apiVersion: ApiVersion) {
-    if (apiVersion.isSinceInception()) {
-      return
-    }
-    currentApiVersions.putClass(name, apiVersion)
-  }
-
-  override fun consumeMemberVersionInfo(
-    className: String,
-    member: String,
-    memberType: String,
-    apiVersion: ApiVersion
-  ) {
-    if (apiVersion.isSinceInception()) {
-      return
-    }
-
-    var identifier = member
-    if (memberType == TAG_METHOD) {
-      // strip return type to save some memory
-      identifier = member.substring(0, member.lastIndexOf(')') + 1)
-    }
-
-    currentApiVersions.putMember(className, identifier, apiVersion)
-  }
-
   override fun clear() {
     versions.clear()
   }
+
+  private class ApiVersionsParserInternal(
+    private val currentApiVersions: DefaultApiVersions
+  ) : ApiVersionsParser() {
+    override fun isDuplicateClass(name: String): Boolean {
+      return currentApiVersions.containsClass(name)
+    }
+
+    override fun isDuplicateMember(className: String, memberName: String): Boolean {
+      return currentApiVersions.containsClassMember(className, memberName)
+    }
+
+    override fun consumeClassVersionInfo(name: String, apiVersion: ApiVersion) {
+      if (apiVersion.isSinceInception()) {
+        return
+      }
+      currentApiVersions.putClass(name, apiVersion)
+    }
+
+    override fun consumeMemberVersionInfo(
+      className: String,
+      member: String,
+      memberType: String,
+      apiVersion: ApiVersion
+    ) {
+      if (apiVersion.isSinceInception()) {
+        return
+      }
+
+      var identifier = member
+      if (memberType == TAG_METHOD) {
+        // strip return type to save some memory
+        identifier = member.substring(0, member.lastIndexOf(')') + 1)
+      }
+
+      currentApiVersions.putMember(className, identifier, apiVersion)
+    }
+  }
 }