From 218bcfd027a2b1b9edd73a7ab75f51ebfabfa1ff Mon Sep 17 00:00:00 2001 From: Ryan Miles Date: Wed, 12 Jun 2024 17:47:17 -0700 Subject: [PATCH 1/4] Adding aliases and integ tests for ratings Still need to fill out integration testing for ratings --- .../java/cwms/cda/api/RatingController.java | 62 +++++++++---------- .../java/cwms/cda/formatters/Formats.java | 7 ++- .../cwms/cda/api/RatingsControllerTestIT.java | 15 +++++ 3 files changed, 49 insertions(+), 35 deletions(-) create mode 100644 cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java diff --git a/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java b/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java index a1bdfe637..a53268fce 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java @@ -50,6 +50,7 @@ import static cwms.cda.api.Controllers.UNIT; import static cwms.cda.api.Controllers.UPDATE; import static cwms.cda.api.Controllers.VERSION_DATE; +import static cwms.cda.api.Controllers.addDeprecatedContentTypeWarning; import static cwms.cda.data.dao.JooqDao.getDslContext; import com.codahale.metrics.Histogram; @@ -59,9 +60,12 @@ import cwms.cda.data.dao.JsonRatingUtils; import cwms.cda.data.dao.RatingDao; import cwms.cda.data.dao.RatingSetDao; +import cwms.cda.data.dto.CwmsDTOBase; import cwms.cda.formatters.ContentType; import cwms.cda.formatters.Formats; -import cwms.cda.formatters.FormattingException; +import cwms.cda.formatters.annotations.FormattableWith; +import cwms.cda.formatters.json.JsonV2; +import cwms.cda.formatters.xml.XMLv2; import cwms.cda.helpers.DateUtils; import hec.data.RatingException; import hec.data.cwmsRating.RatingSet; @@ -287,7 +291,6 @@ public void getAll(@NotNull Context ctx) { RatingDao ratingDao = getRatingDao(dsl); - String format = ctx.queryParamAsClass(FORMAT, String.class).getOrDefault("json"); String names = ctx.queryParamAsClass(NAME, String.class).getOrDefault("*"); String unit = ctx.queryParam(UNIT); String datum = ctx.queryParam(DATUM); @@ -296,40 +299,18 @@ public void getAll(@NotNull Context ctx) { String end = ctx.queryParam(END); String timezone = ctx.queryParamAsClass(TIMEZONE, String.class).getOrDefault("UTC"); - switch (format) { - case "json": { - ctx.contentType(Formats.JSON); - break; - } - case "tab": { - ctx.contentType(Formats.TAB); - break; - } - case "csv": { - ctx.contentType(Formats.CSV); - break; - } - case "xml": { - ctx.contentType(Formats.XML); - break; - } - case "wml2": { - ctx.contentType(Formats.WML2); - break; - } - case "jpg": // same handler - case "png": - default: { - ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED) - .json(CdaError.notImplemented()); - return; - } - } + String format = ctx.queryParamAsClass(FORMAT, String.class).getOrDefault("json"); + String header = ctx.header(Header.ACCEPT); - String results = ratingDao.retrieveRatings(format, names, unit, datum, office, start, + ContentType contentType = Formats.parseHeaderAndQueryParm(header, format, RatingAliasMarker.class); + ctx.contentType(contentType.getType()); + String legacyFormat = Formats.getLegacyTypeFromContentType(contentType); + + String results = ratingDao.retrieveRatings(legacyFormat, names, unit, datum, office, start, end, timezone); ctx.status(HttpServletResponse.SC_OK); ctx.result(results); + addDeprecatedContentTypeWarning(ctx, contentType); requestResultSize.update(results.length()); } } @@ -404,12 +385,16 @@ private String getRatingSetString(Context ctx, RatingSet.DatabaseLoadMethod meth try (final Timer.Context ignored = markAndTime("getRatingSetString")) { String acceptHeader = ctx.header(Header.ACCEPT); + ContentType contentType = Formats.parseHeader(acceptHeader, RatingAliasMarker.class); + + boolean isJson = Formats.JSONV2.equals(contentType.toString()); + boolean isXml = Formats.XMLV2.equals(contentType.toString()); - if (Formats.JSONV2.equals(acceptHeader) || Formats.XMLV2.equals(acceptHeader)) { + if (isJson || isXml) { try { RatingSet ratingSet = getRatingSet(ctx, method, officeId, rating, begin, end); if (ratingSet != null) { - if (Formats.JSONV2.equals(acceptHeader)) { + if (isJson) { retval = JsonRatingUtils.toJson(ratingSet); } else { retval = RatingXmlFactory.toXml(ratingSet, " "); @@ -438,6 +423,8 @@ private String getRatingSetString(Context ctx, RatingSet.DatabaseLoadMethod meth ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED); ctx.json(CdaError.notImplemented()); } + + addDeprecatedContentTypeWarning(ctx, contentType); } return retval; } @@ -486,4 +473,11 @@ public void update(@NotNull Context ctx, @NotNull String ratingId) { } } + /** + * This is a marker interface only used by Formats to support ContentType aliases. + * There's no reason to implement this interface. + */ + @FormattableWith(contentType = Formats.JSONV2, formatter = JsonV2.class, aliases = {Formats.JSON, Formats.DEFAULT}) + @FormattableWith(contentType = Formats.XMLV2, formatter = XMLv2.class, aliases = {Formats.XML}) + private interface RatingAliasMarker extends CwmsDTOBase { } } diff --git a/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java b/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java index fb4e7498e..2ec00b758 100644 --- a/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java +++ b/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java @@ -297,12 +297,17 @@ public static ContentType parseHeader(String header) { } public static ContentType parseHeader(String header, Class klass) { - ArrayList contentTypes = new ArrayList<>(); ContentTypeAliasMap aliasMap = ContentTypeAliasMap.empty(); if (klass != null) { aliasMap = ContentTypeAliasMap.forDtoClass(klass); } + return parseHeader(header, aliasMap); + } + + public static ContentType parseHeader(String header, ContentTypeAliasMap aliasMap) + { + ArrayList contentTypes = new ArrayList<>(); if (header != null && !header.isEmpty()) { String[] all = header.split(","); logger.log(Level.FINEST, "Finding handlers {0}", all.length); diff --git a/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java new file mode 100644 index 000000000..fc2b00934 --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2024. Hydrologic Engineering Center (HEC). + * United States Army Corps of Engineers + * All Rights Reserved. HEC PROPRIETARY/CONFIDENTIAL. + * Source may not be released without written approval from HEC + */ + +package cwms.cda.api; + +import org.junit.jupiter.api.Tag; + +@Tag("integration") +public class RatingsControllerTestIT extends DataApiTestIT +{ +} From e86b683c2584dbfadbe2b60aebce0167a6aca2c8 Mon Sep 17 00:00:00 2001 From: Ryan Miles Date: Fri, 14 Jun 2024 11:33:48 -0700 Subject: [PATCH 2/4] Removing parseHeader refactor --- .../src/main/java/cwms/cda/formatters/Formats.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java b/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java index 2ec00b758..335761648 100644 --- a/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java +++ b/cwms-data-api/src/main/java/cwms/cda/formatters/Formats.java @@ -301,12 +301,6 @@ public static ContentType parseHeader(String header, Class contentTypes = new ArrayList<>(); if (header != null && !header.isEmpty()) { String[] all = header.split(","); From 7eb1b2d1412f2a0668cf07ac217fc7c94a909da0 Mon Sep 17 00:00:00 2001 From: Ryan Miles Date: Fri, 14 Jun 2024 11:34:28 -0700 Subject: [PATCH 3/4] Switching XML to default --- .../src/main/java/cwms/cda/api/RatingController.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java b/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java index a53268fce..6642199d4 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java @@ -477,7 +477,7 @@ public void update(@NotNull Context ctx, @NotNull String ratingId) { * This is a marker interface only used by Formats to support ContentType aliases. * There's no reason to implement this interface. */ - @FormattableWith(contentType = Formats.JSONV2, formatter = JsonV2.class, aliases = {Formats.JSON, Formats.DEFAULT}) - @FormattableWith(contentType = Formats.XMLV2, formatter = XMLv2.class, aliases = {Formats.XML}) + @FormattableWith(contentType = Formats.JSONV2, formatter = JsonV2.class, aliases = {Formats.JSON}) + @FormattableWith(contentType = Formats.XMLV2, formatter = XMLv2.class, aliases = {Formats.XML, Formats.DEFAULT}) private interface RatingAliasMarker extends CwmsDTOBase { } } From d3f2c8434c293ba43db19cbac0d78ac6e324eef6 Mon Sep 17 00:00:00 2001 From: Ryan Miles Date: Fri, 14 Jun 2024 15:40:08 -0700 Subject: [PATCH 4/4] Filling in integration tests for ratings Resolving issues that came up from the integration tests performed. Found and resolved a bug in the RatingSetDao::create method where the error output was not being handled correctly. This may need additional parsing, but for now it works. --- .../java/cwms/cda/api/RatingController.java | 20 +- .../java/cwms/cda/data/dao/RatingSetDao.java | 6 +- .../cwms/cda/api/RatingsControllerTestIT.java | 203 ++++++++++++++++++ 3 files changed, 225 insertions(+), 4 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java b/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java index 6642199d4..15141ddb0 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/RatingController.java @@ -299,15 +299,28 @@ public void getAll(@NotNull Context ctx) { String end = ctx.queryParam(END); String timezone = ctx.queryParamAsClass(TIMEZONE, String.class).getOrDefault("UTC"); - String format = ctx.queryParamAsClass(FORMAT, String.class).getOrDefault("json"); + String format = ctx.queryParamAsClass(FORMAT, String.class).getOrDefault(""); String header = ctx.header(Header.ACCEPT); ContentType contentType = Formats.parseHeaderAndQueryParm(header, format, RatingAliasMarker.class); - ctx.contentType(contentType.getType()); - String legacyFormat = Formats.getLegacyTypeFromContentType(contentType); + if (format.isEmpty()) + { + //Use the full content type here (i.e. application/json;version=2) + ctx.contentType(contentType.toString()); + } + else + { + //Legacy content type only includes the basic type (i.e. application/json) + ctx.contentType(contentType.getType()); + } + + //At the moment, we still use the legacy formatting here, since we don't have a newer API for serializing/deserializing + //a collection of rating sets - unlike getOne. + String legacyFormat = Formats.getLegacyTypeFromContentType(contentType); String results = ratingDao.retrieveRatings(legacyFormat, names, unit, datum, office, start, end, timezone); + ctx.status(HttpServletResponse.SC_OK); ctx.result(results); addDeprecatedContentTypeWarning(ctx, contentType); @@ -391,6 +404,7 @@ private String getRatingSetString(Context ctx, RatingSet.DatabaseLoadMethod meth boolean isXml = Formats.XMLV2.equals(contentType.toString()); if (isJson || isXml) { + ctx.contentType(contentType.toString()); try { RatingSet ratingSet = getRatingSet(ctx, method, officeId, rating, begin, end); if (ratingSet != null) { diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/RatingSetDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/RatingSetDao.java index 6045c3c4d..37374c40f 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/RatingSetDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/RatingSetDao.java @@ -55,8 +55,12 @@ public void create(String ratingSetXml, boolean storeTemplate) throws IOExceptio // can't exist if we are creating, if it exists use store String office = extractOfficeId(ratingSetXml); DSLContext context = getDslContext(c, office); - CWMS_RATING_PACKAGE.call_STORE_RATINGS_XML__5(context.configuration(), + String errs = CWMS_RATING_PACKAGE.call_STORE_RATINGS_XML__5(context.configuration(), ratingSetXml, "T", storeTemplate ? "T" : "F"); + if (errs != null && !errs.isEmpty()) + { + throw new DataAccessException(errs); + } }); } catch (DataAccessException ex) { Throwable cause = ex.getCause(); diff --git a/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java index fc2b00934..2a55eb8c0 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/RatingsControllerTestIT.java @@ -7,9 +7,212 @@ package cwms.cda.api; +import cwms.cda.formatters.Formats; +import fixtures.TestAccounts; +import hec.data.cwmsRating.io.RatingSetContainer; +import hec.data.cwmsRating.io.RatingSpecContainer; +import io.restassured.filter.log.LogDetail; +import mil.army.usace.hec.cwms.rating.io.xml.RatingContainerXmlFactory; +import mil.army.usace.hec.cwms.rating.io.xml.RatingSetContainerXmlFactory; +import mil.army.usace.hec.cwms.rating.io.xml.RatingSpecXmlFactory; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import javax.servlet.http.HttpServletResponse; + +import java.util.Arrays; + +import static cwms.cda.api.Controllers.FORMAT; +import static cwms.cda.api.Controllers.OFFICE; +import static cwms.cda.api.Controllers.STORE_TEMPLATE; +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.is; @Tag("integration") public class RatingsControllerTestIT extends DataApiTestIT { + private static final String EXISTING_LOC = "RatingsControllerTestIT"; + private static final String EXISTING_SPEC = EXISTING_LOC + ".Stage;Flow.COE.Production"; + private static final String SPK = "SPK"; + + @BeforeAll + static void beforeAll() throws Exception + { + //Make sure we always have something. + createLocation(EXISTING_LOC, true, SPK); + + String ratingXml = readResourceFile("cwms/cda/api/Zanesville_Stage_Flow_COE_Production.xml"); + ratingXml = ratingXml.replaceAll("Zanesville", EXISTING_LOC); + RatingSetContainer container = RatingSetContainerXmlFactory.ratingSetContainerFromXml(ratingXml); + RatingSpecContainer specContainer = container.ratingSpecContainer; + specContainer.officeId = SPK; + specContainer.specOfficeId = SPK; + specContainer.locationId = EXISTING_LOC; + String specXml = RatingSpecXmlFactory.toXml(specContainer, "", 0, true); + String templateXml = RatingSpecXmlFactory.toXml(specContainer, "", 0); + String setXml = RatingContainerXmlFactory.toXml(container, "", 0, true, false); + TestAccounts.KeyUser user = TestAccounts.KeyUser.SPK_NORMAL; + + //Create Template + given() + .log().ifValidationFails(LogDetail.ALL,true) + .contentType(Formats.XMLV2) + .body(templateXml) + .header("Authorization", user.toHeaderValue()) + .queryParam(OFFICE, SPK) + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/ratings/template") + .then() + .assertThat() + .log().ifValidationFails(LogDetail.ALL,true) + .statusCode(is(HttpServletResponse.SC_CREATED)); + + //Create Spec + given() + .log().ifValidationFails(LogDetail.ALL,true) + .contentType(Formats.XMLV2) + .body(specXml) + .header("Authorization", user.toHeaderValue()) + .queryParam(OFFICE, SPK) + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/ratings/spec") + .then() + .assertThat() + .log().ifValidationFails(LogDetail.ALL,true) + .statusCode(is(HttpServletResponse.SC_CREATED)); + + //Create the set + given() + .log().ifValidationFails(LogDetail.ALL,true) + .contentType(Formats.XMLV2) + .body(setXml) + .header("Authorization", user.toHeaderValue()) + .queryParam(OFFICE, SPK) + .queryParam(STORE_TEMPLATE, false) + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/ratings") + .then() + .assertThat() + .log().ifValidationFails(LogDetail.ALL,true) + .statusCode(is(HttpServletResponse.SC_OK)); + } + + @ParameterizedTest + @EnumSource(GetAllLegacyTest.class) + void test_getAll_legacy(GetAllLegacyTest test) + { + given() + .log().ifValidationFails(LogDetail.ALL,true) + .queryParam(FORMAT, test._queryParam) + .when() + .redirects().follow(true) + .redirects().max(3) + .get("/ratings") + .then() + .assertThat() + .log().ifValidationFails(LogDetail.ALL,true) + .statusCode(is(HttpServletResponse.SC_OK)) + .contentType(is(test._expectedContentType)); + } + + @ParameterizedTest + @EnumSource(GetAllTest.class) + void test_getAll(GetAllTest test) + { + given() + .log().ifValidationFails(LogDetail.ALL,true) + .accept(test._accept) + .when() + .redirects().follow(true) + .redirects().max(3) + .get("/ratings") + .then() + .assertThat() + .log().ifValidationFails(LogDetail.ALL,true) + .statusCode(is(HttpServletResponse.SC_OK)) + .contentType(is(test._expectedContentType)); + } + + @ParameterizedTest + @EnumSource(GetOneTest.class) + void test_getOne(GetOneTest test) + { + given() + .log().ifValidationFails(LogDetail.ALL,true) + .accept(test._accept) + .queryParam(OFFICE, SPK) + .when() + .redirects().follow(true) + .redirects().max(3) + .get("/ratings/" + EXISTING_SPEC) + .then() + .assertThat() + .log().ifValidationFails(LogDetail.ALL,true) + .statusCode(is(HttpServletResponse.SC_OK)) + .contentType(is(test._expectedContentType)); + } + + enum GetOneTest + { + DEFAULT(Formats.DEFAULT, Formats.XMLV2), + XML(Formats.XML, Formats.XMLV2), + XMLV2(Formats.XMLV2, Formats.XMLV2), + JSON(Formats.JSON, Formats.JSONV2), + JSONV2(Formats.JSONV2, Formats.JSONV2), + ; + + final String _accept; + final String _expectedContentType; + + GetOneTest(String accept, String expectedContentType) + { + _accept = accept; + _expectedContentType = expectedContentType; + } + } + + enum GetAllLegacyTest + { + JSON(Formats.JSON_LEGACY, Formats.JSON), + XML(Formats.XML_LEGACY, Formats.XML), + ; + + final String _queryParam; + final String _expectedContentType; + + GetAllLegacyTest(String queryParam, String expectedContentType) + { + _queryParam = queryParam; + _expectedContentType = expectedContentType; + } + } + + enum GetAllTest + { + DEFAULT(Formats.DEFAULT, Formats.XMLV2), + XML(Formats.XML, Formats.XMLV2), + XMLV1(Formats.XMLV1, Formats.XMLV1), + XMLV2(Formats.XMLV2, Formats.XMLV2), + JSON(Formats.JSON, Formats.JSONV2), + JSONV1(Formats.JSONV1, Formats.JSONV1), + JSONV2(Formats.JSONV2, Formats.JSONV2), + ; + + final String _accept; + final String _expectedContentType; + + GetAllTest(String accept, String expectedContentType) + { + _accept = accept; + _expectedContentType = expectedContentType; + } + } }