From 5f277b64723ab09f2e768dc70fda566ddaa1895a Mon Sep 17 00:00:00 2001 From: nik Date: Mon, 18 Jun 2012 09:53:54 +0400 Subject: [PATCH] get rid of unnecessary creations of ModifiableRootModel --- .../roots/impl/storage/ClasspathStorage.java | 20 ++++---- .../storage/ClasspathStorageProvider.java | 4 +- .../EclipseClasspathStorageProvider.java | 29 ++++++++---- .../export/ExportEclipseProjectsAction.java | 47 +++++++++---------- .../idea/eclipse/EclipseClasspathTest.java | 7 +-- .../idea/eclipse/EclipseEmlTest.java | 16 +++---- .../idea/eclipse/EclipseImlTest.java | 15 +++--- 7 files changed, 70 insertions(+), 68 deletions(-) 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 e4c70d8f3651..b3497c61fcf3 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 @@ -80,7 +80,8 @@ public class ClasspathStorage implements StateStorage { public ClasspathStorage(Module module) { myConverter = getProvider(getStorageType(module)).createConverter(module); final MessageBus messageBus = module.getMessageBus(); - final VirtualFileTracker virtualFileTracker = (VirtualFileTracker)module.getPicoContainer().getComponentInstanceOfType(VirtualFileTracker.class); + final VirtualFileTracker virtualFileTracker = + (VirtualFileTracker)module.getPicoContainer().getComponentInstanceOfType(VirtualFileTracker.class); if (virtualFileTracker != null && messageBus != null) { final ArrayList files = new ArrayList(); try { @@ -125,7 +126,7 @@ public class ClasspathStorage implements StateStorage { model.dispose(); } } - + final boolean macrosOk = ProjectMacrosUtil.checkNonIgnoredMacros(module.getProject(), macros); PathMacroManager.getInstance(module).expandPaths(element); ModuleRootManagerImpl.ModuleRootManagerState moduleRootManagerState = new ModuleRootManagerImpl.ModuleRootManagerState(); @@ -195,7 +196,7 @@ public class ClasspathStorage implements StateStorage { } @Nullable - public Set analyzeExternalChanges(final Set> changedFiles) { + public Set analyzeExternalChanges(final Set> changedFiles) { return null; } @@ -235,7 +236,8 @@ public class ClasspathStorage implements StateStorage { public void finishSave(final SaveSession saveSession) { try { LOG.assertTrue(mySession == saveSession); - } finally { + } + finally { mySession = null; } } @@ -305,7 +307,7 @@ public class ClasspathStorage implements StateStorage { } } - public static void setStorageType(final ModifiableRootModel model, final String storageID) { + public static void setStorageType(final ModuleRootModel model, final String storageID) { final Module module = model.getModule(); final String oldStorageType = getStorageType(module); if (oldStorageType.equals(storageID)) { @@ -343,7 +345,7 @@ public class ClasspathStorage implements StateStorage { return DEFAULT_STORAGE_DESCR; } - public void assertCompatible(final ModifiableRootModel model) throws ConfigurationException { + public void assertCompatible(final ModuleRootModel model) throws ConfigurationException { } public void detach(Module module) { @@ -357,7 +359,7 @@ public class ClasspathStorage implements StateStorage { throw new UnsupportedOperationException(getDescription()); } - public String getContentRoot(ModifiableRootModel model) { + public String getContentRoot(ModuleRootModel model) { return null; } @@ -383,7 +385,7 @@ public class ClasspathStorage implements StateStorage { return "Unsupported classpath format " + myType; } - public void assertCompatible(final ModifiableRootModel model) throws ConfigurationException { + public void assertCompatible(final ModuleRootModel model) throws ConfigurationException { throw new UnsupportedOperationException(getDescription()); } @@ -411,7 +413,7 @@ public class ClasspathStorage implements StateStorage { }; } - public String getContentRoot(ModifiableRootModel model) { + public String getContentRoot(ModuleRootModel model) { return null; } 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 20935866199b..b00df210303d 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 @@ -43,7 +43,7 @@ public interface ClasspathStorageProvider { @Nls String getDescription(); - void assertCompatible(final ModifiableRootModel model) throws ConfigurationException; + void assertCompatible(final ModuleRootModel model) throws ConfigurationException; void detach(Module module); @@ -51,7 +51,7 @@ public interface ClasspathStorageProvider { ClasspathConverter createConverter(Module module); - String getContentRoot(ModifiableRootModel model); + String getContentRoot(ModuleRootModel model); void modulePathChanged(Module module, String path); 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 86867b40a344..eff535917510 100644 --- a/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/EclipseClasspathStorageProvider.java +++ b/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/EclipseClasspathStorageProvider.java @@ -63,7 +63,7 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider return DESCR; } - public void assertCompatible(final ModifiableRootModel model) throws ConfigurationException { + public void assertCompatible(final ModuleRootModel model) throws ConfigurationException { final String moduleName = model.getModule().getName(); for (OrderEntry entry : model.getOrderEntries()) { if (entry instanceof LibraryOrderEntry) { @@ -74,7 +74,11 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider libraryEntry.getRootUrls(OrderRootType.CLASSES).length != 1 || library.isJarDirectory(library.getUrls(OrderRootType.CLASSES)[0])) { throw new ConfigurationException( - "Library \'" + entry.getPresentableName() + "\' from module \'" + moduleName + "\' dependencies is incompatible with eclipse format which supports only one library content root"); + "Library \'" + + entry.getPresentableName() + + "\' from module \'" + + moduleName + + "\' dependencies is incompatible with eclipse format which supports only one library content root"); } } } @@ -84,8 +88,12 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider } final String output = model.getModuleExtension(CompilerModuleExtension.class).getCompilerOutputUrl(); final String contentRoot = getContentRoot(model); - if (output == null || !StringUtil.startsWith(VfsUtil.urlToPath(output), contentRoot) && PathMacroManager.getInstance(model.getModule()).collapsePath(output).equals(output)) { - throw new ConfigurationException("Module \'" + moduleName + "\' output path is incompatible with eclipse format which supports output under content root only.\nPlease make sure that \"Inherit project compile output path\" is not selected"); + if (output == null || + !StringUtil.startsWith(VfsUtil.urlToPath(output), contentRoot) && + PathMacroManager.getInstance(model.getModule()).collapsePath(output).equals(output)) { + throw new ConfigurationException("Module \'" + + moduleName + + "\' output path is incompatible with eclipse format which supports output under content root only.\nPlease make sure that \"Inherit project compile output path\" is not selected"); } } @@ -97,7 +105,7 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider return new EclipseClasspathConverter(module); } - public String getContentRoot(ModifiableRootModel model) { + public String getContentRoot(ModuleRootModel model) { final VirtualFile contentRoot = EPathUtil.getContentRoot(model); if (contentRoot != null) return contentRoot.getPath(); return model.getContentRoots()[0].getPath(); @@ -111,7 +119,10 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider } } - public static void registerFiles(final CachedXmlDocumentSet fileCache, final Module module, final String moduleRoot, final String storageRoot) { + public static void registerFiles(final CachedXmlDocumentSet fileCache, + final Module module, + final String moduleRoot, + final String storageRoot) { fileCache.register(EclipseXml.CLASSPATH_FILE, storageRoot); fileCache.register(EclipseXml.PROJECT_FILE, storageRoot); fileCache.register(EclipseXml.PLUGIN_XML_FILE, storageRoot); @@ -185,13 +196,15 @@ public class EclipseClasspathStorageProvider implements ClasspathStorageProvider if (documentSet.exists(EclipseXml.CLASSPATH_FILE)) { classpathReader.readClasspath(model, new ArrayList(), new ArrayList(), usedVariables, new HashSet(), null, documentSet.read(EclipseXml.CLASSPATH_FILE).getRootElement()); - } else { + } + else { EclipseClasspathReader.setupOutput(model, path + "/bin"); } final String eml = model.getModule().getName() + EclipseXml.IDEA_SETTINGS_POSTFIX; if (documentSet.exists(eml)) { IdeaSpecificSettings.readIDEASpecific(documentSet.read(eml).getRootElement(), model); - } else { + } + else { model.getModuleExtension(CompilerModuleExtension.class).setExcludeOutput(false); } diff --git a/plugins/eclipse/src/org/jetbrains/idea/eclipse/export/ExportEclipseProjectsAction.java b/plugins/eclipse/src/org/jetbrains/idea/eclipse/export/ExportEclipseProjectsAction.java index 39d6f9f1d06e..58b879143bba 100644 --- a/plugins/eclipse/src/org/jetbrains/idea/eclipse/export/ExportEclipseProjectsAction.java +++ b/plugins/eclipse/src/org/jetbrains/idea/eclipse/export/ExportEclipseProjectsAction.java @@ -24,8 +24,8 @@ import com.intellij.openapi.module.ModuleManager; import com.intellij.openapi.options.ConfigurationException; import com.intellij.openapi.project.DumbAware; import com.intellij.openapi.project.Project; -import com.intellij.openapi.roots.ModifiableRootModel; import com.intellij.openapi.roots.ModuleRootManager; +import com.intellij.openapi.roots.ModuleRootModel; import com.intellij.openapi.roots.impl.storage.ClasspathStorage; import com.intellij.openapi.ui.DialogWrapper; import com.intellij.openapi.ui.Messages; @@ -53,61 +53,62 @@ public class ExportEclipseProjectsAction extends AnAction implements DumbAware { public void update(final AnActionEvent e) { final Project project = e.getData(PlatformDataKeys.PROJECT); - e.getPresentation().setEnabled( project != null ); + e.getPresentation().setEnabled(project != null); } public void actionPerformed(AnActionEvent e) { final Project project = e.getData(PlatformDataKeys.PROJECT); - if ( project == null ) return; + if (project == null) return; project.save(); // to flush iml files final List modules = new ArrayList(); final List incompatibleModules = new ArrayList(); for (Module module : ModuleManager.getInstance(project).getModules()) { if (!EclipseClasspathStorageProvider.ID.equals(ClasspathStorage.getStorageType(module))) { - final ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); try { - ClasspathStorage.getProvider(EclipseClasspathStorageProvider.ID).assertCompatible(model); + ClasspathStorage.getProvider(EclipseClasspathStorageProvider.ID).assertCompatible(ModuleRootManager.getInstance(module)); modules.add(module); } catch (ConfigurationException e1) { incompatibleModules.add(module); - } finally { - model.dispose(); } } } //todo suggest smth with hierarchy modules if (!incompatibleModules.isEmpty()) { - if (Messages.showOkCancelDialog(project, "Eclipse incompatible modules found:

  • " + StringUtil.join(incompatibleModules, new Function() { - public String fun(Module module) { - return module.getName(); - } - }, "
  • ") + "

Would you like to proceed and possibly lose your configurations?", "Eclipse Incompatible Modules Found", Messages.getWarningIcon()) != DialogWrapper.OK_EXIT_CODE) { + if (Messages.showOkCancelDialog(project, "Eclipse incompatible modules found:

  • " + + StringUtil.join(incompatibleModules, new Function() { + public String fun(Module module) { + return module.getName(); + } + }, "
  • ") + + "

Would you like to proceed and possibly lose your configurations?", + "Eclipse Incompatible Modules Found", Messages.getWarningIcon()) != DialogWrapper.OK_EXIT_CODE) { return; } - } else if (modules.isEmpty()){ - Messages.showInfoMessage(project, EclipseBundle.message("eclipse.export.nothing.to.do"), EclipseBundle.message("eclipse.export.dialog.title")); + } + else if (modules.isEmpty()) { + Messages.showInfoMessage(project, EclipseBundle.message("eclipse.export.nothing.to.do"), + EclipseBundle.message("eclipse.export.dialog.title")); return; } modules.addAll(incompatibleModules); final ExportEclipseProjectsDialog dialog = new ExportEclipseProjectsDialog(project, modules); - dialog.show (); - if(dialog.isOK()){ + dialog.show(); + if (dialog.isOK()) { if (dialog.isLink()) { for (Module module : dialog.getSelectedModules()) { - final ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); - ClasspathStorage.setStorageType(model, EclipseClasspathStorageProvider.ID); - model.dispose(); + ClasspathStorage.setStorageType(ModuleRootManager.getInstance(module), EclipseClasspathStorageProvider.ID); } } else { for (Module module : dialog.getSelectedModules()) { - final ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); + final ModuleRootModel model = ModuleRootManager.getInstance(module); final VirtualFile[] contentRoots = model.getContentRoots(); //todo - final String storageRoot = contentRoots.length == 1 ? contentRoots[0].getPath() : ClasspathStorage.getStorageRootFromOptions(module); + final String storageRoot = + contentRoots.length == 1 ? contentRoots[0].getPath() : ClasspathStorage.getStorageRootFromOptions(module); try { final Element classpathEleemnt = new Element(EclipseXml.CLASSPATH_TAG); @@ -135,9 +136,6 @@ public class ExportEclipseProjectsAction extends AnAction implements DumbAware { catch (WriteExternalException e1) { LOG.error(e1); } - finally { - model.dispose(); - } } } try { @@ -149,5 +147,4 @@ public class ExportEclipseProjectsAction extends AnAction implements DumbAware { project.save(); } } - } diff --git a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseClasspathTest.java b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseClasspathTest.java index e2e67229c90f..ec5305cf7786 100644 --- a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseClasspathTest.java +++ b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseClasspathTest.java @@ -29,6 +29,7 @@ import com.intellij.openapi.options.ConfigurationException; import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.ModifiableRootModel; import com.intellij.openapi.roots.ModuleRootManager; +import com.intellij.openapi.roots.ModuleRootModel; import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.JDOMUtil; import com.intellij.openapi.util.SystemInfo; @@ -62,8 +63,6 @@ public class EclipseClasspathTest extends IdeaTestCase { assertTrue(currentTestRoot.getAbsolutePath(), currentTestRoot.isDirectory()); FileUtil.copyDir(currentTestRoot, new File(getProject().getBaseDir().getPath())); - - } private void doTest() throws Exception { @@ -113,10 +112,9 @@ public class EclipseClasspathTest extends IdeaTestCase { fileText1 = fileText1.replaceAll(EclipseXml.FILE_PROTOCOL + "/", EclipseXml.FILE_PROTOCOL); } final Element classpathElement1 = JDOMUtil.loadDocument(fileText1).getRootElement(); - final ModifiableRootModel model = ModuleRootManager.getInstance(module).getModifiableModel(); + final ModuleRootModel model = ModuleRootManager.getInstance(module); final Element resultClasspathElement = new Element(EclipseXml.CLASSPATH_TAG); new EclipseClasspathWriter(model).writeClasspath(resultClasspathElement, classpathElement1); - model.dispose(); String resulted = new String(JDOMUtil.printDocument(new Document(resultClasspathElement), "\n")); Assert.assertTrue(resulted.replaceAll(StringUtil.escapeToRegexp(module.getProject().getBaseDir().getPath()), "\\$ROOT\\$"), @@ -124,7 +122,6 @@ public class EclipseClasspathTest extends IdeaTestCase { } - public void testAbsolutePaths() throws Exception { doTest("/parent/parent/test", getProject()); } diff --git a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseEmlTest.java b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseEmlTest.java index 17f6d9514c82..d8cd4841bfd8 100644 --- a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseEmlTest.java +++ b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseEmlTest.java @@ -28,6 +28,8 @@ import com.intellij.openapi.module.StdModuleTypes; import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.ModifiableRootModel; import com.intellij.openapi.roots.ModuleRootManager; +import com.intellij.openapi.roots.ModuleRootModel; +import com.intellij.openapi.roots.ModuleRootModificationUtil; import com.intellij.openapi.util.*; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.util.text.StringUtil; @@ -54,12 +56,9 @@ public class EclipseEmlTest extends IdeaTestCase { assertTrue(currentTestRoot.getAbsolutePath(), currentTestRoot.isDirectory()); FileUtil.copyDir(currentTestRoot, new File(getProject().getBaseDir().getPath())); - - } - protected static void doTest(String relativePath, final Project project) throws Exception { final String path = project.getBaseDir().getPath() + relativePath; final Module module = doLoadModule(path, project); @@ -84,7 +83,8 @@ public class EclipseEmlTest extends IdeaTestCase { new EclipseClasspathStorageProvider.EclipseClasspathConverter(module); final ModifiableRootModel rootModel = ModuleRootManager.getInstance(module).getModifiableModel(); - final Element classpathElement = JDOMUtil.loadDocument(FileUtil.loadFile(new File(path, EclipseXml.DOT_CLASSPATH_EXT))).getRootElement(); + final Element classpathElement = + JDOMUtil.loadDocument(FileUtil.loadFile(new File(path, EclipseXml.DOT_CLASSPATH_EXT))).getRootElement(); converter.getClasspath(rootModel, classpathElement); ApplicationManager.getApplication().runWriteAction(new Runnable() { public void run() { @@ -95,11 +95,9 @@ public class EclipseEmlTest extends IdeaTestCase { } protected static void checkModule(String path, Module module) throws WriteExternalException, IOException, JDOMException { - ModifiableRootModel rootModel; - rootModel = ModuleRootManager.getInstance(module).getModifiableModel(); + ModuleRootModel rootModel = ModuleRootManager.getInstance(module); final Element root = new Element("component"); IdeaSpecificSettings.writeIDEASpecificClasspath(root, rootModel); - rootModel.dispose(); final String resulted = new String(JDOMUtil.printDocument(new Document(root), "\n")); @@ -128,9 +126,7 @@ public class EclipseEmlTest extends IdeaTestCase { final Module module = doLoadModule(path, project); - final ModifiableRootModel modifiableModel = ModuleRootManager.getInstance(module).getModifiableModel(); - modifiableModel.inheritSdk(); - modifiableModel.commit(); + ModuleRootModificationUtil.setSdkInherited(module); checkModule(projectBasePath + "/test/expected", module); } diff --git a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseImlTest.java b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseImlTest.java index b47da4e11558..6d19e5988921 100644 --- a/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseImlTest.java +++ b/plugins/eclipse/testSources/org/jetbrains/idea/eclipse/EclipseImlTest.java @@ -31,7 +31,7 @@ import com.intellij.openapi.module.StdModuleTypes; import com.intellij.openapi.project.Project; import com.intellij.openapi.roots.ModifiableRootModel; import com.intellij.openapi.roots.ModuleRootManager; -import com.intellij.openapi.roots.impl.RootModelImpl; +import com.intellij.openapi.roots.impl.ModuleRootManagerImpl; import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.JDOMUtil; import com.intellij.openapi.util.SystemInfo; @@ -61,8 +61,6 @@ public class EclipseImlTest extends IdeaTestCase { assertTrue(currentTestRoot.getAbsolutePath(), currentTestRoot.isDirectory()); FileUtil.copyDir(currentTestRoot, new File(getProject().getBaseDir().getPath())); - - } private void doTest() throws Exception { @@ -95,24 +93,24 @@ public class EclipseImlTest extends IdeaTestCase { final EclipseClasspathReader classpathReader = new EclipseClasspathReader(path, project, null); classpathReader.init(rootModel); classpathReader - .readClasspath(rootModel, new ArrayList(), new ArrayList(), new HashSet(), new HashSet(), null, classpathElement); + .readClasspath(rootModel, new ArrayList(), new ArrayList(), new HashSet(), new HashSet(), null, + classpathElement); ApplicationManager.getApplication().runWriteAction(new Runnable() { public void run() { rootModel.commit(); } }); - final RootModelImpl model = (RootModelImpl)ModuleRootManager.getInstance(module).getModifiableModel(); final Element actualImlElement = new Element("root"); - model.writeExternal(actualImlElement); - model.dispose(); + ((ModuleRootManagerImpl)ModuleRootManager.getInstance(module)).getState().writeExternal(actualImlElement); PathMacros.getInstance().setMacro(JUNIT, communityAppDir); PathMacroManager.getInstance(module).collapsePaths(actualImlElement); PathMacroManager.getInstance(project).collapsePaths(actualImlElement); PathMacros.getInstance().removeMacro(JUNIT); - final Element expectedIml = JDOMUtil.loadDocument(new File(project.getBaseDir().getPath() + "/expected", "expected.iml")).getRootElement(); + final Element expectedIml = + JDOMUtil.loadDocument(new File(project.getBaseDir().getPath() + "/expected", "expected.iml")).getRootElement(); Assert.assertTrue(new String(JDOMUtil.printDocument(new Document(actualImlElement), "\n")), JDOMUtil.areElementsEqual(expectedIml, actualImlElement)); } @@ -138,5 +136,4 @@ public class EclipseImlTest extends IdeaTestCase { public void testRoot() throws Exception { doTest(); } - }