IJPL-136 getXmlDataFromController must not return resolved if not applicable

GitOrigin-RevId: 727e1da822b5dadb8fc1f35f4918d29ef2fb807d

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

@@ -168,8 +168,7 @@ private fun <T : Any> getXmlSerializationState(
   var result = mergeInto
   val bindings = rootBinding.bindings!!
 
-  for ((index, binding) in bindings
-    .withIndex()) {
+  for ((index, binding) in bindings.withIndex()) {
     val data = getXmlDataFromController(
       key = createSettingDescriptor(key = "$componentName.${binding.accessor.name}"),
       controller = controller,
@@ -181,14 +180,7 @@ private fun <T : Any> getXmlSerializationState(
           @Suppress("UNCHECKED_CAST")
           result = rootBinding.newInstance() as T
         }
-        deserializeBeanInto(
-          result = result,
-          element = oldData,
-          accessorNameTracker = null,
-          bindings = bindings,
-          start = index,
-          end = index + 1,
-        )
+        deserializeBeanInto(result = result, element = oldData, bindings = bindings, start = index, end = index + 1)
       }
       continue
     }
@@ -224,7 +216,7 @@ private fun getXmlDataFromController(key: SettingDescriptor<ByteArray>, controll
   catch (e: Throwable) {
     LOG.error("Cannot deserialize value for $key", e)
   }
-  return GetResult.resolved(null)
+  return GetResult.inapplicable()
 }
 
 private val shimPluginId = PluginId.getId("__controller_shim__")

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

@@ -15,6 +15,7 @@ import com.intellij.serviceContainer.ComponentManagerImpl
 import com.intellij.testFramework.*
 import com.intellij.testFramework.assertions.Assertions.assertThat
 import com.intellij.testFramework.rules.InMemoryFsRule
+import com.intellij.util.io.write
 import com.intellij.util.xmlb.XmlSerializerUtil
 import com.intellij.util.xmlb.annotations.Attribute
 import kotlinx.coroutines.Dispatchers
@@ -181,7 +182,6 @@ class ApplicationStoreTest {
 
   @Test
   fun `import deprecated settings`() {
-
     @State(name = "Comp", storages = [
       Storage("old.xml", roamingType = RoamingType.PER_OS, deprecated = true),
       Storage("new.xml", roamingType = RoamingType.PER_OS)])
@@ -199,9 +199,11 @@ class ApplicationStoreTest {
     val component = Comp()
     ApplicationManager.getApplication().registerServiceInstance(Comp::class.java, component)
     try {
-      val allItems = getExportableComponentsMap(isComputePresentableNames = false,
-                                                storageManager = storageManager,
-                                                withDeprecated = true)
+      val allItems = getExportableComponentsMap(
+        isComputePresentableNames = false,
+        storageManager = storageManager,
+        withDeprecated = true,
+      )
       assertThat(allItems).containsKeys(
         fileSpec("old.xml"),
         fileSpec("$os/old.xml"),
@@ -550,9 +552,6 @@ class ApplicationStoreTest {
     assertThat(component.state.bar).isEqualTo("42")
   }
 
-  private data class TestStateWithMap(@JvmField @Attribute var foo: String = "",
-                                      @JvmField val map: Map<String, Set<String>> = HashMap())
-
   private fun createComponentFileContent(fooValue: String, componentName: String = "A"): String {
     return """<application>${createComponentData(fooValue, componentName)}</application>"""
   }
@@ -560,18 +559,26 @@ class ApplicationStoreTest {
   @State(name = "A", storages = [Storage(value = "per-os.xml", roamingType = RoamingType.PER_OS)])
   private class PerOsComponent : FooComponent()
 
-  private fun writeConfig(fileName: String, @Language("XML") data: String): Path =
-    testAppConfig.resolve(fileName).createParentDirectories().apply { writeText(data) }
+  private fun writeConfig(fileName: String, @Language("XML") data: String): Path {
+    val file = testAppConfig.resolve(fileName)
+    file.write(data)
+    return file
+  }
 
   private class MyStreamProvider : StreamProvider {
     override val isExclusive = true
 
-    override fun processChildren(path: String, roamingType: RoamingType, filter: (String) -> Boolean, processor: (String, InputStream, Boolean) -> Boolean) = true
+    override fun processChildren(
+      path: String,
+      roamingType: RoamingType,
+      filter: (String) -> Boolean,
+      processor: (String, InputStream, Boolean) -> Boolean,
+    ) = true
 
     val data: MutableMap<RoamingType, MutableMap<String, String>> = EnumMap(RoamingType::class.java)
 
     override fun write(fileSpec: String, content: ByteArray, roamingType: RoamingType) {
-      getMap(roamingType).put(fileSpec, String(content, Charsets.UTF_8))
+      getMap(roamingType).put(fileSpec, content.decodeToString())
     }
 
     private fun getMap(roamingType: RoamingType): MutableMap<String, String> = data.computeIfAbsent(roamingType) { HashMap() }
@@ -583,7 +590,7 @@ class ApplicationStoreTest {
     }
 
     override fun delete(fileSpec: String, roamingType: RoamingType): Boolean {
-      data[roamingType]?.remove(fileSpec)
+      data.get(roamingType)?.remove(fileSpec)
       return true
     }
   }

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

@@ -16,8 +16,10 @@ import com.intellij.testFramework.ApplicationRule
 import com.intellij.testFramework.DisposableRule
 import com.intellij.testFramework.assertions.Assertions.assertThat
 import com.intellij.testFramework.rules.InMemoryFsRule
+import com.intellij.util.io.write
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.runBlocking
+import org.intellij.lang.annotations.Language
 import org.junit.Before
 import org.junit.ClassRule
 import org.junit.Rule
@@ -48,6 +50,10 @@ class ControllerBackedStoreTest {
   @Before
   fun setUp() {
     testAppConfig = fsRule.fs.getPath("/app-config")
+  }
+
+  @Test
+  fun `cache storage`() = runBlocking<Unit>(Dispatchers.Default) {
     componentStore = ControllerBackedTestComponentStore(
       testAppConfigPath = testAppConfig,
       controller = SettingsControllerMediator(
@@ -65,14 +71,8 @@ class ControllerBackedStoreTest {
         isPersistenceStateComponentProxy = true,
       ),
     )
-  }
 
-  @Test
-  fun `settingsController - cache storage`() = runBlocking<Unit>(Dispatchers.Default) {
-    @State(name = "TestState", storages = [Storage(value = StoragePathMacros.NON_ROAMABLE_FILE)])
-    class Component : SerializablePersistentStateComponent<TestState>(TestState())
-
-    val component = Component()
+    val component = TestComponent()
     componentStore.initComponent(component = component, serviceDescriptor = null, pluginId = null)
 
     assertThat(component.state.foo).isEmpty()
@@ -90,6 +90,40 @@ class ControllerBackedStoreTest {
     componentStore.initComponent(component = component, serviceDescriptor = null, pluginId = null)
     assertThat(component.state.bar).isEqualTo("12")
   }
+
+  @Test
+  fun `not applicable`() = runBlocking<Unit>(Dispatchers.Default) {
+    componentStore = ControllerBackedTestComponentStore(
+      testAppConfigPath = testAppConfig,
+      controller = SettingsControllerMediator(isPersistenceStateComponentProxy = true),
+    )
+
+    val oldContent = """
+      <application>
+        <component name="TestState" foo="old"/>
+      </application>
+      """.trimMargin()
+    writeConfig(StoragePathMacros.NON_ROAMABLE_FILE, oldContent)
+
+    val component = TestComponent()
+    componentStore.initComponent(component = component, serviceDescriptor = null, pluginId = null)
+
+    assertThat(component.state.foo).isEqualTo("old")
+    assertThat(component.state.bar).isEmpty()
+
+    component.state = TestState(bar = "42")
+    componentStore.save(forceSavingAllSettings = true)
+
+    componentStore.initComponent(component = component, serviceDescriptor = null, pluginId = null)
+    assertThat(component.state.bar).isEqualTo("42")
+  }
+
+  @Suppress("SameParameterValue")
+  private fun writeConfig(fileName: String, @Language("XML") data: String): Path {
+    val file = testAppConfig.resolve(fileName)
+    file.write(data)
+    return file
+  }
 }
 
 private class ControllerBackedTestComponentStore(
@@ -110,3 +144,6 @@ private class ControllerBackedTestComponentStore(
     storageManager.setMacros(listOf(Macro(APP_CONFIG, path), Macro(ROOT_CONFIG, path), Macro(StoragePathMacros.CACHE_FILE, path)))
   }
 }
+
+@State(name = "TestState", storages = [Storage(value = StoragePathMacros.NON_ROAMABLE_FILE)])
+private class TestComponent : SerializablePersistentStateComponent<TestState>(TestState())

platform/settings-local/src/com/intellij/platform/settings/local/SettingsControllerMediator.kt

@@ -9,6 +9,7 @@ import com.intellij.openapi.extensions.ExtensionPointName
 import com.intellij.openapi.util.IntellijInternalApi
 import com.intellij.platform.settings.*
 import org.jetbrains.annotations.ApiStatus.Internal
+import org.jetbrains.annotations.TestOnly
 import org.jetbrains.annotations.VisibleForTesting
 import java.nio.file.Path
 
@@ -23,6 +24,10 @@ class SettingsControllerMediator(
   private val controllers: List<DelegatedSettingsController> = SETTINGS_CONTROLLER_EP_NAME.extensionList,
   private val isPersistenceStateComponentProxy: Boolean = controllers.size > 1,
 ) : SettingsController {
+  @TestOnly
+  constructor(isPersistenceStateComponentProxy: Boolean)
+    : this(controllers = SETTINGS_CONTROLLER_EP_NAME.extensionList, isPersistenceStateComponentProxy = isPersistenceStateComponentProxy)
+
   override fun <T : Any> getItem(key: SettingDescriptor<T>): T? {
     return doGetItem(key).get()
   }

platform/testFramework/extensions/src/com/intellij/testFramework/rules/InMemoryFsRule.kt

@@ -1,4 +1,4 @@
-// Copyright 2000-2021 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.testFramework.rules
 
 import com.github.marschall.memoryfilesystem.MemoryFileSystemBuilder
@@ -25,8 +25,12 @@ class InMemoryFsRule(private val windows: Boolean = false) : ExternalResource()
   val fs: FileSystem
     get() {
       if (_fs == null) {
-        _fs = (if (windows) MemoryFileSystemBuilder.newWindows().setCurrentWorkingDirectory("C:\\")
-               else MemoryFileSystemBuilder.newLinux().setCurrentWorkingDirectory("/")).build(sanitizedName)
+        _fs = (if (windows) {
+          MemoryFileSystemBuilder.newWindows().setCurrentWorkingDirectory("C:\\")
+        }
+        else {
+          MemoryFileSystemBuilder.newLinux().setCurrentWorkingDirectory("/")
+        }).build(sanitizedName)
       }
       return _fs!!
     }
@@ -49,8 +53,12 @@ class InMemoryFsExtension(private val windows: Boolean = false) : BeforeEachCall
   val fs: FileSystem
     get() {
       if (_fs == null) {
-        _fs = (if (windows) MemoryFileSystemBuilder.newWindows().setCurrentWorkingDirectory("C:\\")
-               else MemoryFileSystemBuilder.newLinux().setCurrentWorkingDirectory("/")).build(sanitizedName)
+        _fs = (if (windows) {
+          MemoryFileSystemBuilder.newWindows().setCurrentWorkingDirectory("C:\\")
+        }
+        else {
+          MemoryFileSystemBuilder.newLinux().setCurrentWorkingDirectory("/")
+        }).build(sanitizedName)
       }
       return _fs!!
     }