Skip to content

CADC-16019 catch runtime exception when unable to get the recursive set node job #48

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.

Sign up for GitHub

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

Merged
jburke-cadc merged 6 commits into from
Sep 23, 2026
Merged

CADC-16019 catch runtime exception when unable to get the recursive set node job #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3973949
restore json only logging
jburke-cadc Sep 17, 2026
0c82072
catch runtime when unable to get the recursive lock nodes job phase
jburke-cadc Sep 17, 2026
cb47492
clean up info logging
jburke-cadc Sep 17, 2026
47340d3
replace deprecated AccessControlException with PermissionDeniedException
jburke-cadc Sep 17, 2026
fd4a1dc
add null check for contentType
jburke-cadc Sep 17, 2026
ec31954
remove redundant test at end of identifier
jburke-cadc Sep 23, 2026
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
1 change: 1 addition & 0 deletions doi/helm/config/catalina.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ tomcat.connector.scheme=https
tomcat.connector.proxyName={{ include "doi.hostname" . }}
tomcat.connector.proxyPort=443
ca.nrc.cadc.auth.PrincipalExtractor.enableClientCertHeader=true
ca.nrc.cadc.util.Log4jInit.messageOnly=true

# (default: ca.nrc.cadc.auth.NoOpIdentityManager)
ca.nrc.cadc.auth.IdentityManager={{ .Values.application.identityManagerClass }}
8 changes: 4 additions & 4 deletions doi/src/intTest/java/ca/nrc/cadc/doi/IntTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import ca.nrc.cadc.doi.status.Status;
import ca.nrc.cadc.net.FileContent;
import ca.nrc.cadc.net.HttpPost;
import ca.nrc.cadc.net.PermissionDeniedException;
import ca.nrc.cadc.reg.Standards;
import ca.nrc.cadc.reg.client.RegistryClient;
import ca.nrc.cadc.util.FileUtil;
Expand All @@ -84,7 +85,6 @@
import java.io.IOException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.security.AccessControlException;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.HashMap;
Expand Down Expand Up @@ -304,9 +304,9 @@ protected void cleanup(String doiSuffix, DOISettingsType doiSettingsType) {
log.debug(String.format("RecursiveDeleteNode done, phase: %s exception: %s",
recursiveDeleteNode.getPhase(), recursiveDeleteNode.getException()));
log.debug("deleted node: " + nodeUri.getPath());
} catch (AccessControlException e) {
log.error("unexpected AccessControlException: ", e);
Assert.fail("unexpected AccessControlException: " + e);
} catch (PermissionDeniedException e) {
log.error("unexpected PermissionDeniedException: ", e);
Assert.fail("unexpected PermissionDeniedException: " + e);
} catch (Exception e) {
log.error("unexpected exception", e);
Assert.fail("unexpected exception: " + e);
Expand Down
5 changes: 2 additions & 3 deletions doi/src/intTest/java/ca/nrc/cadc/doi/LifecycleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@
import ca.nrc.cadc.net.FileContent;
import ca.nrc.cadc.net.HttpGet;
import ca.nrc.cadc.net.HttpPost;
import ca.nrc.cadc.net.PermissionDeniedException;
import ca.nrc.cadc.util.Log4jInit;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.StringReader;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.security.AccessControlException;
import java.security.PrivilegedExceptionAction;
import java.time.LocalDate;
import java.time.ZoneId;
Expand Down Expand Up @@ -257,8 +257,7 @@ Resource create(Resource expected, DOISettingsType doiSettingsType) throws Exce
VOSURI target1 = getVOSURI(writeFile, doiSettingsType);
DataNode dataNode1 = new DataNode(writeName);
vosClient.createNode(target1, dataNode1);
} catch (
AccessControlException e) {
} catch (PermissionDeniedException e) {
log.debug("expected exception: " + e.getMessage());
} catch (Exception e) {
Assert.fail("exception writing file: " + e.getMessage());
Expand Down
6 changes: 3 additions & 3 deletions doi/src/main/java/ca/nrc/cadc/doi/DeleteAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
package ca.nrc.cadc.doi;

import ca.nrc.cadc.doi.status.Status;
import java.security.AccessControlException;
import ca.nrc.cadc.net.PermissionDeniedException;
import java.security.PrivilegedExceptionAction;
import javax.security.auth.Subject;
import org.apache.log4j.Logger;
Expand Down Expand Up @@ -108,7 +108,7 @@ private void authorizeResourceAccess() throws NodeNotFoundException {
return;
}

throw new AccessControlException("Not authorized to Delete this resource.");
throw new PermissionDeniedException("Not authorized to Delete this resource.");
}

private void doActionImpl() throws Exception {
Expand All @@ -125,7 +125,7 @@ private void doActionImpl() throws Exception {
// check the state of the doi
String doiStatus = doiContainer.getPropertyValue(DOI.VOSPACE_DOI_STATUS_PROPERTY);
if (doiStatus != null && doiStatus.equals(Status.MINTED.getValue())) {
throw new AccessControlException("Unable to delete " + doiSuffix + "DOI already minted.\n");
throw new PermissionDeniedException("Unable to delete " + doiSuffix + "DOI already minted.\n");
}

// Delete the DOI group. Will be format DOI-<DOINumInputStr>
Expand Down
94 changes: 52 additions & 42 deletions doi/src/main/java/ca/nrc/cadc/doi/DoiAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import ca.nrc.cadc.doi.status.DoiStatusListXmlWriter;
import ca.nrc.cadc.doi.status.Status;
import ca.nrc.cadc.net.OutputStreamWrapper;
import ca.nrc.cadc.net.PermissionDeniedException;
import ca.nrc.cadc.net.ResourceNotFoundException;
import ca.nrc.cadc.reg.Standards;
import ca.nrc.cadc.reg.client.LocalAuthority;
Expand All @@ -97,7 +98,6 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.net.UnknownHostException;
import java.security.AccessControlException;
import java.security.Principal;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
Expand Down Expand Up @@ -218,7 +218,7 @@ protected void authorize() {
}
// authorization, for now, is defined as having a set of principals
if (callingSubject == null || callingSubject.getPrincipals().isEmpty()) {
throw new AccessControlException("Unauthorized");
throw new PermissionDeniedException("Unauthorized");
}
}

Expand Down Expand Up @@ -402,7 +402,7 @@ protected DoiStatus getDoiStatus(String doiSuffixString, ContainerNode doiContai
doiStatus.reviewer = doiContainerNode.getPropertyValue(DOI.VOSPACE_DOI_REVIEWER_PROPERTY);
} else {
String msg = "Access Denied to " + doiSuffixString + ".";
throw new AccessControlException(msg);
throw new PermissionDeniedException(msg);
}
return doiStatus;
}
Expand All @@ -418,44 +418,54 @@ protected String updateMintingStatus(final ContainerNode doiContainerNode, final
VOSURI vosuri = new VOSURI(vaultResourceID, String.format("%s/%s", parentPath, doiContainerNode.getName()));
RecursiveSetNode recursiveSetNode = new RecursiveSetNode(jobURL, doiContainerNode);
recursiveSetNode.setSchemaValidation(false);
ExecutionPhase phase = recursiveSetNode.getPhase(20); // seconds
switch (phase) {
case COMPLETED:
case ARCHIVED:
// job finished, set corresponding status
if (status.equals(Status.LOCKING_DATA.getValue())) {
localStatus = Status.LOCKED_DATA.getValue();
} else if (status.equals(Status.REGISTERING.getValue())) {
localStatus = Status.MINTED.getValue();
}
// delete jobURL property
doiContainerNode.getProperties().remove(new NodeProperty(DOI.VOSPACE_DOI_JOB_URL_PROPERTY));
doiContainerNode.getProperty(DOI.VOSPACE_DOI_STATUS_PROPERTY).setValue(localStatus);
vospaceDoiClient.getVOSpaceClient().setNode(vosuri, doiContainerNode);
break;
case ERROR:
case ABORTED:
case UNKNOWN:
case SUSPENDED:
case HELD:
// assume job resulted in error, set corresponding status
if (status.equals(Status.LOCKING_DATA.getValue())) {
localStatus = Status.ERROR_LOCKING_DATA.getValue();
} else if (status.equals(Status.REGISTERING.getValue())) {
localStatus = Status.ERROR_REGISTERING.getValue();
}
// delete jobURL property
doiContainerNode.getProperties().remove(new NodeProperty(DOI.VOSPACE_DOI_JOB_URL_PROPERTY));
doiContainerNode.getProperty(DOI.VOSPACE_DOI_STATUS_PROPERTY).setValue(localStatus);
vospaceDoiClient.getVOSpaceClient().setNode(vosuri, doiContainerNode);
break;
case PENDING:
case QUEUED:
case EXECUTING:
// job is in progress, do nothing
break;
default:
// do nothing

ExecutionPhase phase;
try {
phase = recursiveSetNode.getPhase(20); // seconds
} catch (RuntimeException ex) {
log.error("error getting recursive lock nodes job status for " + doiContainerNode.getName(), ex);
phase = ExecutionPhase.ERROR;
}

if (phase != null) {
switch (phase) {
case COMPLETED:
case ARCHIVED:
// job finished, set corresponding status
if (status.equals(Status.LOCKING_DATA.getValue())) {
localStatus = Status.LOCKED_DATA.getValue();
} else if (status.equals(Status.REGISTERING.getValue())) {
localStatus = Status.MINTED.getValue();
}
// delete jobURL property
doiContainerNode.getProperties().remove(new NodeProperty(DOI.VOSPACE_DOI_JOB_URL_PROPERTY));
doiContainerNode.getProperty(DOI.VOSPACE_DOI_STATUS_PROPERTY).setValue(localStatus);
vospaceDoiClient.getVOSpaceClient().setNode(vosuri, doiContainerNode);
break;
case ERROR:
case ABORTED:
case UNKNOWN:
case SUSPENDED:
case HELD:
// assume job resulted in error, set corresponding status
if (status.equals(Status.LOCKING_DATA.getValue())) {
localStatus = Status.ERROR_LOCKING_DATA.getValue();
} else if (status.equals(Status.REGISTERING.getValue())) {
localStatus = Status.ERROR_REGISTERING.getValue();
}
// delete jobURL property
doiContainerNode.getProperties().remove(new NodeProperty(DOI.VOSPACE_DOI_JOB_URL_PROPERTY));
doiContainerNode.getProperty(DOI.VOSPACE_DOI_STATUS_PROPERTY).setValue(localStatus);
vospaceDoiClient.getVOSpaceClient().setNode(vosuri, doiContainerNode);
break;
case PENDING:
case QUEUED:
case EXECUTING:
// job is in progress, do nothing
break;
default:
// do nothing
}
}
}
return localStatus;
Expand Down Expand Up @@ -489,7 +499,7 @@ protected void uploadDOIDocument(Resource resource, VOSURI docVOSUIRI) throws Re
throw new ResourceNotFoundException(message);
}
if (message.contains("PermissionDenied")) {
throw new java.security.AccessControlException(message);
throw new PermissionDeniedException(message);
}
}
throw new RuntimeException((clientTransfer.getThrowable().getMessage()));
Expand Down
18 changes: 9 additions & 9 deletions doi/src/main/java/ca/nrc/cadc/doi/DoiInlineContentHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,27 +102,27 @@ public Content accept(String name, String contentType, InputStream inputStream)

InlineContentHandler.Content content = new InlineContentHandler.Content();
content.name = name;
log.info("content name: " + name);
log.info("content type: " + contentType);
log.debug("content name: " + name);
log.debug("content type: " + contentType);
if (META_DATA_KEY.equals(name)) {
if (contentType.equalsIgnoreCase(XML_CONTENT_TYPE)) {
log.info("content type: " + XML_CONTENT_TYPE);
log.debug("content type: " + XML_CONTENT_TYPE);
try {
// read xml file
DoiXmlReader reader = new DoiXmlReader(false);
content.value = reader.read(inputStream);
log.info("content value: " + content.value);
log.debug("content value: " + content.value);
} catch (DoiParsingException dpe) {
log.debug(dpe);
throw new InlineContentException(dpe.getMessage());
}
} else if (contentType.equalsIgnoreCase(JSON_CONTENT_TYPE)) {
log.info("content type: " + JSON_CONTENT_TYPE);
log.debug("content type: " + JSON_CONTENT_TYPE);
try {
// read json file
DoiJsonReader reader = new DoiJsonReader();
content.value = reader.read(inputStream);
log.info("content value: " + content.value);
log.debug("content value: " + content.value);
} catch (DoiParsingException dpe) {
log.debug(dpe);
throw new InlineContentException(dpe.getMessage());
Expand All @@ -131,14 +131,14 @@ public Content accept(String name, String contentType, InputStream inputStream)
}
else if (NODE_DATA_KEY.equals(name)) {
if (contentType.equalsIgnoreCase(XML_CONTENT_TYPE)) {
log.info("content type: " + XML_CONTENT_TYPE);
log.debug("content type: " + XML_CONTENT_TYPE);
// xml not supported for node property updates
throw new IllegalArgumentException("XML node updates are not supported");
} else if (contentType.equalsIgnoreCase(JSON_CONTENT_TYPE)) {
log.info("content type: " + JSON_CONTENT_TYPE);
log.debug("content type: " + JSON_CONTENT_TYPE);
// read json file
content.value = new JSONObject(new JSONTokener(inputStream));
log.info("content value: " + content.value);
log.debug("content value: " + content.value);
}
} else {
throw new IllegalArgumentException("Unrecognized content type: " + name);
Expand Down
9 changes: 4 additions & 5 deletions doi/src/main/java/ca/nrc/cadc/doi/PostAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import ca.nrc.cadc.net.HttpPost;
import ca.nrc.cadc.net.HttpTransfer;
import ca.nrc.cadc.net.HttpUpload;
import ca.nrc.cadc.net.PermissionDeniedException;
import ca.nrc.cadc.net.ResourceNotFoundException;
import ca.nrc.cadc.util.Base64;
import ca.nrc.cadc.util.StringUtil;
Expand All @@ -94,7 +95,6 @@
import java.net.URI;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.security.AccessControlException;
import java.security.PrivilegedExceptionAction;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
Expand Down Expand Up @@ -168,7 +168,7 @@ private void authorizeResourceAccess() throws NodeNotFoundException {
if (isPublisher && !isRequester) {
return;
} else {
throw new AccessControlException("Not authorized to Mint this resource: " + doiSuffix);
throw new PermissionDeniedException("Not authorized to Mint this resource: " + doiSuffix);
}
}

Expand All @@ -177,7 +177,7 @@ private void authorizeResourceAccess() throws NodeNotFoundException {
return;
}

throw new AccessControlException("Not authorized to update this resource: " + doiSuffix);
throw new PermissionDeniedException("Not authorized to update this resource: " + doiSuffix);
}

private String getDataciteCredentials() {
Expand Down Expand Up @@ -268,7 +268,6 @@ private String getRandomDOISuffix() {
sb.append(allowed.charAt(index));
}
}
sb.append(".test");
return sb.toString();
}

Expand Down Expand Up @@ -983,7 +982,7 @@ private void processResponse(Throwable throwable, int responseCode, InputStream
// check if an exception was thrown
if (throwable != null) {
if ((responseCode == 401) || (responseCode == 403)) {
throw new AccessControlException(throwable.getMessage());
throw new PermissionDeniedException(throwable.getMessage());
} else {
throw new RuntimeException(body + ", " + throwable);
}
Expand Down
6 changes: 3 additions & 3 deletions doi/src/main/java/ca/nrc/cadc/doi/SearchAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public SearchAction() {
* java.lang.IllegalArgumentException : 400
* ca.nrc.cadc.auth.NotAuthenticatedException : 401
* java.security.cert.CertificateException : 403 -- should be 401 with a suitable challenge
* java.security.AccessControlException : 403
* java.security.PermissionDeniedException : 403
* ca.nrc.cadc.net.ResourceNotFoundException : 404
* ca.nrc.cadc.net.ResourceAlreadyExistsException : 409
* ca.nrc.cadc.net.PreconditionFailedException (and subclasses) : 412
Expand All @@ -119,7 +119,7 @@ public void doAction() throws Exception {

JSONObject jsonObject = (JSONObject) syncInput.getContent(SearchInlineContentHandler.CONTENT_KEY);
Set<String> keys = jsonObject.keySet();
log.info("jsonObject: " + jsonObject.toString(2));
log.debug("jsonObject: " + jsonObject.toString(2));
validateKeys(keys);

DoiSearchFilter searchFilter = new DoiSearchFilter();
Expand All @@ -137,7 +137,7 @@ public void doAction() throws Exception {
List<String> statusList = jsonArray.toList().stream()
.map(Object::toString)
.collect(Collectors.toList());
log.info("statusList: " + statusList);
log.debug("statusList: " + statusList);
searchFilter.prepareStatusList(statusList);
}

Expand Down
2 changes: 1 addition & 1 deletion doi/src/main/java/ca/nrc/cadc/doi/SearchInlineContentHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public Content accept(String name, String contentType, InputStream inputStream)
if (inputStream == null) {
throw new IOException("The InputStream is closed");
}
if (!contentType.toLowerCase().contains("application/json")) {
if (contentType == null || !contentType.toLowerCase().contains("application/json")) {
throw new InlineContentException("Content-Type must be application/json");
}

Expand Down
Loading
Loading