From 9428be03b416ac5f9e2bac8044e553d3de754dcc Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Wed, 2 Sep 2015 10:26:41 +0200 Subject: [PATCH 1/3] cleanup --- platform/configuration-store-impl/src/ComponentStoreImpl.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/platform/configuration-store-impl/src/ComponentStoreImpl.kt b/platform/configuration-store-impl/src/ComponentStoreImpl.kt index be72f34d9085..2921901b5308 100644 --- a/platform/configuration-store-impl/src/ComponentStoreImpl.kt +++ b/platform/configuration-store-impl/src/ComponentStoreImpl.kt @@ -240,10 +240,9 @@ abstract class ComponentStoreImpl : IComponentStore { val defaultState = if (stateSpec.defaultStateAsResource) getDefaultState(component, name, stateClass) else null val storageSpecs = getStorageSpecs(component, stateSpec, StateStorageOperation.READ) - val stateStorageChooser = component as? StateStorageChooserEx + val storageChooser = component as? StateStorageChooserEx for (storageSpec in storageSpecs) { - val resolution = if (stateStorageChooser == null) Resolution.DO else stateStorageChooser.getResolution(storageSpec, StateStorageOperation.READ) - if (resolution === Resolution.SKIP) { + if (storageChooser?.getResolution(storageSpec, StateStorageOperation.READ) == Resolution.SKIP) { continue } From de241251c83efbd117b34bc03e9829e1d870b180 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Wed, 2 Sep 2015 11:42:13 +0200 Subject: [PATCH 2/3] don't apply "don't save if only format is changed --- .../src/ComponentStoreImpl.kt | 12 ++++++++-- .../src/DirectoryBasedStorage.kt | 2 +- .../src/ProjectStoreImpl.kt | 2 ++ .../src/StorageBaseEx.kt | 9 +++----- .../src/XmlElementStorage.kt | 2 +- .../testSrc/ApplicationStoreTest.kt | 23 ++++++++++++++++++- .../testSrc/XmlElementStorageTest.kt | 4 ++-- .../roots/impl/storage/ClasspathStorage.java | 2 +- .../impl/stores/StateStorageBase.kt | 10 +++++--- 9 files changed, 49 insertions(+), 17 deletions(-) diff --git a/platform/configuration-store-impl/src/ComponentStoreImpl.kt b/platform/configuration-store-impl/src/ComponentStoreImpl.kt index 2921901b5308..dcd128e91820 100644 --- a/platform/configuration-store-impl/src/ComponentStoreImpl.kt +++ b/platform/configuration-store-impl/src/ComponentStoreImpl.kt @@ -33,6 +33,7 @@ import com.intellij.openapi.project.Project import com.intellij.openapi.util import com.intellij.openapi.util.* import com.intellij.openapi.util.Pair +import com.intellij.openapi.util.registry.Registry import com.intellij.openapi.vfs.VirtualFile import com.intellij.openapi.vfs.newvfs.impl.VfsRootAccess import com.intellij.util.ArrayUtilRt @@ -247,7 +248,7 @@ abstract class ComponentStoreImpl : IComponentStore { } val storage = storageManager.getStateStorage(storageSpec) - var stateGetter = (storage as? StorageBaseEx<*>)?.createGetSession(component, name, stateClass) + var stateGetter = if (isUseLoadedStateAsExisting(storageSpec) && Registry.`is`("use.loaded.state.as.existing", false)) (storage as? StorageBaseEx<*>)?.createGetSession(component, name, stateClass) else null var state = if (stateGetter == null) storage.getState(component, name, stateClass, defaultState, reloadData) else stateGetter.getState(defaultState) if (state == null) { if (changedStorages != null && changedStorages.contains(storage)) { @@ -260,7 +261,12 @@ abstract class ComponentStoreImpl : IComponentStore { } } - stateGetter?.use { component.loadState(state) } + try { + component.loadState(state) + } + finally { + stateGetter?.close() + } return name } @@ -270,6 +276,8 @@ abstract class ComponentStoreImpl : IComponentStore { return name } + protected open fun isUseLoadedStateAsExisting(storageSpec: Storage): Boolean = true + protected open fun getPathMacroManagerForDefaults(): PathMacroManager? = null private fun getDefaultState(component: Any, componentName: String, stateClass: Class): T? { diff --git a/platform/configuration-store-impl/src/DirectoryBasedStorage.kt b/platform/configuration-store-impl/src/DirectoryBasedStorage.kt index c31da745fab6..cf7dc58e7166 100644 --- a/platform/configuration-store-impl/src/DirectoryBasedStorage.kt +++ b/platform/configuration-store-impl/src/DirectoryBasedStorage.kt @@ -62,7 +62,7 @@ open class DirectoryBasedStorage(private val myPathMacroSubstitutor: TrackingPat } } - override fun getState(storageData: Map, component: Any?, componentName: String) = getCompositeStateAndArchive(storageData, componentName, mySplitter) + override fun getState(storageData: Map, component: Any?, componentName: String, archive: Boolean) = getCompositeStateAndArchive(storageData, componentName, mySplitter) override fun loadData(): MutableMap { return fromMap(DirectoryStorageUtil.loadFrom(getVirtualFile(), myPathMacroSubstitutor)) diff --git a/platform/configuration-store-impl/src/ProjectStoreImpl.kt b/platform/configuration-store-impl/src/ProjectStoreImpl.kt index 5ca7c61f9f6c..f3a5128b3aa6 100644 --- a/platform/configuration-store-impl/src/ProjectStoreImpl.kt +++ b/platform/configuration-store-impl/src/ProjectStoreImpl.kt @@ -288,6 +288,8 @@ open class ProjectStoreImpl(override val project: ProjectImpl, private val pathM } override fun selectDefaultStorages(storages: Array, operation: StateStorageOperation) = selectDefaultStorages(storages, operation, scheme) + + override fun isUseLoadedStateAsExisting(storageSpec: Storage) = storageSpec.file != StoragePathMacros.WORKSPACE_FILE } fun selectDefaultStorages(storages: Array, operation: StateStorageOperation, scheme: StorageScheme): Array { diff --git a/platform/configuration-store-impl/src/StorageBaseEx.kt b/platform/configuration-store-impl/src/StorageBaseEx.kt index 217b257d5f35..0266f17b5155 100644 --- a/platform/configuration-store-impl/src/StorageBaseEx.kt +++ b/platform/configuration-store-impl/src/StorageBaseEx.kt @@ -15,12 +15,9 @@ */ package com.intellij.configurationStore -import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.components.PersistentStateComponent import com.intellij.openapi.components.impl.stores.StateStorageBase -import com.intellij.openapi.util.registry.Registry import org.jdom.Element -import java.io.Closeable abstract class StorageBaseEx : StateStorageBase() { fun createGetSession(component: PersistentStateComponent, componentName: String, stateClass: Class, reload: Boolean = false) = StateGetter(component, componentName, getStorageData(reload), stateClass, this) @@ -31,7 +28,7 @@ abstract class StorageBaseEx : StateStorageBase() { abstract fun archiveState(storageData: T, componentName: String, serializedState: Element?) } -class StateGetter(private val component: PersistentStateComponent, private val componentName: String, private val storageData: T, private val stateClass: Class, private val storage: StorageBaseEx) : Closeable { +class StateGetter(private val component: PersistentStateComponent, private val componentName: String, private val storageData: T, private val stateClass: Class, private val storage: StorageBaseEx) { var serializedState: Element? = null fun getState(mergeInto: S? = null): S? { @@ -44,7 +41,7 @@ class StateGetter(private val component: PersistentStateCompon return storage.deserializeState(serializedState, stateClass, mergeInto) } - override fun close() { + fun close() { if (serializedState == null) { return } @@ -53,7 +50,7 @@ class StateGetter(private val component: PersistentStateCompon val stateAfterLoad: S? try { - stateAfterLoad = if (ApplicationManager.getApplication().isUnitTestMode() || Registry.`is`("use.loaded.state.as.existing", false)) component.getState() else null + stateAfterLoad = component.getState() } catch(e: Throwable) { LOG.error("Cannot get state after load", e) diff --git a/platform/configuration-store-impl/src/XmlElementStorage.kt b/platform/configuration-store-impl/src/XmlElementStorage.kt index 67bed1a75dac..d5a6f05e5c28 100644 --- a/platform/configuration-store-impl/src/XmlElementStorage.kt +++ b/platform/configuration-store-impl/src/XmlElementStorage.kt @@ -37,7 +37,7 @@ abstract class XmlElementStorage protected constructor(protected val fileSpec: S protected abstract fun loadLocalData(): Element? - override final fun getState(storageData: StateMap, component: Any?, componentName: String) = storageData.getState(componentName) + override final fun getState(storageData: StateMap, component: Any?, componentName: String, archive: Boolean) = storageData.getState(componentName) override fun archiveState(storageData: StateMap, componentName: String, serializedState: Element?) { storageData.archive(componentName, serializedState) diff --git a/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt b/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt index 5e5c5d89d58d..66bf4c5dced3 100644 --- a/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt +++ b/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt @@ -110,7 +110,7 @@ class ApplicationStoreTest { } @State(name = "A", storages = arrayOf(Storage(file = "a.xml"))) - private class A : PersistentStateComponent { + private open class A : PersistentStateComponent { data class State(@Attribute var foo: String = "", @Attribute var bar: String = "") var state = State() @@ -144,6 +144,24 @@ class ApplicationStoreTest { assertThat(file).hasContent("\n \n") } + @Test fun `don't apply "don't save if only format is changed" logic to workspace storage`() { + @State(name = "A", storages = arrayOf(Storage(file = StoragePathMacros.WORKSPACE_FILE))) + class AWorkspace : A() + + val oldContent = "" + val file = writeConfig("workspace.xml", oldContent) + val oldModificationTime = file.getLastModifiedTime() + testAppConfig.refreshVfs() + + val component = AWorkspace() + componentStore.initComponent(component, false) + assertThat(component.state).isEqualTo(A.State("old")) + + saveStore() + + assertThat(file).hasContent("\n \n") + } + private fun saveStore() { runInEdtAndWait { componentStore.save(SmartList()) } } @@ -188,7 +206,10 @@ class ApplicationStoreTest { override fun setPath(path: String) { storageManager.addMacro(StoragePathMacros.APP_CONFIG, path) + storageManager.addMacro(StoragePathMacros.WORKSPACE_FILE, "$path/workspace.xml") } + + override fun isUseLoadedStateAsExisting(storageSpec: Storage) = storageSpec.file != StoragePathMacros.WORKSPACE_FILE } abstract class Foo { diff --git a/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt b/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt index ad55501e98c5..d31a37ac7e95 100644 --- a/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt +++ b/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt @@ -24,7 +24,7 @@ import org.junit.Test class XmlElementStorageTest { @Test fun testGetStateSucceeded() { val storage = MyXmlElementStorage(tag("root", tag("component", attr("name", "test"), tag("foo")))) - val state = storage.getState(this, "test", javaClass(), null, false) + val state = storage.getState(this, "test", javaClass()) assertThat(state).isNotNull() assertThat(state!!.getName()).isEqualTo("component") assertThat(state.getChild("foo")).isNotNull() @@ -32,7 +32,7 @@ class XmlElementStorageTest { @Test fun `get state not succeeded`() { val storage = MyXmlElementStorage(tag("root")) - val state = storage.getState(this, "test", javaClass(), null, false) + val state = storage.getState(this, "test", javaClass()) assertThat(state).isNull() } 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 7b07acf493ce..47bd6ed0a415 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 @@ -135,7 +135,7 @@ public class ClasspathStorage extends StateStorageBase { @Nullable @Override - public Element getState(@NotNull Boolean storageData, Object component, @NotNull String componentName) { + public Element getState(@NotNull Boolean storageData, Object component, @NotNull String componentName, boolean archive) { if (storageData) { return null; } diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageBase.kt b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageBase.kt index 0223a27d6ce8..39e513cf442b 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageBase.kt +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StateStorageBase.kt @@ -29,15 +29,19 @@ public abstract class StateStorageBase : StateStorage { protected val storageDataRef: AtomicReference = AtomicReference() - override fun getState(component: Any?, componentName: String, stateClass: Class, mergeInto: S?, reload: Boolean): S? { - return deserializeState(getState(getStorageData(reload), component, componentName), stateClass, mergeInto) + override final fun getState(component: Any?, componentName: String, stateClass: Class, mergeInto: S?, reload: Boolean): S? { + return getState(component, componentName, stateClass, true, reload, mergeInto) + } + + fun getState(component: Any?, componentName: String, stateClass: Class, archive: Boolean = true, reload: Boolean = false, mergeInto: S? = null): S? { + return deserializeState(getState(getStorageData(reload), component, componentName, archive), stateClass, mergeInto) } open fun deserializeState(serializedState: Element?, stateClass: Class, mergeInto: S?): S? { return DefaultStateSerializer.deserializeState(serializedState, stateClass, mergeInto) } - abstract fun getState(storageData: T, component: Any?, componentName: String): Element? + abstract fun getState(storageData: T, component: Any?, componentName: String, archive: Boolean = true): Element? protected abstract fun hasState(storageData: T, componentName: String): Boolean From 33eef709cbe3e835437e5bd54a56f334fcb70698 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Wed, 2 Sep 2015 11:57:32 +0200 Subject: [PATCH 3/3] fix test name to avoid win issues --- .../configuration-store-impl/testSrc/ApplicationStoreTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt b/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt index 66bf4c5dced3..2120a265f162 100644 --- a/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt +++ b/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt @@ -144,7 +144,7 @@ class ApplicationStoreTest { assertThat(file).hasContent("\n \n") } - @Test fun `don't apply "don't save if only format is changed" logic to workspace storage`() { + @Test fun `do not apply to workspace storage - do not save if only format is changed`() { @State(name = "A", storages = arrayOf(Storage(file = StoragePathMacros.WORKSPACE_FILE))) class AWorkspace : A()