Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support projects providing several artifacts #88

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support projects providing several artifacts #88

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions src/main/java/org/spdx/sbom/gradle/utils/SpdxDocumentBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,15 @@
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -82,7 +81,7 @@ public class SpdxDocumentBuilder {

private final HashMap<ComponentIdentifier, LinkedHashSet<ComponentIdentifier>> tree =
new LinkedHashMap<>();
private final Map<ComponentIdentifier, File> resolvedExternalArtifacts;
private final Map<ComponentIdentifier, Collection<File>> resolvedExternalArtifacts;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think using List everywhere instead of Collection would be nice. We don't plan on using a non-list anyway

private final Map<String, URI> mavenArtifactRepositories;
private final Map<String, PomInfo> poms;
private final Logger logger;
Expand Down Expand Up @@ -159,21 +158,7 @@ public SpdxDocumentBuilder(
allProjects.stream().collect(Collectors.toMap(ProjectInfo::getPath, Function.identity()));
this.describesProject = knownProjects.get(projectPath);

this.resolvedExternalArtifacts =
resolvedExternalArtifacts.entrySet().stream()
.filter(
e -> !(e.getKey().getComponentIdentifier() instanceof ProjectComponentIdentifier))
.collect(
Collectors.toMap(
e -> e.getKey().getComponentIdentifier(),
Entry::getValue,
(file1, file2) -> {
if (Objects.equals(file1, file2)) {
return file1;
} else
throw new IllegalStateException(
"cannot merge duplicate " + file1 + " and " + file2);
}));
this.resolvedExternalArtifacts = fromResolvedArtifacts(resolvedExternalArtifacts);
this.mavenArtifactRepositories = mavenArtifactRepositories;
this.poms = poms;

Expand All @@ -183,8 +168,7 @@ public SpdxDocumentBuilder(
public void add(ResolvedComponentResult root) throws InvalidSPDXAnalysisException, IOException {
add(rootPackageId, root, new HashSet<>());
doc.setDocumentDescribes(
Collections.singletonList(
rootPackage == null ? spdxPackages.get(root.getId()) : rootPackage));
List.of(rootPackage == null ? spdxPackages.get(root.getId()) : rootPackage));

for (var pkg : tree.keySet()) {
for (var child : tree.get(pkg)) {
Expand Down Expand Up @@ -225,13 +209,13 @@ private boolean maybeAddPackage(ComponentIdentifier parent, ResolvedComponentRes
return true;
}

Optional<SpdxPackage> maybePackage = createPackageIfNeeded(component);
if (maybePackage.isEmpty()) {
List<SpdxPackage> createdPackages = createPackageIfNeeded(component);
if (createdPackages.isEmpty()) {
logger.info("ignoring: " + component.getId());
return false;
}

spdxPackages.put(component.getId(), maybePackage.get());
// TODO: support createdPackages list if several packages created
spdxPackages.put(component.getId(), createdPackages.get(0));
Copy link
Collaborator

Choose a reason for hiding this comment

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

is 0 expected to be the "main" variant?

tree.putIfAbsent(component.getId(), new LinkedHashSet<>());
if (parent != null) {
tree.get(parent).add(component.getId());
Expand All @@ -240,14 +224,14 @@ private boolean maybeAddPackage(ComponentIdentifier parent, ResolvedComponentRes
return true;
}

private Optional<SpdxPackage> createPackageIfNeeded(ResolvedComponentResult component)
private List<SpdxPackage> createPackageIfNeeded(ResolvedComponentResult component)
throws InvalidSPDXAnalysisException, IOException {
if (component.getId() instanceof ProjectComponentIdentifier) {
return shouldCreatePackageForProject(component)
? Optional.of(createProjectPackage(component))
: Optional.empty();
? List.of(createProjectPackage(component))
: List.of();
} else if (component.getId() instanceof ModuleComponentIdentifier) {
return createMavenModulePackage(component);
return createMavenModulePackages(component);
} else {
throw new RuntimeException(
"Unknown package type: "
Expand All @@ -270,6 +254,7 @@ private boolean shouldCreatePackageForProject(ResolvedComponentResult resolvedCo

private SpdxPackage createProjectPackage(ResolvedComponentResult resolvedComponentResult)
throws InvalidSPDXAnalysisException {
// TODO: project can expose several artifacts
var projectId = (ProjectComponentIdentifier) resolvedComponentResult.getId();

resolvedComponentResult.getVariants();
Expand All @@ -289,7 +274,7 @@ private SpdxPackage createProjectPackage(ResolvedComponentResult resolvedCompone
SpdxPackageBuilder builder =
doc.createPackage(
doc.getModelStore().getNextId(IdType.SpdxId, doc.getDocumentUri()),
pi.getName(),
pi.getName(), // TODO: name could be from file in case several files declared
Copy link
Collaborator

Choose a reason for hiding this comment

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

do variants have names? or is it all implicit?

new SpdxNoAssertionLicense(),
"NOASSERTION",
new SpdxNoAssertionLicense())
Expand All @@ -309,13 +294,17 @@ private SpdxPackage createProjectPackage(ResolvedComponentResult resolvedCompone
return builder.build();
}

private Optional<SpdxPackage> createMavenModulePackage(
private List<SpdxPackage> createMavenModulePackages(
ResolvedComponentResult resolvedComponentResult)
throws InvalidSPDXAnalysisException, IOException {

// if the project doesn't resolve to anything, ignore it
File dependencyFile = resolvedExternalArtifacts.get(resolvedComponentResult.getId());
if (dependencyFile != null) {
Collection<File> dependencyFiles =
resolvedExternalArtifacts.get(resolvedComponentResult.getId());
if (dependencyFiles == null) return List.of();
loosebazooka marked this conversation as resolved.
Show resolved Hide resolved

List<SpdxPackage> results = new ArrayList<>();
for (var dependencyFile : dependencyFiles) {
ModuleVersionIdentifier moduleId = resolvedComponentResult.getModuleVersion();
PomInfo pomInfo = poms.get(resolvedComponentResult.getId().getDisplayName());

Expand Down Expand Up @@ -375,9 +364,25 @@ private Optional<SpdxPackage> createMavenModulePackage(
spdxPkgBuilder.setChecksums(List.of(checksumSha1, checksumSha256));
spdxPkgBuilder.setFilesAnalyzed(false);

return Optional.of(spdxPkgBuilder.build());
results.add(spdxPkgBuilder.build());
}
return Optional.empty();
return results;
}

private static Map<ComponentIdentifier, Collection<File>> fromResolvedArtifacts(
Map<ComponentArtifactIdentifier, File> artifacts) {
Map<ComponentIdentifier, Collection<File>> results = new HashMap<>();
artifacts.forEach(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this still needs to filter out project components (for #69)

(identifier, file) -> {
if (results.containsKey(identifier.getComponentIdentifier())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This section can be compressed down to:

results.getOrDefault(identifier.getComponentIdentifier(), new ArrayList<File>()).add(file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But getOrDefault does not put the defaultValue back into Map

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh right, yeah you have to add it in. oops

Copy link
Collaborator

@loosebazooka loosebazooka Dec 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think computeIfAbsent is what I actually wanted, it does the insert.

results.computeIfAbsent(identifier.getComponentIdentifier(), l -> new ArrayList<File>()).add(file)

results.get(identifier.getComponentIdentifier()).add(file);
} else {
List<File> files = new ArrayList<>();
files.add(file);
results.put(identifier.getComponentIdentifier(), files);
}
});
return results;
}

public SpdxDocument getSpdxDocument() {
Expand Down
Loading