Skip to content

Commit d67a059

Browse files
rma-brysonrma-bryson
authored
CDA-84 - Fixes issue with default units not being correct for legacy json (#1657)
1 parent 97e248b commit d67a059

File tree

7 files changed

+275
-22
lines changed

7 files changed

+275
-22
lines changed

‎cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ public void delete(Context ctx, String id) {
6767
@OpenApiParam(name = FORMAT, deprecated = true, description = "Specifies the"
6868
+ " encoding format of the response. Valid value for the format field"
6969
+ " for this URI are:"
70-
+ "\n* `tab`"
71-
+ "\n* `csv`"
70+
+ "\n* `tab` (deprecated)"
71+
+ "\n* `csv` (deprecated)"
7272
+ "\n* `xml`"
7373
+ "\n* `json` (default)"
7474
+ "\n\nSee <a href=\"legacy-format/\">this page</a> for more information about accept header usage."),

‎cwms-data-api/src/main/java/cwms/cda/data/dao/ParameterDao.java‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,47 @@
11
package cwms.cda.data.dao;
22

33
import cwms.cda.data.dto.Parameter;
4+
import cwms.cda.formatters.ContentType;
5+
import cwms.cda.formatters.Formats;
6+
import cwms.cda.data.dto.ParameterLegacy;
7+
import com.fasterxml.jackson.databind.JsonNode;
8+
import com.fasterxml.jackson.databind.ObjectMapper;
9+
import com.fasterxml.jackson.databind.node.ArrayNode;
10+
import com.fasterxml.jackson.databind.node.ObjectNode;
11+
import cwms.cda.formatters.FormattingException;
12+
import mil.army.usace.hec.metadata.UnitUtil;
413
import org.jooq.DSLContext;
514
import org.jooq.Record;
615
import usace.cwms.db.jooq.codegen.packages.CWMS_CAT_PACKAGE;
716

17+
import java.io.IOException;
818
import java.util.List;
9-
import java.util.stream.Collectors;
19+
import cwms.cda.formatters.json.JsonV2;
20+
21+
import static java.util.stream.Collectors.toList;
1022

1123
public class ParameterDao extends JooqDao<ParameterDao> {
1224

25+
private static final String LEGACY_JSON_FIELD_NAME = "parameters";
26+
1327
public ParameterDao(DSLContext dsl) {
1428
super(dsl);
1529
}
1630

1731
public String getParameters(String format) {
18-
return CWMS_CAT_PACKAGE.call_RETRIEVE_PARAMETERS_F(dsl.configuration(), format);
32+
String retVal = CWMS_CAT_PACKAGE.call_RETRIEVE_PARAMETERS_F(dsl.configuration(), format);
33+
if (Formats.JSON_LEGACY.equals(format)) {
34+
retVal = fixDefaultUnits(format, retVal);
35+
}
36+
return retVal;
1937
}
2038

2139
public List<Parameter> getParametersV2(String office)
2240
{
2341
return CWMS_CAT_PACKAGE.call_CAT_PARAMETER(dsl.configuration(), office)
2442
.stream()
2543
.map(this::buildParameter)
26-
.collect(Collectors.toList());
44+
.collect(toList());
2745
}
2846

2947
private Parameter buildParameter(Record record)
@@ -38,4 +56,36 @@ private Parameter buildParameter(Record record)
3856
String unitDesc = record.get("UNIT_DESCRIPTION", String.class);
3957
return new Parameter(param, baseParam, subParam, subParamDesc, dbOfficeId, dbUnitId, unitLongName, unitDesc);
4058
}
59+
60+
private String fixDefaultUnits(String format, String retVal) {
61+
try {
62+
ObjectMapper mapper = JsonV2.buildObjectMapper();
63+
JsonNode root = mapper.readTree(retVal);
64+
JsonNode wrapper = root.path(LEGACY_JSON_FIELD_NAME);
65+
JsonNode paramsNode = wrapper.path(LEGACY_JSON_FIELD_NAME);
66+
ContentType contentType = new ContentType(format);
67+
List<ParameterLegacy> params = Formats.parseContentList(contentType, paramsNode.toString(), ParameterLegacy.class);
68+
params = params.stream()
69+
.map(this::fixDefaultUnits)
70+
.collect(toList());
71+
ArrayNode newArray = mapper.valueToTree(params);
72+
((ObjectNode) wrapper).set(LEGACY_JSON_FIELD_NAME, newArray);
73+
retVal = mapper.writeValueAsString(root);
74+
} catch (IOException e) {
75+
throw new FormattingException("Error processing legacy JSON: " + e.getMessage(), e);
76+
}
77+
return retVal;
78+
}
79+
80+
private ParameterLegacy fixDefaultUnits(ParameterLegacy parameterLegacy) {
81+
var param = mil.army.usace.hec.metadata.Parameter.getParameterForUnitsString(parameterLegacy.getDefaultSiUnit());
82+
String siUnits = param.getUnitsStringForSystem(UnitUtil.SI_ID);
83+
String enUnits = param.getUnitsStringForSystem(UnitUtil.ENGLISH_ID);
84+
parameterLegacy = new ParameterLegacy.Builder()
85+
.from(parameterLegacy)
86+
.withDefaultSiUnit(siUnits)
87+
.withDefaultEnglishUnit(enUnits)
88+
.build();
89+
return parameterLegacy;
90+
}
4191
}

‎cwms-data-api/src/main/java/cwms/cda/data/dto/ParameterLegacy.java‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
package cwms.cda.data.dto;
2+
3+
import com.fasterxml.jackson.annotation.JsonIgnore;
4+
import com.fasterxml.jackson.annotation.JsonInclude;
5+
import com.fasterxml.jackson.annotation.JsonRootName;
6+
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
7+
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
8+
import com.fasterxml.jackson.databind.annotation.JsonNaming;
9+
import cwms.cda.formatters.Formats;
10+
import cwms.cda.formatters.annotations.FormattableWith;
11+
import cwms.cda.formatters.json.JsonV1;
12+
13+
/**
14+
* DTO used exclusively for legacy JSON (version 1) parameter payloads.
15+
* The property names follow kebab-case to match the legacy API fields.
16+
*/
17+
@FormattableWith(contentType = Formats.JSON_LEGACY, formatter = JsonV1.class, aliases = {Formats.DEFAULT, Formats.JSON, Formats.JSONV1})
18+
@JsonRootName("parameter")
19+
@JsonDeserialize(builder = ParameterLegacy.Builder.class)
20+
@JsonInclude(JsonInclude.Include.NON_NULL)
21+
@JsonNaming(PropertyNamingStrategies.KebabCaseStrategy.class)
22+
public final class ParameterLegacy extends CwmsDTOBase {
23+
24+
private final String abstractParam;
25+
private final String name;
26+
private final String office;
27+
private final String defaultEnglishUnit;
28+
private final String defaultSiUnit;
29+
private final String longName;
30+
private final String description;
31+
32+
private ParameterLegacy(Builder builder) {
33+
this.abstractParam = builder.abstractParam;
34+
this.name = builder.name;
35+
this.office = builder.office;
36+
this.defaultEnglishUnit = builder.defaultEnglishUnit;
37+
this.defaultSiUnit = builder.defaultSiUnit;
38+
this.longName = builder.longName;
39+
this.description = builder.description;
40+
}
41+
42+
public String getAbstractParam() {
43+
return abstractParam;
44+
}
45+
46+
public String getName() {
47+
return name;
48+
}
49+
50+
public String getOffice() {
51+
return office;
52+
}
53+
54+
public String getDefaultEnglishUnit() {
55+
return defaultEnglishUnit;
56+
}
57+
58+
public String getDefaultSiUnit() {
59+
return defaultSiUnit;
60+
}
61+
62+
public String getLongName() {
63+
return longName;
64+
}
65+
66+
public String getDescription() {
67+
return description;
68+
}
69+
70+
public static class Builder {
71+
private String abstractParam;
72+
private String name;
73+
private String office;
74+
private String defaultEnglishUnit;
75+
private String defaultSiUnit;
76+
private String longName;
77+
private String description;
78+
79+
public Builder withAbstractParam(String abstractParam) {
80+
this.abstractParam = abstractParam;
81+
return this;
82+
}
83+
84+
public Builder withName(String name) {
85+
this.name = name;
86+
return this;
87+
}
88+
89+
public Builder withOffice(String office) {
90+
this.office = office;
91+
return this;
92+
}
93+
94+
public Builder withDefaultEnglishUnit(String defaultEnglishUnit) {
95+
this.defaultEnglishUnit = defaultEnglishUnit;
96+
return this;
97+
}
98+
99+
public Builder withDefaultSiUnit(String defaultSiUnit) {
100+
this.defaultSiUnit = defaultSiUnit;
101+
return this;
102+
}
103+
104+
public Builder withLongName(String longName) {
105+
this.longName = longName;
106+
return this;
107+
}
108+
109+
public Builder withDescription(String description) {
110+
this.description = description;
111+
return this;
112+
}
113+
114+
@JsonIgnore
115+
public Builder from(ParameterLegacy parameter) {
116+
this.abstractParam = parameter.getAbstractParam();
117+
this.name = parameter.getName();
118+
this.office = parameter.getOffice();
119+
this.defaultEnglishUnit = parameter.getDefaultEnglishUnit();
120+
this.defaultSiUnit = parameter.getDefaultSiUnit();
121+
this.longName = parameter.getLongName();
122+
this.description = parameter.getDescription();
123+
return this;
124+
}
125+
126+
public ParameterLegacy build() {
127+
return new ParameterLegacy(this);
128+
}
129+
}
130+
}

‎cwms-data-api/src/test/java/cwms/cda/api/ParametersControllerTestIT.java‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import static cwms.cda.api.Controllers.OFFICE;
1313
import static io.restassured.RestAssured.given;
1414
import static org.hamcrest.Matchers.is;
15+
import static org.hamcrest.Matchers.greaterThan;
16+
import io.restassured.response.ValidatableResponse;
1517

1618
@Tag("integration")
1719
class ParametersControllerTestIT extends DataApiTestIT
@@ -37,23 +39,29 @@ void test_get_all_parameters(GetAllTest test)
3739
.contentType(is(test._expectedContentType));
3840
}
3941

40-
@ParameterizedTest
41-
@EnumSource(GetAllLegacyTest.class)
42-
void test_get_all_parameters_legacy_types(GetAllLegacyTest test)
43-
{
44-
given()
45-
.log().ifValidationFails(LogDetail.ALL,true)
46-
.queryParam(FORMAT, test._accept)
47-
.when()
48-
.redirects().follow(true)
49-
.redirects().max(3)
50-
.get("/parameters/")
51-
.then()
52-
.log().ifValidationFails(LogDetail.ALL,true)
53-
.assertThat()
54-
.statusCode(is(HttpServletResponse.SC_OK))
55-
.contentType(is(test._expectedContentType));
56-
}
42+
@ParameterizedTest
43+
@EnumSource(GetAllLegacyTest.class)
44+
void test_get_all_parameters_legacy_types(GetAllLegacyTest test)
45+
{
46+
ValidatableResponse response = given()
47+
.log().ifValidationFails(LogDetail.ALL,true)
48+
.queryParam(FORMAT, test._accept)
49+
.when()
50+
.redirects().follow(true)
51+
.redirects().max(3)
52+
.get("/parameters/")
53+
.then()
54+
.log().ifValidationFails(LogDetail.ALL,true)
55+
.assertThat()
56+
.statusCode(is(HttpServletResponse.SC_OK))
57+
.contentType(is(test._expectedContentType));
58+
59+
// ensure default-english-unit and default-si-unit values are no longer the same
60+
if (test == GetAllLegacyTest.JSON) {
61+
response.body("parameters.parameters.findAll { it.'default-english-unit' != it.'default-si-unit' }.size()",
62+
greaterThan(0));
63+
}
64+
}
5765

5866
enum GetAllLegacyTest
5967
{

‎cwms-data-api/src/test/java/cwms/cda/data/dto/ParameterLegacyTest.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package cwms.cda.data.dto;
2+
3+
import cwms.cda.formatters.ContentType;
4+
import cwms.cda.formatters.Formats;
5+
import cwms.cda.helpers.DTOMatch;
6+
import org.apache.commons.io.IOUtils;
7+
import org.junit.jupiter.api.Test;
8+
9+
import java.io.InputStream;
10+
import java.nio.charset.StandardCharsets;
11+
12+
import static cwms.cda.helpers.DTOMatch.assertMatch;
13+
import static org.junit.jupiter.api.Assertions.assertNotNull;
14+
15+
public class ParameterLegacyTest {
16+
17+
@Test
18+
void test_serialization() {
19+
ParameterLegacy expected = new ParameterLegacy.Builder()
20+
.withAbstractParam("Area")
21+
.withName("Area")
22+
.withOffice("All")
23+
.withDefaultEnglishUnit("ft2")
24+
.withDefaultSiUnit("m2")
25+
.withLongName("Surface Area")
26+
.withDescription("Area of a surface")
27+
.build();
28+
ContentType contentType = new ContentType(Formats.JSON_LEGACY);
29+
String json = Formats.format(contentType, expected);
30+
ParameterLegacy parsedParam = Formats.parseContent(contentType, json, ParameterLegacy.class);
31+
assertMatch(expected, parsedParam);
32+
}
33+
34+
@Test
35+
void test_from_resource_file() throws Exception {
36+
InputStream resource = getClass().getClassLoader().getResourceAsStream("cwms/cda/data/dto/parameter_legacy.json");
37+
assertNotNull(resource);
38+
String json = IOUtils.toString(resource, StandardCharsets.UTF_8);
39+
ContentType contentType = new ContentType(Formats.JSON_LEGACY);
40+
ParameterLegacy receivedTzs = Formats.parseContent(contentType, json, ParameterLegacy.class);
41+
assertNotNull(receivedTzs);
42+
}
43+
}

‎cwms-data-api/src/test/java/cwms/cda/helpers/DTOMatch.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import cwms.cda.data.dto.CwmsIdTimeExtentsEntry;
2828
import cwms.cda.data.dto.Entity;
29+
import cwms.cda.data.dto.ParameterLegacy;
2930
import cwms.cda.data.dto.TimeExtents;
3031
import cwms.cda.data.dto.TimeSeriesExtents;
3132
import cwms.cda.data.dto.catalog.LocationAlias;
@@ -507,6 +508,18 @@ public static void assertMatch(StreamflowMeasurement first, StreamflowMeasuremen
507508
);
508509
}
509510

511+
public static void assertMatch(ParameterLegacy first, ParameterLegacy second) {
512+
assertAll(
513+
() -> assertEquals(first.getAbstractParam(), second.getAbstractParam(), "Abstract parameter does not match"),
514+
() -> assertEquals(first.getName(), second.getName(), "Parameter name does not match"),
515+
() -> assertEquals(first.getOffice(), second.getOffice(), "Office does not match"),
516+
() -> assertEquals(first.getDefaultEnglishUnit(), second.getDefaultEnglishUnit(), "Default English unit does not match"),
517+
() -> assertEquals(first.getDefaultSiUnit(), second.getDefaultSiUnit(), "Default SI unit does not match"),
518+
() -> assertEquals(first.getLongName(), second.getLongName(), "Long name does not match"),
519+
() -> assertEquals(first.getDescription(), second.getDescription(), "Description does not match")
520+
);
521+
}
522+
510523
public static void assertMatch(SupplementalStreamflowMeasurement first, SupplementalStreamflowMeasurement second) {
511524
assertAll(
512525
() -> assertEquals(first.getChannelFlow(), second.getChannelFlow(), DEFAULT_DELTA, "Channel flow does not match"),

‎cwms-data-api/src/test/resources/cwms/cda/data/dto/parameter_legacy.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"abstract-param": "Area",
3+
"name": "Area",
4+
"office": "All",
5+
"default-english-unit": "ft2",
6+
"default-si-unit": "m2",
7+
"long-name": "Surface Area",
8+
"description": "Area of a surface"
9+
}

0 commit comments

Comments
 (0)