CDA-98 Workaround to updating forecast location for a spec#1647
CDA-98 Workaround to updating forecast location for a spec#1647rma-bryson wants to merge 4 commits intodevelopfrom
Conversation
rma-bryson
changed the title
rma-bryson
force-pushed
the
bugfix/bad_forecast_spec_behavior
branch
from
3952168 to
c15971f
Compare
rma-bryson
force-pushed
the
bugfix/bad_forecast_spec_behavior
branch
from
c15971f to
eca6345
Compare
rma-bryson
changed the title
cwms-data-api/src/main/java/cwms/cda/data/dao/ForecastSpecDao.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/ForecastSpecControllerTestIT.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/data/dao/ForecastSpecDao.java
Outdated
Show resolved
Hide resolved
| DeleteRule.DELETE_ALL.getRule(), OFFICE); | ||
| CWMS_FCST_PACKAGE.call_DELETE_FCST_SPEC(OracleDSL.using(c).configuration(), SPEC_ID + "-NULL-DESIGNATOR", null, | ||
| DeleteRule.DELETE_ALL.getRule(), OFFICE); | ||
| Map<String, Optional<String>> specs = Map.of( |
There was a problem hiding this comment.
Usage of Optional is intended primarily for public API. Nulls are fine in local scope as you control where and how they are handled.
There was a problem hiding this comment.
Updated to use null. Side note: Map.of(...) annotates both the key and values as NotNull, but for this purpose I think its fine for the reasons you stated.
| }); | ||
| } | ||
|
|
||
| public void updateSpecWithLocationIdChange(ForecastSpec forecastSpec) { |
There was a problem hiding this comment.
This does not only update the location id, it updates all fields given your deletes and re-store call.
Additionally, if the location id is the only the thing that needs to be updated, and this is a workaround, direct table edits are much better so you can avoid such destructive calls.
| dao.create(forecastSpec); | ||
| try { | ||
| ForecastSpec existing = dao.getForecastSpec(forecastSpec.getOfficeId(), forecastSpec.getSpecId(), forecastSpec.getDesignator()); | ||
| if(locationsAreDifferent(forecastSpec, existing)) { |
There was a problem hiding this comment.
This reverts previous behavior of allowing POST to provide full updates to the spec
There was a problem hiding this comment.
good catch. Fixed
2 participants
…ation id, does not overwrite the original spec, but creates a new one for the new location.