From 0f36e8d19946dee121fd5c2696f603227314cb6d Mon Sep 17 00:00:00 2001 From: zack-rma Date: Fri, 15 Nov 2024 08:54:41 -0800 Subject: [PATCH] Updates to tests and error handling following feedback --- .../cwms/cda/api/LocationGroupController.java | 7 ++-- .../cda/api/TimeSeriesGroupController.java | 33 +++++++++++++++---- .../api/LocationCategoryControllerTestIT.java | 14 -------- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/LocationGroupController.java b/cwms-data-api/src/main/java/cwms/cda/api/LocationGroupController.java index 05f66840f..e2f292dac 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/LocationGroupController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/LocationGroupController.java @@ -227,10 +227,9 @@ public void create(@NotNull Context ctx) { if (!deserialize.getLocationCategory().getOfficeId().equalsIgnoreCase(CWMS_OFFICE) && (!deserialize.getOfficeId().equalsIgnoreCase(deserialize.getLocationCategory().getOfficeId()) || deserialize.getOfficeId().equalsIgnoreCase(CWMS_OFFICE))) { - CdaError re = new CdaError("Office ID cannot be CWMS and must match the location category office ID"); - logger.info(() -> re + System.lineSeparator() + "for request " + ctx.fullUrl()); - ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); - return; + CdaError re = new CdaError("Location Group office ID cannot be CWMS and must match the " + + "Location Category office ID"); + throw new IllegalArgumentException(re.toString()); } LocationGroupDao dao = new LocationGroupDao(dsl); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesGroupController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesGroupController.java index a9691a753..91606e973 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesGroupController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesGroupController.java @@ -25,7 +25,28 @@ package cwms.cda.api; import static com.codahale.metrics.MetricRegistry.name; -import static cwms.cda.api.Controllers.*; +import static cwms.cda.api.Controllers.CATEGORY_ID; +import static cwms.cda.api.Controllers.CATEGORY_OFFICE_ID; +import static cwms.cda.api.Controllers.CREATE; +import static cwms.cda.api.Controllers.CWMS_OFFICE; +import static cwms.cda.api.Controllers.FAIL_IF_EXISTS; +import static cwms.cda.api.Controllers.GET_ALL; +import static cwms.cda.api.Controllers.GET_ONE; +import static cwms.cda.api.Controllers.GROUP_ID; +import static cwms.cda.api.Controllers.GROUP_OFFICE_ID; +import static cwms.cda.api.Controllers.INCLUDE_ASSIGNED; +import static cwms.cda.api.Controllers.OFFICE; +import static cwms.cda.api.Controllers.REPLACE_ASSIGNED_TS; +import static cwms.cda.api.Controllers.RESULTS; +import static cwms.cda.api.Controllers.SIZE; +import static cwms.cda.api.Controllers.STATUS_200; +import static cwms.cda.api.Controllers.STATUS_404; +import static cwms.cda.api.Controllers.STATUS_501; +import static cwms.cda.api.Controllers.TIMESERIES_CATEGORY_LIKE; +import static cwms.cda.api.Controllers.TIMESERIES_GROUP_LIKE; +import static cwms.cda.api.Controllers.UPDATE; +import static cwms.cda.api.Controllers.queryParamAsClass; +import static cwms.cda.api.Controllers.requiredParam; import static cwms.cda.data.dao.JooqDao.getDslContext; import com.codahale.metrics.Histogram; @@ -237,10 +258,9 @@ public void create(@NotNull Context ctx) { if (!deserialize.getTimeSeriesCategory().getOfficeId().equalsIgnoreCase(CWMS_OFFICE) && (!deserialize.getOfficeId().equalsIgnoreCase(deserialize.getTimeSeriesCategory().getOfficeId()) || deserialize.getOfficeId().equalsIgnoreCase(CWMS_OFFICE))) { - CdaError re = new CdaError("Office ID cannot be CWMS and must match the TimeSeries category office ID"); - logger.info(() -> re + System.lineSeparator() + "for request " + ctx.fullUrl()); - ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); - return; + CdaError re = new CdaError("TimeSeries Group office ID cannot be CWMS and must match the " + + "TimeSeries Category office ID"); + throw new IllegalArgumentException(re.toString()); } boolean failIfExists = ctx.queryParamAsClass(FAIL_IF_EXISTS, Boolean.class).getOrDefault(true); @@ -264,7 +284,8 @@ public void create(@NotNull Context ctx) { + "Default: false"), @OpenApiParam(name = OFFICE, required = true, description = "Specifies the " + "office of the user making the request. This is the office that the timeseries, group, and category " - + "belong to. If the group and/or category belong to the CWMS office, this only identifies the timeseries."), + + "belong to. If the group and/or category belong to the CWMS office, " + + "this only identifies the timeseries."), }, method = HttpMethod.PATCH, tags = {TAG} diff --git a/cwms-data-api/src/test/java/cwms/cda/api/LocationCategoryControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/LocationCategoryControllerTestIT.java index 117a5723d..5d6887564 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/LocationCategoryControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/LocationCategoryControllerTestIT.java @@ -141,20 +141,6 @@ void test_create_already_existing_CWMS_category() { .log().ifValidationFails(LogDetail.ALL,true) .assertThat() .statusCode(is(HttpServletResponse.SC_CONFLICT)); - //Read Empty - given() - .log().ifValidationFails(LogDetail.ALL,true) - .accept(Formats.JSON) - .contentType(Formats.JSON) - .queryParam("office", officeId) - .when() - .redirects().follow(true) - .redirects().max(3) - .get("/location/category/" + cat.getId()) - .then() - .log().ifValidationFails(LogDetail.ALL,true) - .assertThat() - .statusCode(is(HttpServletResponse.SC_NOT_FOUND)); } @Test