[JENKINS-27993] Return useful plugin data from /pluginManager/api/json - #27083
[JENKINS-27993] Return useful plugin data from /pluginManager/api/json#27083umbertocicciaa wants to merge 1 commit into
Conversation
by default Increase @exported visibility to 2 on PluginWrapper's shortName, url, version, active, enabled, and hasUpdate, and export displayName (the non-deprecated replacement for getLongName()), so these fields are no longer dropped when nested under PluginManager.plugins at the API's default depth. Issue jenkinsci#21047
There was a problem hiding this comment.
Pull request overview
This PR addresses JENKINS-27993 by making /pluginManager/api/json return meaningful plugin information at the default API depth, instead of a list of mostly empty plugin objects. It does so by increasing the @Exported visibility of key PluginWrapper properties so they are retained when serialized under PluginManager.plugins.
Changes:
- Increase
@Exportedvisibility to2for keyPluginWrapperfields (shortName,url,version,active,enabled,hasUpdate). - Export
displayName(viagetDisplayName()) at visibility2to provide a non-deprecated name in nested plugin output. - Add a regression test validating that
/pluginManager/api/jsonincludes key plugin fields at default depth.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/src/main/java/hudson/PluginWrapper.java | Raises export visibility of commonly needed plugin fields so they appear in /pluginManager/api/json at default depth; also exports displayName. |
| test/src/test/java/hudson/PluginManagerTest.java | Adds a regression test asserting key plugin properties are present in the /pluginManager/api/json response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assertNotNull(htmlPublisher.optString("shortName", null), "shortName should be exported at default depth"); | ||
| assertNotNull(htmlPublisher.optString("version", null), "version should be exported at default depth"); | ||
| assertNotNull(htmlPublisher.optString("displayName", null), "displayName should be exported at default depth"); |
| */ | ||
| @Exported | ||
| @Exported(visibility = 2) | ||
| public String getUrl() { |
|
Hi @MarkEWaite : ) would you prefer that I rebase, or are there any other best practices you typically follow for this repository? |
Please don't rebase. When a pull request is rebased, it risks breaking the association between review comments and the code. We squash merge most pull requests, so there is generally very little benefit from a rebase. Wait patiently while the review process proceeds. Consider the GitHub Copilot review comments carefully. It is not a perfect code reviewer, but it is quite good. |
Thanks a lot for the advice and for sharing your experience maintaining the repository, it’s really helpful :) |
Generated with core pull request: * jenkinsci/jenkins#27206 Includes pull requests: * jenkinsci/jenkins#27205 * jenkinsci/jenkins#27201 * jenkinsci/jenkins#27193 * jenkinsci/jenkins#27163 * jenkinsci/jenkins#27088 * jenkinsci/jenkins#27083 * jenkinsci/jenkins#27067 * jenkinsci/jenkins#27065 * jenkinsci/jenkins#27063 * jenkinsci/jenkins#27032 * jenkinsci/jenkins#26923 * jenkinsci/jenkins#26922 * jenkinsci/jenkins#26913 * jenkinsci/jenkins#26880 * jenkinsci/jenkins#26690 * jenkinsci/jenkins#26668 * jenkinsci/jenkins#26600 * jenkinsci/jenkins#26587 * jenkinsci/jenkins#11216 * jenkinsci/jenkins#10432 * jenkinsci/jenkins#8559
Return useful data from /pluginManager/api/json by default
Increase @exported visibility to 2 on PluginWrapper's shortName, url, version, active, enabled, and hasUpdate, and export displayName (the non-deprecated replacement for getLongName()), so these fields are no longer dropped when nested under PluginManager.plugins at the API's default depth.
Fixes #21047
Testing done
Added
PluginManagerTest#pluginManagerApiJsonReturnsPluginDetails, which installshtmlpublisher.jpi, hitspluginManager/api/jsonwith nodepthparameter, and asserts thatshortName,version,displayName,active,enabled, andhasUpdateare all present and non-null on the returned plugin object. The test fails against the pre-patch code (all of those fields are absent/empty at default depth) and passes after the@Exported(visibility = 2)changes.Screenshots (UI changes only)
Before
After
Proposed changelog entries
shortName,url,version,displayName,active,enabled, andhasUpdatein/pluginManager/api/jsonat the default depthProposed changelog category
/label rfe
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered.