Merge pull request #16 from grafana/refactor-catalog-constructors

refactor catalog constructors

Author: pablochacin

.golangci.yml

@@ -24,6 +24,8 @@ issues:
        - gocognit
        - funlen
        - lll
+       - forbidigo
+       - gosec
    - path: js\/modules\/k6\/http\/.*_test\.go
      linters:
        # k6/http module's tests are quite complex because they often have several nested levels.

catalog.go

@@ -2,7 +2,9 @@
 package k6catalog
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -18,10 +20,13 @@ const (
 )
 
 var (
-	ErrCannotSatisfy     = errors.New("cannot satisfy dependency") //nolint:revive
-	ErrInvalidConstrain  = errors.New("invalid constrain")         //nolint:revive
-	ErrUnknownDependency = errors.New("unknown dependency")        //nolint:revive
-	ErrDownload          = errors.New("downloading catalog")       //nolint:revive
+	ErrCannotSatisfy      = errors.New("cannot satisfy dependency")        //nolint:revive
+	ErrDownload           = errors.New("downloading catalog")              //nolint:revive
+	ErrInvalidConstrain   = errors.New("invalid constrain")                //nolint:revive
+	ErrInvalidCatalog     = fmt.Errorf("invalid catalog")                  //nolint:revive
+	ErrOpening            = errors.New("opening catalog")                  //nolint:revive
+	ErrUnknownDependency  = errors.New("unknown dependency")               //nolint:revive
+	ErrDependencyNotFound = fmt.Errorf("dependency not found in registry") //nolint:revive
 )
 
 // Dependency defines a Dependency with a version constrain
@@ -45,22 +50,54 @@ type Catalog interface {
 	Resolve(ctx context.Context, dep Dependency) (Module, error)
 }
 
+// entry defines a catalog entry
+type entry struct {
+	Module   string   `json:"module,omitempty"`
+	Versions []string `json:"versions,omitempty"`
+}
+
 type catalog struct {
-	registry Registry
+	dependencies map[string]entry
 }
 
-// NewCatalog creates a catalog from a registry
-func NewCatalog(registry Registry) Catalog {
-	return catalog{registry: registry}
+// getVersions returns the versions for a given module
+func (c catalog) getVersions(_ context.Context, mod string) (entry, error) {
+	e, found := c.dependencies[mod]
+	if !found {
+		return entry{}, fmt.Errorf("%w : %s", ErrDependencyNotFound, mod)
+	}
+
+	return e, nil
 }
 
 // NewCatalogFromJSON creates a Catalog from a json file
-func NewCatalogFromJSON(catalogFile string) (Catalog, error) {
-	registry, err := loadRegistryFromJSON(catalogFile)
+func NewCatalogFromJSON(stream io.Reader) (Catalog, error) {
+	buff := &bytes.Buffer{}
+	_, err := buff.ReadFrom(stream)
+	if err != nil {
+		return nil, fmt.Errorf("%w: %w", ErrInvalidCatalog, err)
+	}
+
+	dependencies := map[string]entry{}
+	err = json.Unmarshal(buff.Bytes(), &dependencies)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%w: %w", ErrInvalidCatalog, err)
 	}
-	return catalog{registry: registry}, nil
+
+	return catalog{
+		dependencies: dependencies,
+	}, nil
+}
+
+// NewCatalogFromFile creates a Catalog from a json file
+func NewCatalogFromFile(catalogFile string) (Catalog, error) {
+	json, err := os.ReadFile(catalogFile) //nolint:forbidigo,gosec
+	if err != nil {
+		return nil, fmt.Errorf("%w: %w", ErrOpening, err)
+	}
+
+	buff := bytes.NewBuffer(json)
+	return NewCatalogFromJSON(buff)
 }
 
 // NewCatalogFromURL creates a Catalog from a URL
@@ -80,24 +117,7 @@ func NewCatalogFromURL(ctx context.Context, catalogURL string) (Catalog, error)
 		return nil, fmt.Errorf("%w %s", ErrDownload, resp.Status)
 	}
 
-	catalogFile, err := os.CreateTemp("", "catalog*.json") //nolint:forbidigo
-	if err != nil {
-		return nil, fmt.Errorf("%w %w", ErrDownload, err)
-	}
-
-	_, err = io.Copy(catalogFile, resp.Body)
-	if err != nil {
-		_ = catalogFile.Close()
-		_ = os.Remove(catalogFile.Name()) //nolint:forbidigo
-		return nil, fmt.Errorf("%w %w", ErrDownload, err)
-	}
-
-	err = catalogFile.Close()
-	if err != nil {
-		return nil, fmt.Errorf("%w %w", ErrDownload, err)
-	}
-
-	catalog, err := NewCatalogFromJSON(catalogFile.Name())
+	catalog, err := NewCatalogFromJSON(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("%w %w", ErrDownload, err)
 	}
@@ -107,11 +127,11 @@ func NewCatalogFromURL(ctx context.Context, catalogURL string) (Catalog, error)
 
 // DefaultCatalog creates a Catalog from the default json file 'catalog.json'
 func DefaultCatalog() (Catalog, error) {
-	return NewCatalogFromJSON(defaultCatalogFile)
+	return NewCatalogFromFile(defaultCatalogFile)
 }
 
 func (c catalog) Resolve(ctx context.Context, dep Dependency) (Module, error) {
-	entry, err := c.registry.GetVersions(ctx, dep.Name)
+	entry, err := c.getVersions(ctx, dep.Name)
 	if err != nil {
 		return Module{}, err
 	}

catalog_test.go

@@ -1,26 +1,20 @@
 package k6catalog
 
 import (
+	"bytes"
 	"context"
 	"errors"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"testing"
 )
 
-type fakeRegistry struct {
-	entries map[string]Entry
-}
-
-func (r fakeRegistry) GetVersions(_ context.Context, dep string) (Entry, error) {
-	e, found := r.entries[dep]
-	if !found {
-		return Entry{}, fmt.Errorf("entry not found : %s", dep)
-	}
-
-	return Entry{Module: e.Module, Versions: e.Versions}, nil
-}
+const (
+	testCatalog    = `{ "dep": {"Module": "github.com/dep", "Versions": ["v0.1.0", "v0.2.0"]}}`
+	invalidCatalog = `{ "dep": {}}`
+)
 
 func TestResolve(t *testing.T) {
 	t.Parallel()
@@ -53,13 +47,11 @@ func TestResolve(t *testing.T) {
 		},
 	}
 
-	registry := fakeRegistry{
-		entries: map[string]Entry{
-			"dep": {Module: "github.com/dep", Versions: []string{"v0.1.0", "v0.2.0"}},
-		},
+	json := bytes.NewBuffer([]byte(testCatalog))
+	catalog, err := NewCatalogFromJSON(json)
+	if err != nil {
+		t.Fatalf("test setup %v", err)
 	}
-
-	catalog := NewCatalog(registry)
 	for _, tc := range testCases {
 		tc := tc
 
@@ -78,9 +70,38 @@ func TestResolve(t *testing.T) {
 	}
 }
 
-const testCatalog = `{
-	"k6/x/output-kafka": {"Module": "github.com/grafana/xk6-output-kafka", "Versions": ["v0.1.0", "v0.2.0"]}
-}`
+func TestCatalogFromJSON(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name      string
+		json      string
+		expectErr error
+	}{
+		{
+			name:      "load json",
+			json:      testCatalog,
+			expectErr: nil,
+		},
+		{
+			name:      "empty json",
+			json:      "",
+			expectErr: ErrInvalidCatalog,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			json := bytes.NewBuffer([]byte(tc.json))
+			_, err := NewCatalogFromJSON(json)
+			if !errors.Is(err, tc.expectErr) {
+				t.Fatalf("expected %v got %v", tc.expectErr, err)
+			}
+		})
+	}
+}
 
 func TestCatalogFromURL(t *testing.T) {
 	t.Parallel()
@@ -104,13 +125,6 @@ func TestCatalogFromURL(t *testing.T) {
 			},
 			expectErr: ErrDownload,
 		},
-		{
-			name: "empty catalog",
-			handler: func(w http.ResponseWriter, _ *http.Request) {
-				w.WriteHeader(http.StatusOK)
-			},
-			expectErr: ErrInvalidRegistry,
-		},
 	}
 
 	for _, tc := range testCases {
@@ -127,3 +141,42 @@ func TestCatalogFromURL(t *testing.T) {
 		})
 	}
 }
+
+func TestCatalogFromFile(t *testing.T) {
+	t.Parallel()
+
+	catalogFile := filepath.Join(t.TempDir(), "catalog.json")
+	err := os.WriteFile(catalogFile, []byte(testCatalog), 0o644)
+	if err != nil {
+		t.Fatalf("test setup: %v", err)
+	}
+
+	testCases := []struct {
+		name      string
+		file      string
+		expectErr error
+	}{
+		{
+			name:      "open catalog",
+			file:      catalogFile,
+			expectErr: nil,
+		},
+		{
+			name:      "catalog not found",
+			file:      "/path/not/found",
+			expectErr: ErrOpening,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := NewCatalogFromFile(tc.file)
+
+			if !errors.Is(err, tc.expectErr) {
+				t.Fatalf("expected %v got %v", tc.expectErr, err)
+			}
+		})
+	}
+}

cmd/cmd.go

@@ -39,7 +39,7 @@ func New() *cobra.Command {
 				return fmt.Errorf("path to registry must be specified")
 			}
 
-			catalog, err := k6catalog.NewCatalogFromJSON(path)
+			catalog, err := k6catalog.NewCatalogFromFile(path)
 			if err != nil {
 				return err
 			}