Skip to content

Commit 83d8740

Browse files
MikeNeilsonMikeNeilson
committed
Correct general behavior in retrieveLocationLevel (#1245)
Location levels could only be retrieved by exact data. Expected behavior was most recent. during #1086 the "exact" match flag was changed from true to false. Change is behavior was not correctly caught due to all tests using the exact dates and the endpoint itself not providing appropriate description. Description has been updated. The virtual location level tests were modified to specify exact date. In diagnosing the issue I've discovered that the order of these operations is wrong: https://github.com/HydrologicEngineeringCenter/cwms-database/blob/f2670f46de2f41b724ac9d516505fa9a37c50cfa/schema/src/cwms/cwms_level_pkg_body.sql#L612 Virtual location levels should still support "or nearest before behavior" but at the least with match being false while providing the exact date should return the level and it does not. I've set the default behavior of exact match to False, knowing this will break existing virtual location level behavior, however, that will be corrected with a database procedure fix. (cherry picked from commit b2dfea9)
1 parent aaaadd1 commit 83d8740

8 files changed

Lines changed: 90 additions & 19 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ public final class Controllers {
110110
public static final String TEMPLATE_ID_MASK = "template-id-mask";
111111
public static final String STORE_TEMPLATE = "store-template";
112112
public static final String REPLACE_BASE_CURVE = "replace-base-curve";
113+
public static final String EFFECTIVE_DATE_EXACT = "use-exact-effective-date";
113114

114115
public static final String TIMESERIES_ID_REGEX = "timeseries-id-regex";
115116
public static final String TIMESERIES_ID = "timeseries-id";

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,14 @@ public void getAll(@NotNull Context ctx) {
330330
@OpenApiParam(name = OFFICE, required = true, description = "Specifies the "
331331
+ "office of the Location Level to be returned"),
332332
@OpenApiParam(name = EFFECTIVE_DATE, required = true, description = "Specifies "
333-
+ "the effective date of Location Level to be returned. "
333+
+ "the effective date of Location Level to be returned."
334334
+ "Expected formats are `YYYY-MM-DDTHH:MM` or `YYYY-MM-DDTHH:MM:SS`"),
335+
@OpenApiParam(name= EFFECTIVE_DATE_EXACT, required = false, description = "If true"
336+
+ " only a level with the exact provided date will be returned. If false"
337+
+ " The most recent level on or before this time will be returned."
338+
+ " The default is false.",
339+
type = Boolean.class
340+
),
335341
@OpenApiParam(name = TIMEZONE, description = "Specifies the time zone of "
336342
+ "the values of the effective date field (unless otherwise "
337343
+ "specified), as well as the time zone of any times in the response."
@@ -364,6 +370,8 @@ public void getOne(@NotNull Context ctx, @NotNull String levelId) {
364370
String dateString = queryParamAsClass(ctx, new String[]{EFFECTIVE_DATE, DATE},
365371
String.class, null, metrics, name(LevelsController.class.getName(),
366372
GET_ONE));
373+
boolean exactDateMatch = queryParamAsClass(ctx, new String[]{EFFECTIVE_DATE_EXACT},
374+
Boolean.class, false, metrics, name(LevelsController.class.getName(),GET_ONE));
367375
String timezone = ctx.queryParamAsClass(TIMEZONE, String.class)
368376
.getOrDefault("UTC");
369377

@@ -374,7 +382,7 @@ String.class, null, metrics, name(LevelsController.class.getName(),
374382
LocationLevelsDao levelsDao = getLevelsDao(dsl);
375383
//retrieveLocationLevel will throw an error if level does not exist
376384
LocationLevel locationLevel = levelsDao.retrieveLocationLevel(levelId,
377-
units, unmarshalledDateTime, office);
385+
units, unmarshalledDateTime, office, exactDateMatch);
378386
ctx.json(locationLevel);
379387
ctx.status(HttpServletResponse.SC_OK);
380388
}
@@ -431,7 +439,7 @@ public void update(@NotNull Context ctx, @NotNull String oldLevelId) {
431439
ZoneId.systemDefault().getId());
432440
//retrieveLocationLevel will throw an error if level does not exist
433441
LocationLevel existingLevelLevel = levelsDao.retrieveLocationLevel(oldLevelId,
434-
UnitSystem.EN.getValue(), unmarshalledDateTime, officeId);
442+
UnitSystem.EN.getValue(), unmarshalledDateTime, officeId, true);
435443
existingLevelLevel = updatedClearedFields(ctx.body(), contentType.getType(),
436444
existingLevelLevel);
437445
//only store (update) if level does exist

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ void deleteLocationLevel(String locationLevelName, ZonedDateTime date, String of
4343
void renameLocationLevel(String oldLocationLevelName, String newLocationLevelName, String officeId);
4444

4545
LocationLevel retrieveLocationLevel(String locationLevelName, String unitSystem,
46-
ZonedDateTime effectiveDate, String officeId);
46+
ZonedDateTime effectiveDate, String officeId, boolean exactDateMatch);
4747

4848
String getLocationLevels(String format, String names, String office, String unit,
4949
String datum, String begin,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ public void renameLocationLevel(String oldLocationLevelName, String newLocationL
458458

459459
@Override
460460
public LocationLevel retrieveLocationLevel(String locationLevelName, String pUnits,
461-
ZonedDateTime effectiveDate, String officeId) {
461+
ZonedDateTime effectiveDate, String officeId,boolean exactDateMatch) {
462462
Timestamp date = Timestamp.from(effectiveDate.toInstant());
463463
String[] levelIdParts = locationLevelIdParsingPattern.split(locationLevelName);
464464
if (levelIdParts.length <= 2) {
@@ -483,9 +483,9 @@ public LocationLevel retrieveLocationLevel(String locationLevelName, String pUni
483483
LOCATION_LEVEL_T level = CWMS_LEVEL_PACKAGE.call_RETRIEVE_LOCATION_LEVEL__2(
484484
configuration, locationLevelName, units, date,
485485
"UTC", null, null,
486-
null, "T", officeId, "VN");
486+
null, formatBool(exactDateMatch), officeId, "VN");
487487
if (level == null) {
488-
throw new NotFoundException("Location level not found: " + locationLevelName);
488+
throw new NotFoundException("Location level not found: " + officeId + "/" + locationLevelName);
489489
}
490490
Timestamp pEffectiveDate = level.getLEVEL_DATE();
491491
ZonedDateTime realEffectiveDate = ZonedDateTime.ofInstant(pEffectiveDate.toInstant(), effectiveDate.getZone());

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

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,19 +101,20 @@ void cleanup() throws Exception {
101101
});
102102
}
103103

104-
@Test
105-
void test_location_level() throws Exception {
106-
createLocation("level_as_single_value", true, OFFICE);
104+
@ParameterizedTest
105+
@ValueSource(strings = {"SPK", "SWT", "MVP"})
106+
void test_location_level(String office) throws Exception {
107+
createLocation("level_as_single_value", true, office);
107108
String levelId = "level_as_single_value.Stor.Ave.1Day.Regulating";
108109
ZonedDateTime time = ZonedDateTime.of(2023, 6, 1, 0, 0, 0, 0, ZoneId.of("America/Los_Angeles"));
109110
LocationLevel level = new ConstantLocationLevel.Builder(levelId, time)
110-
.withOfficeId(OFFICE)
111+
.withOfficeId(office)
111112
.withLevelUnitsId("ac-ft")
112113
.withConstantValue(1.0)
113114
.build();
114115
levelList.add(level);
115116
CwmsDataApiSetupCallback.getDatabaseLink().connection(c -> {
116-
DSLContext dsl = dslContext(c, OFFICE);
117+
DSLContext dsl = dslContext(c, office);
117118
LocationLevelsDaoImpl dao = new LocationLevelsDaoImpl(dsl);
118119
dao.storeLocationLevel(level);
119120
});
@@ -123,7 +124,7 @@ void test_location_level() throws Exception {
123124
.log().ifValidationFails(LogDetail.ALL,true)
124125
.accept(Formats.JSONV2)
125126
.contentType(Formats.JSONV2)
126-
.queryParam("office", OFFICE)
127+
.queryParam("office", office)
127128
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
128129
.when()
129130
.redirects().follow(true)
@@ -143,7 +144,7 @@ void test_location_level() throws Exception {
143144
.log().ifValidationFails(LogDetail.ALL,true)
144145
.accept(Formats.JSONV2)
145146
.contentType(Formats.JSONV2)
146-
.queryParam("office", OFFICE)
147+
.queryParam("office", office)
147148
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
148149
.queryParam(UNIT, "ac-ft")
149150
.when()
@@ -156,6 +157,43 @@ void test_location_level() throws Exception {
156157
.statusCode(is(HttpServletResponse.SC_OK))
157158
.body("level-units-id",equalTo("ac-ft"))
158159
.body("constant-value",equalTo(1.0F));
160+
161+
// test modified effective date
162+
given()
163+
.log().ifValidationFails(LogDetail.ALL,true)
164+
.accept(Formats.JSONV2)
165+
.contentType(Formats.JSONV2)
166+
.queryParam("office", office)
167+
.queryParam(EFFECTIVE_DATE, time.plusDays(1L).toInstant().toString())
168+
.queryParam(UNIT, "ac-ft")
169+
.when()
170+
.redirects().follow(true)
171+
.redirects().max(3)
172+
.get("/levels/{level-id}", levelId)
173+
.then()
174+
.assertThat()
175+
.log().ifValidationFails(LogDetail.ALL,true)
176+
.statusCode(is(HttpServletResponse.SC_OK))
177+
.body("level-units-id",equalTo("ac-ft"))
178+
.body("constant-value",equalTo(1.0F));
179+
180+
// test modified effective date using exact match
181+
given()
182+
.log().ifValidationFails(LogDetail.ALL,true)
183+
.accept(Formats.JSONV2)
184+
.contentType(Formats.JSONV2)
185+
.queryParam("office", office)
186+
.queryParam(EFFECTIVE_DATE, time.plusDays(1L).toInstant().toString())
187+
.queryParam(EFFECTIVE_DATE_EXACT, true)
188+
.queryParam(UNIT, "ac-ft")
189+
.when()
190+
.redirects().follow(true)
191+
.redirects().max(3)
192+
.get("/levels/{level-id}", levelId)
193+
.then()
194+
.assertThat()
195+
.log().ifValidationFails(LogDetail.ALL,true)
196+
.statusCode(is(HttpServletResponse.SC_NOT_FOUND));
159197
}
160198

161199
@Test
@@ -968,6 +1006,7 @@ void testStoreRetrieveVirtualLocationLevels() throws Exception {
9681006
.queryParam(Controllers.OFFICE, OFFICE)
9691007
.queryParam(UNIT, "SI")
9701008
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1009+
.queryParam(EFFECTIVE_DATE_EXACT, true)
9711010
.when()
9721011
.redirects().follow(true)
9731012
.redirects().max(3)
@@ -984,6 +1023,7 @@ void testStoreRetrieveVirtualLocationLevels() throws Exception {
9841023
.contentType(Formats.JSONV2)
9851024
.queryParam("office", OFFICE)
9861025
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1026+
.queryParam(EFFECTIVE_DATE_EXACT, true)
9871027
.queryParam(UNIT, "ft")
9881028
.when()
9891029
.redirects().follow(true)
@@ -1001,6 +1041,7 @@ void testStoreRetrieveVirtualLocationLevels() throws Exception {
10011041
.contentType(Formats.JSONV2)
10021042
.queryParam("office", OFFICE)
10031043
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1044+
.queryParam(EFFECTIVE_DATE_EXACT, true)
10041045
.queryParam(UNIT, "EN")
10051046
.when()
10061047
.redirects().follow(true)
@@ -1019,6 +1060,7 @@ void testStoreRetrieveVirtualLocationLevels() throws Exception {
10191060
.contentType(Formats.JSONV2)
10201061
.queryParam("office", OFFICE)
10211062
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1063+
.queryParam(EFFECTIVE_DATE_EXACT, true)
10221064
.queryParam(UNIT, "ft3")
10231065
.when()
10241066
.redirects().follow(true)
@@ -1137,6 +1179,7 @@ void testStoreRetrieveAllVirtualLocationLevels() throws Exception {
11371179
.queryParam(Controllers.OFFICE, OFFICE)
11381180
.queryParam(UNIT, "SI")
11391181
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1182+
.queryParam(EFFECTIVE_DATE_EXACT, true)
11401183
.when()
11411184
.redirects().follow(true)
11421185
.redirects().max(3)
@@ -1154,6 +1197,7 @@ void testStoreRetrieveAllVirtualLocationLevels() throws Exception {
11541197
.queryParam(Controllers.OFFICE, OFFICE)
11551198
.queryParam(UNIT, "SI")
11561199
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1200+
.queryParam(EFFECTIVE_DATE_EXACT, true)
11571201
.when()
11581202
.redirects().follow(true)
11591203
.redirects().max(3)
@@ -1171,6 +1215,7 @@ void testStoreRetrieveAllVirtualLocationLevels() throws Exception {
11711215
.queryParam(Controllers.OFFICE, OFFICE)
11721216
.queryParam(UNIT, "SI")
11731217
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1218+
.queryParam(EFFECTIVE_DATE_EXACT, true)
11741219
.when()
11751220
.redirects().follow(true)
11761221
.redirects().max(3)
@@ -1335,6 +1380,7 @@ void testStoreRetrieveAllVirtualLocationLevelsPaged() throws Exception {
13351380
.queryParam(Controllers.OFFICE, OFFICE)
13361381
.queryParam(UNIT, "SI")
13371382
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1383+
.queryParam(EFFECTIVE_DATE_EXACT, true)
13381384
.when()
13391385
.redirects().follow(true)
13401386
.redirects().max(3)
@@ -1352,6 +1398,7 @@ void testStoreRetrieveAllVirtualLocationLevelsPaged() throws Exception {
13521398
.queryParam(Controllers.OFFICE, OFFICE)
13531399
.queryParam(UNIT, "SI")
13541400
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1401+
.queryParam(EFFECTIVE_DATE_EXACT, true)
13551402
.when()
13561403
.redirects().follow(true)
13571404
.redirects().max(3)
@@ -1369,6 +1416,7 @@ void testStoreRetrieveAllVirtualLocationLevelsPaged() throws Exception {
13691416
.queryParam(Controllers.OFFICE, OFFICE)
13701417
.queryParam(UNIT, "SI")
13711418
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1419+
.queryParam(EFFECTIVE_DATE_EXACT, true)
13721420
.when()
13731421
.redirects().follow(true)
13741422
.redirects().max(3)
@@ -1525,6 +1573,7 @@ void testStoreDeleteVirtualLocationLevel(String deletionMethod) throws Exception
15251573
.queryParam(Controllers.OFFICE, OFFICE)
15261574
.queryParam(UNIT, "SI")
15271575
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1576+
.queryParam(EFFECTIVE_DATE_EXACT, true)
15281577
.when()
15291578
.redirects().follow(true)
15301579
.redirects().max(3)
@@ -1542,6 +1591,7 @@ void testStoreDeleteVirtualLocationLevel(String deletionMethod) throws Exception
15421591
.contentType(Formats.JSONV2)
15431592
.queryParam("office", OFFICE)
15441593
.queryParam(EFFECTIVE_DATE, time.toInstant().toString())
1594+
.queryParam(EFFECTIVE_DATE_EXACT, true)
15451595
.queryParam(UNIT, "ft3")
15461596
.when()
15471597
.redirects().follow(true)

‎cwms-data-api/src/test/java/cwms/cda/data/dao/LocationLevelsDaoTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ void testStore() throws Exception
7070
locationsDao.storeLocation(location, false);
7171
LocationLevelsDao levelsDao = new LocationLevelsDaoImpl(getDslContext(getConnection(), OFFICE_ID));
7272
levelsDao.storeLocationLevel(levelToStore);
73-
LocationLevel retrievedLevel = levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelToStore.getLevelDate(), "LRL");
73+
LocationLevel retrievedLevel = levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelToStore.getLevelDate(), "LRL", true);
7474
assertNotNull(retrievedLevel);
7575
assertEquals(levelToStore.getLocationLevelId(), retrievedLevel.getLocationLevelId());
7676
assertEquals(levelToStore.getLevelDate(), retrievedLevel.getLevelDate());
@@ -92,10 +92,10 @@ void testDeleteLocationLevel() throws Exception
9292
Location location = buildTestLocation("TEST_LOC5");
9393
locationsDao.storeLocation(location, false);
9494
levelsDao.storeLocationLevel(levelToStore);
95-
LocationLevel retrievedLevel = levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelToStore.getLevelDate(), OFFICE_ID);
95+
LocationLevel retrievedLevel = levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelToStore.getLevelDate(), OFFICE_ID, true);
9696
assertNotNull(retrievedLevel);
9797
levelsDao.deleteLocationLevel(levelToStore.getLocationLevelId(), levelToStore.getLevelDate(), OFFICE_ID, true);
98-
assertThrows(IOException.class, () -> levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelToStore.getLevelDate(), OFFICE_ID));
98+
assertThrows(IOException.class, () -> levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelToStore.getLevelDate(), OFFICE_ID, true));
9999
}
100100

101101
@Disabled
@@ -114,7 +114,7 @@ void testUpdate() throws Exception
114114
String format = Formats.JSON;
115115

116116
SeasonalLocationLevel levelFromBody = deserializeLocationLevel(body, Formats.JSON, OFFICE_ID);
117-
existingLocationLevel = (SeasonalLocationLevel) levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelFromBody.getLevelDate(), OFFICE_ID);
117+
existingLocationLevel = (SeasonalLocationLevel) levelsDao.retrieveLocationLevel(levelToStore.getLocationLevelId(), UnitSystem.EN.getValue(), levelFromBody.getLevelDate(), OFFICE_ID, true);
118118
existingLocationLevel = updatedClearedFields(body, format, existingLocationLevel);
119119
//only store (update) if level does exist
120120
updatedLocationLevel = getUpdatedLocationLevel(existingLocationLevel, levelFromBody);
@@ -126,7 +126,7 @@ void testUpdate() throws Exception
126126
} else {
127127
levelsDao.storeLocationLevel(updatedLocationLevel);
128128
}
129-
LocationLevel retrievedLevel = levelsDao.retrieveLocationLevel(updatedLocationLevel.getLocationLevelId(), UnitSystem.EN.getValue(), updatedLocationLevel.getLevelDate(), OFFICE_ID);
129+
LocationLevel retrievedLevel = levelsDao.retrieveLocationLevel(updatedLocationLevel.getLocationLevelId(), UnitSystem.EN.getValue(), updatedLocationLevel.getLevelDate(), OFFICE_ID, true);
130130
assertNotNull(retrievedLevel);
131131
}
132132
finally {

‎cwms-data-api/src/test/java/fixtures/CwmsDataApiSetupCallback.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ private void loadDefaultData(CwmsDatabaseContainer cwmsDb) throws SQLException {
220220
user = cwmsDb.getUsername();
221221
}
222222
logger.atInfo().log(String.format("Running %s as %s %s", data, user, cwmsDb.getPassword()));
223+
logger.atInfo().log("Webuser = " + webUser);
223224
cwmsDb.executeSQL(loadResourceAsString(user_resource[1]).replace("&pduser", cwmsDb.getPdUser())
224225
.replace("&user", cwmsDb.getUsername())
225226
.replace("&webuser", webUser)

‎cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ begin
4141
cwms_sec.add_user_to_group('&webuser','CWMS Users', 'SWT');
4242
cwms_sec.add_user_to_group('&webuser','CWMS PD Users', 'SWT');
4343
cwms_sec.add_user_to_group('&webuser','CWMS DBA Users', 'SWT');
44+
45+
cwms_sec.add_user_to_group('&webuser','All Users', 'MVP');
46+
4447
cwms_sec.add_user_to_group('&user','All Users', 'HQ');
4548
cwms_sec.add_user_to_group('&user','CWMS Users', 'HQ');
4649
cwms_sec.add_user_to_group('&user','CWMS PD Users', 'HQ');
@@ -49,6 +52,14 @@ begin
4952
cwms_sec.add_user_to_group('&user','CWMS Users', 'SPK');
5053
cwms_sec.add_user_to_group('&user','CWMS PD Users', 'SPK');
5154
cwms_sec.add_user_to_group('&user','CWMS DBA Users', 'SPK');
55+
cwms_sec.add_user_to_group('&user','All Users', 'SWT');
56+
cwms_sec.add_user_to_group('&user','CWMS Users', 'SWT');
57+
cwms_sec.add_user_to_group('&user','CWMS PD Users', 'SWT');
58+
cwms_sec.add_user_to_group('&user','CWMS DBA Users', 'SWT');
59+
cwms_sec.add_user_to_group('&user','All Users', 'MVP');
60+
cwms_sec.add_user_to_group('&user','CWMS Users', 'MVP');
61+
cwms_sec.add_user_to_group('&user','CWMS PD Users', 'MVP');
62+
cwms_sec.add_user_to_group('&user','CWMS DBA Users', 'MVP');
5263
execute immediate 'grant execute on cwms_upass to web_user';
5364
/** Add a couple of districts*/
5465
begin

0 commit comments

Comments
 (0)