Avoid writing the keystore if no changes were made

[rogin]

Allow for using the keystore on a readonly filesystem. Currently, a write to the keystore will always occur. This change limits writes to only when necessary.

I welcome others to test by mounting the keystore as readonly. The keystore should not have the default storepass and keypass that that will trigger a write.

I'm considering a test in DefaultConfigurationControllerTest where the modification time of a keystore is checked before and after the call, but that's a lot of investigation to get it written correctly.

Minor other code cleanup was also included.

Related
Original ticket - nextgenhealthcare/connect#5467
Previous PR by @jonbartels - nextgenhealthcare/connect@development...jonbartels:connect:5467-keytore-avoid-writes
Summary table in #121

[tonygermano]

I haven't tested it yet, but I'm fine with this approach.

I would like to see the deprecation of ResourceUtil.closeResourceQuietly broken out to a different PR. It's not related to this keystore change, and the current state of the PR leaves a lot of references calling the deprecated method. I wonder if in some situations we would do better to change the calls to a try-with-resource block rather than using IOUtils.

Could you also please update the javadoc for the two private methods you changed to indicate the purpose of the return value?

[Copilot]

Pull request overview

This PR optimizes keystore management to avoid unnecessary writes to the filesystem, enabling the use of keystores on read-only filesystems. The primary change introduces a dirtiedKeystore flag that tracks whether modifications were made to the keystore during initialization, and only writes the keystore back to disk if changes occurred. Additionally, the PR replaces deprecated ResourceUtil.closeResourceQuietly() calls with IOUtils.closeQuietly() from Apache Commons IO, and includes minor code cleanup (removing unused imports and reformatting JavaDoc).

• Tracks keystore modifications with a boolean flag to prevent unnecessary writes
• Refactors configureEncryption() and generateDefaultCertificate() to return boolean values indicating if the keystore was modified
• Migrates from ResourceUtil.closeResourceQuietly() to IOUtils.closeQuietly() for resource cleanup

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
server/src/com/mirth/connect/server/util/ResourceUtil.java	Adds deprecation annotation and documentation to closeResourceQuietly() method
server/src/com/mirth/connect/server/controllers/DefaultConfigurationController.java	Implements conditional keystore writes using dirtiedKeystore flag; refactors configureEncryption() and generateDefaultCertificate() to return boolean modification status; replaces ResourceUtil.closeResourceQuietly() with IOUtils.closeQuietly(); removes unused import and reformats JavaDoc

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jbeckers]

This bites me when rolling out our mirth instances with a custom keystore. Ansible sees the written-to keystore as a change and always overwrites it with ours.

PR looks good to me, though I agree the deprecation should be a different PR

[github-actions]

Test Results

667 tests  +4   667 ✅ +4   2m 17s ⏱️ + 1m 0s
111 suites ±0     0 💤 ±0 
111 files   ±0     0 ❌ ±0 

Results for commit 33e687b. ± Comparison against base commit 762e072.

♻️ This comment has been updated with latest results.

[jonbartels]

I have added tests to address copilot remarks.

I still need to build and manually test this locally. It should still be fine since the code didn't change in the last 2 years but I'll do that work and share my results.

I want to leave in the deprecation. It was observed as an issue while working this case. Leaving the deprecation flag helps others avoid using sub-optimal methods. Making it a separate PR adds to the growing pile of PRs and begging for reviews.