Skip to content

Add Maven Endpoints#568

Open
mohamed-abead wants to merge 1 commit intofinos:masterfrom
goldmansachs:review-new-dependency-resolution
Open

Add Maven Endpoints#568
mohamed-abead wants to merge 1 commit intofinos:masterfrom
goldmansachs:review-new-dependency-resolution

Conversation

@mohamed-abead
Copy link

@mohamed-abead mohamed-abead commented Mar 12, 2026

Add Maven Endpoints to Shift to Using Maven Repo System in resolving dependencies

@mohamed-abead mohamed-abead requested a review from a team as a code owner March 12, 2026 22:49
@Produces(MediaType.APPLICATION_JSON)
public Response analyzeDependencyTreeFromArtifactDependenciesMaven(@ApiParam("projectDependencies") List<ArtifactDependency> projectDependencies)
{
return handleResponse(GET_PROJECT_DEPENDENCY_TREE, () -> this.projectApi.getProjectDependencyReportMaven(projectDependencies));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return handleResponse(GET_PROJECT_DEPENDENCY_TREE, () -> this.projectApi.getProjectDependencyReportMaven(projectDependencies));
return handleResponse(GET_PROJECT_DEPENDENCY_TREE_FROM_MAVEN, () -> this.projectApi.getProjectDependencyReportMaven(projectDependencies));


@POST
@Path("/projects/analyzeDependencyTreeFromArtifactDependenciesMaven")
@ApiOperation(GET_PROJECT_DEPENDENCY_TREE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@ApiOperation(GET_PROJECT_DEPENDENCY_TREE)
@ApiOperation(GET_PROJECT_DEPENDENCY_TREE_FROM_MAVEN)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also can we probably point the old API to using this

}

@Override
public Set<ProjectVersion> getDependenciesMaven(List<ArtifactDependency> artifactDependencies, boolean transitive)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few pieces missing here:

  1. we calculate transitive dependencies for projects when we store a project in metadata, we need to use the same resolution to store those pieces as well
  2. this should probably not be in project services (look into DependencyUtils)

@Path("/projects/dependenciesFromArtifactDependenciesMaven")
@ApiOperation(GET_VERSIONS_DEPENDENCY_ENTITIES_MAVEN)
@Produces(MediaType.APPLICATION_JSON)
public Response getAllEntitiesFromArtifactDependenciesMaven(@ApiParam("projectDependencies") List<ArtifactDependency> projectDependencies,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe don't need another API for this, we could get the existing API to point to using this resolution

RepositorySystem system = newRepositorySystem();
DefaultRepositorySystemSession session = newSession(system);

InMemoryArtifactDescriptorReader reader = new InMemoryArtifactDescriptorReader(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment by GitHub copilot:
This looks like there's a potential bug here. In both getDependenciesMaven() and getProjectDependencyReportMaven() methods, you're creating two separate instances of InMemoryArtifactDescriptorReader:

  1. First instance - Created inside newRepositorySystem() at line ~205 and registered with the locator
  2. Second instance - Created at line ~381 (in getDependenciesMaven()) where exclusions are actually set via reader.setExclusions()

The Problem: Maven's resolver uses the first reader instance (which has no exclusions), while the second reader (which has the exclusions) is never used by the resolver.

Result: Dependency exclusions will be silently ignored.

Suggested Fix: Either:

  • Pass artifactDependencies to newRepositorySystem() so it can set exclusions on its reader instance before returning, OR
  • Create the reader once before calling newRepositorySystem() and reuse the same instance

The same issue exists in getProjectDependencyReportMaven() at line ~495.

@@ -325,6 +490,111 @@ public ProjectDependencyReport getProjectDependencyReportFromProjectVersionList(
return getProjectDependencyReport(artifactDependencies);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also do we need to cleanup old code for resolution (which has exclusions)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants