From 3875b4e742c663dad3f54454230f6da87042915d Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Wed, 23 Oct 2024 11:06:19 -0400 Subject: [PATCH] chore: address PR feedback Signed-off-by: Keith Zantow --- syft/pkg/cataloger/java/archive_parser.go | 12 ++++++------ syft/pkg/cataloger/java/archive_parser_test.go | 6 +++--- syft/pkg/cataloger/java/cataloger.go | 2 +- syft/pkg/cataloger/java/internal/maven/resolver.go | 8 ++++---- syft/pkg/cataloger/java/parse_pom_xml_test.go | 12 +++++++----- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/syft/pkg/cataloger/java/archive_parser.go b/syft/pkg/cataloger/java/archive_parser.go index fbd2acdf0f7..e0ed965b212 100644 --- a/syft/pkg/cataloger/java/archive_parser.go +++ b/syft/pkg/cataloger/java/archive_parser.go @@ -69,13 +69,13 @@ func newGenericArchiveParserAdapter(cfg ArchiveCatalogerConfig) genericArchivePa return genericArchiveParserAdapter{cfg: cfg} } -// parseJavaArchive is a parser function for java archive contents, returning all Java libraries and nested archives. -func (gap genericArchiveParserAdapter) parseJavaArchiveMain(ctx context.Context, _ file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) { - return gap.parseJavaArchive(ctx, reader, nil) +// parseJavaArchive is a parser function for java archive contents, returning all Java libraries and nested archives +func (gap genericArchiveParserAdapter) parseJavaArchive(ctx context.Context, _ file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) { + return gap.processJavaArchive(ctx, reader, nil) } -// parseJavaArchive is a parser function for java archive contents, returning all Java libraries and nested archives. -func (gap genericArchiveParserAdapter) parseJavaArchive(ctx context.Context, reader file.LocationReadCloser, parentPkg *pkg.Package) ([]pkg.Package, []artifact.Relationship, error) { +// processJavaArchive processes an archive for java contents, returning all Java libraries and nested archives +func (gap genericArchiveParserAdapter) processJavaArchive(ctx context.Context, reader file.LocationReadCloser, parentPkg *pkg.Package) ([]pkg.Package, []artifact.Relationship, error) { parser, cleanupFn, err := newJavaArchiveParser(ctx, reader, true, gap.cfg) // note: even on error, we should always run cleanup functions defer cleanupFn() @@ -583,7 +583,7 @@ func discoverPkgsFromOpener(ctx context.Context, location file.Location, pathWit nestedLocation := file.NewLocationFromCoordinates(location.Coordinates) nestedLocation.AccessPath = nestedPath gap := newGenericArchiveParserAdapter(cfg) - nestedPkgs, nestedRelationships, err := gap.parseJavaArchive(ctx, file.LocationReadCloser{ + nestedPkgs, nestedRelationships, err := gap.processJavaArchive(ctx, file.LocationReadCloser{ Location: nestedLocation, ReadCloser: archiveReadCloser, }, parentPkg) diff --git a/syft/pkg/cataloger/java/archive_parser_test.go b/syft/pkg/cataloger/java/archive_parser_test.go index 468ff350640..b1779bbf185 100644 --- a/syft/pkg/cataloger/java/archive_parser_test.go +++ b/syft/pkg/cataloger/java/archive_parser_test.go @@ -638,7 +638,7 @@ func TestParseNestedJar(t *testing.T) { require.NoError(t, err) gap := newGenericArchiveParserAdapter(ArchiveCatalogerConfig{}) - actual, _, err := gap.parseJavaArchive(context.Background(), file.LocationReadCloser{ + actual, _, err := gap.processJavaArchive(context.Background(), file.LocationReadCloser{ Location: file.NewLocation(fixture.Name()), ReadCloser: fixture, }, nil) @@ -1368,7 +1368,7 @@ func Test_parseJavaArchive_regressions(t *testing.T) { return true }), ). - TestParser(t, gap.parseJavaArchiveMain) + TestParser(t, gap.parseJavaArchive) }) } } @@ -1523,5 +1523,5 @@ func Test_corruptJarArchive(t *testing.T) { pkgtest.NewCatalogTester(). FromFile(t, "test-fixtures/corrupt/example.jar"). WithError(). - TestParser(t, ap.parseJavaArchiveMain) + TestParser(t, ap.parseJavaArchive) } diff --git a/syft/pkg/cataloger/java/cataloger.go b/syft/pkg/cataloger/java/cataloger.go index 96a52996d7e..2affb83fd7f 100644 --- a/syft/pkg/cataloger/java/cataloger.go +++ b/syft/pkg/cataloger/java/cataloger.go @@ -13,7 +13,7 @@ func NewArchiveCataloger(cfg ArchiveCatalogerConfig) pkg.Cataloger { gap := newGenericArchiveParserAdapter(cfg) c := generic.NewCataloger("java-archive-cataloger"). - WithParserByGlobs(gap.parseJavaArchiveMain, archiveFormatGlobs...) + WithParserByGlobs(gap.parseJavaArchive, archiveFormatGlobs...) if cfg.IncludeIndexedArchives { // java archives wrapped within zip files diff --git a/syft/pkg/cataloger/java/internal/maven/resolver.go b/syft/pkg/cataloger/java/internal/maven/resolver.go index 6df67c5ea85..710600ae2f6 100644 --- a/syft/pkg/cataloger/java/internal/maven/resolver.go +++ b/syft/pkg/cataloger/java/internal/maven/resolver.go @@ -315,10 +315,10 @@ func (r *Resolver) FindPom(ctx context.Context, groupID, artifactID, version str } id := ID{groupID, artifactID, version} - pom := r.resolved[id] + existingPom := r.resolved[id] - if pom != nil { - return pom, nil + if existingPom != nil { + return existingPom, nil } var errs error @@ -334,7 +334,7 @@ func (r *Resolver) FindPom(ctx context.Context, groupID, artifactID, version str } // resolve via network maven repository - if pom == nil && r.cfg.UseNetwork { + if r.cfg.UseNetwork { pom, err := r.findPomInRemotes(ctx, groupID, artifactID, version) if pom != nil { r.resolved[id] = pom diff --git a/syft/pkg/cataloger/java/parse_pom_xml_test.go b/syft/pkg/cataloger/java/parse_pom_xml_test.go index 18c68a5f0f7..6bd0a5f2382 100644 --- a/syft/pkg/cataloger/java/parse_pom_xml_test.go +++ b/syft/pkg/cataloger/java/parse_pom_xml_test.go @@ -138,7 +138,7 @@ func Test_parseCommonsTextPomXMLProject(t *testing.T) { UseNetwork: false, UseMavenLocalRepository: false, }, - expected: getCommonsTextExpectedPackages(file.NewLocation("pom.xml"), false), + expected: getCommonsTextExpectedPackages(false), }, { name: "use network", @@ -148,7 +148,7 @@ func Test_parseCommonsTextPomXMLProject(t *testing.T) { MavenBaseURL: mavenBaseURL, UseMavenLocalRepository: false, }, - expected: getCommonsTextExpectedPackages(file.NewLocation("pom.xml"), true), + expected: getCommonsTextExpectedPackages(true), }, { name: "use local repository", @@ -158,7 +158,7 @@ func Test_parseCommonsTextPomXMLProject(t *testing.T) { UseMavenLocalRepository: true, MavenLocalRepositoryDir: mavenLocalRepoDir, }, - expected: getCommonsTextExpectedPackages(file.NewLocation("pom.xml"), true), + expected: getCommonsTextExpectedPackages(true), }, { name: "transitive dependencies", @@ -468,7 +468,9 @@ type expected struct { relationships []artifact.Relationship } -func getCommonsTextExpectedPackages(loc file.Location, resolved bool) expected { +func getCommonsTextExpectedPackages(resolved bool) expected { + pomXmlLocation := file.NewLocationSet(file.NewLocation("pom.xml")) + commonsText := pkg.Package{ Name: "commons-text", Version: "1.10.0", @@ -673,7 +675,7 @@ func getCommonsTextExpectedPackages(loc file.Location, resolved bool) expected { var relationships []artifact.Relationship for i := range pkgs { p := &pkgs[i] - p.Locations = file.NewLocationSet(loc) + p.Locations = pomXmlLocation finalizePackage(p) if i == 0 { continue