From 9800b7ab97043f4901a5f1dbcfbe06338741a09a Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Thu, 3 Sep 2015 13:24:14 +0200 Subject: [PATCH] =?UTF-8?q?simplify=20DirectoryBasedStorage=20=E2=80=94=20?= =?UTF-8?q?only=20one=20component=20could=20be=20stored=20add=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/DirectoryBasedStorage.kt | 335 ++++++------------ .../src/SaveSessionBase.kt | 6 +- .../src/SchemeManagerImpl.kt | 11 +- .../configuration-store-impl/src/StateMap.kt | 64 +++- .../src/StateStorageManagerImpl.kt | 3 +- .../src/StorageVirtualFileTracker.kt | 4 +- .../src/XmlElementStorage.kt | 68 +--- .../testSrc/DirectoryBasedStorageTest.kt | 90 +++++ .../testSrc/ProjectStoreTest.kt | 3 +- .../testSrc/StoreTestSuite.kt | 3 +- .../testSrc/XmlElementStorageTest.kt | 4 +- .../components/impl/stores/StorageUtil.java | 2 - .../openapi/components/StateSplitter.java | 5 +- .../openapi/components/StateStorage.java | 2 +- .../com/intellij/core/CoreProjectLoader.java | 4 +- .../impl/stores/DirectoryStorageUtil.java | 27 +- platform/testFramework/testFramework.iml | 1 + .../testFramework/TemporaryDirectory.kt | 2 +- .../com/intellij/testFramework/assertJ.kt | 42 +++ .../eclipse/config/ClasspathSaveSession.java | 2 +- 20 files changed, 337 insertions(+), 341 deletions(-) create mode 100644 platform/configuration-store-impl/testSrc/DirectoryBasedStorageTest.kt create mode 100644 platform/testFramework/testSrc/com/intellij/testFramework/assertJ.kt diff --git a/platform/configuration-store-impl/src/DirectoryBasedStorage.kt b/platform/configuration-store-impl/src/DirectoryBasedStorage.kt index 75b0933332bb..ffdcfa4330a8 100644 --- a/platform/configuration-store-impl/src/DirectoryBasedStorage.kt +++ b/platform/configuration-store-impl/src/DirectoryBasedStorage.kt @@ -15,7 +15,7 @@ */ package com.intellij.configurationStore -import com.intellij.openapi.application.WriteAction +import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.components.StateSplitter import com.intellij.openapi.components.StateSplitterEx import com.intellij.openapi.components.StateStorage @@ -29,7 +29,6 @@ import com.intellij.openapi.util.Pair import com.intellij.openapi.vfs.CharsetToolkit import com.intellij.openapi.vfs.LocalFileSystem import com.intellij.openapi.vfs.VirtualFile -import com.intellij.util.ArrayUtil import com.intellij.util.LineSeparator import com.intellij.util.SmartList import com.intellij.util.SystemProperties @@ -41,321 +40,199 @@ import java.io.FileNotFoundException import java.io.IOException import java.nio.ByteBuffer -open class DirectoryBasedStorage(private val myPathMacroSubstitutor: TrackingPathMacroSubstitutor?, private val myDir: File, private val mySplitter: StateSplitter) : StateStorageBase>() { +open class DirectoryBasedStorage(private val dir: File, + private val splitter: StateSplitter, + private val pathMacroSubstitutor: TrackingPathMacroSubstitutor? = null) : StateStorageBase() { private volatile var virtualFile: VirtualFile? = null - public fun setVirtualDir(dir: VirtualFile?) { + private var componentName: String? = null + + fun setVirtualDir(dir: VirtualFile?) { virtualFile = dir } override fun analyzeExternalChangesAndUpdateIfNeed(componentNames: MutableSet) { // todo reload only changed file, compute diff - val oldData = storageDataRef.get() val newData = loadData() storageDataRef.set(newData) - if (oldData == null) { - componentNames.addAll(newData.keySet()) + if (componentName != null) { + componentNames.add(componentName!!) + } + } + + override fun getSerializedState(storageData: StateMap, component: Any?, componentName: String, archive: Boolean): Element? { + this.componentName = componentName + val state = Element(FileStorageCoreUtil.COMPONENT) + if (storageData.isEmpty()) { + return state + } + + if (splitter is StateSplitterEx) { + for (fileName in storageData.keys()) { + val subState = storageData.getState(fileName, archive) ?: return null + splitter.mergeStateInto(state, subState) + } } else { - componentNames.addAll(oldData.keySet()) - componentNames.addAll(newData.keySet()) + val subElements = SmartList() + for (fileName in storageData.keys()) { + val subState = storageData.getState(fileName, archive) ?: return null + subElements.add(subState) + } + + if (!subElements.isEmpty()) { + splitter.mergeStatesInto(state, subElements.toTypedArray()) + } } + return state } - override fun getSerializedState(storageData: Map, component: Any?, componentName: String, archive: Boolean) = getCompositeStateAndArchive(storageData, componentName, mySplitter) - - override fun loadData(): MutableMap { - return fromMap(DirectoryStorageUtil.loadFrom(getVirtualFile(), myPathMacroSubstitutor)) - } + override fun loadData() = StateMap.fromMap(DirectoryStorageUtil.loadFrom(getVirtualFile(), pathMacroSubstitutor)) private fun getVirtualFile(): VirtualFile? { - var virtualFile = virtualFile - if (virtualFile == null) { - virtualFile = LocalFileSystem.getInstance().findFileByIoFile(myDir) - virtualFile = virtualFile + var result = virtualFile + if (result == null) { + result = LocalFileSystem.getInstance().findFileByIoFile(dir) + virtualFile = result } - return virtualFile + return result } - override fun startExternalization(): StateStorage.ExternalizationSession? { - return if (checkIsSavingDisabled()) null else MySaveSession(this, getStorageData()) - } + override fun startExternalization(): StateStorage.ExternalizationSession? = if (checkIsSavingDisabled()) null else MySaveSession(this, getStorageData()) - private class MySaveSession(private val storage: DirectoryBasedStorage, private val originalStates: Map) : SaveSessionBase() { - private var copiedStorageData: MutableMap>? = null + private class MySaveSession(private val storage: DirectoryBasedStorage, private val originalStates: StateMap) : SaveSessionBase() { + private var copiedStorageData: MutableMap? = null private val dirtyFileNames = SmartHashSet() - private val removedFileNames = SmartHashSet() + private var someFileRemoved = false - private fun getFileNames(states: Map, componentName: String): Array { - val fileToState = states.get(componentName) - return if (fileToState == null || fileToState.isEmpty()) ArrayUtil.EMPTY_STRING_ARRAY else fileToState.keys() - } + override fun setSerializedState(componentName: String, element: Element?) { + storage.componentName = componentName - override fun setSerializedState(component: Any, componentName: String, element: Element?) { - removedFileNames.addAll(getFileNames(originalStates, componentName)) if (JDOMUtil.isEmpty(element)) { - doSetState(componentName, null, null) + if (copiedStorageData != null) { + copiedStorageData!!.clear() + } + else if (!originalStates.isEmpty()) { + copiedStorageData = THashMap() + } } else { - for (pair in storage.mySplitter.splitState(element)) { - removedFileNames.remove(pair.second) - doSetState(componentName, pair.second, pair.first) + val stateAndFileNameList = storage.splitter.splitState(element!!) + for (pair in stateAndFileNameList) { + doSetState(pair.second, pair.first) } - if (!removedFileNames.isEmpty()) { - for (fileName in removedFileNames) { - doSetState(componentName, fileName, null) + outerLoop@ + for (key in originalStates.keys()) { + for (pair in stateAndFileNameList) { + if (pair.second == key) { + continue@outerLoop + } } + + if (copiedStorageData == null) { + copiedStorageData = originalStates.toMutableMap() + } + someFileRemoved = true + copiedStorageData!!.remove(key) } } } - private fun doSetState(componentName: String, fileName: String?, subState: Element?) { + private fun doSetState(fileName: String, subState: Element) { if (copiedStorageData == null) { - copiedStorageData = setStateAndCloneIfNeed(componentName, fileName, subState, originalStates) - if (copiedStorageData != null && fileName != null) { + copiedStorageData = setStateAndCloneIfNeed(fileName, subState, originalStates) + if (copiedStorageData != null) { dirtyFileNames.add(fileName) } } - else if (DirectoryBasedStorage.setState(copiedStorageData!!, componentName, fileName, subState) != null && fileName != null) { + else if (updateState(copiedStorageData!!, fileName, subState)) { dirtyFileNames.add(fileName) } } - override fun createSaveSession(): StateStorage.SaveSession? { - return if (storage.checkIsSavingDisabled() || copiedStorageData == null) null else this - } + override fun createSaveSession() = if (storage.checkIsSavingDisabled() || copiedStorageData == null) null else this override fun save() { + val stateMap = StateMap.fromMap(copiedStorageData!!) + var dir = storage.getVirtualFile() if (copiedStorageData!!.isEmpty()) { if (dir != null && dir.exists()) { deleteFile(this, dir) } - storage.setStorageData(copiedStorageData!!) + storage.setStorageData(stateMap) return } if (dir == null || !dir.isValid()) { - dir = createDir(storage.myDir, this) + dir = createDir(storage.dir, this) storage.virtualFile = dir } if (!dirtyFileNames.isEmpty()) { - saveStates(dir) + saveStates(dir, stateMap) } - if (dir.exists() && !removedFileNames.isEmpty()) { + if (someFileRemoved && dir.exists()) { deleteFiles(dir) } - storage.setStorageData(copiedStorageData!!) + storage.setStorageData(stateMap) } - private fun saveStates(dir: VirtualFile) { + private fun saveStates(dir: VirtualFile, states: StateMap) { val storeElement = Element(FileStorageCoreUtil.COMPONENT) - for (componentNameToFileNameToStates in copiedStorageData!!.entrySet()) { - for (entry in componentNameToFileNameToStates.getValue().entrySet()) { - val fileName = entry.getKey() - if (!dirtyFileNames.contains(fileName)) { - continue - } + for (fileName in states.keys()) { + if (!dirtyFileNames.contains(fileName)) { + continue + } - var element: Element? = null - try { - element = StateMap.stateToElement(fileName, entry.getValue()) - storage.myPathMacroSubstitutor?.collapsePaths(element) + var element: Element? = null + try { + element = states.getElement(fileName, null) + storage.pathMacroSubstitutor?.collapsePaths(element) - storeElement.setAttribute(FileStorageCoreUtil.NAME, componentNameToFileNameToStates.getKey()) - storeElement.addContent(element) + storeElement.setAttribute(FileStorageCoreUtil.NAME, storage.componentName!!) + storeElement.addContent(element) - val file = getFile(fileName, dir, this) - // we don't write xml prolog due to historical reasons (and should not in any case) - writeFile(null, this, file, storeElement, LineSeparator.fromString(if (file.exists()) loadFile(file).second else SystemProperties.getLineSeparator()), false) - } - catch (e: IOException) { - LOG.error(e) - } - finally { - if (element != null) { - element.detach() - } + val file = getFile(fileName, dir, this) + // we don't write xml prolog due to historical reasons (and should not in any case) + writeFile(null, this, file, storeElement, LineSeparator.fromString(if (file.exists()) loadFile(file).second else SystemProperties.getLineSeparator()), false) + } + catch (e: IOException) { + LOG.error(e) + } + finally { + if (element != null) { + element.detach() } } } } - throws(IOException::class) private fun deleteFiles(dir: VirtualFile) { - val token = WriteAction.start() - try { + runWriteAction { for (file in dir.getChildren()) { - if (removedFileNames.contains(file.getName())) { + val fileName = file.getName() + if (fileName.endsWith(FileStorageCoreUtil.DEFAULT_EXT) && !copiedStorageData!!.containsKey(fileName)) { try { file.delete(this) } catch (e: FileNotFoundException) { throw ReadOnlyModificationException(file, e, null) } - } } } - finally { - token.finish() - } } } - private fun setStorageData(newStates: Map>) { - storageDataRef.set(fromMap(newStates)) + private fun setStorageData(newStates: StateMap) { + storageDataRef.set(newStates) } - override fun hasState(storageData: Map, componentName: String) = storageData.get(componentName)?.hasStates() ?: false - - companion object { - fun setState(states: MutableMap>, componentName: String, fileName: String?, newState: Element?): Any? { - var fileToState = states.get(componentName) - if (fileName == null || newState == null || JDOMUtil.isEmpty(newState)) { - if (fileToState == null) { - return null - } - else if (fileName == null) { - return states.remove(componentName) - } - else { - val oldState = fileToState.remove(fileName) - if (fileToState.isEmpty()) { - states.remove(componentName) - } - return oldState - } - } - - if (fileToState == null) { - fileToState = THashMap() - fileToState.put(fileName, newState) - states.put(componentName, fileToState) - } - else { - val oldState = fileToState.get(fileName) - var newBytes: ByteArray? = null - if (oldState is Element) { - if (JDOMUtil.areElementsEqual(oldState as Element?, newState)) { - return null - } - } - else if (oldState != null) { - newBytes = StateMap.getNewByteIfDiffers(fileName, newState, oldState as ByteArray) ?: return null - } - - fileToState.put(fileName, newBytes ?: newState) - } - return newState - } - - public fun Map.toMutableMap(): MutableMap> { - val map = THashMap>(size()) - for (entry in entrySet()) { - map.put(entry.getKey(), entry.getValue().toMutableMap()) - } - return map - } - - fun fromMap(map: Map>): MutableMap { - val states = THashMap(map.size()) - for (entry in map.entrySet()) { - states.put(entry.getKey(), StateMap.fromMap(entry.getValue())) - } - return states - } - - private fun getCompositeStateAndArchive(states: Map, componentName: String, SuppressWarnings("deprecation") splitter: StateSplitter): Element? { - val fileToState = states.get(componentName) - val state = Element(FileStorageCoreUtil.COMPONENT) - if (fileToState == null || fileToState.isEmpty()) { - return state - } - - if (splitter is StateSplitterEx) { - for (fileName in fileToState.keys()) { - val subState = fileToState.getStateAndArchive(fileName) ?: return null - splitter.mergeStateInto(state, subState) - } - } - else { - val subElements = SmartList() - for (fileName in fileToState.keys()) { - val subState = fileToState.getStateAndArchive(fileName) ?: return null - subElements.add(subState) - } - - if (!subElements.isEmpty()) { - splitter.mergeStatesInto(state, subElements.toTypedArray()) - } - } - return state - } - - public fun setStateAndCloneIfNeed(componentName: String, fileName: String?, newState: Element?, oldStates: Map): MutableMap>? { - val fileToState = oldStates.get(componentName) - val oldState: Any? = if (fileToState == null || fileName == null) null else fileToState.get(fileName) - if (fileName == null || newState == null || JDOMUtil.isEmpty(newState)) { - if (fileName == null) { - if (fileToState == null) { - return null - } - } - else if (oldState == null) { - return null - } - - val newStorageData = oldStates.toMutableMap() - if (fileName == null) { - newStorageData.remove(componentName) - } - else { - val clonedFileToState = newStorageData.get(componentName) - if (clonedFileToState!!.size() == 1) { - newStorageData.remove(componentName) - } - else { - clonedFileToState.remove(fileName) - if (clonedFileToState.isEmpty()) { - newStorageData.remove(componentName) - } - } - } - return newStorageData - } - - var newBytes: ByteArray? = null - if (oldState is Element) { - if (JDOMUtil.areElementsEqual(oldState as Element?, newState)) { - return null - } - } - else if (oldState != null) { - newBytes = StateMap.getNewByteIfDiffers(componentName, newState, oldState as ByteArray) - if (newBytes == null) { - return null - } - } - - val newStorageData = oldStates.toMutableMap() - put(newStorageData, componentName, fileName, newBytes ?: newState) - return newStorageData - } - - private fun put(states: MutableMap>, componentName: String, fileName: String, state: Any) { - var fileToState: MutableMap? = states.get(componentName) - if (fileToState == null) { - fileToState = THashMap() - states.put(componentName, fileToState) - } - fileToState.put(fileName, state) - } - } + override fun hasState(storageData: StateMap, componentName: String) = storageData.hasStates() } private val NON_EXISTENT_FILE_DATA = Pair.create(null, SystemProperties.getLineSeparator()) diff --git a/platform/configuration-store-impl/src/SaveSessionBase.kt b/platform/configuration-store-impl/src/SaveSessionBase.kt index 5f7214235d93..75fad25736bd 100644 --- a/platform/configuration-store-impl/src/SaveSessionBase.kt +++ b/platform/configuration-store-impl/src/SaveSessionBase.kt @@ -25,7 +25,7 @@ import com.intellij.util.xmlb.XmlSerializer import org.jdom.Element abstract class SaveSessionBase : StateStorage.SaveSession, StateStorage.ExternalizationSession, SafeWriteRequestor { - override final fun setState(component: Any, componentName: String, state: Any) { + override final fun setState(component: Any?, componentName: String, state: Any) { val element: Element? try { element = serializeState(state) @@ -39,10 +39,10 @@ abstract class SaveSessionBase : StateStorage.SaveSession, StateStorage.External return } - setSerializedState(component, componentName, element) + setSerializedState(componentName, element) } - protected abstract fun setSerializedState(component: Any, componentName: String, element: Element?) + protected abstract fun setSerializedState(componentName: String, element: Element?) } private val skipDefaultsSerializationFilter = ThreadLocal>() diff --git a/platform/configuration-store-impl/src/SchemeManagerImpl.kt b/platform/configuration-store-impl/src/SchemeManagerImpl.kt index 03e5d3f1b392..b47ac1706896 100644 --- a/platform/configuration-store-impl/src/SchemeManagerImpl.kt +++ b/platform/configuration-store-impl/src/SchemeManagerImpl.kt @@ -23,7 +23,8 @@ import com.intellij.openapi.application.invokeAndWaitIfNeed import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.components.RoamingType import com.intellij.openapi.components.impl.ServiceManagerImpl -import com.intellij.openapi.components.impl.stores.StorageUtil +import com.intellij.openapi.components.impl.stores.FileStorageCoreUtil +import com.intellij.openapi.components.impl.stores.FileStorageCoreUtil.DEFAULT_EXT import com.intellij.openapi.components.service import com.intellij.openapi.extensions.AbstractExtensionPointBean import com.intellij.openapi.options.* @@ -84,7 +85,7 @@ public class SchemeManagerImpl(private val updateExtension = processor.isUpgradeNeeded() } else { - schemeExtension = StorageUtil.DEFAULT_EXT + schemeExtension = FileStorageCoreUtil.DEFAULT_EXT updateExtension = false } @@ -226,8 +227,8 @@ public class SchemeManagerImpl(private val return if (StringUtilRt.endsWithIgnoreCase(fileName, schemeExtension)) { schemeExtension } - else if (StringUtilRt.endsWithIgnoreCase(fileName, StorageUtil.DEFAULT_EXT)) { - StorageUtil.DEFAULT_EXT + else if (StringUtilRt.endsWithIgnoreCase(fileName, DEFAULT_EXT)) { + DEFAULT_EXT } else if (allowAny) { PathUtil.getFileExtension(fileName.toString())!! @@ -392,7 +393,7 @@ public class SchemeManagerImpl(private val private val ExternalizableScheme.fileName: String? get() = schemeToInfo.get(this)?.fileNameWithoutExtension - private fun canRead(name: CharSequence) = updateExtension && StringUtilRt.endsWithIgnoreCase(name, StorageUtil.DEFAULT_EXT) || StringUtilRt.endsWithIgnoreCase(name, schemeExtension) + private fun canRead(name: CharSequence) = updateExtension && StringUtilRt.endsWithIgnoreCase(name, DEFAULT_EXT) || StringUtilRt.endsWithIgnoreCase(name, schemeExtension) private fun readSchemeFromFile(file: VirtualFile, duringLoad: Boolean): E? { val fileName = file.getNameSequence() diff --git a/platform/configuration-store-impl/src/StateMap.kt b/platform/configuration-store-impl/src/StateMap.kt index 93a11d6deafa..146534159ae9 100644 --- a/platform/configuration-store-impl/src/StateMap.kt +++ b/platform/configuration-store-impl/src/StateMap.kt @@ -15,7 +15,6 @@ */ package com.intellij.configurationStore -import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.util.JDOMUtil import com.intellij.openapi.util.io.BufferExposingByteArrayOutputStream import com.intellij.openapi.util.text.StringUtil @@ -35,8 +34,6 @@ import java.util.concurrent.atomic.AtomicReferenceArray class StateMap private constructor(private val names: Array, private val states: AtomicReferenceArray) { companion object { - private val LOG = Logger.getInstance(javaClass()) - private val XML_FORMAT = Format.getRawFormat().setTextMode(Format.TextMode.TRIM).setOmitEncoding(true).setOmitDeclaration(true) val EMPTY = StateMap(emptyArray(), AtomicReferenceArray(0)) @@ -118,7 +115,7 @@ class StateMap private constructor(private val names: Array, private val return if (index < 0) null else states.get(index) } - fun getElement(key: String, newLiveStates: Map) = stateToElement(key, get(key), newLiveStates) + fun getElement(key: String, newLiveStates: Map? = null) = stateToElement(key, get(key), newLiveStates) fun isEmpty() = names.isEmpty() @@ -150,8 +147,6 @@ class StateMap private constructor(private val names: Array, private val } } - fun getStateAndArchive(key: String) = getState(key, true) - fun getState(key: String, archive: Boolean = false): Element? { val index = Arrays.binarySearch(names, key) if (index < 0) { @@ -175,4 +170,61 @@ class StateMap private constructor(private val names: Array, private val LOG.assertTrue(currentState is Element) states.set(index, if (state == null) null else archiveState(state)) } +} + +fun setStateAndCloneIfNeed(key: String, newState: Element?, oldStates: StateMap, newLiveStates: MutableMap? = null): MutableMap? { + val oldState = oldStates.get(key) + if (newState == null || JDOMUtil.isEmpty(newState)) { + if (oldState == null) { + return null + } + + val newStates = oldStates.toMutableMap() + newStates.remove(key) + return newStates + } + + newLiveStates?.put(key, newState) + + var newBytes: ByteArray? = null + if (oldState is Element) { + if (JDOMUtil.areElementsEqual(oldState as Element?, newState)) { + return null + } + } + else if (oldState != null) { + newBytes = StateMap.getNewByteIfDiffers(key, newState, oldState as ByteArray) + if (newBytes == null) { + return null + } + } + + val newStates = oldStates.toMutableMap() + newStates.put(key, newBytes ?: newState) + return newStates +} + +// true if updated (not equals to previous state) +private fun updateState(states: MutableMap, key: String, newState: Element?, newLiveStates: MutableMap? = null): Boolean { + if (newState == null || JDOMUtil.isEmpty(newState)) { + states.remove(key) + return true + } + + newLiveStates?.put(key, newState) + + val oldState = states.get(key) + + var newBytes: ByteArray? = null + if (oldState is Element) { + if (JDOMUtil.areElementsEqual(oldState as Element?, newState)) { + return false + } + } + else if (oldState != null) { + newBytes = StateMap.getNewByteIfDiffers(key, newState, oldState as ByteArray) ?: return false + } + + states.put(key, newBytes ?: newState) + return true } \ No newline at end of file diff --git a/platform/configuration-store-impl/src/StateStorageManagerImpl.kt b/platform/configuration-store-impl/src/StateStorageManagerImpl.kt index daa992ac9f8c..ebd0de554a1c 100644 --- a/platform/configuration-store-impl/src/StateStorageManagerImpl.kt +++ b/platform/configuration-store-impl/src/StateStorageManagerImpl.kt @@ -213,7 +213,8 @@ open class StateStorageManagerImpl(private val rootTagName: String, return storage } - private class MyDirectoryStorage(override val storageManager: StateStorageManagerImpl, file: File, splitter: StateSplitter) : DirectoryBasedStorage(storageManager.pathMacroSubstitutor, file, splitter), StorageVirtualFileTracker.TrackedStorage + private class MyDirectoryStorage(override val storageManager: StateStorageManagerImpl, file: File, splitter: StateSplitter) : + DirectoryBasedStorage(file, splitter, storageManager.pathMacroSubstitutor), StorageVirtualFileTracker.TrackedStorage private class MyFileStorage(override val storageManager: StateStorageManagerImpl, file: File, diff --git a/platform/configuration-store-impl/src/StorageVirtualFileTracker.kt b/platform/configuration-store-impl/src/StorageVirtualFileTracker.kt index f9c22afb4824..f35b41c11d06 100644 --- a/platform/configuration-store-impl/src/StorageVirtualFileTracker.kt +++ b/platform/configuration-store-impl/src/StorageVirtualFileTracker.kt @@ -1,8 +1,8 @@ package com.intellij.configurationStore import com.intellij.openapi.components.StateStorage +import com.intellij.openapi.components.impl.stores.FileStorageCoreUtil import com.intellij.openapi.components.impl.stores.StateStorageManager -import com.intellij.openapi.components.impl.stores.StorageUtil import com.intellij.openapi.util.text.StringUtilRt import com.intellij.openapi.vfs.VfsUtil import com.intellij.openapi.vfs.VirtualFile @@ -76,7 +76,7 @@ class StorageVirtualFileTracker(private val messageBus: MessageBus) { // but we should detect deletion - but again, it is not supported case. So, we don't check if some of registered storages located inside changed directory. // but if we have DirectoryBasedStorage, we check - if file located inside it - if (storage == null && hasDirectoryBasedStorages && StringUtilRt.endsWithIgnoreCase(path, StorageUtil.DEFAULT_EXT)) { + if (storage == null && hasDirectoryBasedStorages && StringUtilRt.endsWithIgnoreCase(path, FileStorageCoreUtil.DEFAULT_EXT)) { storage = filePathToStorage.get(VfsUtil.getParentDir(path)) } } diff --git a/platform/configuration-store-impl/src/XmlElementStorage.kt b/platform/configuration-store-impl/src/XmlElementStorage.kt index 90f158aed3e6..52361b24e482 100644 --- a/platform/configuration-store-impl/src/XmlElementStorage.kt +++ b/platform/configuration-store-impl/src/XmlElementStorage.kt @@ -29,7 +29,7 @@ import java.io.IOException abstract class XmlElementStorage protected constructor(protected val fileSpec: String, protected val rootElementName: String, - protected val pathMacroSubstitutor: TrackingPathMacroSubstitutor?, + protected val pathMacroSubstitutor: TrackingPathMacroSubstitutor? = null, roamingType: RoamingType? = RoamingType.DEFAULT, provider: StreamProvider? = null) : StorageBaseEx() { val roamingType: RoamingType = roamingType ?: RoamingType.DEFAULT @@ -116,7 +116,8 @@ abstract class XmlElementStorage protected constructor(protected val fileSpec: S override fun createSaveSession() = if (storage.checkIsSavingDisabled() || copiedStates == null) null else this - override fun setSerializedState(component: Any, componentName: String, element: Element?) { + override fun setSerializedState(componentName: String, element: Element?) { + element?.normalizeRootName() if (copiedStates == null) { copiedStates = setStateAndCloneIfNeed(componentName, element, originalStates, newLiveStates) } @@ -127,7 +128,7 @@ abstract class XmlElementStorage protected constructor(protected val fileSpec: S override fun save() { val stateMap = StateMap.fromMap(copiedStates!!) - var element = save(stateMap, newLiveStates, storage.rootElementName) + var element = save(stateMap, storage.rootElementName, newLiveStates) if (element == null || JDOMUtil.isEmpty(element)) { element = null } @@ -197,7 +198,7 @@ abstract class XmlElementStorage protected constructor(protected val fileSpec: S } } -private fun save(states: StateMap, newLiveStates: Map, rootElementName: String): Element? { +fun save(states: StateMap, rootElementName: String, newLiveStates: Map? = null): Element? { if (states.isEmpty()) { return null } @@ -230,40 +231,6 @@ private fun save(states: StateMap, newLiveStates: Map, rootElem return rootElement } -fun setStateAndCloneIfNeed(componentName: String, newState: Element?, oldStates: StateMap, newLiveStates: MutableMap): MutableMap? { - val oldState = oldStates.get(componentName) - if (newState == null || JDOMUtil.isEmpty(newState)) { - if (oldState == null) { - return null - } - - val newStates = oldStates.toMutableMap() - newStates.remove(componentName) - return newStates - } - - newState.normalizeRootName() - - newLiveStates.put(componentName, newState) - - var newBytes: ByteArray? = null - if (oldState is Element) { - if (JDOMUtil.areElementsEqual(oldState as Element?, newState)) { - return null - } - } - else if (oldState != null) { - newBytes = StateMap.getNewByteIfDiffers(componentName, newState, oldState as ByteArray) - if (newBytes == null) { - return null - } - } - - val newStates = oldStates.toMutableMap() - newStates.put(componentName, newBytes ?: newState) - return newStates -} - fun Element.normalizeRootName(): Element { if (getParent() != null) { LOG.warn("State element must not have parent ${JDOMUtil.writeElement(this)}") @@ -273,31 +240,6 @@ fun Element.normalizeRootName(): Element { return this } -private fun updateState(states: MutableMap, componentName: String, newState: Element?, newLiveStates: MutableMap) { - if (newState == null || JDOMUtil.isEmpty(newState)) { - states.remove(componentName) - return - } - - newState.normalizeRootName() - - newLiveStates.put(componentName, newState) - - val oldState = states.get(componentName) - - var newBytes: ByteArray? = null - if (oldState is Element) { - if (JDOMUtil.areElementsEqual(oldState as Element?, newState)) { - return - } - } - else if (oldState != null) { - newBytes = StateMap.getNewByteIfDiffers(componentName, newState, oldState as ByteArray) ?: return - } - - states.put(componentName, newBytes ?: newState) -} - // newStorageData - myStates contains only live (unarchived) states private fun StateMap.getChangedComponentNames(newStates: StateMap): Set { val bothStates = keys().toMutableSet() diff --git a/platform/configuration-store-impl/testSrc/DirectoryBasedStorageTest.kt b/platform/configuration-store-impl/testSrc/DirectoryBasedStorageTest.kt new file mode 100644 index 000000000000..74dabf111e0a --- /dev/null +++ b/platform/configuration-store-impl/testSrc/DirectoryBasedStorageTest.kt @@ -0,0 +1,90 @@ +/* + * 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.MainConfigurationStateSplitter +import com.intellij.openapi.components.StateStorage +import com.intellij.openapi.components.impl.stores.StateStorageBase +import com.intellij.openapi.util.JDOMUtil +import com.intellij.testFramework.ProjectRule +import com.intellij.testFramework.RuleChain +import com.intellij.testFramework.TemporaryDirectory +import com.intellij.testFramework.runInEdtAndWait +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.hasChildren +import org.jdom.Element +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test + +private fun StateStorage.ExternalizationSession.save() { + runInEdtAndWait { + createSaveSession()!!.save() + } +} + +private fun StateStorageBase<*>.setStateAndSave(componentName: String, state: String?) { + var externalizationSession = startExternalization()!! + externalizationSession.setState(null, componentName, if (state == null) Element("state") else JDOMUtil.load(state.reader)) + externalizationSession.save() +} + +class DirectoryBasedStorageTest { + companion object { + @ClassRule val projectRule = ProjectRule() + } + + val tempDirManager = TemporaryDirectory() + + private val ruleChain = RuleChain(tempDirManager) + @Rule fun getChain() = ruleChain + + @Test fun save() { + val dir = tempDirManager.newPath() + val storage = DirectoryBasedStorage(dir.toFile(), object : MainConfigurationStateSplitter() { + override fun getComponentStateFileName() = "main" + + override fun getSubStateTagName() = "sub" + + override fun getSubStateFileName(element: Element) = element.getAttributeValue("name") + }) + + val componentName = "test" + + storage.setStateAndSave(componentName,"""""") + + assertThat(dir).hasChildren("foo.xml", "bar.xml", "main.xml") + + assertThat(dir.resolve("foo.xml")).hasContent(generateData("foo")) + assertThat(dir.resolve("bar.xml")).hasContent(generateData("bar")) + assertThat(dir.resolve("main.xml")).hasContent(generateData("test")) + + storage.setStateAndSave(componentName, """""") + + assertThat(dir).hasChildren("main.xml", "bar.xml") + assertThat(dir.resolve("bar.xml")).hasContent(generateData("bar")) + assertThat(dir.resolve("main.xml")).hasContent(generateData("test")) + + storage.setStateAndSave(componentName, null) + assertThat(dir).doesNotExist() + } + + private fun generateData(name: String): String { + return """ + <${if (name == "test") "component" else "sub"} name="$name" /> +""" + } +} \ No newline at end of file diff --git a/platform/configuration-store-impl/testSrc/ProjectStoreTest.kt b/platform/configuration-store-impl/testSrc/ProjectStoreTest.kt index fe194b98819b..e3f4e94c1efe 100644 --- a/platform/configuration-store-impl/testSrc/ProjectStoreTest.kt +++ b/platform/configuration-store-impl/testSrc/ProjectStoreTest.kt @@ -70,8 +70,7 @@ class ProjectStoreTest { val tempDirManager = TemporaryDirectory() private val ruleChain = RuleChain(tempDirManager) - - public Rule fun getChain(): RuleChain = ruleChain + @Rule fun getChain() = ruleChain Language("XML") private val iprFileContent = diff --git a/platform/configuration-store-impl/testSrc/StoreTestSuite.kt b/platform/configuration-store-impl/testSrc/StoreTestSuite.kt index edecabb20fe6..07759bbd03fb 100644 --- a/platform/configuration-store-impl/testSrc/StoreTestSuite.kt +++ b/platform/configuration-store-impl/testSrc/StoreTestSuite.kt @@ -26,6 +26,7 @@ Suite.SuiteClasses( ModuleStoreTest::class, ModuleStoreRenameTest::class, StorageManagerTest::class, SchemeManagerTest::class, - XmlElementStorageTest::class + XmlElementStorageTest::class, + DirectoryBasedStorageTest::class ) class StoreTestSuite \ No newline at end of file diff --git a/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt b/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt index d31a37ac7e95..f94298331401 100644 --- a/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt +++ b/platform/configuration-store-impl/testSrc/XmlElementStorageTest.kt @@ -40,14 +40,14 @@ class XmlElementStorageTest { val storage = MyXmlElementStorage(tag("root", tag("component", attr("name", "test"), tag("foo")))) val newState = tag("component", attr("name", "test"), tag("bar")) val externalizationSession = storage.startExternalization()!! - externalizationSession.setState(this, "test", newState) + externalizationSession.setState(null, "test", newState) externalizationSession.createSaveSession()!!.save() assertThat(storage.savedElement).isNotNull() assertThat(storage.savedElement!!.getChild("component").getChild("bar")).isNotNull() assertThat(storage.savedElement!!.getChild("component").getChild("foo")).isNull() } - private class MyXmlElementStorage(private val myElement: Element) : XmlElementStorage("", "root", null, null, null) { + private class MyXmlElementStorage(private val myElement: Element) : XmlElementStorage("", "root") { var savedElement: Element? = null override fun loadLocalData() = myElement diff --git a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StorageUtil.java b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StorageUtil.java index 9801270a2cfd..81ee31baf8fa 100644 --- a/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StorageUtil.java +++ b/platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StorageUtil.java @@ -48,8 +48,6 @@ import java.io.IOException; import java.util.*; public class StorageUtil { - public static final String DEFAULT_EXT = ".xml"; - private static final Logger LOG = Logger.getInstance(StorageUtil.class); @TestOnly diff --git a/platform/projectModel-api/src/com/intellij/openapi/components/StateSplitter.java b/platform/projectModel-api/src/com/intellij/openapi/components/StateSplitter.java index cd7c68463030..c28ca6790644 100644 --- a/platform/projectModel-api/src/com/intellij/openapi/components/StateSplitter.java +++ b/platform/projectModel-api/src/com/intellij/openapi/components/StateSplitter.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * 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. @@ -17,6 +17,7 @@ package com.intellij.openapi.components; import com.intellij.openapi.util.Pair; import org.jdom.Element; +import org.jetbrains.annotations.NotNull; import java.util.List; @@ -24,7 +25,7 @@ import java.util.List; * @deprecated Use {@link StateSplitterEx} */ public interface StateSplitter { - List> splitState(Element e); + List> splitState(@NotNull Element e); void mergeStatesInto(Element target, Element[] elements); } diff --git a/platform/projectModel-api/src/com/intellij/openapi/components/StateStorage.java b/platform/projectModel-api/src/com/intellij/openapi/components/StateStorage.java index 09b8c8d82cdf..cd9ed5fad9b1 100644 --- a/platform/projectModel-api/src/com/intellij/openapi/components/StateStorage.java +++ b/platform/projectModel-api/src/com/intellij/openapi/components/StateStorage.java @@ -41,7 +41,7 @@ public interface StateStorage { void analyzeExternalChangesAndUpdateIfNeed(@NotNull Set componentNames); interface ExternalizationSession { - void setState(@NotNull Object component, @NotNull String componentName, @NotNull Object state); + void setState(@Nullable Object component, @NotNull String componentName, @NotNull Object state); /** * return null if nothing to save diff --git a/platform/projectModel-impl/src/com/intellij/core/CoreProjectLoader.java b/platform/projectModel-impl/src/com/intellij/core/CoreProjectLoader.java index 2f92381bbcc6..2a33411b15fe 100644 --- a/platform/projectModel-impl/src/com/intellij/core/CoreProjectLoader.java +++ b/platform/projectModel-impl/src/com/intellij/core/CoreProjectLoader.java @@ -85,8 +85,8 @@ public class CoreProjectLoader { VirtualFile libraries = dotIdea.findChild("libraries"); if (libraries != null) { - Map> data = DirectoryStorageUtil.loadFrom(libraries, PathMacroManager.getInstance(project).createTrackingSubstitutor()); - Element libraryTable = DefaultStateSerializer.deserializeState(DirectoryStorageUtil.getCompositeState(data, "libraryTable", new ProjectLibraryTable.LibraryStateSplitter()), Element.class, null); + Map data = DirectoryStorageUtil.loadFrom(libraries, PathMacroManager.getInstance(project).createTrackingSubstitutor()); + Element libraryTable = DefaultStateSerializer.deserializeState(DirectoryStorageUtil.getCompositeState(data, new ProjectLibraryTable.LibraryStateSplitter()), Element.class, null); ((LibraryTableBase) ProjectLibraryTable.getInstance(project)).loadState(libraryTable); } diff --git a/platform/projectModel-impl/src/com/intellij/openapi/components/impl/stores/DirectoryStorageUtil.java b/platform/projectModel-impl/src/com/intellij/openapi/components/impl/stores/DirectoryStorageUtil.java index fc7c5c8983b6..f71a81d027b4 100644 --- a/platform/projectModel-impl/src/com/intellij/openapi/components/impl/stores/DirectoryStorageUtil.java +++ b/platform/projectModel-impl/src/com/intellij/openapi/components/impl/stores/DirectoryStorageUtil.java @@ -36,13 +36,13 @@ public class DirectoryStorageUtil { private static final Logger LOG = Logger.getInstance(DirectoryStorageUtil.class); @NotNull - public static Map> loadFrom(@Nullable VirtualFile dir, @Nullable TrackingPathMacroSubstitutor pathMacroSubstitutor) { + public static Map loadFrom(@Nullable VirtualFile dir, @Nullable TrackingPathMacroSubstitutor pathMacroSubstitutor) { if (dir == null || !dir.exists()) { return Collections.emptyMap(); } StringInterner interner = new StringInterner(); - Map> map = new THashMap>(); + Map fileToState = new THashMap(); for (VirtualFile file : dir.getChildren()) { // ignore system files like .DS_Store on Mac if (!StringUtilRt.endsWithIgnoreCase(file.getNameSequence(), FileStorageCoreUtil.DEFAULT_EXT)) { @@ -56,8 +56,8 @@ public class DirectoryStorageUtil { } Element element = JDOMUtil.load(file.getInputStream()); - String name = FileStorageCoreUtil.getComponentNameIfValid(element); - if (name == null) { + String componentName = FileStorageCoreUtil.getComponentNameIfValid(element); + if (componentName == null) { continue; } @@ -79,31 +79,22 @@ public class DirectoryStorageUtil { JDOMUtil.internElement(state, interner); if (pathMacroSubstitutor != null) { pathMacroSubstitutor.expandPaths(state); - pathMacroSubstitutor.addUnknownMacros(name, PathMacrosCollector.getMacroNames(state)); + pathMacroSubstitutor.addUnknownMacros(componentName, PathMacrosCollector.getMacroNames(state)); } - Map fileToState = map.get(name); - if (fileToState == null) { - fileToState = new THashMap(); - fileToState.put(file.getName(), state); - map.put(name, fileToState); - } - else { - fileToState.put(file.getName(), state); - } + fileToState.put(file.getName(), state); } catch (Throwable e) { LOG.warn("Unable to load state", e); } } - return map; + return fileToState; } @Nullable - public static Element getCompositeState(@NotNull Map> states, @NotNull String componentName, @NotNull StateSplitterEx splitter) { - Map fileToState = states.get(componentName); + public static Element getCompositeState(@NotNull Map fileToState, @NotNull StateSplitterEx splitter) { Element state = new Element(FileStorageCoreUtil.COMPONENT); - if (fileToState == null || fileToState.isEmpty()) { + if (fileToState.isEmpty()) { return state; } diff --git a/platform/testFramework/testFramework.iml b/platform/testFramework/testFramework.iml index da612ae434bc..16e04696afaa 100644 --- a/platform/testFramework/testFramework.iml +++ b/platform/testFramework/testFramework.iml @@ -25,5 +25,6 @@ + \ No newline at end of file diff --git a/platform/testFramework/testSrc/com/intellij/testFramework/TemporaryDirectory.kt b/platform/testFramework/testSrc/com/intellij/testFramework/TemporaryDirectory.kt index 475695c1257c..827584a034b4 100644 --- a/platform/testFramework/testSrc/com/intellij/testFramework/TemporaryDirectory.kt +++ b/platform/testFramework/testSrc/com/intellij/testFramework/TemporaryDirectory.kt @@ -154,4 +154,4 @@ public fun Path.refreshVfs() { } val VirtualFile.path: String - get() = getPath() \ No newline at end of file + get() = getPath() diff --git a/platform/testFramework/testSrc/com/intellij/testFramework/assertJ.kt b/platform/testFramework/testSrc/com/intellij/testFramework/assertJ.kt new file mode 100644 index 000000000000..356abe9c9538 --- /dev/null +++ b/platform/testFramework/testSrc/com/intellij/testFramework/assertJ.kt @@ -0,0 +1,42 @@ +/* + * 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 org.assertj.core.api + +import org.assertj.core.internal.ComparatorBasedComparisonStrategy +import org.assertj.core.internal.Iterables +import java.nio.file.Files +import java.nio.file.Path +import java.util.Comparator + +fun AbstractPathAssert<*>.hasChildren(vararg names: String) { + paths.assertIsDirectory(info, actual) + + Iterables(ComparatorBasedComparisonStrategy(object : Comparator { + override fun compare(o1: Any, o2: Any): Int { + if (o1 is Path && o2 is Path) { + return o1.compareTo(o2) + } + else if (o1 is String && o2 is String) { + return o1.compareTo(o2) + } + else { + return if ((o1 as Path).endsWith(o2 as String)) 0 else -1 + } + } + })) + .assertContainsOnly(info, Files.newDirectoryStream(actual).toList(), names) +} + diff --git a/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/ClasspathSaveSession.java b/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/ClasspathSaveSession.java index 529ea9cca672..223d79bae8a4 100644 --- a/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/ClasspathSaveSession.java +++ b/plugins/eclipse/src/org/jetbrains/idea/eclipse/config/ClasspathSaveSession.java @@ -65,7 +65,7 @@ final class ClasspathSaveSession implements StateStorage.ExternalizationSession, } @Override - public void setState(@NotNull Object component, @NotNull String componentName, @NotNull Object state) { + public void setState(Object component, @NotNull String componentName, @NotNull Object state) { try { CachedXmlDocumentSet fileSet = EclipseClasspathStorageProvider.getFileCache(module);