diff --git a/platform/configuration-store-impl/src/ComponentStoreImpl.kt b/platform/configuration-store-impl/src/ComponentStoreImpl.kt index 9958fac6292a..b7fca615e950 100644 --- a/platform/configuration-store-impl/src/ComponentStoreImpl.kt +++ b/platform/configuration-store-impl/src/ComponentStoreImpl.kt @@ -208,7 +208,7 @@ abstract class ComponentStoreImpl : IComponentStore { LOG.error(e) } - val element = storageManager.getOldStorage(component, componentName, StateStorageOperation.READ)?.getState(component, componentName, javaClass()) ?: return null + val element = storageManager.getOldStorage(component, componentName, StateStorageOperation.READ)?.getState(component, componentName, javaClass(), null, false) ?: return null try { component.readExternal(element) } @@ -248,7 +248,8 @@ abstract class ComponentStoreImpl : IComponentStore { } val storage = storageManager.getStateStorage(storageSpec) - var state = storage.getState(component, name, stateClass, defaultState, reloadData) + var stateGetter = (storage as? StorageBaseEx<*>)?.createGetSession(component, name, stateClass) + 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)) { // state will be null if file deleted @@ -260,7 +261,12 @@ abstract class ComponentStoreImpl : IComponentStore { } } - component.loadState(state) + try { + component.loadState(state) + } + finally { + stateGetter?.close() + } return name } @@ -438,4 +444,4 @@ abstract class ComponentStoreImpl : IComponentStore { return errors } } -} +} \ No newline at end of file diff --git a/platform/configuration-store-impl/src/StateMap.kt b/platform/configuration-store-impl/src/StateMap.kt index 1abcab0d2944..b90b8d5b815f 100644 --- a/platform/configuration-store-impl/src/StateMap.kt +++ b/platform/configuration-store-impl/src/StateMap.kt @@ -33,7 +33,7 @@ import java.util.Arrays import java.util.TreeMap import java.util.concurrent.atomic.AtomicReferenceArray -class StateMap private constructor(private val names: Array, private val states: AtomicReferenceArray) { +class StateMap private constructor(private val names: Array, private val states: AtomicReferenceArray) { companion object { private val LOG = Logger.getInstance(javaClass()) @@ -51,7 +51,7 @@ class StateMap private constructor(private val names: Array, private val Arrays.sort(names) } - val states = AtomicReferenceArray(names.size()) + val states = AtomicReferenceArray(names.size()) for (i in names.indices) { states.set(i, map.get(names[i])) } @@ -72,16 +72,16 @@ class StateMap private constructor(private val names: Array, private val if (Arrays.equals(newBytes, oldState)) { return null } - else if (LOG.isDebugEnabled() && SystemProperties.getBooleanProperty("idea.log.changed.components", false)) { + else if (SystemProperties.getBooleanProperty("idea.log.changed.components", false)) { fun stateToString(state: Any) = JDOMUtil.writeParent(state as? Element ?: unarchiveState(state as ByteArray), "\n") val before = stateToString(oldState) val after = stateToString(newState) if (before == after) { - LOG.debug("Serialization error: serialized are different, but unserialized are equal") + LOG.info("Serialization error: serialized are different, but unserialized are equal") } else { - LOG.debug("$key ${StringUtil.repeat("=", 80 - key.length())}\nBefore:\n$before\nAfter:\n$after") + LOG.info("$key ${StringUtil.repeat("=", 80 - key.length())}\nBefore:\n$before\nAfter:\n$after") } } return newBytes @@ -161,4 +161,15 @@ class StateMap private constructor(private val names: Array, private val val state = states.get(index) as? Element ?: return null return if (states.compareAndSet(index, state, archiveState(state))) state else getStateAndArchive(key) } + + public fun archive(key: String, state: Element?) { + val index = Arrays.binarySearch(names, key) + if (index < 0) { + return + } + + val currentState = states.get(index) + LOG.assertTrue(currentState is Element) + states.set(index, if (state == null) null else archiveState(state)) + } } \ No newline at end of file diff --git a/platform/configuration-store-impl/src/StorageBaseEx.kt b/platform/configuration-store-impl/src/StorageBaseEx.kt new file mode 100644 index 000000000000..94affaa5693e --- /dev/null +++ b/platform/configuration-store-impl/src/StorageBaseEx.kt @@ -0,0 +1,72 @@ +/* + * Copyright 2000-2015 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.configurationStore + +import com.intellij.openapi.components.PersistentStateComponent +import com.intellij.openapi.components.impl.stores.StateStorageBase +import org.jdom.Element + +abstract class StorageBaseEx : StateStorageBase() { + fun createGetSession(component: PersistentStateComponent, componentName: String, stateClass: Class, reload: Boolean = false) = StateGetter(component, componentName, getStorageData(reload), stateClass, this) + + /** + * serializedState is null if state equals to default (see XmlSerializer.serializeIfNotDefault) + */ + 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) { + var serializedState: Element? = null + + fun getState(mergeInto: S? = null): S? { + LOG.assertTrue(serializedState == null) + + serializedState = storage.getState(storageData, component, componentName) + if (serializedState != null) { + //System.out.println("open $componentName to read state, ${hashCode()} $storage, ${Thread.currentThread()}") + } + return storage.deserializeState(serializedState, stateClass, mergeInto) + } + + /** + * nullable - because PersistentStateComponent can return nullable state + */ + fun close() { + if (serializedState == null) { + return + } + + //System.out.println("close $componentName to read state, ${hashCode()} $storage, ${Thread.currentThread()}") + + val stateAfterLoad: S? + try { + stateAfterLoad = component.getState() + } + catch(e: Throwable) { + LOG.error("Cannot get state after load", e) + stateAfterLoad = null + } + + val serializedStateAfterLoad = if (stateAfterLoad == null) { + serializedState + } + else { + serializeState(stateAfterLoad)?.normalizeRootName() + } + + storage.archiveState(storageData, componentName, serializedStateAfterLoad) + } +} \ No newline at end of file diff --git a/platform/configuration-store-impl/src/XmlElementStorage.kt b/platform/configuration-store-impl/src/XmlElementStorage.kt index a12ac2d52dec..67bed1a75dac 100644 --- a/platform/configuration-store-impl/src/XmlElementStorage.kt +++ b/platform/configuration-store-impl/src/XmlElementStorage.kt @@ -19,7 +19,6 @@ import com.intellij.openapi.components.RoamingType import com.intellij.openapi.components.StateStorage import com.intellij.openapi.components.TrackingPathMacroSubstitutor import com.intellij.openapi.components.impl.stores.FileStorageCoreUtil -import com.intellij.openapi.components.impl.stores.StateStorageBase import com.intellij.openapi.util.JDOMUtil import com.intellij.util.containers.ContainerUtil import com.intellij.util.containers.SmartHashSet @@ -32,13 +31,17 @@ abstract class XmlElementStorage protected constructor(protected val fileSpec: S protected val rootElementName: String, protected val pathMacroSubstitutor: TrackingPathMacroSubstitutor?, roamingType: RoamingType? = RoamingType.DEFAULT, - provider: StreamProvider? = null) : StateStorageBase() { + provider: StreamProvider? = null) : StorageBaseEx() { protected val roamingType: RoamingType = roamingType ?: RoamingType.DEFAULT private val provider: StreamProvider? = if (provider == null || roamingType == RoamingType.DISABLED || !provider.isApplicable(fileSpec, this.roamingType)) null else provider protected abstract fun loadLocalData(): Element? - override fun getStateAndArchive(storageData: StateMap, component: Any, componentName: String) = storageData.getStateAndArchive(componentName) + override final fun getState(storageData: StateMap, component: Any?, componentName: String) = storageData.getState(componentName) + + override fun archiveState(storageData: StateMap, componentName: String, serializedState: Element?) { + storageData.archive(componentName, serializedState) + } override fun hasState(storageData: StateMap, componentName: String) = storageData.hasState(componentName) @@ -239,7 +242,7 @@ fun setStateAndCloneIfNeed(componentName: String, newState: Element?, oldStates: return newStates } - prepareElement(newState) + newState.normalizeRootName() newLiveStates.put(componentName, newState) @@ -261,12 +264,13 @@ fun setStateAndCloneIfNeed(componentName: String, newState: Element?, oldStates: return newStates } -fun prepareElement(state: Element) { - if (state.getParent() != null) { - LOG.warn("State element must not have parent ${JDOMUtil.writeElement(state)}") - state.detach() +fun Element.normalizeRootName(): Element { + if (getParent() != null) { + LOG.warn("State element must not have parent ${JDOMUtil.writeElement(this)}") + detach() } - state.setName(FileStorageCoreUtil.COMPONENT) + setName(FileStorageCoreUtil.COMPONENT) + return this } private fun updateState(states: MutableMap, componentName: String, newState: Element?, newLiveStates: MutableMap) { @@ -275,7 +279,7 @@ private fun updateState(states: MutableMap, componentName: String, return } - prepareElement(newState) + newState.normalizeRootName() newLiveStates.put(componentName, newState) diff --git a/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt b/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt index 2276c9e24a8a..5e5c5d89d58d 100644 --- a/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt +++ b/platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt @@ -20,10 +20,14 @@ import com.intellij.openapi.components.* import com.intellij.openapi.vfs.CharsetToolkit import com.intellij.testFramework.* import com.intellij.util.SmartList +import com.intellij.util.xmlb.XmlSerializer import com.intellij.util.xmlb.XmlSerializerUtil +import com.intellij.util.xmlb.annotations.Attribute +import com.intellij.util.xmlb.serialize import gnu.trove.THashMap import org.assertj.core.api.Assertions.assertThat import org.intellij.lang.annotations.Language +import org.jdom.Element import org.junit.Before import org.junit.ClassRule import org.junit.Rule @@ -63,15 +67,15 @@ class ApplicationStoreTest { component.foo = "newValue" componentStore.save(SmartList()) - assertThat(streamProvider.data.get(RoamingType.DEFAULT)!!.get("proxy.settings.xml")).isEqualTo("\n" + " \n" + " \n" + "") + assertThat(streamProvider.data.get(RoamingType.DEFAULT)!!.get("proxy.xml")).isEqualTo("\n" + " \n" + " \n" + "") } - @Test fun testLoadFromStreamProvider() { + @Test fun `load from stream provider`() { val component = SeveralStoragesConfigured() val streamProvider = MyStreamProvider() val map = THashMap() - val fileSpec = "proxy.settings.xml" + val fileSpec = "proxy.xml" map.put(fileSpec, "\n \n \n") streamProvider.data.put(RoamingType.DEFAULT, map) @@ -92,7 +96,7 @@ class ApplicationStoreTest { private fun doRemoveDeprecatedStorageOnWrite(component: Foo) { val oldFile = writeConfig("other.xml", "") - writeConfig("proxy.settings.xml", "") + writeConfig("proxy.xml", "") testAppConfig.refreshVfs() @@ -100,11 +104,50 @@ class ApplicationStoreTest { assertThat(component.foo).isEqualTo("new") component.foo = "new2" - runInEdtAndWait { componentStore.save(SmartList()) } + saveStore() assertThat(oldFile).doesNotExist() } + @State(name = "A", storages = arrayOf(Storage(file = "a.xml"))) + private class A : PersistentStateComponent { + data class State(@Attribute var foo: String = "", @Attribute var bar: String = "") + + var state = State() + + override fun getState() = state.serialize() + + override fun loadState(state: Element) { + this.state = XmlSerializer.deserialize(state, javaClass()) + } + } + + @Test fun `don't save if only format is changed`() { + val oldContent = "" + val file = writeConfig("a.xml", oldContent) + val oldModificationTime = file.getLastModifiedTime() + testAppConfig.refreshVfs() + + val component = A() + componentStore.initComponent(component, false) + assertThat(component.state).isEqualTo(A.State("old")) + + saveStore() + + assertThat(file).hasContent(oldContent) + assertThat(oldModificationTime).isEqualTo(file.getLastModifiedTime()) + + component.state.bar = "2" + component.state.foo = "1" + saveStore() + + assertThat(file).hasContent("\n \n") + } + + private fun saveStore() { + runInEdtAndWait { componentStore.save(SmartList()) } + } + private fun writeConfig(fileName: String, Language("XML") data: String) = testAppConfig.writeChild(fileName, data) private class MyStreamProvider : StreamProvider { @@ -152,7 +195,7 @@ class ApplicationStoreTest { public var foo: String = "defaultValue" } - @State(name = "HttpConfigurable", storages = arrayOf(Storage(file = "proxy.settings.xml"), Storage(file = StoragePathMacros.APP_CONFIG + "/other.xml", deprecated = true))) + @State(name = "HttpConfigurable", storages = arrayOf(Storage(file = "proxy.xml"), Storage(file = StoragePathMacros.APP_CONFIG + "/other.xml", deprecated = true))) class SeveralStoragesConfigured : Foo(), PersistentStateComponent { override fun getState(): SeveralStoragesConfigured? { return this @@ -163,7 +206,7 @@ class ApplicationStoreTest { } } - @State(name = "HttpConfigurable", storages = arrayOf(Storage(file = "other.xml", deprecated = true), Storage(file = "${StoragePathMacros.APP_CONFIG}/proxy.settings.xml"))) + @State(name = "HttpConfigurable", storages = arrayOf(Storage(file = "other.xml", deprecated = true), Storage(file = "${StoragePathMacros.APP_CONFIG}/proxy.xml"))) class ActualStorageLast : Foo(), PersistentStateComponent { override fun getState() = this