bugfixes for basin controller (USACE#825)

standardize on delete "method" query parameter name fix getOne
serialization that was using an array
fix rename to not require a body

cwms-data-api/src/main/java/cwms/cda/api/BasinController.java

@@ -24,22 +24,25 @@
 
 package cwms.cda.api;
 
+import static cwms.cda.api.Controllers.METHOD;
 import static cwms.cda.api.Controllers.STATUS_200;
 import static cwms.cda.api.Controllers.STATUS_204;
 import static cwms.cda.api.Controllers.STATUS_404;
 import static cwms.cda.api.Controllers.STATUS_501;
-import static cwms.cda.api.Controllers.DELETE_MODE;
 import static cwms.cda.api.Controllers.GET_ALL;
 import static cwms.cda.api.Controllers.GET_ONE;
 import static cwms.cda.api.Controllers.NAME;
 import static cwms.cda.api.Controllers.OFFICE;
 import static cwms.cda.api.Controllers.UNIT;
+import static cwms.cda.api.Controllers.requiredParam;
+import static cwms.cda.api.Controllers.requiredParamAs;
 import static cwms.cda.data.dao.JooqDao.getDslContext;
 
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.Timer;
 import cwms.cda.api.enums.UnitSystem;
 import cwms.cda.api.errors.CdaError;
+import cwms.cda.data.dao.JooqDao;
 import cwms.cda.data.dao.basinconnectivity.BasinDao;
 import cwms.cda.data.dto.CwmsId;
 import cwms.cda.data.dto.basinconnectivity.Basin;
@@ -184,6 +187,7 @@ public void getOne(@NotNull Context ctx, @NotNull String name) {
 
         try (final Timer.Context ignored = markAndTime(GET_ONE)) {
             DSLContext dsl = getDslContext(ctx);
+
             String units =
                     ctx.queryParamAsClass(UNIT, String.class).getOrDefault(UnitSystem.EN.value());
             String office = ctx.queryParam(OFFICE);
@@ -206,8 +210,7 @@ public void getOne(@NotNull Context ctx, @NotNull String name) {
                         .withOfficeId(office)
                         .build();
                 cwms.cda.data.dto.basin.Basin basin = basinDao.getBasin(basinId, units);
-                result = Formats.format(contentType, Collections.singletonList(basin),
-                        cwms.cda.data.dto.basin.Basin.class);
+                result = Formats.format(contentType, basin);
                 ctx.result(result);
                 ctx.status(HttpServletResponse.SC_OK);
             }
@@ -224,6 +227,8 @@ public void getOne(@NotNull Context ctx, @NotNull String name) {
             @OpenApiParam(name = NAME, required = true, description = "Specifies the new name for the basin.")
         },
         pathParams = {
+            @OpenApiParam(name = OFFICE, required = true, description = "Specifies the owning office of "
+                + "the basin to be renamed."),
             @OpenApiParam(name = NAME, description = "Specifies the name of "
                     + "the basin to be renamed.")
         },
@@ -233,35 +238,26 @@ public void getOne(@NotNull Context ctx, @NotNull String name) {
             @OpenApiResponse(status = STATUS_501, description = "Requested format is not "
                     + "implemented")
         },
+        method = HttpMethod.PATCH,
         description = "Renames CWMS Basin",
         tags = {TAG}
     )
     @Override
     public void update(@NotNull Context ctx, @NotNull String name) {
         DSLContext dsl = getDslContext(ctx);
-        String formatHeader = ctx.header(Header.ACCEPT) != null ? ctx.header(Header.ACCEPT) :
-                Formats.JSONV1;
-        ContentType contentType = Formats.parseHeader(formatHeader, Basin.class);
-        ctx.contentType(contentType.toString());
-
-        cwms.cda.data.dto.basin.Basin basin = Formats.parseContent(contentType, ctx.body(),
-                cwms.cda.data.dto.basin.Basin.class);
-
-        if (contentType.getType().equals(Formats.NAMED_PGJSON)) {
-            ctx.status(HttpCode.NOT_IMPLEMENTED).json(CdaError.notImplemented());
-        } else {
-            cwms.cda.data.dao.basin.BasinDao basinDao = new cwms.cda.data.dao.basin.BasinDao(dsl);
-            CwmsId oldLoc = new CwmsId.Builder()
-                    .withName(name)
-                    .withOfficeId(basin.getBasinId().getOfficeId())
-                    .build();
-            CwmsId newLoc = new CwmsId.Builder()
-                    .withName(ctx.queryParam(NAME))
-                    .withOfficeId(basin.getBasinId().getOfficeId())
-                    .build();
-            basinDao.renameBasin(oldLoc, newLoc);
-            ctx.status(HttpServletResponse.SC_OK).json("Updated Location");
-        }
+        String officeId = requiredParam(ctx, OFFICE);
+        String newStreamId = requiredParam(ctx, NAME);
+        cwms.cda.data.dao.basin.BasinDao basinDao = new cwms.cda.data.dao.basin.BasinDao(dsl);
+        CwmsId oldLoc = new CwmsId.Builder()
+            .withName(name)
+            .withOfficeId(officeId)
+            .build();
+        CwmsId newLoc = new CwmsId.Builder()
+            .withName(newStreamId)
+            .withOfficeId(officeId)
+            .build();
+        basinDao.renameBasin(oldLoc, newLoc);
+        ctx.status(HttpServletResponse.SC_OK).json("Updated Location");
     }
 
     @OpenApi(
@@ -294,7 +290,8 @@ public void create(@NotNull Context ctx) {
         queryParams = {
             @OpenApiParam(name = OFFICE, required = true, description = "Specifies the"
                     + " owning office of the basin to be renamed."),
-            @OpenApiParam(name = DELETE_MODE, required = true, description = "Specifies the delete method used.")
+            @OpenApiParam(name = METHOD, required = true, description = "Specifies the delete method used.",
+                type = JooqDao.DeleteMethod.class)
         },
         pathParams = {
             @OpenApiParam(name = NAME, description = "Specifies the name of "
@@ -312,19 +309,13 @@ public void create(@NotNull Context ctx) {
     @Override
     public void delete(@NotNull Context ctx, @NotNull String name) {
         DSLContext dsl = getDslContext(ctx);
-        String deleteMethod = ctx.queryParam(DELETE_MODE);
+        JooqDao.DeleteMethod deleteMethod = requiredParamAs(ctx, METHOD, JooqDao.DeleteMethod.class);
         cwms.cda.data.dao.basin.BasinDao basinDao = new cwms.cda.data.dao.basin.BasinDao(dsl);
         CwmsId basinId = new CwmsId.Builder()
                 .withName(name)
                 .withOfficeId(ctx.queryParam(OFFICE))
                 .build();
-        cwms.cda.data.dto.basin.Basin retBasin = basinDao.getBasin(basinId, "EN");
-        if (retBasin == null) {
-            CdaError error = new CdaError("No matching basin " + name);
-            ctx.status(HttpServletResponse.SC_NOT_FOUND).json(error);
-            return;
-        }
-        basinDao.deleteBasin(basinId, deleteMethod);
+        basinDao.deleteBasin(basinId, deleteMethod.getRule());
         ctx.status(HttpServletResponse.SC_NO_CONTENT).json(basinId.getName() + " Deleted");
     }
 }

cwms-data-api/src/main/java/cwms/cda/api/Controllers.java

@@ -164,7 +164,6 @@ public final class Controllers {
     public static final String STATUS_501 = "501";
     public static final String STATUS_400 = "400";
     public static final String TEXT_MASK = "text-mask";
-    public static final String DELETE_MODE = "delete-mode";
     public static final String MIN_ATTRIBUTE = "min-attribute";
     public static final String MAX_ATTRIBUTE = "max-attribute";
     public static final String STANDARD_TEXT_ID_MASK = "standard-text-id-mask";
@@ -345,6 +344,18 @@ public static String requiredParam(io.javalin.http.Context ctx, String name) {
         return param;
     }
 
+    /**
+     * Returns the first matching query param or throws RequiredQueryParameterException.
+     * @param ctx Request Context
+     * @param name Query parameter name
+     * @return value of the parameter
+     * @throws RequiredQueryParameterException if the parameter is not found
+     */
+    public static <T> T requiredParamAs(io.javalin.http.Context ctx, String name, Class<T> type) {
+        return ctx.queryParamAsClass(name, type)
+            .getOrThrow(e -> new RequiredQueryParameterException(name));
+    }
+
     @Nullable
     public static ZonedDateTime queryParamAsZdt(Context ctx, String param, String timezone) {
         ZonedDateTime beginZdt = null;

cwms-data-api/src/main/java/cwms/cda/data/dao/basin/BasinDao.java

@@ -24,6 +24,7 @@
 
 package cwms.cda.data.dao.basin;
 
+import cwms.cda.data.dao.DeleteRule;
 import cwms.cda.data.dao.JooqDao;
 import cwms.cda.data.dto.CwmsId;
 import cwms.cda.data.dto.basin.Basin;
@@ -128,12 +129,13 @@ public void renameBasin(CwmsId oldBasin, CwmsId newBasin) {
 
     }
 
-    public void deleteBasin(CwmsId basinId, String deleteAction) {
+    public void deleteBasin(CwmsId basinId, DeleteRule deleteAction) {
         basinId.validate();
 
         connection(dsl, c -> {
             setOffice(c, basinId.getOfficeId());
-            CWMS_BASIN_PACKAGE.call_DELETE_BASIN(DSL.using(c).configuration(), basinId.getName(), deleteAction, basinId.getOfficeId());
+            CWMS_BASIN_PACKAGE.call_DELETE_BASIN(DSL.using(c).configuration(), basinId.getName(),
+                deleteAction.getRule(), basinId.getOfficeId());
         });
     }
 

cwms-data-api/src/test/java/cwms/cda/api/BasinControllerIT.java

@@ -25,6 +25,7 @@
 package cwms.cda.api;
 
 import cwms.cda.api.enums.Nation;
+import cwms.cda.data.dao.DeleteRule;
 import cwms.cda.data.dao.LocationsDaoImpl;
 import cwms.cda.data.dao.basin.BasinDao;
 import cwms.cda.data.dto.Location;
@@ -61,7 +62,6 @@ class BasinControllerIT extends DataApiTestIT
 	private static final String OFFICE = "SWT";
 	private static final Basin BASIN;
 	private static final Basin BASIN_CONNECT;
-	private static final String DELETE_ACTION = "DELETE ALL";
 	static {
 		try {
 			BASIN = new Basin.Builder()
@@ -110,7 +110,8 @@ public static void setup() throws Exception {
 					.withActive(true)
 					.withNearestCity("Davis")
 					.build();
-			Location loc2 = new Location.Builder(BASIN_CONNECT.getBasinId().getOfficeId(), BASIN_CONNECT.getBasinId().getName())
+			Location loc2 = new Location.Builder(BASIN_CONNECT.getBasinId().getOfficeId(),
+				BASIN_CONNECT.getBasinId().getName())
 					.withStateInitial("CO")
 					.withNation(Nation.US)
 					.withLocationKind("BASIN")
@@ -143,7 +144,7 @@ public static void tearDown() throws Exception {
 			LocationsDaoImpl locationsDao = new LocationsDaoImpl(ctx);
 			BasinDao basinDao = new BasinDao(ctx);
 			locationsDao.deleteLocation(BASIN.getBasinId().getName(), OFFICE, true);
-			basinDao.deleteBasin(BASIN_CONNECT.getBasinId(), DELETE_ACTION);
+			basinDao.deleteBasin(BASIN_CONNECT.getBasinId(), DeleteRule.DELETE_ALL);
 			locationsDao.deleteLocation(BASIN_CONNECT.getBasinId().getName(), OFFICE, true);
 		}, CwmsDataApiSetupCallback.getWebUser());
 	}
@@ -262,12 +263,12 @@ void test_get_create_delete() {
 				.log().ifValidationFails(LogDetail.ALL, true)
 			.assertThat()
 				.statusCode(is(HttpServletResponse.SC_OK))
-				.body("basin-id[0].name", equalTo(BASIN.getBasinId().getName()))
-				.body("basin-id[0].office-id", equalTo(BASIN.getBasinId().getOfficeId()))
-				.body("[0].sort-order", equalTo(BASIN.getSortOrder().floatValue()))
-				.body("[0].area-unit", equalTo(BASIN.getAreaUnit()))
-				.body("[0].total-drainage-area", equalTo(BASIN.getTotalDrainageArea().floatValue()))
-				.body("[0].contributing-drainage-area", equalTo(BASIN.getContributingDrainageArea().floatValue()))
+				.body("basin-id.name", equalTo(BASIN.getBasinId().getName()))
+				.body("basin-id.office-id", equalTo(BASIN.getBasinId().getOfficeId()))
+				.body("sort-order", equalTo(BASIN.getSortOrder().floatValue()))
+				.body("area-unit", equalTo(BASIN.getAreaUnit()))
+				.body("total-drainage-area", equalTo(BASIN.getTotalDrainageArea().floatValue()))
+				.body("contributing-drainage-area", equalTo(BASIN.getContributingDrainageArea().floatValue()))
 			;
 		}
 
@@ -276,7 +277,7 @@ void test_get_create_delete() {
 			.log().ifValidationFails(LogDetail.ALL, true)
 			.accept(Formats.JSONV1)
 			.queryParam(Controllers.OFFICE, OFFICE)
-			.queryParam(DELETE_MODE, DELETE_ACTION)
+			.queryParam(METHOD, DeleteRule.DELETE_ALL.getRule())
 			.header(AUTH_HEADER, user.toHeaderValue())
 		.when()
 			.redirects().follow(true)
@@ -314,6 +315,7 @@ void test_update_does_not_exist() {
 			.accept(Formats.JSONV1)
 			.contentType(Formats.JSONV1)
 			.queryParam(NAME, "newFakeName")
+			.queryParam(Controllers.OFFICE, OFFICE)
 			.body(json)
 			.header(AUTH_HEADER, user.toHeaderValue())
 		.when()
@@ -335,7 +337,7 @@ void test_delete_does_not_exist() {
 			.log().ifValidationFails(LogDetail.ALL, true)
 			.accept(Formats.JSONV1)
 			.queryParam(Controllers.OFFICE, OFFICE)
-			.queryParam(DELETE_MODE, DELETE_ACTION)
+			.queryParam(METHOD, DeleteRule.DELETE_ALL.getRule())
 			.header(AUTH_HEADER, user.toHeaderValue())
 		.when()
 			.redirects().follow(true)
@@ -476,7 +478,7 @@ void test_get_all() {
 			.log().ifValidationFails(LogDetail.ALL, true)
 			.accept(Formats.JSONV1)
 			.queryParam(Controllers.OFFICE, OFFICE)
-			.queryParam(DELETE_MODE, DELETE_ACTION)
+			.queryParam(METHOD, DeleteRule.DELETE_ALL.getRule())
 			.header(AUTH_HEADER, user.toHeaderValue())
 		.when()
 			.redirects().follow(true)

cwms-data-api/src/test/java/cwms/cda/api/ControllersTest.java

@@ -399,4 +399,18 @@ void testAliasedQueryParamAsType(){
         // if its present it should work
         assertEquals(JooqDao.DeleteMethod.DELETE_KEY, Controllers.queryParamAsClass(ctx, JooqDao.DeleteMethod.class, null, "Not a key", Controllers.METHOD));
     }
+
+    @Test
+    void testRequiredParamAs() {
+        HttpServletRequest request = mock(HttpServletRequest.class);
+        HttpServletResponse response = mock(HttpServletResponse.class);
+        Map<String, String> urlParams = new LinkedHashMap<>();
+        urlParams.put("boring", "the_value");
+        String paramStr = ControllerTest.buildParamStr(urlParams);
+        when(request.getQueryString()).thenReturn(paramStr);
+        Context ctx = new Context(request, response, new LinkedHashMap<String, String>());
+        assertEquals("the_value", Controllers.requiredParamAs(ctx, "boring", String.class));
+        assertThrows(RequiredQueryParameterException.class,
+            () -> Controllers.requiredParamAs(ctx, Controllers.OFFICE, String.class));
+    }
 }

cwms-data-api/src/test/java/cwms/cda/data/dao/basin/BasinDaoIT.java

@@ -60,7 +60,6 @@ class BasinDaoIT extends DataApiTestIT {
     private static final String OFFICE_ID = TestAccounts.KeyUser.SWT_NORMAL.getOperatingOffice();
     private static final String UNIT_SYSTEM = "SI";
     private static final String UNITS = "km2";
-    final String DELETE_ACTION = "DELETE ALL";
     private static final ArrayList<Location> locationsCreated = new ArrayList<>();
     private static final Basin parentBasin = new Basin.Builder()
             .withBasinId(new CwmsId.Builder()
@@ -296,7 +295,7 @@ void testDeleteBasin() throws Exception {
                 assertNotNull(basin);
                 BasinDao basinDao = new BasinDao(ctx);
                 basinDao.storeBasin(basin);
-                basinDao.deleteBasin(basin.getBasinId(), DELETE_ACTION);
+                basinDao.deleteBasin(basin.getBasinId(), DeleteRule.DELETE_ALL);
                 CwmsId basinId = basin.getBasinId();
                 assertThrows(NotFoundException.class, () -> basinDao.getBasin(basinId, UNIT_SYSTEM));
             } catch (Exception e) {
@@ -312,7 +311,7 @@ private void cleanUpRoutine(Basin basin) throws Exception {
             DSLContext ctx = getDslContext(c, OFFICE_ID);
             try {
                 BasinDao basinDao = new BasinDao(ctx);
-                basinDao.deleteBasin(basin.getBasinId(), DELETE_ACTION);
+                basinDao.deleteBasin(basin.getBasinId(), DeleteRule.DELETE_ALL);
             } catch (Exception e) {
                 throw new RuntimeException(e);
             }