Merge pull request #95 from folio-org/EDGPATRON-102

EDGPATRON-102: Adding null checking to hold placement
folio-org · Sep 15, 2022 · 406b615 · 406b615
2 parents 7413055 + 0efd63f
commit 406b615
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 0 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,7 @@
+## 4.10.0 IN-PROGRESS
+
+* Hold requests without a JSON body will now trigger a 400 error rather than a 500. (EDGPATRON-102) 
+
 ## 4.9.0 2022-06-15
 
 * Fix header injection security vulnerability when tenant header is present in a request. ([EDGPATRON-89](https://issues.folio.org/browse/EDGPATRON-89))

diff --git a/src/main/java/org/folio/edge/patron/Constants.java b/src/main/java/org/folio/edge/patron/Constants.java
@@ -28,6 +28,7 @@ public class Constants {
   public static final String MSG_ACCESS_DENIED = "Access Denied";
   public static final String MSG_INTERNAL_SERVER_ERROR = "Internal Server Error";
   public static final String MSG_REQUEST_TIMEOUT = "Request to FOLIO timed out";
+  public static final String MSG_HOLD_NOBODY = "No hold data provided";
 
   public static final String FIELD_EXPIRATION_DATE = "expirationDate";
   public static final String FIELD_REQUEST_DATE = "requestDate";

diff --git a/src/main/java/org/folio/edge/patron/PatronHandler.java b/src/main/java/org/folio/edge/patron/PatronHandler.java
@@ -6,6 +6,7 @@
 import static org.folio.edge.patron.Constants.MSG_ACCESS_DENIED;
 import static org.folio.edge.patron.Constants.MSG_INTERNAL_SERVER_ERROR;
 import static org.folio.edge.patron.Constants.MSG_REQUEST_TIMEOUT;
+import static org.folio.edge.patron.Constants.MSG_HOLD_NOBODY;
 import static org.folio.edge.patron.Constants.PARAM_HOLD_ID;
 import static org.folio.edge.patron.Constants.PARAM_INCLUDE_CHARGES;
 import static org.folio.edge.patron.Constants.PARAM_INCLUDE_HOLDS;
@@ -132,6 +133,10 @@ public void handleRenew(RoutingContext ctx) {
   }
 
   public void handlePlaceItemHold(RoutingContext ctx) {
+    if (ctx.body().asJsonObject() == null) {
+      badRequest(ctx, MSG_HOLD_NOBODY);
+      return;
+    }
     final String body = checkDates(ctx.body().asJsonObject());
     handleCommon(ctx,
         new String[] { PARAM_ITEM_ID },
@@ -170,6 +175,10 @@ public void handleCancelHold(RoutingContext ctx) {
   }
 
   public void handlePlaceInstanceHold(RoutingContext ctx) {
+    if (ctx.body().asJsonObject() == null) {
+      badRequest(ctx, MSG_HOLD_NOBODY);
+      return;
+    }
     final String body = checkDates(ctx.body().asJsonObject());
     handleCommon(ctx,
         new String[] { PARAM_INSTANCE_ID },

diff --git a/src/test/java/org/folio/edge/patron/MainVerticleTest.java b/src/test/java/org/folio/edge/patron/MainVerticleTest.java
@@ -11,6 +11,7 @@
 import static org.folio.edge.core.Constants.TEXT_PLAIN;
 import static org.folio.edge.patron.Constants.MSG_ACCESS_DENIED;
 import static org.folio.edge.patron.Constants.MSG_REQUEST_TIMEOUT;
+import static org.folio.edge.patron.Constants.MSG_HOLD_NOBODY;
 import static org.folio.edge.patron.utils.PatronMockOkapi.holdCancellationHoldId;
 import static org.folio.edge.patron.utils.PatronMockOkapi.holdReqId_notFound;
 import static org.folio.edge.patron.utils.PatronMockOkapi.holdReqTs;
@@ -791,6 +792,24 @@ public void testPlaceInstanceHoldRequestTimeout(TestContext context) throws Exce
       .body("errorMessage", is(MSG_REQUEST_TIMEOUT));
   }
 
+  @Test
+  public void testPlaceInstanceHoldNoBody(TestContext context) throws Exception {
+    logger.info("=== Test place instance hold request with no request body ===");
+
+    mockOkapi.setDelay(requestTimeoutMs * 3);
+    RestAssured
+      .with()
+      .contentType(APPLICATION_JSON)
+      .post(
+          String.format("/patron/account/%s/instance/%s/hold?apikey=%s", patronId, instanceId,
+              apiKey))
+      .then()
+      .contentType(APPLICATION_JSON)
+      .statusCode(400)
+      .body("code", is(400))
+      .body("errorMessage", is(MSG_HOLD_NOBODY));
+  }
+
   @Test
   public void testPlaceItemHoldSuccess(TestContext context) throws Exception {
     logger.info("=== Test successful item hold ===");
@@ -1013,6 +1032,23 @@ public void testPlaceItemHoldRequestTimeout(TestContext context) throws Exceptio
       .body("errorMessage", is(MSG_REQUEST_TIMEOUT));
   }
 
+  @Test
+  public void testPlaceItemHoldRequestNoBody(TestContext context) throws Exception {
+    logger.info("=== Test place item hold request with no request body ===");
+
+    RestAssured
+      .with()
+      .contentType(APPLICATION_JSON)
+      .post(
+          String.format("/patron/account/%s/item/%s/hold?apikey=%s", patronId, itemId,
+              apiKey))
+      .then()
+      .contentType(APPLICATION_JSON)
+      .statusCode(400)
+      .body("code", is(400))
+      .body("errorMessage", is(MSG_HOLD_NOBODY));
+  }
+
   @Test
   public void testCancelHoldSuccess(TestContext context) throws Exception {
     logger.info("=== Test cancel hold success ===");