Skip to content

Commit

Permalink
Merge pull request #212 from pdowler/master
Browse files Browse the repository at this point in the history 
bug fix and test for immutable and ilvalid node props
  • Loading branch information
@pdowler
pdowler authored Jan 16, 2024
2 parents f17553d + fe06f93 commit f6ab7e1
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cadc-test-vos/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ sourceCompatibility = 1.8

group = 'org.opencadc'

version = '2.1.3'
version = '2.1.4'

description = 'OpenCADC VOSpace test library'
def git_url = 'https://github.com/opencadc/vos'
Expand Down
66 changes: 64 additions & 2 deletions cadc-test-vos/src/main/java/org/opencadc/conformance/vos/NodesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
******************* CANADIAN ASTRONOMY DATA CENTRE *******************
************** CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES **************
*
* (c) 2023. (c) 2023.
* (c) 2024. (c) 2024.
* Government of Canada Gouvernement du Canada
* National Research Council Conseil national de recherches
* Ottawa, Canada, K1A 0R6 Ottawa, Canada, K1A 0R6
Expand Down Expand Up @@ -142,6 +142,10 @@ public void testContainerNode() {
URL nodeURL = getNodeURL(nodesServiceURL, name);
VOSURI nodeURI = getVOSURI(name);
ContainerNode testNode = new ContainerNode(name);
// try to add an immutable prop at creation: should be ignored
// --- use length aka file size since we are not writing file content
NodeProperty immutable = new NodeProperty(VOS.PROPERTY_URI_CONTENTLENGTH, "123");
testNode.getProperties().add(immutable);

// cleanup
if (nodelockSupported) {
Expand All @@ -164,6 +168,12 @@ public void testContainerNode() {
ContainerNode persistedNode = (ContainerNode) result.node;
Assert.assertEquals(testNode, persistedNode);
Assert.assertEquals(nodeURI, result.vosURI);
NodeProperty len = persistedNode.getProperty(VOS.PROPERTY_URI_CONTENTLENGTH);
if (len == null || len.getValue().equals("0")) {
log.info("immutable prop test: " + len);
} else {
Assert.fail("immutable prop test: " + len);
}

// POST an update to the node
NodeProperty nodeProperty = new NodeProperty(VOS.PROPERTY_URI_LANGUAGE, "English");
Expand All @@ -183,7 +193,25 @@ public void testContainerNode() {
if (nodelockSupported) {
Assert.assertTrue(updatedNode.isLocked);
}

len = persistedNode.getProperty(VOS.PROPERTY_URI_CONTENTLENGTH);
if (len == null || len.getValue().equals("0")) {
log.info("immutable prop test: " + len);
} else {
Assert.fail("immutable prop test: " + len);
}

// fail to update with a sketch property URI
URI illegal = new URI(VOS.VOSPACE_URI_NAMESPACE + "core#make-stuff-up");
NodeProperty illegalProp = new NodeProperty(illegal, "that should not work");
testNode.getProperties().add(illegalProp);
try {
post(nodeURL, nodeURI, testNode, true, 400);
Assert.fail("expected IllegalArgumentException, but updated testNode with " + illegal);
} catch (IllegalArgumentException expected) {
log.info("caught expected Exception: " + expected);
}
testNode.getProperties().remove(illegalProp);

// failed to add a subdirectory (node locked)
if (nodelockSupported) {
String subDirName = name + "/subDir";
Expand Down Expand Up @@ -233,15 +261,28 @@ public void testDataNode() {
// PUT the node
log.info("put: " + nodeURI + " -> " + nodeURL);
DataNode testNode = new DataNode(name);
// try to add an immutable prop at creation: should be ignored
// --- use length aka file size since we are not writing file content
NodeProperty immutable = new NodeProperty(VOS.PROPERTY_URI_CONTENTLENGTH, "123");
testNode.getProperties().add(immutable);
put(nodeURL, nodeURI, testNode);

// GET the new node
NodeReader.NodeReaderResult result = get(nodeURL, 200, XML_CONTENT_TYPE);
log.info("found: " + result.vosURI + " owner: " + result.node.ownerDisplay);
Assert.assertTrue(result.node instanceof DataNode);
DataNode persistedNode = (DataNode) result.node;
for (NodeProperty np : persistedNode.getProperties()) {
log.info("persisted prop: " + np.getKey() + " = " + np.getValue());
}
Assert.assertEquals(testNode, persistedNode);
Assert.assertEquals(nodeURI, result.vosURI);
NodeProperty len = persistedNode.getProperty(VOS.PROPERTY_URI_CONTENTLENGTH);
if (len == null || len.getValue().equals("0")) {
log.info("immutable prop test: " + len);
} else {
Assert.fail("immutable prop test: " + len);
}

// POST an update to the node
NodeProperty nodeProperty = new NodeProperty(VOS.PROPERTY_URI_LANGUAGE, "English");
Expand All @@ -251,10 +292,31 @@ public void testDataNode() {
// GET the updated node
result = get(nodeURL, 200, XML_CONTENT_TYPE);
DataNode updatedNode = (DataNode) result.node;
for (NodeProperty np : updatedNode.getProperties()) {
log.info("updated prop: " + np.getKey() + " = " + np.getValue());
}
Assert.assertEquals(testNode, updatedNode);
Assert.assertEquals(nodeURI, result.vosURI);
Assert.assertEquals(testNode.getName(), updatedNode.getName());
Assert.assertTrue(updatedNode.getProperties().contains(nodeProperty));
len = persistedNode.getProperty(VOS.PROPERTY_URI_CONTENTLENGTH);
if (len == null || len.getValue().equals("0")) {
log.info("immutable prop test: " + len);
} else {
Assert.fail("immutable prop test: " + len);
}

// fail to update with a sketch property URI
URI illegal = new URI(VOS.VOSPACE_URI_NAMESPACE + "core#make-stuff-up");
NodeProperty illegalProp = new NodeProperty(illegal, "that should not work");
testNode.getProperties().add(illegalProp);
try {
post(nodeURL, nodeURI, testNode, true, 400);
Assert.fail("expected IllegalArgumentException, but updated testNode with " + illegal);
} catch (IllegalArgumentException expected) {
log.info("caught expected Exception: " + expected);
}
testNode.getProperties().remove(illegalProp);

if (cleanupOnSuccess) {
delete(nodeURL);
Expand Down
20 changes: 14 additions & 6 deletions cadc-test-vos/src/main/java/org/opencadc/conformance/vos/VOSTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,15 @@ public NodeReader.NodeReaderResult get(URL url, int responseCode, String content
}
}

public void post(URL nodeURL, VOSURI vosURI, Node node) throws IOException {
post(nodeURL, vosURI, node, true);
public void post(URL nodeURL, VOSURI vosURI, Node node) throws Exception {
post(nodeURL, vosURI, node, true, 200);
}

public void post(URL nodeURL, VOSURI vosURI, Node node, boolean verify) throws IOException {
public void post(URL nodeURL, VOSURI vosURI, Node node, boolean verify) throws Exception {
post(nodeURL, vosURI, node, verify, 200);
}

public void post(URL nodeURL, VOSURI vosURI, Node node, boolean verify, int expectedCode) throws Exception {
StringBuilder sb = new StringBuilder();
NodeWriter writer = new NodeWriter();
writer.write(vosURI, node, sb, VOS.Detail.max);
Expand All @@ -278,8 +282,12 @@ public void post(URL nodeURL, VOSURI vosURI, Node node, boolean verify) throws I
if (!verify && post.getResponseCode() == 404) {
return;
}
Assert.assertEquals("expected POST response code = 200",
200, post.getResponseCode());
Assert.assertEquals("expected POST response code = " + expectedCode,
expectedCode, post.getResponseCode());
if (expectedCode >= 400) {
Assert.assertNotNull(post.getThrowable());
throw (Exception) post.getThrowable();
}
Assert.assertNull("expected POST throwable == null", post.getThrowable());
}

Expand Down Expand Up @@ -329,7 +337,7 @@ protected void cleanupNodeTree(String[] nodes) throws MalformedURLException {
}

protected void makeWritable(String[] subdirNames, GroupURI accessGroup)
throws NodeParsingException, NodeNotSupportedException, IOException {
throws Exception {
for (String nodeName : subdirNames) {
URL nodeURL = getNodeURL(nodesServiceURL, nodeName);
NodeReader.NodeReaderResult result = get(nodeURL, 200, XML_CONTENT_TYPE);
Expand Down
2 changes: 1 addition & 1 deletion cadc-vos-server/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ sourceCompatibility = 1.8

group = 'org.opencadc'

version = '2.0.6'
version = '2.0.7'

description = 'OpenCADC VOSpace server'
def git_url = 'https://github.com/opencadc/vos'
Expand Down
45 changes: 38 additions & 7 deletions cadc-vos-server/src/main/java/org/opencadc/vospace/server/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@

import ca.nrc.cadc.auth.AuthenticationUtil;
import java.net.URI;
import java.net.URISyntaxException;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Iterator;
Expand All @@ -80,6 +81,7 @@
import org.opencadc.vospace.ContainerNode;
import org.opencadc.vospace.Node;
import org.opencadc.vospace.NodeProperty;
import org.opencadc.vospace.VOS;

/**
* Utility methods
Expand Down Expand Up @@ -158,17 +160,46 @@ public static String getPath(Node node) {
* @param newProps set of new Node Properties to be used for the update
* @param immutable set of immutable property keys to skip
*/
public static void updateNodeProperties(Set<NodeProperty> oldProps, Set<NodeProperty> newProps, Set<URI> immutable) {
public static void updateNodeProperties(Set<NodeProperty> oldProps, Set<NodeProperty> newProps, Set<URI> immutable)
throws Exception {
for (Iterator<NodeProperty> newIter = newProps.iterator(); newIter.hasNext(); ) {
NodeProperty newProperty = newIter.next();
if (!immutable.contains(newProperty.getKey())) {
if (oldProps.contains(newProperty)) {
oldProps.remove(newProperty);
}
if (!newProperty.isMarkedForDeletion()) {
oldProps.add(newProperty);
if (newProperty.getKey().toASCIIString().startsWith(VOS.VOSPACE_URI_NAMESPACE)) {
try {
validatePropertyKey(newProperty.getKey());
} catch (URISyntaxException ex) {
throw NodeFault.InvalidArgument.getStatus(ex.getMessage());
}
}
if (oldProps.contains(newProperty)) {
oldProps.remove(newProperty);
}
if (!newProperty.isMarkedForDeletion() && !immutable.contains(newProperty.getKey())) {
oldProps.add(newProperty);
}
}
}

private static void validatePropertyKey(URI key) throws URISyntaxException {
if (key.getScheme() == null) {
throw new URISyntaxException("invalid structure: no scheme", key.toASCIIString());
}
if (!key.getScheme().equals("ivo") && !key.getScheme().equals("http") && !key.getScheme().equals("https")) {
throw new URISyntaxException("invalid structure: unknown scheme", key.toASCIIString());
}
if (key.getAuthority() == null) {
throw new URISyntaxException("invalid structure: no authority", key.toASCIIString());
}
if (key.getPath() == null) {
throw new URISyntaxException("invalid structure: no path", key.toASCIIString());
}
if (key.getFragment() == null) {
throw new URISyntaxException("invalid structure: no fragment", key.toASCIIString());
}
if (key.toASCIIString().startsWith(VOS.VOSPACE_URI_NAMESPACE)) {
if (!VOS.VOSPACE_CORE_PROPERTIES.contains(key)) {
throw new URISyntaxException("unrecognized property URI in vospace namespace", key.toASCIIString());
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion cadc-vos-server/src/main/java/org/opencadc/vospace/server/actions/CreateNodeAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,13 @@ public void doAction() throws Exception {
}

// pick out eligible admin-only props (they are immutable to normal users)
List<NodeProperty> allowedAdminProps = Utils.getAdminProps(clientNode, nodePersistence.getAdminProps(), caller,
final List<NodeProperty> allowedAdminProps = Utils.getAdminProps(clientNode, nodePersistence.getAdminProps(), caller,
nodePersistence);

// sanitize input properties into clean set
Set<NodeProperty> np = new HashSet<>();
Utils.updateNodeProperties(np, clientNode.getProperties(), nodePersistence.getImmutableProps());
clientNode.getProperties().clear();
clientNode.getProperties().addAll(np);
clientNode.getProperties().addAll(allowedAdminProps);

Expand Down
2 changes: 1 addition & 1 deletion cadc-vos-server/src/main/java/org/opencadc/vospace/server/actions/UpdateNodeAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public void doAction() throws Exception {
}

public static Node updateProperties(Node serverNode, Node clientNode, NodePersistence nodePersistence, Subject caller)
throws NodeNotSupportedException {
throws Exception {
// merge change request
if (clientNode.clearReadOnlyGroups || !clientNode.getReadOnlyGroup().isEmpty()) {
serverNode.getReadOnlyGroup().clear();
Expand Down
2 changes: 1 addition & 1 deletion cavern/VERSION
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## deployable containers have a semantic and build tag
# semantic version tag: major.minor
# build version tag: timestamp
VER=0.6.2
VER=0.6.3
TAGS="${VER} ${VER}-$(date -u +"%Y%m%dT%H%M%S")"
unset VER
1 change: 0 additions & 1 deletion cavern/src/main/java/org/opencadc/cavern/nodes/FileSystemNodePersistence.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@
import org.opencadc.vospace.VOS;
import org.opencadc.vospace.VOSURI;
import org.opencadc.vospace.server.LocalServiceURI;
import org.opencadc.vospace.server.NodeFault;
import org.opencadc.vospace.server.NodePersistence;
import org.opencadc.vospace.server.PathResolver;
import org.opencadc.vospace.server.Views;
Expand Down

0 comments on commit f6ab7e1

Please sign in to comment.