diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java index 28004b35e9..90913bf46e 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java @@ -32,8 +32,11 @@ import cwms.cda.api.errors.FieldLengthExceededException; import cwms.cda.api.errors.InvalidItemException; import cwms.cda.api.errors.NotFoundException; +import cwms.cda.api.Controllers; import cwms.cda.datasource.ConnectionPreparingDataSource; import cwms.cda.datasource.DelegatingConnectionPreparer; +import cwms.cda.datasource.LrtsSessionPreparer; +import cwms.cda.datasource.SessionOfficePreparer; import cwms.cda.helpers.DatabaseHelpers.SCHEMA_VERSION; import cwms.cda.security.CwmsAuthException; import io.javalin.http.Context; @@ -139,9 +142,19 @@ public static DSLContext getDslContext(Context ctx) { final boolean isNewLRTS = ctx.header(ApiServlet.IS_NEW_LRTS) != null && Boolean.parseBoolean(ctx.header(ApiServlet.IS_NEW_LRTS)); + // Snapshot client-info and the requested office up front so the per-checkout + // preparer lambdas don't capture the Javalin Context — async work (e.g. the + // total-count future in TimeSeriesDaoImpl) can outlive the request facade. + final String module = (ctx.handlerType() == HandlerType.BEFORE) + ? "BEFORE-HANDLER" : ctx.endpointHandlerPath(); + final String action = ctx.method(); + final String clientId = ctx.url().replace(ctx.path(), "") + ctx.contextPath(); + final String office = resolveRequestOffice(ctx); + DelegatingConnectionPreparer preparer = new DelegatingConnectionPreparer( - connection -> setClientInfo(ctx, connection), - new cwms.cda.datasource.LrtsSessionPreparer(isNewLRTS)); + connection -> setClientInfo(connection, module, action, clientId), + new LrtsSessionPreparer(isNewLRTS), + new SessionOfficePreparer(office)); DataSource wrappedDataSource = new ConnectionPreparingDataSource(preparer, dataSource); retVal = DSL.using(wrappedDataSource, SQLDialect.ORACLE18C); @@ -150,6 +163,14 @@ public static DSLContext getDslContext(Context ctx) { return retVal; } + private static String resolveRequestOffice(Context ctx) { + String office = ctx.queryParam(Controllers.OFFICE); + if (office == null) { + office = ctx.attribute(ApiServlet.OFFICE_ID); + } + return office; + } + protected static Timestamp buildTimestamp(Instant date) { return date != null ? Timestamp.from(date) : null; @@ -170,19 +191,15 @@ public static DSLContext getDslContext(Connection connection, String officeId) { return dsl; } - private static Connection setClientInfo(Context ctx, Connection connection) { + private static Connection setClientInfo(Connection connection, String module, String action, String clientId) { try { final String apiVersion = ApiServlet.getApiVersion(); connection.setClientInfo("OCSID.ECID", ApiServlet.APPLICATION_TITLE + " " + apiVersion.substring(0, Math.min(ORACLE_ECID_MAX_LENGTH, apiVersion.length()))); - if (ctx.handlerType() == HandlerType.BEFORE) { - connection.setClientInfo("OCSID.MODULE", "BEFORE-HANDLER"); - } else { - connection.setClientInfo("OCSID.MODULE", ctx.endpointHandlerPath()); - } - connection.setClientInfo("OCSID.ACTION", ctx.method()); - connection.setClientInfo("OCSID.CLIENTID", ctx.url().replace(ctx.path(), "") + ctx.contextPath()); + connection.setClientInfo("OCSID.MODULE", module); + connection.setClientInfo("OCSID.ACTION", action); + connection.setClientInfo("OCSID.CLIENTID", clientId); } catch (SQLClientInfoException ex) { logger.atFinest() // this is usually useless information .withCause(ex) diff --git a/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTest.java b/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTest.java index 13b8f05c83..74c03816bc 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTest.java @@ -16,6 +16,7 @@ import fixtures.TestServletInputStream; import io.javalin.core.util.Header; import io.javalin.http.Context; +import io.javalin.http.ContextResolver; import io.javalin.http.HandlerType; import io.javalin.http.util.ContextUtil; import io.javalin.plugin.json.JavalinJackson; @@ -39,8 +40,15 @@ void bad_format_returns_501() throws Exception { HashMap attributes = new HashMap<>(); attributes.put(ContextUtil.maxRequestSizeKey, Integer.MAX_VALUE); attributes.put(JsonMapperKt.JSON_MAPPER_KEY, new JavalinJackson()); + // JooqDao.getDslContext uses ctx.url() which delegates through ContextResolver + attributes.put("contextResolver", new ContextResolver()); when(request.getInputStream()).thenReturn(new TestServletInputStream(testBody)); + // JooqDao.getDslContext snapshots client-info from the request for connection preparers + when(request.getMethod()).thenReturn("GET"); + when(request.getRequestURI()).thenReturn("/cwms-data/clobs"); + when(request.getRequestURL()).thenReturn(new StringBuffer("http://localhost:7000/cwms-data/clobs")); + when(request.getContextPath()).thenReturn("/cwms-data"); Context context = ContextUtil.init(request, response, "*", new HashMap<>(), HandlerType.GET, attributes); diff --git a/cwms-data-api/src/test/java/cwms/cda/data/dao/JooqDaoTestIT.java b/cwms-data-api/src/test/java/cwms/cda/data/dao/JooqDaoTestIT.java new file mode 100644 index 0000000000..1548d3cc32 --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/data/dao/JooqDaoTestIT.java @@ -0,0 +1,76 @@ +package cwms.cda.data.dao; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import cwms.cda.api.DataApiTestIT; +import cwms.cda.datasource.SessionOfficePreparer; +import fixtures.CwmsDataApiSetupCallback; +import java.sql.Connection; +import java.sql.SQLException; +import mil.army.usace.hec.test.database.CwmsDatabaseContainer; +import org.jooq.SQLDialect; +import org.jooq.impl.DSL; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag("integration") +public final class JooqDaoTestIT extends DataApiTestIT { + + private static final String SESSION_OFFICE_QUERY = + "SELECT SYS_CONTEXT('CWMS_ENV','SESSION_OFFICE_ID') FROM dual"; + + /** + * Verifies that SessionOfficePreparer sets the Oracle session variable + * on a cold connection. This is the preparer added to the chain in + * JooqDao.getDslContext — without it, cold pooled connections fail + * intermittently with ORA-20047 when CWMS PL/SQL checks session office. + * + * Uses a raw JDBC connection (not the API pool) to guarantee cold state. + */ + @Test + void test_session_office_preparer_sets_office_on_cold_connection_success() throws SQLException { + CwmsDatabaseContainer db = CwmsDataApiSetupCallback.getDatabaseLink(); + String officeId = db.getOfficeId(); + String webUser = CwmsDataApiSetupCallback.getWebUser(); + + db.connection(c -> { + assertNull(readSessionOffice(c), + "Precondition: fresh connection should have no session office"); + + new SessionOfficePreparer(officeId).prepare(c); + + assertEquals(officeId.toUpperCase(), readSessionOffice(c).toUpperCase(), + "SessionOfficePreparer should set SYS_CONTEXT session office"); + }, webUser); + } + + /** + * Verifies that SessionOfficePreparer correctly overrides a stale session + * office from a previous request — the cross-office contamination case. + * Without the preparer in the checkout chain, a pooled connection warmed + * by one office retains that office for subsequent requests. + */ + @Test + void test_session_office_preparer_overrides_stale_office_success() throws SQLException { + CwmsDatabaseContainer db = CwmsDataApiSetupCallback.getDatabaseLink(); + String officeId = db.getOfficeId(); + String webUser = CwmsDataApiSetupCallback.getWebUser(); + + db.connection(c -> { + new SessionOfficePreparer(officeId).prepare(c); + assertEquals(officeId.toUpperCase(), readSessionOffice(c).toUpperCase(), + "Session should be " + officeId + " after first prepare"); + + new SessionOfficePreparer("SPK").prepare(c); + assertEquals("SPK", readSessionOffice(c).toUpperCase(), + "Session should switch to SPK — stale " + officeId + " must be overridden"); + }, webUser); + } + + private static String readSessionOffice(Connection c) { + return DSL.using(c, SQLDialect.ORACLE18C) + .fetchOne(SESSION_OFFICE_QUERY) + .get(0, String.class); + } +}