readOnlyExternalizableSchemes must not use identity strategy
authorVladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Tue, 19 Jul 2016 10:38:53 +0000 (12:38 +0200)
committerVladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Tue, 19 Jul 2016 10:38:53 +0000 (12:38 +0200)
platform/configuration-store-impl/src/SchemeManagerImpl.kt
plugins/settings-repository/testSrc/LoadTest.kt

index dceef36389f1cadcccae466d1f06b12b71117805..95e92a28c2bcf92ff17be9dc941707afc48b4888 100644 (file)
@@ -71,7 +71,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
   private val schemes: ConcurrentList<T>
     get() = schemesRef.get()
 
-  private val readOnlyExternalizableSchemes = ContainerUtil.newConcurrentMap<String, T>(ContainerUtil.identityStrategy())
+  private val readOnlyExternalizableSchemes = ContainerUtil.newConcurrentMap<String, T>()
 
   /**
    * Schemes can be lazy loaded, so, client should be able to set current scheme by name, not only by instance.
@@ -278,7 +278,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
     if (provider != null && provider.enabled) {
       provider.processChildren(fileSpec, roamingType, { canRead(it) }) { name, input, readOnly ->
         catchAndLog(name) {
-          val scheme = loadScheme(name, input, true)
+          val scheme = loadScheme(name, input, schemes)
           if (readOnly && scheme != null) {
             readOnlyExternalizableSchemes.put(scheme.name, scheme)
           }
@@ -352,11 +352,6 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
   private fun findExternalizableSchemeByFileName(fileName: String) = schemes.firstOrNull { fileName == "${it.fileName}$schemeExtension" } as MUTABLE_SCHEME?
 
   private fun isOverwriteOnLoad(existingScheme: T): Boolean {
-    if (!processor.isExternalizable(existingScheme) || readOnlyExternalizableSchemes.get(existingScheme.name) === existingScheme) {
-      // so, bundled scheme is shadowed
-      return true
-    }
-
     val info = schemeToInfo.get(existingScheme)
     // scheme from file with old extension, so, we must ignore it
     return info != null && schemeExtension != info.fileExtension
@@ -391,13 +386,18 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
       }
 
       schemes.firstOrNull({ it.name == schemeName})?.let { existingScheme ->
-        if (isOverwriteOnLoad(existingScheme)) {
+        if (readOnlyExternalizableSchemes.get(existingScheme.name) === existingScheme) {
+          // so, bundled scheme is shadowed
+          removeFirstScheme({ it === existingScheme }, schemes, scheduleDelete = false)
+          return true
+        }
+        else if (processor.isExternalizable(existingScheme) && isOverwriteOnLoad(existingScheme)) {
           removeFirstScheme({ it === existingScheme }, schemes)
         }
         else {
           if (schemeExtension != extension && schemeToInfo.get(existingScheme as Scheme)?.fileNameWithoutExtension == fileNameWithoutExtension) {
             // 1.oldExt is loading after 1.newExt - we should delete 1.oldExt
-            filesToDelete.add(fileName.toString())
+            filesToDelete.add(fileName)
           }
           else {
             // We don't load scheme with duplicated name - if we generate unique name for it, it will be saved then with new name.
@@ -908,7 +908,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
     removeFirstScheme({ it == scheme }, schemes)
   }
 
-  private fun removeFirstScheme(condition: (T) -> Boolean, schemes: MutableList<T>): T? {
+  private fun removeFirstScheme(condition: (T) -> Boolean, schemes: MutableList<T>, scheduleDelete: Boolean = true): T? {
     val iterator = schemes.iterator()
     for (scheme in iterator) {
       if (!condition(scheme)) {
@@ -921,7 +921,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
 
       iterator.remove()
 
-      if (processor.isExternalizable(scheme)) {
+      if (scheduleDelete && processor.isExternalizable(scheme)) {
         schemeToInfo.remove(scheme)?.scheduleDelete()
       }
       return scheme
index d9420887f264a180880cc380de4bbf266bcfedf0..37dca7caa90739ce555902176ceb705a46751204 100644 (file)
@@ -29,21 +29,21 @@ import org.jetbrains.settingsRepository.git.commit
 import org.junit.ClassRule
 import org.junit.Test
 
+private val dirName = "keymaps"
+
 class LoadTest : IcsTestCase() {
   companion object {
     @JvmField
     @ClassRule val projectRule = ProjectRule()
   }
 
-  private val dirPath = "\$ROOT_CONFIG$/keymaps"
-
   private fun createSchemeManager(dirPath: String) = SchemeManagerImpl(dirPath, TestSchemesProcessor(), provider, tempDirManager.newPath("schemes"))
 
   @Test fun `load scheme`() {
     val localScheme = TestScheme("local")
-    provider.write("$dirPath/local.xml", localScheme.serialize().toByteArray())
+    provider.write("$dirName/local.xml", localScheme.serialize().toByteArray())
 
-    val schemesManager = createSchemeManager(dirPath)
+    val schemesManager = createSchemeManager(dirName)
     schemesManager.loadSchemes()
     assertThat(schemesManager.allSchemes).containsOnly(localScheme)
   }
@@ -51,10 +51,10 @@ class LoadTest : IcsTestCase() {
   @Test fun `load scheme with the same names`() {
     val localScheme = TestScheme("local")
     val data = localScheme.serialize().toByteArray()
-    provider.write("$dirPath/local.xml", data)
-    provider.write("$dirPath/local2.xml", data)
+    provider.write("$dirName/local.xml", data)
+    provider.write("$dirName/local2.xml", data)
 
-    val schemesManager = createSchemeManager(dirPath)
+    val schemesManager = createSchemeManager(dirName)
     schemesManager.loadSchemes()
     assertThat(schemesManager.allSchemes).containsOnly(localScheme)
   }
@@ -62,16 +62,16 @@ class LoadTest : IcsTestCase() {
   @Test fun `load scheme from repo and read-only repo`() {
     val localScheme = TestScheme("local")
 
-    provider.write("$dirPath/local.xml", localScheme.serialize().toByteArray())
+    provider.write("$dirName/local.xml", localScheme.serialize().toByteArray())
 
     val remoteScheme = TestScheme("remote")
     val remoteRepository = tempDirManager.createRepository()
     remoteRepository
-      .add("$dirPath/Mac OS X from RubyMine.xml", remoteScheme.serialize().toByteArray())
+      .add("$dirName/Mac OS X from RubyMine.xml", remoteScheme.serialize().toByteArray())
       .commit("")
 
     remoteRepository.useAsReadOnlySource {
-      val schemesManager = createSchemeManager(dirPath)
+      val schemesManager = createSchemeManager(dirName)
       schemesManager.loadSchemes()
       assertThat(schemesManager.allSchemes).containsOnly(remoteScheme, localScheme)
       assertThat(schemesManager.isMetadataEditable(localScheme)).isTrue()
@@ -83,19 +83,19 @@ class LoadTest : IcsTestCase() {
     val schemeName = "Emacs"
     val localScheme = TestScheme(schemeName, "local")
 
-    provider.write("$dirPath/$schemeName.xml", localScheme.serialize().toByteArray())
+    provider.write("$dirName/$schemeName.xml", localScheme.serialize().toByteArray())
 
     val remoteScheme = TestScheme(schemeName, "remote")
     val remoteRepository = tempDirManager.createRepository("remote")
     remoteRepository
-      .add("$dirPath/$schemeName.xml", remoteScheme.serialize().toByteArray())
+      .add("$dirName/$schemeName.xml", remoteScheme.serialize().toByteArray())
       .commit("")
 
     remoteRepository.useAsReadOnlySource {
-      val schemesManager = createSchemeManager(dirPath)
-      schemesManager.loadSchemes()
-      assertThat(schemesManager.allSchemes).containsOnly(localScheme)
-      assertThat(schemesManager.isMetadataEditable(localScheme)).isFalse()
+      val schemeManager = createSchemeManager(dirName)
+      schemeManager.loadSchemes()
+      assertThat(schemeManager.allSchemes).containsOnly(localScheme)
+      assertThat(schemeManager.isMetadataEditable(localScheme)).isFalse()
     }
   }