Skip to content

Fix SonarCloud reliability bugs on new code#6138

Open
dkayiwa wants to merge 1 commit into
masterfrom
fix/sonar-reliability-bugs
Open

Fix SonarCloud reliability bugs on new code#6138
dkayiwa wants to merge 1 commit into
masterfrom
fix/sonar-reliability-bugs

Conversation

@dkayiwa

@dkayiwa dkayiwa commented May 27, 2026

Copy link
Copy Markdown
Member

Description

The SonarCloud quality gate on master is failing with a Reliability Rating of D on new code (required ≥ A). This PR fixes the legitimate Java reliability issues driving that rating.

Fixed

Rule Count Fix
S2142 16 Re-interrupt the thread (Thread.currentThread().interrupt()) when catching InterruptedException
S899 12 Act on (log) the boolean returned by File.delete() / createNewFile()
S2119 5 Reuse a single Random/SecureRandom; use SecureRandom for password generation
S2201 6 Replace the unused Collection.size() lazy-init trick with Hibernate.initialize(...)
S1143 2 Remove throw/return from finally blocks (ORUR01Handler, DatabaseDetective)
S2390 1 Break the ResultEmptyResult static-init cycle with a lazy holder
S2153 S2184 S4973 S2674 S2886 S2259 9 Assorted boxing / int-overflow / String comparison / unchecked-read / synchronization / possible-NPE fixes

The DatabaseDetective change is a genuine behavioural fix: a return inside a finally could report a populated database as empty when closing the connection threw.

Intentionally not changed (need SonarCloud "won't fix")

  • S1872 (7): all compare Class objects or test-class names that are not on the production classpath (e.g. "org.openmrs.api.db.UserDAOTest"), so instanceof cannot be used without weakening security caller-checks.
  • S2276 (UpdateFilter): the Thread.sleep in a synchronized method is a deliberate anti-brute-force delay.
  • plsql BLOCKER: lives in the historical update-to-1.4.2.01-db.mysqldiff.sql migration, which must not be edited.

Note: many flagged files are old (docs package.html, legacy SQL/CSS/JS), which suggests the SonarCloud new-code-period baseline was reset and is scoring pre-existing code as "new". That baseline (and the won't-fix items above) likely needs an admin adjustment in SonarCloud for the gate to fully go green.

Testing

mvn -pl liquibase,api,web -am compile → BUILD SUCCESS.
Targeted tests pass: ResultTest, OpenmrsUtilTest, ORUR01HandlerTest, UserContextTest, ContextTest, ContextDAOTest (177 tests, 0 failures).

🤖 Generated with Claude Code

Resolves the legitimate Java reliability issues flagged by the SonarCloud
quality gate (Reliability Rating D on new code):

- S2142 (16): re-interrupt the current thread when catching
  InterruptedException across Daemon, ServiceContext, HibernateContextDAO,
  S3StorageService, Hl7InArchivesMigrateThread, WebDaemon, TestInstallUtil,
  UpdateFilter and InitializationFilter.
- S899 (12): act on the boolean returned by File.delete()/createNewFile()
  (log a warning on failure) in ModuleClassLoader, ModuleFactory, Listener,
  AbstractSnapshotTuner, TestInstallUtil and InitializationFilter.
- S2119 (5): reuse a single Random/SecureRandom instance instead of creating
  one per call; use SecureRandom where the value is a password
  (MigrationHelper, InitializationFilter) and a shared Random elsewhere
  (OpenmrsUtil).
- S2201 (6): replace the unused Collection.size() lazy-init trick with the
  self-documenting Hibernate.initialize(...) in UserContext and
  HibernateContextDAO.
- S1143 (2): remove the throw from the finally block in ORUR01Handler and the
  return from the finally block in DatabaseDetective (the latter could wrongly
  report a populated database as empty when closing the connection failed).
- S2390: break the Result -> EmptyResult static-init cycle with a lazy holder.
- S2153 (2), S2184 (2), S4973, S2674, S2886 (2), S2259 (2): assorted boxing,
  integer-overflow, String comparison, unchecked-read, missing-synchronization
  and possible-NPE fixes.

S1872 hits are left as false positives (they compare Class objects / test
class names that are not on the production classpath, so instanceof cannot be
used). The S2276 sleep-in-synchronized in UpdateFilter is a deliberate
anti-brute-force delay and the plsql BLOCKER lives in a historical migration
file; these need a SonarCloud "won't fix" rather than a code change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

@codecov-commenter

codecov-commenter commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 24.70588% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.99%. Comparing base (9282315) to head (8215c63).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
...eb/filter/initialization/InitializationFilter.java 5.55% 15 Missing and 2 partials ⚠️
.../src/main/java/org/openmrs/api/context/Daemon.java 8.33% 11 Missing ⚠️
...src/main/java/org/openmrs/logic/result/Result.java 42.85% 4 Missing ⚠️
...ain/java/org/openmrs/module/ModuleClassLoader.java 33.33% 1 Missing and 3 partials ⚠️
...pi/src/main/java/org/openmrs/util/OpenmrsUtil.java 20.00% 4 Missing ⚠️
...in/java/org/openmrs/migration/MigrationHelper.java 0.00% 3 Missing ⚠️
...mrs/web/filter/initialization/TestInstallUtil.java 0.00% 3 Missing ⚠️
...n/java/org/openmrs/api/context/ServiceContext.java 0.00% 2 Missing ⚠️
...java/org/openmrs/api/storage/S3StorageService.java 0.00% 2 Missing ⚠️
...rc/main/java/org/openmrs/module/ModuleFactory.java 0.00% 2 Missing ⚠️
... and 10 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6138      +/-   ##
============================================
- Coverage     59.04%   58.99%   -0.05%     
+ Complexity     9239     9236       -3     
============================================
  Files           693      693              
  Lines         37257    37280      +23     
  Branches       5485     5490       +5     
============================================
- Hits          21998    21993       -5     
- Misses        13287    13312      +25     
- Partials       1972     1975       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants