Skip to content

Resolving issues identified from static analysis #1540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RyanM-RMA merged 7 commits into from
Jan 31, 2026
Merged

Resolving issues identified from static analysis #1540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d142d94
Replace `UnsupportedOperationException` with standardized "Not Implem…
RyanM-RMA Jan 19, 2026
cb84360
Adding documentation for undocumented query parameters and removing d…
RyanM-RMA Jan 27, 2026
36ed842
Unhide and document `CASCADE_DELETE` query parameter in `LocationCont…
RyanM-RMA Jan 27, 2026
937b445
Refactor Controllers to extend `BaseCrudHandler` or `BaseHandler`, st…
RyanM-RMA Jan 30, 2026
b2f3ee1
Merge branch 'develop' into bugfix/cda-68_resolving_static_analysis_i…
RyanM-RMA Jan 30, 2026
5afccac
Fixing issues from merge
RyanM-RMA Jan 30, 2026
79e7b40
Adding documentation for deprecated fields
RyanM-RMA Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions cwms-data-api/src/main/java/cwms/cda/api/BaseCrudHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@
import com.codahale.metrics.Histogram;
import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.Timer;
import com.google.common.flogger.FluentLogger;
import io.javalin.apibuilder.CrudHandler;
import io.javalin.http.Context;
import static com.codahale.metrics.MetricRegistry.name;
import static cwms.cda.api.Controllers.RESULTS;
import static cwms.cda.api.Controllers.SIZE;

public abstract class BaseCrudHandler implements CrudHandler {
private static final FluentLogger LOGGER = FluentLogger.forEnclosingClass();
private final MetricRegistry metrics;
private final Histogram requestResultSize;

Expand All @@ -46,4 +49,21 @@ protected final Timer.Context markAndTime(String subject) {
protected final void updateResultSize(String responseString) {
requestResultSize.update(responseString.length());
}

protected final void updateResultSize(int responseLength) {
requestResultSize.update(responseLength);
}

protected final void updateResultSize(long responseLength) {
requestResultSize.update(responseLength);
}

public MetricRegistry getMetrics() {
return metrics;
}

protected final void logUnusedPathParameter(Context ctx, String pathParam, String reason) {
String param = ctx.pathParam(pathParam);
LOGGER.atFinest().log("Path parameter '%s' is documented but not used in handler '%s'\nValue: '%s'\nReason: '%s'", pathParam, this.getClass().getSimpleName(), param, reason);
}
}
12 changes: 12 additions & 0 deletions cwms-data-api/src/main/java/cwms/cda/api/BaseHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
import com.codahale.metrics.Histogram;
import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.Timer;
import com.google.common.flogger.FluentLogger;
import io.javalin.http.Context;
import io.javalin.http.Handler;
import static com.codahale.metrics.MetricRegistry.name;
import static cwms.cda.api.Controllers.RESULTS;
import static cwms.cda.api.Controllers.SIZE;

public abstract class BaseHandler implements Handler {

private static final FluentLogger LOGGER = FluentLogger.forEnclosingClass();
private final MetricRegistry metrics;
private final Histogram requestResultSize;

Expand All @@ -47,4 +50,13 @@ protected final Timer.Context markAndTime(String subject) {
protected final void updateResultSize(int value) {
requestResultSize.update(value);
}

protected final void updateResultSize(long value) {
requestResultSize.update(value);
}

protected final void logUnusedPathParameter(Context ctx, String pathParam, String reason) {
String param = ctx.pathParam(pathParam);
LOGGER.atFinest().log("Path parameter '%s' is documented but not used in handler '%s'\nValue: '%s'\nReason: '%s'", pathParam, this.getClass().getSimpleName(), param, reason);
}
}
20 changes: 7 additions & 13 deletions cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,18 @@
import org.jooq.DSLContext;


public class BinaryTimeSeriesController implements CrudHandler {
public class BinaryTimeSeriesController extends BaseCrudHandler {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
static final String TAG = "Binary-TimeSeries";

public static final String REPLACE_ALL = "replace-all";
private static final String DEFAULT_BIN_TYPE_MASK = "*";
public static final String BINARY_TYPE_MASK = "binary-type-mask";
private final MetricRegistry metrics;



public BinaryTimeSeriesController(MetricRegistry metrics) {
this.metrics = metrics;
super(metrics);
}

@NotNull
Expand All @@ -91,11 +90,6 @@ protected TimeSeriesBinaryDao getDao(DSLContext dsl) {
}


private Timer.Context markAndTime(String subject) {
return Controllers.markAndTime(metrics, getClass().getName(), subject);
}


@OpenApi(
summary = "Retrieve binary time series values for a provided time window and date version."
+ "If individual values exceed 64 kilobytes, a URL to a separate download is "
Expand Down Expand Up @@ -179,7 +173,7 @@ public void getAll(@NotNull Context ctx) {
@OpenApi(ignore = true)
@Override
public void getOne(@NotNull Context ctx, @NotNull String templateId) {
throw new UnsupportedOperationException(NOT_SUPPORTED_YET);
ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(CdaError.notImplemented());
}

@OpenApi(
Expand Down Expand Up @@ -232,8 +226,8 @@ public void create(@NotNull Context ctx) {
tags = {TAG}
)
@Override
public void update(@NotNull Context ctx, @NotNull String oldBinaryTimeSeriesId) {

public void update(@NotNull Context ctx, @NotNull String name) {
logUnusedPathParameter(ctx, NAME, "Body contains information");
try (Timer.Context ignored = markAndTime(UPDATE)) {
boolean maxVersion = true;
boolean replaceAll = ctx.queryParamAsClass(REPLACE_ALL, Boolean.class).getOrDefault(false);
Expand Down Expand Up @@ -276,7 +270,7 @@ public void update(@NotNull Context ctx, @NotNull String oldBinaryTimeSeriesId)
tags = {TAG}
)
@Override
public void delete(@NotNull Context ctx, @NotNull String binaryTimeSeriesId) {
public void delete(@NotNull Context ctx, @NotNull String name) {
try (Timer.Context ignored = markAndTime(DELETE)) {
DSLContext dsl = getDslContext(ctx);
String office = requiredParam(ctx, OFFICE);
Expand All @@ -289,7 +283,7 @@ public void delete(@NotNull Context ctx, @NotNull String binaryTimeSeriesId) {

TimeSeriesBinaryDao dao = getDao(dsl);

dao.delete(office, binaryTimeSeriesId, mask, begin, end, version);
dao.delete(office, name, mask, begin, end, version);

ctx.status(HttpServletResponse.SC_NO_CONTENT);
}
Expand Down
24 changes: 7 additions & 17 deletions cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesValueController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.codahale.metrics.Histogram;
import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.Timer;
import com.google.common.flogger.FluentLogger;
import cwms.cda.api.errors.CdaError;
import cwms.cda.data.dao.BlobDao;
import cwms.cda.data.dao.StreamConsumer;
Expand All @@ -47,18 +48,11 @@
import static cwms.cda.data.dao.JooqDao.getDslContext;


public class BinaryTimeSeriesValueController implements Handler {
private final MetricRegistry metrics;
private final Histogram requestResultSize;

public class BinaryTimeSeriesValueController extends BaseHandler {
private static final FluentLogger LOGGER = FluentLogger.forEnclosingClass();

public BinaryTimeSeriesValueController(MetricRegistry metrics) {
this.metrics = metrics;
requestResultSize = this.metrics.histogram((name(BinaryTimeSeriesValueController.class, RESULTS, SIZE)));
}

private Timer.Context markAndTime(String subject) {
return Controllers.markAndTime(metrics, getClass().getName(), subject);
super(metrics);
}

@OpenApi(
Expand All @@ -69,12 +63,6 @@ private Timer.Context markAndTime(String subject) {
queryParams = {
@OpenApiParam(name = OFFICE, required = true, description = "Specifies the owning office of "
+ "the Binary TimeSeries whose data is to be included in the response."),
@OpenApiParam(name = TIMEZONE, description = "Specifies "
+ "the time zone of the values of the begin and end fields (unless "
+ "otherwise specified). If this field is not specified, "
+ "the default time zone of UTC shall be used."),
@OpenApiParam(name = DATE, required = true, description = "The date of the binary value to retrieve"),
@OpenApiParam(name = VERSION_DATE, description = "The version date for the value to retrieve."),
@OpenApiParam(name = BLOB_ID, description = "Will be removed in a schema update. " +
"This is a placeholder for integration testing with schema 23.3.16", deprecated = true)
},
Expand All @@ -89,6 +77,8 @@ private Timer.Context markAndTime(String subject) {
public void handle(@NotNull Context ctx) {
//Implementation will change with new CWMS schema
//https://www.hec.usace.army.mil/confluence/display/CWMS/2024-02-29+Task2A+Text-ts+and+Binary-ts+Design
logUnusedPathParameter(ctx, NAME, "Handled as " + BLOB_ID + " in query parameter. May change with schema.");

try (Timer.Context ignored = markAndTime(GET_ALL)) {
String binaryId = requiredParam(ctx, BLOB_ID);
String officeId = requiredParam(ctx, OFFICE);
Expand All @@ -113,7 +103,7 @@ public void handle(@NotNull Context ctx) {
ctx.status(HttpServletResponse.SC_NOT_FOUND).json(new CdaError("Unable to find "
+ "blob based on given parameters"));
} else {
requestResultSize.update(totalLength);
updateResultSize(totalLength);
RangeRequestUtil.seekableStream(ctx, is, isPosition, mediaType, totalLength);
}
};
Expand Down
39 changes: 21 additions & 18 deletions cwms-data-api/src/main/java/cwms/cda/api/BlobController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,12 @@
/**
*
*/
public class BlobController implements CrudHandler {
public class BlobController extends BaseCrudHandler {
private static final int DEFAULT_PAGE_SIZE = 20;
public static final String TAG = "Blob";

private final MetricRegistry metrics;


private final Histogram requestResultSize;

public BlobController(MetricRegistry metrics) {
this.metrics = metrics;
String className = BlobController.class.getName();

requestResultSize = this.metrics.histogram((name(className, RESULTS, SIZE)));
}

private Timer.Context markAndTime(String subject) {
return Controllers.markAndTime(metrics, getClass().getName(), subject);
super(metrics);
}

protected DSLContext getDslContext(Context ctx) {
Expand Down Expand Up @@ -91,6 +79,11 @@ private BlobAccess chooseBlobAccess(DSLContext dsl) {
+ "identifies where in the request you are. This is an opaque"
+ " value, and can be obtained from the 'next-page' value in "
+ "the response."),
@OpenApiParam(name = CURSOR, deprecated = true,
description = "This end point can return a lot of data, this "
+ "identifies where in the request you are. This is an opaque"
+ " value, and can be obtained from the 'next-page' value in "
+ "the response. Deprecated, use " + PAGE + " instead."),
@OpenApiParam(name = PAGE_SIZE,
type = Integer.class,
description = "How many entries per page returned. Default "
Expand All @@ -116,7 +109,7 @@ public void getAll(@NotNull Context ctx) {
String office = ctx.queryParam(OFFICE);

String cursor = queryParamAsClass(ctx, new String[]{PAGE, CURSOR},
String.class, "", metrics, name(BlobController.class.getName(), GET_ALL));
String.class, "", getMetrics(), name(BlobController.class.getName(), GET_ALL));

if (!CwmsDTOPaginated.CURSOR_CHECK.invoke(cursor)) {
ctx.json(new CdaError("cursor or page passed in but failed validation"))
Expand All @@ -125,7 +118,7 @@ public void getAll(@NotNull Context ctx) {
}

int pageSize = queryParamAsClass(ctx, new String[]{PAGE_SIZE},
Integer.class, DEFAULT_PAGE_SIZE, metrics,
Integer.class, DEFAULT_PAGE_SIZE, getMetrics(),
name(BlobController.class.getName(), GET_ALL));

String like = ctx.queryParamAsClass(LIKE, String.class).getOrDefault(".*");
Expand All @@ -140,13 +133,21 @@ public void getAll(@NotNull Context ctx) {

ctx.result(result);
ctx.contentType(contentType.toString());
requestResultSize.update(result.length());
updateResultSize(result.length());
}
}

@OpenApi(
description = "Returns the binary value of the requested blob as a seekable stream with the "
+ "appropriate media type.",
pathParams = {
@OpenApiParam(name = BLOB_ID, description = "If the _query_ parameter is provided this _path_ parameter "
+ "is ignored and the value of the query parameter is used. "
+ "Note: the _query_ parameter is necessary for id's that contain '/' or other special "
+ "characters. This is due to limitations in path pattern matching. "
+ "We will likely add support for encoding the ID in the path in the future. For now use the id field for those IDs. "
+ "Client libraries should detect slashes and choose the appropriate field. \"ignored\" is suggested for the path endpoint."),
},
queryParams = {
@OpenApiParam(name = OFFICE, description = "Specifies the owning office."),
@OpenApiParam(name = BLOB_ID, description = "If this _query_ parameter is provided the id _path_ parameter "
Expand Down Expand Up @@ -198,7 +199,7 @@ public void getOne(@NotNull Context ctx, @NotNull String blobId) {
ctx.status(HttpServletResponse.SC_NOT_FOUND).json(new CdaError("Unable to find "
+ "blob based on given parameters"));
} else {
requestResultSize.update(totalLength);
updateResultSize(totalLength);
// is OracleBlobInputStream or something from MinIO
RangeRequestUtil.seekableStream(ctx, is, isPosition, mediaType, totalLength);
}
Expand Down Expand Up @@ -265,6 +266,8 @@ public void create(@NotNull Context ctx) {
)
@Override
public void update(@NotNull Context ctx, @NotNull String blobId) {
logUnusedPathParameter(ctx, BLOB_ID, "Body contains information");

try (final Timer.Context ignored = markAndTime(UPDATE)) {
String idQueryParam = ctx.queryParam(BLOB_ID);
if (idQueryParam != null) {
Expand Down
15 changes: 10 additions & 5 deletions cwms-data-api/src/main/java/cwms/cda/api/CatalogController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map;
import java.util.Set;
import com.google.common.flogger.FluentLogger;
import javax.servlet.http.HttpServletResponse;
import org.jetbrains.annotations.NotNull;
import org.jooq.DSLContext;
import org.owasp.html.PolicyFactory;
Expand Down Expand Up @@ -63,19 +64,19 @@ private Timer.Context markAndTime(String subject) {
@OpenApi(tags = {TAG}, ignore = true)
@Override
public void create(Context ctx) {
ctx.status(HttpCode.NOT_IMPLEMENTED).result("cannot perform this action");
ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(CdaError.notImplemented());
}

@OpenApi(tags = {"Catalog"}, ignore = true)
@Override
public void delete(Context ctx, @NotNull String entry) {
ctx.status(HttpCode.NOT_IMPLEMENTED).result("cannot perform this action");
ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(CdaError.notImplemented());
}

@OpenApi(tags = {"Catalog"}, ignore = true)
@Override
public void getAll(Context ctx) {
ctx.status(HttpCode.NOT_IMPLEMENTED).result("cannot perform this action");
ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(CdaError.notImplemented());
}

@OpenApi(
Expand All @@ -84,7 +85,11 @@ public void getAll(Context ctx) {
description = "This end point can return a lot of data, this "
+ "identifies where in the request you are."
),

@OpenApiParam(name = CURSOR, deprecated = true,
description = "This end point can return a lot of data, this "
+ "identifies where in the request you are. This is an opaque"
+ " value, and can be obtained from the 'next-page' value in "
+ "the response. Deprecated, use " + PAGE + " instead."),
@OpenApiParam(name = PAGE_SIZE,
type = Integer.class,
description = "How many entries per page returned. Default 500."
Expand Down Expand Up @@ -311,7 +316,7 @@ private static void warnAboutNotSupported(@NotNull Context ctx, String[] warnAbo
@OpenApi(tags = {"Catalog"}, ignore = true)
@Override
public void update(Context ctx, @NotNull String entry) {
ctx.status(HttpCode.NOT_IMPLEMENTED).json(CdaError.notImplemented());
ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(CdaError.notImplemented());
}

}
13 changes: 13 additions & 0 deletions cwms-data-api/src/main/java/cwms/cda/api/ClobController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ protected DSLContext getDslContext(Context ctx) {
+ "identifies where in the request you are. This is an opaque"
+ " value, and can be obtained from the 'next-page' value in "
+ "the response."),
@OpenApiParam(name = CURSOR, deprecated = true,
description = "This end point can return a lot of data, this "
+ "identifies where in the request you are. This is an opaque"
+ " value, and can be obtained from the 'next-page' value in "
+ "the response. Deprecated, use " + PAGE + " instead."),
@OpenApiParam(name = PAGE_SIZE,
type = Integer.class,
description = "How many entries per page returned. Default "
Expand Down Expand Up @@ -152,6 +157,14 @@ public void getAll(@NotNull Context ctx) {
+ "When the accept header is set to " + Formats.JSONV2 + " the clob will be returned as a serialized Clob "
+ "object with fields for office-id, id, description and value. "
+ "For more information about accept header usage, <a href=\"legacy-format/\">see this page.</a>",
pathParams = {
@OpenApiParam(name = CLOB_ID, description = "If the _query_ parameter is provided this _path_ parameter "
+ "is ignored and the value of the query parameter is used. "
+ "Note: the query parameter is necessary for id's that contain '/' or other special "
+ "characters. This is due to limitations in path pattern matching. "
+ "We will likely add support for encoding the ID in the path in the future. For now use the id field for those IDs. "
+ "Client libraries should detect slashes and choose the appropriate field. \"ignored\" is suggested for the path endpoint."),
},
queryParams = {
@OpenApiParam(name = OFFICE, description = "Specifies the owning office."),
@OpenApiParam(name = CLOB_ID, description = "If this _query_ parameter is provided the id _path_ parameter "
Expand Down
1 change: 1 addition & 0 deletions cwms-data-api/src/main/java/cwms/cda/api/Controllers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ public final class Controllers {
private static final String DEPRECATED_CSV = "2024-11-01 CSV is not used often.";

public static final String QUERY = "query";
public static final String INCLUDE_ROLES = "include-roles";


static {
Expand Down
2 changes: 1 addition & 1 deletion cwms-data-api/src/main/java/cwms/cda/api/ForecastFileController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private Timer.Context markAndTime(String subject) {
tags = {ForecastSpecController.TAG}
)
public void handle(@NotNull Context ctx) {
String specId = requiredParam(ctx, NAME);
String specId = ctx.pathParam(NAME);
String office = requiredParam(ctx, OFFICE);
String designator = ctx.queryParamAsClass(DESIGNATOR, String.class).allowNullable().get();
String forecastDate = requiredParam(ctx, FORECAST_DATE);
Expand Down
Loading
Loading