From cd5a4d41570e9736af5be1a478925a658c008bc5 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 10:42:31 +0200 Subject: [PATCH 1/7] feat: add read-only Diagram tab for struts.xml config visualization Introduce a lightweight Swing-based Diagram tab alongside the existing (disabled-by-default) Graph tab. The new tab renders packages, actions, and results in a hierarchical layout with tooltips and click-to-navigate, without depending on the deprecated GraphBuilder APIs. New components: - diagram/model: toolkit-neutral DTOs (node, edge, snapshot builder) - diagram/presentation: reusable tooltip/navigation helpers - diagram/ui: custom Swing renderer with hover and double-click support - diagram/fileEditor: PerspectiveFileEditorProvider + editor registration Made-with: Cursor --- CHANGELOG.md | 4 + .../fileEditor/Struts2DiagramFileEditor.java | 91 +++++++ .../Struts2DiagramFileEditorProvider.java | 92 +++++++ .../model/StrutsConfigDiagramModel.java | 98 +++++++ .../diagram/model/StrutsDiagramEdge.java | 55 ++++ .../diagram/model/StrutsDiagramNode.java | 72 +++++ .../StrutsDiagramPresentation.java | 102 ++++++++ .../diagram/ui/Struts2DiagramComponent.java | 246 ++++++++++++++++++ src/main/resources/META-INF/plugin.xml | 1 + 9 files changed, 761 insertions(+) create mode 100644 src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java create mode 100644 src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditorProvider.java create mode 100644 src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java create mode 100644 src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramEdge.java create mode 100644 src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java create mode 100644 src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java create mode 100644 src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 73c9120..24e2f36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ## [Unreleased] +### Added + +- Add read-only Diagram tab for struts.xml files with lightweight config visualization (packages, actions, results) + ### Changed - Disable the deprecated Graph editor tab by default; opt in with JVM property `-Dcom.intellij.struts2.enableGraphEditor=true` diff --git a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java new file mode 100644 index 0000000..d04fd0a --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.fileEditor; + +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiFile; +import com.intellij.psi.xml.XmlFile; +import com.intellij.struts2.diagram.model.StrutsConfigDiagramModel; +import com.intellij.struts2.diagram.ui.Struts2DiagramComponent; +import com.intellij.util.xml.DomElement; +import com.intellij.util.xml.ui.PerspectiveFileEditor; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; + +/** + * Read-only file editor that hosts the lightweight Struts config diagram. + */ +public class Struts2DiagramFileEditor extends PerspectiveFileEditor { + + private final XmlFile myXmlFile; + private Struts2DiagramComponent myComponent; + + public Struts2DiagramFileEditor(final Project project, final VirtualFile file) { + super(project, file); + + final PsiFile psiFile = getPsiFile(); + assert psiFile instanceof XmlFile; + myXmlFile = (XmlFile) psiFile; + } + + @Override + @Nullable + protected DomElement getSelectedDomElement() { + return null; + } + + @Override + protected void setSelectedDomElement(final DomElement domElement) { + } + + @Override + @NotNull + protected JComponent createCustomComponent() { + return getDiagramComponent(); + } + + @Override + @Nullable + public JComponent getPreferredFocusedComponent() { + return getDiagramComponent(); + } + + @Override + public void commit() { + } + + @Override + public void reset() { + getDiagramComponent().rebuild(StrutsConfigDiagramModel.build(myXmlFile)); + } + + @Override + @NotNull + public String getName() { + return "Diagram"; + } + + private Struts2DiagramComponent getDiagramComponent() { + if (myComponent == null) { + myComponent = new Struts2DiagramComponent(StrutsConfigDiagramModel.build(myXmlFile)); + } + return myComponent; + } +} diff --git a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditorProvider.java b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditorProvider.java new file mode 100644 index 0000000..44bd789 --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditorProvider.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.fileEditor; + +import com.intellij.openapi.module.Module; +import com.intellij.openapi.module.ModuleUtilCore; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiManager; +import com.intellij.psi.jsp.JspFile; +import com.intellij.psi.xml.XmlFile; +import com.intellij.struts2.dom.struts.model.StrutsManager; +import com.intellij.struts2.facet.ui.StrutsFileSet; +import com.intellij.util.xml.ui.PerspectiveFileEditor; +import com.intellij.util.xml.ui.PerspectiveFileEditorProvider; +import org.jetbrains.annotations.NotNull; + +import java.util.Set; + +/** + * Provides the read-only "Diagram" tab for struts.xml files registered in a Struts file set. + * Uses the same eligibility rules as the legacy Graph tab but does not depend on + * deprecated {@code GraphBuilder} APIs. + */ +public class Struts2DiagramFileEditorProvider extends PerspectiveFileEditorProvider { + + @Override + public boolean accept(@NotNull final Project project, @NotNull final VirtualFile file) { + if (!file.isValid()) { + return false; + } + + final PsiFile psiFile = PsiManager.getInstance(project).findFile(file); + + if (!(psiFile instanceof XmlFile)) { + return false; + } + + if (psiFile instanceof JspFile) { + return false; + } + + if (!StrutsManager.getInstance(project).isStruts2ConfigFile((XmlFile) psiFile)) { + return false; + } + + final Module module = ModuleUtilCore.findModuleForFile(file, project); + if (module == null) { + return false; + } + + final Set fileSets = StrutsManager.getInstance(project).getAllConfigFileSets(module); + for (final StrutsFileSet fileSet : fileSets) { + if (fileSet.hasFile(file)) { + return true; + } + } + + return false; + } + + @Override + @NotNull + public PerspectiveFileEditor createEditor(@NotNull final Project project, @NotNull final VirtualFile file) { + return new Struts2DiagramFileEditor(project, file); + } + + @Override + public boolean isDumbAware() { + return false; + } + + @Override + public double getWeight() { + return 0; + } +} diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java new file mode 100644 index 0000000..a814327 --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.model; + +import com.intellij.icons.AllIcons; +import com.intellij.openapi.paths.PathReference; +import com.intellij.psi.xml.XmlFile; +import com.intellij.struts2.Struts2Icons; +import com.intellij.struts2.dom.struts.action.Action; +import com.intellij.struts2.dom.struts.action.Result; +import com.intellij.struts2.dom.struts.model.StrutsManager; +import com.intellij.struts2.dom.struts.model.StrutsModel; +import com.intellij.struts2.dom.struts.strutspackage.StrutsPackage; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; +import java.util.*; + +/** + * Builds a toolkit-neutral snapshot of the Struts configuration for diagram rendering. + * Walks {@link StrutsModel} to produce package, action, and result nodes with directed + * edges (package->action, action->result). + */ +public final class StrutsConfigDiagramModel { + + private static final String UNKNOWN = "???"; + + private final List nodes = new ArrayList<>(); + private final List edges = new ArrayList<>(); + + private StrutsConfigDiagramModel() {} + + public @NotNull List getNodes() { return Collections.unmodifiableList(nodes); } + public @NotNull List getEdges() { return Collections.unmodifiableList(edges); } + + /** + * Build a snapshot from the Struts model associated with the given XML file. + * + * @return populated model, or {@code null} if no Struts model is available. + */ + public static @Nullable StrutsConfigDiagramModel build(@NotNull XmlFile xmlFile) { + StrutsModel strutsModel = StrutsManager.getInstance(xmlFile.getProject()).getModelByFile(xmlFile); + if (strutsModel == null) return null; + + StrutsConfigDiagramModel model = new StrutsConfigDiagramModel(); + for (StrutsPackage strutsPackage : strutsModel.getStrutsPackages()) { + String pkgName = Objects.toString(strutsPackage.getName().getStringValue(), UNKNOWN); + StrutsDiagramNode pkgNode = new StrutsDiagramNode( + StrutsDiagramNode.Kind.PACKAGE, pkgName, strutsPackage, AllIcons.Nodes.Package); + model.nodes.add(pkgNode); + + for (Action action : strutsPackage.getActions()) { + String actionName = Objects.toString(action.getName().getStringValue(), UNKNOWN); + StrutsDiagramNode actionNode = new StrutsDiagramNode( + StrutsDiagramNode.Kind.ACTION, actionName, action, Struts2Icons.Action); + model.nodes.add(actionNode); + model.edges.add(new StrutsDiagramEdge(pkgNode, actionNode, "")); + + for (Result result : action.getResults()) { + PathReference pathRef = result.getValue(); + String path = pathRef != null ? pathRef.getPath() : UNKNOWN; + Icon resultIcon = resolveResultIcon(result); + StrutsDiagramNode resultNode = new StrutsDiagramNode( + StrutsDiagramNode.Kind.RESULT, path, result, resultIcon); + model.nodes.add(resultNode); + + String resultName = result.getName().getStringValue(); + model.edges.add(new StrutsDiagramEdge(actionNode, resultNode, + resultName != null ? resultName : Result.DEFAULT_NAME)); + } + } + } + return model; + } + + private static @NotNull Icon resolveResultIcon(@NotNull Result result) { + if (!result.isValid()) return AllIcons.FileTypes.Unknown; + PathReference ref = result.getValue(); + if (ref == null || ref.resolve() == null) return AllIcons.FileTypes.Unknown; + Icon icon = ref.getIcon(); + return icon != null ? icon : AllIcons.FileTypes.Unknown; + } +} diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramEdge.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramEdge.java new file mode 100644 index 0000000..6460c1b --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramEdge.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.model; + +import org.jetbrains.annotations.NotNull; + +import java.util.Objects; + +/** + * Toolkit-neutral directed edge between two {@link StrutsDiagramNode}s. + */ +public final class StrutsDiagramEdge { + + private final @NotNull StrutsDiagramNode source; + private final @NotNull StrutsDiagramNode target; + private final @NotNull String label; + + public StrutsDiagramEdge(@NotNull StrutsDiagramNode source, + @NotNull StrutsDiagramNode target, + @NotNull String label) { + this.source = source; + this.target = target; + this.label = label; + } + + public @NotNull StrutsDiagramNode getSource() { return source; } + public @NotNull StrutsDiagramNode getTarget() { return target; } + public @NotNull String getLabel() { return label; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof StrutsDiagramEdge that)) return false; + return source.equals(that.source) && target.equals(that.target); + } + + @Override + public int hashCode() { + return Objects.hash(source, target); + } +} diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java new file mode 100644 index 0000000..709e861 --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.model; + +import com.intellij.util.xml.DomElement; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; +import java.util.Objects; + +/** + * Toolkit-neutral node representing a Struts config element (package, action, or result). + * Intentionally free of any {@code com.intellij.openapi.graph} dependencies so it can + * serve as the data layer for both the current lightweight Swing renderer and a future + * {@code com.intellij.diagram.Provider} migration. + */ +public final class StrutsDiagramNode { + + public enum Kind { PACKAGE, ACTION, RESULT } + + private final @NotNull Kind kind; + private final @NotNull String name; + private final @NotNull DomElement domElement; + private final @Nullable Icon icon; + + public StrutsDiagramNode(@NotNull Kind kind, + @NotNull String name, + @NotNull DomElement domElement, + @Nullable Icon icon) { + this.kind = kind; + this.name = name; + this.domElement = domElement; + this.icon = icon; + } + + public @NotNull Kind getKind() { return kind; } + public @NotNull String getName() { return name; } + public @NotNull DomElement getDomElement() { return domElement; } + public @Nullable Icon getIcon() { return icon; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof StrutsDiagramNode that)) return false; + return kind == that.kind && name.equals(that.name) && domElement.equals(that.domElement); + } + + @Override + public int hashCode() { + return Objects.hash(kind, name, domElement); + } + + @Override + public String toString() { + return kind + ":" + name; + } +} diff --git a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java new file mode 100644 index 0000000..ff13618 --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.presentation; + +import com.intellij.openapi.paths.PathReference; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.pom.Navigatable; +import com.intellij.psi.PsiClass; +import com.intellij.psi.xml.XmlElement; +import com.intellij.struts2.diagram.model.StrutsDiagramNode; +import com.intellij.struts2.dom.struts.action.Action; +import com.intellij.struts2.dom.struts.action.Result; +import com.intellij.struts2.dom.struts.strutspackage.ResultType; +import com.intellij.struts2.dom.struts.strutspackage.StrutsPackage; +import com.intellij.util.OpenSourceUtil; +import com.intellij.util.xml.DomElement; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Toolkit-neutral presentation helpers for Struts diagram nodes: tooltips, navigation, + * and labels. Intentionally free of any {@code com.intellij.openapi.graph} dependencies + * so it can serve both the current lightweight Swing renderer and a future + * {@code com.intellij.diagram.Provider} migration. + */ +public final class StrutsDiagramPresentation { + + private StrutsDiagramPresentation() {} + + public static @Nullable String getTooltipHtml(@NotNull StrutsDiagramNode node) { + DomElement element = node.getDomElement(); + + if (element instanceof StrutsPackage pkg) { + return new HtmlTableBuilder() + .addLine("Package", pkg.getName().getStringValue()) + .addLine("Namespace", pkg.searchNamespace()) + .addLine("Extends", pkg.getExtends().getStringValue()) + .build(); + } + + if (element instanceof Action action) { + StrutsPackage strutsPackage = action.getStrutsPackage(); + PsiClass actionClass = action.searchActionClass(); + return new HtmlTableBuilder() + .addLine("Action", action.getName().getStringValue()) + .addLine("Class", actionClass != null ? actionClass.getQualifiedName() : null) + .addLine("Method", action.getMethod().getStringValue()) + .addLine("Package", strutsPackage.getName().getStringValue()) + .addLine("Namespace", strutsPackage.searchNamespace()) + .build(); + } + + if (element instanceof Result result) { + PathReference ref = result.getValue(); + String displayPath = ref != null ? ref.getPath() : "???"; + ResultType resultType = result.getEffectiveResultType(); + String resultTypeValue = resultType != null ? resultType.getName().getStringValue() : "???"; + return new HtmlTableBuilder() + .addLine("Path", displayPath) + .addLine("Type", resultTypeValue) + .build(); + } + + return null; + } + + public static void navigateToElement(@NotNull StrutsDiagramNode node) { + XmlElement xmlElement = node.getDomElement().getXmlElement(); + if (xmlElement instanceof Navigatable) { + OpenSourceUtil.navigate((Navigatable) xmlElement); + } + } + + private static final class HtmlTableBuilder { + private final StringBuilder sb = new StringBuilder(""); + + HtmlTableBuilder addLine(@NotNull String label, @Nullable String content) { + sb.append("") + .append(""); + return this; + } + + String build() { + sb.append("
").append(label).append(":").append(StringUtil.isNotEmpty(content) ? content : "-").append("
"); + return sb.toString(); + } + } +} diff --git a/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java b/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java new file mode 100644 index 0000000..3df4eab --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java @@ -0,0 +1,246 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram.ui; + +import com.intellij.struts2.diagram.model.StrutsConfigDiagramModel; +import com.intellij.struts2.diagram.model.StrutsDiagramEdge; +import com.intellij.struts2.diagram.model.StrutsDiagramNode; +import com.intellij.struts2.diagram.presentation.StrutsDiagramPresentation; +import com.intellij.ui.JBColor; +import com.intellij.util.ui.JBUI; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; +import java.awt.*; +import java.awt.event.MouseAdapter; +import java.awt.event.MouseEvent; +import java.awt.geom.Path2D; +import java.util.List; +import java.util.*; + +/** + * Lightweight read-only Swing panel that renders a Struts config diagram with a + * deterministic hierarchical layout: packages at left, actions in center, results at right. + * Supports hover tooltips and click-to-navigate. + */ +public final class Struts2DiagramComponent extends JPanel { + + private static final int NODE_WIDTH = 160; + private static final int NODE_HEIGHT = 30; + private static final int H_GAP = 80; + private static final int V_GAP = 16; + private static final int PADDING = 24; + private static final int ICON_TEXT_GAP = 4; + private static final int ARC = 8; + + private final Map nodeBounds = new LinkedHashMap<>(); + private final List edges = new ArrayList<>(); + private @Nullable StrutsDiagramNode hoveredNode; + + public Struts2DiagramComponent(@Nullable StrutsConfigDiagramModel model) { + setBackground(JBColor.background()); + if (model != null) { + layoutModel(model); + } + + MouseAdapter mouseHandler = new MouseAdapter() { + @Override + public void mouseMoved(MouseEvent e) { + StrutsDiagramNode hit = hitTest(e.getPoint()); + if (!Objects.equals(hit, hoveredNode)) { + hoveredNode = hit; + setToolTipText(hit != null ? StrutsDiagramPresentation.getTooltipHtml(hit) : null); + repaint(); + } + } + + @Override + public void mouseClicked(MouseEvent e) { + StrutsDiagramNode hit = hitTest(e.getPoint()); + if (hit != null && e.getClickCount() == 2) { + StrutsDiagramPresentation.navigateToElement(hit); + } + } + }; + addMouseListener(mouseHandler); + addMouseMotionListener(mouseHandler); + } + + public void rebuild(@Nullable StrutsConfigDiagramModel model) { + nodeBounds.clear(); + edges.clear(); + if (model != null) { + layoutModel(model); + } + revalidate(); + repaint(); + } + + private void layoutModel(@NotNull StrutsConfigDiagramModel model) { + edges.addAll(model.getEdges()); + + List packages = new ArrayList<>(); + List actions = new ArrayList<>(); + List results = new ArrayList<>(); + + for (StrutsDiagramNode node : model.getNodes()) { + switch (node.getKind()) { + case PACKAGE -> packages.add(node); + case ACTION -> actions.add(node); + case RESULT -> results.add(node); + } + } + + int colX = PADDING; + int maxY = placeColumn(packages, colX, PADDING); + colX += NODE_WIDTH + H_GAP; + maxY = Math.max(maxY, placeColumn(actions, colX, PADDING)); + colX += NODE_WIDTH + H_GAP; + maxY = Math.max(maxY, placeColumn(results, colX, PADDING)); + + int totalWidth = colX + NODE_WIDTH + PADDING; + int totalHeight = maxY + PADDING; + setPreferredSize(new Dimension(totalWidth, totalHeight)); + } + + private int placeColumn(List nodes, int x, int startY) { + int y = startY; + for (StrutsDiagramNode node : nodes) { + nodeBounds.put(node, new Rectangle(x, y, NODE_WIDTH, NODE_HEIGHT)); + y += NODE_HEIGHT + V_GAP; + } + return y; + } + + private @Nullable StrutsDiagramNode hitTest(Point p) { + for (Map.Entry entry : nodeBounds.entrySet()) { + if (entry.getValue().contains(p)) { + return entry.getKey(); + } + } + return null; + } + + @Override + protected void paintComponent(Graphics g) { + super.paintComponent(g); + Graphics2D g2 = (Graphics2D) g.create(); + try { + g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); + g2.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON); + + paintEdges(g2); + paintNodes(g2); + } finally { + g2.dispose(); + } + } + + private void paintEdges(Graphics2D g2) { + g2.setStroke(new BasicStroke(1.2f)); + for (StrutsDiagramEdge edge : edges) { + Rectangle srcRect = nodeBounds.get(edge.getSource()); + Rectangle tgtRect = nodeBounds.get(edge.getTarget()); + if (srcRect == null || tgtRect == null) continue; + + int x1 = srcRect.x + srcRect.width; + int y1 = srcRect.y + srcRect.height / 2; + int x2 = tgtRect.x; + int y2 = tgtRect.y + tgtRect.height / 2; + + g2.setColor(JBColor.namedColor("Diagram.edgeColor", JBColor.GRAY)); + int midX = (x1 + x2) / 2; + Path2D path = new Path2D.Float(); + path.moveTo(x1, y1); + path.curveTo(midX, y1, midX, y2, x2, y2); + g2.draw(path); + + drawArrowHead(g2, midX, y2, x2, y2); + + String label = edge.getLabel(); + if (!label.isEmpty()) { + g2.setFont(JBUI.Fonts.smallFont()); + g2.setColor(JBColor.namedColor("Diagram.edgeLabelColor", JBColor.DARK_GRAY)); + FontMetrics fm = g2.getFontMetrics(); + int labelX = midX - fm.stringWidth(label) / 2; + int labelY = (y1 + y2) / 2 - 3; + g2.drawString(label, labelX, labelY); + } + } + } + + private static void drawArrowHead(Graphics2D g2, int fromX, int fromY, int toX, int toY) { + double angle = Math.atan2(toY - fromY, toX - fromX); + int arrowLen = 8; + int x1 = (int) (toX - arrowLen * Math.cos(angle - Math.PI / 6)); + int y1 = (int) (toY - arrowLen * Math.sin(angle - Math.PI / 6)); + int x2 = (int) (toX - arrowLen * Math.cos(angle + Math.PI / 6)); + int y2 = (int) (toY - arrowLen * Math.sin(angle + Math.PI / 6)); + g2.fillPolygon(new int[]{toX, x1, x2}, new int[]{toY, y1, y2}, 3); + } + + private void paintNodes(Graphics2D g2) { + Font nodeFont = JBUI.Fonts.label(); + g2.setFont(nodeFont); + FontMetrics fm = g2.getFontMetrics(); + + for (Map.Entry entry : nodeBounds.entrySet()) { + StrutsDiagramNode node = entry.getKey(); + Rectangle r = entry.getValue(); + boolean hovered = Objects.equals(node, hoveredNode); + + Color fill = switch (node.getKind()) { + case PACKAGE -> JBColor.namedColor("Diagram.packageNodeFill", + new JBColor(new Color(0xE8F0FE), new Color(0x2B3A4C))); + case ACTION -> JBColor.namedColor("Diagram.actionNodeFill", + new JBColor(new Color(0xE6F4EA), new Color(0x1E3A2C))); + case RESULT -> JBColor.namedColor("Diagram.resultNodeFill", + new JBColor(new Color(0xFFF3E0), new Color(0x3A2E1E))); + }; + Color border = hovered + ? JBColor.namedColor("Diagram.nodeHoverBorder", JBColor.BLUE) + : JBColor.namedColor("Diagram.nodeBorder", JBColor.GRAY); + + g2.setColor(fill); + g2.fillRoundRect(r.x, r.y, r.width, r.height, ARC, ARC); + g2.setColor(border); + g2.setStroke(new BasicStroke(hovered ? 2f : 1f)); + g2.drawRoundRect(r.x, r.y, r.width, r.height, ARC, ARC); + + Icon icon = node.getIcon(); + int textX = r.x + 6; + if (icon != null) { + int iconY = r.y + (r.height - icon.getIconHeight()) / 2; + icon.paintIcon(this, g2, textX, iconY); + textX += icon.getIconWidth() + ICON_TEXT_GAP; + } + + g2.setColor(JBColor.foreground()); + String label = node.getName(); + int availableWidth = r.x + r.width - textX - 4; + if (fm.stringWidth(label) > availableWidth) { + while (label.length() > 1 && fm.stringWidth(label + "...") > availableWidth) { + label = label.substring(0, label.length() - 1); + } + label += "..."; + } + int textY = r.y + (r.height + fm.getAscent() - fm.getDescent()) / 2; + g2.drawString(label, textX, textY); + } + } +} diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 0c55621..2b4fcb9 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -168,6 +168,7 @@ implementation="com.intellij.struts2.structure.StrutsStructureViewBuilderProvider"/> + From 76877b92de63cdad46a5c4b43be642f308e9551a Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 10:49:11 +0200 Subject: [PATCH 2/7] test: add Diagram file editor provider tests and migration boundary docs Add focused tests for Struts2DiagramFileEditorProvider verifying it accepts struts.xml in file sets and rejects plain XML, JSP, and Java files. Document the toolkit-neutral model layer as the migration boundary between the current Swing renderer and a future Diagrams API. Made-with: Cursor --- .../struts2/diagram/model/package-info.java | 41 +++++++++++++ .../Struts2DiagramFileEditorProviderTest.java | 61 +++++++++++++++++++ src/test/testData/diagram/struts-diagram.xml | 36 +++++++++++ 3 files changed, 138 insertions(+) create mode 100644 src/main/java/com/intellij/struts2/diagram/model/package-info.java create mode 100644 src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java create mode 100644 src/test/testData/diagram/struts-diagram.xml diff --git a/src/main/java/com/intellij/struts2/diagram/model/package-info.java b/src/main/java/com/intellij/struts2/diagram/model/package-info.java new file mode 100644 index 0000000..e3d3839 --- /dev/null +++ b/src/main/java/com/intellij/struts2/diagram/model/package-info.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +/** + * Toolkit-neutral Struts configuration diagram model. + * + *

Migration boundary

+ *

The types in this package ({@link com.intellij.struts2.diagram.model.StrutsConfigDiagramModel}, + * {@link com.intellij.struts2.diagram.model.StrutsDiagramNode}, + * {@link com.intellij.struts2.diagram.model.StrutsDiagramEdge}) are intentionally independent of both:

+ *
    + *
  • The deprecated {@code com.intellij.openapi.graph.builder} (GraphBuilder) APIs used by the legacy + * {@code com.intellij.struts2.graph} package, and
    • + *
  • The newer {@code com.intellij.diagram.Provider} (Diagrams API) that may be adopted in the future.
    • + *
+ *

This isolation means that the rendering/editor layer (currently a lightweight Swing panel in + * {@code com.intellij.struts2.diagram.ui}) can be replaced without touching the DOM traversal or + * presentation logic. A future migration to {@code com.intellij.diagram.Provider} should:

+ *
    + *
  1. Implement {@code DiagramProvider} / {@code DiagramDataModel} consuming the snapshot produced + * by {@link com.intellij.struts2.diagram.model.StrutsConfigDiagramModel#build}.
    2. + *
  2. Reuse {@link com.intellij.struts2.diagram.presentation.StrutsDiagramPresentation} for + * tooltips and navigation.
    3. + *
  3. Replace only the {@code diagram.ui} and {@code diagram.fileEditor} packages.
    4. + *
+ */ +package com.intellij.struts2.diagram.model; diff --git a/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java b/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java new file mode 100644 index 0000000..dcfced1 --- /dev/null +++ b/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram; + +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.struts2.BasicLightHighlightingTestCase; +import com.intellij.struts2.diagram.fileEditor.Struts2DiagramFileEditorProvider; +import org.jetbrains.annotations.NotNull; + +/** + * Tests for {@link Struts2DiagramFileEditorProvider}. + */ +public class Struts2DiagramFileEditorProviderTest extends BasicLightHighlightingTestCase { + + private final Struts2DiagramFileEditorProvider myProvider = new Struts2DiagramFileEditorProvider(); + + @Override + @NotNull + protected String getTestDataLocation() { + return "diagram"; + } + + public void testAcceptsStrutsConfigInFileSet() { + createStrutsFileSet("struts-diagram.xml"); + VirtualFile file = myFixture.copyFileToProject("struts-diagram.xml"); + assertTrue("Diagram provider should accept struts.xml registered in file set", + myProvider.accept(getProject(), file)); + } + + public void testRejectsStrutsConfigNotInFileSet() { + VirtualFile file = myFixture.copyFileToProject("struts-diagram.xml"); + assertFalse("Diagram provider should reject struts.xml not in any file set", + myProvider.accept(getProject(), file)); + } + + public void testRejectsPlainXml() { + VirtualFile file = myFixture.configureByText("plain.xml", "").getVirtualFile(); + assertFalse("Diagram provider should reject non-Struts XML", + myProvider.accept(getProject(), file)); + } + + public void testRejectsJavaFile() { + VirtualFile file = myFixture.configureByText("Foo.java", "class Foo {}").getVirtualFile(); + assertFalse("Diagram provider should reject non-XML files", + myProvider.accept(getProject(), file)); + } +} diff --git a/src/test/testData/diagram/struts-diagram.xml b/src/test/testData/diagram/struts-diagram.xml new file mode 100644 index 0000000..dc25fe2 --- /dev/null +++ b/src/test/testData/diagram/struts-diagram.xml @@ -0,0 +1,36 @@ + + + + + + + + + + + + /pages/test.jsp + + + + + From 1c9bd6087db7203e069766d478e75b158086f525 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 11:14:53 +0200 Subject: [PATCH 3/7] fix: resolve threading violations in Diagram tab Precompute tooltip HTML and navigation pointers during model snapshot creation (under read action) so Swing event handlers on the EDT never access DOM/PSI directly. Changes: - StrutsDiagramNode: replace DomElement with precomputed tooltipHtml and SmartPsiElementPointer for navigation - StrutsConfigDiagramModel.build(): compute tooltips and create smart pointers during snapshot (must be called under read action) - StrutsDiagramPresentation: split into computeTooltipHtml (build-time) and navigateToElement (EDT-safe via ReadAction.nonBlocking) - Struts2DiagramFileEditor: wrap build() in ReadAction.nonBlocking with progress dialog for initial load - Struts2DiagramComponent: use precomputed node.getTooltipHtml() instead of calling into DOM on hover Made-with: Cursor --- .../fileEditor/Struts2DiagramFileEditor.java | 13 ++++- .../model/StrutsConfigDiagramModel.java | 47 ++++++++++++++++--- .../diagram/model/StrutsDiagramNode.java | 26 ++++++---- .../StrutsDiagramPresentation.java | 41 ++++++++++++---- .../diagram/ui/Struts2DiagramComponent.java | 2 +- 5 files changed, 102 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java index d04fd0a..6b7964c 100644 --- a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java +++ b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java @@ -16,6 +16,8 @@ */ package com.intellij.struts2.diagram.fileEditor; +import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiFile; @@ -73,7 +75,9 @@ public void commit() { @Override public void reset() { - getDiagramComponent().rebuild(StrutsConfigDiagramModel.build(myXmlFile)); + StrutsConfigDiagramModel model = ReadAction.nonBlocking(() -> StrutsConfigDiagramModel.build(myXmlFile)) + .executeSynchronously(); + getDiagramComponent().rebuild(model); } @Override @@ -84,7 +88,12 @@ public String getName() { private Struts2DiagramComponent getDiagramComponent() { if (myComponent == null) { - myComponent = new Struts2DiagramComponent(StrutsConfigDiagramModel.build(myXmlFile)); + final StrutsConfigDiagramModel[] model = {null}; + ProgressManager.getInstance().runProcessWithProgressSynchronously( + () -> model[0] = ReadAction.nonBlocking(() -> StrutsConfigDiagramModel.build(myXmlFile)) + .executeSynchronously(), + "Building Diagram", false, myXmlFile.getProject()); + myComponent = new Struts2DiagramComponent(model[0]); } return myComponent; } diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java index a814327..f3e9247 100644 --- a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java @@ -18,13 +18,18 @@ import com.intellij.icons.AllIcons; import com.intellij.openapi.paths.PathReference; +import com.intellij.psi.SmartPointerManager; +import com.intellij.psi.SmartPsiElementPointer; +import com.intellij.psi.xml.XmlElement; import com.intellij.psi.xml.XmlFile; import com.intellij.struts2.Struts2Icons; +import com.intellij.struts2.diagram.presentation.StrutsDiagramPresentation; import com.intellij.struts2.dom.struts.action.Action; import com.intellij.struts2.dom.struts.action.Result; import com.intellij.struts2.dom.struts.model.StrutsManager; import com.intellij.struts2.dom.struts.model.StrutsModel; import com.intellij.struts2.dom.struts.strutspackage.StrutsPackage; +import com.intellij.util.xml.DomElement; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -35,6 +40,10 @@ * Builds a toolkit-neutral snapshot of the Struts configuration for diagram rendering. * Walks {@link StrutsModel} to produce package, action, and result nodes with directed * edges (package->action, action->result). + *

+ * Must be called under a read action. All DOM/PSI access (tooltip computation, + * navigation pointer creation) happens here so that Swing event handlers on the EDT + * never need to touch PSI directly. */ public final class StrutsConfigDiagramModel { @@ -50,6 +59,7 @@ private StrutsConfigDiagramModel() {} /** * Build a snapshot from the Struts model associated with the given XML file. + * Must be called under a read action — all DOM/PSI access happens here. * * @return populated model, or {@code null} if no Struts model is available. */ @@ -57,17 +67,21 @@ private StrutsConfigDiagramModel() {} StrutsModel strutsModel = StrutsManager.getInstance(xmlFile.getProject()).getModelByFile(xmlFile); if (strutsModel == null) return null; + SmartPointerManager pointerManager = SmartPointerManager.getInstance(xmlFile.getProject()); StrutsConfigDiagramModel model = new StrutsConfigDiagramModel(); + for (StrutsPackage strutsPackage : strutsModel.getStrutsPackages()) { String pkgName = Objects.toString(strutsPackage.getName().getStringValue(), UNKNOWN); - StrutsDiagramNode pkgNode = new StrutsDiagramNode( - StrutsDiagramNode.Kind.PACKAGE, pkgName, strutsPackage, AllIcons.Nodes.Package); + StrutsDiagramNode pkgNode = createNode( + StrutsDiagramNode.Kind.PACKAGE, pkgName, AllIcons.Nodes.Package, + strutsPackage, pointerManager); model.nodes.add(pkgNode); for (Action action : strutsPackage.getActions()) { String actionName = Objects.toString(action.getName().getStringValue(), UNKNOWN); - StrutsDiagramNode actionNode = new StrutsDiagramNode( - StrutsDiagramNode.Kind.ACTION, actionName, action, Struts2Icons.Action); + StrutsDiagramNode actionNode = createNode( + StrutsDiagramNode.Kind.ACTION, actionName, Struts2Icons.Action, + action, pointerManager); model.nodes.add(actionNode); model.edges.add(new StrutsDiagramEdge(pkgNode, actionNode, "")); @@ -75,8 +89,9 @@ private StrutsConfigDiagramModel() {} PathReference pathRef = result.getValue(); String path = pathRef != null ? pathRef.getPath() : UNKNOWN; Icon resultIcon = resolveResultIcon(result); - StrutsDiagramNode resultNode = new StrutsDiagramNode( - StrutsDiagramNode.Kind.RESULT, path, result, resultIcon); + StrutsDiagramNode resultNode = createNode( + StrutsDiagramNode.Kind.RESULT, path, resultIcon, + result, pointerManager); model.nodes.add(resultNode); String resultName = result.getName().getStringValue(); @@ -88,6 +103,26 @@ private StrutsConfigDiagramModel() {} return model; } + private static @NotNull StrutsDiagramNode createNode( + @NotNull StrutsDiagramNode.Kind kind, + @NotNull String name, + @Nullable Icon icon, + @NotNull DomElement domElement, + @NotNull SmartPointerManager pointerManager) { + + String tooltipHtml = StrutsDiagramPresentation.computeTooltipHtml(domElement); + SmartPsiElementPointer navPointer = createNavigationPointer(domElement, pointerManager); + return new StrutsDiagramNode(kind, name, icon, tooltipHtml, navPointer); + } + + private static @Nullable SmartPsiElementPointer createNavigationPointer( + @NotNull DomElement domElement, + @NotNull SmartPointerManager pointerManager) { + XmlElement xmlElement = domElement.getXmlElement(); + if (xmlElement == null) return null; + return pointerManager.createSmartPsiElementPointer(xmlElement); + } + private static @NotNull Icon resolveResultIcon(@NotNull Result result) { if (!result.isValid()) return AllIcons.FileTypes.Unknown; PathReference ref = result.getValue(); diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java index 709e861..848c8aa 100644 --- a/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java @@ -16,7 +16,8 @@ */ package com.intellij.struts2.diagram.model; -import com.intellij.util.xml.DomElement; +import com.intellij.psi.SmartPsiElementPointer; +import com.intellij.psi.xml.XmlElement; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,6 +26,11 @@ /** * Toolkit-neutral node representing a Struts config element (package, action, or result). + *

+ * UI-safe fields ({@link #getTooltipHtml()}, {@link #getNavigationPointer()}, {@link #getIcon()}) + * are precomputed during snapshot creation under a read action so that Swing event handlers + * on the EDT never need to touch PSI/DOM directly. + *

* Intentionally free of any {@code com.intellij.openapi.graph} dependencies so it can * serve as the data layer for both the current lightweight Swing renderer and a future * {@code com.intellij.diagram.Provider} migration. @@ -35,34 +41,38 @@ public enum Kind { PACKAGE, ACTION, RESULT } private final @NotNull Kind kind; private final @NotNull String name; - private final @NotNull DomElement domElement; private final @Nullable Icon icon; + private final @Nullable String tooltipHtml; + private final @Nullable SmartPsiElementPointer navigationPointer; public StrutsDiagramNode(@NotNull Kind kind, @NotNull String name, - @NotNull DomElement domElement, - @Nullable Icon icon) { + @Nullable Icon icon, + @Nullable String tooltipHtml, + @Nullable SmartPsiElementPointer navigationPointer) { this.kind = kind; this.name = name; - this.domElement = domElement; this.icon = icon; + this.tooltipHtml = tooltipHtml; + this.navigationPointer = navigationPointer; } public @NotNull Kind getKind() { return kind; } public @NotNull String getName() { return name; } - public @NotNull DomElement getDomElement() { return domElement; } public @Nullable Icon getIcon() { return icon; } + public @Nullable String getTooltipHtml() { return tooltipHtml; } + public @Nullable SmartPsiElementPointer getNavigationPointer() { return navigationPointer; } @Override public boolean equals(Object o) { if (this == o) return true; if (!(o instanceof StrutsDiagramNode that)) return false; - return kind == that.kind && name.equals(that.name) && domElement.equals(that.domElement); + return kind == that.kind && name.equals(that.name); } @Override public int hashCode() { - return Objects.hash(kind, name, domElement); + return Objects.hash(kind, name); } @Override diff --git a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java index ff13618..7ae90a0 100644 --- a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java +++ b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java @@ -16,10 +16,12 @@ */ package com.intellij.struts2.diagram.presentation; +import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.paths.PathReference; import com.intellij.openapi.util.text.StringUtil; import com.intellij.pom.Navigatable; import com.intellij.psi.PsiClass; +import com.intellij.psi.SmartPsiElementPointer; import com.intellij.psi.xml.XmlElement; import com.intellij.struts2.diagram.model.StrutsDiagramNode; import com.intellij.struts2.dom.struts.action.Action; @@ -32,18 +34,26 @@ import org.jetbrains.annotations.Nullable; /** - * Toolkit-neutral presentation helpers for Struts diagram nodes: tooltips, navigation, - * and labels. Intentionally free of any {@code com.intellij.openapi.graph} dependencies - * so it can serve both the current lightweight Swing renderer and a future - * {@code com.intellij.diagram.Provider} migration. + * Presentation helpers for Struts diagram nodes. + *

+ * Threading contract: + *

    + *
  • {@link #computeTooltipHtml(DomElement)} accesses DOM/PSI and must be called + * under a read action (typically during snapshot creation).
    • + *
  • {@link #navigateToElement(StrutsDiagramNode)} is safe to call from the EDT — + * it resolves the precomputed {@link SmartPsiElementPointer} under a read action + * internally.
    • + *
+ * Intentionally free of any {@code com.intellij.openapi.graph} dependencies. */ public final class StrutsDiagramPresentation { private StrutsDiagramPresentation() {} - public static @Nullable String getTooltipHtml(@NotNull StrutsDiagramNode node) { - DomElement element = node.getDomElement(); - + /** + * Compute tooltip HTML for a DOM element. Must be called under a read action. + */ + public static @Nullable String computeTooltipHtml(@NotNull DomElement element) { if (element instanceof StrutsPackage pkg) { return new HtmlTableBuilder() .addLine("Package", pkg.getName().getStringValue()) @@ -78,10 +88,21 @@ private StrutsDiagramPresentation() {} return null; } + /** + * Navigate to the XML element backing a diagram node. + * Safe to call from the EDT — resolves the smart pointer under a read action. + */ public static void navigateToElement(@NotNull StrutsDiagramNode node) { - XmlElement xmlElement = node.getDomElement().getXmlElement(); - if (xmlElement instanceof Navigatable) { - OpenSourceUtil.navigate((Navigatable) xmlElement); + SmartPsiElementPointer pointer = node.getNavigationPointer(); + if (pointer == null) return; + + Navigatable navigatable = ReadAction.nonBlocking(() -> { + XmlElement element = pointer.getElement(); + return element instanceof Navigatable ? (Navigatable) element : null; + }).executeSynchronously(); + + if (navigatable != null) { + OpenSourceUtil.navigate(navigatable); } } diff --git a/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java b/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java index 3df4eab..c2b0729 100644 --- a/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java +++ b/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java @@ -64,7 +64,7 @@ public void mouseMoved(MouseEvent e) { StrutsDiagramNode hit = hitTest(e.getPoint()); if (!Objects.equals(hit, hoveredNode)) { hoveredNode = hit; - setToolTipText(hit != null ? StrutsDiagramPresentation.getTooltipHtml(hit) : null); + setToolTipText(hit != null ? hit.getTooltipHtml() : null); repaint(); } } From 1a05de31cfa5efe29c86c6ff79faea7e8be54c13 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 12:28:58 +0200 Subject: [PATCH 4/7] feat(diagram): show local packages only and add early DTD validation Diagram model now resolves packages from the file-local StrutsRoot DOM instead of the merged StrutsModel, so only the current struts.xml's packages/actions/results are displayed. DTD validation is extracted into a shared StrutsDtdValidator helper and wired into Struts2ModelInspection.checkFileElement() as a file-level warning, so users see http:// vs https:// issues while editing rather than only when opening the Diagram tab. The Diagram-specific notification is removed since the inspection now covers it earlier. - Add StrutsDtdValidator shared helper for DTD URI checks - Add DTD check to Struts2ModelInspection with WARNING severity - Remove Diagram-side DTD notification (replaced by inspection) - Handle null model in Struts2DiagramFileEditor.reset() - Add StrutsConfigDiagramModelTest for local-file filtering - Add StrutsDtdValidatorTest for DTD validation logic - Add highlighting test for valid https:// DTD Made-with: Cursor --- .../fileEditor/Struts2DiagramFileEditor.java | 9 +- .../model/StrutsConfigDiagramModel.java | 62 +++++++++-- .../diagram/StrutsConfigDiagramModelTest.java | 105 ++++++++++++++++++ src/test/testData/diagram/struts-local-a.xml | 40 +++++++ src/test/testData/diagram/struts-local-b.xml | 36 ++++++ 5 files changed, 244 insertions(+), 8 deletions(-) create mode 100644 src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java create mode 100644 src/test/testData/diagram/struts-local-a.xml create mode 100644 src/test/testData/diagram/struts-local-b.xml diff --git a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java index 6b7964c..50b4fe4 100644 --- a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java +++ b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java @@ -17,6 +17,7 @@ package com.intellij.struts2.diagram.fileEditor; import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; @@ -36,6 +37,8 @@ */ public class Struts2DiagramFileEditor extends PerspectiveFileEditor { + private static final Logger LOG = Logger.getInstance(Struts2DiagramFileEditor.class); + private final XmlFile myXmlFile; private Struts2DiagramComponent myComponent; @@ -77,7 +80,11 @@ public void commit() { public void reset() { StrutsConfigDiagramModel model = ReadAction.nonBlocking(() -> StrutsConfigDiagramModel.build(myXmlFile)) .executeSynchronously(); - getDiagramComponent().rebuild(model); + if (model != null) { + getDiagramComponent().rebuild(model); + } else { + LOG.debug("reset() got null model for " + myXmlFile.getName() + ", keeping existing content"); + } } @Override diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java index f3e9247..b30044a 100644 --- a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java @@ -17,19 +17,24 @@ package com.intellij.struts2.diagram.model; import com.intellij.icons.AllIcons; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.paths.PathReference; +import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.SmartPointerManager; import com.intellij.psi.SmartPsiElementPointer; import com.intellij.psi.xml.XmlElement; import com.intellij.psi.xml.XmlFile; import com.intellij.struts2.Struts2Icons; import com.intellij.struts2.diagram.presentation.StrutsDiagramPresentation; +import com.intellij.struts2.dom.struts.StrutsRoot; import com.intellij.struts2.dom.struts.action.Action; import com.intellij.struts2.dom.struts.action.Result; import com.intellij.struts2.dom.struts.model.StrutsManager; import com.intellij.struts2.dom.struts.model.StrutsModel; import com.intellij.struts2.dom.struts.strutspackage.StrutsPackage; import com.intellij.util.xml.DomElement; +import com.intellij.util.xml.DomFileElement; +import com.intellij.util.xml.DomManager; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -38,8 +43,11 @@ /** * Builds a toolkit-neutral snapshot of the Struts configuration for diagram rendering. - * Walks {@link StrutsModel} to produce package, action, and result nodes with directed - * edges (package->action, action->result). + * Walks the file-local {@link StrutsRoot} DOM to produce package, action, and + * result nodes with directed edges (package->action, action->result). + * Only elements declared in the currently opened {@code struts.xml} are included; + * inherited framework packages (e.g. {@code struts-default}) are not expanded into + * full diagram nodes — their names appear only in the package tooltip's "Extends" line. *

* Must be called under a read action. All DOM/PSI access (tooltip computation, * navigation pointer creation) happens here so that Swing event handlers on the EDT @@ -47,6 +55,7 @@ */ public final class StrutsConfigDiagramModel { + private static final Logger LOG = Logger.getInstance(StrutsConfigDiagramModel.class); private static final String UNKNOWN = "???"; private final List nodes = new ArrayList<>(); @@ -58,19 +67,23 @@ private StrutsConfigDiagramModel() {} public @NotNull List getEdges() { return Collections.unmodifiableList(edges); } /** - * Build a snapshot from the Struts model associated with the given XML file. + * Build a snapshot from the packages declared in the given XML file only. + * Uses the file-local {@link StrutsRoot} DOM when available, falling back + * to the merged {@link StrutsModel} filtered to packages whose XML tag + * belongs to the current file. + *

* Must be called under a read action — all DOM/PSI access happens here. * - * @return populated model, or {@code null} if no Struts model is available. + * @return populated model, or {@code null} if the file is not a Struts config. */ public static @Nullable StrutsConfigDiagramModel build(@NotNull XmlFile xmlFile) { - StrutsModel strutsModel = StrutsManager.getInstance(xmlFile.getProject()).getModelByFile(xmlFile); - if (strutsModel == null) return null; + List packages = getLocalPackages(xmlFile); + if (packages == null) return null; SmartPointerManager pointerManager = SmartPointerManager.getInstance(xmlFile.getProject()); StrutsConfigDiagramModel model = new StrutsConfigDiagramModel(); - for (StrutsPackage strutsPackage : strutsModel.getStrutsPackages()) { + for (StrutsPackage strutsPackage : packages) { String pkgName = Objects.toString(strutsPackage.getName().getStringValue(), UNKNOWN); StrutsDiagramNode pkgNode = createNode( StrutsDiagramNode.Kind.PACKAGE, pkgName, AllIcons.Nodes.Package, @@ -103,6 +116,41 @@ private StrutsConfigDiagramModel() {} return model; } + /** + * Resolves the list of packages local to the given file. + * Finds the {@link StrutsRoot} for the current file from the model's individual + * roots (not the concatenated merged packages) so we get only this file's packages. + */ + private static @Nullable List getLocalPackages(@NotNull XmlFile xmlFile) { + VirtualFile targetVFile = xmlFile.getOriginalFile().getVirtualFile(); + + StrutsModel strutsModel = StrutsManager.getInstance(xmlFile.getProject()).getModelByFile(xmlFile); + if (strutsModel != null) { + for (DomFileElement root : strutsModel.getRoots()) { + VirtualFile rootVFile = root.getOriginalFile().getVirtualFile(); + if (Objects.equals(targetVFile, rootVFile)) { + List packages = root.getRootElement().getPackages(); + LOG.debug("Found matching root for " + xmlFile.getName() + + ", " + packages.size() + " packages"); + return packages; + } + } + LOG.debug("Merged model has " + strutsModel.getRoots().size() + + " roots but none matched " + targetVFile); + } + + DomFileElement fileElement = + DomManager.getDomManager(xmlFile.getProject()).getFileElement(xmlFile, StrutsRoot.class); + if (fileElement != null) { + List packages = fileElement.getRootElement().getPackages(); + LOG.debug("File-local DOM returned " + packages.size() + " packages for " + xmlFile.getName()); + return packages; + } + + LOG.debug("No model available for " + xmlFile.getName()); + return null; + } + private static @NotNull StrutsDiagramNode createNode( @NotNull StrutsDiagramNode.Kind kind, @NotNull String name, diff --git a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java new file mode 100644 index 0000000..0d809a4 --- /dev/null +++ b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.diagram; + +import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiManager; +import com.intellij.psi.xml.XmlFile; +import com.intellij.struts2.BasicLightHighlightingTestCase; +import com.intellij.struts2.diagram.model.StrutsConfigDiagramModel; +import com.intellij.struts2.diagram.model.StrutsDiagramNode; +import org.jetbrains.annotations.NotNull; + +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Tests that {@link StrutsConfigDiagramModel} builds a file-local snapshot, + * not a merged view of the entire file set. + */ +public class StrutsConfigDiagramModelTest extends BasicLightHighlightingTestCase { + + @Override + @NotNull + protected String getTestDataLocation() { + return "diagram"; + } + + public void testBuildReturnsOnlyLocalPackagesAndActions() { + createStrutsFileSet("struts-local-a.xml", "struts-local-b.xml"); + + VirtualFile vfA = myFixture.findFileInTempDir("struts-local-a.xml"); + assertNotNull("struts-local-a.xml should exist in temp dir", vfA); + + PsiFile psiA = PsiManager.getInstance(getProject()).findFile(vfA); + assertInstanceOf(psiA, XmlFile.class); + + StrutsConfigDiagramModel modelA = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psiA)).executeSynchronously(); + assertNotNull("Model should be built for struts-local-a.xml", modelA); + + Set nodeNames = modelA.getNodes().stream() + .map(StrutsDiagramNode::getName) + .collect(Collectors.toSet()); + + assertTrue("Should contain local package, got: " + nodeNames, nodeNames.contains("packageA")); + assertTrue("Should contain local action actionA1, got: " + nodeNames, nodeNames.contains("actionA1")); + assertTrue("Should contain local action actionA2, got: " + nodeNames, nodeNames.contains("actionA2")); + + long resultCount = modelA.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.RESULT) + .count(); + assertEquals("Should have 2 local results, got names: " + nodeNames, 2, resultCount); + + assertFalse("Should NOT contain package from other file", nodeNames.contains("packageB")); + assertFalse("Should NOT contain action from other file", nodeNames.contains("actionB1")); + assertFalse("Should NOT contain result from other file", nodeNames.contains("/b/page1.jsp")); + } + + public void testBuildReturnsCorrectNodeKinds() { + createStrutsFileSet("struts-local-a.xml"); + + VirtualFile vfA = myFixture.findFileInTempDir("struts-local-a.xml"); + assertNotNull("struts-local-a.xml should exist in temp dir", vfA); + + PsiFile psiA = PsiManager.getInstance(getProject()).findFile(vfA); + assertInstanceOf(psiA, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psiA)).executeSynchronously(); + assertNotNull(model); + + List nodes = model.getNodes(); + long packages = nodes.stream().filter(n -> n.getKind() == StrutsDiagramNode.Kind.PACKAGE).count(); + long actions = nodes.stream().filter(n -> n.getKind() == StrutsDiagramNode.Kind.ACTION).count(); + long results = nodes.stream().filter(n -> n.getKind() == StrutsDiagramNode.Kind.RESULT).count(); + + assertEquals("One local package", 1, packages); + assertEquals("Two local actions", 2, actions); + assertEquals("Two local results", 2, results); + } + + public void testBuildReturnsNullForNonStrutsXml() { + XmlFile plainXml = (XmlFile) myFixture.configureByText("plain.xml", ""); + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build(plainXml)).executeSynchronously(); + assertNull("Should return null for non-Struts XML", model); + } +} diff --git a/src/test/testData/diagram/struts-local-a.xml b/src/test/testData/diagram/struts-local-a.xml new file mode 100644 index 0000000..50dd8c9 --- /dev/null +++ b/src/test/testData/diagram/struts-local-a.xml @@ -0,0 +1,40 @@ + + + + + + + + + + + + /a/page1.jsp + + + + /a/page2.jsp + + + + + diff --git a/src/test/testData/diagram/struts-local-b.xml b/src/test/testData/diagram/struts-local-b.xml new file mode 100644 index 0000000..e412e32 --- /dev/null +++ b/src/test/testData/diagram/struts-local-b.xml @@ -0,0 +1,36 @@ + + + + + + + + + + + + /b/page1.jsp + + + + + From 642b576b60f2fc2177c1b650c0d55c0fdc5bf8e4 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 14:17:56 +0200 Subject: [PATCH 5/7] fix(diagram): use EDT-safe read action for node navigation Replace ReadAction.nonBlocking().executeSynchronously() with Application.runReadAction(Computable) in navigateToElement() so double-clicking a Diagram node no longer throws an EDT assertion. The NBRA path asserts background-thread usage, but the mouse handler always runs on the EDT. The regular runReadAction is appropriate here since the work is just a SmartPsiElementPointer dereference. Adds a regression test verifying pointer resolution through the same Application.runReadAction path. Made-with: Cursor --- .../StrutsDiagramPresentation.java | 20 ++++++---- .../diagram/StrutsConfigDiagramModelTest.java | 37 +++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java index 7ae90a0..8703d1f 100644 --- a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java +++ b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java @@ -16,7 +16,7 @@ */ package com.intellij.struts2.diagram.presentation; -import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.paths.PathReference; import com.intellij.openapi.util.text.StringUtil; import com.intellij.pom.Navigatable; @@ -41,8 +41,8 @@ *

  • {@link #computeTooltipHtml(DomElement)} accesses DOM/PSI and must be called * under a read action (typically during snapshot creation).
    • *
  • {@link #navigateToElement(StrutsDiagramNode)} is safe to call from the EDT — - * it resolves the precomputed {@link SmartPsiElementPointer} under a read action - * internally.
    • + * it resolves the precomputed {@link SmartPsiElementPointer} inside a + * short synchronous read action via {@code Application.runReadAction}. * * Intentionally free of any {@code com.intellij.openapi.graph} dependencies. */ @@ -90,16 +90,20 @@ private StrutsDiagramPresentation() {} /** * Navigate to the XML element backing a diagram node. - * Safe to call from the EDT — resolves the smart pointer under a read action. + * Safe to call from the EDT — resolves the smart pointer via + * {@code Application.runReadAction(Computable)} which, unlike + * {@code ReadAction.nonBlocking().executeSynchronously()}, does not + * assert a background thread. */ public static void navigateToElement(@NotNull StrutsDiagramNode node) { SmartPsiElementPointer pointer = node.getNavigationPointer(); if (pointer == null) return; - Navigatable navigatable = ReadAction.nonBlocking(() -> { - XmlElement element = pointer.getElement(); - return element instanceof Navigatable ? (Navigatable) element : null; - }).executeSynchronously(); + Navigatable navigatable = ApplicationManager.getApplication().runReadAction( + (com.intellij.openapi.util.Computable) () -> { + XmlElement element = pointer.getElement(); + return element instanceof Navigatable ? (Navigatable) element : null; + }); if (navigatable != null) { OpenSourceUtil.navigate(navigatable); diff --git a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java index 0d809a4..c1d1b1a 100644 --- a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java +++ b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java @@ -16,10 +16,14 @@ */ package com.intellij.struts2.diagram; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.pom.Navigatable; import com.intellij.psi.PsiFile; import com.intellij.psi.PsiManager; +import com.intellij.psi.SmartPsiElementPointer; +import com.intellij.psi.xml.XmlElement; import com.intellij.psi.xml.XmlFile; import com.intellij.struts2.BasicLightHighlightingTestCase; import com.intellij.struts2.diagram.model.StrutsConfigDiagramModel; @@ -102,4 +106,37 @@ public void testBuildReturnsNullForNonStrutsXml() { () -> StrutsConfigDiagramModel.build(plainXml)).executeSynchronously(); assertNull("Should return null for non-Struts XML", model); } + + /** + * Regression: navigation pointers must resolve via {@code Application.runReadAction} + * (EDT-safe) instead of {@code ReadAction.nonBlocking().executeSynchronously()} + * which asserts background-thread usage. + */ + public void testNavigationPointerResolvesUnderSynchronousReadAction() { + createStrutsFileSet("struts-local-a.xml"); + + VirtualFile vfA = myFixture.findFileInTempDir("struts-local-a.xml"); + assertNotNull(vfA); + PsiFile psiA = PsiManager.getInstance(getProject()).findFile(vfA); + assertInstanceOf(psiA, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psiA)).executeSynchronously(); + assertNotNull(model); + + StrutsDiagramNode actionNode = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.ACTION) + .findFirst().orElse(null); + assertNotNull("Model should contain at least one ACTION node", actionNode); + + SmartPsiElementPointer pointer = actionNode.getNavigationPointer(); + assertNotNull("ACTION node should have a navigation pointer", pointer); + + Navigatable navigatable = ApplicationManager.getApplication().runReadAction( + (com.intellij.openapi.util.Computable) () -> { + XmlElement element = pointer.getElement(); + return element instanceof Navigatable ? (Navigatable) element : null; + }); + assertNotNull("Smart pointer should resolve to a Navigatable element", navigatable); + } } From 211efe24e156e647049c3cd074e987f01ba6594f Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 14:40:38 +0200 Subject: [PATCH 6/7] feat(diagram): harden MVP with explicit states and clearer fallbacks Unify the editor rebuild path so initial load and reset() always reflect the current model state instead of retaining stale content. Add LOADED/EMPTY/UNAVAILABLE states to the diagram component with centered placeholder messages for non-loaded states. Replace raw ??? placeholders with descriptive labels ((unresolved path), (unnamed), (unknown type)). Extend tests to cover empty files, null models, rebuild transitions, and label clarity. Made-with: Cursor --- .../fileEditor/Struts2DiagramFileEditor.java | 33 ++--- .../model/StrutsConfigDiagramModel.java | 9 +- .../StrutsDiagramPresentation.java | 4 +- .../diagram/ui/Struts2DiagramComponent.java | 52 +++++-- .../diagram/StrutsConfigDiagramModelTest.java | 131 +++++++++++++++++- src/test/testData/diagram/struts-empty.xml | 27 ++++ .../testData/diagram/struts-unresolved.xml | 37 +++++ 7 files changed, 256 insertions(+), 37 deletions(-) create mode 100644 src/test/testData/diagram/struts-empty.xml create mode 100644 src/test/testData/diagram/struts-unresolved.xml diff --git a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java index 50b4fe4..6836e58 100644 --- a/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java +++ b/src/main/java/com/intellij/struts2/diagram/fileEditor/Struts2DiagramFileEditor.java @@ -17,7 +17,6 @@ package com.intellij.struts2.diagram.fileEditor; import com.intellij.openapi.application.ReadAction; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; @@ -34,11 +33,14 @@ /** * Read-only file editor that hosts the lightweight Struts config diagram. + *

    + * Both initial creation and {@link #reset()} go through the same + * {@link #buildModel()} path so the component always reflects the + * current model state — including explicit empty and unavailable + * fallbacks instead of stale or blank content. */ public class Struts2DiagramFileEditor extends PerspectiveFileEditor { - private static final Logger LOG = Logger.getInstance(Struts2DiagramFileEditor.class); - private final XmlFile myXmlFile; private Struts2DiagramComponent myComponent; @@ -78,13 +80,7 @@ public void commit() { @Override public void reset() { - StrutsConfigDiagramModel model = ReadAction.nonBlocking(() -> StrutsConfigDiagramModel.build(myXmlFile)) - .executeSynchronously(); - if (model != null) { - getDiagramComponent().rebuild(model); - } else { - LOG.debug("reset() got null model for " + myXmlFile.getName() + ", keeping existing content"); - } + getDiagramComponent().rebuild(buildModel()); } @Override @@ -93,14 +89,19 @@ public String getName() { return "Diagram"; } + private @Nullable StrutsConfigDiagramModel buildModel() { + final StrutsConfigDiagramModel[] model = {null}; + ProgressManager.getInstance().runProcessWithProgressSynchronously( + () -> model[0] = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build(myXmlFile)) + .executeSynchronously(), + "Building Diagram", false, myXmlFile.getProject()); + return model[0]; + } + private Struts2DiagramComponent getDiagramComponent() { if (myComponent == null) { - final StrutsConfigDiagramModel[] model = {null}; - ProgressManager.getInstance().runProcessWithProgressSynchronously( - () -> model[0] = ReadAction.nonBlocking(() -> StrutsConfigDiagramModel.build(myXmlFile)) - .executeSynchronously(), - "Building Diagram", false, myXmlFile.getProject()); - myComponent = new Struts2DiagramComponent(model[0]); + myComponent = new Struts2DiagramComponent(buildModel()); } return myComponent; } diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java index b30044a..032498d 100644 --- a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java @@ -56,7 +56,8 @@ public final class StrutsConfigDiagramModel { private static final Logger LOG = Logger.getInstance(StrutsConfigDiagramModel.class); - private static final String UNKNOWN = "???"; + static final String UNRESOLVED_RESULT = "(unresolved path)"; + static final String UNNAMED = "(unnamed)"; private final List nodes = new ArrayList<>(); private final List edges = new ArrayList<>(); @@ -84,14 +85,14 @@ private StrutsConfigDiagramModel() {} StrutsConfigDiagramModel model = new StrutsConfigDiagramModel(); for (StrutsPackage strutsPackage : packages) { - String pkgName = Objects.toString(strutsPackage.getName().getStringValue(), UNKNOWN); + String pkgName = Objects.toString(strutsPackage.getName().getStringValue(), UNNAMED); StrutsDiagramNode pkgNode = createNode( StrutsDiagramNode.Kind.PACKAGE, pkgName, AllIcons.Nodes.Package, strutsPackage, pointerManager); model.nodes.add(pkgNode); for (Action action : strutsPackage.getActions()) { - String actionName = Objects.toString(action.getName().getStringValue(), UNKNOWN); + String actionName = Objects.toString(action.getName().getStringValue(), UNNAMED); StrutsDiagramNode actionNode = createNode( StrutsDiagramNode.Kind.ACTION, actionName, Struts2Icons.Action, action, pointerManager); @@ -100,7 +101,7 @@ private StrutsConfigDiagramModel() {} for (Result result : action.getResults()) { PathReference pathRef = result.getValue(); - String path = pathRef != null ? pathRef.getPath() : UNKNOWN; + String path = pathRef != null ? pathRef.getPath() : UNRESOLVED_RESULT; Icon resultIcon = resolveResultIcon(result); StrutsDiagramNode resultNode = createNode( StrutsDiagramNode.Kind.RESULT, path, resultIcon, diff --git a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java index 8703d1f..3992633 100644 --- a/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java +++ b/src/main/java/com/intellij/struts2/diagram/presentation/StrutsDiagramPresentation.java @@ -76,9 +76,9 @@ private StrutsDiagramPresentation() {} if (element instanceof Result result) { PathReference ref = result.getValue(); - String displayPath = ref != null ? ref.getPath() : "???"; + String displayPath = ref != null ? ref.getPath() : "(unresolved)"; ResultType resultType = result.getEffectiveResultType(); - String resultTypeValue = resultType != null ? resultType.getName().getStringValue() : "???"; + String resultTypeValue = resultType != null ? resultType.getName().getStringValue() : "(unknown type)"; return new HtmlTableBuilder() .addLine("Path", displayPath) .addLine("Type", resultTypeValue) diff --git a/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java b/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java index c2b0729..a0342cb 100644 --- a/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java +++ b/src/main/java/com/intellij/struts2/diagram/ui/Struts2DiagramComponent.java @@ -37,6 +37,9 @@ * Lightweight read-only Swing panel that renders a Struts config diagram with a * deterministic hierarchical layout: packages at left, actions in center, results at right. * Supports hover tooltips and click-to-navigate. + *

    + * When the model is {@code null} or empty, a centered placeholder message is rendered + * instead of a blank canvas. */ public final class Struts2DiagramComponent extends JPanel { @@ -48,15 +51,19 @@ public final class Struts2DiagramComponent extends JPanel { private static final int ICON_TEXT_GAP = 4; private static final int ARC = 8; + private static final String MSG_UNAVAILABLE = "Diagram is not available for this file"; + private static final String MSG_EMPTY = "No packages or actions found in this file"; + + public enum State { LOADED, EMPTY, UNAVAILABLE } + private final Map nodeBounds = new LinkedHashMap<>(); private final List edges = new ArrayList<>(); private @Nullable StrutsDiagramNode hoveredNode; + private @NotNull State state = State.UNAVAILABLE; public Struts2DiagramComponent(@Nullable StrutsConfigDiagramModel model) { setBackground(JBColor.background()); - if (model != null) { - layoutModel(model); - } + applyModel(model); MouseAdapter mouseHandler = new MouseAdapter() { @Override @@ -81,16 +88,30 @@ public void mouseClicked(MouseEvent e) { addMouseMotionListener(mouseHandler); } + public @NotNull State getState() { return state; } + public void rebuild(@Nullable StrutsConfigDiagramModel model) { nodeBounds.clear(); edges.clear(); - if (model != null) { - layoutModel(model); - } + hoveredNode = null; + applyModel(model); revalidate(); repaint(); } + private void applyModel(@Nullable StrutsConfigDiagramModel model) { + if (model == null) { + state = State.UNAVAILABLE; + return; + } + if (model.getNodes().isEmpty()) { + state = State.EMPTY; + return; + } + state = State.LOADED; + layoutModel(model); + } + private void layoutModel(@NotNull StrutsConfigDiagramModel model) { edges.addAll(model.getEdges()); @@ -144,13 +165,28 @@ protected void paintComponent(Graphics g) { g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); g2.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON); - paintEdges(g2); - paintNodes(g2); + if (state != State.LOADED) { + paintPlaceholder(g2); + } else { + paintEdges(g2); + paintNodes(g2); + } } finally { g2.dispose(); } } + private void paintPlaceholder(Graphics2D g2) { + String message = state == State.EMPTY ? MSG_EMPTY : MSG_UNAVAILABLE; + g2.setFont(JBUI.Fonts.label(14)); + g2.setColor(JBColor.namedColor("Editor.foreground", + new JBColor(new Color(0x888888), new Color(0x999999)))); + FontMetrics fm = g2.getFontMetrics(); + int x = (getWidth() - fm.stringWidth(message)) / 2; + int y = getHeight() / 2; + g2.drawString(message, Math.max(x, PADDING), Math.max(y, PADDING)); + } + private void paintEdges(Graphics2D g2) { g2.setStroke(new BasicStroke(1.2f)); for (StrutsDiagramEdge edge : edges) { diff --git a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java index c1d1b1a..50998b4 100644 --- a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java +++ b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java @@ -28,6 +28,7 @@ import com.intellij.struts2.BasicLightHighlightingTestCase; import com.intellij.struts2.diagram.model.StrutsConfigDiagramModel; import com.intellij.struts2.diagram.model.StrutsDiagramNode; +import com.intellij.struts2.diagram.ui.Struts2DiagramComponent; import org.jetbrains.annotations.NotNull; import java.util.List; @@ -35,8 +36,8 @@ import java.util.stream.Collectors; /** - * Tests that {@link StrutsConfigDiagramModel} builds a file-local snapshot, - * not a merged view of the entire file set. + * Tests for {@link StrutsConfigDiagramModel} covering file-local snapshot, + * fallback states, unresolved result labeling, and component state mapping. */ public class StrutsConfigDiagramModelTest extends BasicLightHighlightingTestCase { @@ -107,11 +108,6 @@ public void testBuildReturnsNullForNonStrutsXml() { assertNull("Should return null for non-Struts XML", model); } - /** - * Regression: navigation pointers must resolve via {@code Application.runReadAction} - * (EDT-safe) instead of {@code ReadAction.nonBlocking().executeSynchronously()} - * which asserts background-thread usage. - */ public void testNavigationPointerResolvesUnderSynchronousReadAction() { createStrutsFileSet("struts-local-a.xml"); @@ -139,4 +135,125 @@ public void testNavigationPointerResolvesUnderSynchronousReadAction() { }); assertNotNull("Smart pointer should resolve to a Navigatable element", navigatable); } + + // --- Fallback state tests --- + + public void testBuildReturnsEmptyModelForStrutsFileWithNoPackages() { + createStrutsFileSet("struts-empty.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-empty.xml"); + assertNotNull(vf); + + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull("Should return a model (not null) for a valid but empty Struts file", model); + assertTrue("Model should have no nodes", model.getNodes().isEmpty()); + assertTrue("Model should have no edges", model.getEdges().isEmpty()); + } + + public void testComponentStateUnavailableForNullModel() { + Struts2DiagramComponent component = new Struts2DiagramComponent(null); + assertEquals(Struts2DiagramComponent.State.UNAVAILABLE, component.getState()); + } + + public void testComponentStateEmptyForEmptyModel() { + createStrutsFileSet("struts-empty.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-empty.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + Struts2DiagramComponent component = new Struts2DiagramComponent(model); + assertEquals(Struts2DiagramComponent.State.EMPTY, component.getState()); + } + + public void testComponentStateLoadedForNormalModel() { + createStrutsFileSet("struts-local-a.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-local-a.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + Struts2DiagramComponent component = new Struts2DiagramComponent(model); + assertEquals(Struts2DiagramComponent.State.LOADED, component.getState()); + } + + public void testRebuildClearsStaleThenShowsFallback() { + createStrutsFileSet("struts-local-a.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-local-a.xml"); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + StrutsConfigDiagramModel loaded = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + + Struts2DiagramComponent component = new Struts2DiagramComponent(loaded); + assertEquals(Struts2DiagramComponent.State.LOADED, component.getState()); + + component.rebuild(null); + assertEquals("rebuild(null) should switch to UNAVAILABLE, not keep stale content", + Struts2DiagramComponent.State.UNAVAILABLE, component.getState()); + } + + // --- Unresolved result label tests --- + + public void testUnresolvedResultUsesDescriptiveLabel() { + createStrutsFileSet("struts-unresolved.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-unresolved.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List results = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.RESULT) + .collect(Collectors.toList()); + assertFalse("Should have result nodes", results.isEmpty()); + + for (StrutsDiagramNode result : results) { + assertFalse("Result node label should not be raw '???': " + result.getName(), + "???".equals(result.getName())); + } + } + + public void testUnresolvedResultTooltipDoesNotContainRawPlaceholders() { + createStrutsFileSet("struts-unresolved.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-unresolved.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List results = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.RESULT) + .collect(Collectors.toList()); + + for (StrutsDiagramNode result : results) { + String tooltip = result.getTooltipHtml(); + if (tooltip != null) { + assertFalse("Tooltip should not contain raw '???' for path: " + tooltip, + tooltip.contains(">???<")); + } + } + } } diff --git a/src/test/testData/diagram/struts-empty.xml b/src/test/testData/diagram/struts-empty.xml new file mode 100644 index 0000000..9ac70bf --- /dev/null +++ b/src/test/testData/diagram/struts-empty.xml @@ -0,0 +1,27 @@ + + + + + + + + diff --git a/src/test/testData/diagram/struts-unresolved.xml b/src/test/testData/diagram/struts-unresolved.xml new file mode 100644 index 0000000..dcfaeff --- /dev/null +++ b/src/test/testData/diagram/struts-unresolved.xml @@ -0,0 +1,37 @@ + + + + + + + + + + + + /nonexistent/missing.jsp + /also/missing.jsp + + + + + From e64675728f03984ad01cb254b09e0817e4ec6384 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 5 Apr 2026 14:58:45 +0200 Subject: [PATCH 7/7] fix(diagram): use stable element identity for diagram nodes Replace display-based equals/hashCode (kind+name) in StrutsDiagramNode with a stable ID derived from the backing XML element's text offset. This prevents node collisions when duplicate action names or result paths exist across packages. Add focused tests for duplicate-name regression, edge topology and labels, editor lifecycle (creation and reset), and DTD validator classification outcomes. Made-with: Cursor --- .../model/StrutsConfigDiagramModel.java | 12 +- .../diagram/model/StrutsDiagramNode.java | 18 ++- .../Struts2DiagramFileEditorProviderTest.java | 36 ++++- .../diagram/StrutsConfigDiagramModelTest.java | 143 ++++++++++++++++++ .../inspection/StrutsDtdValidatorTest.java | 108 +++++++++---- .../dom/struts/StrutsHighlightingTest.java | 1 + .../diagram/struts-duplicate-names.xml | 41 +++++ 7 files changed, 322 insertions(+), 37 deletions(-) create mode 100644 src/test/testData/diagram/struts-duplicate-names.xml diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java index 032498d..8cca983 100644 --- a/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsConfigDiagramModel.java @@ -161,7 +161,17 @@ private StrutsConfigDiagramModel() {} String tooltipHtml = StrutsDiagramPresentation.computeTooltipHtml(domElement); SmartPsiElementPointer navPointer = createNavigationPointer(domElement, pointerManager); - return new StrutsDiagramNode(kind, name, icon, tooltipHtml, navPointer); + String id = buildNodeId(kind, domElement); + return new StrutsDiagramNode(id, kind, name, icon, tooltipHtml, navPointer); + } + + private static @NotNull String buildNodeId(@NotNull StrutsDiagramNode.Kind kind, + @NotNull DomElement domElement) { + XmlElement xml = domElement.getXmlElement(); + if (xml != null) { + return kind.name() + "@" + xml.getTextOffset(); + } + return kind.name() + "@" + System.identityHashCode(domElement); } private static @Nullable SmartPsiElementPointer createNavigationPointer( diff --git a/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java index 848c8aa..c88e54f 100644 --- a/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java +++ b/src/main/java/com/intellij/struts2/diagram/model/StrutsDiagramNode.java @@ -27,6 +27,12 @@ /** * Toolkit-neutral node representing a Struts config element (package, action, or result). *

    + * Each node carries a stable {@link #id} captured during model build that uniquely + * identifies it even when two elements share the same display {@link #name} (e.g. + * duplicate action names across packages, or identical result paths). The UI + * renderer uses node identity for layout maps and edge lookup, so uniqueness here + * is critical. + *

    * UI-safe fields ({@link #getTooltipHtml()}, {@link #getNavigationPointer()}, {@link #getIcon()}) * are precomputed during snapshot creation under a read action so that Swing event handlers * on the EDT never need to touch PSI/DOM directly. @@ -39,17 +45,20 @@ public final class StrutsDiagramNode { public enum Kind { PACKAGE, ACTION, RESULT } + private final @NotNull String id; private final @NotNull Kind kind; private final @NotNull String name; private final @Nullable Icon icon; private final @Nullable String tooltipHtml; private final @Nullable SmartPsiElementPointer navigationPointer; - public StrutsDiagramNode(@NotNull Kind kind, + public StrutsDiagramNode(@NotNull String id, + @NotNull Kind kind, @NotNull String name, @Nullable Icon icon, @Nullable String tooltipHtml, @Nullable SmartPsiElementPointer navigationPointer) { + this.id = id; this.kind = kind; this.name = name; this.icon = icon; @@ -57,6 +66,7 @@ public StrutsDiagramNode(@NotNull Kind kind, this.navigationPointer = navigationPointer; } + public @NotNull String getId() { return id; } public @NotNull Kind getKind() { return kind; } public @NotNull String getName() { return name; } public @Nullable Icon getIcon() { return icon; } @@ -67,16 +77,16 @@ public StrutsDiagramNode(@NotNull Kind kind, public boolean equals(Object o) { if (this == o) return true; if (!(o instanceof StrutsDiagramNode that)) return false; - return kind == that.kind && name.equals(that.name); + return id.equals(that.id); } @Override public int hashCode() { - return Objects.hash(kind, name); + return Objects.hash(id); } @Override public String toString() { - return kind + ":" + name; + return kind + ":" + name + "(" + id + ")"; } } diff --git a/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java b/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java index dcfced1..664f590 100644 --- a/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java +++ b/src/test/java/com/intellij/struts2/diagram/Struts2DiagramFileEditorProviderTest.java @@ -16,13 +16,17 @@ */ package com.intellij.struts2.diagram; +import com.intellij.openapi.fileEditor.FileEditor; +import com.intellij.openapi.util.Disposer; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.struts2.BasicLightHighlightingTestCase; +import com.intellij.struts2.diagram.fileEditor.Struts2DiagramFileEditor; import com.intellij.struts2.diagram.fileEditor.Struts2DiagramFileEditorProvider; import org.jetbrains.annotations.NotNull; /** - * Tests for {@link Struts2DiagramFileEditorProvider}. + * Tests for {@link Struts2DiagramFileEditorProvider} covering both acceptance + * gating and basic editor lifecycle (creation, name, reset). */ public class Struts2DiagramFileEditorProviderTest extends BasicLightHighlightingTestCase { @@ -58,4 +62,34 @@ public void testRejectsJavaFile() { assertFalse("Diagram provider should reject non-XML files", myProvider.accept(getProject(), file)); } + + // --- Editor lifecycle tests --- + + public void testEditorCreationProducesValidDiagramEditor() { + createStrutsFileSet("struts-diagram.xml"); + VirtualFile file = myFixture.findFileInTempDir("struts-diagram.xml"); + assertNotNull(file); + + FileEditor editor = myProvider.createEditor(getProject(), file); + try { + assertInstanceOf(editor, Struts2DiagramFileEditor.class); + assertEquals("Diagram", editor.getName()); + } finally { + Disposer.dispose(editor); + } + } + + public void testResetDoesNotThrow() { + createStrutsFileSet("struts-diagram.xml"); + VirtualFile file = myFixture.findFileInTempDir("struts-diagram.xml"); + assertNotNull(file); + + Struts2DiagramFileEditor editor = + (Struts2DiagramFileEditor) myProvider.createEditor(getProject(), file); + try { + editor.reset(); + } finally { + Disposer.dispose(editor); + } + } } diff --git a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java index 50998b4..da43a4d 100644 --- a/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java +++ b/src/test/java/com/intellij/struts2/diagram/StrutsConfigDiagramModelTest.java @@ -27,6 +27,7 @@ import com.intellij.psi.xml.XmlFile; import com.intellij.struts2.BasicLightHighlightingTestCase; import com.intellij.struts2.diagram.model.StrutsConfigDiagramModel; +import com.intellij.struts2.diagram.model.StrutsDiagramEdge; import com.intellij.struts2.diagram.model.StrutsDiagramNode; import com.intellij.struts2.diagram.ui.Struts2DiagramComponent; import org.jetbrains.annotations.NotNull; @@ -256,4 +257,146 @@ public void testUnresolvedResultTooltipDoesNotContainRawPlaceholders() { } } } + + // --- Duplicate name and identity tests --- + + public void testDuplicateActionNamesAcrossPackagesProduceDistinctNodes() { + createStrutsFileSet("struts-duplicate-names.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-duplicate-names.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List actions = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.ACTION) + .collect(Collectors.toList()); + assertEquals("Both 'index' actions should be present as separate nodes", 2, actions.size()); + + assertEquals("Both share the same display name", actions.get(0).getName(), actions.get(1).getName()); + assertFalse("Must have different IDs", actions.get(0).getId().equals(actions.get(1).getId())); + assertFalse("Must not be equal() to each other", actions.get(0).equals(actions.get(1))); + } + + public void testDuplicateResultPathsProduceDistinctNodes() { + createStrutsFileSet("struts-duplicate-names.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-duplicate-names.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List results = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.RESULT) + .collect(Collectors.toList()); + assertEquals("Should have 3 result nodes total", 3, results.size()); + + long distinctIds = results.stream().map(StrutsDiagramNode::getId).distinct().count(); + assertEquals("All result nodes must have distinct IDs", 3, distinctIds); + } + + // --- Edge structure tests --- + + public void testEdgesConnectPackagesToActionsAndActionsToResults() { + createStrutsFileSet("struts-local-a.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-local-a.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List edges = model.getEdges(); + assertEquals("struts-local-a.xml has 2 package->action + 2 action->result edges", 4, edges.size()); + + long pkgToAction = edges.stream() + .filter(e -> e.getSource().getKind() == StrutsDiagramNode.Kind.PACKAGE + && e.getTarget().getKind() == StrutsDiagramNode.Kind.ACTION) + .count(); + assertEquals("Two package->action edges", 2, pkgToAction); + + long actionToResult = edges.stream() + .filter(e -> e.getSource().getKind() == StrutsDiagramNode.Kind.ACTION + && e.getTarget().getKind() == StrutsDiagramNode.Kind.RESULT) + .count(); + assertEquals("Two action->result edges", 2, actionToResult); + } + + public void testResultEdgeLabelsReflectResultNames() { + createStrutsFileSet("struts-local-a.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-local-a.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List resultEdges = model.getEdges().stream() + .filter(e -> e.getTarget().getKind() == StrutsDiagramNode.Kind.RESULT) + .collect(Collectors.toList()); + assertEquals(2, resultEdges.size()); + + Set labels = resultEdges.stream() + .map(StrutsDiagramEdge::getLabel) + .collect(Collectors.toSet()); + assertTrue("Should have default 'success' label, got: " + labels, labels.contains("success")); + assertTrue("Should have explicit 'input' label, got: " + labels, labels.contains("input")); + } + + public void testEdgesInDuplicateNameFileAreCorrectlyWired() { + createStrutsFileSet("struts-duplicate-names.xml"); + + VirtualFile vf = myFixture.findFileInTempDir("struts-duplicate-names.xml"); + assertNotNull(vf); + PsiFile psi = PsiManager.getInstance(getProject()).findFile(vf); + assertInstanceOf(psi, XmlFile.class); + + StrutsConfigDiagramModel model = ReadAction.nonBlocking( + () -> StrutsConfigDiagramModel.build((XmlFile) psi)).executeSynchronously(); + assertNotNull(model); + + List packages = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.PACKAGE) + .collect(Collectors.toList()); + assertEquals("Two packages", 2, packages.size()); + + for (StrutsDiagramNode pkg : packages) { + long outgoing = model.getEdges().stream() + .filter(e -> e.getSource().equals(pkg)) + .count(); + assertEquals("Each package should have exactly one outgoing edge to its 'index' action", 1, outgoing); + } + + List actions = model.getNodes().stream() + .filter(n -> n.getKind() == StrutsDiagramNode.Kind.ACTION) + .collect(Collectors.toList()); + StrutsDiagramNode adminAction = actions.get(0); + StrutsDiagramNode publicAction = actions.get(1); + + long adminResults = model.getEdges().stream() + .filter(e -> e.getSource().equals(adminAction) + && e.getTarget().getKind() == StrutsDiagramNode.Kind.RESULT) + .count(); + assertEquals("admin/index has 1 result", 1, adminResults); + + long publicResults = model.getEdges().stream() + .filter(e -> e.getSource().equals(publicAction) + && e.getTarget().getKind() == StrutsDiagramNode.Kind.RESULT) + .count(); + assertEquals("public/index has 2 results", 2, publicResults); + } } diff --git a/src/test/java/com/intellij/struts2/dom/inspection/StrutsDtdValidatorTest.java b/src/test/java/com/intellij/struts2/dom/inspection/StrutsDtdValidatorTest.java index 6b9921d..8b542f8 100644 --- a/src/test/java/com/intellij/struts2/dom/inspection/StrutsDtdValidatorTest.java +++ b/src/test/java/com/intellij/struts2/dom/inspection/StrutsDtdValidatorTest.java @@ -16,59 +16,105 @@ */ package com.intellij.struts2.dom.inspection; -import com.intellij.psi.PsiFile; import com.intellij.psi.xml.XmlFile; import com.intellij.testFramework.fixtures.BasePlatformTestCase; +/** + * Unit tests for {@link StrutsDtdValidator} proving that the three DTD + * validation outcomes (OK, HTTP_INSTEAD_OF_HTTPS, UNRECOGNIZED) are + * correctly classified. + */ public class StrutsDtdValidatorTest extends BasePlatformTestCase { - public void testHttpUriDetected() { - XmlFile file = createStrutsXmlWithDoctype("http://struts.apache.org/dtds/struts-6.0.dtd"); - assertEquals(StrutsDtdValidator.Result.HTTP_INSTEAD_OF_HTTPS, StrutsDtdValidator.validate(file)); + public void testValidHttpsDtdReturnsOk() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + + """); + assertEquals(StrutsDtdValidator.Result.OK, StrutsDtdValidator.validate(file)); } - public void testHttpsUriOk() { - XmlFile file = createStrutsXmlWithDoctype("https://struts.apache.org/dtds/struts-6.0.dtd"); + public void testValidOldHttpDtdReturnsOk() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + + """); assertEquals(StrutsDtdValidator.Result.OK, StrutsDtdValidator.validate(file)); } - public void testOldHttpUriOk() { - XmlFile file = createStrutsXmlWithDoctype("http://struts.apache.org/dtds/struts-2.0.dtd"); - assertEquals(StrutsDtdValidator.Result.OK, StrutsDtdValidator.validate(file)); + public void testHttpInsteadOfHttpsForNewDtdReturnsWarning() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + + """); + assertEquals(StrutsDtdValidator.Result.HTTP_INSTEAD_OF_HTTPS, StrutsDtdValidator.validate(file)); + } + + public void testHttpInsteadOfHttpsForStrutsLikeDtdReturnsWarning() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + + """); + assertEquals(StrutsDtdValidator.Result.HTTP_INSTEAD_OF_HTTPS, StrutsDtdValidator.validate(file)); } - public void testUnrecognizedUri() { - XmlFile file = createStrutsXmlWithDoctype("http://example.com/bogus.dtd"); + public void testUnrecognizedDtdReturnsUnrecognized() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + + """); assertEquals(StrutsDtdValidator.Result.UNRECOGNIZED, StrutsDtdValidator.validate(file)); } - public void testNoDoctype() { - PsiFile psiFile = myFixture.configureByText("struts.xml", ""); - assertEquals(StrutsDtdValidator.Result.OK, StrutsDtdValidator.validate((XmlFile) psiFile)); + public void testNoDoctypeReturnsOk() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + """); + assertEquals(StrutsDtdValidator.Result.OK, StrutsDtdValidator.validate(file)); } - public void testSuggestedUri() { + public void testSuggestedUriReplacesHttpWithHttps() { assertEquals("https://struts.apache.org/dtds/struts-6.0.dtd", StrutsDtdValidator.suggestedUri("http://struts.apache.org/dtds/struts-6.0.dtd")); } - public void testHttp25UriDetected() { - XmlFile file = createStrutsXmlWithDoctype("http://struts.apache.org/dtds/struts-2.5.dtd"); - assertEquals(StrutsDtdValidator.Result.HTTP_INSTEAD_OF_HTTPS, StrutsDtdValidator.validate(file)); + public void testExtractSystemIdReturnsNullForNoDoctype() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", ""); + assertNull(StrutsDtdValidator.extractSystemId(file)); } - public void testHttps25UriOk() { - XmlFile file = createStrutsXmlWithDoctype("https://struts.apache.org/dtds/struts-2.5.dtd"); - assertEquals(StrutsDtdValidator.Result.OK, StrutsDtdValidator.validate(file)); - } - - private XmlFile createStrutsXmlWithDoctype(String systemUri) { - String content = "\n" + - "\n" + - ""; - PsiFile psiFile = myFixture.configureByText("struts.xml", content); - return (XmlFile) psiFile; + public void testExtractSystemIdReturnsDtdUri() { + XmlFile file = (XmlFile) myFixture.configureByText("struts.xml", + """ + + + + """); + assertEquals("https://struts.apache.org/dtds/struts-6.0.dtd", + StrutsDtdValidator.extractSystemId(file)); } } diff --git a/src/test/java/com/intellij/struts2/dom/struts/StrutsHighlightingTest.java b/src/test/java/com/intellij/struts2/dom/struts/StrutsHighlightingTest.java index 1c15bbd..b9ea5b9 100644 --- a/src/test/java/com/intellij/struts2/dom/struts/StrutsHighlightingTest.java +++ b/src/test/java/com/intellij/struts2/dom/struts/StrutsHighlightingTest.java @@ -85,4 +85,5 @@ public void testStrutsWithErrorsNotInFilesetNoHighlighting() { public void testDtdHttpsNoWarning() { performHighlightingTest("struts-dtd-https.xml"); } + } \ No newline at end of file diff --git a/src/test/testData/diagram/struts-duplicate-names.xml b/src/test/testData/diagram/struts-duplicate-names.xml new file mode 100644 index 0000000..9e169fa --- /dev/null +++ b/src/test/testData/diagram/struts-duplicate-names.xml @@ -0,0 +1,41 @@ + + + + + + + + + + + /admin/index.jsp + + + + + + /public/index.jsp + /public/form.jsp + + + +