Repository navigation
Refactor remaining Getter.java JDBC sites to try-with-resources #855
Description
Summary
Refactor the remaining JDBC resource sites in src/main/java/dbProcs/Getter.java to use try-with-resources, so every PreparedStatement / CallableStatement / ResultSet is deterministically closed.
This is the Getter.java counterpart to the work already completed for the SSO path (#840 / PR #845) and for Setter.java (#846 / PR #852 — Setter.java is now clean). It is the last large file still triggering the recurring "Potential database resource leak" code-scanning comments seen on #816.
Why this is one issue, not a trickle
The code-quality bot reviews each push's diff, so partial fixes keep surfacing "new" comments on untouched regions of Getter.java while stale comments linger on code that has since moved. The fix is a single comprehensive pass over the file so the scan goes quiet in one shot. The scope is finite and enumerated below — there is a definite end.
Scope — 44 methods with genuine leak sites
Each creates a statement/ResultSet in the method body (outside a try (...) resource header). Clean pattern to apply (already used by e.g. getClassInfo, authUser, findPlayerById):
try (Connection conn = Database.getCoreConnection(ApplicationRoot);
CallableStatement callstmt = conn.prepareCall("...")) {
try (ResultSet rs = callstmt.executeQuery()) {
...
}
}SIMPLE — single statement + ResultSet, straightforward wrap (42)
getClassCount, getClassInfo(String,String), getCsrfForumWithIframe, getCsrfForumWithImg, getFeedback, getIncrementalModules, getIncrementalModulesWithoutScript, getLessons, getModuleAddress, getModuleCategory, getModuleHash, getModuleIdFromHash, getModuleKeyType, getModuleNameLocaleKey, getModuleResult, getModuleResultFromHash, getModulesInOptionTags, getModulesInOptionTagsCTF, getModuleSolution, getModuleStatusMenu, getOpenCloseCategoryMenu, getProgress, getProgressJSON, getTournamentModules, getUserClassFromName, getUserIdFromName, getUserName, isCsrfLevelComplete, findAdminById, getPlayerCheatStatus, getModuleLayout, getFeedbackStatus, getRegistrationStatus, getScoreboardStatus, getScoreboardClass, getStartTimeStatus, getStartTime, getLockTimeStatus, getLockTime, getEndTimeStatus, getEndTime, getDefaultClass
CONDITIONAL — statement assigned in if/else branches, needs care (1)
getJsonScore—CallableStatementinitialized tonull, then assigned in one of two branches (totalScoreboard()vsclassScoreboard(?));ResultSetalso created in body. Cannot go straight into a TWR header — wrap after the branch, or hoist the SQL selection.
PARTIAL MITIGATION — ResultSet manually closed, statement still leaks (1)
isModuleOpen— has a manualrs.close()but thePreparedStatement/CallableStatementis not closed.
Acceptance criteria
- All 44 methods wrap JDBC resources in try-with-resources; SQL and branching unchanged (no behavior change).
- No remaining body-level statement/ResultSet creation in
Getter.java(CSRF/challenge behavior preserved). - "Potential database resource leak" code-scanning comments for
Getter.javacease. -
GetterITand challenge integration tests pass (run viaworkflow_dispatch"Build and Test" if on a feature→feature branch).
Notes
Setter.javais already fully converted (Refactor remaining Setter.java JDBC sites to try-with-resources (structural cases) #846 / PR fix: try-with-resources for remaining Setter.java JDBC sites (#846) #852) — this issue isGetter.javaonly.getJsonScoreandisModuleOpenwarrant targeted review for behavior preservation.
Inventory generated by static analysis of the current dev#536 tree; line numbers omitted as they drift, method names are stable.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsBacklog
Activity
refactor: try-with-resources for remaining Getter.java JDBC sites (#855…
feat: add connection pooling to fix db connection exhaustion (#536) (#…