[workspace model] correctly save facets which type is unknown (IDEA-252791)

We should save configuration for such facets and use real type ID instead of 'invalid'.

GitOrigin-RevId: 7a6259c38c7d0dfd4947b83c62682f6848155cf4

java/idea-ui/testSrc/com/intellij/facet/FacetTypeUnloadingTest.kt

@@ -2,17 +2,11 @@
 package com.intellij.facet
 
 import com.intellij.facet.impl.invalid.InvalidFacetManager
-import com.intellij.facet.mock.MockFacet
-import com.intellij.facet.mock.MockFacetConfiguration
-import com.intellij.facet.mock.MockFacetType
-import com.intellij.facet.mock.MockSubFacetType
-import com.intellij.openapi.Disposable
+import com.intellij.facet.mock.*
 import com.intellij.openapi.application.runWriteAction
 import com.intellij.openapi.module.impl.ProjectLoadingErrorsHeadlessNotifier
-import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.util.JDOMUtil
 import com.intellij.testFramework.HeavyPlatformTestCase
-import junit.framework.TestCase
 
 class FacetTypeUnloadingTest : HeavyPlatformTestCase() {
   fun `test unload and load facet`() {
@@ -106,30 +100,6 @@ class FacetTypeUnloadingTest : HeavyPlatformTestCase() {
     assertSame(mockFacet, mockSubFacet.underlyingFacet)
   }
 
-  private inline fun <T> runWithRegisteredFacetTypes(vararg types: FacetType<*, *>, action: () -> T): T {
-    val disposable = Disposer.newDisposable()
-    for (type in types) {
-      registerFacetType(type, disposable)
-    }
-
-    try {
-      return action()
-    }
-    finally {
-      Disposer.dispose(disposable)
-    }
-  }
-
-  private fun registerFacetType(type: FacetType<*, *>, disposable: Disposable) {
-    val facetTypeDisposable = Disposer.newDisposable()
-    Disposer.register(disposable, Disposable {
-      runWriteAction {
-        Disposer.dispose(facetTypeDisposable)
-      }
-    })
-    FacetType.EP_NAME.getPoint().registerExtension(type, facetTypeDisposable)
-  }
-
   override fun setUp() {
     super.setUp()
     //initialize facet types and register listeners

java/java-tests/testData/configurationStore/single-facet/.idea/modules.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="ProjectModuleManager">
+    <modules>
+      <module fileurl="file://$PROJECT_DIR$/foo/foo.iml" filepath="$PROJECT_DIR$/foo/foo.iml" />
+    </modules>
+  </component>
+</project>

java/java-tests/testData/configurationStore/single-facet/foo/foo.iml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="EMPTY_MODULE" version="4">
+  <component name="FacetManager">
+    <facet type="MockFacetId" name="MockFacet">
+      <configuration data="my-data" />
+    </facet>
+  </component>
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>

java/java-tests/testSrc/com/intellij/java/configurationStore/SaveFacetsTest.kt

@@ -0,0 +1,60 @@
+// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.java.configurationStore
+
+import com.intellij.facet.FacetManager
+import com.intellij.facet.impl.FacetUtil
+import com.intellij.facet.mock.MockFacetConfiguration
+import com.intellij.facet.mock.MockFacetType
+import com.intellij.facet.mock.registerFacetType
+import com.intellij.facet.mock.runWithRegisteredFacetTypes
+import com.intellij.openapi.application.runWriteActionAndWait
+import com.intellij.openapi.module.impl.ProjectLoadingErrorsHeadlessNotifier
+import com.intellij.testFramework.ApplicationRule
+import com.intellij.testFramework.DisposableRule
+import com.intellij.testFramework.rules.ProjectModelRule
+import com.intellij.util.io.assertMatches
+import com.intellij.util.io.directoryContentOf
+import org.junit.Before
+import org.junit.ClassRule
+import org.junit.Rule
+import org.junit.Test
+
+class SaveFacetsTest {
+  companion object {
+    @JvmField
+    @ClassRule
+    val appRule = ApplicationRule()
+  }
+
+  @Rule
+  @JvmField
+  val projectModel = ProjectModelRule()
+
+  @Rule
+  @JvmField
+  val disposable = DisposableRule()
+
+  @Before
+  fun setUp() {
+    ProjectLoadingErrorsHeadlessNotifier.setErrorHandler({}, disposable.disposable)
+  }
+
+  @Test
+  fun `single facet`() {
+    registerFacetType(MockFacetType(), disposable.disposable)
+    val module = projectModel.createModule("foo")
+    projectModel.addFacet(module, MockFacetType.getInstance(), MockFacetConfiguration("my-data"))
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("single-facet")))
+  }
+
+  @Test
+  fun `single invalid facet`() {
+    val module = projectModel.createModule("foo")
+    runWithRegisteredFacetTypes(MockFacetType()) {
+      projectModel.addFacet(module, MockFacetType.getInstance(), MockFacetConfiguration("my-data"))
+    }
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("single-facet")))
+  }
+}

java/java-tests/testSrc/com/intellij/java/configurationStore/SaveProjectTest.kt

@@ -1,15 +1,12 @@
 // Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.java.configurationStore
 
-import com.intellij.openapi.application.ex.PathManagerEx
 import com.intellij.openapi.application.runReadAction
 import com.intellij.openapi.application.runWriteActionAndWait
-import com.intellij.openapi.components.stateStore
 import com.intellij.openapi.roots.OrderRootType
 import com.intellij.openapi.vfs.VfsUtil
 import com.intellij.testFramework.ApplicationRule
 import com.intellij.testFramework.rules.ProjectModelRule
-import com.intellij.testFramework.runInEdtAndWait
 import com.intellij.util.io.assertMatches
 import com.intellij.util.io.directoryContentOf
 import com.intellij.util.io.systemIndependentPath
@@ -17,8 +14,6 @@ import kotlinx.coroutines.runBlocking
 import org.junit.ClassRule
 import org.junit.Rule
 import org.junit.Test
-import java.nio.file.Path
-import java.nio.file.Paths
 
 /**
  * This class actually doesn't depend on Java. It's located in intellij.java.tests module because if Java plugin is enabled additional elements
@@ -37,14 +32,14 @@ class SaveProjectTest {
   val projectModel = ProjectModelRule()
 
   @Test
-  fun `save single module`() = runBlocking {
+  fun `save single module`() {
     projectModel.createModule("foo")
-    saveProjectState()
-    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(testDataRoot.resolve("single-module")))
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("single-module")))
   }
 
   @Test
-  fun `save module with group`() = runBlocking {
+  fun `save module with group`() {
     val module = projectModel.createModule("foo")
     runWriteActionAndWait {
       val model = projectModel.moduleManager.modifiableModel
@@ -52,43 +47,37 @@ class SaveProjectTest {
       model.commit()
     }
 
-    saveProjectState()
-    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(testDataRoot.resolve("module-in-group")))
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("module-in-group")))
   }
 
   @Test
-  fun `save detached module`() = runBlocking {
+  fun `save detached module`() {
     projectModel.createModule("foo")
     val module = projectModel.createModule("bar")
-    saveProjectState()
+    projectModel.saveProjectState()
     projectModel.removeModule(module)
-    saveProjectState()
-    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(testDataRoot.resolve("detached-module")))
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("detached-module")))
   }
 
   @Test
-  fun `save single library`() = runBlocking {
+  fun `save single library`() {
     projectModel.addProjectLevelLibrary("foo") {
-      it.addRoot(VfsUtil.pathToUrl(projectModel.baseProjectDir.rootPath.resolve("lib/classes").systemIndependentPath), OrderRootType.CLASSES)
+      it.addRoot(VfsUtil.pathToUrl(projectModel.baseProjectDir.rootPath.resolve("lib/classes").systemIndependentPath),
+                 OrderRootType.CLASSES)
     }
-    saveProjectState()
-    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(testDataRoot.resolve("single-library")))
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("single-library")))
   }
 
   @Test
-  fun `save renamed module`() = runBlocking {
+  fun `save renamed module`() {
     val model = runReadAction { projectModel.moduleManager.modifiableModel }
     val module = projectModel.createModule("foo", model)
     model.renameModule(module, "bar")
     runWriteActionAndWait { model.commit() }
-    saveProjectState()
-    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(testDataRoot.resolve("single-module-renamed")))
+    projectModel.saveProjectState()
+    projectModel.baseProjectDir.root.assertMatches(directoryContentOf(configurationStoreTestDataRoot.resolve("single-module-renamed")))
   }
-
-  private suspend fun saveProjectState() {
-    projectModel.project.stateStore.save()
-  }
-
-  private val testDataRoot: Path
-    get() = Paths.get(PathManagerEx.getCommunityHomePath()).resolve("java/java-tests/testData/configurationStore")
 }

java/java-tests/testSrc/com/intellij/java/configurationStore/testUtils.kt

@@ -0,0 +1,16 @@
+// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.java.configurationStore
+
+import com.intellij.openapi.application.ex.PathManagerEx
+import com.intellij.openapi.components.stateStore
+import com.intellij.testFramework.rules.ProjectModelRule
+import kotlinx.coroutines.runBlocking
+import java.nio.file.Path
+import java.nio.file.Paths
+
+internal val configurationStoreTestDataRoot: Path
+  get() = Paths.get(PathManagerEx.getCommunityHomePath()).resolve("java/java-tests/testData/configurationStore")
+
+internal fun ProjectModelRule.saveProjectState() {
+  runBlocking { project.stateStore.save() }
+}

platform/lang-impl/src/com/intellij/facet/impl/FacetTypeRegistryImpl.kt

@@ -53,17 +53,13 @@ class FacetTypeRegistryImpl : FacetTypeRegistry() {
 
   @Synchronized
   override fun unregisterFacetType(facetType: FacetType<*, *>) {
-    try {
-      ProjectManager.getInstance().openProjects.forEach {
-        convertFacetsToInvalid(it, facetType)
-      }
-    }
-    finally {
-      val id = facetType.id
-      val stringId = facetType.stringId
-      LOG.assertTrue(myFacetTypes.remove(id) != null, "Facet type '$stringId' is not registered")
-      myFacetTypes.remove(id)
-      myTypeIds.remove(stringId)
+    val id = facetType.id
+    val stringId = facetType.stringId
+    LOG.assertTrue(myFacetTypes.remove(id) != null, "Facet type '$stringId' is not registered")
+    myFacetTypes.remove(id)
+    myTypeIds.remove(stringId)
+    ProjectManager.getInstance().openProjects.forEach {
+      convertFacetsToInvalid(it, facetType)
     }
   }
 

platform/lang-impl/src/com/intellij/facet/impl/FacetUtil.java

@@ -4,6 +4,7 @@ package com.intellij.facet.impl;
 import com.intellij.configurationStore.ComponentSerializationUtil;
 import com.intellij.configurationStore.XmlSerializer;
 import com.intellij.facet.*;
+import com.intellij.facet.impl.invalid.InvalidFacetConfiguration;
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.application.WriteAction;
 import com.intellij.openapi.components.PersistentStateComponent;
@@ -80,6 +81,9 @@ public class FacetUtil {
         return result == null ? new Element(JpsFacetSerializer.CONFIGURATION_TAG) : result;
       }
     }
+    else if (configuration instanceof InvalidFacetConfiguration) {
+      return ((InvalidFacetConfiguration)configuration).getFacetState().getConfiguration();
+    }
     else {
       final Element config = new Element(JpsFacetSerializer.CONFIGURATION_TAG);
       configuration.writeExternal(config);

platform/lang-impl/src/com/intellij/workspaceModel/ide/impl/legacyBridge/facet/ModifiableFacetModelBridgeImpl.kt

@@ -6,6 +6,7 @@ import com.intellij.facet.FacetManagerImpl
 import com.intellij.facet.ModifiableFacetModel
 import com.intellij.facet.impl.FacetModelBase
 import com.intellij.facet.impl.FacetUtil
+import com.intellij.facet.impl.invalid.InvalidFacet
 import com.intellij.openapi.Disposable
 import com.intellij.openapi.diagnostic.logger
 import com.intellij.openapi.project.isExternalStorageEnabled
@@ -53,7 +54,8 @@ class ModifiableFacetModelBridgeImpl(private val initialStorage: WorkspaceEntity
     }
     val facetConfigurationXml = FacetUtil.saveFacetConfiguration(facet)?.let { JDOMUtil.write(it) }
     val underlyingEntity = facet.underlyingFacet?.let { diff.facetMapping().getEntities(it).single() as FacetEntity }
-    val entity = diff.addFacetEntity(facet.name, facet.type.stringId, facetConfigurationXml, moduleEntity, underlyingEntity, source)
+    val facetTypeId = if (facet !is InvalidFacet) facet.type.stringId else facet.configuration.facetState.facetType
+    val entity = diff.addFacetEntity(facet.name, facetTypeId, facetConfigurationXml, moduleEntity, underlyingEntity, source)
     diff.mutableFacetMapping().addMapping(entity, facet)
     FacetManagerImpl.setExternalSource(facet, externalSource)
     facetsChanged()

platform/testFramework/src/com/intellij/facet/mock/facetTestUtils.kt

@@ -0,0 +1,36 @@
+// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+@file:JvmName("FacetTestUtils")
+package com.intellij.facet.mock
+
+import com.intellij.facet.FacetType
+import com.intellij.openapi.Disposable
+import com.intellij.openapi.application.runWriteAction
+import com.intellij.openapi.application.runWriteActionAndWait
+import com.intellij.openapi.util.Disposer
+
+inline fun <T> runWithRegisteredFacetTypes(vararg types: FacetType<*, *>, action: () -> T): T {
+  val disposable = Disposer.newDisposable()
+  for (type in types) {
+    registerFacetType(type, disposable)
+  }
+
+  try {
+    return action()
+  }
+  finally {
+    Disposer.dispose(disposable)
+  }
+}
+
+
+fun registerFacetType(type: FacetType<*, *>, disposable: Disposable) {
+  val facetTypeDisposable = Disposer.newDisposable()
+  Disposer.register(disposable, Disposable {
+    runWriteActionAndWait {
+      Disposer.dispose(facetTypeDisposable)
+    }
+  })
+  runWriteActionAndWait {
+    FacetType.EP_NAME.point.registerExtension(type, facetTypeDisposable)
+  }
+}

platform/testFramework/src/com/intellij/testFramework/rules/ProjectModelRule.kt

@@ -1,6 +1,10 @@
 // Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.testFramework.rules
 
+import com.intellij.facet.Facet
+import com.intellij.facet.FacetConfiguration
+import com.intellij.facet.FacetManager
+import com.intellij.facet.FacetType
 import com.intellij.openapi.application.runReadAction
 import com.intellij.openapi.application.runWriteActionAndWait
 import com.intellij.openapi.module.EmptyModuleType
@@ -185,6 +189,15 @@ class ProjectModelRule(private val forceEnableWorkspaceModel: Boolean = false) :
     runWriteActionAndWait { moduleManager.disposeModule(module) }
   }
 
+  fun <F: Facet<C>, C: FacetConfiguration> addFacet(module: Module, type: FacetType<F, C>, configuration: C): F {
+    val facetManager = FacetManager.getInstance(module)
+    val model = facetManager.createModifiableModel()
+    val facet = facetManager.createFacet(type, type.defaultFacetName, configuration, null)
+    model.addFacet(facet)
+    runWriteActionAndWait { model.commit() }
+    return facet
+  }
+
   val sdkType: SdkTypeId
     get() = SimpleJavaSdkType.getInstance()