Skip to content

Refactor remaining Getter.java JDBC sites to try-with-resources #855

Open

Description

@ismisepaul

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 — CallableStatement initialized to null, then assigned in one of two branches (totalScoreboard() vs classScoreboard(?)); ResultSet also 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 manual rs.close() but the PreparedStatement/CallableStatement is 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.java cease.
  • GetterIT and challenge integration tests pass (run via workflow_dispatch "Build and Test" if on a feature→feature branch).

Notes

Inventory generated by static analysis of the current dev#536 tree; line numbers omitted as they drift, method names are stable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions