Merge remote-tracking branch 'origin/master'

platform/configuration-store-impl/src/BinaryXmlOutputter.kt

@@ -1,41 +1,51 @@
 package com.intellij.configurationStore
 
-import com.intellij.util.containers.isNullOrEmpty
 import org.jdom.*
+import java.io.DataInputStream
 import java.io.DataOutputStream
+import java.io.InputStream
+import java.io.OutputStream
 
 private enum class TypeMarker {
   ELEMENT, CDATA, TEXT, ELEMENT_END
 }
 
-fun output(doc: Document, out: DataOutputStream) {
-  val content = doc.content
-  val size = content.size
-  for (i in 0..size - 1) {
-    val obj = content[i]
-    when (obj) {
-      is Element -> printElement(out, doc.rootElement)
-    }
-  }
-
-  out.flush()
-}
+fun writeElement(element: Element, out: OutputStream) = writeElement(element, DataOutputStream(out))
 
 fun writeElement(element: Element, out: DataOutputStream) {
-  printElement(out, element)
-  out.flush()
+  writeElement(out, element)
 }
 
-private fun printElement(out: DataOutputStream, element: Element) {
-  out.writeByte(TypeMarker.ELEMENT.ordinal)
+fun readElement(input: InputStream) = readElement(DataInputStream(input))
+
+fun readElement(input: DataInputStream): Element {
+  val element = Element(input.readUTF())
+  readAttributes(element, input)
+  readContent(element, input)
+  return element
+}
+
+private fun readContent(element: Element, input: DataInputStream) {
+  while (true) {
+    when (input.read()) {
+      TypeMarker.ELEMENT.ordinal -> element.addContent(readElement(input))
+      TypeMarker.TEXT.ordinal -> element.addContent(Text(input.readUTF()))
+      TypeMarker.CDATA.ordinal -> element.addContent(CDATA(input.readUTF()))
+      TypeMarker.ELEMENT_END.ordinal -> return
+    }
+  }
+}
+
+private fun writeElement(out: DataOutputStream, element: Element) {
   out.writeUTF(element.name)
 
-  val content = element.content
-  printAttributes(out, element.attributes)
+  writeAttributes(out, element.attributes)
 
+  val content = element.content
   for (item in content) {
     if (item is Element) {
-      printElement(out, item)
+      out.writeByte(TypeMarker.ELEMENT.ordinal)
+      writeElement(out, item)
     }
     else if (item is Text) {
       if (!isAllWhitespace(item)) {
@@ -51,19 +61,28 @@ private fun printElement(out: DataOutputStream, element: Element) {
   out.writeByte(TypeMarker.ELEMENT_END.ordinal)
 }
 
-private fun printAttributes(out: DataOutputStream, attributes: List<Attribute>?) {
-  if (attributes.isNullOrEmpty()) {
-    val size = attributes?.size ?: 0
-    if (size > 255) {
-      throw UnsupportedOperationException("attributes size > 255")
-    }
-    out.writeByte(size)
+private fun writeAttributes(out: DataOutputStream, attributes: List<Attribute>?) {
+  val size = attributes?.size ?: 0
+  out.write(size)
+  if (size == 0) {
     return
   }
 
-  for (attribute in attributes!!) {
-    out.writeUTF(attribute.name)
-    out.writeUTF(attribute.value)
+  if (size > 255) {
+    throw UnsupportedOperationException("attributes size > 255")
+  }
+  else {
+    for (attribute in attributes!!) {
+      out.writeUTF(attribute.name)
+      out.writeUTF(attribute.value)
+    }
+  }
+}
+
+private fun readAttributes(element: Element, input: DataInputStream) {
+  val size = input.readUnsignedByte()
+  for (i in 0..size - 1) {
+    element.setAttribute(Attribute(input.readUTF(), input.readUTF()))
   }
 }
 

platform/configuration-store-impl/src/ComponentStoreImpl.kt

@@ -34,7 +34,6 @@ import com.intellij.openapi.util.InvalidDataException
 import com.intellij.openapi.util.JDOMExternalizable
 import com.intellij.openapi.util.JDOMUtil
 import com.intellij.openapi.util.NamedJDOMExternalizable
-import com.intellij.openapi.util.registry.Registry
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.openapi.vfs.newvfs.impl.VfsRootAccess
 import com.intellij.ui.AppUIUtil
@@ -126,7 +125,7 @@ abstract class ComponentStoreImpl : IComponentStore {
       var timeLog = if (LOG.isDebugEnabled) StringBuilder(timeLogPrefix) else null
       for (name in names) {
         val start = if (timeLog == null) 0 else System.currentTimeMillis()
-        commitComponent(externalizationSession, components[name]!!, name)
+        commitComponent(externalizationSession, components.get(name)!!, name)
         timeLog?.let {
           val duration = System.currentTimeMillis() - start
           if (duration > 10) {
@@ -268,7 +267,8 @@ abstract class ComponentStoreImpl : IComponentStore {
         }
 
         val storage = storageManager.getStateStorage(storageSpec)
-        var stateGetter = if (isUseLoadedStateAsExisting(storage) && (ApplicationManager.getApplication().isUnitTestMode || Registry.`is`("use.loaded.state.as.existing", false))) {
+        // todo "ProjectModuleManager" investigate why after loadState we get empty state on getState, test CMakeWorkspaceContentRootsTest
+        var stateGetter = if (isUseLoadedStateAsExisting(storage) && name != "ProjectModuleManager") {
           (storage as? StorageBaseEx<*>)?.createGetSession(component, name, stateClass)
         }
         else {

platform/configuration-store-impl/src/StateMap.kt

@@ -18,56 +18,26 @@ package com.intellij.configurationStore
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.util.JDOMUtil
 import com.intellij.openapi.util.io.BufferExposingByteArrayOutputStream
-import com.intellij.openapi.util.registry.Registry
 import com.intellij.openapi.util.text.StringUtil
-import com.intellij.openapi.vfs.CharsetToolkit
 import com.intellij.util.ArrayUtil
 import com.intellij.util.SystemProperties
-import com.intellij.util.loadElement
 import gnu.trove.THashMap
 import org.iq80.snappy.SnappyInputStream
 import org.iq80.snappy.SnappyOutputStream
 import org.jdom.Element
-import org.jdom.output.Format
 import java.io.ByteArrayInputStream
-import java.io.DataOutputStream
-import java.io.OutputStreamWriter
 import java.util.*
 import java.util.concurrent.atomic.AtomicReferenceArray
 
-private val XML_FORMAT = Format.getRawFormat().setTextMode(Format.TextMode.TRIM).setOmitEncoding(true).setOmitDeclaration(true)
-
-// must be mot modified during app life
-private val isUseNewSaving by lazy { ApplicationManager.getApplication().isUnitTestMode || Registry.`is`("configuration.saving.v3", false) }
-
-private fun archiveState(state: Element): BufferExposingByteArrayOutputStream {
-  if (isUseNewSaving) {
-    return archiveStateBinary(state)
-  }
-  else {
-    return archiveStateXml(state)
-  }
-}
-
-fun archiveStateBinary(state: Element): BufferExposingByteArrayOutputStream {
+fun archiveState(state: Element): BufferExposingByteArrayOutputStream {
   val byteOut = BufferExposingByteArrayOutputStream()
-  DataOutputStream(SnappyOutputStream(byteOut)).use {
+  SnappyOutputStream(byteOut).use {
     writeElement(state, it)
   }
   return byteOut
 }
 
-fun archiveStateXml(state: Element): BufferExposingByteArrayOutputStream {
-  val byteOut = BufferExposingByteArrayOutputStream()
-  OutputStreamWriter(SnappyOutputStream(byteOut), CharsetToolkit.UTF8_CHARSET).use {
-    val xmlOutputter = JDOMUtil.MyXMLOutputter()
-    xmlOutputter.format = XML_FORMAT
-    xmlOutputter.output(state, it)
-  }
-  return byteOut
-}
-
-private fun unarchiveState(state: ByteArray) = loadElement(SnappyInputStream(ByteArrayInputStream(state)).reader())
+private fun unarchiveState(state: ByteArray) = SnappyInputStream(ByteArrayInputStream(state)).use { readElement(it) }
 
 fun getNewByteIfDiffers(key: String, newState: Any, oldState: ByteArray): ByteArray? {
   val newBytes: ByteArray
@@ -86,15 +56,16 @@ fun getNewByteIfDiffers(key: String, newState: Any, oldState: ByteArray): ByteAr
     }
   }
 
-  if (SystemProperties.getBooleanProperty("idea.log.changed.components", false)) {
+  val logChangedComponents = SystemProperties.getBooleanProperty("idea.log.changed.components", false)
+  if (ApplicationManager.getApplication().isUnitTestMode || logChangedComponents ) {
     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.info("Serialization error: serialized are different, but unserialized are equal")
+      throw IllegalStateException("$key serialization error - serialized are different, but unserialized are equal")
     }
-    else {
+    else if (logChangedComponents) {
       LOG.info("$key ${StringUtil.repeat("=", 80 - key.length)}\nBefore:\n$before\nAfter:\n$after")
     }
   }
@@ -203,9 +174,7 @@ class StateMap private constructor(private val names: Array<String>, private val
       return
     }
 
-    val currentState = states.get(index)
-    LOG.assertTrue(currentState is Element, currentState?.let { it.javaClass.name } ?: "null")
-    states.set(index, if (state == null) null else archiveState(state).toByteArray())
+    states.set(index, state?.let { archiveState(state).toByteArray() })
   }
 }
 

platform/configuration-store-impl/src/StateStorageManagerImpl.kt

@@ -337,7 +337,7 @@ open class StateStorageManagerImpl(private val rootTagName: String,
   fun collapseMacros(path: String): String {
     var result = path
     for ((key, value) in macros) {
-      result = StringUtil.replace(result, value, key)
+      result = result.replace(value, key)
     }
     return normalizeFileSpec(result)
   }
@@ -364,7 +364,7 @@ open class StateStorageManagerImpl(private val rootTagName: String,
     }
 
     private fun getExternalizationSession(storage: StateStorage): StateStorage.ExternalizationSession? {
-      var session = sessions[storage]
+      var session = sessions.get(storage)
       if (session == null) {
         session = storage.startExternalization()
         if (session != null) {
@@ -393,7 +393,7 @@ open class StateStorageManagerImpl(private val rootTagName: String,
           saveSessions.add(saveSession)
         }
       }
-      return ContainerUtil.notNullize(saveSessions)
+      return saveSessions ?: emptyList()
     }
   }
 

platform/configuration-store-impl/src/StorageBaseEx.kt

@@ -17,6 +17,7 @@ package com.intellij.configurationStore
 
 import com.intellij.openapi.components.PersistentStateComponent
 import com.intellij.openapi.components.impl.stores.StateStorageBase
+import com.intellij.openapi.util.JDOMUtil
 import org.jdom.Element
 
 abstract class StorageBaseEx<T : Any> : StateStorageBase<T>() {
@@ -65,7 +66,9 @@ class StateGetter<S : Any, T : Any>(private val component: PersistentStateCompon
       serializedState
     }
     else {
-      serializeState(stateAfterLoad)?.normalizeRootName()
+      serializeState(stateAfterLoad)?.normalizeRootName().let {
+        if (JDOMUtil.isEmpty(it)) null else it
+      }
     }
 
     storage.archiveState(storageData, componentName, serializedStateAfterLoad)

platform/configuration-store-impl/testSrc/BinaryXmlOutputterTest.kt

@@ -0,0 +1,28 @@
+package com.intellij.configurationStore
+
+import com.intellij.openapi.util.JDOMUtil
+import com.intellij.openapi.util.io.BufferExposingByteArrayOutputStream
+import com.intellij.util.loadElement
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Test
+
+class BinaryXmlOutputterTest {
+  @Test fun noAttributes() {
+    test("""<foo />""")
+  }
+
+  @Test fun attributes() {
+    test("""<foo bar="1" />""")
+  }
+
+  private fun test(xml: String) {
+    val byteOut = BufferExposingByteArrayOutputStream()
+    byteOut.use {
+      writeElement(loadElement(xml), it)
+    }
+
+    val xmlAfter = JDOMUtil.writeElement(byteOut.toByteArray().inputStream().use { readElement(it) })
+
+    assertThat(xml.trimIndent()).isEqualTo(xmlAfter)
+  }
+}

platform/projectModel-impl/src/com/intellij/openapi/module/impl/ModuleManagerImpl.java

@@ -218,8 +218,7 @@ public abstract class ModuleManagerImpl extends ModuleManager implements Project
     final List<ModulePath> paths = new ArrayList<ModulePath>();
     final Element modules = element.getChild(ELEMENT_MODULES);
     if (modules != null) {
-      for (final Object value : modules.getChildren(ELEMENT_MODULE)) {
-        Element moduleElement = (Element)value;
+      for (final Element moduleElement : modules.getChildren(ELEMENT_MODULE)) {
         final String fileUrlValue = moduleElement.getAttributeValue(ATTRIBUTE_FILEURL);
         final String filepath;
         if (fileUrlValue != null) {

platform/util/resources/misc/registry.properties

@@ -694,9 +694,7 @@ testDiscovery.enabled=false
 ruby.remote.debugger.supports.catchpoint.removal=true
 
 use.read.action.to.init.service=false
-use.loaded.state.as.existing=true
 saving.state.in.new.format.is.allowed=false
-configuration.saving.v3=false
 
 ide.mac.new.color.picker=false