gnodet-bot commented on code in PR #13603:
URL: https://github.com/apache/maven/pull/13603#discussion_r4312298452
##########
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
Review Comment:
⚠️ **Misleading `@param` null semantics** — `or {@code null} to use the
legacy snapshot flags` is inaccurate. Legacy path activates only when **both**
`getArtifactsUpdatePolicy()` and `getMetadataUpdatePolicy()` are null (the `&&`
condition in `DefaultRepositorySystemSessionFactory`). If the caller sets this
policy but leaves metadata null, the null dimension enters the `else`-branch
and receives no `setMetadataUpdatePolicy()` call at all — not legacy behavior.
```suggestion
* @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}); a lone null here
* simply leaves the metadata dimension unset in the
session
```
##########
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
Review Comment:
⚠️ **Same misleading `@param`** — identical issue on
`setMetadataUpdatePolicy`. Null here only falls back to legacy when the
artifacts update policy is also null.
```suggestion
* @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}); a lone null here
* simply leaves the artifact dimension unset in the
session
```
##########
impl/maven-core/src/test/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactoryTest.java:
##########
@@ -498,6 +498,50 @@ private static RemoteRepository repository(String id,
String url) {
return new RemoteRepository.Builder(id, "default", url).build();
}
+ @Test
+ void artifactsUpdatePolicyTest() throws InvalidRepositoryException {
Review Comment:
⚠️ **Missing mixed-flag test** — `artifactsUpdatePolicyTest()` covers 4
scenarios but omits the conflict case: one new-style policy set **and** `-U` or
`-nsu` also active. The `logger.warn()` branch in
`DefaultRepositorySystemSessionFactory` is currently untested. Add a 5th case,
e.g.:
```java
// Mixed: -UA with -U (legacy flag should be ignored, warning logged)
request = new DefaultMavenExecutionRequest();
request.setLocalRepository(getLocalRepository());
request.setArtifactsUpdatePolicy(RepositoryPolicy.UPDATE_POLICY_ALWAYS);
request.setUpdateSnapshots(true); // legacy -U flag
session = systemSessionFactory.newRepositorySession(request);
assertEquals(RepositoryPolicy.UPDATE_POLICY_ALWAYS,
session.getArtifactUpdatePolicy());
assertNull(session.getMetadataUpdatePolicy());
// verify logger.warn was invoked once
```
--
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]
