diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java index 028da1e38..6eba3b795 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java +++ b/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"); } } \ No newline at end of file diff --git a/cwms-data-api/src/main/java/cwms/cda/api/Controllers.java b/cwms-data-api/src/main/java/cwms/cda/api/Controllers.java index c12b0fae9..99589b592 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/Controllers.java +++ b/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 requiredParamAs(io.javalin.http.Context ctx, String name, Class 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; diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/basin/BasinDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/basin/BasinDao.java index 4d30d2c87..00bfb622c 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/basin/BasinDao.java +++ b/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()); }); } diff --git a/cwms-data-api/src/test/java/cwms/cda/api/BasinControllerIT.java b/cwms-data-api/src/test/java/cwms/cda/api/BasinControllerIT.java index f761088d6..c002f6edc 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/BasinControllerIT.java +++ b/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) diff --git a/cwms-data-api/src/test/java/cwms/cda/api/ControllersTest.java b/cwms-data-api/src/test/java/cwms/cda/api/ControllersTest.java index e3c589532..2b07b7a2c 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/ControllersTest.java +++ b/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 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()); + assertEquals("the_value", Controllers.requiredParamAs(ctx, "boring", String.class)); + assertThrows(RequiredQueryParameterException.class, + () -> Controllers.requiredParamAs(ctx, Controllers.OFFICE, String.class)); + } } \ No newline at end of file diff --git a/cwms-data-api/src/test/java/cwms/cda/data/dao/basin/BasinDaoIT.java b/cwms-data-api/src/test/java/cwms/cda/data/dao/basin/BasinDaoIT.java index c937643ce..a13110512 100644 --- a/cwms-data-api/src/test/java/cwms/cda/data/dao/basin/BasinDaoIT.java +++ b/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 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); }