From 65242559b180ac5b33c4feb86aa1f06608ac1165 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Thu, 4 Sep 2014 14:29:34 +0200 Subject: [PATCH] =?UTF-8?q?simplification=20=E2=80=94=20avoid=20String=20i?= =?UTF-8?q?ntern,=20avoid=20macros=20->=20macro=20name=20->=20macros=20tra?= =?UTF-8?q?nsformations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../impl/stores/ModuleStoreImpl.java | 7 +-- .../application/impl/ApplicationImpl.java | 4 +- .../impl/stores/ApplicationStoreImpl.java | 6 +- .../impl/stores/DefaultProjectStoreImpl.java | 3 +- .../impl/stores/IApplicationStore.java | 10 +-- .../impl/stores/ProjectStoreImpl.java | 10 +-- .../impl/stores/StateStorageManager.java | 4 +- .../impl/stores/StateStorageManagerImpl.java | 56 +++++++---------- .../options/SchemesManagerFactoryImpl.java | 63 ++----------------- .../components/impl/ApplicationStoreTest.java | 2 +- .../impl/StateStorageManagerImplTest.java | 6 +- .../openapi/components/StoragePathMacros.java | 1 + 12 files changed, 52 insertions(+), 120 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/openapi/components/impl/stores/ModuleStoreImpl.java b/platform/lang-impl/src/com/intellij/openapi/components/impl/stores/ModuleStoreImpl.java index 3517b71c22b2..fec4e95f6578 100644 --- a/platform/lang-impl/src/com/intellij/openapi/components/impl/stores/ModuleStoreImpl.java +++ b/platform/lang-impl/src/com/intellij/openapi/components/impl/stores/ModuleStoreImpl.java @@ -32,7 +32,6 @@ import com.intellij.util.ArrayUtil; import gnu.trove.THashMap; import org.jdom.Attribute; import org.jdom.Element; -import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -45,12 +44,10 @@ import java.util.Set; public class ModuleStoreImpl extends BaseFileConfigurableStoreImpl implements IModuleStore { private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.components.impl.stores.ModuleStoreImpl"); - @NonNls private static final String MODULE_FILE_MACRO = "MODULE_FILE"; private final ModuleImpl myModule; - public static final String DEFAULT_STATE_STORAGE = "$" + MODULE_FILE_MACRO + "$"; - + public static final String DEFAULT_STATE_STORAGE = "$MODULE_FILE$"; @SuppressWarnings({"UnusedDeclaration"}) public ModuleStoreImpl(final ComponentManagerImpl componentManager, final ModuleImpl module) { @@ -189,7 +186,7 @@ public class ModuleStoreImpl extends BaseFileConfigurableStoreImpl implements IM LocalFileSystem.getInstance().refreshAndFindFileByPath(path); final StateStorageManager storageManager = getStateStorageManager(); storageManager.clearStateStorage(DEFAULT_STATE_STORAGE); - storageManager.addMacro(MODULE_FILE_MACRO, path); + storageManager.addMacro(DEFAULT_STATE_STORAGE, path); } @Override diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java index e3a36a2e59ab..a8eca00b505e 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java @@ -531,10 +531,10 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App @Override public void load(@Nullable String optionsPath) throws IOException { - load(PathManager.getConfigPath(), optionsPath); + load(PathManager.getConfigPath(), optionsPath == null ? PathManager.getOptionsPath() : optionsPath); } - public void load(@NotNull String configPath, @Nullable String optionsPath) throws IOException { + public void load(@NotNull String configPath, @NotNull String optionsPath) throws IOException { getStateStore().setOptionsPath(optionsPath); getStateStore().setConfigPath(configPath); diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ApplicationStoreImpl.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ApplicationStoreImpl.java index 86ccd77be20d..bf03cb11f1d6 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ApplicationStoreImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ApplicationStoreImpl.java @@ -92,13 +92,13 @@ class ApplicationStoreImpl extends ComponentStoreImpl implements IApplicationSto } @Override - public void setOptionsPath(final String path) { - myStateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.APP_CONFIG), path); + public void setOptionsPath(@NotNull String path) { + myStateStorageManager.addMacro(StoragePathMacros.APP_CONFIG, path); } @Override public void setConfigPath(@NotNull final String configPath) { - myStateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.ROOT_CONFIG), configPath); + myStateStorageManager.addMacro(StoragePathMacros.ROOT_CONFIG, configPath); myConfigPath = configPath; } diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/DefaultProjectStoreImpl.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/DefaultProjectStoreImpl.java index b2bc30418038..d0d2f62f7bfb 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/DefaultProjectStoreImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/DefaultProjectStoreImpl.java @@ -114,7 +114,7 @@ public class DefaultProjectStoreImpl extends ProjectStoreImpl { return new StateStorageManager() { @Override - public void addMacro(String macro, String expansion) { + public void addMacro(@NotNull String macro, @NotNull String expansion) { throw new UnsupportedOperationException("Method addMacro not implemented in " + getClass()); } @@ -163,6 +163,7 @@ public class DefaultProjectStoreImpl extends ProjectStoreImpl { storage.finishSave(((MySaveSession)saveSession).saveSession); } + @NotNull @Override public String expandMacros(@NotNull String file) { throw new UnsupportedOperationException("Method expandMacros not implemented in " + getClass()); diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/IApplicationStore.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/IApplicationStore.java index c669bd1f37be..0ebfae887771 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/IApplicationStore.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/IApplicationStore.java @@ -15,18 +15,18 @@ */ package com.intellij.openapi.components.impl.stores; -import com.intellij.openapi.components.StateStorageException; -import com.intellij.openapi.vfs.VirtualFile; import com.intellij.openapi.components.StateStorage; +import com.intellij.openapi.components.StateStorageException; import com.intellij.openapi.util.Pair; +import com.intellij.openapi.vfs.VirtualFile; import org.jetbrains.annotations.NotNull; -import java.util.Set; -import java.util.Collection; import java.io.IOException; +import java.util.Collection; +import java.util.Set; public interface IApplicationStore extends IComponentStore { - void setOptionsPath(String path); + void setOptionsPath(@NotNull String path); void setConfigPath(@NotNull String configPath); diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ProjectStoreImpl.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ProjectStoreImpl.java index c15da28fbd4d..968e84907517 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ProjectStoreImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/ProjectStoreImpl.java @@ -166,15 +166,15 @@ class ProjectStoreImpl extends BaseFileConfigurableStoreImpl implements IProject final File dirStore = file.isDirectory() ? new File(file, Project.DIRECTORY_STORE_FOLDER) : new File(file.getParentFile(), Project.DIRECTORY_STORE_FOLDER); - stateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.PROJECT_FILE), new File(dirStore, "misc.xml").getPath()); + stateStorageManager.addMacro(StoragePathMacros.PROJECT_FILE, new File(dirStore, "misc.xml").getPath()); final File ws = new File(dirStore, "workspace.xml"); - stateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.WORKSPACE_FILE), ws.getPath()); + stateStorageManager.addMacro(StoragePathMacros.WORKSPACE_FILE, ws.getPath()); if (!ws.exists() && !file.isDirectory()) { useOldWsContent(filePath, ws); } - stateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.PROJECT_CONFIG_DIR), dirStore.getPath()); + stateStorageManager.addMacro(StoragePathMacros.PROJECT_CONFIG_DIR, dirStore.getPath()); ApplicationManager.getApplication().invokeAndWait(new Runnable() { @Override @@ -186,10 +186,10 @@ class ProjectStoreImpl extends BaseFileConfigurableStoreImpl implements IProject else { myScheme = StorageScheme.DEFAULT; - stateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.PROJECT_FILE), filePath); + stateStorageManager.addMacro(StoragePathMacros.PROJECT_FILE, filePath); final String workspacePath = composeWsPath(filePath); - stateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.WORKSPACE_FILE), workspacePath); + stateStorageManager.addMacro(StoragePathMacros.WORKSPACE_FILE, workspacePath); ApplicationManager.getApplication().invokeAndWait(new Runnable() { @Override diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManager.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManager.java index d89ceda20a1e..d3e9d1c03d2e 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManager.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManager.java @@ -31,7 +31,7 @@ import java.util.Set; * @author mike */ public interface StateStorageManager { - void addMacro(String macro, String expansion); + void addMacro(@NotNull String macro, @NotNull String expansion); @Nullable TrackingPathMacroSubstitutor getMacroSubstitutor(); @@ -65,7 +65,7 @@ public interface StateStorageManager { @Nullable StateStorage getOldStorage(Object component, String componentName, StateStorageOperation operation) throws StateStorageException; - @Nullable + @NotNull String expandMacros(@NotNull String file); @Deprecated diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManagerImpl.java index e7b4e87f4941..1f136345560d 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageManagerImpl.java @@ -30,6 +30,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.ObjectUtils; import com.intellij.util.PathUtilRt; +import com.intellij.util.ReflectionUtil; import com.intellij.util.SmartList; import gnu.trove.THashMap; import gnu.trove.THashSet; @@ -100,9 +101,14 @@ public abstract class StateStorageManagerImpl implements StateStorageManager, Di } @Override - public synchronized void addMacro(String macro, String expansion) { - // avoid hundreds of $MODULE_FILE$ instances - myMacros.put(("$" + macro + "$").intern(), expansion); + public synchronized void addMacro(@NotNull String macro, @NotNull String expansion) { + assert !macro.isEmpty(); + // backward compatibility + if (macro.charAt(0) != '$') { + LOG.warn("Add macros instead of macro name: " + macro); + expansion = '$' + macro + '$'; + } + myMacros.put(macro, expansion); } @Override @@ -217,33 +223,19 @@ public abstract class StateStorageManagerImpl implements StateStorageManager, Di @Nullable private StateStorage createDirectoryStateStorage(String file, Class splitterClass) throws StateStorageException { - String expandedFile = expandMacros(file); - if (expandedFile == null) { - myStorages.put(file, null); - return null; - } - final StateSplitter splitter; try { - splitter = splitterClass.newInstance(); + splitter = ReflectionUtil.newInstance(splitterClass); } - catch (InstantiationException e) { + catch (RuntimeException e) { throw new StateStorageException(e); } - catch (IllegalAccessException e) { - throw new StateStorageException(e); - } - - return new DirectoryBasedStorage(myPathMacroSubstitutor, expandedFile, splitter, this, myPicoContainer); + return new DirectoryBasedStorage(myPathMacroSubstitutor, expandMacros(file), splitter, this, myPicoContainer); } @Nullable private StateStorage createFileStateStorage(@NotNull final String fileSpec, @Nullable RoamingType roamingType) { String expandedFile = expandMacros(fileSpec); - if (expandedFile == null) { - myStorages.put(fileSpec, null); - return null; - } if (!ourHeadlessEnvironment && PathUtilRt.getFileName(expandedFile).lastIndexOf('.') < 0) { throw new IllegalArgumentException("Extension is missing for storage file: " + expandedFile); @@ -341,26 +333,22 @@ public abstract class StateStorageManagerImpl implements StateStorageManager, Di private static final Pattern MACRO_PATTERN = Pattern.compile("(\\$[^\\$]*\\$)"); @Override - @Nullable + @NotNull public synchronized String expandMacros(@NotNull String file) { - final Matcher matcher = MACRO_PATTERN.matcher(file); + String expanded = file; + for (String macro : myMacros.keySet()) { + expanded = StringUtil.replace(expanded, macro, myMacros.get(macro)); + } + + final Matcher matcher = MACRO_PATTERN.matcher(expanded); while (matcher.find()) { String m = matcher.group(1); - if (!myMacros.containsKey(m) || !ApplicationManager.getApplication().isUnitTestMode() && myMacros.get(m) == null) { - throw new IllegalArgumentException("Unknown macro: " + m + " in storage spec: " + file); + if (!myMacros.containsKey(m)) { + throw new IllegalArgumentException("Unknown macro: " + m + " in storage file spec: " + file); } } - String actualFile = file; - - for (String macro : myMacros.keySet()) { - final String replacement = myMacros.get(macro); - if (replacement != null) { - actualFile = StringUtil.replace(actualFile, macro, replacement); - } - } - - return actualFile; + return expanded; } @NotNull diff --git a/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerFactoryImpl.java b/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerFactoryImpl.java index 449a2180e3c1..c2f3b4d72b3e 100644 --- a/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerFactoryImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/options/SchemesManagerFactoryImpl.java @@ -28,8 +28,6 @@ import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import java.io.File; -import java.util.Collection; -import java.util.Collections; import java.util.List; public class SchemesManagerFactoryImpl extends SchemesManagerFactory implements SettingsSavingComponent { @@ -47,63 +45,10 @@ public class SchemesManagerFactoryImpl extends SchemesManagerFactory implements } IApplicationStore applicationStore = ((ApplicationImpl)application).getStateStore(); String baseDirPath = applicationStore.getStateStorageManager().expandMacros(fileSpec); - if (baseDirPath != null) { - StreamProvider provider = applicationStore.getStateStorageManager().getStreamProvider(); - SchemesManagerImpl manager = new SchemesManagerImpl(fileSpec, processor, roamingType, provider, new File(baseDirPath)); - myRegisteredManagers.add(manager); - return manager; - } - else { - return new AbstractSchemesManager() { - @Override - @NotNull - public Collection loadSchemes() { - return Collections.emptyList(); - } - - @Override - @NotNull - public Collection> loadSharedSchemes(final Collection currentSchemeList) { - return Collections.emptyList(); - } - - @Override - public void exportScheme(@NotNull final E scheme, final String name, final String description) { - } - - @Override - public boolean isImportAvailable() { - return false; - } - - @Override - public boolean isShared(final Scheme scheme) { - return false; - } - - @Override - public void save() { - } - - @Override - protected void onSchemeDeleted(final Scheme toDelete) { - } - - @Override - protected void onSchemeAdded(final T scheme) { - } - - @Override - public boolean isExportAvailable() { - return false; - } - - @Override - public File getRootDirectory() { - return null; - } - }; - } + StreamProvider provider = applicationStore.getStateStorageManager().getStreamProvider(); + SchemesManagerImpl manager = new SchemesManagerImpl(fileSpec, processor, roamingType, provider, new File(baseDirPath)); + myRegisteredManagers.add(manager); + return manager; } @Override diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/ApplicationStoreTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/ApplicationStoreTest.java index 2d5bcb4537ff..719635a19a30 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/ApplicationStoreTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/ApplicationStoreTest.java @@ -161,7 +161,7 @@ public class ApplicationStoreTest extends LightPlatformLangTestCase { } }; - stateStorageManager.addMacro(StoragePathMacros.getMacroName(StoragePathMacros.APP_CONFIG), testAppConfigPath); + stateStorageManager.addMacro(StoragePathMacros.APP_CONFIG, testAppConfigPath); } @Override diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/StateStorageManagerImplTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/StateStorageManagerImplTest.java index 6accd1016d2a..da54b2a65e35 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/StateStorageManagerImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/components/impl/StateStorageManagerImplTest.java @@ -56,7 +56,7 @@ public class StateStorageManagerImplTest extends LightPlatformLangTestCase { return null; } }; - myStateStorageManager.addMacro("MACRO1", "/temp/m1"); + myStateStorageManager.addMacro("$MACRO1$", "/temp/m1"); } @Override @@ -76,12 +76,12 @@ public class StateStorageManagerImplTest extends LightPlatformLangTestCase { fail("Exception expected"); } catch (IllegalArgumentException e) { - assertEquals("Unknown macro: $UNKNOWN_MACRO$ in storage spec: $UNKNOWN_MACRO$/test.xml", e.getMessage()); + assertEquals("Unknown macro: $UNKNOWN_MACRO$ in storage file spec: $UNKNOWN_MACRO$/test.xml", e.getMessage()); } } public void testCreateFileStateStorageMacroSubstitutedWhenExpansionHas$() { - myStateStorageManager.addMacro("DOLLAR_MACRO", "/temp/d$"); + myStateStorageManager.addMacro("$DOLLAR_MACRO$", "/temp/d$"); StateStorage data = myStateStorageManager.getStateStorage("$DOLLAR_MACRO$/test.xml", RoamingType.PER_USER); assertThat(data, is(notNullValue())); } diff --git a/platform/projectModel-api/src/com/intellij/openapi/components/StoragePathMacros.java b/platform/projectModel-api/src/com/intellij/openapi/components/StoragePathMacros.java index 57c536a5547f..3a292dd7175c 100644 --- a/platform/projectModel-api/src/com/intellij/openapi/components/StoragePathMacros.java +++ b/platform/projectModel-api/src/com/intellij/openapi/components/StoragePathMacros.java @@ -68,6 +68,7 @@ public class StoragePathMacros { * @throws IllegalArgumentException if given macro definition has unexpected format */ @NotNull + @Deprecated public static String getMacroName(@NotNull String macro) throws IllegalArgumentException { if (macro.length() < 3 || macro.charAt(0) != '$' || macro.charAt(macro.length() - 1) != '$') { throw new IllegalArgumentException("Malformed macro definition (" + macro + ")");