refactor(facet): migrate FacetConfiguration persistence to PersistentStateComponent

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

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() {

src/main/java/com/intellij/struts2/facet/StrutsFacetConfiguration.java

@@ -23,16 +23,16 @@
 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 @@
  *
  * @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 FacetEditorTab[] createEditorTabs(final FacetEditorContext editorContext,
   }
 
   @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 void readExternal(final Element element) throws InvalidDataException {
       }
     }
 
-    // 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() {

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"));
+  }
+}