gnodet-bot commented on code in PR #13603:
URL: https://github.com/apache/maven/pull/13603#discussion_r4308742421
##########
impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -258,6 +258,44 @@ public interface MavenExecutionRequest {
boolean isNoSnapshotUpdates();
+ // Resolver 2.x artifact and metadata update policies
+
+ /**
+ * Sets the update policy to apply to artifact resolution.
+ * Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
+ *
+ * @param policy the update policy string, or {@code null} to use the
legacy snapshot flags
+ * @return this request
+ * @since 4.0.0
+ */
+ MavenExecutionRequest setArtifactsUpdatePolicy(String policy);
Review Comment:
💡 **Javadoc misleads about null semantics**
The `@param` says *"or {@code null} to use the legacy snapshot flags"*, but
the actual dispatch in `DefaultRepositorySystemSessionFactory` only falls back
to legacy when **both** dimensions are null. If only this dimension is null
while the other is non-null, null here means "skip" (the resolver default
applies), not "use legacy".
```suggestion
* Sets the update policy to apply to artifact resolution.
* Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
*
* @param policy the update policy string, or {@code null} to defer to
the legacy snapshot flags
* (only when the metadata update policy is also {@code
null})
* @return this request
* @since 4.0.0
*/
MavenExecutionRequest setArtifactsUpdatePolicy(String policy);
```
##########
impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -258,6 +258,44 @@ public interface MavenExecutionRequest {
boolean isNoSnapshotUpdates();
+ // Resolver 2.x artifact and metadata update policies
+
+ /**
+ * Sets the update policy to apply to artifact resolution.
+ * Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
+ *
+ * @param policy the update policy string, or {@code null} to use the
legacy snapshot flags
+ * @return this request
+ * @since 4.0.0
+ */
+ MavenExecutionRequest setArtifactsUpdatePolicy(String policy);
+
+ /**
+ * Returns the update policy to apply to artifact resolution.
+ *
+ * @return the policy string, or {@code null} if the legacy snapshot flags
are used
+ * @since 4.0.0
+ */
+ String getArtifactsUpdatePolicy();
+
+ /**
+ * Sets the update policy to apply to metadata resolution.
+ * Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
+ *
+ * @param policy the update policy string, or {@code null} to use the
legacy snapshot flags
+ * @return this request
+ * @since 4.0.0
+ */
+ MavenExecutionRequest setMetadataUpdatePolicy(String policy);
Review Comment:
💡 **Same Javadoc issue on `setMetadataUpdatePolicy`**
Same as above — the *"or {@code null} to use the legacy snapshot flags"*
claim only holds when both dimensions are null.
```suggestion
* Sets the update policy to apply to metadata resolution.
* Valid values: {@code always}, {@code daily}, {@code interval:N}
(minutes), {@code never}.
*
* @param policy the update policy string, or {@code null} to defer to
the legacy snapshot flags
* (only when the artifacts update policy is also {@code
null})
* @return this request
* @since 4.0.0
*/
MavenExecutionRequest setMetadataUpdatePolicy(String policy);
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -195,10 +195,22 @@ public SessionBuilder
newRepositorySessionBuilder(MavenExecutionRequest request)
sessionBuilder.setOffline(request.isOffline());
sessionBuilder.setChecksumPolicy(request.getGlobalChecksumPolicy());
- sessionBuilder.setUpdatePolicy(
- request.isNoSnapshotUpdates()
- ? RepositoryPolicy.UPDATE_POLICY_NEVER
- : request.isUpdateSnapshots() ?
RepositoryPolicy.UPDATE_POLICY_ALWAYS : null);
+
+ if (request.getArtifactsUpdatePolicy() == null &&
request.getMetadataUpdatePolicy() == null) {
+ // we go "old" way
+ sessionBuilder.setUpdatePolicy(
+ request.isNoSnapshotUpdates()
+ ? RepositoryPolicy.UPDATE_POLICY_NEVER
+ : request.isUpdateSnapshots() ?
RepositoryPolicy.UPDATE_POLICY_ALWAYS : null);
+ } else {
+ // we go "new" way: only set explicitly-specified dimensions
+ if (request.getArtifactsUpdatePolicy() != null) {
+
sessionBuilder.setArtifactUpdatePolicy(request.getArtifactsUpdatePolicy());
+ }
+ if (request.getMetadataUpdatePolicy() != null) {
+
sessionBuilder.setMetadataUpdatePolicy(request.getMetadataUpdatePolicy());
Review Comment:
⚠️ **Behavioral change: `-U`/`-nsu` silently discarded in mixed mode**
When any new-style policy is set, this block fires and only applies the
**explicitly provided** dimension — the other dimension is left at the resolver
default. This means `mvn -U --metadata-update-policy daily` will apply `daily`
for metadata but will NOT apply `ALWAYS` for artifacts (because
`getArtifactsUpdatePolicy()` is null and the legacy `-U` flag is never
consulted in this branch).
Consider bridging: when entering the new-way branch, initialise the unset
dimension from the legacy flags before entering the if-checks:
```suggestion
} else {
// "new" way: explicit per-dimension policies take precedence;
legacy flags fill unset dimensions
String legacyPolicy = request.isNoSnapshotUpdates()
? RepositoryPolicy.UPDATE_POLICY_NEVER
: request.isUpdateSnapshots() ?
RepositoryPolicy.UPDATE_POLICY_ALWAYS : null;
String artifactPolicy = request.getArtifactsUpdatePolicy() !=
null
? request.getArtifactsUpdatePolicy()
: legacyPolicy;
String metadataPolicy = request.getMetadataUpdatePolicy() != null
? request.getMetadataUpdatePolicy()
: legacyPolicy;
if (artifactPolicy != null) {
sessionBuilder.setArtifactUpdatePolicy(artifactPolicy);
}
if (metadataPolicy != null) {
sessionBuilder.setMetadataUpdatePolicy(metadataPolicy);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
