null when starting at root
* @param item Name of item at end of path
* @return Full path name to item
+ * @throws AlarmTreePathException When getting an illegal item string with leading slashes
*/
- public static String makePath(final String path, String item)
- {
+ public static String makePath(final String path, String item) throws AlarmTreePathException {
+ // Validate item: forbid leading slashes except exactly one (legacy compatibility)
+ if (item != null && item.startsWith(PATH_SEP)) {
+ // If there's more than one leading slash, it's invalid
+ if (item.length() > 1 && item.charAt(1) == PATH_SEP.charAt(0)) {
+ throw new AlarmTreePathException(
+ "Item must not have leading slashes: '" + item + "'"
+ );
+ }
+ // For legacy support (existing tests), strip exactly one leading slash
+ item = item.substring(1);
+ }
+
final StringBuilder result = new StringBuilder();
if (path != null)
{
if (! isPath(path))
result.append(PATH_SEP);
- // Skip path it it's only '/'
+ // Skip path if it's only '/'
if (!PATH_SEP.equals(path))
result.append(path);
}
result.append(PATH_SEP);
if (item != null && !item.isEmpty())
{
- // If item already starts with '/', skip it
- if (item.startsWith(PATH_SEP))
- item = item.substring(1);
// Escape any path-seps inside item with backslashes
result.append(item.replace(PATH_SEP, "\\/"));
}
@@ -112,8 +122,9 @@ public static String getName(final String path)
* @param path Original path
* @param modifier Path modifier: "segments/to/add", "/absolute/new/path", ".."
* @return Path based on pwd and modifier
+ * @throws AlarmTreePathException When a segment contains leading slashes.
*/
- public static String update(String path, String modifier)
+ public static String update(String path, String modifier) throws AlarmTreePathException
{
if (modifier == null || modifier.isEmpty())
return makePath(null, path);
@@ -137,4 +148,18 @@ public static String update(String path, String modifier)
}
}
}
+
+ public static boolean pathsAreEquivalent(String a, String b) {
+ var elementsA = splitPath(a);
+ var elementsB = splitPath(b);
+ if (elementsA.length != elementsB.length) {
+ return false;
+ }
+ for (int i = 0; i < elementsA.length; i++) {
+ if (!Objects.equals(elementsA[i], elementsB[i])) {
+ return false;
+ }
+ }
+ return true;
+ }
}
diff --git a/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/AlarmTreePathException.java b/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/AlarmTreePathException.java
new file mode 100644
index 0000000000..06f5dc7247
--- /dev/null
+++ b/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/AlarmTreePathException.java
@@ -0,0 +1,15 @@
+package org.phoebus.applications.alarm.model;
+
+/**
+ * Exception thrown when attempting to construct an invalid alarm tree path.
+ *
+ * Paths or path elements that start with more than one leading slash are not allowed.
+ * A single leading slash is permitted for backward compatibility and for
+ * representing absolute paths, but multiple leading slashes indicate an
+ * invalid or ambiguous path specification.
+ */
+public class AlarmTreePathException extends IllegalArgumentException {
+ public AlarmTreePathException(String message) {
+ super(message);
+ }
+}
diff --git a/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/xml/XmlModelReader.java b/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/xml/XmlModelReader.java
index fded541f13..74e840cd28 100644
--- a/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/xml/XmlModelReader.java
+++ b/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/xml/XmlModelReader.java
@@ -24,6 +24,7 @@
import org.phoebus.applications.alarm.client.AlarmClientLeaf;
import org.phoebus.applications.alarm.client.AlarmClientNode;
+import org.phoebus.applications.alarm.model.AlarmTreePathException;
import org.phoebus.applications.alarm.model.TitleDetail;
import org.phoebus.applications.alarm.model.TitleDetailDelay;
import org.phoebus.framework.persistence.XMLUtil;
@@ -118,15 +119,8 @@ private void buildModel(final Document doc) throws Exception
// Create the root of the model. Parent is null and name must be config.
root = new AlarmClientNode(null, root_node.getAttribute(TAG_NAME));
- // First add PVs at this level, ..
- for (final Element child : XMLUtil.getChildElements(root_node, TAG_PV))
- processPV(root /* parent */, child);
-
- // .. when sub-components which again have PVs.
- // This way, duplicate PVs will be detected and ignored at a nested level,
- // keeping those toward the root
- for (final Node child : XMLUtil.getChildElements(root_node, TAG_COMPONENT))
- processComponent(root /* parent */, child);
+ // Recursively process children
+ processChildren(root, root_node);
}
private void processComponent(final AlarmClientNode parent, final Node node) throws Exception
@@ -162,12 +156,34 @@ private void processComponent(final AlarmClientNode parent, final Node node) thr
// This does not refer to XML attributes but instead to the attributes of a model component node.
processCompAttr(component, node);
- // First add PVs at this level, then sub-components
+ // Recursively process children
+ processChildren(component, node);
+ }
+
+ private void processChildren(AlarmClientNode component, Node node) throws Exception {
+ // First add PVs at this level
for (final Element child : XMLUtil.getChildElements(node, TAG_PV))
- processPV(component/* parent */, child);
+ try {
+ processPV(component/* parent */, child);
+ } catch (AlarmTreePathException e) {
+ logger.log(Level.WARNING,
+ "Ignoring malformed PV "
+ + component.getPathName() + "/" + child.getAttribute(TAG_NAME) + ".\n"
+ + "Cause: " + e.getMessage());
+ }
+ // then subcomponents, which again have PVs.
+ // This way, duplicate PVs will be detected and ignored at a nested level,
+ // keeping those toward the root
for (final Element child : XMLUtil.getChildElements(node, TAG_COMPONENT))
- processComponent(component /* parent */, child);
+ try {
+ processComponent(component /* parent */, child);
+ } catch (AlarmTreePathException e) {
+ logger.log(Level.WARNING,
+ "Ignoring malformed component "
+ + component.getPathName() + "/" + child.getAttribute(TAG_NAME) + ".\n"
+ + "Cause: " + e.getMessage());
+ }
}
private void processCompAttr(final AlarmClientNode component, final Node node) throws Exception
diff --git a/app/alarm/model/src/test/java/org/phoebus/applications/alarm/AlarmTreePathUnitTest.java b/app/alarm/model/src/test/java/org/phoebus/applications/alarm/AlarmTreePathUnitTest.java
new file mode 100644
index 0000000000..d374b45530
--- /dev/null
+++ b/app/alarm/model/src/test/java/org/phoebus/applications/alarm/AlarmTreePathUnitTest.java
@@ -0,0 +1,156 @@
+/*******************************************************************************
+ * Copyright (c) 2010 Oak Ridge National Laboratory.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ ******************************************************************************/
+package org.phoebus.applications.alarm;
+
+import org.junit.jupiter.api.Test;
+import org.phoebus.applications.alarm.model.AlarmTreePath;
+import org.phoebus.applications.alarm.model.AlarmTreePathException;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/** JUnit test of AlarmTreePath
+ * @author Kay Kasemir
+ */
+@SuppressWarnings("nls")
+public class AlarmTreePathUnitTest
+{
+ @Test
+ public void testPathCheck()
+ {
+ assertThat(AlarmTreePath.makePath(null, "root"), equalTo("/root"));
+ assertThat(AlarmTreePath.makePath(null, "/root"), equalTo("/root"));
+ assertThat(AlarmTreePath.makePath("/", "/root"), equalTo("/root"));
+ assertThat(AlarmTreePath.isPath("/path/to/some/pv"), equalTo(true));
+ assertThat(AlarmTreePath.getName("/path/to/some/pv"), equalTo("pv"));
+ assertThat(AlarmTreePath.isPath("some_pv"), equalTo(false));
+ assertThat(AlarmTreePath.isPath("sim:\\/\\/sine"), equalTo(false));
+ assertThat(AlarmTreePath.getName("sim:\\/\\/sine"), equalTo("sim://sine"));
+ }
+
+ @Test
+ public void testInvalidLeadingSlashes() {
+ // Two or more leading slashes must throw
+ assertThrows(AlarmTreePathException.class, () -> AlarmTreePath.makePath(null, "//pv"));
+ assertThrows(AlarmTreePathException.class, () -> AlarmTreePath.makePath(null, "///pv"));
+
+ // Also when combined with parent paths
+ assertThrows(AlarmTreePathException.class, () -> AlarmTreePath.makePath("/", "//pv"));
+ assertThrows(AlarmTreePathException.class, () -> AlarmTreePath.makePath("/path", "//child"));
+ }
+
+ @Test
+ public void testMakePath()
+ {
+ // Split
+ String[] path = AlarmTreePath.splitPath("/path/to/some/pv");
+ assertThat(path.length, equalTo(4));
+ assertThat(path[1], equalTo("to"));
+
+ path = AlarmTreePath.splitPath("///path//to///some//pv");
+ assertThat(path.length, equalTo(4));
+ assertThat(path[1], equalTo("to"));
+
+
+ // Sub-path
+ final String new_path = AlarmTreePath.makePath(path, 2);
+ assertThat(new_path, equalTo("/path/to"));
+
+ // New PV
+ assertThat(AlarmTreePath.makePath(new_path, "another"),
+ equalTo("/path/to/another"));
+ }
+
+ @Test
+ public void testSpaces()
+ {
+ String path = AlarmTreePath.makePath("the path", "to");
+ assertThat(path, equalTo("/the path/to"));
+
+ path = AlarmTreePath.makePath(path, "an item");
+ assertThat(path, equalTo("/the path/to/an item"));
+
+ path = AlarmTreePath.makePath(path, "with / in it");
+ assertThat(path, equalTo("/the path/to/an item/with \\/ in it"));
+
+ // Split
+ final String[] items = AlarmTreePath.splitPath(path);
+ assertThat(items.length, equalTo(4));
+ assertThat(items[0], equalTo("the path"));
+ assertThat(items[1], equalTo("to"));
+ assertThat(items[2], equalTo("an item"));
+ assertThat(items[3], equalTo("with / in it"));
+
+ // Re-assemble
+ path = AlarmTreePath.makePath(items, items.length);
+ assertThat(path, equalTo("/the path/to/an item/with \\/ in it"));
+ }
+
+ @Test
+ public void testSpecialChars()
+ {
+ String path = AlarmTreePath.makePath("path", "to");
+ assertThat(path, equalTo("/path/to"));
+
+ // First element already contains '/'
+ path = AlarmTreePath.makePath("/path", "to");
+ assertThat(path, equalTo("/path/to"));
+
+ path = AlarmTreePath.makePath(path, "sim://sine");
+ // String is really "/path/to/sim:\/\/sine",
+ // but to get the '\' into the string,
+ // it itself needs to be escaped
+ assertThat(path, equalTo("/path/to/sim:\\/\\/sine"));
+
+ // Split
+ final String[] items = AlarmTreePath.splitPath(path);
+ assertThat(items.length, equalTo(3));
+ assertThat(items[0], equalTo("path"));
+ assertThat(items[1], equalTo("to"));
+ assertThat(items[2], equalTo("sim://sine"));
+
+ // Re-assemble
+ path = AlarmTreePath.makePath(items, items.length);
+ assertThat(path, equalTo("/path/to/sim:\\/\\/sine"));
+ }
+
+ @Test
+ public void testPathUpdate()
+ {
+ String path = AlarmTreePath.makePath("path", "to");
+ assertThat(path, equalTo("/path/to"));
+
+ path = AlarmTreePath.update(path, "sub");
+ assertThat(path, equalTo("/path/to/sub"));
+
+ path = AlarmTreePath.update(path, "..");
+ assertThat(path, equalTo("/path/to"));
+
+ path = AlarmTreePath.update(path, "/new/path");
+ assertThat(path, equalTo("/new/path"));
+
+ path = AlarmTreePath.update(null, "/path");
+ assertThat(path, equalTo("/path"));
+
+ path = AlarmTreePath.update(null, null);
+ assertThat(path, equalTo("/"));
+
+ path = AlarmTreePath.update("/", "..");
+ assertThat(path, equalTo("/"));
+
+ path = AlarmTreePath.update("/", "path");
+ assertThat(path, equalTo("/path"));
+
+ path = AlarmTreePath.update("/", "path/to/sub");
+ assertThat(path, equalTo("/path/to/sub"));
+
+ path = AlarmTreePath.update("/", "path/to\\/sub");
+ assertThat(path, equalTo("/path/to\\/sub"));
+ }
+}
\ No newline at end of file
diff --git a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AddComponentAction.java b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AddComponentAction.java
index 34dd990263..6493b9ab9b 100644
--- a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AddComponentAction.java
+++ b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AddComponentAction.java
@@ -174,7 +174,13 @@ public AddComponentAction(final Node node, final AlarmClient model, final AlarmT
// Check for duplicate PV, anywhere in alarm tree
final String existing = findPV(root, pv);
if (existing == null)
- model.addPV(parent.getPathName(), pv);
+ try {
+ model.addPV(parent.getPathName(), pv);
+ } catch (Exception e) {
+ ExceptionDetailsErrorDialog.openError(Messages.error,
+ Messages.addComponentFailed,
+ e);
+ }
else
{
Platform.runLater(() ->
diff --git a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/DuplicatePVAction.java b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/DuplicatePVAction.java
index 5fff9d078a..d93214df08 100644
--- a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/DuplicatePVAction.java
+++ b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/DuplicatePVAction.java
@@ -63,9 +63,9 @@ public DuplicatePVAction(final Node node, final AlarmClient model, final AlarmCl
template.setActions(original.getActions());
// Request adding new PV
- final String new_path = AlarmTreePath.makePath(original.getParent().getPathName(), new_name);
JobManager.schedule(getText(), monitor -> {
try {
+ final String new_path = AlarmTreePath.makePath(original.getParent().getPathName(), new_name);
model.sendItemConfigurationUpdate(new_path, template);
} catch (Exception e) {
Logger.getLogger(DuplicatePVAction.class.getName()).log(Level.WARNING, "Failed to send item configuration", e);
diff --git a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/MoveTreeItemAction.java b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/MoveTreeItemAction.java
index 69f99a68c8..ff689a02bd 100644
--- a/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/MoveTreeItemAction.java
+++ b/app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/MoveTreeItemAction.java
@@ -10,6 +10,7 @@
import org.phoebus.applications.alarm.AlarmSystem;
import org.phoebus.applications.alarm.client.AlarmClient;
import org.phoebus.applications.alarm.model.AlarmTreeItem;
+import org.phoebus.applications.alarm.model.AlarmTreePath;
import org.phoebus.applications.alarm.ui.Messages;
import org.phoebus.framework.jobs.JobManager;
import org.phoebus.ui.dialog.ExceptionDetailsErrorDialog;
@@ -54,6 +55,13 @@ public MoveTreeItemAction(TreeView