From ff7a67afc97c3bdfce6791a3d4d8359c76a72aea Mon Sep 17 00:00:00 2001 From: Albert Louis Rossi Date: Wed, 17 May 2023 08:36:54 -0500 Subject: [PATCH] dcache-bulk,dcache-frontend: distinguish "Not Found" from "Invalid" request Motivation: See https://github.com/dCache/dcache/issues/7165 REST API (Frontend): non-existent request returns 400 instead of 404 Modification: Check existence of the request before permissions. Result: Correct error code is returned. Target: master Request: 9.0 Request: 8.2 Closes: #7165 Patch: https://rb.dcache.org/r/13984/ Requires-notes: yes Acked-by: Tigran --- .../java/org/dcache/services/bulk/BulkService.java | 12 ++++++++++++ .../store/jdbc/request/JdbcBulkRequestStore.java | 3 +-- .../dcache/restful/resources/bulk/BulkResources.java | 1 + .../restful/resources/tape/StageResources.java | 2 ++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/BulkService.java b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/BulkService.java index 0221c57399e..f44180d367d 100644 --- a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/BulkService.java +++ b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/BulkService.java @@ -209,6 +209,10 @@ public Reply messageArrived(BulkRequestStatusMessage message) { try { Subject subject = message.getSubject(); String uuid = message.getRequestUuid(); + /* + * First check to see if the request corresponds to a stored one. + */ + requestStore.getKey(uuid); checkRestrictions(message.getRestriction(), uuid); matchActivity(message.getActivity(), uuid); BulkRequestInfo status = requestStore.getRequestInfo(subject, uuid, @@ -236,6 +240,10 @@ public Reply messageArrived(BulkRequestCancelMessage message) { try { Subject subject = message.getSubject(); String uuid = message.getRequestUuid(); + /* + * First check to see if the request corresponds to a stored one. + */ + requestStore.getKey(uuid); checkRestrictions(message.getRestriction(), uuid); matchActivity(message.getActivity(), uuid); List targetPaths = message.getTargetPaths(); @@ -267,6 +275,10 @@ public Reply messageArrived(BulkRequestClearMessage message) { try { String uuid = message.getRequestUuid(); Subject subject = message.getSubject(); + /* + * First check to see if the request corresponds to a stored one. + */ + requestStore.getKey(uuid); checkRestrictions(message.getRestriction(), uuid); matchActivity(message.getActivity(), uuid); submissionHandler.clearRequest(subject, uuid, message.isCancelIfRunning()); diff --git a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java index 75c7b92cb68..d732452c81d 100644 --- a/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java +++ b/modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java @@ -816,8 +816,7 @@ private BulkRequestTargetInfo toRequestTargetInfo(BulkRequestTarget target, FsP private BulkRequest valid(String uid) throws BulkStorageException { BulkRequest stored = get(uid); if (stored == null) { - String error = "request id " + uid + " is no longer valid!"; - throw new BulkRequestNotFoundException(error); + throw new BulkRequestNotFoundException("request " + uid + " not found"); } return stored; } diff --git a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java index b5a8ad3cc59..17a5039f79a 100644 --- a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java +++ b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java @@ -374,6 +374,7 @@ public Response update(@ApiParam("The unique id of the request.") @ApiResponse(code = 400, message = "Bad request"), @ApiResponse(code = 401, message = "Unauthorized"), @ApiResponse(code = 403, message = "Forbidden"), + @ApiResponse(code = 404, message = "Not Found"), @ApiResponse(code = 500, message = "Internal Server Error") }) @Path("/{id}") diff --git a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java index 61583b633e0..27b86788675 100644 --- a/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java +++ b/modules/dcache-frontend/src/main/java/org/dcache/restful/resources/tape/StageResources.java @@ -197,6 +197,7 @@ public StageRequestInfo getStageInfo(@ApiParam("The unique id of the request.") @ApiResponse(code = 400, message = "Bad request"), @ApiResponse(code = 401, message = "Unauthorized"), @ApiResponse(code = 403, message = "Forbidden"), + @ApiResponse(code = 404, message = "Not Found"), @ApiResponse(code = 429, message = "Too many requests"), @ApiResponse(code = 500, message = "Internal Server Error") }) @@ -322,6 +323,7 @@ public Response submit( @ApiResponse(code = 400, message = "Bad request"), @ApiResponse(code = 401, message = "Unauthorized"), @ApiResponse(code = 403, message = "Forbidden"), + @ApiResponse(code = 404, message = "Not Found"), @ApiResponse(code = 500, message = "Internal Server Error") }) @Path("/{id}")