Feature/1004 update javalin drop tomcat by MikeNeilson · Pull Request #1985 · USACE/cwms-data-api

MikeNeilson · 2026-09-30T17:47:35Z

Summary

Move to latest javalin (7.2.3)

Remove Use of Tomcat, no Tomcat 11+ in our local environment so will just use Javalin's default jetty and package it up.

NOTE: Due to Javalin updating the annotation plugin (this happened in javalin 5) this is going to be a large change as it touches every file and will need to alter how the OpenAPI static test works.

TODO:

- Make a decision on examples (thinking a follow up given the current size)
- Security operations behaving correctly. (I may give up for now and just put a note on the Swagger UI about the lock icons.)
- Get the version behaving correctly.

Related Issue

Validation

Existing integration tests

Checklist

AI tools used

MikeNeilson · 2026-09-30T17:53:03Z

NOTE: at best I will have the annotations updated today. Getting the API running again in it's standalone mode will likely take until the end of the week if not longer.

MikeNeilson · 2026-09-30T19:23:43Z

For the record. I have considered keeping with tomcat and just using like embedded tomcat for the packaging, but Javalin was made for Jetty so since that would be a fair amount of work anyway, better to use a tool that was actually built for this given style of packaging an app.

MikeNeilson · 2026-10-02T21:40:35Z

FYI, I am aware this PR is rather huge.

The OpenAPI changes basically have to be all or nothing. I think we can all agree on that.

It also requires the migration to Jakarta EE (javalin update) which would may or may not (java.security.Principal so probably not) affect the CwmsAaaIdentityProvider.

So fair amount of changes to Tomcat anyway.

So is this likely to be a pain... yes, but I'm arguing it's worth it.

I could pivot back and do the move to standalone Javalin (e.g. Jetty whatever version that was) first, but then without updating javalin frankly it wouldn't be in a releasable state (security concerns, that's a really old Jetty)

So. for now I'm going to barrel through. I'm open to addressing concerns and pivoting if they're strong enough.

We do have a lot of plans. and the bulk are passing. What's failing is most likely various validation I've intentionally disabled due to the location of initialization changing.

rma-rripken · 2026-10-08T19:06:26Z

+
+
+
+        //TODO: The rest


Are these gonna get added back or dropped. Some of this commented out stuff seems important

Still figuring that out. I did get the scheme processor working,

but the returned OpenAPI object is now a builder from javalin-openapi that doesn't behave the same way... plus most of what we were doing here can no be moved to the controllers, like examples.

so I might decide to let those go for this PR, and then come back and do a follow up on them. As much as I want it all to work. at a minimum if this gets merged into main it needs to work for existing usage. And there's already new work that not going to be super trivial to rebase into this.

rma-rripken · 2026-10-08T19:14:45Z

    }
-    classpath += configurations.tomcatLibs
+    classpath += configurations.baseLibs
+    jvmArgs += "-Dlogback.configurationFile=${projectDir}/logback-tests.xml"


should this be logback-test.xml ?

.... that would explain some issues with what was visible.

rma-rripken · 2026-10-08T20:04:07Z

Not because of your change but this class looks wonky, ApplicationException already has a details map. We should be able to build a map with our extra key and pass it to super and we're done. We don't have to hold our own "details" map and modify it after the super call.

good point... which means the QueryParameters is likely just as wonky, I copy pasted it from that.

rma-rripken · 2026-10-08T20:11:47Z

+                });
+                configureRoutes(config.routes, metrics, cdaAccessManager);
+            });
+        QueueManager.ensureRssSubscribers(ds);


Its kinda gross that ensureRssSubscribers(ds); gets called from the constructor. That means you can't build the object without it touching the database. Maybe it could move to the first line of start() ?

hmm, seams reasonable.

though then there's no reference to the DataSource (easily). good idea though, will think about best place. Maybe just save a simple reference to the datasource.

Co-authored-by: Ryan Ripken <89810919+rma-rripken@users.noreply.github.com>

MikeNeilson

Thanks. Definitely more to do than what you've already pointed out. But that'll be some good cleanup before starting it.

MikeNeilson · 2026-10-08T20:09:34Z

good point... which means the QueryParameters is likely just as wonky, I copy pasted it from that.

MikeNeilson · 2026-10-08T20:10:19Z

        if (path.length() > 1 && path.endsWith("/")) {
            path = path.substring(0, path.length() - 1);
        }
        return SPA_ROUTES.contains(path) || path.matches("/user-roles/[A-Za-z0-9-]+");


Going to open an issue. You're correct, but there's already enough change in this PR, even if this is small.

MikeNeilson · 2026-10-08T20:14:50Z

                    + " implemented")},
        description = "Returns Projects Data",
-        tags = {TAG})
+        tags = {TAG}, path = "/project")


MikeNeilson · 2026-10-08T20:40:04Z

+
+
+
+        //TODO: The rest


Still figuring that out. I did get the scheme processor working,

but the returned OpenAPI object is now a builder from javalin-openapi that doesn't behave the same way... plus most of what we were doing here can no be moved to the controllers, like examples.

so I might decide to let those go for this PR, and then come back and do a follow up on them. As much as I want it all to work. at a minimum if this gets merged into main it needs to work for existing usage. And there's already new work that not going to be super trivial to rebase into this.

MikeNeilson · 2026-10-08T20:43:04Z

+        dsConfig.setJdbcUrl(ConfigVariables.getConfigString("CDA_JDBC_URL"));
+        dsConfig.setUsername(ConfigVariables.getConfigString("CDA_JDBC_USERNAME"));
+        dsConfig.setPassword(ConfigVariables.getConfigString("CDA_JDBC_PASSWORD"));
+        dsConfig.setMaximumPoolSize(ConfigVariables.getConfigInt("CDA_POOL_MAX_ACTIVE", 1));


Yeah, I'm kinda assuming if those aren't set it's a test environment but that should likely be a different value with the integration tests specifying the 1.

I'll go with 10. I suspect most devs that would run CDA directly that will be testing something won't necessarily be testing pool issues but trying to do some bulk work.

MikeNeilson · 2026-10-08T20:43:28Z

+        public CwmsDataApi build()
+        {
+            if (sessionManager == null) {
+                this.sessionManager = new SessionHandler();


Yeah, that's a good idea.

Co-authored-by: Mike Neilson <gamingmike@gmail.com>

MikeNeilson · 2026-10-08T21:02:32Z

Will attempt rebase from main tomorrow. The conflicts shouldn't be difficult to fix though.

rma-rripken · 2026-10-08T21:19:37Z

-        tags = UserListController.TAG
+        methods = HttpMethod.DELETE,
+        tags = UserListController.TAG,
+        path = "/users/list/{" + USER_LIST_ID + "}/members/{" + USER_ID + "}"


Not saying this is wrong its just curious that we have 3 controllers, one at /users/list/ and /user/list/{blah} and /user/list

rma-rripken · 2026-10-08T21:25:13Z

-            tags = TAG
+            methods = HttpMethod.POST,
+            tags = TAG,
+            path = "/forecast-spec"


This class has a variety of paths /forecast-spec /forecasts-spec/{} /forecasts-spec and /forecasts/spec we have a number of other controllers with inconsistencies.

rma-rripken · 2026-10-08T21:36:02Z

            ContentType contentType = Formats.parseHeader(formatHeader, WaterUser.class);
            ctx.contentType(contentType.toString());
            WaterUser user = Formats.parseContent(contentType, ctx.body(), WaterUser.class);
+            if (!projectId.equals(user.getProjectId().getName())) {


I wonder if case-sensitive matters?

rma-rripken · 2026-10-08T22:03:44Z

-        method = HttpMethod.GET,
-        path = "/locations/with-kind",
+        methods = HttpMethod.GET,
+        path = "/locations/with-kinds",


not sure if this was supposed to change from with-kind to with-kinds . If so, fine.

rma-rripken · 2026-10-08T22:18:00Z

        int lastSlash = path.lastIndexOf('/');
        String pathWithOffice = path.substring(0, lastSlash) + format("/{%s}", OFFICE) + path.substring(lastSlash);
-        return format("/v2/" + pathWithOffice, args);
+        return format("/v2" + pathWithOffice, args);


Is this change intentional? /v2 vs /v2/ ?

rma-rripken · 2026-10-08T22:48:24Z

I made it one time thru all the changed files and made comments on all the obvious things that I saw. There are a number of inconsistencies with how controllers are registered. I didn't check whether the controllers were already like that or if they are new changes but if they are new than I expect the integration tests won't be passing. Once the integration tests pass I think this will be ready

MikeNeilson force-pushed the feature/1004-update-javalin-drop-tomcat branch from 15a7212 to 6829e73 Compare October 7, 2026 17:56

MikeNeilson and others added 26 commits October 7, 2026 21:17

Compile worked targetting Java 17.

e4faecf

Start shifting annotations.

0ba23dc

Batch of controllers.

2c20fd9

Batch of controllers.

7da3cf1

Disable ci build while changes are worked through.

24e4331

Another batch.

bde2b87

Batch of controllers.

44367b1

Last controllers.

d576e6d

Start on baseline javalin changes.

fa421c5

Start on tests

bfbfc76

'./gradlew run' 'works'

5fe0f7c

UI renders. (at root context.)

45cd53f

Tests compile.

18ef5e3

Fix compile issue.

c5ee509

At least AccessManagerTestIt runs and passes.

188e46a

Update behavior.

424e6e1

Centralized validation setup.

94fb4ce

Sort out behavioral different with content type charasets.

c93e307

Correct value retrieval for updated Validation class.

323dbc6

restore rate limiting.

26d875a

Rate limiting actually working.

c8f28c3

Primary intergration tests pass.

3d16bd6

Enable integration tests for CI build.

f26d82b

Restore base feature properties file.

13f4c26

Start on unit/functional tests.

93bf820

ControllerTest passing with new Context test double.

2bfbf12

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/main/webapp/WEB-INF/web.xml Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/build.gradle Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread gradle/libs.versions.toml Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/main/java/cwms/cda/api/CatalogController.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/main/java/cwms/cda/validation/ValidationSetup.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread buildSrc/src/main/groovy/cda.java-conventions.gradle

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/test/java/fixtures/TestHttpServletResponse.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/test/java/fixtures/TestHttpServletResponse.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/test/java/fixtures/TestHttpServletResponse.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Comment thread cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java Outdated

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Apply batched suggestions from code review

6372b33

Co-authored-by: Ryan Ripken <89810919+rma-rripken@users.noreply.github.com>

MikeNeilson commented Oct 8, 2026

View reviewed changes

MikeNeilson and others added 5 commits October 8, 2026 13:48

Apply batched suggestions from code review

c377868

Co-authored-by: Mike Neilson <gamingmike@gmail.com>

MOve test code to test directory.

7c4113d

Correct comment.

26c2508

Cleanup.

2853df8

PR review cleanup.

d87aed0

rma-rripken reviewed Oct 8, 2026

View reviewed changes

Conversation

MikeNeilson commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Validation

Checklist

Uh oh!

MikeNeilson commented Sep 30, 2026

Uh oh!

MikeNeilson commented Sep 30, 2026

Uh oh!

MikeNeilson commented Oct 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeNeilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MikeNeilson commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rma-rripken commented Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

MikeNeilson commented Sep 30, 2026 •

edited

Loading