From 1ae54699a576699ff8e9d6f014fdf28d8749199c Mon Sep 17 00:00:00 2001 From: Rick Herrick <jrherrick@wustl.edu> Date: Fri, 22 Jul 2016 15:02:40 -0500 Subject: [PATCH] XNAT-4332, XNAT-4412, XNAT-4431 Explicitly set ID of updated DICOM SCP instances to ID specified in URI. Added checks in DICOM API and manager classes to catch enabled instances that duplicate the port of another enabled instance. --- .../java/org/nrg/dcm/DicomSCPManager.java | 117 ++++++++++-------- ...COMReceiverWithDuplicatePortException.java | 29 +++++ .../dcm/preferences/DicomSCPPreference.java | 14 ++- .../DuplicateEnabledDICOMReceiverPort.java | 9 -- .../nrg/xapi/rest/dicomscp/DicomSCPApi.java | 62 ++++------ .../xapi/rest/dicomscp/DicomSCPApiAdvice.java | 6 +- .../restlet/extensions/DicomSCPRestlet.java | 7 +- 7 files changed, 141 insertions(+), 103 deletions(-) create mode 100644 src/main/java/org/nrg/dcm/exceptions/EnabledDICOMReceiverWithDuplicatePortException.java delete mode 100644 src/main/java/org/nrg/xapi/exceptions/DuplicateEnabledDICOMReceiverPort.java diff --git a/src/main/java/org/nrg/dcm/DicomSCPManager.java b/src/main/java/org/nrg/dcm/DicomSCPManager.java index 89b88477..f7d91ef5 100644 --- a/src/main/java/org/nrg/dcm/DicomSCPManager.java +++ b/src/main/java/org/nrg/dcm/DicomSCPManager.java @@ -10,6 +10,7 @@ */ package org.nrg.dcm; +import org.nrg.dcm.exceptions.EnabledDICOMReceiverWithDuplicatePortException; import org.nrg.dcm.preferences.DicomSCPInstance; import org.nrg.dcm.preferences.DicomSCPPreference; import org.nrg.framework.exceptions.NrgServiceError; @@ -25,7 +26,6 @@ import java.io.IOException; import java.util.*; public class DicomSCPManager { - @Autowired public DicomSCPManager(final DicomSCPPreference dicomScpPreferences, final SiteConfigPreferences siteConfigPreferences) { _dicomScpPreferences = dicomScpPreferences; @@ -38,25 +38,17 @@ public class DicomSCPManager { stopDicomSCPs(); } - public DicomSCP create(final DicomSCPInstance instance) throws NrgServiceException { - final DicomSCPInstance atPort = _dicomScpPreferences.getDicomSCPAtPort(instance.getPort()); - if (atPort != null) { - throw new NrgServiceException(NrgServiceError.AlreadyInitialized, "Unable to create DICOM SCP [" + instance.toString() + "]: there is an existing enabled receiver already using the same port."); - } + public DicomSCP create(final DicomSCPInstance instance) throws IOException, EnabledDICOMReceiverWithDuplicatePortException { instance.setId(getNextKey()); - try { - _dicomScpPreferences.setDicomSCPInstance(instance); - if (_log.isDebugEnabled()) { - _log.debug("Created new DICOM SCP: " + instance.toString()); - } - final DicomSCP dicomSCP = _dicomScpPreferences.getDicomSCP(instance.getId()); - if (instance.isEnabled()) { - dicomSCP.start(); - } - return dicomSCP; - } catch (IOException e) { - throw new NrgServiceException(NrgServiceError.Unknown, "Unable to create DICOM SCP: " + instance.getAeTitle() + ":" + instance.getPort(), e); + _dicomScpPreferences.setDicomSCPInstance(instance); + if (_log.isDebugEnabled()) { + _log.debug("Created new DICOM SCP: " + instance.toString()); + } + final DicomSCP dicomSCP = _dicomScpPreferences.getDicomSCP(instance.getId()); + if (instance.isEnabled()) { + dicomSCP.start(); } + return dicomSCP; } public void delete(final int id) throws NrgServiceException { @@ -69,11 +61,27 @@ public class DicomSCPManager { _dicomScpPreferences.deleteDicomSCPInstance(id); } + public boolean hasDicomSCP(final int id) { + return _dicomScpPreferences.hasDicomSCPInstance(id); + } + + public Map<DicomSCP, Boolean> areDicomSCPsStarted() { + final Map<DicomSCP, Boolean> statuses = new HashMap<>(); + for (final DicomSCP dicomSCP : _dicomScpPreferences.getDicomSCPs()) { + statuses.put(dicomSCP, dicomSCP.isStarted()); + } + return statuses; + } + public List<DicomSCPInstance> getDicomSCPInstances() { return new ArrayList<>(_dicomScpPreferences.getDicomSCPInstances().values()); } - public void setDicomSCPInstance(final DicomSCPInstance instance) { + public DicomSCPInstance getDicomSCPInstance(final int id) { + return _dicomScpPreferences.getDicomSCPInstance(id); + } + + public void setDicomSCPInstance(final DicomSCPInstance instance) throws EnabledDICOMReceiverWithDuplicatePortException { try { _dicomScpPreferences.setDicomSCPInstance(instance); } catch (IOException e) { @@ -90,21 +98,41 @@ public class DicomSCPManager { final List<DicomSCP> started = new ArrayList<>(); for (final DicomSCPInstance instance : _dicomScpPreferences.getDicomSCPInstances().values()) { if (instance.isEnabled()) { - started.add(startDicomSCP(instance)); + final DicomSCP dicomSCP = startDicomSCP(instance); + if (dicomSCP != null) { + started.add(dicomSCP); + } } } return started; } - public void startDicomSCP(final int id) { - startDicomSCP(_dicomScpPreferences.getDicomSCPInstance(id)); + public DicomSCP startDicomSCP(final int id) { + final DicomSCPInstance instance = _dicomScpPreferences.getDicomSCPInstance(id); + if (instance == null) { + throw new NrgServiceRuntimeException(NrgServiceError.UnknownEntity, "Couldn't find the DICOM SCP instance identified by " + id); + } + return startDicomSCP(instance); + } + + public DicomSCP startDicomSCP(final DicomSCPInstance instance) { + try { + final DicomSCP dicomSCP = _dicomScpPreferences.getDicomSCP(instance.getId()); + if (!dicomSCP.isStarted()) { + dicomSCP.start(); + return dicomSCP; + } + return null; + } catch (IOException e) { + throw new NrgServiceRuntimeException(NrgServiceError.Unknown, "Unable to start DICOM SCP: " + instance.getAeTitle() + ":" + instance.getPort(), e); + } } public List<DicomSCP> stopDicomSCPs() { final List<DicomSCP> stopped = new ArrayList<>(); - for (final DicomSCP dicomSCP : _dicomScpPreferences.getDicomSCPs()) { - if (dicomSCP.isStarted()) { - dicomSCP.stop(); + for (final DicomSCPInstance instance : _dicomScpPreferences.getDicomSCPInstances().values()) { + final DicomSCP dicomSCP = stopDicomSCP(instance); + if (dicomSCP != null) { stopped.add(dicomSCP); } } @@ -116,19 +144,25 @@ public class DicomSCPManager { if (instance == null) { throw new NrgServiceRuntimeException(NrgServiceError.UnknownEntity, "Couldn't find the DICOM SCP instance identified by " + id); } + stopDicomSCP(instance); + } + + public DicomSCP stopDicomSCP(final DicomSCPInstance instance) { try { - final DicomSCP dicomSCP = _dicomScpPreferences.getDicomSCP(id); + final DicomSCP dicomSCP = _dicomScpPreferences.getDicomSCP(instance.getId()); if (dicomSCP != null) { if (dicomSCP.isStarted()) { dicomSCP.stop(); + return dicomSCP; } } + return null; } catch (IOException e) { throw new NrgServiceRuntimeException(NrgServiceError.Unknown, "Unable to stop DICOM SCP: " + instance.getAeTitle() + ":" + instance.getPort(), e); } } - public void enableDicomSCP(final int id) { + public void enableDicomSCP(final int id) throws EnabledDICOMReceiverWithDuplicatePortException { final DicomSCPInstance instance = _dicomScpPreferences.getDicomSCPInstance(id); try { if (!instance.isEnabled()) { @@ -157,34 +191,13 @@ public class DicomSCPManager { } } catch (IOException e) { throw new NrgServiceRuntimeException(NrgServiceError.Unknown, "Unable to disable DICOM SCP: " + instance.getAeTitle() + ":" + instance.getPort(), e); + } catch (EnabledDICOMReceiverWithDuplicatePortException ignored) { + // We can ignore this: the exception comes when an enabled instance is inserted with the same port as + // another enabled instance. Since we're explicitly disabling this instance, we won't actually get this + // error. } } - public Map<DicomSCP, Boolean> areDicomSCPsStarted() { - final Map<DicomSCP, Boolean> statuses = new HashMap<>(); - for (final DicomSCP dicomSCP : _dicomScpPreferences.getDicomSCPs()) { - statuses.put(dicomSCP, dicomSCP.isStarted()); - } - return statuses; - } - - public boolean hasDicomSCP(final int id) { - return _dicomScpPreferences.hasDicomSCPInstance(id); - } - - public DicomSCPInstance getDicomSCPInstance(final int id) { - return _dicomScpPreferences.getDicomSCPInstance(id); - } - - private DicomSCP startDicomSCP(final DicomSCPInstance instance) { - try { - final DicomSCP dicomSCP = _dicomScpPreferences.getDicomSCP(instance.getId()); - dicomSCP.start(); - return dicomSCP; - } catch (IOException e) { - throw new NrgServiceRuntimeException(NrgServiceError.Unknown, "Unable to start DICOM SCP: " + instance.getAeTitle() + ":" + instance.getPort(), e); - } - } private int getNextKey() { final Set<String> keys = _dicomScpPreferences.getDicomSCPInstances().keySet(); diff --git a/src/main/java/org/nrg/dcm/exceptions/EnabledDICOMReceiverWithDuplicatePortException.java b/src/main/java/org/nrg/dcm/exceptions/EnabledDICOMReceiverWithDuplicatePortException.java new file mode 100644 index 00000000..9effe5bd --- /dev/null +++ b/src/main/java/org/nrg/dcm/exceptions/EnabledDICOMReceiverWithDuplicatePortException.java @@ -0,0 +1,29 @@ +package org.nrg.dcm.exceptions; + +import org.nrg.dcm.preferences.DicomSCPInstance; +import org.nrg.framework.exceptions.NrgServiceError; +import org.nrg.framework.exceptions.NrgServiceException; + +public class EnabledDICOMReceiverWithDuplicatePortException extends NrgServiceException { + public EnabledDICOMReceiverWithDuplicatePortException(final DicomSCPInstance existing, final DicomSCPInstance inserted) { + super(NrgServiceError.AlreadyInitialized); + _existing = existing; + _inserted = inserted; + } + + public DicomSCPInstance getExisting() { + return _existing; + } + + public DicomSCPInstance getInserted() { + return _inserted; + } + + @Override + public String toString() { + return "Tried to insert DICOM SCP receiver ID " + _inserted.getId() + " [" + _inserted.toString() + "], which replicated the port as the enabled receiver ID " + _existing.getId() + " [" + _existing.toString() + "]"; + } + + private final DicomSCPInstance _existing; + private final DicomSCPInstance _inserted; +} diff --git a/src/main/java/org/nrg/dcm/preferences/DicomSCPPreference.java b/src/main/java/org/nrg/dcm/preferences/DicomSCPPreference.java index fff20886..de3cf973 100644 --- a/src/main/java/org/nrg/dcm/preferences/DicomSCPPreference.java +++ b/src/main/java/org/nrg/dcm/preferences/DicomSCPPreference.java @@ -3,6 +3,7 @@ package org.nrg.dcm.preferences; import org.apache.commons.lang3.StringUtils; import org.nrg.dcm.DicomFileNamer; import org.nrg.dcm.DicomSCP; +import org.nrg.dcm.exceptions.EnabledDICOMReceiverWithDuplicatePortException; import org.nrg.framework.exceptions.NrgServiceError; import org.nrg.framework.exceptions.NrgServiceRuntimeException; import org.nrg.prefs.annotations.NrgPreference; @@ -76,9 +77,18 @@ public class DicomSCPPreference extends AbstractPreferenceBean { return getDicomSCPInstances().get(Integer.toString(id)); } - public void setDicomSCPInstance(final DicomSCPInstance instance) throws IOException { + public void setDicomSCPInstance(final DicomSCPInstance instance) throws IOException, EnabledDICOMReceiverWithDuplicatePortException { final int id = instance.getId(); - deleteDicomSCP(id); + + final DicomSCPInstance atPort = getDicomSCPAtPort(instance.getPort()); + if (atPort != null && atPort.getId() != id) { + throw new EnabledDICOMReceiverWithDuplicatePortException(atPort, instance); + } + + if (hasDicomSCPInstance(id)) { + deleteDicomSCP(id); + } + try { set(serialize(instance), PREF_ID, Integer.toString(id)); } catch (InvalidPreferenceName invalidPreferenceName) { diff --git a/src/main/java/org/nrg/xapi/exceptions/DuplicateEnabledDICOMReceiverPort.java b/src/main/java/org/nrg/xapi/exceptions/DuplicateEnabledDICOMReceiverPort.java deleted file mode 100644 index 292b4df5..00000000 --- a/src/main/java/org/nrg/xapi/exceptions/DuplicateEnabledDICOMReceiverPort.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.nrg.xapi.exceptions; - -import org.nrg.framework.exceptions.NrgServiceException; - -public class DuplicateEnabledDICOMReceiverPort extends NrgServiceException { - public DuplicateEnabledDICOMReceiverPort(final String message) { - super(message); - } -} diff --git a/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApi.java b/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApi.java index 04c79ec7..ece0cd4d 100644 --- a/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApi.java +++ b/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApi.java @@ -3,10 +3,10 @@ package org.nrg.xapi.rest.dicomscp; import io.swagger.annotations.*; import org.nrg.dcm.DicomSCP; import org.nrg.dcm.DicomSCPManager; +import org.nrg.dcm.exceptions.EnabledDICOMReceiverWithDuplicatePortException; import org.nrg.dcm.preferences.DicomSCPInstance; import org.nrg.framework.annotations.XapiRestController; import org.nrg.framework.exceptions.NrgServiceException; -import org.nrg.xapi.exceptions.DuplicateEnabledDICOMReceiverPort; import org.nrg.xapi.rest.NotFoundException; import org.nrg.xdat.rest.AbstractXapiRestController; import org.nrg.xdat.security.services.RoleHolder; @@ -19,13 +19,13 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; +import java.io.IOException; import java.util.List; @Api(description = "XNAT DICOM SCP management API") @XapiRestController @RequestMapping(value = "/dicomscp") public class DicomSCPApi extends AbstractXapiRestController { - @Autowired public DicomSCPApi(final DicomSCPManager manager, final UserManagementServiceI userManagementService, final RoleHolder roleHolder) { super(userManagementService, roleHolder); @@ -44,28 +44,18 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiResponses({@ApiResponse(code = 200, message = "The newly created DICOM SCP receiver definition."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to view this DICOM SCP receiver definition."), @ApiResponse(code = 409, message = "A DICOM SCP receiver already exists and is enabled at the same port."), @ApiResponse(code = 500, message = "Unexpected error")}) @RequestMapping(consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE, method = RequestMethod.POST) @ResponseBody - public ResponseEntity<DicomSCPInstance> dicomSCPCreate(@RequestBody final DicomSCPInstance instance) throws DuplicateEnabledDICOMReceiverPort { + public ResponseEntity<DicomSCPInstance> dicomSCPCreate(@RequestBody final DicomSCPInstance instance) throws IOException, EnabledDICOMReceiverWithDuplicatePortException { HttpStatus status = isPermitted(); if (status != null) { return new ResponseEntity<>(status); } - try { - _manager.create(instance); - } catch (final NrgServiceException e) { - switch (e.getServiceError()) { - case AlreadyInitialized: - throw new DuplicateEnabledDICOMReceiverPort(e.getMessage()); - case Unknown: - _log.error("An error occurred trying to create a new DICOM SCP instance", e); - return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); - } - } + _manager.create(instance); return new ResponseEntity<>(instance, HttpStatus.OK); } @ApiOperation(value = "Gets the DICOM SCP receiver definition with the specified ID.", notes = "Returns the DICOM SCP receiver definition with the specified ID.", response = DicomSCPInstance.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver definition successfully retrieved."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to view this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.GET}) + @RequestMapping(value = {"{id}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.GET}) public ResponseEntity<DicomSCPInstance> dicomSCPInstanceGet(@ApiParam(value = "ID of the DICOM SCP receiver definition to fetch", required = true) @PathVariable("id") final int id) { HttpStatus status = isPermitted(); if (status != null) { @@ -76,23 +66,23 @@ public class DicomSCPApi extends AbstractXapiRestController { } @ApiOperation(value = "Updates the DICOM SCP receiver definition object with the specified ID.", notes = "Returns the updated DICOM SCP receiver definition.", response = DicomSCPInstance.class) - @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver definition successfully created or updated."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to create or update this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) - public ResponseEntity<DicomSCPInstance> dicomSCPInstanceCreateOrUpdate(@ApiParam(value = "The ID of the DICOM SCP receiver definition to create or update.", required = true) @PathVariable("id") final int id, @RequestBody final DicomSCPInstance instance) throws NotFoundException { + @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver definition successfully updated."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to create or update this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) + @RequestMapping(value = {"{id}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) + public ResponseEntity<DicomSCPInstance> dicomSCPInstanceCreateOrUpdate(@ApiParam(value = "The ID of the DICOM SCP receiver definition to update.", required = true) @PathVariable("id") final int id, @RequestBody final DicomSCPInstance instance) throws NotFoundException, EnabledDICOMReceiverWithDuplicatePortException { HttpStatus status = isPermitted(); if (status != null) { return new ResponseEntity<>(status); } - if (!_manager.hasDicomSCP(id)) { - try { - _manager.create(instance); - } catch (NrgServiceException e) { - _log.error("An error occurred trying to create a new DICOM SCP instance", e); - return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); - } - } else { + + if (_manager.hasDicomSCP(id)) { + // Set the ID to the value specified in the REST call. If ID not specified on PUT, value will be zero, so we + // need to make sure it's set to the proper value. If they submit it under the wrong ID well... + instance.setId(id); _manager.setDicomSCPInstance(instance); + } else { + return new ResponseEntity<>(HttpStatus.NOT_FOUND); } + if(instance.isEnabled()){ _manager.startDicomSCP(id); } @@ -104,7 +94,7 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Deletes the DICOM SCP receiver definition object with the specified ID.", notes = "This call will stop the receiver if it's currently running.", response = Void.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver definition successfully created or updated."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to delete this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.DELETE}) + @RequestMapping(value = {"{id}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.DELETE}) public ResponseEntity<Void> dicomSCPInstanceDelete(@ApiParam(value = "The ID of the DICOM SCP receiver definition to delete.", required = true) @PathVariable("id") final int id) throws NotFoundException { HttpStatus status = isPermitted(); if (status != null) { @@ -124,7 +114,7 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Returns whether the DICOM SCP receiver definition with the specified ID is enabled.", notes = "Returns true or false based on whether the specified DICOM SCP receiver definition is enabled or not.", response = Boolean.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver definition enabled status successfully retrieved."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to view this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}/enabled"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.GET}) + @RequestMapping(value = {"{id}/enabled"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.GET}) public ResponseEntity<Boolean> dicomSCPInstanceEnabledGet(@ApiParam(value = "The ID of the DICOM SCP receiver definition to retrieve the enabled status for.", required = true) @PathVariable("id") final int id) { HttpStatus status = isPermitted(); if (status != null) { @@ -136,17 +126,17 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Sets the DICOM SCP receiver definition's enabled state.", notes = "Sets the enabled state of the DICOM SCP receiver definition with the specified ID to the value of the flag parameter.", response = Void.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver definition enabled status successfully set."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to enable or disable this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}/enabled/{flag}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) + @RequestMapping(value = {"{id}/enabled/{flag}"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) public ResponseEntity<Void> dicomSCPInstanceSetEnabledFlag(@ApiParam(value = "ID of the DICOM SCP receiver definition to modify", required = true) @PathVariable("id") final int id, - @ApiParam(value = "The value to set for the enabled status.", required = true) @PathVariable("flag") final Boolean flag) { - HttpStatus status = isPermitted(); + @ApiParam(value = "The value to set for the enabled status.", required = true) @PathVariable("flag") final Boolean flag) throws EnabledDICOMReceiverWithDuplicatePortException { + final HttpStatus status = isPermitted(); if (status != null) { return new ResponseEntity<>(status); } if (!_manager.hasDicomSCP(id)) { return new ResponseEntity<>(HttpStatus.NOT_FOUND); } - DicomSCPInstance instanceToChange = _manager.getDicomSCPInstance(id); + final DicomSCPInstance instanceToChange = _manager.getDicomSCPInstance(id); instanceToChange.setEnabled(flag); _manager.setDicomSCPInstance(instanceToChange); @@ -161,7 +151,7 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Starts all enabled DICOM SCP receivers.", notes = "This starts all enabled DICOM SCP receivers. The return value contains the definitions of all of the started receivers.", responseContainer = "List", response = DicomSCP.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receivers successfully started."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to enable or disable this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/start"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) + @RequestMapping(value = {"start"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) public ResponseEntity<List<DicomSCP>> dicomSCPInstancesStart() { HttpStatus status = isPermitted(); if (status != null) { @@ -172,7 +162,7 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Starts the DICOM SCP receiver.", notes = "This starts the DICOM SCP receiver. Note that this will start the receiver regardless of its enabled or disabled setting. This returns true if the instance was started and false if not.", response = Boolean.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver successfully started."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to enable or disable this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}/start"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) + @RequestMapping(value = {"{id}/start"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) public ResponseEntity<Boolean> dicomSCPInstanceStart(@ApiParam(value = "ID of the DICOM SCP receiver to start.", required = true) @PathVariable("id") final int id) { HttpStatus status = isPermitted(); if (status != null) { @@ -187,7 +177,7 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Stops all enabled DICOM SCP receivers.", notes = "This stops all enabled DICOM SCP receivers. The return value contains the definitions of all of the started receivers.", responseContainer = "List", response = DicomSCP.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receivers successfully stopped."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to enable or disable this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/stop"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) + @RequestMapping(value = {"stop"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) public ResponseEntity<List<DicomSCP>> dicomSCPInstancesStop() { HttpStatus status = isPermitted(); if (status != null) { @@ -198,7 +188,7 @@ public class DicomSCPApi extends AbstractXapiRestController { @ApiOperation(value = "Stops the DICOM SCP receiver.", notes = "This stops the DICOM SCP receiver. Note that this will stop the receiver regardless of its enabled or disabled setting. This returns true if the instance was stopped and false if not.", response = Boolean.class) @ApiResponses({@ApiResponse(code = 200, message = "DICOM SCP receiver successfully stopped."), @ApiResponse(code = 401, message = "Must be authenticated to access the XNAT REST API."), @ApiResponse(code = 403, message = "Not authorized to enable or disable this DICOM SCP receiver definition."), @ApiResponse(code = 404, message = "DICOM SCP receiver definition not found."), @ApiResponse(code = 500, message = "Unexpected error")}) - @RequestMapping(value = {"/{id}/stop"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) + @RequestMapping(value = {"{id}/stop"}, produces = MediaType.APPLICATION_JSON_VALUE, method = {RequestMethod.PUT}) public ResponseEntity<Boolean> dicomSCPInstanceStop(@ApiParam(value = "ID of the DICOM SCP receiver to stop.", required = true) @PathVariable("id") final int id) { HttpStatus status = isPermitted(); if (status != null) { diff --git a/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApiAdvice.java b/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApiAdvice.java index 7e80a118..910e7a40 100644 --- a/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApiAdvice.java +++ b/src/main/java/org/nrg/xapi/rest/dicomscp/DicomSCPApiAdvice.java @@ -1,7 +1,7 @@ package org.nrg.xapi.rest.dicomscp; +import org.nrg.dcm.exceptions.EnabledDICOMReceiverWithDuplicatePortException; import org.nrg.framework.exceptions.NrgServiceException; -import org.nrg.xapi.exceptions.DuplicateEnabledDICOMReceiverPort; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; @@ -9,8 +9,8 @@ import org.springframework.web.bind.annotation.ExceptionHandler; @ControllerAdvice public class DicomSCPApiAdvice { - @ExceptionHandler(DuplicateEnabledDICOMReceiverPort.class) - public ResponseEntity<String> handleDuplicateEnabledDICOMReceiverPort(final DuplicateEnabledDICOMReceiverPort exception) { + @ExceptionHandler(EnabledDICOMReceiverWithDuplicatePortException.class) + public ResponseEntity<String> handleEnabledDICOMReceiverWithDuplicatePort(final EnabledDICOMReceiverWithDuplicatePortException exception) { return new ResponseEntity<>(exception.getMessage(), HttpStatus.CONFLICT); } diff --git a/src/main/java/org/nrg/xnat/restlet/extensions/DicomSCPRestlet.java b/src/main/java/org/nrg/xnat/restlet/extensions/DicomSCPRestlet.java index 330db9e5..cc2dd960 100644 --- a/src/main/java/org/nrg/xnat/restlet/extensions/DicomSCPRestlet.java +++ b/src/main/java/org/nrg/xnat/restlet/extensions/DicomSCPRestlet.java @@ -14,6 +14,7 @@ package org.nrg.xnat.restlet.extensions; import com.google.common.base.Joiner; import org.apache.commons.lang3.StringUtils; import org.nrg.dcm.DicomSCPManager; +import org.nrg.dcm.exceptions.EnabledDICOMReceiverWithDuplicatePortException; import org.nrg.dcm.preferences.DicomSCPInstance; import org.nrg.framework.exceptions.NrgServiceError; import org.nrg.framework.exceptions.NrgServiceException; @@ -150,7 +151,11 @@ public class DicomSCPRestlet extends SecureResource { if (_scpId == null) { getResponse().setStatus(Status.CLIENT_ERROR_BAD_REQUEST, "You must specify a specific DICOM SCP instance to enable."); } else { - _dicomSCPManager.enableDicomSCP(_scpId); + try { + _dicomSCPManager.enableDicomSCP(_scpId); + } catch (EnabledDICOMReceiverWithDuplicatePortException e) { + getResponse().setStatus(Status.CLIENT_ERROR_BAD_REQUEST, "There is already another DICOM SCP instance enabled with the same port: " + e.getExisting().toString()); + } returnDefaultRepresentation(); } } else if (_action.equalsIgnoreCase("disable")) { -- GitLab