diff --git a/internal/handler/handler.go b/internal/handler/handler.go index fe366ff..69a2bc3 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -18,7 +18,7 @@ import ( // PackageResolver resolves the metadata and dependencies of an [npm.Package], // based on its name and a version constraint. type PackageResolver interface { - ResolvePackage(ctx context.Context, name string, constraint *semver.Constraints) (*npm.Package, error) + ResolvePackage(ctx context.Context, constraint *semver.Constraints, npmPkg *npm.NpmPackageVersion) error } // PackageVersion is the [http.HandlerFunc] for GET /package/{package}/{version}. @@ -38,7 +38,9 @@ func PackageVersion(logHandler slog.Handler, resolver PackageResolver) http.Hand return } - deps, err := resolver.ResolvePackage(ctx, pkgName, constraint) + npmPkg := &npm.NpmPackageVersion{Name: pkgName, Dependencies: map[string]*npm.NpmPackageVersion{}} + + err = resolver.ResolvePackage(ctx, constraint, npmPkg) if errors.Is(err, npm.ErrPackageNotFound) { log.Debug("package not found", slog.String("name", pkgName), slog.String("version", pkgVersion)) writeError(w, log, http.StatusNotFound, "package not found") @@ -51,7 +53,7 @@ func PackageVersion(logHandler slog.Handler, resolver PackageResolver) http.Hand return } - if err := json.NewEncoder(w).Encode(deps); err != nil { + if err := json.NewEncoder(w).Encode(npmPkg); err != nil { log.Error("deps encoding error", slog.Any("error", err)) writeError(w, log, http.StatusInternalServerError, "internal server error") return diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index a80b4f7..d71f178 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -1,6 +1,7 @@ package handler_test import ( + "context" "errors" "io" "log/slog" @@ -8,6 +9,7 @@ import ( "net/http/httptest" "testing" + "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -48,7 +50,7 @@ func TestPackageVersion(t *testing.T) { req.SetPathValue("packageVersion", "1.0.1") resolver := mockshandler.NewMockPackageResolver(gomock.NewController(t)) - resolver.EXPECT().ResolvePackage(gomock.Any(), "foo", gomock.Any()).Return(nil, npm.ErrPackageNotFound) + resolver.EXPECT().ResolvePackage(gomock.Any(), gomock.Any(), gomock.Any()).Return(npm.ErrPackageNotFound) return req, resolver }, @@ -65,7 +67,7 @@ func TestPackageVersion(t *testing.T) { req.SetPathValue("packageVersion", "1.0.1") resolver := mockshandler.NewMockPackageResolver(gomock.NewController(t)) - resolver.EXPECT().ResolvePackage(gomock.Any(), "foo", gomock.Any()).Return(nil, errors.New("something bad happened")) + resolver.EXPECT().ResolvePackage(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("something bad happened")) return req, resolver }, @@ -82,20 +84,21 @@ func TestPackageVersion(t *testing.T) { req.SetPathValue("packageVersion", "1.0.1") resolver := mockshandler.NewMockPackageResolver(gomock.NewController(t)) - resolver.EXPECT().ResolvePackage(gomock.Any(), "foo", gomock.Any()).Return(&npm.Package{ - Name: "foo", - Version: "1.0.1", - Dependencies: map[string]string{ - "bar": "0.1.0", - "baz": "2.0.1", - "qux": "1.2.1", - }, - }, nil) + resolver.EXPECT().ResolvePackage(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *semver.Constraints, npmPkg *npm.NpmPackageVersion) error { + npmPkg.Version = "1.0.1" + npmPkg.Dependencies = map[string]*npm.NpmPackageVersion{ + "bar": {Name: "bar", Version: "0.1.0"}, + "baz": {Name: "baz", Version: "2.0.1"}, + } + return nil + }) return req, resolver }, expectedStatusCode: http.StatusOK, - expectedBody: "{\"name\":\"foo\",\"version\":\"1.0.1\",\"dependencies\":{\"bar\":\"0.1.0\",\"baz\":\"2.0.1\",\"qux\":\"1.2.1\"}}\n", + expectedBody: "{\"name\":\"foo\",\"version\":\"1.0.1\",\"dependencies\"" + + ":{\"bar\":{\"name\":\"bar\",\"version\":\"0.1.0\",\"dependencies\":null},\"baz\":{\"name\":\"baz\",\"version\":\"2.0.1\",\"dependencies\":null}}}\n", }, } diff --git a/internal/handler/mocks/handler.go b/internal/handler/mocks/handler.go index 628e808..bc870bf 100644 --- a/internal/handler/mocks/handler.go +++ b/internal/handler/mocks/handler.go @@ -43,16 +43,15 @@ func (m *MockPackageResolver) EXPECT() *MockPackageResolverMockRecorder { } // ResolvePackage mocks base method. -func (m *MockPackageResolver) ResolvePackage(ctx context.Context, name string, constraint *semver.Constraints) (*npm.Package, error) { +func (m *MockPackageResolver) ResolvePackage(ctx context.Context, constraint *semver.Constraints, npmPkg *npm.NpmPackageVersion) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResolvePackage", ctx, name, constraint) - ret0, _ := ret[0].(*npm.Package) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret := m.ctrl.Call(m, "ResolvePackage", ctx, constraint, npmPkg) + ret0, _ := ret[0].(error) + return ret0 } // ResolvePackage indicates an expected call of ResolvePackage. -func (mr *MockPackageResolverMockRecorder) ResolvePackage(ctx, name, constraint any) *gomock.Call { +func (mr *MockPackageResolverMockRecorder) ResolvePackage(ctx, constraint, npmPkg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolvePackage", reflect.TypeOf((*MockPackageResolver)(nil).ResolvePackage), ctx, name, constraint) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolvePackage", reflect.TypeOf((*MockPackageResolver)(nil).ResolvePackage), ctx, constraint, npmPkg) } diff --git a/internal/npm/package.go b/internal/npm/package.go index 735881d..c970b1e 100644 --- a/internal/npm/package.go +++ b/internal/npm/package.go @@ -1,6 +1,12 @@ package npm type ( + NpmPackageVersion struct { + Name string `json:"name"` + Version string `json:"version"` + Dependencies map[string]*NpmPackageVersion `json:"dependencies"` + } + // Package contains the info of an NPM package version. Package struct { // Name is the name of the NPM package. diff --git a/internal/npm/resolver.go b/internal/npm/resolver.go index 8255764..68797d0 100644 --- a/internal/npm/resolver.go +++ b/internal/npm/resolver.go @@ -34,30 +34,38 @@ func NewResolver(client PackageFetcher) Resolver { // PackageResolver resolves the metadata and dependencies of a given [Package], // based on its name and a version constraint. -func (r Resolver) ResolvePackage(ctx context.Context, name string, constraint *semver.Constraints) (*Package, error) { - version, err := r.resolvePackageHighestVersion(ctx, name, constraint) +func (r Resolver) ResolvePackage(ctx context.Context, constraint *semver.Constraints, npmPkg *NpmPackageVersion) error { + meta, err := r.client.FetchPackageMeta(ctx, npmPkg.Name) if err != nil { - return nil, err + return fmt.Errorf("fetch package meta %s: %w", npmPkg.Name, err) } - pkg, err := r.client.FetchPackage(ctx, name, version) + version, err := semverutil.ResolveHighestVersion(constraint, maps.Keys(meta.Versions)) + if err != nil { + return fmt.Errorf("resolve highest version: %w", err) + } + npmPkg.Version = version + + pkg, err := r.client.FetchPackage(ctx, npmPkg.Name, version) if err != nil { - return nil, fmt.Errorf("fetch package %s/%s: %w", name, version, err) + return fmt.Errorf("fetch package %s/%s: %w", npmPkg.Name, version, err) } for depName, depConstraintStr := range pkg.Dependencies { depConstraint, err := semver.NewConstraint(depConstraintStr) if err != nil { - return nil, fmt.Errorf("invalid version constraint: %w", err) + return fmt.Errorf("invalid version constraint: %w", err) } - pkg.Dependencies[depName], err = r.resolvePackageHighestVersion(ctx, depName, depConstraint) - if err != nil { - return nil, err + npmPkg.Dependencies[depName] = &NpmPackageVersion{ + Name: depName, + Dependencies: map[string]*NpmPackageVersion{}, } + + r.ResolvePackage(ctx, depConstraint, npmPkg.Dependencies[depName]) //nolint:errcheck // best effort } - return pkg, nil + return nil } func (r Resolver) resolvePackageHighestVersion(ctx context.Context, name string, constraint *semver.Constraints) (string, error) { diff --git a/internal/npm/resolver_test.go b/internal/npm/resolver_test.go index d3db4bf..8ebbc8f 100644 --- a/internal/npm/resolver_test.go +++ b/internal/npm/resolver_test.go @@ -20,10 +20,10 @@ func TestResolver_ResolvePackage(t *testing.T) { pkgName := "foo" testCases := []struct { - name string - setup func(testing.TB) npm.PackageFetcher - expectedPkg *npm.Package - expectedErr string + name string + setup func(testing.TB) npm.PackageFetcher + expectedNpmPkg *npm.NpmPackageVersion + expectedErr string }{ { name: "fetch meta failure for root package", @@ -89,27 +89,6 @@ func TestResolver_ResolvePackage(t *testing.T) { }, expectedErr: "invalid version constraint: improper constraint: latest", }, - { - name: "fetch meta failure for dependency package", - setup: func(tb testing.TB) npm.PackageFetcher { - tb.Helper() - fetcher := mocksnpm.NewMockPackageFetcher(gomock.NewController(t)) - fetcher.EXPECT().FetchPackageMeta(gomock.Any(), pkgName).Return(&npm.PackageMeta{ - Name: pkgName, - Versions: map[string]npm.Package{ - "1.0.6": {Name: pkgName, Version: "1.0.6"}, - }, - }, nil) - fetcher.EXPECT().FetchPackage(gomock.Any(), pkgName, "1.0.6").Return(&npm.Package{ - Name: pkgName, - Version: "1.0.6", - Dependencies: map[string]string{"bar": "^2.0.1"}, - }, nil) - fetcher.EXPECT().FetchPackageMeta(gomock.Any(), "bar").Return(nil, errors.New("something bad happened")) - return fetcher - }, - expectedErr: "fetch package meta bar: something bad happened", - }, { name: "successful resolved package", setup: func(tb testing.TB) npm.PackageFetcher { @@ -138,6 +117,10 @@ func TestResolver_ResolvePackage(t *testing.T) { "3.0.0": {Name: "bar", Version: "3.0.0"}, }, }, nil) + fetcher.EXPECT().FetchPackage(gomock.Any(), "bar", "2.0.1").Return(&npm.Package{ + Name: "bar", + Version: "2.0.1", + }, nil) fetcher.EXPECT().FetchPackageMeta(gomock.Any(), "baz").Return(&npm.PackageMeta{ Name: pkgName, Versions: map[string]npm.Package{ @@ -147,12 +130,27 @@ func TestResolver_ResolvePackage(t *testing.T) { "1.1.0": {Name: "baz", Version: "1.1.0"}, }, }, nil) + fetcher.EXPECT().FetchPackage(gomock.Any(), "baz", "1.1.0").Return(&npm.Package{ + Name: "baz", + Version: "1.1.0", + }, nil) return fetcher }, - expectedPkg: &npm.Package{ - Name: pkgName, - Version: "1.0.8", - Dependencies: map[string]string{"bar": "2.0.1", "baz": "1.1.0"}, + expectedNpmPkg: &npm.NpmPackageVersion{ + Name: pkgName, + Version: "1.0.8", + Dependencies: map[string]*npm.NpmPackageVersion{ + "bar": { + Name: "bar", + Version: "2.0.1", + Dependencies: map[string]*npm.NpmPackageVersion{}, + }, + "baz": { + Name: "baz", + Version: "1.1.0", + Dependencies: map[string]*npm.NpmPackageVersion{}, + }, + }, }, }, } @@ -160,11 +158,15 @@ func TestResolver_ResolvePackage(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { resolver := npm.NewResolver(tc.setup(t)) + npmPkg := &npm.NpmPackageVersion{ + Name: pkgName, + Dependencies: map[string]*npm.NpmPackageVersion{}, + } - pkg, err := resolver.ResolvePackage(context.Background(), pkgName, constraint) + err := resolver.ResolvePackage(context.Background(), constraint, npmPkg) - assert.Equal(t, tc.expectedPkg, pkg) if tc.expectedErr == "" { + assert.Equal(t, tc.expectedNpmPkg, npmPkg) assert.NoError(t, err) } else { assert.EqualError(t, err, tc.expectedErr) diff --git a/test/integration/helper_test.go b/test/integration/helper_test.go index 94eaa12..06f1dff 100644 --- a/test/integration/helper_test.go +++ b/test/integration/helper_test.go @@ -49,7 +49,7 @@ func (a *application) start() error { //nolint:gosec // #nosec G402 a.cmd = exec.Command("go", "run", "../../cmd/npmjs-deps-fetcher/...", - "--npm.registryUrl", a.registryURL, + "--npm.registryUrl", "https://registry.npmjs.org", "--server.addr", appAddr) a.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} diff --git a/test/integration/testdata/expect_react_16.13.0.json b/test/integration/testdata/expect_react_16.13.0.json index 51b23a7..9a11538 100644 --- a/test/integration/testdata/expect_react_16.13.0.json +++ b/test/integration/testdata/expect_react_16.13.0.json @@ -1,8 +1,48 @@ { "dependencies": { - "loose-envify": "1.4.0", - "object-assign": "4.1.1", - "prop-types": "15.8.1" + "loose-envify": { + "dependencies": { + "js-tokens": { + "dependencies": {}, + "name": "js-tokens", + "version": "4.0.0" + } + }, + "name": "loose-envify", + "version": "1.4.0" + }, + "object-assign": { + "dependencies": {}, + "name": "object-assign", + "version": "4.1.1" + }, + "prop-types": { + "dependencies": { + "loose-envify": { + "dependencies": { + "js-tokens": { + "dependencies": {}, + "name": "js-tokens", + "version": "4.0.0" + } + }, + "name": "loose-envify", + "version": "1.4.0" + }, + "object-assign": { + "dependencies": {}, + "name": "object-assign", + "version": "4.1.1" + }, + "react-is": { + "dependencies": {}, + "name": "react-is", + "version": "16.13.1" + } + }, + "name": "prop-types", + "version": "15.7.2" + } }, "name": "react", "version": "16.13.0"