Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,8 @@ private AlarmTreeItem<?> findOrCreateNode(final String path, final boolean is_le
* @param path_name to parent Root or parent component under which to add the component
* @param new_name Name of the new component
*/
public void addComponent(final String path_name, final String new_name) {
try {
sendNewItemInfo(path_name, new_name, new AlarmClientNode(null, new_name));
} catch (final Exception ex) {
logger.log(Level.WARNING, "Cannot add component " + new_name + " to " + path_name, ex);
}
public void addComponent(final String path_name, final String new_name) throws Exception {
sendNewItemInfo(path_name, new_name, new AlarmClientNode(null, new_name));
}

/**
Expand All @@ -498,12 +494,8 @@ public void addComponent(final String path_name, final String new_name) {
* @param path_name to parent Root or parent component under which to add the component
* @param new_name Name of the new component
*/
public void addPV(final String path_name, final String new_name) {
try {
sendNewItemInfo(path_name, new_name, new AlarmClientLeaf(null, new_name));
} catch (final Exception ex) {
logger.log(Level.WARNING, "Cannot add pv " + new_name + " to " + path_name, ex);
}
public void addPV(final String path_name, final String new_name) throws Exception {
sendNewItemInfo(path_name, new_name, new AlarmClientLeaf(null, new_name));
}

private void sendNewItemInfo(String path_name, final String new_name, final AlarmTreeItem<?> content) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/** Helper for handling the path names of alarm tree elements.
* Path looks like "/root/area/system/subsystem/pv_name".
Expand All @@ -32,24 +33,33 @@ public static boolean isPath(final String path)
* @param path Parent path or <code>null</code> 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, "\\/"));
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.phoebus.applications.alarm.model;

/**
* Exception thrown when attempting to construct an invalid alarm tree path.
* <p>
* 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(() ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,6 +55,13 @@ public MoveTreeItemAction(TreeView<AlarmTreeItem<?>> node,
prompt = "Invalid path. Try again or cancel";
}

// The move is done by copying the node from the old path to the new path,
// and then deleting the item at the old path.
// Without this check, entering the old path would result in just deleting the item.
if (AlarmTreePath.pathsAreEquivalent(path, item.getPathName())) {
return;
}

// Tree view keeps the selection indices, which will point to wrong content
// after those items have been removed.
if (node instanceof TreeView<?>)
Expand Down
Loading