Keep the setup wizard closed when a configured database is unreachable#6333
Open
dkayiwa wants to merge 1 commit into
Open
Keep the setup wizard closed when a configured database is unreachable#6333dkayiwa wants to merge 1 commit into
dkayiwa wants to merge 1 commit into
Conversation
DatabaseDetective.isDatabaseEmpty caught any connection failure and reported the database as empty. Both callers, Listener startup and InitializationFilter, use that result to decide whether to present the unauthenticated setup wizard, so a database-connectivity failure on an already-configured deployment reopened the wizard, exposing the stored database credentials in the rendered form and letting an unauthenticated user reconfigure and persist their own settings. Only a genuinely unconfigured instance (no connection URL) is now treated as empty on a connection failure; a configured instance that cannot be reached is reported as not empty, so it fails startup with a connection error instead of reopening setup. Also stop the finally block from returning inside a close() failure, which could otherwise flip a non-empty result to empty. Adds a regression test that a configured but unreachable database is not reported as empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6333 +/- ##
============================================
+ Coverage 59.47% 59.48% +0.01%
- Complexity 9541 9544 +3
============================================
Files 731 731
Lines 38323 38323
Branches 5587 5588 +1
============================================
+ Hits 22794 22798 +4
+ Misses 13489 13485 -4
Partials 2040 2040 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ibacher
approved these changes
Jul 17, 2026
| // already-configured deployment during a database outage, so only a genuinely | ||
| // unconfigured instance (no connection URL) counts as empty. | ||
| String url = props.getProperty(CONNECTION_URL); | ||
| return url == null || url.trim().isEmpty(); |
Member
There was a problem hiding this comment.
isBlank() exists and is better than .trim().isEmpty() for this purpose since it doesn't need to allocate a new string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DatabaseDetective.isDatabaseEmpty(Properties)treated any exception while connecting as "the database is empty" (catch (Exception e) { return true; }). Both callers,Listenerat startup andInitializationFilter, use that to decide whether to show the unauthenticated setup wizard. So on an already-configured deployment, a database-connectivity failure at startup (a DB outage or restart) made the instance report itself empty and reopen the unauthenticated setup wizard, which renders the stored DB credentials and lets an unauthenticated user reconfigure the instance and persist their own settings back toopenmrs-runtime.properties.Change
On a connection failure, only treat the instance as empty when it is genuinely unconfigured (no
connection.url). A configured instance that cannot be reached is reported as not empty, so startup fails with a connection error instead of reopening setup. Unconfigured/fresh installs (null or empty properties) still return empty, so the normal first-run wizard is unaffected. Also removed thereturn trueinside thefinallyblock'sclose()handler, which could otherwise override a non-empty result.Tests
DatabaseDetectiveTest.shouldNotReportConfiguredButUnreachableDatabaseAsEmpty: a configured instance whose database is unreachable is not reported as empty. The existing null-properties and empty-properties cases (still empty) continue to pass.Addresses GHSA-jqhq-p753-2hwj. This is the reopen root cause; it is distinct from #6152, which fixes credential wiping mid-flow (TRUNK-6645).