From d23b168d1527d0e3ce90eeebaf617d9f14a29b05 Mon Sep 17 00:00:00 2001 From: Nikolay Chashnikov Date: Tue, 20 Oct 2020 14:13:17 +0300 Subject: [PATCH] [workspace model] eclipse: support changing storage format for existing modules (IDEA-253364) When ClasspathStorageProvider is changed, it also updates entity source accordingly. A test is added and tests are updated to manually force save in round-trip tests only. GitOrigin-RevId: a10b0ac0f548a8b6e538b84ef1d91766e7e05330 --- ...ternalSystemModulePropertyManagerBridge.kt | 21 +------ .../roots/impl/storage/ClasspathStorage.java | 9 ++- .../storage/ClasspathStorageProvider.java | 3 + .../module/ModuleManagerComponentBridge.kt | 26 ++++++++ .../ModuleImlFileEntitiesSerializer.kt | 1 + .../EclipseClasspathStorageProvider.java | 33 ++++++++++ .../testData/storageType/default/test.iml | 11 ++++ .../storageType/default/test/.project | 15 +++++ .../storageType/default/test/src/dummy.txt | 0 .../storageType/eclipse/test/.classpath | 6 ++ .../idea/eclipse/ChangeStorageTypeTest.kt | 63 +++++++++++++++++++ .../org/jetbrains/idea/eclipse/testUtils.kt | 25 +++++--- 12 files changed, 181 insertions(+), 32 deletions(-) create mode 100644 plugins/eclipse/testData/storageType/default/test.iml create mode 100644 plugins/eclipse/testData/storageType/default/test/.project create mode 100644 plugins/eclipse/testData/storageType/default/test/src/dummy.txt create mode 100644 plugins/eclipse/testData/storageType/eclipse/test/.classpath create mode 100644 plugins/eclipse/testSources/org/jetbrains/idea/eclipse/ChangeStorageTypeTest.kt diff --git a/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/ExternalSystemModulePropertyManagerBridge.kt b/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/ExternalSystemModulePropertyManagerBridge.kt index dabc0179d397..f9216059ab3a 100644 --- a/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/ExternalSystemModulePropertyManagerBridge.kt +++ b/platform/external-system-impl/src/com/intellij/openapi/externalSystem/service/project/ExternalSystemModulePropertyManagerBridge.kt @@ -14,6 +14,7 @@ import com.intellij.workspaceModel.storage.WorkspaceEntityStorageDiffBuilder import com.intellij.workspaceModel.ide.JpsFileEntitySource import com.intellij.workspaceModel.ide.JpsImportedEntitySource import com.intellij.workspaceModel.ide.WorkspaceModel +import com.intellij.workspaceModel.ide.impl.legacyBridge.module.ModuleManagerComponentBridge import com.intellij.workspaceModel.ide.impl.legacyBridge.module.ModuleManagerComponentBridge.Companion.findModuleEntity import com.intellij.workspaceModel.ide.legacyBridge.ModuleBridge import com.intellij.workspaceModel.storage.bridgeEntities.* @@ -61,25 +62,7 @@ class ExternalSystemModulePropertyManagerBridge(private val module: Module) : Ex val internalFile = entitySource as? JpsFileEntitySource ?: (entitySource as JpsImportedEntitySource).internalFile JpsImportedEntitySource(internalFile, externalSystemId, module.project.isExternalStorageEnabled) } - - fun changeSources(diffBuilder: WorkspaceEntityStorageDiffBuilder, storage: WorkspaceEntityStorage) { - val entitiesMap = storage.entitiesBySource { it == entitySource } - entitiesMap.values.asSequence().flatMap { it.values.asSequence().flatten() }.forEach { - if (it !is FacetEntity) { - diffBuilder.changeSource(it, newSource) - } - } - } - - val diff = module.diff - if (diff != null) { - changeSources(diff, storage) - } - else { - WorkspaceModel.getInstance(module.project).updateProjectModel { builder -> - changeSources(builder, builder) - } - } + ModuleManagerComponentBridge.changeModuleEntitySource(module, newSource) } } diff --git a/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorage.java b/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorage.java index 5350e6012993..db969c63b8c2 100644 --- a/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorage.java +++ b/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorage.java @@ -248,9 +248,12 @@ public final class ClasspathStorage extends StateStorageBase { provider.detach(module); } - provider = getProvider(storageId); - module.setOption(JpsProjectLoader.CLASSPATH_ATTRIBUTE, provider == null ? null : storageId); - module.setOption(JpsProjectLoader.CLASSPATH_DIR_ATTRIBUTE, provider == null ? null : provider.getContentRoot(model)); + ClasspathStorageProvider newProvider = getProvider(storageId); + module.setOption(JpsProjectLoader.CLASSPATH_ATTRIBUTE, newProvider == null ? null : storageId); + module.setOption(JpsProjectLoader.CLASSPATH_DIR_ATTRIBUTE, newProvider == null ? null : newProvider.getContentRoot(model)); + if (newProvider != null) { + newProvider.attach(module); + } } public static void modulePathChanged(@NotNull Module module) { diff --git a/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorageProvider.java b/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorageProvider.java index 2900adbf0598..705c89566994 100644 --- a/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorageProvider.java +++ b/platform/lang-impl/src/com/intellij/openapi/roots/impl/storage/ClasspathStorageProvider.java @@ -34,6 +34,9 @@ public interface ClasspathStorageProvider { void detach(@NotNull Module module); + default void attach(@NotNull Module module) { + } + default void moduleRenamed(@NotNull Module module, @NotNull String oldName, @NotNull String newName) { } diff --git a/platform/lang-impl/src/com/intellij/workspaceModel/ide/impl/legacyBridge/module/ModuleManagerComponentBridge.kt b/platform/lang-impl/src/com/intellij/workspaceModel/ide/impl/legacyBridge/module/ModuleManagerComponentBridge.kt index 3ba270bab37e..cc17470b355b 100644 --- a/platform/lang-impl/src/com/intellij/workspaceModel/ide/impl/legacyBridge/module/ModuleManagerComponentBridge.kt +++ b/platform/lang-impl/src/com/intellij/workspaceModel/ide/impl/legacyBridge/module/ModuleManagerComponentBridge.kt @@ -600,6 +600,8 @@ class ModuleManagerComponentBridge(private val project: Project) : ModuleManager get() = getExternalMapping(INDEX_ID) internal val WorkspaceEntityStorageDiffBuilder.mutableModuleMap: MutableExternalEntityMapping get() = getMutableExternalMapping(INDEX_ID) + + @JvmStatic fun WorkspaceEntityStorage.findModuleEntity(module: ModuleBridge) = moduleMap.getEntities(module).firstOrNull() as ModuleEntity? @@ -613,6 +615,30 @@ class ModuleManagerComponentBridge(private val project: Project) : ModuleManager DFSTBuilder(buildModuleGraph(storage, true)).comparator() } + @JvmStatic + fun changeModuleEntitySource(module: ModuleBridge, newSource: EntitySource) { + val storage = module.entityStorage.current + val oldEntitySource = storage.findModuleEntity(module)?.entitySource ?: return + fun changeSources(diffBuilder: WorkspaceEntityStorageDiffBuilder, storage: WorkspaceEntityStorage) { + val entitiesMap = storage.entitiesBySource { it == oldEntitySource } + entitiesMap.values.asSequence().flatMap { it.values.asSequence().flatten() }.forEach { + if (it !is FacetEntity) { + diffBuilder.changeSource(it, newSource) + } + } + } + + val diff = module.diff + if (diff != null) { + changeSources(diff, storage) + } + else { + WorkspaceModel.getInstance(module.project).updateProjectModel { builder -> + changeSources(builder, builder) + } + } + } + private fun buildModuleGraph(storage: WorkspaceEntityStorage, includeTests: Boolean): Graph { return GraphGenerator.generate(CachingSemiGraph.cache(object : InboundSemiGraph { override fun getNodes(): Collection { diff --git a/platform/workspaceModel/ide/src/com/intellij/workspaceModel/ide/impl/jps/serialization/ModuleImlFileEntitiesSerializer.kt b/platform/workspaceModel/ide/src/com/intellij/workspaceModel/ide/impl/jps/serialization/ModuleImlFileEntitiesSerializer.kt index a8116b972270..18b306e11834 100644 --- a/platform/workspaceModel/ide/src/com/intellij/workspaceModel/ide/impl/jps/serialization/ModuleImlFileEntitiesSerializer.kt +++ b/platform/workspaceModel/ide/src/com/intellij/workspaceModel/ide/impl/jps/serialization/ModuleImlFileEntitiesSerializer.kt @@ -363,6 +363,7 @@ internal open class ModuleImlFileEntitiesSerializer(internal val modulePath: Mod if (serializer != null) { val customDir = moduleOptions[JpsProjectLoader.CLASSPATH_DIR_ATTRIBUTE] serializer.saveRoots(module, entities, writer, customDir, fileUrl, storage, virtualFileManager) + writer.saveComponent(fileUrl.url, MODULE_ROOT_MANAGER_COMPONENT_NAME, null) } else { LOG.warn("Classpath storage provider $customSerializerId not found") diff --git a/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/EclipseClasspathStorageProvider.java b/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/EclipseClasspathStorageProvider.java index 9f251b8580ac..4b6cc6d4b552 100644 --- a/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/EclipseClasspathStorageProvider.java +++ b/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/EclipseClasspathStorageProvider.java @@ -28,6 +28,14 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.VfsUtilCore; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.workspaceModel.ide.JpsFileEntitySource; +import com.intellij.workspaceModel.ide.VirtualFileUrlManagerUtil; +import com.intellij.workspaceModel.ide.WorkspaceModel; +import com.intellij.workspaceModel.ide.impl.legacyBridge.module.ModuleManagerComponentBridge; +import com.intellij.workspaceModel.ide.legacyBridge.ModuleBridge; +import com.intellij.workspaceModel.storage.EntitySource; +import com.intellij.workspaceModel.storage.bridgeEntities.ModuleEntity; +import com.intellij.workspaceModel.storage.url.VirtualFileUrlManager; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -39,6 +47,7 @@ import org.jetbrains.idea.eclipse.conversion.EclipseClasspathWriter; import org.jetbrains.jps.eclipse.model.JpsEclipseClasspathSerializer; import java.io.IOException; +import java.util.function.Function; /** * @author Vladislav.Kaznacheev @@ -90,6 +99,30 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider @Override public void detach(@NotNull Module module) { EclipseModuleManagerImpl.getInstance(module).setDocumentSet(null); + updateEntitySource(module, source -> ((EclipseProjectFile)source).getInternalSource()); + } + + private static void updateEntitySource(Module module, Function updateSource) { + if (WorkspaceModel.isEnabled()) { + WriteAction.run(() -> { + ModuleBridge moduleBridge = (ModuleBridge)module; + ModuleEntity moduleEntity = ModuleManagerComponentBridge.findModuleEntity(moduleBridge.getEntityStorage().getCurrent(), moduleBridge); + if (moduleEntity != null) { + EntitySource entitySource = moduleEntity.getEntitySource(); + ModuleManagerComponentBridge.changeModuleEntitySource(moduleBridge, updateSource.apply(entitySource)); + } + }); + } + } + + @Override + public void attach(@NotNull Module module) { + updateEntitySource(module, source -> { + VirtualFileUrlManager virtualFileUrlManager = VirtualFileUrlManagerUtil.getInstance(VirtualFileUrlManager.Companion, module.getProject()); + String contentRoot = getContentRoot(ModuleRootManager.getInstance(module)); + String classpathFileUrl = VfsUtilCore.pathToUrl(contentRoot) + "/" + EclipseXml.CLASSPATH_FILE; + return new EclipseProjectFile(virtualFileUrlManager.fromUrl(classpathFileUrl), (JpsFileEntitySource)source); + }); } @NotNull diff --git a/plugins/eclipse/testData/storageType/default/test.iml b/plugins/eclipse/testData/storageType/default/test.iml new file mode 100644 index 000000000000..228ac7c7db09 --- /dev/null +++ b/plugins/eclipse/testData/storageType/default/test.iml @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/plugins/eclipse/testData/storageType/default/test/.project b/plugins/eclipse/testData/storageType/default/test/.project new file mode 100644 index 000000000000..4766dbaa2c60 --- /dev/null +++ b/plugins/eclipse/testData/storageType/default/test/.project @@ -0,0 +1,15 @@ + + + test + + + + + org.eclipse.jdt.core.javabuilder + + + + + org.eclipse.jdt.core.javanature + + diff --git a/plugins/eclipse/testData/storageType/default/test/src/dummy.txt b/plugins/eclipse/testData/storageType/default/test/src/dummy.txt new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/plugins/eclipse/testData/storageType/eclipse/test/.classpath b/plugins/eclipse/testData/storageType/eclipse/test/.classpath new file mode 100644 index 000000000000..800ec69ed59a --- /dev/null +++ b/plugins/eclipse/testData/storageType/eclipse/test/.classpath @@ -0,0 +1,6 @@ + + + + + + diff --git a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/ChangeStorageTypeTest.kt b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/ChangeStorageTypeTest.kt new file mode 100644 index 000000000000..5887a21cdba3 --- /dev/null +++ b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/ChangeStorageTypeTest.kt @@ -0,0 +1,63 @@ +// 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 org.jetbrains.idea.eclipse + +import com.intellij.openapi.module.ModuleManager +import com.intellij.openapi.project.Project +import com.intellij.openapi.roots.ModuleRootManager +import com.intellij.openapi.roots.impl.storage.ClassPathStorageUtil +import com.intellij.openapi.roots.impl.storage.ClasspathStorage +import com.intellij.testFramework.ApplicationRule +import com.intellij.testFramework.rules.ProjectModelRule +import com.intellij.testFramework.rules.TempDirectory +import com.intellij.util.io.copy +import com.intellij.util.io.div +import org.jetbrains.jps.eclipse.model.JpsEclipseClasspathSerializer +import org.junit.Assume.assumeTrue +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TestName + +class ChangeStorageTypeTest { + @JvmField + @Rule + val tempDirectory = TempDirectory() + + @JvmField + @Rule + val testName = TestName() + + @Test + fun `switch to default storage`() { + val commonRoot = eclipseTestDataRoot / "common" / "testModuleWithClasspathStorage" + val classpathRoot = eclipseTestDataRoot / "storageType" / "eclipse" + loadEditSaveAndCheck(listOf(commonRoot, classpathRoot), tempDirectory, false, listOf("test" to "test/test"), + { switchStorage(it, ClassPathStorageUtil.DEFAULT_STORAGE) }, + { (eclipseTestDataRoot / "storageType" / "default" / "test.iml").copy(it / "test" / "test.iml")}) + } + + @Test + fun `switch to classpath storage`() { + assumeTrue(ProjectModelRule.isWorkspaceModelEnabled) + val commonRoot = eclipseTestDataRoot / "common" / "testModuleWithClasspathStorage" + val defaultRoot = eclipseTestDataRoot / "storageType" / "default" + loadEditSaveAndCheck(listOf(commonRoot, defaultRoot), tempDirectory, false, listOf("test" to "test/test"), + { switchStorage(it, JpsEclipseClasspathSerializer.CLASSPATH_STORAGE_ID) }, + { + (eclipseTestDataRoot / "common" / "testModuleWithClasspathStorage" / "test.iml").copy(it / "test" / "test.iml") + (eclipseTestDataRoot / "storageType" / "eclipse" / "test" / ".classpath").copy(it / "test" / ".classpath") + }) + } + + private fun switchStorage(project: Project, storageId: String) { + val module = ModuleManager.getInstance(project).modules.single() + ClasspathStorage.setStorageType(ModuleRootManager.getInstance(module), storageId) + } + + companion object { + @JvmField + @ClassRule + val appRule = ApplicationRule() + } + +} \ No newline at end of file diff --git a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/testUtils.kt b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/testUtils.kt index c47d67a2a16a..3d16ce21dce5 100644 --- a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/testUtils.kt +++ b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/testUtils.kt @@ -7,6 +7,9 @@ import com.intellij.openapi.application.runWriteActionAndWait import com.intellij.openapi.components.stateStore import com.intellij.openapi.module.ModuleManager import com.intellij.openapi.project.Project +import com.intellij.openapi.roots.ModuleRootManager +import com.intellij.openapi.roots.impl.storage.ClassPathStorageUtil +import com.intellij.openapi.roots.impl.storage.ClasspathStorage import com.intellij.openapi.util.SystemInfo import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.vfs.VfsUtil @@ -28,7 +31,7 @@ internal fun checkLoadSaveRoundTrip(testDataDirs: List, tempDirectory: TempDirectory, setupPathVariables: Boolean = false, imlFilePaths: List>) { - loadEditSaveAndCheck(testDataDirs, tempDirectory, setupPathVariables, imlFilePaths, {}, {}) + loadEditSaveAndCheck(testDataDirs, tempDirectory, setupPathVariables, imlFilePaths, ::forceSave, {}) } internal fun checkEmlFileGeneration(testDataDirs: List, @@ -48,8 +51,7 @@ internal fun checkConvertToStandardStorage(testDataDirs: List, fun edit(project: Project) { val moduleName = imlFilePaths.first().second.substringAfterLast('/') val module = ModuleManager.getInstance(project).findModuleByName(moduleName) ?: error("Cannot find module '$moduleName'") - module.clearOption(JpsProjectLoader.CLASSPATH_ATTRIBUTE) - module.clearOption(JpsProjectLoader.CLASSPATH_DIR_ATTRIBUTE) + ClasspathStorage.setStorageType(ModuleRootManager.getInstance(module), ClassPathStorageUtil.DEFAULT_STORAGE) } fun updateExpectedDir(projectDir: Path) { @@ -105,13 +107,6 @@ internal fun loadEditSaveAndCheck(testDataDirs: List, loadProject(projectDir) { project -> runWriteActionAndWait { edit(project) - ModuleManager.getInstance(project).modules.forEach { - it.moduleFile!!.delete(this) - it.stateStore.clearCaches() - } - if (WorkspaceModel.isEnabled) { - JpsProjectModelSynchronizer.getInstance(project)!!.markAllEntitiesAsDirty() - } } project.stateStore.save(true) projectDir.assertMatches(directoryContentOf(originalProjectDir), filePathFilter = { path -> @@ -128,4 +123,14 @@ internal fun loadEditSaveAndCheck(testDataDirs: List, PathMacros.getInstance().setMacro(it, null) } } +} + +private fun forceSave(project: Project) { + ModuleManager.getInstance(project).modules.forEach { + it.moduleFile!!.delete(project) + it.stateStore.clearCaches() + } + if (WorkspaceModel.isEnabled) { + JpsProjectModelSynchronizer.getInstance(project)!!.markAllEntitiesAsDirty() + } } \ No newline at end of file