Skip to content

Commit

Permalink
cadc-gms: fix GroupURI validation and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@pdowler
pdowler committed Oct 20, 2023
1 parent b868f48 commit ff1372b
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 39 deletions.
2 changes: 1 addition & 1 deletion cadc-gms/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 = '1.0.8'
version = '1.0.9'

description = 'OpenCADC GMS API library'
def git_url = 'https://github.com/opencadc/ac'
Expand Down
54 changes: 32 additions & 22 deletions cadc-gms/src/main/java/org/opencadc/gms/GroupURI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,40 +91,37 @@ public class GroupURI implements Comparable<GroupURI> {
*/
public GroupURI(URI uri) throws IllegalArgumentException {
if (uri == null) {
throw new IllegalArgumentException("Null URI");
throw new IllegalArgumentException("null URI");
}

// Ensure the scheme is correct
if (uri.getScheme() == null || !"ivo".equals(uri.getScheme())) {
throw new IllegalArgumentException("GroupURI scheme must be 'ivo'.");
throw new IllegalArgumentException("scheme must be 'ivo' in resourceID: " + uri);
}

if (uri.getAuthority() == null) {
throw new IllegalArgumentException("Group authority is required.");
throw new IllegalArgumentException("authority is required in resourceID: " + uri);
}

if (uri.getPath() == null || uri.getPath().length() == 0) {
throw new IllegalArgumentException("Missing authority and/or path.");
throw new IllegalArgumentException("path is required in resourceID: " + uri);
}

if (uri.getFragment() != null) {
throw new IllegalArgumentException("fragment not allowed in uri: " + uri);
}

if (uri.getQuery() == null) {
throw new IllegalArgumentException("query (group name) required in uri: " + uri);
}

String fragment = uri.getFragment();
String query = uri.getQuery();
String name = null;
if (query == null) {
throw new IllegalArgumentException("Group name is required.");
} else {
if (fragment != null) {
throw new IllegalArgumentException("Fragment not allowed in group URIs");
}

if (isValidGroupName(query)) {
name = query;
} else {
throw new IllegalArgumentException(GROUP_NAME_ERRORMSG);
}
String name = uri.getQuery();
if (!isValidGroupName(name)) {
throw new IllegalArgumentException("invalid group name: " + name + " reason: " + GROUP_NAME_ERRORMSG);

}

this.uri = URI.create(uri.getScheme() + "://" + uri.getAuthority() + uri.getPath() + "?" + name);
this.uri = uri;
}

/**
Expand All @@ -142,7 +139,10 @@ public GroupURI(String uri) throws IllegalArgumentException, URISyntaxException

public GroupURI(URI resourceID, String name) {
if (resourceID == null) {
throw new IllegalArgumentException("Null URI");
throw new IllegalArgumentException("null GMS resourceID");
}
if (name == null) {
throw new IllegalArgumentException("null group name");
}

// Ensure the scheme is correct
Expand All @@ -157,9 +157,19 @@ public GroupURI(URI resourceID, String name) {
if (resourceID.getPath() == null || resourceID.getPath().length() == 0) {
throw new IllegalArgumentException("path is required in resourceID: " + resourceID);
}

if (resourceID.getQuery() != null) {
throw new IllegalArgumentException("query not allowed in resourceID: " + resourceID);
}

if (resourceID.getFragment() != null) {
throw new IllegalArgumentException("fragment not allowed in resourceID: " + resourceID.getFragment());
throw new IllegalArgumentException("fragment not allowed in resourceID: " + resourceID);
}

if (!isValidGroupName(name)) {
throw new IllegalArgumentException("invalid group name: " + name + " reason: " + GROUP_NAME_ERRORMSG);
}

this.uri = URI.create(resourceID.toASCIIString() + "?" + name);
}

Expand Down
57 changes: 41 additions & 16 deletions cadc-gms/src/test/java/org/opencadc/gms/GroupURITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class GroupURITest {
private static Logger log = Logger.getLogger(GroupURITest.class);

static {
Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
Log4jInit.setLevel("org.opencadc.gms", Level.INFO);
}

@Test
Expand All @@ -28,24 +28,39 @@ public void testEquals() {

@Test
public void testMalformed() {

try {

assertIllegalArgument(URI.create("ivo://example.org/[email protected]"));
assertIllegalArgument(URI.create("ivo://example.org/gms"), "[email protected]");

// no scheme
assertIllegalArgument("example.org/gms?gname", "scheme");
assertIllegalArgument(URI.create("example.org/gms?gname"));
assertIllegalArgument(URI.create("example.org/gms"), "gname");

// wrong scheme
assertIllegalArgument("gms://example.org/gms?gname", "scheme");
assertIllegalArgument(URI.create("gms://example.org/gms?gname"));
assertIllegalArgument(URI.create("gms://example.org/gms"), "gname");

// no authority
assertIllegalArgument("ivo://gms?gname", "authority");
assertIllegalArgument(URI.create("ivo://gms?gname"));
assertIllegalArgument(URI.create("ivo://gms"), "gname");

// no path
assertIllegalArgument("ivo://example.org/gname", "name");
assertIllegalArgument(URI.create("ivo://example.org?gname"));
assertIllegalArgument(URI.create("ivo://example.org"), "gname");

// group name in fragment no longer allowed
assertIllegalArgument("ivo://my.authority/gms#name", "name");
// no group name
assertIllegalArgument(URI.create("ivo://example.org/gms"));
assertIllegalArgument(URI.create("ivo://example.org/gms"), null);
assertIllegalArgument(URI.create("ivo://example.org/gms"), "");
assertIllegalArgument(URI.create("ivo://example.org/gms"), " ");

// fragment not allowed
assertIllegalArgument("ivo://my.authority/gms?name#name", "fragment");
assertIllegalArgument(URI.create("ivo://my.authority/gms#name"));
assertIllegalArgument(URI.create("ivo://my.authority/gms#name"), null);
assertIllegalArgument(URI.create("ivo://my.authority/gms?name#name"));
assertIllegalArgument(URI.create("ivo://my.authority/gms?name#name"));

} catch (Exception unexpected) {
log.error("unexpected exception", unexpected);
Expand All @@ -56,7 +71,7 @@ public void testMalformed() {
@Test
public void testSimpleGroupName() {
try {
GroupURI g = new GroupURI("ivo://my.authority/gms?name");
GroupURI g = new GroupURI(URI.create("ivo://my.authority/gms?name"));
Assert.assertEquals("ivo", g.getURI().getScheme());
Assert.assertEquals("/gms", g.getURI().getPath());
Assert.assertEquals("name", g.getName());
Expand All @@ -72,7 +87,7 @@ public void testSimpleGroupName() {
public void testHierarchicalGroupName() {
try {
String name = "hierachical/group/structure";
GroupURI g = new GroupURI("ivo://my.authority/gms?" + name);
GroupURI g = new GroupURI(URI.create("ivo://my.authority/gms?" + name));
Assert.assertEquals("ivo", g.getURI().getScheme());
Assert.assertEquals("/gms", g.getURI().getPath());
Assert.assertEquals(name, g.getName());
Expand All @@ -84,14 +99,24 @@ public void testHierarchicalGroupName() {
}
}

private void assertIllegalArgument(String uri, String message) throws URISyntaxException {
private void assertIllegalArgument(URI uri) {
try {
GroupURI gu = new GroupURI(uri);
Assert.fail("expected IllegalArgumentException, got: " + gu);
} catch (IllegalArgumentException e) {
log.info("caught expected: " + e);
} catch (Exception unexpected) {
log.error("unexpected exception", unexpected);
Assert.fail("unexpected exception: " + unexpected);
}
}

private void assertIllegalArgument(URI resourceID, String name) {
try {
new GroupURI(uri);
Assert.fail("expected IllegalArgumentException, got: " + uri);
GroupURI gu = new GroupURI(resourceID, name);
Assert.fail("expected IllegalArgumentException, got: " + gu);
} catch (IllegalArgumentException e) {
// expected
log.debug("Checking if message '" + e.getMessage() + "' contains '" + message + "'");
Assert.assertTrue(e.getMessage().toLowerCase().contains(message));
log.info("caught expected: " + e);
} catch (Exception unexpected) {
log.error("unexpected exception", unexpected);
Assert.fail("unexpected exception: " + unexpected);
Expand Down

0 comments on commit ff1372b

Please sign in to comment.