-
Notifications
You must be signed in to change notification settings - Fork 79
Detect property to exclude Cargo dependencies #1421
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.
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
base: master
Are you sure you want to change the base?
Changes from 16 commits
10883f5
0354a8e
2830904
1ac92d4
5344176
fba1188
f6e48d9
b66b9f7
37c4ed6
9e1e4a4
95d1bac
1293fb9
1a0e93a
0d8f6f2
d2a8a6c
5eb9b16
d2be399
83ebdf1
ae1ee8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.blackduck.integration.detectable.detectables.cargo; | ||
|
||
public enum CargoDependencyType { | ||
NORMAL, | ||
BUILD, | ||
DEV, | ||
PROC_MACRO | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.blackduck.integration.detectable.detectables.cargo; | ||
|
||
import com.blackduck.integration.detectable.detectable.util.EnumListFilter; | ||
|
||
public class CargoDetectableOptions { | ||
private final EnumListFilter<CargoDependencyType> dependencyTypeFilter; | ||
|
||
public CargoDetectableOptions(EnumListFilter<CargoDependencyType> dependencyTypeFilter) { | ||
this.dependencyTypeFilter = dependencyTypeFilter; | ||
} | ||
|
||
public EnumListFilter<CargoDependencyType> getDependencyTypeFilter() { | ||
return dependencyTypeFilter; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,13 @@ | |
import java.nio.charset.StandardCharsets; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Set; | ||
import java.util.HashSet; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.stream.Collectors; | ||
|
||
import com.blackduck.integration.detectable.detectables.cargo.data.CargoLockPackageData; | ||
import org.apache.commons.io.FileUtils; | ||
import org.jetbrains.annotations.Nullable; | ||
|
||
|
@@ -39,26 +43,72 @@ public CargoExtractor( | |
this.cargoLockPackageTransformer = cargoLockPackageTransformer; | ||
} | ||
|
||
public Extraction extract(File cargoLockFile, @Nullable File cargoTomlFile) throws IOException, DetectableException, MissingExternalIdException { | ||
public Extraction extract(File cargoLockFile, @Nullable File cargoTomlFile, CargoDetectableOptions cargoDetectableOptions) throws IOException, DetectableException, MissingExternalIdException { | ||
CargoLockData cargoLockData = new Toml().read(cargoLockFile).to(CargoLockData.class); | ||
List<CargoLockPackage> packages = cargoLockData.getPackages() | ||
.orElse(new ArrayList<>()).stream() | ||
List<CargoLockPackageData> cargoLockPackageDataList = cargoLockData.getPackages().orElse(new ArrayList<>()); | ||
List<CargoLockPackageData> filteredPackages = cargoLockPackageDataList; | ||
String cargoTomlContents = FileUtils.readFileToString(cargoTomlFile, StandardCharsets.UTF_8); | ||
|
||
if (cargoDetectableOptions != null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of checking on cargoDetectableOptions object, can we check on dependencyTypeFilter instead? There could be more properties added to this detector in future not related to dependency exclusion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, you're right. The actual filtering is handled by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that, my only concern is that if in future lets say we add some new property for excluding workspaces as an example then those options will also be passed in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current code setup doesn't call the methods of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know, currently your code is not getting executed if cargoDetectableOptions is null. It is not guaranteed that you are going to work on this in any future enhancement related to Cargo and it could be easily missed. I believe we should address this now, its a simple change and not a complicated ask. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, sure. I'll update accordingly. |
||
Map<String, String> excludableDependencyMap = cargoTomlParser.parseDependenciesToExclude(cargoTomlContents, cargoDetectableOptions); | ||
filteredPackages = excludeDependencies(cargoLockPackageDataList, excludableDependencyMap); | ||
} | ||
|
||
List<CargoLockPackage> packages = filteredPackages.stream() | ||
.map(cargoLockPackageDataTransformer::transform) | ||
.collect(Collectors.toList()); | ||
|
||
DependencyGraph graph = cargoLockPackageTransformer.transformToGraph(packages); | ||
|
||
Optional<NameVersion> projectNameVersion = Optional.empty(); | ||
if (cargoTomlFile != null) { | ||
String cargoTomlContents = FileUtils.readFileToString(cargoTomlFile, StandardCharsets.UTF_8); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously we were checking whether the file is not null before reading it. Now, we don't. Also, I think something like this should happen:
|
||
projectNameVersion = cargoTomlParser.parseNameVersionFromCargoToml(cargoTomlContents); | ||
} | ||
|
||
CodeLocation codeLocation = new CodeLocation(graph); //TODO: Consider for producing a ProjectDependencyGraph | ||
|
||
return new Extraction.Builder() | ||
.success(codeLocation) | ||
.nameVersionIfPresent(projectNameVersion) | ||
.build(); | ||
} | ||
|
||
private List<CargoLockPackageData> excludeDependencies( | ||
List<CargoLockPackageData> packages, | ||
Map<String, String> excludableDependencyMap | ||
) { | ||
Set<String> excludedNames = new HashSet<>(); | ||
|
||
List<CargoLockPackageData> filtered = packages.stream() | ||
.filter(pkg -> { | ||
String name = pkg.getName().orElse(null); | ||
String version = pkg.getVersion().orElse(null); | ||
if (name == null || version == null) return true; | ||
|
||
if (excludableDependencyMap.containsKey(name)) { | ||
String constraint = excludableDependencyMap.get(name); | ||
boolean matches = constraint == null || VersionUtils.versionMatches(constraint, version); | ||
if (matches) { | ||
excludedNames.add(name); | ||
return false; | ||
} | ||
} | ||
|
||
return true; | ||
}) | ||
.collect(Collectors.toList()); | ||
|
||
return filtered.stream() | ||
.map(pkg -> new CargoLockPackageData( | ||
pkg.getName().orElse(null), | ||
pkg.getVersion().orElse(null), | ||
pkg.getSource().orElse(null), | ||
pkg.getChecksum().orElse(null), | ||
pkg.getDependencies() | ||
.orElse(new ArrayList<>()) | ||
.stream() | ||
.filter(dep -> !excludedNames.contains(dep)) | ||
.collect(Collectors.toList()) | ||
)) | ||
.collect(Collectors.toList()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you describe situations in which PROC_MACRO exclusion is useful? And I guess it's not applicable to lock file extractions?