Copilot commented on code in PR #1957:
URL: https://github.com/apache/maven-resolver/pull/1957#discussion_r3570246251
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java:
##########
@@ -182,77 +219,106 @@ public DefaultRepositorySystem(
@Override
public VersionResult resolveVersion(RepositorySystemSession session,
VersionRequest request)
throws VersionResolutionException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateVersionRequest(session, request);
+ if (!isReentrant(request.getTrace())) {
+ validateSession(session);
+ repositorySystemValidator.validateVersionRequest(session, request);
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
return versionResolver.resolveVersion(session, request);
}
@Override
public VersionRangeResult resolveVersionRange(RepositorySystemSession
session, VersionRangeRequest request)
throws VersionRangeResolutionException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateVersionRangeRequest(session,
request);
+ if (!isReentrant(request.getTrace())) {
+ validateSession(session);
+ repositorySystemValidator.validateVersionRangeRequest(session,
request);
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
return versionRangeResolver.resolveVersionRange(session, request);
}
@Override
public ArtifactDescriptorResult readArtifactDescriptor(
RepositorySystemSession session, ArtifactDescriptorRequest
request) throws ArtifactDescriptorException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateArtifactDescriptorRequest(session,
request);
+ boolean outermost = !isReentrant(request.getTrace());
+ if (outermost) {
+ validateSession(session);
+
repositorySystemValidator.validateArtifactDescriptorRequest(session, request);
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
ArtifactDescriptorResult descriptorResult =
artifactDescriptorReader.readArtifactDescriptor(session, request);
- for (ArtifactDecorator decorator :
Utils.getArtifactDecorators(session, artifactDecoratorFactories)) {
-
descriptorResult.setArtifact(decorator.decorateArtifact(descriptorResult));
+ if (outermost) {
+ for (ArtifactDecorator decorator :
Utils.getArtifactDecorators(session, artifactDecoratorFactories)) {
+
descriptorResult.setArtifact(decorator.decorateArtifact(descriptorResult));
+ }
}
return descriptorResult;
}
@Override
public ArtifactResult resolveArtifact(RepositorySystemSession session,
ArtifactRequest request)
throws ArtifactResolutionException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateArtifactRequests(session,
Collections.singleton(request));
+ if (!isReentrant(request.getTrace())) {
+ validateSession(session);
+ repositorySystemValidator.validateArtifactRequests(session,
Collections.singleton(request));
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
return artifactResolver.resolveArtifact(session, request);
}
@Override
public List<ArtifactResult> resolveArtifacts(
RepositorySystemSession session, Collection<? extends
ArtifactRequest> requests)
throws ArtifactResolutionException {
- validateSession(session);
requireNonNull(requests, "requests cannot be null");
- repositorySystemValidator.validateArtifactRequests(session, requests);
+ RequestTrace firstTrace =
+
requests.stream().map(ArtifactRequest::getTrace).findFirst().orElse(null);
+ if (!isReentrant(firstTrace)) {
+ validateSession(session);
+ repositorySystemValidator.validateArtifactRequests(session,
requests);
Review Comment:
`resolveArtifacts(...)` uses the trace of an arbitrary “first” request to
decide re-entrancy and does not stamp the `REPOSITORY_SYSTEM_CALL` marker into
the requests when the call is outermost. This makes re-entrancy detection
unreliable for collections (iteration order can be undefined) and can also fail
to propagate the marker for subsequent nested `RepositorySystem` calls
triggered during artifact resolution.
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java:
##########
@@ -182,77 +219,106 @@ public DefaultRepositorySystem(
@Override
public VersionResult resolveVersion(RepositorySystemSession session,
VersionRequest request)
throws VersionResolutionException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateVersionRequest(session, request);
+ if (!isReentrant(request.getTrace())) {
+ validateSession(session);
+ repositorySystemValidator.validateVersionRequest(session, request);
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
return versionResolver.resolveVersion(session, request);
}
@Override
public VersionRangeResult resolveVersionRange(RepositorySystemSession
session, VersionRangeRequest request)
throws VersionRangeResolutionException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateVersionRangeRequest(session,
request);
+ if (!isReentrant(request.getTrace())) {
+ validateSession(session);
+ repositorySystemValidator.validateVersionRangeRequest(session,
request);
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
return versionRangeResolver.resolveVersionRange(session, request);
}
@Override
public ArtifactDescriptorResult readArtifactDescriptor(
RepositorySystemSession session, ArtifactDescriptorRequest
request) throws ArtifactDescriptorException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateArtifactDescriptorRequest(session,
request);
+ boolean outermost = !isReentrant(request.getTrace());
+ if (outermost) {
+ validateSession(session);
+
repositorySystemValidator.validateArtifactDescriptorRequest(session, request);
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
ArtifactDescriptorResult descriptorResult =
artifactDescriptorReader.readArtifactDescriptor(session, request);
- for (ArtifactDecorator decorator :
Utils.getArtifactDecorators(session, artifactDecoratorFactories)) {
-
descriptorResult.setArtifact(decorator.decorateArtifact(descriptorResult));
+ if (outermost) {
+ for (ArtifactDecorator decorator :
Utils.getArtifactDecorators(session, artifactDecoratorFactories)) {
+
descriptorResult.setArtifact(decorator.decorateArtifact(descriptorResult));
+ }
}
return descriptorResult;
}
@Override
public ArtifactResult resolveArtifact(RepositorySystemSession session,
ArtifactRequest request)
throws ArtifactResolutionException {
- validateSession(session);
requireNonNull(request, "request cannot be null");
- repositorySystemValidator.validateArtifactRequests(session,
Collections.singleton(request));
+ if (!isReentrant(request.getTrace())) {
+ validateSession(session);
+ repositorySystemValidator.validateArtifactRequests(session,
Collections.singleton(request));
+ request.setTrace(RequestTrace.newChild(request.getTrace(),
REPOSITORY_SYSTEM_CALL));
+ }
return artifactResolver.resolveArtifact(session, request);
}
@Override
public List<ArtifactResult> resolveArtifacts(
RepositorySystemSession session, Collection<? extends
ArtifactRequest> requests)
throws ArtifactResolutionException {
- validateSession(session);
requireNonNull(requests, "requests cannot be null");
- repositorySystemValidator.validateArtifactRequests(session, requests);
+ RequestTrace firstTrace =
+
requests.stream().map(ArtifactRequest::getTrace).findFirst().orElse(null);
+ if (!isReentrant(firstTrace)) {
+ validateSession(session);
+ repositorySystemValidator.validateArtifactRequests(session,
requests);
+ }
return artifactResolver.resolveArtifacts(session, requests);
}
@Override
public List<MetadataResult> resolveMetadata(
RepositorySystemSession session, Collection<? extends
MetadataRequest> requests) {
- validateSession(session);
requireNonNull(requests, "requests cannot be null");
- repositorySystemValidator.validateMetadataRequests(session, requests);
+ RequestTrace firstTrace =
+
requests.stream().map(MetadataRequest::getTrace).findFirst().orElse(null);
+ if (!isReentrant(firstTrace)) {
+ validateSession(session);
+ repositorySystemValidator.validateMetadataRequests(session,
requests);
Review Comment:
`resolveMetadata(...)` has the same collection-level re-entrancy issue as
`resolveArtifacts(...)`: it checks only the first request’s trace (potentially
nondeterministic) and does not stamp the `REPOSITORY_SYSTEM_CALL` marker into
the requests on the outermost call. This can cause nested `RepositorySystem`
calls during metadata resolution to be treated as non-reentrant and re-run
validation/decoration unexpectedly.
--
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]
