From a628cbd5e886d25acaeb7eadf32a6b71fb6cfea3 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Thu, 7 Mar 2024 12:41:10 +0100 Subject: [PATCH] IJPL-736 ability to provide local value to save on set (second and final part) GitOrigin-RevId: a5ee7654ad41a6cd16b3aa5702288894a433b190 --- .../src/StateStorageBase.kt | 17 +++++-- .../testSrc/ControllerBackedStoreTest.kt | 49 +++++++++++++++++++ .../local/SettingsControllerMediator.kt | 2 +- platform/settings/.editorconfig | 3 ++ platform/settings/docs/Dockerfile | 6 +-- platform/settings/docs/build.sh | 28 +++++++++-- platform/settings/docs/nginx.conf | 10 +--- .../settings/docs/settings-docs-draft.tree | 1 + .../settings/docs/topics/bridge-to-old-api.md | 13 +++-- .../settings/docs/topics/storage-design.md | 18 +++++++ .../platform/settings/SettingsController.kt | 7 ++- 11 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 platform/settings/.editorconfig create mode 100644 platform/settings/docs/topics/storage-design.md diff --git a/platform/configuration-store-impl/src/StateStorageBase.kt b/platform/configuration-store-impl/src/StateStorageBase.kt index 6be46d815e29..8656bfc8aa02 100644 --- a/platform/configuration-store-impl/src/StateStorageBase.kt +++ b/platform/configuration-store-impl/src/StateStorageBase.kt @@ -244,19 +244,28 @@ private fun serializeWithController( val isPropertySkipped = isPropertySkipped(filter = filter, binding = binding, bean = state, rootBinding = rootBinding, isFilterPropertyItself = true) val key = SettingDescriptor(key = createSettingKey(componentName, binding), pluginId = pluginId, tags = keyTags, serializer = JsonElementSettingSerializerDescriptor) val result = controller.doSetItem(key = key, value = if (isPropertySkipped) null else binding.toJson(state, filter)) - if (result != SetResult.inapplicable()) { + if (isPropertySkipped) { continue } - if (isPropertySkipped) { - continue + var effectiveState = state + if (result != SetResult.inapplicable()) { + val value = result.value + if (value is JsonElement) { + // substituted value + effectiveState = rootBinding.newInstance() + binding.setFromJson(effectiveState, value) + } + else { + continue + } } if (element == null) { element = Element(rootBinding.tagName) } - binding.serialize(bean = state, parent = element, filter = filter) + binding.serialize(bean = effectiveState, parent = element, filter = filter) } return element } diff --git a/platform/configuration-store-impl/testSrc/ControllerBackedStoreTest.kt b/platform/configuration-store-impl/testSrc/ControllerBackedStoreTest.kt index aae1d3d18ade..bddb33f84e0a 100644 --- a/platform/configuration-store-impl/testSrc/ControllerBackedStoreTest.kt +++ b/platform/configuration-store-impl/testSrc/ControllerBackedStoreTest.kt @@ -367,6 +367,55 @@ class ControllerBackedStoreTest { assertThat(Json.encodeToString(saved)).isEqualTo("""{"foo":"newValue","bar":"test2"}""") } + + @Suppress("unused") + @Test + fun substitute_value_on_set() = runBlocking(Dispatchers.Default) { + data class TestState(var foo: String = "", var bar: String = "") + + val store = createStore(object : DelegatedSettingsController { + override fun getItem(key: SettingDescriptor): GetResult = GetResult.inapplicable() + + override fun setItem(key: SettingDescriptor, value: T?): SetResult { + if (key.key == "TestState.foo") { + return SetResult.substituted(JsonPrimitive("overridden")) + } + else { + return SetResult.inapplicable() + } + } + }) + + @State(name = "TestState", storages = [Storage(value = TEST_COMPONENT_FILE_NAME)], allowLoadInTests = true) + class TestComponent : SerializablePersistentStateComponent(TestState()) { + override fun noStateLoaded() { + loadState(TestState()) + } + } + + val component = TestComponent() + store.initComponent(component = component, serviceDescriptor = null, pluginId = PluginManagerCore.CORE_ID) + + assertThat(component.state.foo).isEmpty() + + component.state.foo = "newValue" + store.save(forceSavingAllSettings = true) + assertThat(component.state.foo).isEqualTo("newValue") + + // but the new value on disk + assertThat(JDOMUtil.load(appConfig.resolve(TEST_COMPONENT_FILE_NAME))).isEqualTo(""" + + + + + """.trimIndent()) + + store.reloadStates(setOf("TestState")) + // and after reload, the new value is applied to the component + assertThat(component.state.foo).isEqualTo("overridden") + } + @Suppress("unused") @Test fun cache_storage() = runBlocking(Dispatchers.Default) { diff --git a/platform/settings-local/src/com/intellij/platform/settings/local/SettingsControllerMediator.kt b/platform/settings-local/src/com/intellij/platform/settings/local/SettingsControllerMediator.kt index 7dcf943510a8..13c07b63c889 100644 --- a/platform/settings-local/src/com/intellij/platform/settings/local/SettingsControllerMediator.kt +++ b/platform/settings-local/src/com/intellij/platform/settings/local/SettingsControllerMediator.kt @@ -51,7 +51,7 @@ class SettingsControllerMediator( var totalResult = SetResult.inapplicable() for (controller in controllers) { val result = controller.setItem(key = key, value = value) - if (result == SetResult.forbid()) { + if (result == SetResult.forbid() || result.value !is Enum<*>) { return result } else if (result == SetResult.done()) { diff --git a/platform/settings/.editorconfig b/platform/settings/.editorconfig new file mode 100644 index 000000000000..f9dece3e918d --- /dev/null +++ b/platform/settings/.editorconfig @@ -0,0 +1,3 @@ +[*] +# critical subsystem - named params used a lot +max_line_length = 180 \ No newline at end of file diff --git a/platform/settings/docs/Dockerfile b/platform/settings/docs/Dockerfile index 13bdaae8bc03..d88f1b0f1cbf 100644 --- a/platform/settings/docs/Dockerfile +++ b/platform/settings/docs/Dockerfile @@ -1,5 +1,3 @@ -FROM nginxinc/nginx-unprivileged -COPY --chown=101:101 site /usr/share/nginx/html +FROM nginxinc/nginx-unprivileged:1.25.4 COPY --chown=101:101 nginx.conf /etc/nginx/conf.d/default.conf - -# overview.html as index file for nginx \ No newline at end of file +COPY --chown=101:101 site /usr/share/nginx/html \ No newline at end of file diff --git a/platform/settings/docs/build.sh b/platform/settings/docs/build.sh index 5fb9b03f7f20..4e3c193f78d8 100755 --- a/platform/settings/docs/build.sh +++ b/platform/settings/docs/build.sh @@ -1,14 +1,14 @@ #!/usr/bin/env bash -# Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +# Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. set -e -u -x BASEDIR=$(dirname "$0") cd "$BASEDIR/../../../../out" || exit -rm -rf settings-doc-draf/ -mkdir -p settings-doc-draf/site +rm -rf settings-doc-draft/ +mkdir -p settings-doc-draft/site -cd settings-doc-draf +cd settings-doc-draft unzip ../webHelpSETTINGS-DOCS-DRAFT2-all.zip -d site cp "$BASEDIR"/Dockerfile ./ @@ -17,4 +17,22 @@ cp "$BASEDIR"/nginx.conf ./ docker build --platform=linux/amd64 . -t registry.jetbrains.team/p/ij/structurizr/settings-docs:latest docker push registry.jetbrains.team/p/ij/structurizr/settings-docs:latest -echo "$PWD" \ No newline at end of file +echo "$PWD" + +# docker doesn't have D2 +#OUT_DIR="$BASEDIR/../../../../out/settings-doc-draft" +#rm -rf "$OUT_DIR" +#mkdir -p "$OUT_DIR" +# +## see latest version in https://jetbrains.team/p/writerside/packages/container/builder/writerside-builder +#docker run --rm -v .:/opt/sources -v "$OUT_DIR":/opt/out registry.jetbrains.team/p/writerside/builder/writerside-builder:2.1.1755-p5358 /bin/bash -c " +#export DISPLAY=:99 && +#Xvfb :99 & +#/opt/builder/bin/idea.sh helpbuilderinspect \ +#--source-dir /opt/sources \ +#--product Writerside/settings-docs-draft \ +#--runner other \ +#--output-dir /opt/out +#" +# +#cd "$OUT_DIR" \ No newline at end of file diff --git a/platform/settings/docs/nginx.conf b/platform/settings/docs/nginx.conf index a805b37d27b7..8d99e4e64a61 100644 --- a/platform/settings/docs/nginx.conf +++ b/platform/settings/docs/nginx.conf @@ -3,8 +3,8 @@ server { absolute_redirect off; - rewrite ^(/.*)\.html(\?.*)?$ $1$2 permanent; - rewrite ^/(.*)/$ /$1 permanent; + rewrite ^(/.*)\.html(\?.*)?$ $1$2 redirect; + rewrite ^/(.*)/$ /$1 redirect; root /usr/share/nginx/html; @@ -15,10 +15,4 @@ server { location = / { return 302 overview; } - - # redirect server error pages to the static page /50x.html - error_page 500 502 503 504 /50x.html; - location = /50x.html { - root /usr/share/nginx/html; - } } diff --git a/platform/settings/docs/settings-docs-draft.tree b/platform/settings/docs/settings-docs-draft.tree index 394ec6ecb197..527f9c452320 100644 --- a/platform/settings/docs/settings-docs-draft.tree +++ b/platform/settings/docs/settings-docs-draft.tree @@ -10,4 +10,5 @@ + \ No newline at end of file diff --git a/platform/settings/docs/topics/bridge-to-old-api.md b/platform/settings/docs/topics/bridge-to-old-api.md index f5d47db96e9f..1f3247a1d1ff 100644 --- a/platform/settings/docs/topics/bridge-to-old-api.md +++ b/platform/settings/docs/topics/bridge-to-old-api.md @@ -1,17 +1,20 @@ -# Implicit support for PersistenceStateComponent +# Seamless Support for PersistenceStateComponent The new Settings Controller API is currently not intended for use by end clients. -All existing implementations of `PersistenceStateComponent` that don't use the deprecated API like `JDOMExternalizable` are fully supported, and no changes are required. +All existing implementations of `PersistenceStateComponent` that don't use the deprecated API `JDOMExternalizable` are fully supported, and no changes are required. Support here means that each component property serves as a key, not the whole component. ## Unified Format +The Settings Controller operates with values in a unified format, where each value is a [JsonElement](https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-json/kotlinx.serialization.json/-json-element/). +Please note that the corresponding API is advanced and experimental, and the `JsonElement` API may be changed in the future. + For primitive values (no binding or binding plus converter, meaning effectively is a string): `"a string"`, `number`, `boolean`. In other words, a string is quoted with double quotes and other primitive values as is (JSON syntax). For complex values (binding exists), like * map (`MapBinding`), -* collection (`AbstractCollectionBinding` — `ArrayBinding` or `CollectionBinding`), +* collection (`CollectionBinding`), * bean (`BeanBinding` and `KotlinAwareBeanBinding`, about `KotlinxSerializationBinding` see note about custom bindings), also a JSON syntax is used. @@ -40,7 +43,7 @@ If a map has a complex key, then it is an array of objects, with each object rep ``` Maps with non-primitive keys are expected to be rare. -Currently, `SettingDescriptor` lacks tags for distinguishing formats or providing format details. +Currently, `SettingDescriptor` lacks tags which could be used to distinguish formats or provide format details. These could be added if necessary. ### Collections @@ -107,4 +110,4 @@ In a nutshell: * `BeanBinding.deserializeInto` to deserialize. Please note that this API is internal and low-level. It requires a profound understanding of the IntelliJ Platform. It may not be as straightforward as it sounds. -Therefore, its use outside of the IJ Platform is strongly discouraged. \ No newline at end of file +Therefore, its use outside the IJ Platform is strongly discouraged. \ No newline at end of file diff --git a/platform/settings/docs/topics/storage-design.md b/platform/settings/docs/topics/storage-design.md new file mode 100644 index 000000000000..2b619ef65771 --- /dev/null +++ b/platform/settings/docs/topics/storage-design.md @@ -0,0 +1,18 @@ +# Storage Design + +Please note these are technical notes, not end user documentation. + +## StateStorageBackedByController + +`StateStorageBackedByController` is a special implementation of `StateStorage`. +It's another PSC bridge implementation and currently, it's only used for cache (`StoragePathMacros.CACHE_FILE`). + +You might wonder why it's not used for [internal](setting-types.md) settings. Well, this is because it lacks numerous features like support for `StreamProvider` (settings sync, settings import). While we could potentially overcome this, the return on investment isn't evident for now. + +So, how does it work? It utilizes the same [unified format](bridge-to-old-api.md#unified-format) and also employs `JsonElementSettingSerializerDescriptor` as the setting key serializator. +Although the implementation shares some similarities with the PSC bridge, it's not completely identical. As `StateStorageBackedByController` is an exclusive storage, we don't need to handle local data (e.g., merging). + +Currently, `LocalSettingsController` only supports keys with `CacheTag` and `NonShareableInternalTag` tags. +It uses [MVStore](https://www.h2database.com/html/mvstore.html) to store data on the disk. Specifically, we use `MVMap` — where the setting key is transformed into a string (plugin id + setting key). Any type of value is serialized into CBOR. This allows us to deserialize the value later without knowing the data type - whether it's a string, number, etc. As of now, this feature is utilized by `DumpDevIdeaCacheDb` to dump the MVStore database into YAML. + +For `JsonElementSettingSerializerDescriptor`, where the serializer cannot be used for any format beyond JSON, we encode the JSON element into a string, and subsequently encode it to CBOR. So, the value gets stored as a CBOR string. \ No newline at end of file diff --git a/platform/settings/src/com/intellij/platform/settings/SettingsController.kt b/platform/settings/src/com/intellij/platform/settings/SettingsController.kt index 21655a42a289..7278a01ed387 100644 --- a/platform/settings/src/com/intellij/platform/settings/SettingsController.kt +++ b/platform/settings/src/com/intellij/platform/settings/SettingsController.kt @@ -3,6 +3,8 @@ package com.intellij.platform.settings import com.intellij.openapi.components.ComponentManager import com.intellij.util.concurrency.annotations.RequiresBackgroundThread +import com.intellij.util.xmlb.SettingsInternalApi +import kotlinx.serialization.json.JsonElement import org.jetbrains.annotations.ApiStatus.* import java.nio.file.Path import java.util.* @@ -61,15 +63,18 @@ value class GetResult @PublishedApi internal constructor(@Publishe @Internal @JvmInline -value class SetResult @PublishedApi internal constructor(@PublishedApi internal val value: Any?) { +value class SetResult @PublishedApi internal constructor(@Internal @SettingsInternalApi val value: Any?) { companion object { fun inapplicable(): SetResult = SetResult(SetResultResolution.INAPPLICABLE) fun forbid(): SetResult = SetResult(SetResultResolution.FORBID) fun done(): SetResult = SetResult(SetResultResolution.DONE) + + fun substituted(value: JsonElement): SetResult = SetResult(value) } + @OptIn(SettingsInternalApi::class) override fun toString(): String = "SetResult($value)" }