Feat slim plugin dependency #1074
Conversation
WalkthroughThis change refactors the module slimming logic in the Maven plugin by renaming Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MavenPlugin (RepackageMojo)
participant ModuleSlimExecutor
participant MavenUtils
participant ProjectBuilder
participant RepositorySystem
User->>MavenPlugin: Run plugin goal
MavenPlugin->>ModuleSlimExecutor: Instantiate with project, config, ProjectBuilder, RepositorySystem, etc.
ModuleSlimExecutor->>MavenUtils: Retrieve artifact/parent identities
ModuleSlimExecutor->>ProjectBuilder: Resolve POM models for base plugins
ModuleSlimExecutor->>RepositorySystem: (If needed) Resolve artifacts
ModuleSlimExecutor->>MavenPlugin: Return slimmed artifacts
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutor.java (1)
95-105:⚠️ Potential issueConstructor allows
nullcollaborators that are later dereferenced
repositorySystemandprojectBuilderare optional in the tests, but they are required at runtime (seeresolvePomAsOriginalModel).
Passingnullwill lead to aNullPointerExceptionduring real execution or during certain test paths that do not stub the method.Add
Objects.requireNonNullchecks (or fall-back strategies) so incorrect wiring fails fast and clearly.this.repositorySystem = repositorySystem; this.projectBuilder = projectBuilder; +Objects.requireNonNull(this.repositorySystem, "repositorySystem must not be null"); +Objects.requireNonNull(this.projectBuilder, "projectBuilder must not be null");
🧹 Nitpick comments (4)
sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/MavenUtils.java (1)
170-181: Add null–safety (and reuse existing helpers) in the new GA/GAV helpers**All newly-added helpers blindly dereference
artifact/parent, which is consistent with older methods in the class, but it propagates NPEs outward and forces every caller to guard againstnull.
Because these helpers will be called from slimming code that processes partially-resolved models, an occasionalnullis realistic (e.g., optional‐scope artifacts, failed model resolution).Consider a defensive implementation that returns
nullor an empty string when the input isnull, or useObjects.requireNonNull()so the failure is explicit.
Additionally, note thatgetArtifactIdentityWithoutVersion(Artifact)already returns “GA[:classifier]:type”, so the GA helpers could delegate to it to avoid duplication.- public static String getGAIdentity(Artifact artifact) { - return artifact.getGroupId() + STRING_COLON + artifact.getArtifactId(); - } + public static String getGAIdentity(Artifact artifact) { + if (artifact == null) { + return null; + } + return artifact.getGroupId() + STRING_COLON + artifact.getArtifactId(); + }sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutor.java (2)
151-159: Minor optimisation: avoid building large interim sets twice
dependencyIdentitiesis recomputed for every call togetSameArtifactsInBasePlugin, yet it only depends on the resolved BOM models.
Moving this calculation togetBasePluginModel()(and caching the result) would avoid repeated stream traversals when multiple modules are processed.
188-223: Version-mismatch check may still throw NPEs
baseDependencyIdentityWithoutVersion.get(identity)can returnnull(e.g., shrink-wrapped list mismatch) causinggetDependencyIdentity(null)to NPE when logging.
Guard the call or log the raw identity to keep the build from crashing while still surfacing the error.sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutorTest.java (1)
150-159: Mocked BOM dependencies lacktype/scope– diverges from real data
mockBasePluginBomModelconstructsDependencyinstances without settingtypeorscope.
In production, these fields will be"jar"/""or inherit from the parent POM, influencing the filtering logic that checksgetType()andgetScope().
Consider populating them to avoid false positives/negatives in tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pom.xml(1 hunks)sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/MavenUtils.java(2 hunks)sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimConfig.java(0 hunks)sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutor.java(12 hunks)sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/RepackageMojo.java(4 hunks)sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/utils/ParseUtils.java(1 hunks)sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutorTest.java(17 hunks)sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/RepackageMojoTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimConfig.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/RepackageMojoTest.java (1)
sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutor.java (3)
ModuleSlimExecutor(75-906)ExcludeConfig(803-904)ExcludeConfigResponse(780-801)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: sca
🔇 Additional comments (7)
pom.xml (1)
17-17: Version updated to development snapshotThe version property
<sofa.ark.version>has been updated from2.3.1to2.3.2-SNAPSHOT, indicating the beginning of development for the next release that will include the new dependency slimming feature.sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/RepackageMojoTest.java (1)
19-20: Import paths updated to reflect class renameThe imports have been updated to reference the renamed class
ModuleSlimExecutorinstead of the previousModuleSlimStrategy. This change aligns the test with the core refactoring where the slimming logic was restructured and enhanced.sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/utils/ParseUtils.java (1)
75-85: Well-designed utility method for combined configuration sourcesThis new method enhances configuration handling by checking for boolean values in both Properties and YAML sources, prioritizing Properties if the value exists in both. This centralizes configuration logic and reduces duplicate code, supporting the broader refactoring of the plugin's slimming capabilities.
The method follows good practices:
- Checks both configuration sources
- Uses clear variable names
- Provides appropriate fallback to default values
- Properly handles type conversion
sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/RepackageMojo.java (3)
100-102: Added ProjectBuilder component for enhanced POM processingThe
ProjectBuildercomponent has been injected, providing capabilities to properly resolve and build complete Maven project models. This allows the plugin to better analyze dependencies and their relationships when implementing the slimming logic.
324-324: Expanded base dependency identity format supportThe Javadoc now indicates that
baseDependencyParentIdentitycan be specified in two formats:
${groupId}:${artifactId}:${version}(original format)${groupId}:${artifactId}(new simplified format)This change provides more flexibility when specifying the base plugin for dependency filtering.
562-566: Core slimming logic refactored and enhancedThe core dependency slimming implementation now:
- Uses the renamed
ModuleSlimExecutor(previouslyModuleSlimStrategy)- Includes the new
projectBuilderparameter for enhanced POM resolution- Maintains the same interface for retrieving slimmed artifacts
This refactoring enables the improved dependency filtering based on the base plugin's BOM, which is the main feature of this PR.
sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimExecutorTest.java (1)
83-91: Tests mask NPE risk by passingnullinfrastructure objectsThe new constructor parameters (
RepositorySystem,ProjectBuilder) are passed asnull, yetModuleSlimExecutordereferences them when resolving POMs.
The tests succeed only because the relevant code paths are stubbed withdoReturn.
Add minimal mocks (or useMockito.RETURNS_DEEP_STUBS) so the tests exercise the real code and fail if future changes access these collaborators unexpectedly.
| protected Model resolvePomAsOriginalModel(String groupId, String artifactId, String version) { | ||
| try { | ||
| Artifact artifact = repositorySystem | ||
| .createProjectArtifact(groupId, artifactId, version); | ||
| return projectBuilder.build(artifact, project.getProjectBuildingRequest()).getProject() | ||
| .getOriginalModel(); | ||
| } catch (ProjectBuildingException e) { | ||
| log.warn("resolve pom as project error: with " + groupId + ":" + artifactId + ":" | ||
| + version); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
project.getProjectBuildingRequest() does not exist – compilation will fail
MavenProject has no getProjectBuildingRequest() method in any Maven 3.x API.
The current code therefore does not compile.
You either need the ProjectBuildingRequest that is available from the MavenSession, or you must build one manually.
- Artifact artifact = repositorySystem
- .createProjectArtifact(groupId, artifactId, version);
- return projectBuilder.build(artifact, project.getProjectBuildingRequest()).getProject()
- .getOriginalModel();
+ Artifact artifact = repositorySystem.createProjectArtifact(groupId, artifactId, version);
+
+ // Acquire a ProjectBuildingRequest (e.g. injected from Mojo or built here)
+ ProjectBuildingRequest buildingRequest = new DefaultProjectBuildingRequest();
+ buildingRequest.setRepositorySession(project.getProjectBuildingRequest().getRepositorySession());
+ buildingRequest.setRemoteRepositories(project.getRemoteArtifactRepositories());
+
+ return projectBuilder.build(artifact, buildingRequest)
+ .getProject()
+ .getOriginalModel();You will also need to pass a valid ProjectBuildingRequest (or a MavenSession) into ModuleSlimExecutor, and update callers/tests accordingly.
Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1074 +/- ##
============================================
+ Coverage 77.72% 77.86% +0.13%
Complexity 910 910
============================================
Files 175 175
Lines 7376 7376
Branches 1082 1082
============================================
+ Hits 5733 5743 +10
+ Misses 1093 1082 -11
- Partials 550 551 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
根据基座插件打出的 bom 瘦身及瘦身优化：
a. 配置优化：如果模块配置的 parent 是基座 starter，且模块 sofa-ark-maven-plugin 中配置了 baseDependencyParentIdentity 标识，那么就会根据该 starter 做瘦身：将瘦身所有基座中有的依赖（GroupId 和 artifactId 相同即可）；
b. 日志优化：打印瘦身了但基座中没有的依赖；打印瘦身了但基座里不同版本的依赖。
c. 构建防御：如果配置了 buildFailWhenExcludeDiffBaseDependency = true 且瘦身了基座中没有或基座有不同版本的依赖，那么直接构建失败
当模块配置的 parent 是基座 starter、且模块 sofa-ark-maven-plugin 中配置了 baseDependencyParentIdentity 标识、且模块正在 dependencyManagement 中引入了基座插件打出的 plugin-bom （打 plugin-bom 的方式见 Feat build plugin bom koupleless/runtime#244 ），那么瘦身时，将一同瘦身该 bom 中 dependencyManagement 中声明的依赖。如：