don't save if only format is changed

2026-09-27 10:03:11 +07:00 · 2015-09-02 07:43:50 +02:00
parent e5eb8fda18
commit d884934843
5 changed files with 162 additions and 26 deletions
@@ -208,7 +208,7 @@ abstract class ComponentStoreImpl : IComponentStore {
      LOG.error(e)
    }

-    val element = storageManager.getOldStorage(component, componentName, StateStorageOperation.READ)?.getState(component, componentName, javaClass<Element>()) ?: return null
+    val element = storageManager.getOldStorage(component, componentName, StateStorageOperation.READ)?.getState(component, componentName, javaClass<Element>(), 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
    }
  }
-}
+}
@@ -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<String>, private val states: AtomicReferenceArray<Any>) {
+class StateMap private constructor(private val names: Array<String>, private val states: AtomicReferenceArray<Any?>) {
  companion object {
    private val LOG = Logger.getInstance(javaClass<StateMap>())

@@ -51,7 +51,7 @@ class StateMap private constructor(private val names: Array<String>, private val
        Arrays.sort(names)
      }

-      val states = AtomicReferenceArray<Any>(names.size())
+      val states = AtomicReferenceArray<Any?>(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<String>, 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<String>, 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))
+  }
 }
@@ -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<T : Any> : StateStorageBase<T>() {
+  fun <S : Any> createGetSession(component: PersistentStateComponent<S>, componentName: String, stateClass: Class<S>, 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<S : Any, T : Any>(private val component: PersistentStateComponent<S>, private val componentName: String, private val storageData: T, private val stateClass: Class<S>, private val storage: StorageBaseEx<T>) {
+  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)
+  }
+}
@@ -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<StateMap>() {
+                                                       provider: StreamProvider? = null) : StorageBaseEx<StateMap>() {
  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<String, Any>, componentName: String, newState: Element?, newLiveStates: MutableMap<String, Element>) {
@@ -275,7 +279,7 @@ private fun updateState(states: MutableMap<String, Any>, componentName: String,
    return
  }

-  prepareElement(newState)
+  newState.normalizeRootName()

  newLiveStates.put(componentName, newState)

@@ -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("<application>\n" + "  <component name=\"HttpConfigurable\">\n" + "    <option name=\"foo\" value=\"newValue\" />\n" + "  </component>\n" + "</application>")
+    assertThat(streamProvider.data.get(RoamingType.DEFAULT)!!.get("proxy.xml")).isEqualTo("<application>\n" + "  <component name=\"HttpConfigurable\">\n" + "    <option name=\"foo\" value=\"newValue\" />\n" + "  </component>\n" + "</application>")
  }

-  @Test fun testLoadFromStreamProvider() {
+  @Test fun `load from stream provider`() {
    val component = SeveralStoragesConfigured()

    val streamProvider = MyStreamProvider()
    val map = THashMap<String, String>()
-    val fileSpec = "proxy.settings.xml"
+    val fileSpec = "proxy.xml"
    map.put(fileSpec, "<application>\n  <component name=\"HttpConfigurable\">\n    <option name=\"foo\" value=\"newValue\" />\n  </component>\n</application>")
    streamProvider.data.put(RoamingType.DEFAULT, map)

@@ -92,7 +96,7 @@ class ApplicationStoreTest {

  private fun doRemoveDeprecatedStorageOnWrite(component: Foo) {
    val oldFile = writeConfig("other.xml", "<application><component name=\"HttpConfigurable\"><option name=\"foo\" value=\"old\" /></component></application>")
-    writeConfig("proxy.settings.xml", "<application><component name=\"HttpConfigurable\"><option name=\"foo\" value=\"new\" /></component></application>")
+    writeConfig("proxy.xml", "<application><component name=\"HttpConfigurable\"><option name=\"foo\" value=\"new\" /></component></application>")

    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<Element> {
+    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<State>())
+    }
+  }
+
+  @Test fun `don't save if only format is changed`() {
+    val oldContent = "<application><component name=\"A\" foo=\"old\" deprecated=\"old\"/></application>"
+    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("<application>\n  <component name=\"A\" foo=\"1\" bar=\"2\" />\n</application>")
+  }
+
+  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<SeveralStoragesConfigured> {
    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<ActualStorageLast> {
    override fun getState() = this