Skip to content

SONARJAVA-7095 Save and restore the Spring context gathering model between analyses - #6289

Open
aurelien-coet-sonarsource wants to merge 1 commit into
ac/SONARJAVA-7095-2from
ac/SONARJAVA-7095-3
Open

aurelien-coet-sonarsource wants to merge 1 commit into
ac/SONARJAVA-7095-2from
ac/SONARJAVA-7095-3

Conversation

@aurelien-coet-sonarsource

@aurelien-coet-sonarsource aurelien-coet-sonarsource commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary by Gitar

  • Spring context persistence:
    • Added SpringContextModelPersistence to save and load SpringContextGatheringModel across analyses
    • Added sonar.java.springContext.model.path property to configure spring context model persistence file path
  • Spring context gathering model:
    • Added restoreFrom, removeUnvisitedFiles, and lifecycle support in SpringContextGatheringModel to preserve unchanged files between analyses

This will update automatically on new commits.

@aurelien-coet-sonarsource
aurelien-coet-sonarsource added this pull request to stack #6284 October 2, 2026 07:22
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

SONARJAVA-7095

@aurelien-coet-sonarsource
aurelien-coet-sonarsource force-pushed the ac/SONARJAVA-7095-3 branch 2 times, most recently from 15913cd to 3a5cee8 Compare October 2, 2026 12:10
@datadog-sonarsource

This comment has been minimized.

@gitar-bot

gitar-bot Bot commented Oct 2, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 4 closed / 4 findings

🟡 Medium risk · Adds configurable Spring-model persistence and changes incremental analysis reconstruction.

Saves and restores the Spring context gathering model between analyses, addressing stale beans from untracked file deletions, incomplete test coverage for root-project resolution, premature file renaming before flush completion, and error handling during model serialization.

✅ 4 closed
✅ Bug: Restored model never drops deleted/renamed files, keeping stale beans

📄 java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextGatheringModel.java:80-89 📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaSensor.java:115-118 📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java:78-84
restoreFrom copies every module/file entry from the persisted JSON into the gathering model (putIfAbsent). Nothing ever removes an entry. collectBeans/collectPackages only overwrite files that are parsed or restored from cache in the current analysis, and SpringContextModelSensor then writes the whole merged model back to disk. So when a file is deleted, renamed, moved to another module, excluded, or no longer visited by the gatherers (for example, Spring was removed from that module's classpath), its old beans and component-scan packages stay in SpringContextModel.of(...) forever.

Concrete trigger: analysis 1 has A.java and B.java, each declaring a bean of type T, and a consumer that injects T, so S9352 is raised. The user deletes A.java to fix it. In analysis 2, A's entry is restored from the JSON with inputFile == null, and the ambiguity is still reported on the consumer. Every later save keeps the entry again, so the false positive never goes away until someone deletes the JSON file by hand.

Fix: prune restored entries that don't match a file in the current analysis before building the model. For example, keep only file keys present in context.fileSystem().inputFiles(...) for the analyzed modules, or drop restored entries that still have inputFile == null once a full (non-incremental) analysis finishes.

✅ Quality: Root-project path test never exercises root-project resolution

📄 sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextGatheringModelTest.java:103-109 📄 sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextGatheringModelTest.java:133-135 📄 java-frontend/src/main/java/org/sonar/java/SonarComponents.java:525-528
resolves_the_default_path_from_the_root_project builds SonarComponents with the 6-argument constructor. That constructor sets projectDefinition to null, so projectLevelBaseDir() takes the fs.baseDir() fallback, and the file system here was created on root directly. The SensorContextTester built on module only supplies configuration, so its base dir is ignored. As a result, the test only checks that root.resolve(DEFAULT) equals itself. It would still pass if ModuleMetadataUtils.getRootProject resolution were broken. Fix: pass a ProjectDefinition for the module whose parent is a root ProjectDefinition at root, and use a file system based on the module directory.

✅ Bug: Temp file is renamed over the model before the writer is flushed

📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java:120-126 📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaSensor.java:168-176
In saveSpringContextGatheringModel, Files.move(temporaryFile, path, REPLACE_EXISTING) runs inside the try-with-resources, before the BufferedWriter is closed. Gson.toJson(Object, Appendable) doesn't flush, so the buffered tail of the JSON (up to 8 KB, or the whole document for small models) is written only when close() runs, after the rename. The temp-file-then-rename pattern is meant to keep the old file intact until the new one is complete, but here the old model is already replaced by a partial file. If the process dies or close() fails (for example, disk full), path is left with truncated JSON. On the next run loadSpringContextGatheringModel treats that as fatal and throws IllegalStateException, so every later analysis fails until someone deletes the file by hand. Fix: close (or flush) the writer before the move, e.g. move the Files.move call out of the try-with-resources block, and consider ATOMIC_MOVE.

✅ Bug: Saving the model always runs, and any write error fails the analysis

📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java:89-90 📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java:127-129 📄 sonar-java-plugin/src/main/java/org/sonar/plugins/java/JavaPlugin.java:89-97
SpringContextModelSensor.execute always writes the model to disk. When sonar.java.springContext.model.path is unset (the default), it writes to <baseDir>/.sonar/spring-context-model.json, even though that file is never read back without the property. Any IOException is turned into an IllegalStateException, which aborts the post-phase sensor and the whole analysis. The sensor is registered for every product, including SonarLint. So a read-only checkout or base directory now makes every scan fail with the default config, and SonarLint also writes this file into the user's project on every analysis. Before this PR, Spring model handling did no file I/O. Fix: only save when the property is set, and/or log a warning on write failure instead of throwing (and consider skipping the save in SonarLint).

Review coverage

🧪 Functional validation 2 of 2 objectives covered

📋 Rules No rules evaluated

Cross-repo coverage 5 repositories selected

🤖 Auto-approval Not enabled · Set up

Implementation Status ✅ 2 of 2 objectives covered
✅ SONARJAVA-7095 - 2 of 2 objectives covered

This PR implements on-disk serialization and de-serialization for the Spring context model at the start and end of analyses.

✅ 2 covered here
  • ✅ Implement on-disk serialization for the Spring context model at the end of analyses
  • ✅ Implement on-disk deserialization for the Spring context model at the start of analyses
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

sonarqube-next Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

This branch has not been deployed

No deployments
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.

1 participant