Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

Rework of Artifact::mergeWith #622

Open
1 task
neubs-bsi opened this issue Nov 10, 2020 · 1 comment
Open
1 task

Rework of Artifact::mergeWith #622

neubs-bsi opened this issue Nov 10, 2020 · 1 comment
Labels
bug Something isn't working

Comments

@neubs-bsi
Copy link
Contributor

neubs-bsi commented Nov 10, 2020

Summary of the Bug

The Artifact::mergeWith method will not work correctly if there are two artifacts with the same coordinates and facts but different content.

Steps to reproduce

  1. Option:
    Run example-projects/example-project with the CSVAnalyzer and csv file, which contains two artifacts with same coordinates, but different declared licenses:
Artifact Id;Group Id;Version;Coordinate Type;Effective License;Declared License;Observed License;Copyrights;Hash;Source URL;Release Tag URL;Software Heritage ID;Clearing State;Change Status;CPE;File Name
commons-cli;org.apache.commons;1.4;mvn;;Apache-2.0;;;;;;;;;;
commons-cli;org.apache.commons;1.4;mvn;;MIT;;;;;;;;;;
  1. Option:
    Run the code snippet from the comment below

Acceptance Criteria

  • The mergeWith method works properly when merging two artifacts with the same coordinates and facts.

Definition of Done

  • Acceptance criteria fulfilled
  • A test case is created to reproduce the bug
  • A PR is created, the CI infrastructure reports green, the bug test case proves that bug is fixed
  • The PR is reviewed and approved
  • No TODOs left in the code unless explained in the ticket, if something else is still open, this is summarized in a comment in the issue
  • Documentation is updated
@neubs-bsi neubs-bsi added the bug Something isn't working label Nov 10, 2020
@neubs-bsi
Copy link
Contributor Author

From the discussion:
Artifact::mergeWith: This method should be reworked because it does not work correctly if there are two artifacts with the same coordinates and facts but different content e.g.:

License declaredLicenses1 = new License();
declaredLicenses1.setName("license1");
declaredLicenses1.setLongName("licenseNumber1");
License declaredLicenses2 = new License();
declaredLicenses2.setName("license2");
declaredLicenses2.setLongName("licenseNumber2");

Artifact a1 = new Artifact()
        .addFact(new ArtifactCoordinates(new Coordinate(Coordinate.Types.MAVEN, "com.example", "lib1", "1.0")))
        .addFact((new ArtifactFilename("lib1", "hash1")))
        .addFact(new DeclaredLicenseInformation(declaredLicenses1));

Artifact a2 = new Artifact()
        .addFact(new ArtifactCoordinates(new Coordinate(Coordinate.Types.MAVEN, "com.example", "lib1", "1.0")))
        .addFact(new OverriddenLicenseInformation(declaredLicenses2));

a1.mergeWith(a2);

a1 before mergeWith (license1):
Screen Shot 2019-11-22 at 13 02 27
a1 after mergeWith (license2):
Screen Shot 2019-11-22 at 13 02 46

ProcessingState::applyWorkflowStepResult:
IMO this method works properly and serves its purpose when using it the analyzers and processors. All artifacts from the analyzers are flattened to one list and every processor will overwrite the artifacts from the step before, because it was enriched with new information.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant