Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug fix and test for immutable and ilvalid node props #212

Merged
merged 6 commits into from
Jan 16, 2024
Merged
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
2 changes: 1 addition & 1 deletion cadc-test-vos/build.gradle
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
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
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
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
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
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
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
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
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
Loading