Close the database connection pool when the plugin is disabled

[dmccoystephenson]

Summary

• onDisable is now declared on AlternateAccountFinder and closes the HikariCP pool built during startup. The pool was previously never closed, so every disable short of a full server shutdown — /reload, and any plugin manager that disables and re-enables the plugin — left its connections and housekeeping threads running while the next onEnable built a second pool beside them.
• The close itself is delegated to a package-private static helper, closeDataSource(DataSource, Logger), following the PlayerJoinListener.parseValidUuids precedent for logic that would otherwise be locked behind JavaPlugin. A pool that was never built (an onEnable that threw before the assignment) and a DataSource that is not closeable are both no-ops; a close that throws is logged at WARNING rather than rethrown, since an exception out of onDisable is reported by Bukkit as a plugin fault.
• Four tests cover the helper: a real HikariDataSource over in-memory H2 is asserted closed, and the null, not-closeable and throwing paths are asserted not to throw.
• A Fixed entry was added under [Unreleased] in CHANGELOG.md.

No behaviour visible to a command, a config key or the stored data changes, and no IP address reaches any new output or log path.

Test plan

• ./gradlew clean build passes locally, including the four new tests.
• Regression evidence: with the body of closeDataSource replaced by an early return, closesAPoolThatIsCloseable and warnsRatherThanThrowsWhenTheCloseFails both fail; both pass once it is restored.
• Manual check on a live server, which no automated test here can perform: with the plugin loaded on a Spigot/Paper server, /reload confirm is run twice, and a thread dump (or /dpm-style disable and re-enable) is inspected for HikariPool-* housekeeping threads. Exactly one pool should be present afterwards rather than one per reload, and no Failed to close the database connection pool warning should appear in the log.

Coverage gap

The Build workflow exercises the JVM build against H2 only and cannot start a Bukkit server, so the onDisable wiring — as opposed to the helper it calls — is not verified by CI. The manual step above is what covers it.

Backlog deferred this cycle

The rest of the open backlog was left untouched because every item is gated on a human: #89 is down to its branch-protection item, which needs repository-settings access; #62 requires authorization to edit agent-loaded configuration (.github/copilot-instructions.md); and epic #47 is awaiting confirmation that its scope is fully covered.

Closes #97

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric, scored against the diff and command output rather than intent:

• Scope: PASS — three files, all required by The HikariCP connection pool is never closed: the plugin has no onDisable #97: the fix, its test, and the changelog entry. git diff --stat origin/main reports 161 insertions and no deletions, no reformatting and no unrelated renames.
• Tests-new: PARTIAL — closeDataSource, the new helper, has four tests. onDisable, also new and public, has none: AlternateAccountFinder extends JavaPlugin and throws on construction outside a plugin classloader, so the wiring is covered by the manual step in the PR body instead. This is the same boundary AafCommandTest documents for the server-bound branches of the command layer.
• Tests-fix: PASS — verified empirically, not reasoned. With the body of closeDataSource replaced by an early return, closesAPoolThatIsCloseable and warnsRatherThanThrowsWhenTheCloseFails both FAILED; with it restored, all four PASS.
• Sibling structure: PASS — the package-private static helper plus a plain JUnit test on it mirrors PlayerJoinListener.parseValidUuids and PlayerJoinListenerTest, the existing precedent for logic that would otherwise sit behind an unconstructable Bukkit class.
• Sibling renames: N/A — nothing renamed.
• Docs: PASS — a Fixed entry was added under [Unreleased], after Added, per the section order CONTRIBUTING.md records. Every other row of the sources-of-truth table was checked and none is affected: no command, permission, config key or data-folder file changes, and a grep of the Markdown documents for reload/disable/shutdown finds no claim about pool lifetime to correct.
• Issue resolution: PASS — The HikariCP connection pool is never closed: the plugin has no onDisable #97 asked for a close that tolerates a pool that was never built, tolerates a non-closeable implementation and logs rather than throws; all three are in the diff and each has a test.
• CI: PASS — the Build check is green on the PR head (run 31770541652).
• Privacy: PASS — the one new log line is a fixed string plus the close exception. It names no account, no record and no address, and no new code path reaches command output or tab completion.
• plugin.yml — permissions: N/A — no permission is referenced by the diff.
• plugin.yml — commands: N/A — AafCommand routing is untouched.
• @OverRide coverage: PASS — onDisable overrides JavaPlugin#onDisable and carries @Override; the helper overrides nothing.
• MariaDB compatibility: N/A — no SQL and no migration in the diff.
• Encryption determinism: N/A — IpEncryption and every ciphertext-equality lookup are untouched.
• Migration safety: N/A — no data mutation.

Findings folded in from reading the diff, none of which changes the verdict:

• src/main/java/com/dansplugins/detectionsystem/AlternateAccountFinder.java:125 — closing the pool in onDisable is a real behaviour trade-off worth recording. Bukkit cancels a plugin's scheduled tasks after onDisable returns, so an async login-recording task still in flight at that moment now meets a pool that is shutting down and will log a scheduler error where it previously completed. The exposure is bounded: HikariCP's close() evicts softly and waits for in-flight connections to be returned before tearing the pool down, so a query already running is given time to finish, and the alternative — leaking the pool on every reload — is the worse failure. Cancelling this plugin's own tasks before the close would narrow the window further but cannot close it, since the scheduler does not interrupt a task already running; that is left out deliberately rather than overlooked.
• src/main/java/com/dansplugins/detectionsystem/AlternateAccountFinder.java:143 — the test for instanceof AutoCloseable is deliberately broader than instanceof HikariDataSource. It keeps the helper honest about the only thing it needs (close()), and it is what lets the not-closeable and throwing paths be exercised by a dynamic proxy without a mock framework, which this project does not configure.
• src/main/java/com/dansplugins/detectionsystem/AlternateAccountFinder.java:126 — dataSource is nulled after the close so a second disable is a no-op rather than a second close. loginService and notificationService are deliberately left as they are: nothing reads them after a disable, and clearing them would only widen the window in which the in-flight task described above dereferences null instead of receiving a database error.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson