From ac447039b4299b94cbdca572b3fe49009c3018ba Mon Sep 17 00:00:00 2001
From: Vladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Date: Thu, 26 Jan 2017 15:59:09 +0100
Subject: [PATCH] IDEA-147530 ModifiableModuleModel#newModule should always
 create a new module from scratch

---
 .../configuration-store-impl/src/FileBasedStorage.kt   |  4 +++-
 .../configuration-store-impl/src/ModuleStoreImpl.kt    |  7 ++++++-
 .../src/StateStorageManagerImpl.kt                     | 10 ++++++++--
 .../testSrc/ModuleStoreTest.kt                         | 10 ++++++++++
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/platform/configuration-store-impl/src/FileBasedStorage.kt b/platform/configuration-store-impl/src/FileBasedStorage.kt
index 58b3176b25c5..74db699aa40d 100644
--- a/platform/configuration-store-impl/src/FileBasedStorage.kt
+++ b/platform/configuration-store-impl/src/FileBasedStorage.kt
@@ -58,6 +58,8 @@ open class FileBasedStorage(file: Path,
   private var lineSeparator: LineSeparator? = null
   private var blockSavingTheContent = false
 
+  var resolveVirtualFileOnlyOnWrite = false
+
   @Volatile var file = file
     private set
 
@@ -120,7 +122,7 @@ open class FileBasedStorage(file: Path,
         return loadLocalDataUsingIo()
       }
 
-      val file = virtualFile
+      val file = if (resolveVirtualFileOnlyOnWrite) cachedVirtualFile else virtualFile
       if (file == null || file.isDirectory || !file.isValid) {
         LOG.debug { "Document was not loaded for $fileSpec, not a file" }
       }
diff --git a/platform/configuration-store-impl/src/ModuleStoreImpl.kt b/platform/configuration-store-impl/src/ModuleStoreImpl.kt
index 2c910763e6f7..31a17383f389 100644
--- a/platform/configuration-store-impl/src/ModuleStoreImpl.kt
+++ b/platform/configuration-store-impl/src/ModuleStoreImpl.kt
@@ -71,6 +71,11 @@ abstract class ModuleStoreBase : ComponentStoreImpl() {
       return
     }
 
-    (storageManager.getOrCreateStorage(StoragePathMacros.MODULE_FILE) as FileBasedStorage).setFile(file, null)
+    storageManager.getOrCreateStorage(StoragePathMacros.MODULE_FILE, storageCustomizer = {
+      (this as FileBasedStorage).setFile(file, null)
+      // ModifiableModuleModel#newModule should always create a new module from scratch
+      // https://youtrack.jetbrains.com/issue/IDEA-147530
+      resolveVirtualFileOnlyOnWrite = true
+    })
   }
 }
\ No newline at end of file
diff --git a/platform/configuration-store-impl/src/StateStorageManagerImpl.kt b/platform/configuration-store-impl/src/StateStorageManagerImpl.kt
index bdf5546e0eda..3a46e6d9458f 100644
--- a/platform/configuration-store-impl/src/StateStorageManagerImpl.kt
+++ b/platform/configuration-store-impl/src/StateStorageManagerImpl.kt
@@ -148,11 +148,13 @@ open class StateStorageManagerImpl(private val rootTagName: String,
     return if (path.endsWith('/')) path.substring(0, path.length - 1) else path
   }
 
+  // storageCustomizer - to ensure that other threads will use fully constructed and configured storage (invoked under the same lock as created)
   fun getOrCreateStorage(collapsedPath: String,
                          roamingType: RoamingType = RoamingType.DEFAULT,
                          storageClass: Class<out StateStorage> = StateStorage::class.java,
                          @Suppress("DEPRECATION") stateSplitter: Class<out StateSplitter> = StateSplitterEx::class.java,
-                         exclusive: Boolean = false): StateStorage {
+                         exclusive: Boolean = false,
+                         storageCustomizer: (StateStorage.() -> Unit)? = null): StateStorage {
     val normalizedCollapsedPath = normalizeFileSpec(collapsedPath)
     val key: String
     if (storageClass == StateStorage::class.java) {
@@ -166,7 +168,11 @@ open class StateStorageManagerImpl(private val rootTagName: String,
     }
 
     return storageLock.read { storages.get(key) } ?: storageLock.write {
-      storages.getOrPut(key, { createStateStorage(storageClass, normalizedCollapsedPath, roamingType, stateSplitter, exclusive) })
+      storages.getOrPut(key) {
+        val storage = createStateStorage(storageClass, normalizedCollapsedPath, roamingType, stateSplitter, exclusive)
+        storageCustomizer?.let { storage.it() }
+        storage
+      }
     }
   }
 
diff --git a/platform/configuration-store-impl/testSrc/ModuleStoreTest.kt b/platform/configuration-store-impl/testSrc/ModuleStoreTest.kt
index 964c03838bb5..2e78e7e76ad2 100644
--- a/platform/configuration-store-impl/testSrc/ModuleStoreTest.kt
+++ b/platform/configuration-store-impl/testSrc/ModuleStoreTest.kt
@@ -77,6 +77,16 @@ class ModuleStoreTest {
     }
   }
 
+  @Test fun `newModule should always create a new module from scratch`() {
+    val moduleFile = runWriteAction {
+      VfsTestUtil.createFile(tempDirManager.newVirtualDirectory("module"), "test.iml", "<module type=\"JAVA_MODULE\" foo=\"bar\" version=\"4\" />")
+    }
+
+    Paths.get(moduleFile.path).createModule().useAndDispose {
+      assertThat(getOptionValue("foo")).isNull()
+    }
+  }
+
   @Test fun `must be empty if classpath storage`() {
     // we must not use VFS here, file must not be created
     val moduleFile = tempDirManager.newPath("module", refreshVfs = true).resolve("test.iml")