TRUNK-6593: try-with-resources for the other OpenmrsUtil methods

[liujs123456]

PR #5979 already refactored getFileAsBytes. There are 4 other methods in OpenmrsUtil that use the same manual try/finally/close pattern, so refactored them to use try-with-resources too:

• getFileAsString
• saveDocument
• storeProperties(Properties, File, String)
• loadProperties

getFileAsString also didn't have a finally block at all, so the reader would have been leaked if read() threw an exception.

JIRA: https://issues.openmrs.org/browse/TRUNK-6593

Summary by CodeRabbit

• Refactor
  • Improved file and properties handling for more reliable resource management and consistent UTF-8 handling.
  • Added safer stream closing and additional logging for I/O issues to reduce unexpected failures.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR replaces manual finally-block resource closes with try-with-resources in OpenmrsUtil: getFileAsString, saveDocument, storeProperties, and loadProperties; behavior and exception logging are preserved, and saveDocument adds an IOException catch for stream-close warnings.

Changes

Stream handling modernization

Layer / File(s)

Summary

Try-with-resources I/O refactoring api/src/main/java/org/openmrs/util/OpenmrsUtil.java

getFileAsString(File) now uses a try-with-resources UTF-8 BufferedReader; loadProperties(Properties, InputStream) uses a try-with-resources UTF-8 InputStreamReader; storeProperties(Properties, File, String) opens append FileOutputStream with try-with-resources; saveDocument(Document, File) wraps FileOutputStream in try-with-resources and adds an IOException catch for close-time warnings. All explicit finally closes were removed while preserving existing logging.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: applying try-with-resources to remaining OpenmrsUtil methods as referenced in JIRA ticket TRUNK-6593.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PMD (7.24.0)

api/src/main/java/org/openmrs/util/OpenmrsUtil.java

[ERROR] Cannot load ruleset rulesets/java/android.xml/DoNotHardCodeSDCard: Cannot resolve rule/ruleset reference 'rulesets/java/android.xml/DoNotHardCodeSDCard'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
[WARN] Progressbar rendering conflicts with reporting to STDOUT. No progressbar will be shown. Try running with argument -r to output the report to a file instead.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

🧹 Nitpick comments (1)

api/src/main/java/org/openmrs/util/OpenmrsUtil.java (1)

228-228: ⚡ Quick win

Remove unnecessary buffer reallocation inside the loop.

Great work adopting try-with-resources! 🎉 I noticed one small inefficiency: reallocating buf on every iteration is unnecessary since String.valueOf(buf, 0, numRead) copies the characters into the StringBuilder. The buffer can be reused safely, which will reduce allocations and GC pressure.

♻️ Suggested improvement

 		try (BufferedReader reader = new BufferedReader(
 		        new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8))) {
 			char[] buf = new char[1024];
 			int numRead;
 			while ((numRead = reader.read(buf)) != -1) {
 				String readData = String.valueOf(buf, 0, numRead);
 				fileData.append(readData);
-				buf = new char[1024];
 			}
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/src/main/java/org/openmrs/util/OpenmrsUtil.java` at line 228, The loop
currently reallocates the char[] named buf on every iteration (see usage with
String.valueOf(buf, 0, numRead) and the StringBuilder accumulator), which is
unnecessary; move the allocation of buf (char[] buf = new char[1024]) to before
the loop so the same buffer is reused across iterations in OpenmrsUtil, and
remove the in-loop reallocation to reduce allocations and GC pressure.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@api/src/main/java/org/openmrs/util/OpenmrsUtil.java`:
- Line 228: The loop currently reallocates the char[] named buf on every
iteration (see usage with String.valueOf(buf, 0, numRead) and the StringBuilder
accumulator), which is unnecessary; move the allocation of buf (char[] buf = new
char[1024]) to before the loop so the same buffer is reused across iterations in
OpenmrsUtil, and remove the in-loop reallocation to reduce allocations and GC
pressure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 11974fe4-b7d1-43f7-8d96-ed88e7887248

📥 Commits

Reviewing files that changed from the base of the PR and between d9d7617 and dfb7f67.

📒 Files selected for processing (1)

• api/src/main/java/org/openmrs/util/OpenmrsUtil.java