SLCORE-2238 Add on-demand plugin storage infrastructure#1920
SLCORE-2238 Add on-demand plugin storage infrastructure#1920Krosovok wants to merge 1 commit intofeature/on-demand-analyzersfrom
Conversation
SummaryAdds foundational infrastructure for on-demand plugin storage. Introduces What reviewers should knowStart with
|
| CSHARP_OSS("cs", "cs.version", "/Distribution/sonar-csharp-plugin/sonar-csharp-plugin-%s.jar"), | ||
| OMNISHARP_MONO("omnisharpMonoLocation", "omnisharp.mono.version", "TBA"), // todo urlPattern | ||
| OMNISHARP_NET6("omnisharpNet6Location", "omnisharp.net6.version", "TBA"), // todo urlPattern | ||
| OMNISHARP_WIN("omnisharpWinLocation", "omnisharp.win.version", "TBA"); // todo urlPattern |
There was a problem hiding this comment.
The three OmniSharp entries reference version keys (omnisharp.mono.version, omnisharp.net6.version, omnisharp.win.version) that are absent from ondemand-plugins.properties. Calling version() on any of them throws IllegalStateException at runtime — the guard on line 53 makes it fail loudly, not silently. Since byArtifactKey is public and these artifact keys are real lookup strings, any caller that resolves an OmniSharp artifact and then calls version() will blow up in production.
Either add placeholder entries in the properties file (even empty/commented-out ones with a clear TODO) so the failure surface is obvious, or don't add the OmniSharp entries to the enum until their version and URL are defined.
- Mark as noise
...src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/OnDemandPluginCacheManager.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Adds initial infrastructure for on-demand plugin storage by introducing a cache cleanup manager, a registry of downloadable artifacts, and a resource file intended to provide artifact versions.
Changes:
- Introduce
OnDemandPluginCacheManagerand unit tests to clean up old cached plugin versions based on retention. - Add
DownloadableArtifactenum to describe downloadable artifacts and load their versions from a classpath properties file. - Add
ondemand-plugins.propertiesas the source of artifact version values.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
backend/core/src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/OnDemandPluginCacheManager.java |
New cache cleanup logic for removing old plugin-version directories. |
backend/core/src/test/java/org/sonarsource/sonarlint/core/plugin/ondemand/OnDemandPluginCacheManagerTest.java |
New unit tests covering cleanup behavior for missing/current/old/recent version dirs. |
backend/core/src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/DownloadableArtifact.java |
New artifact registry that loads version strings from a properties resource and builds URLs. |
backend/core/src/main/resources/ondemand-plugins.properties |
New properties file intended to provide versions for on-demand artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
...src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/OnDemandPluginCacheManager.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/OnDemandPluginCacheManager.java
Show resolved
Hide resolved
.../core/src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/DownloadableArtifact.java
Show resolved
Hide resolved
.../core/src/main/java/org/sonarsource/sonarlint/core/plugin/ondemand/DownloadableArtifact.java
Show resolved
Hide resolved
8831045 to
9ace31a
Compare
|
| } | ||
|
|
||
| public String urlPattern() { | ||
| var base = System.getProperty(PROPERTY_URL_PATTERN, "https://binaries.sonarsource.com"); |
There was a problem hiding this comment.
I'd put the binaries URL as a static var
| cfamily.version=6.50.0 | ||
| cs.version=9.32.0 No newline at end of file |
There was a problem hiding this comment.
I think we need the full version here, with the build, no?
There was a problem hiding this comment.
Edit: I noticed the other PR, I guess this is temporary



No description provided.