refactor: split fetcher by type, reorganize fetcher tests

cli/build.go

@@ -13,14 +13,22 @@ func getBuildCommand() *cobra.Command {
 		Use:   "build",
 		Short: "Build service images.",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			pathProvided, err := cmd.PersistentFlags().GetString("definition")
+			var pathProvided string
+			var fetcher service_definition.DefinitionFetcher
+			var def service_definition.ServiceDefinition
+			var err error
 
-			if err != nil {
+			if pathProvided, err = cmd.PersistentFlags().GetString("definition"); err != nil {
 				return fmt.Errorf("%+v", err)
 			}
-			def, err := service_definition.NewDefinitionFetcher().GetDefinition(pathProvided)
 
-			if err != nil {
+			defPathType := service_definition.GetPathType(pathProvided)
+
+			if fetcher, err = service_definition.NewDefinitionFetcher(defPathType); err != nil {
+				return fmt.Errorf("Couldn't set up fetcher from the path provided.")
+			}
+
+			if def, err = fetcher.GetDefinition(pathProvided); err != nil {
 				return fmt.Errorf("Failed to read service definition from file: %+v", err)
 			}
 

cli/start_service.go

@@ -47,11 +47,21 @@ func getStartCommand() *cobra.Command {
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
+			var fetcher service_definition.DefinitionFetcher
+			var def service_definition.ServiceDefinition
+			var err error
+
 			ctx := cmd.Context()
 			flags := ctx.Value("flags").(ParsedFlags)
-			def, err := service_definition.NewDefinitionFetcher().GetDefinition(flags.definitionPath)
 
-			if err != nil {
+			defPath := flags.definitionPath
+			defPathType := service_definition.GetPathType(defPath)
+
+			if fetcher, err = service_definition.NewDefinitionFetcher(defPathType); err != nil {
+				return fmt.Errorf("Couldn't set up fetcher from the path provided.")
+			}
+
+			if def, err = fetcher.GetDefinition(defPath); err != nil {
 				return fmt.Errorf("Failed to read service definition: %+v", err)
 			}
 

service_definition/fetcher.go

@@ -6,64 +6,21 @@
 package service_definition
 
 import (
-	"github.com/goccy/go-yaml"
+	"fmt"
-	"os"
-	git "spud/git"
-	"strings"
 )
 
-type DefinitionFetcher struct {
+type DefinitionFetcher interface {
-	Git git.GitClient
+	GetDefinition(path string) (ServiceDefinition, error)
 }
 
-func NewDefinitionFetcher() DefinitionFetcher {
+func NewDefinitionFetcher(fetcher_type string) (DefinitionFetcher, error) {
-	return DefinitionFetcher{
+	if fetcher_type == "git" {
-		Git: git.Git{},
+		return NewGitDefinitionFetcher(), nil
 	}
-}
+
-
+	if fetcher_type == "file" {
-// Gets a ServiceDefinition from the given location.
+		return NewFileDefinitionFetcher(), nil
-//
+	}
-// Depending on location prefix, different sources are used:
+
-// git+: Clones the target git repository and extracts a service definition from it.
+	return nil, fmt.Errorf("Unrecognized fetcher type: %s", fetcher_type)
-// <no-prefix>: Uses the location as a filepath to the service definition.
-func (f DefinitionFetcher) GetDefinition(path string) (ServiceDefinition, error) {
-	if strings.HasPrefix(path, "git+") {
-		return f.getDefinitionFromGit(path)
-	}
-
-	return f.getDefinitionFromFile(path)
-}
-
-// Clones the target git repository and uses it as a basis to extract
-// a service definition.
-func (f DefinitionFetcher) getDefinitionFromGit(path string) (ServiceDefinition, error) {
-	dir, err := os.MkdirTemp("/tmp", "spud-service-")
-
-	if err != nil {
-		return ServiceDefinition{}, err
-	}
-
-	if _, err := f.Git.Clone(strings.TrimPrefix(path, "git+"), dir); err != nil {
-		return ServiceDefinition{}, err
-	}
-
-	return f.getDefinitionFromFile(dir + "/service.yml")
-}
-
-// Extracts a service definition from the given filepath.
-func (f DefinitionFetcher) getDefinitionFromFile(path string) (ServiceDefinition, error) {
-	var definition ServiceDefinition
-
-	defData, err := os.ReadFile(path)
-
-	if err != nil {
-		return ServiceDefinition{}, &FileDoesNotExistError{Message: "Could not find service configuration file", ExpectedPath: path}
-	}
-
-	if err = yaml.Unmarshal(defData, &definition); err != nil {
-		return ServiceDefinition{}, &InvalidServiceDefinitionError{Path: path}
-	}
-
-	return definition, nil
 }

service_definition/fetcher_test.go

@@ -1,58 +1,19 @@
 package service_definition
 
 import (
-	"os"
-	"strings"
 	"testing"
 )
 
-type MockGit struct {
+func TestGetPathTypeDetectsGitPathPrefix(t *testing.T) {
-	calls []string
+	actual := GetPathType("git+https://test.com")
-}
+	if actual != "git" {
-
+		t.Errorf("Expected 'git' type, got %s", actual)
-func (g *MockGit) Clone(path string, destination string) (string, error) {
-	g.calls = append(g.calls, path+":"+destination)
-
-	return path, nil
-}
-
-func TestGetDefinitionDetectsGitPathPrefix(t *testing.T) {
-	fetcher := NewDefinitionFetcher()
-
-	mockGit := MockGit{}
-	fetcher.Git = &mockGit
-
-	fetcher.GetDefinition("git+https://git.com/owner/repo.git")
-
-	if len(mockGit.calls) == 0 {
-		t.Errorf("Expected at least one call to git, got none.")
 	}
 }
 
-func TestGetDefinitionDefaultsToFilePathIfNoPrefix(t *testing.T) {
+func TestGetPathTypeDetectsFileNoPrefix(t *testing.T) {
-	defPath := t.TempDir() + "/service.yml"
+	actual := GetPathType("file/path.yml")
-
+	if actual != "file" {
-	os.WriteFile(defPath, []byte("name: test-service"), 0755)
+		t.Errorf("Expected 'file' type, got %s", actual)
-
-	fetcher := NewDefinitionFetcher()
-
-	def, _ := fetcher.GetDefinition(defPath)
-
-	if def.Name != "test-service" {
-		t.Errorf("Expected mock service name to be 'test-service', got %s instead.", def.Name)
-	}
-}
-
-func TestGetDefinitionGetDefinitionFromGit(t *testing.T) {
-	fetcher := NewDefinitionFetcher()
-
-	mockGit := MockGit{}
-	fetcher.Git = &mockGit
-
-	mockUrl := "https://git.com/owner/repo.git"
-	fetcher.GetDefinition("git+" + mockUrl)
-
-	if !strings.HasPrefix(mockGit.calls[0], mockUrl) {
-		t.Errorf("Expected git cloning for %s, got %s instead.", mockUrl, mockGit.calls[0])
 	}
 }

service_definition/file_definition_fetcher.go

@@ -0,0 +1,33 @@
+// File Definition fetcher
+//
+// Handles extracting service definitions from local files.
+
+package service_definition
+
+import (
+	"github.com/goccy/go-yaml"
+	"os"
+)
+
+type FileDefinitionFetcher struct{}
+
+func NewFileDefinitionFetcher() *FileDefinitionFetcher {
+	return &FileDefinitionFetcher{}
+}
+
+// Retrieves a service definition from the given filepath.
+func (f FileDefinitionFetcher) GetDefinition(path string) (ServiceDefinition, error) {
+	var definition ServiceDefinition
+
+	defData, err := os.ReadFile(path)
+
+	if err != nil {
+		return ServiceDefinition{}, &FileDoesNotExistError{Message: "Could not find service configuration file", ExpectedPath: path}
+	}
+
+	if err = yaml.Unmarshal(defData, &definition); err != nil {
+		return ServiceDefinition{}, &InvalidServiceDefinitionError{Path: path}
+	}
+
+	return definition, nil
+}

service_definition/git_fetcher.go

@@ -0,0 +1,38 @@
+// Git Definition fetcher
+//
+// Handles fetching and building ServiceDefinition structs from git
+// repositories.
+
+package service_definition
+
+import (
+	"os"
+	git "spud/git"
+	"strings"
+)
+
+type GitDefinitionFetcher struct {
+	Git git.GitClient
+}
+
+func NewGitDefinitionFetcher() *GitDefinitionFetcher {
+	return &GitDefinitionFetcher{
+		Git: git.Git{},
+	}
+}
+
+// Clones the target git repository and uses it as a basis to extract
+// a service definition.
+func (f GitDefinitionFetcher) GetDefinition(path string) (ServiceDefinition, error) {
+	dir, err := os.MkdirTemp("/tmp", "spud-service-")
+
+	if err != nil {
+		return ServiceDefinition{}, err
+	}
+
+	if _, err := f.Git.Clone(strings.TrimPrefix(path, "git+"), dir); err != nil {
+		return ServiceDefinition{}, err
+	}
+
+	return NewFileDefinitionFetcher().GetDefinition((dir + "/service.yml"))
+}

service_definition/git_fetcher_test.go

@@ -0,0 +1,34 @@
+package service_definition
+
+import (
+	"strings"
+	"testing"
+)
+
+type MockGit struct {
+	calls []string
+}
+
+func (g *MockGit) Clone(path string, destination string) (string, error) {
+	g.calls = append(g.calls, path+":"+destination)
+
+	return path, nil
+}
+
+func TestGetDefinitionGetDefinitionFromGit(t *testing.T) {
+	var gitFetcher GitDefinitionFetcher
+
+	fetcher, _ := NewDefinitionFetcher("git")
+
+	gitFetcher, _ = fetcher.(GitDefinitionFetcher)
+
+	mockGit := MockGit{}
+	gitFetcher.Git = &mockGit
+
+	mockUrl := "https://git.com/owner/repo.git"
+	gitFetcher.GetDefinition("git+" + mockUrl)
+
+	if !strings.HasPrefix(mockGit.calls[0], mockUrl) {
+		t.Errorf("Expected git cloning for %s, got %s instead.", mockUrl, mockGit.calls[0])
+	}
+}

service_definition/paths.go

@@ -0,0 +1,13 @@
+package service_definition
+
+import (
+	"strings"
+)
+
+func GetPathType(path string) string {
+	if strings.HasPrefix(path, "git+") {
+		return "git"
+	}
+
+	return "file"
+}

service_definition/service_definition_test.go

@@ -5,7 +5,8 @@ import (
 )
 
 func TestGetServiceDefinitionFromFileDoesNotExist(t *testing.T) {
-	_, err := NewDefinitionFetcher().GetDefinition(t.TempDir() + "/not-a-file.yml")
+	fetcher, _ := NewDefinitionFetcher("file")
+	_, err := fetcher.GetDefinition(t.TempDir() + "/not-a-file.yml")
 
 	if err == nil {
 		t.Errorf("Expected error, got nil.")