From 663866ac65a9614f8401ef470f56b4665fad45fa Mon Sep 17 00:00:00 2001 From: Kayla Arritola Date: Fri, 12 Dec 2025 18:23:45 -0800 Subject: [PATCH 1/4] Refactor and clean Entity Controller due to clean runner build fail (PR1507) - Fixed EntityControllerTestIT to incorporate unique data per test using test utility helpers. - Removed hard coded static values - Fixed teardown to ensure all test entities cleaned up after testsing. - Added check in EntityController to fix unused documented query param errors --- .../java/cwms/cda/api/EntityController.java | 17 +- .../cwms/cda/api/EntityControllerTestIT.java | 192 +++++++++++------- .../cwms/cda/data/dto/entity_test.json | 46 +++++ 3 files changed, 182 insertions(+), 73 deletions(-) create mode 100644 cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json diff --git a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java index e61554a3c..66bffa06d 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java @@ -182,6 +182,7 @@ public void create(@NotNull Context ctx) { public void update(@NotNull Context ctx, @NotNull String entityId) { try (final Timer.Context ignored = markAndTime(UPDATE)) { DSLContext dsl = getDslContext(ctx); + String officeId = requiredParam(ctx, OFFICE); String formatHeader = ctx.req.getContentType(); ContentType contentType = Formats.parseHeader(formatHeader, Entity.class); Entity entity = Formats.parseContent(contentType, ctx.bodyAsInputStream(), Entity.class); @@ -190,13 +191,27 @@ public void update(@NotNull Context ctx, @NotNull String entityId) { ctx.result("Entity ID and Office ID must be provided in the request body."); return; } + + if (!entityId.equalsIgnoreCase(entity.getId().getName())) { + ctx.status(HttpServletResponse.SC_NOT_FOUND); + ctx.result("Entity not found for the given entity-id."); + return; + } + + if (!officeId.equalsIgnoreCase(entity.getId().getOfficeId())) { + ctx.status(HttpServletResponse.SC_BAD_REQUEST); + ctx.result("Office ID in query parameter must match the Office ID in the request body."); + return; + } + EntityDao dao = new EntityDao(dsl); dao.updateEntity(entity); ctx.status(HttpServletResponse.SC_OK); } } - @OpenApi( + + @OpenApi( description = "Delete CWMS Entity.", pathParams = { @OpenApiParam(name = ENTITY_ID, required = true, description = "Specifies the entity ID " + diff --git a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java index 0f7db103d..af2f6e758 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java @@ -1,5 +1,6 @@ package cwms.cda.api; +import com.fasterxml.jackson.databind.ObjectMapper; import cwms.cda.formatters.Formats; import fixtures.TestAccounts; import io.restassured.filter.log.LogDetail; @@ -24,28 +25,30 @@ @Tag("integration") final class EntityControllerTestIT extends DataApiTestIT { private static final String OFFICE_ID = TestAccounts.KeyUser.SPK_NORMAL.getOperatingOffice(); - private static final String ENTITY_ID = "NWS"; - private static final String CASCADE_DELETE = "true"; - private static final String PARENT_ID = "NOAA"; + private static final String OFFICE = Controllers.OFFICE; + private static final String CASCADE_DELETE = Controllers.CASCADE_DELETE; @AfterEach - void tearDown() { - given() - .accept(Formats.JSONV2) - .queryParam(Controllers.OFFICE, OFFICE_ID) - .queryParam(Controllers.CASCADE_DELETE, true) - .header(AUTH_HEADER, TestAccounts.KeyUser.SPK_NORMAL.toHeaderValue()) - .when() - .redirects().follow(true) - .redirects().max(3) - .delete("/entity/" + ENTITY_ID) - .then() - .statusCode(isOneOf( - HttpServletResponse.SC_NO_CONTENT, - HttpServletResponse.SC_NOT_FOUND, - HttpServletResponse.SC_BAD_REQUEST - )); + void tearDown() throws Exception { + + for (Map entityMap : loadTestEntityJsonList()) { + String entityJson = new ObjectMapper().writeValueAsString(entityMap); + String entityName = JsonPath.from(entityJson).getString("id.name"); + given() + .queryParam(OFFICE, OFFICE_ID) + .queryParam(CASCADE_DELETE, true) + .header(AUTH_HEADER, TestAccounts.KeyUser.SPK_NORMAL.toHeaderValue()) + .when() + .redirects().follow(true) + .redirects().max(3) + .delete("/entity/" + entityName) + .then() + .statusCode(isOneOf( + HttpServletResponse.SC_NO_CONTENT, + HttpServletResponse.SC_NOT_FOUND + )); + } } @@ -57,9 +60,9 @@ void tearDown() { void test_entity_create_get_update_delete(String format) throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - InputStream in = this.getClass().getResourceAsStream("/cwms/cda/data/dto/entity.json"); - assertNotNull(in); - String entityJson = IOUtils.toString(in, java.nio.charset.StandardCharsets.UTF_8); + String entityJson = getUniqueTestEntityJsonByIndex(0); + String entityName = JsonPath.from(entityJson).getString("id.name"); + String longName = JsonPath.from(entityJson).getString("long-name"); // CREATE given() @@ -80,64 +83,64 @@ void test_entity_create_get_update_delete(String format) throws Exception { given() .log().ifValidationFails(LogDetail.ALL, true) .accept(format) - .queryParam(Controllers.OFFICE, OFFICE_ID) + .queryParam(OFFICE, OFFICE_ID) .when() .redirects().follow(true) .redirects().max(3) - .get("/entity/" + ENTITY_ID) + .get("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL, true) .assertThat() .statusCode(is(HttpServletResponse.SC_OK)) - .body("id.name", equalTo(ENTITY_ID)) + .body("id.name", equalTo(entityName)) .body("id.office-id", equalTo(OFFICE_ID)) - .body("long-name", equalTo("National Weather Service")); + .body("long-name", equalTo(longName)); // UPDATE — modify long-name to verify persistence String updatedEntityJson = entityJson.replace( - "\"National Weather Service\"", - "\"National Weather Service (Updated)\""); + "\"" + longName + "\"", + "\"Updated long name\""); given() .contentType(Formats.JSONV2) .body(updatedEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) - .queryParam(Controllers.OFFICE, OFFICE_ID) + .queryParam(OFFICE, OFFICE_ID) .when() .redirects().follow(true) .redirects().max(3) - .patch("/entity/" + ENTITY_ID) + .patch("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL,true) .assertThat() .statusCode(is(HttpServletResponse.SC_OK)); - // GET to confirm updated field persisted + // GET to confirm the updated field persisted given() .log().ifValidationFails(LogDetail.ALL, true) .accept(format) - .queryParam(Controllers.OFFICE, OFFICE_ID) + .queryParam(OFFICE, OFFICE_ID) .when() .redirects().follow(true) .redirects().max(3) - .get("/entity/" + ENTITY_ID) + .get("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL, true) .assertThat() .statusCode(is(HttpServletResponse.SC_OK)) - .body("id.name", equalTo(ENTITY_ID)) + .body("id.name", equalTo(entityName)) .body("id.office-id", equalTo(OFFICE_ID)) - .body("long-name", equalTo("National Weather Service (Updated)")); + .body("long-name", equalTo("Updated long name")); // DELETE given() .header(AUTH_HEADER, user.toHeaderValue()) - .queryParam(Controllers.OFFICE, OFFICE_ID) - .queryParam(Controllers.CASCADE_DELETE, CASCADE_DELETE) + .queryParam(OFFICE, OFFICE_ID) + .queryParam(CASCADE_DELETE, true) .when() .redirects().follow(true) .redirects().max(3) - .delete("/entity/" + ENTITY_ID) + .delete("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL,true) .assertThat() @@ -146,11 +149,11 @@ void test_entity_create_get_update_delete(String format) throws Exception { // verify deleted given() .accept(format) - .queryParam(Controllers.OFFICE, OFFICE_ID) + .queryParam(OFFICE, OFFICE_ID) .when() .redirects().follow(true) .redirects().max(3) - .get("/entity/" + ENTITY_ID) + .get("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL, true) .assertThat() @@ -162,15 +165,13 @@ void test_entity_create_get_update_delete(String format) throws Exception { @Test void create_duplicate_entity_bad_request() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - InputStream in = this.getClass().getResourceAsStream("/cwms/cda/data/dto/entity.json"); - assertNotNull(in); - String entityJson = IOUtils.toString(in, java.nio.charset.StandardCharsets.UTF_8); + String entityJson1 = getUniqueTestEntityJsonByIndex(1); // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson) + .body(entityJson1) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -186,7 +187,7 @@ void create_duplicate_entity_bad_request() throws Exception { given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson) + .body(entityJson1) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -202,15 +203,34 @@ void create_duplicate_entity_bad_request() throws Exception { // Controller-owned validation: missing required query param @Test - void get_one_missing_office_bad_request() { + void get_one_missing_office_bad_request() throws Exception { + TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; + String entityJson2 = getUniqueTestEntityJsonByIndex(2); + String entityName = JsonPath.from(entityJson2).getString("id.name"); + // CREATE + given() + .log().ifValidationFails(LogDetail.ALL, true) + .contentType(Formats.JSONV2) + .body(entityJson2) + .header(AUTH_HEADER, user.toHeaderValue()) + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/entity") + .then() + .log().ifValidationFails(LogDetail.ALL, true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_CREATED)); + + // getOne with no office param given() .log().ifValidationFails(LogDetail.ALL, true) .accept(Formats.JSONV2) .when() .redirects().follow(true) .redirects().max(3) - .get("/entity/" + ENTITY_ID) + .get("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL, true) .assertThat() @@ -222,38 +242,52 @@ void get_one_missing_office_bad_request() { @Test void update_non_existing_entity_id_or_missing_office_id_400_or_404() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - InputStream in = this.getClass().getResourceAsStream("/cwms/cda/data/dto/entity.json"); - assertNotNull(in); - String entity = IOUtils.toString(in, java.nio.charset.StandardCharsets.UTF_8); + String entityJson3 = getUniqueTestEntityJsonByIndex(3); + String entityName = JsonPath.from(entityJson3).getString("id.name"); // UPDATE - non-existing entity id - 404 given() .log().ifValidationFails(LogDetail.ALL, true) .accept(Formats.JSONV2) .contentType(Formats.JSONV2) - .body(entity) + .body(entityJson3) .header(AUTH_HEADER, user.toHeaderValue()) - .queryParam(Controllers.OFFICE, OFFICE_ID) + .queryParam(OFFICE, OFFICE_ID) .when() .redirects().follow(true) .redirects().max(3) - .patch("/entity/" + "different") + .patch("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL, true) .assertThat() .statusCode(is(HttpServletResponse.SC_NOT_FOUND)); + // CREATE the non-existing entity id + given() + .log().ifValidationFails(LogDetail.ALL, true) + .contentType(Formats.JSONV2) + .body(entityJson3) + .header(AUTH_HEADER, user.toHeaderValue()) + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/entity") + .then() + .log().ifValidationFails(LogDetail.ALL, true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_CREATED)); + // UPDATE - missing office id - 400 given() .log().ifValidationFails(LogDetail.ALL, true) .accept(Formats.JSONV2) .contentType(Formats.JSONV2) - .body(entity) + .body(entityJson3) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) .redirects().max(3) - .patch("/entity/" + ENTITY_ID) + .patch("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL, true) .assertThat() @@ -287,7 +321,7 @@ void getAll_with_parent_filter_returns_200_empty_list_ok() { given() .log().ifValidationFails(LogDetail.ALL, true) .accept(Formats.JSONV2) - .queryParam(Controllers.PARENT_ENTITY_ID, PARENT_ID) + .queryParam(Controllers.PARENT_ENTITY_ID, "NOAA") .when() .redirects().follow(true) .redirects().max(3) @@ -300,20 +334,16 @@ void getAll_with_parent_filter_returns_200_empty_list_ok() { @Test - void getAll_match_null_parents_flag_() throws Exception { + void getAll_match_null_parents_flag() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - InputStream in = this.getClass().getResourceAsStream("/cwms/cda/data/dto/entity.json"); - assertNotNull(in); - String entity = IOUtils.toString(in, java.nio.charset.StandardCharsets.UTF_8); - // make parent-entity-id null - String nullParentEntity = entity.replace( - "\"parent-entity-id\" : \"NOAA\",", ""); + String entityJson4 = getUniqueTestEntityJsonByIndex(4); + String entityName = JsonPath.from(entityJson4).getString("id.name"); // CREATE entity with null parent - default match-null-parents = true given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(nullParentEntity) + .body(entityJson4) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -343,8 +373,8 @@ void getAll_match_null_parents_flag_() throws Exception { for (Map m : items) { Map id = (Map) m.get("id"); if (id != null - && "SPK".equals(id.get("office-id")) - && "NWS".equals(id.get("name"))) { + && OFFICE_ID.equals(id.get("office-id")) + && entityName.equals(id.get("name"))) { target = m; break; } @@ -375,8 +405,8 @@ void getAll_match_null_parents_flag_() throws Exception { for (Map m : items2) { Map id = (Map) m.get("id"); if (id != null - && "SPK".equals(id.get("office-id")) - && "NWS".equals(id.get("name"))) { + && OFFICE_ID.equals(id.get("office-id")) + && entityName.equals(id.get("name"))) { present = true; break; } @@ -387,15 +417,33 @@ void getAll_match_null_parents_flag_() throws Exception { // DELETE nullParentEntity given() .header(AUTH_HEADER, user.toHeaderValue()) - .queryParam(Controllers.OFFICE, OFFICE_ID) - .queryParam(Controllers.CASCADE_DELETE, CASCADE_DELETE) + .queryParam(OFFICE, OFFICE_ID) + .queryParam(CASCADE_DELETE, true) .when() .redirects().follow(true) .redirects().max(3) - .delete("/entity/" + ENTITY_ID) + .delete("/entity/" + entityName) .then() .log().ifValidationFails(LogDetail.ALL,true) .assertThat() .statusCode(is(HttpServletResponse.SC_NO_CONTENT)); } -} \ No newline at end of file + + // Helper to clean up test Entities + private static List> loadTestEntityJsonList() throws Exception { + InputStream in = EntityControllerTestIT.class.getResourceAsStream("/cwms/cda/data/dto/entity_test.json"); + assertNotNull(in); + String json = IOUtils.toString(in, java.nio.charset.StandardCharsets.UTF_8); + return JsonPath.from(json).getList(""); + } + + // Helper to create unique Test entities + private static String getUniqueTestEntityJsonByIndex(int index) throws Exception { + InputStream in = EntityControllerTestIT.class.getResourceAsStream("/cwms/cda/data/dto/entity_test.json"); + assertNotNull(in); + String json = IOUtils.toString(in, java.nio.charset.StandardCharsets.UTF_8); + List> entities = JsonPath.from(json).getList(""); + ObjectMapper mapper = new ObjectMapper(); + return mapper.writeValueAsString(entities.get(index)); + } +} diff --git a/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json b/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json new file mode 100644 index 000000000..045d29f81 --- /dev/null +++ b/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json @@ -0,0 +1,46 @@ +[ + { + "id" : { + "office-id" : "SPK", + "name" : "TE" + }, + "parent-entity-id" : "NOAA", + "category-id" : "GOV", + "long-name" : "Test Entity" + }, + { + "id" : { + "office-id" : "SPK", + "name" : "TE1" + }, + "parent-entity-id" : "NOAA", + "category-id" : "GOV", + "long-name" : "Test Entity1" + }, + { + "id" : { + "office-id" : "SPK", + "name" : "TE2" + }, + "parent-entity-id" : "NOAA", + "category-id" : "GOV", + "long-name" : "Test Entity2" + }, + { + "id" : { + "office-id" : "SPK", + "name" : "TE3" + }, + "parent-entity-id" : "NOAA", + "category-id" : "GOV", + "long-name" : "Test Entity3" + }, + { + "id" : { + "office-id" : "SPK", + "name" : "TE4" + }, + "category-id" : "GOV", + "long-name" : "Test Entity4" + } +] \ No newline at end of file From 57ad8cc00de24f096b81c3b119ca5490c8de7996 Mon Sep 17 00:00:00 2001 From: Kayla Arritola Date: Wed, 17 Dec 2025 16:39:31 -0800 Subject: [PATCH 2/4] Refactored entity update controller and tests. - Removed office-id query param and redundant checks - Added missing entity-id validation test - Added invalid office authorization test - Improved variable naming - Fixed failing .replace() logic --- .../java/cwms/cda/api/EntityController.java | 21 +--- .../cwms/cda/api/EntityControllerTestIT.java | 119 +++++++++++++----- .../cwms/cda/data/dto/entity_test.json | 34 +++-- 3 files changed, 113 insertions(+), 61 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java index 66bffa06d..bcd57f858 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java @@ -167,10 +167,6 @@ public void create(@NotNull Context ctx) { @OpenApiParam(name = ENTITY_ID, required = true, description = "Specifies the entity ID of the " + " Entity to be updated. (e.g., NWS)") }, - queryParams = { - @OpenApiParam(name = OFFICE, required = true, description = "Specifies the owning office "+ - " of the entity to be updated. (e.g., SPK)") - }, method = HttpMethod.PATCH, tags = {TAG}, responses = { @@ -182,28 +178,17 @@ public void create(@NotNull Context ctx) { public void update(@NotNull Context ctx, @NotNull String entityId) { try (final Timer.Context ignored = markAndTime(UPDATE)) { DSLContext dsl = getDslContext(ctx); - String officeId = requiredParam(ctx, OFFICE); String formatHeader = ctx.req.getContentType(); ContentType contentType = Formats.parseHeader(formatHeader, Entity.class); Entity entity = Formats.parseContent(contentType, ctx.bodyAsInputStream(), Entity.class); - if (entity.getId() == null || entity.getId().getOfficeId() == null || entity.getId().getName() == null) { - ctx.status(HttpServletResponse.SC_BAD_REQUEST); - ctx.result("Entity ID and Office ID must be provided in the request body."); - return; - } + // Validate the office ID and entity ID are provided. + entity.validate(); if (!entityId.equalsIgnoreCase(entity.getId().getName())) { ctx.status(HttpServletResponse.SC_NOT_FOUND); - ctx.result("Entity not found for the given entity-id."); + ctx.result("Entity ID in path parameter must match the Entity ID in the request body."); return; } - - if (!officeId.equalsIgnoreCase(entity.getId().getOfficeId())) { - ctx.status(HttpServletResponse.SC_BAD_REQUEST); - ctx.result("Office ID in query parameter must match the Office ID in the request body."); - return; - } - EntityDao dao = new EntityDao(dsl); dao.updateEntity(entity); ctx.status(HttpServletResponse.SC_OK); diff --git a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java index af2f6e758..e77314a2d 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java @@ -1,6 +1,7 @@ package cwms.cda.api; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import cwms.cda.formatters.Formats; import fixtures.TestAccounts; import io.restassured.filter.log.LogDetail; @@ -60,15 +61,15 @@ void tearDown() throws Exception { void test_entity_create_get_update_delete(String format) throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - String entityJson = getUniqueTestEntityJsonByIndex(0); - String entityName = JsonPath.from(entityJson).getString("id.name"); - String longName = JsonPath.from(entityJson).getString("long-name"); + String crudEntityJson = getUniqueTestEntityJsonByIndex(0); + String entityName = JsonPath.from(crudEntityJson).getString("id.name"); + String longName = JsonPath.from(crudEntityJson).getString("long-name"); // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson) + .body(crudEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -97,15 +98,15 @@ void test_entity_create_get_update_delete(String format) throws Exception { .body("long-name", equalTo(longName)); // UPDATE — modify long-name to verify persistence - String updatedEntityJson = entityJson.replace( - "\"" + longName + "\"", - "\"Updated long name\""); + ObjectMapper mapper = new ObjectMapper(); + ObjectNode root = (ObjectNode) mapper.readTree(crudEntityJson); + root.put("long-name", "Updated long name"); + String updatedEntityJson = mapper.writeValueAsString(root); given() .contentType(Formats.JSONV2) .body(updatedEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) - .queryParam(OFFICE, OFFICE_ID) .when() .redirects().follow(true) .redirects().max(3) @@ -160,18 +161,17 @@ void test_entity_create_get_update_delete(String format) throws Exception { .statusCode(is(HttpServletResponse.SC_NOT_FOUND)); } - // create fails if entity already exists @Test - void create_duplicate_entity_bad_request() throws Exception { + void create_duplicate_entity_400() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - String entityJson1 = getUniqueTestEntityJsonByIndex(1); + String duplicateEntityJson = getUniqueTestEntityJsonByIndex(1); // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson1) + .body(duplicateEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -187,7 +187,7 @@ void create_duplicate_entity_bad_request() throws Exception { given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson1) + .body(duplicateEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -203,16 +203,16 @@ void create_duplicate_entity_bad_request() throws Exception { // Controller-owned validation: missing required query param @Test - void get_one_missing_office_bad_request() throws Exception { + void get_one_missing_office_400() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - String entityJson2 = getUniqueTestEntityJsonByIndex(2); - String entityName = JsonPath.from(entityJson2).getString("id.name"); + String getOneEntityJson = getUniqueTestEntityJsonByIndex(2); + String entityName = JsonPath.from(getOneEntityJson).getString("id.name"); // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson2) + .body(getOneEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -220,7 +220,7 @@ void get_one_missing_office_bad_request() throws Exception { .post("/entity") .then() .log().ifValidationFails(LogDetail.ALL, true) - .assertThat() + .assertThat() .statusCode(is(HttpServletResponse.SC_CREATED)); // getOne with no office param @@ -238,19 +238,62 @@ void get_one_missing_office_bad_request() throws Exception { } - // Entity ID in the URL must match the id.name in the request body + // Test CREATE as SPK user -> UPDATE as SWT user + @Test + void update_entity_other_office_forbidden_401() throws Exception { + TestAccounts.KeyUser spkUser = TestAccounts.KeyUser.SPK_NORMAL; + TestAccounts.KeyUser swtUser = TestAccounts.KeyUser.SWT_NORMAL; + + String updateOfficeEntityJson = getUniqueTestEntityJsonByIndex(4); + + // CREATE the entity in SPK as SPK user + given() + .contentType(Formats.JSONV2) + .body(updateOfficeEntityJson) + .header(AUTH_HEADER, spkUser.toHeaderValue()) // authenticated as SPK + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/entity") + .then() + .log().ifValidationFails(LogDetail.ALL, true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_CREATED)); + + // Try UPDATE on the SAME SPK entity with SWT credentials + ObjectMapper mapper = new ObjectMapper(); + ObjectNode root = (ObjectNode) mapper.readTree(updateOfficeEntityJson); + root.put("long-name", "Malicious Update from SWT"); + String updateJson = mapper.writeValueAsString(root); + String entityName = JsonPath.from(updateJson).getString("id.name"); + + given() + .contentType(Formats.JSONV2) + .body(updateJson) + .header(AUTH_HEADER, swtUser.toHeaderValue()) // authenticated as SWT + .when() + .redirects().follow(true) + .redirects().max(3) + .patch("/entity/" + entityName) + .then() + .log().ifValidationFails(LogDetail.ALL, true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_UNAUTHORIZED)); + } + + // Test UPDATE entity before create -> CREATE -> UPDATE with missing entity id. @Test - void update_non_existing_entity_id_or_missing_office_id_400_or_404() throws Exception { + void update_entity_expected_failing_behavior_400_or_404() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - String entityJson3 = getUniqueTestEntityJsonByIndex(3); - String entityName = JsonPath.from(entityJson3).getString("id.name"); + String newEntityJson = getUniqueTestEntityJsonByIndex(3); + String entityName = JsonPath.from(newEntityJson).getString("id.name"); - // UPDATE - non-existing entity id - 404 + // UPDATE - non-existing new entity id - 404 given() .log().ifValidationFails(LogDetail.ALL, true) .accept(Formats.JSONV2) .contentType(Formats.JSONV2) - .body(entityJson3) + .body(newEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .queryParam(OFFICE, OFFICE_ID) .when() @@ -262,11 +305,11 @@ void update_non_existing_entity_id_or_missing_office_id_400_or_404() throws Exce .assertThat() .statusCode(is(HttpServletResponse.SC_NOT_FOUND)); - // CREATE the non-existing entity id + // CREATE the new Entity, then update it without an entity id given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson3) + .body(newEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -274,15 +317,24 @@ void update_non_existing_entity_id_or_missing_office_id_400_or_404() throws Exce .post("/entity") .then() .log().ifValidationFails(LogDetail.ALL, true) - .assertThat() + .assertThat() .statusCode(is(HttpServletResponse.SC_CREATED)); - // UPDATE - missing office id - 400 + // UPDATE - missing entity-id (id.name) in the BODY + ObjectMapper mapper = new ObjectMapper(); + ObjectNode root = (ObjectNode) mapper.readTree(newEntityJson); + + ObjectNode idNode = (ObjectNode) root.get("id"); + idNode.remove("name"); + + root.put("long-name", "Updated long name"); + String missingIdInBodyJson = mapper.writeValueAsString(root); + given() .log().ifValidationFails(LogDetail.ALL, true) .accept(Formats.JSONV2) .contentType(Formats.JSONV2) - .body(entityJson3) + .body(missingIdInBodyJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) @@ -294,7 +346,6 @@ void update_non_existing_entity_id_or_missing_office_id_400_or_404() throws Exce .statusCode(is(HttpServletResponse.SC_BAD_REQUEST)); } - // getAll with no query params: must return 200 and a list (empty allowed) @ParameterizedTest @ValueSource(strings = {Formats.JSONV2, Formats.DEFAULT}) @@ -332,18 +383,18 @@ void getAll_with_parent_filter_returns_200_empty_list_ok() { .statusCode(is(HttpServletResponse.SC_OK)); } - + // test CREATE null parent entity -> GET + match-null-parents true/false -> DELETE @Test void getAll_match_null_parents_flag() throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; - String entityJson4 = getUniqueTestEntityJsonByIndex(4); - String entityName = JsonPath.from(entityJson4).getString("id.name"); + String nullParentEntityJson = getUniqueTestEntityJsonByIndex(5); + String entityName = JsonPath.from(nullParentEntityJson).getString("id.name"); // CREATE entity with null parent - default match-null-parents = true given() .log().ifValidationFails(LogDetail.ALL, true) .contentType(Formats.JSONV2) - .body(entityJson4) + .body(nullParentEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() .redirects().follow(true) diff --git a/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json b/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json index 045d29f81..c51f8d8d3 100644 --- a/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json +++ b/cwms-data-api/src/test/resources/cwms/cda/data/dto/entity_test.json @@ -1,46 +1,62 @@ [ + { "id" : { "office-id" : "SPK", - "name" : "TE" + "name" : "0_CRUD_TE" }, "parent-entity-id" : "NOAA", "category-id" : "GOV", "long-name" : "Test Entity" }, + { "id" : { "office-id" : "SPK", - "name" : "TE1" + "name" : "1_DUP_ENTITY_ID_TE" }, "parent-entity-id" : "NOAA", "category-id" : "GOV", - "long-name" : "Test Entity1" + "long-name" : "Duplicate Test Entity" }, + + { + "id" : { + "office-id": "SPK", + "name": "2_GET_ONE_TE" + }, + "parent-entity-id" : "NOAA", + "category-id" : "GOV", + "long-name" : "Test Entity for Get One" + }, + { "id" : { "office-id" : "SPK", - "name" : "TE2" + "name" : "3_NON_EXISTENT_TE" }, "parent-entity-id" : "NOAA", "category-id" : "GOV", - "long-name" : "Test Entity2" + "long-name" : "Test Entity that does not exist" }, + { "id" : { "office-id" : "SPK", - "name" : "TE3" + "name" : "4_UPDATE_OFFICE_TE" }, "parent-entity-id" : "NOAA", "category-id" : "GOV", - "long-name" : "Test Entity3" + "long-name" : "Test Entity for Update Office" }, + { "id" : { "office-id" : "SPK", - "name" : "TE4" + "name" : "5_NULL_PARENT_TE" }, "category-id" : "GOV", - "long-name" : "Test Entity4" + "long-name" : "Null Parent Test Entity" } + ] \ No newline at end of file From 8471b8f3ddf59997daaf08ae14288593aa902dba Mon Sep 17 00:00:00 2001 From: Kayla Arritola Date: Fri, 19 Dec 2025 12:26:13 -0800 Subject: [PATCH 3/4] Added logic to getAll when parent id param is provided. - Added IT to verify logic --- .../java/cwms/cda/api/EntityController.java | 10 +++++-- .../cwms/cda/api/EntityControllerTestIT.java | 26 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java index bcd57f858..ca0e558f7 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java @@ -69,8 +69,14 @@ public void getAll(@NotNull Context ctx) { String officeId = ctx.queryParam(OFFICE); String entityId = ctx.queryParam(ENTITY_ID); String parentId = ctx.queryParam(PARENT_ENTITY_ID); - Boolean matchNullParents = ctx.queryParamAsClass(MATCH_NULL_PARENTS, Boolean.class) - .getOrDefault(true); + Boolean matchNullParents; + if (parentId != null) { + matchNullParents = false; + } else { + matchNullParents = ctx.queryParamAsClass(MATCH_NULL_PARENTS, Boolean.class) + .getOrDefault(true); + } + String categoryId = ctx.queryParam(CATEGORY_ID); String entityName = ctx.queryParam(LONG_NAME); diff --git a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java index e77314a2d..cbf48fa75 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java @@ -465,6 +465,32 @@ void getAll_match_null_parents_flag() throws Exception { assertFalse(present, "Entity with null parent-id should be filtered out when match-null-parents=false"); + // GET - parent-entity-id filter is present, match-null-parents should be ignored and treated as false + String jsonParent = + given() + .log().ifValidationFails(LogDetail.ALL, true) + .accept(Formats.JSONV2) + .queryParam(Controllers.PARENT_ENTITY_ID, "NOAA") + .queryParam(Controllers.MATCH_NULL_PARENTS, true) + .when() + .redirects().follow(true) + .redirects().max(3) + .get("/entity") + .then() + .log().ifValidationFails(LogDetail.ALL, true) + .statusCode(HttpServletResponse.SC_OK) + .extract().asString(); + + List> itemsParent = JsonPath.from(jsonParent).getList(""); + boolean foundParent = itemsParent.stream().anyMatch(m -> { + Map id = (Map) m.get("id"); + return id != null + && OFFICE_ID.equals(id.get("office-id")) + && entityName.equals(id.get("name")); + }); + assertFalse(foundParent, "Entity with null parent-id should be filtered out when parent filter is present, regardless of match-null-parents"); + + // DELETE nullParentEntity given() .header(AUTH_HEADER, user.toHeaderValue()) From 2989e95dd866d8f851de86896e3ffb536ee44a9e Mon Sep 17 00:00:00 2001 From: Kayla Arritola Date: Fri, 19 Dec 2025 15:28:53 -0800 Subject: [PATCH 4/4] Replaced JsonV2 with JsonV1 in controller, controllerIT and dto. --- .../java/cwms/cda/api/EntityController.java | 8 ++-- .../main/java/cwms/cda/data/dto/Entity.java | 4 +- .../cwms/cda/api/EntityControllerTestIT.java | 40 +++++++++---------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java index ca0e558f7..6c1d9da93 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/EntityController.java @@ -55,7 +55,7 @@ private Timer.Context markAndTime(String subject) { }, responses = { @OpenApiResponse(status = STATUS_200, content = { - @OpenApiContent(isArray = true, from = Entity.class, type = Formats.JSONV2) + @OpenApiContent(isArray = true, from = Entity.class, type = Formats.JSONV1) }) }, tags = {TAG} @@ -106,7 +106,7 @@ public void getAll(@NotNull Context ctx) { }, responses = { @OpenApiResponse(status = STATUS_200, content = { - @OpenApiContent(from = Entity.class, type = Formats.JSONV2) + @OpenApiContent(from = Entity.class, type = Formats.JSONV1) }) }, tags = {TAG} @@ -139,7 +139,7 @@ public void getOne(@NotNull Context ctx, @NotNull String entityId) { description = "Create CWMS Entity", requestBody = @OpenApiRequestBody( content = { - @OpenApiContent(from = Entity.class, type = Formats.JSONV2) + @OpenApiContent(from = Entity.class, type = Formats.JSONV1) }, required = true), responses = { @@ -167,7 +167,7 @@ public void create(@NotNull Context ctx) { @OpenApi( description = "Update an existing Entity.", requestBody = @OpenApiRequestBody( - content = {@OpenApiContent(from = Entity.class, type = Formats.JSONV2)}, + content = {@OpenApiContent(from = Entity.class, type = Formats.JSONV1)}, required = true), pathParams = { @OpenApiParam(name = ENTITY_ID, required = true, description = "Specifies the entity ID of the " + diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dto/Entity.java b/cwms-data-api/src/main/java/cwms/cda/data/dto/Entity.java index a41afa489..3032cc769 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dto/Entity.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dto/Entity.java @@ -7,9 +7,9 @@ import com.fasterxml.jackson.databind.annotation.JsonNaming; import cwms.cda.formatters.Formats; import cwms.cda.formatters.annotations.FormattableWith; -import cwms.cda.formatters.json.JsonV2; +import cwms.cda.formatters.json.JsonV1; -@FormattableWith(contentType = Formats.JSONV2, formatter = JsonV2.class, aliases = {Formats.DEFAULT, Formats.JSON}) +@FormattableWith(contentType = Formats.JSONV1, formatter = JsonV1.class, aliases = {Formats.DEFAULT, Formats.JSON}) @JsonDeserialize(builder = Entity.Builder.class) @JsonInclude(JsonInclude.Include.NON_NULL) @JsonNaming(PropertyNamingStrategies.KebabCaseStrategy.class) diff --git a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java index cbf48fa75..9c640c30f 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/EntityControllerTestIT.java @@ -57,7 +57,7 @@ void tearDown() throws Exception { // create -> getOne -> update -> getOne to verify updated // delete -> getOne to verify deleted @ParameterizedTest - @ValueSource(strings = {Formats.JSONV2, Formats.DEFAULT}) + @ValueSource(strings = {Formats.JSONV1, Formats.DEFAULT}) void test_entity_create_get_update_delete(String format) throws Exception { TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; @@ -68,7 +68,7 @@ void test_entity_create_get_update_delete(String format) throws Exception { // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(crudEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -104,7 +104,7 @@ void test_entity_create_get_update_delete(String format) throws Exception { String updatedEntityJson = mapper.writeValueAsString(root); given() - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(updatedEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -170,7 +170,7 @@ void create_duplicate_entity_400() throws Exception { // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(duplicateEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -186,7 +186,7 @@ void create_duplicate_entity_400() throws Exception { // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(duplicateEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -211,7 +211,7 @@ void get_one_missing_office_400() throws Exception { // CREATE given() .log().ifValidationFails(LogDetail.ALL, true) - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(getOneEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -226,7 +226,7 @@ void get_one_missing_office_400() throws Exception { // getOne with no office param given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) + .accept(Formats.JSONV1) .when() .redirects().follow(true) .redirects().max(3) @@ -248,7 +248,7 @@ void update_entity_other_office_forbidden_401() throws Exception { // CREATE the entity in SPK as SPK user given() - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(updateOfficeEntityJson) .header(AUTH_HEADER, spkUser.toHeaderValue()) // authenticated as SPK .when() @@ -268,7 +268,7 @@ void update_entity_other_office_forbidden_401() throws Exception { String entityName = JsonPath.from(updateJson).getString("id.name"); given() - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(updateJson) .header(AUTH_HEADER, swtUser.toHeaderValue()) // authenticated as SWT .when() @@ -291,8 +291,8 @@ void update_entity_expected_failing_behavior_400_or_404() throws Exception { // UPDATE - non-existing new entity id - 404 given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) - .contentType(Formats.JSONV2) + .accept(Formats.JSONV1) + .contentType(Formats.JSONV1) .body(newEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .queryParam(OFFICE, OFFICE_ID) @@ -308,7 +308,7 @@ void update_entity_expected_failing_behavior_400_or_404() throws Exception { // CREATE the new Entity, then update it without an entity id given() .log().ifValidationFails(LogDetail.ALL, true) - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(newEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -332,8 +332,8 @@ void update_entity_expected_failing_behavior_400_or_404() throws Exception { given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) - .contentType(Formats.JSONV2) + .accept(Formats.JSONV1) + .contentType(Formats.JSONV1) .body(missingIdInBodyJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -348,7 +348,7 @@ void update_entity_expected_failing_behavior_400_or_404() throws Exception { // getAll with no query params: must return 200 and a list (empty allowed) @ParameterizedTest - @ValueSource(strings = {Formats.JSONV2, Formats.DEFAULT}) + @ValueSource(strings = {Formats.JSONV1, Formats.DEFAULT}) void getAll_no_params_returns_200_empty_list_ok(String format) { given() .log().ifValidationFails(LogDetail.ALL, true) @@ -371,7 +371,7 @@ void getAll_no_params_returns_200_empty_list_ok(String format) { void getAll_with_parent_filter_returns_200_empty_list_ok() { given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) + .accept(Formats.JSONV1) .queryParam(Controllers.PARENT_ENTITY_ID, "NOAA") .when() .redirects().follow(true) @@ -393,7 +393,7 @@ void getAll_match_null_parents_flag() throws Exception { // CREATE entity with null parent - default match-null-parents = true given() .log().ifValidationFails(LogDetail.ALL, true) - .contentType(Formats.JSONV2) + .contentType(Formats.JSONV1) .body(nullParentEntityJson) .header(AUTH_HEADER, user.toHeaderValue()) .when() @@ -409,7 +409,7 @@ void getAll_match_null_parents_flag() throws Exception { String json = given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) + .accept(Formats.JSONV1) .when() .redirects().follow(true) .redirects().max(3) @@ -440,7 +440,7 @@ void getAll_match_null_parents_flag() throws Exception { String json2 = given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) + .accept(Formats.JSONV1) .queryParam(Controllers.MATCH_NULL_PARENTS, false) .when() .redirects().follow(true) @@ -469,7 +469,7 @@ void getAll_match_null_parents_flag() throws Exception { String jsonParent = given() .log().ifValidationFails(LogDetail.ALL, true) - .accept(Formats.JSONV2) + .accept(Formats.JSONV1) .queryParam(Controllers.PARENT_ENTITY_ID, "NOAA") .queryParam(Controllers.MATCH_NULL_PARENTS, true) .when()