This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch refactor/facet-persistent-state-component in repository https://gitbox.apache.org/repos/asf/struts-intellij-plugin.git
commit 67dd04204d3e13e4b97e9ce2065149630f976cac Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Apr 5 18:53:30 2026 +0200 refactor(facet): migrate FacetConfiguration persistence to PersistentStateComponent Replace deprecated readExternal()/writeExternal() overrides in StrutsFacetConfiguration with PersistentStateComponent<Element>, following the migration path documented in the IntelliJ Platform SDK. Uses PersistentStateComponent<Element> to preserve the exact existing XML structure (fileset/file/propertiesKeys tags) for full backward compatibility with saved .iml facet configuration. Also removes redundant ModificationTracker interface (already inherited from SimpleModificationTracker superclass). Made-with: Cursor --- CHANGELOG.md | 3 +- docs/framework-initialization.md | 20 +- .../struts2/facet/StrutsFacetConfiguration.java | 68 +++---- .../facet/StrutsFacetConfigurationTest.java | 201 +++++++++++++++++++++ 4 files changed, 250 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73c9120..0cff072 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,8 @@ - Replace deprecated `URL(String)` constructor with `URI.create().toURL()` - Resolve deprecated API warnings from Marketplace verification: - Migrate `DaemonCodeAnalyzer.restart()` to `restart(PsiFile)` overload - - Suppress unavoidable deprecations with no public replacements (`GraphBuilder`, `FacetConfiguration`, `CheckboxTreeBase`) + - Suppress unavoidable deprecations with no public replacements (`GraphBuilder`, `CheckboxTreeBase`) + - Migrate `FacetConfiguration.readExternal()`/`writeExternal()` to `PersistentStateComponent<Element>` - Strip leading slash from DTD resource path for IntelliJ 2025.3 `PluginClassLoader.findResource()` compatibility - Resolve 21 critical nullability/NPE warnings from Qodana analysis across 15 files - Resolve 22 Qodana warnings: redundant code, incorrect string capitalization, unnecessary null checks diff --git a/docs/framework-initialization.md b/docs/framework-initialization.md index ce94402..b490c51 100644 --- a/docs/framework-initialization.md +++ b/docs/framework-initialization.md @@ -70,11 +70,12 @@ public class StrutsFacetType extends FacetType<StrutsFacet, StrutsFacetConfigura ### 2. FacetConfiguration -Manages framework-specific settings and persistence. +Manages framework-specific settings and persistence. Implements `PersistentStateComponent<Element>` for +XML serialization, which is the modern replacement for the deprecated `readExternal`/`writeExternal` methods. ```java public class StrutsFacetConfiguration extends SimpleModificationTracker - implements FacetConfiguration, ModificationTracker, Disposable { + implements FacetConfiguration, PersistentStateComponent<Element>, Disposable { @Override public FacetEditorTab[] createEditorTabs(final FacetEditorContext editorContext, @@ -86,21 +87,21 @@ public class StrutsFacetConfiguration extends SimpleModificationTracker } @Override - public void readExternal(final Element element) throws InvalidDataException { - // XML persistence logic + public @Nullable Element getState() { + // Build and return JDOM Element representing current configuration } @Override - public void writeExternal(final Element element) throws WriteExternalException { - // XML persistence logic + public void loadState(@NotNull Element state) { + // Restore configuration from JDOM Element } } ``` **Responsibilities:** -- Settings persistence (XML serialization) +- Settings persistence via `PersistentStateComponent` (`getState`/`loadState`) - Editor tabs for configuration UI -- Validation and modification tracking +- Modification tracking - Resource disposal ### 3. Facet @@ -319,7 +320,8 @@ if (requiredFacet == null) { ### 4. Resource Management ```java -public class MyFacetConfiguration implements FacetConfiguration, Disposable { +public class MyFacetConfiguration extends SimpleModificationTracker + implements FacetConfiguration, PersistentStateComponent<Element>, Disposable { @Override public void dispose() { diff --git a/src/main/java/com/intellij/struts2/facet/StrutsFacetConfiguration.java b/src/main/java/com/intellij/struts2/facet/StrutsFacetConfiguration.java index 04f54f1..6696aaf 100644 --- a/src/main/java/com/intellij/struts2/facet/StrutsFacetConfiguration.java +++ b/src/main/java/com/intellij/struts2/facet/StrutsFacetConfiguration.java @@ -23,16 +23,16 @@ import com.intellij.facet.ui.FacetValidatorsManager; import com.intellij.facet.ui.libraries.FacetLibrariesValidator; import com.intellij.facet.ui.libraries.LibraryInfo; import com.intellij.openapi.Disposable; -import com.intellij.openapi.util.InvalidDataException; -import com.intellij.openapi.util.ModificationTracker; +import com.intellij.openapi.components.PersistentStateComponent; import com.intellij.openapi.util.SimpleModificationTracker; -import com.intellij.openapi.util.WriteExternalException; import com.intellij.openapi.vfs.pointers.VirtualFilePointer; import com.intellij.struts2.facet.ui.FeaturesConfigurationTab; import com.intellij.struts2.facet.ui.FileSetConfigurationTab; import com.intellij.struts2.facet.ui.StrutsFileSet; import org.jdom.Element; import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.LinkedHashSet; import java.util.List; @@ -43,7 +43,8 @@ import java.util.Set; * * @author Yann Cébron */ -public class StrutsFacetConfiguration extends SimpleModificationTracker implements FacetConfiguration, ModificationTracker, Disposable { +public class StrutsFacetConfiguration extends SimpleModificationTracker + implements FacetConfiguration, PersistentStateComponent<Element>, Disposable { // Filesets @NonNls @@ -102,9 +103,36 @@ public class StrutsFacetConfiguration extends SimpleModificationTracker implemen } @Override - @SuppressWarnings("deprecation") // TODO: FacetConfiguration.readExternal() deprecated with no public replacement. - public void readExternal(final Element element) throws InvalidDataException { - for (final Element setElement : element.getChildren(FILESET)) { + public @Nullable Element getState() { + final Element element = new Element("state"); + + for (final StrutsFileSet fileSet : myFileSets) { + final Element setElement = new Element(FILESET); + setElement.setAttribute(SET_ID, fileSet.getId()); + setElement.setAttribute(SET_NAME, fileSet.getName()); + setElement.setAttribute(SET_REMOVED, Boolean.toString(fileSet.isRemoved())); + element.addContent(setElement); + + for (final VirtualFilePointer fileName : fileSet.getFiles()) { + final Element fileElement = new Element(FILE); + fileElement.setText(fileName.getUrl()); + setElement.addContent(fileElement); + } + } + + final Element propertiesElement = new Element(PROPERTIES_KEYS); + propertiesElement.setAttribute(PROPERTIES_KEYS_DISABLED, Boolean.toString(myPropertiesKeysDisabled)); + element.addContent(propertiesElement); + + return element; + } + + @Override + public void loadState(@NotNull Element state) { + myFileSets.clear(); + myPropertiesKeysDisabled = false; + + for (final Element setElement : state.getChildren(FILESET)) { final String setName = setElement.getAttributeValue(SET_NAME); final String setId = setElement.getAttributeValue(SET_ID); final String removed = setElement.getAttributeValue(SET_REMOVED); @@ -120,35 +148,11 @@ public class StrutsFacetConfiguration extends SimpleModificationTracker implemen } } - // new in X - final Element propertiesElement = element.getChild(PROPERTIES_KEYS); + final Element propertiesElement = state.getChild(PROPERTIES_KEYS); if (propertiesElement != null) { final String disabled = propertiesElement.getAttributeValue(PROPERTIES_KEYS_DISABLED); myPropertiesKeysDisabled = Boolean.parseBoolean(disabled); } - - } - - @Override - @SuppressWarnings("deprecation") // TODO: FacetConfiguration.writeExternal() deprecated with no public replacement. - public void writeExternal(final Element element) throws WriteExternalException { - for (final StrutsFileSet fileSet : myFileSets) { - final Element setElement = new Element(FILESET); - setElement.setAttribute(SET_ID, fileSet.getId()); - setElement.setAttribute(SET_NAME, fileSet.getName()); - setElement.setAttribute(SET_REMOVED, Boolean.toString(fileSet.isRemoved())); - element.addContent(setElement); - - for (final VirtualFilePointer fileName : fileSet.getFiles()) { - final Element fileElement = new Element(FILE); - fileElement.setText(fileName.getUrl()); - setElement.addContent(fileElement); - } - } - - final Element propertiesElement = new Element(PROPERTIES_KEYS); - propertiesElement.setAttribute(PROPERTIES_KEYS_DISABLED, Boolean.toString(myPropertiesKeysDisabled)); - element.addContent(propertiesElement); } public void setModified() { diff --git a/src/test/java/com/intellij/struts2/facet/StrutsFacetConfigurationTest.java b/src/test/java/com/intellij/struts2/facet/StrutsFacetConfigurationTest.java new file mode 100644 index 0000000..a6f2916 --- /dev/null +++ b/src/test/java/com/intellij/struts2/facet/StrutsFacetConfigurationTest.java @@ -0,0 +1,201 @@ +/* + * Copyright 2025 The authors + * 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.struts2.facet; + +import com.intellij.openapi.Disposable; +import com.intellij.openapi.util.Disposer; +import com.intellij.struts2.BasicLightHighlightingTestCase; +import com.intellij.struts2.facet.ui.StrutsFileSet; +import org.jdom.Element; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +/** + * Tests for {@link StrutsFacetConfiguration} persistence via {@code PersistentStateComponent<Element>}. + */ +public class StrutsFacetConfigurationTest extends BasicLightHighlightingTestCase { + + private final List<Disposable> myDisposables = new ArrayList<>(); + + @Override + @NotNull + protected String getTestDataLocation() { + return ""; + } + + @Override + protected void performTearDown() { + for (Disposable d : myDisposables) { + Disposer.dispose(d); + } + myDisposables.clear(); + } + + private StrutsFacetConfiguration createDisposableConfig() { + StrutsFacetConfiguration config = new StrutsFacetConfiguration(); + myDisposables.add(config); + return config; + } + + private StrutsFileSet createTrackedFileSet(String id, String name, StrutsFacetConfiguration parent) { + StrutsFileSet fileSet = new StrutsFileSet(id, name, parent); + myDisposables.add(fileSet); + return fileSet; + } + + public void testRoundTripPreservesFileSets() { + final StrutsFacet facet = StrutsFacet.getInstance(getModule()); + assertNotNull(facet); + final StrutsFacetConfiguration config = facet.getConfiguration(); + + final StrutsFileSet fileSet = createTrackedFileSet("s2fileset1", "My Struts Config", config); + fileSet.addFile("file:///project/src/main/resources/struts.xml"); + fileSet.addFile("file:///project/src/main/resources/struts-admin.xml"); + config.getFileSets().add(fileSet); + + final StrutsFileSet removedSet = createTrackedFileSet("s2fileset2", "Removed Set", config); + removedSet.setRemoved(true); + removedSet.addFile("file:///project/src/main/resources/old-struts.xml"); + config.getFileSets().add(removedSet); + + final Element state = config.getState(); + assertNotNull(state); + + final StrutsFacetConfiguration loaded = createDisposableConfig(); + loaded.loadState(state); + + final Set<StrutsFileSet> loadedSets = loaded.getFileSets(); + assertEquals(2, loadedSets.size()); + + final List<StrutsFileSet> setList = new ArrayList<>(loadedSets); + + assertEquals("s2fileset1", setList.get(0).getId()); + assertEquals("My Struts Config", setList.get(0).getName()); + assertFalse(setList.get(0).isRemoved()); + assertEquals(2, setList.get(0).getFiles().size()); + assertEquals("file:///project/src/main/resources/struts.xml", setList.get(0).getFiles().get(0).getUrl()); + assertEquals("file:///project/src/main/resources/struts-admin.xml", setList.get(0).getFiles().get(1).getUrl()); + + assertEquals("s2fileset2", setList.get(1).getId()); + assertEquals("Removed Set", setList.get(1).getName()); + assertTrue(setList.get(1).isRemoved()); + assertEquals(1, setList.get(1).getFiles().size()); + } + + public void testRoundTripPreservesPropertiesKeysDisabled() { + final StrutsFacet facet = StrutsFacet.getInstance(getModule()); + assertNotNull(facet); + final StrutsFacetConfiguration config = facet.getConfiguration(); + config.setPropertiesKeysDisabled(true); + + final Element state = config.getState(); + assertNotNull(state); + + final StrutsFacetConfiguration loaded = createDisposableConfig(); + loaded.loadState(state); + assertTrue(loaded.isPropertiesKeysDisabled()); + } + + public void testDefaultStateHasPropertiesKeysEnabled() { + final StrutsFacetConfiguration config = createDisposableConfig(); + + final Element emptyState = new Element("state"); + config.loadState(emptyState); + + assertFalse(config.isPropertiesKeysDisabled()); + assertTrue(config.getFileSets().isEmpty()); + } + + public void testMalformedFileSetWithoutIdIsSkipped() { + final Element state = new Element("state"); + final Element badFileSet = new Element("fileset"); + badFileSet.setAttribute("name", "Has Name"); + state.addContent(badFileSet); + + final StrutsFacetConfiguration config = createDisposableConfig(); + config.loadState(state); + + assertTrue(config.getFileSets().isEmpty()); + } + + public void testMalformedFileSetWithoutNameIsSkipped() { + final Element state = new Element("state"); + final Element badFileSet = new Element("fileset"); + badFileSet.setAttribute("id", "s2fileset1"); + state.addContent(badFileSet); + + final StrutsFacetConfiguration config = createDisposableConfig(); + config.loadState(state); + + assertTrue(config.getFileSets().isEmpty()); + } + + public void testLoadStateClearsPreviousState() { + final StrutsFacetConfiguration config = createDisposableConfig(); + + final Element initialState = new Element("state"); + final Element fileSetElement = new Element("fileset"); + fileSetElement.setAttribute("id", "s2fileset1"); + fileSetElement.setAttribute("name", "First"); + fileSetElement.setAttribute("removed", "false"); + initialState.addContent(fileSetElement); + + final Element propsElement = new Element("propertiesKeys"); + propsElement.setAttribute("disabled", "true"); + initialState.addContent(propsElement); + + config.loadState(initialState); + assertEquals(1, config.getFileSets().size()); + assertTrue(config.isPropertiesKeysDisabled()); + + config.loadState(new Element("state")); + + assertTrue(config.getFileSets().isEmpty()); + assertFalse(config.isPropertiesKeysDisabled()); + } + + public void testXmlStructureBackwardCompatibility() { + final StrutsFacet facet = StrutsFacet.getInstance(getModule()); + assertNotNull(facet); + final StrutsFacetConfiguration config = facet.getConfiguration(); + + final StrutsFileSet fileSet = createTrackedFileSet("s2fileset1", "Test", config); + fileSet.addFile("file:///test/struts.xml"); + fileSet.setRemoved(true); + config.getFileSets().add(fileSet); + config.setPropertiesKeysDisabled(true); + + final Element state = config.getState(); + assertNotNull(state); + + final List<Element> filesets = state.getChildren("fileset"); + assertEquals(1, filesets.size()); + assertEquals("s2fileset1", filesets.get(0).getAttributeValue("id")); + assertEquals("Test", filesets.get(0).getAttributeValue("name")); + assertEquals("true", filesets.get(0).getAttributeValue("removed")); + + final List<Element> files = filesets.get(0).getChildren("file"); + assertEquals(1, files.size()); + assertEquals("file:///test/struts.xml", files.get(0).getText()); + + final Element propertiesKeys = state.getChild("propertiesKeys"); + assertNotNull(propertiesKeys); + assertEquals("true", propertiesKeys.getAttributeValue("disabled")); + } +}
