From 7cc11af3787173e3878ac0e2c42265b99d511f86 Mon Sep 17 00:00:00 2001 From: Marc Cataford Date: Wed, 13 Nov 2024 23:56:19 -0500 Subject: [PATCH] refactor: split fetcher by type, reorganize fetcher tests --- cli/build.go | 16 +++-- cli/start_service.go | 14 +++- service_definition/fetcher.go | 67 ++++--------------- service_definition/fetcher_test.go | 55 +++------------ service_definition/file_definition_fetcher.go | 33 +++++++++ service_definition/git_fetcher.go | 38 +++++++++++ service_definition/git_fetcher_test.go | 34 ++++++++++ service_definition/paths.go | 13 ++++ service_definition/service_definition_test.go | 3 +- 9 files changed, 164 insertions(+), 109 deletions(-) create mode 100644 service_definition/file_definition_fetcher.go create mode 100644 service_definition/git_fetcher.go create mode 100644 service_definition/git_fetcher_test.go create mode 100644 service_definition/paths.go diff --git a/cli/build.go b/cli/build.go index bccbecb..34a19f2 100644 --- a/cli/build.go +++ b/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) } diff --git a/cli/start_service.go b/cli/start_service.go index 4bd12a8..b608b70 100644 --- a/cli/start_service.go +++ b/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) } diff --git a/service_definition/fetcher.go b/service_definition/fetcher.go index 461a5cf..db2269f 100644 --- a/service_definition/fetcher.go +++ b/service_definition/fetcher.go @@ -6,64 +6,21 @@ package service_definition import ( - "github.com/goccy/go-yaml" - "os" - git "spud/git" - "strings" + "fmt" ) -type DefinitionFetcher struct { - Git git.GitClient +type DefinitionFetcher interface { + GetDefinition(path string) (ServiceDefinition, error) } -func NewDefinitionFetcher() DefinitionFetcher { - return DefinitionFetcher{ - Git: git.Git{}, +func NewDefinitionFetcher(fetcher_type string) (DefinitionFetcher, error) { + if fetcher_type == "git" { + return NewGitDefinitionFetcher(), nil } -} - -// Gets a ServiceDefinition from the given location. -// -// Depending on location prefix, different sources are used: -// git+: Clones the target git repository and extracts a service definition from it. -// : 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 + + if fetcher_type == "file" { + return NewFileDefinitionFetcher(), nil + } + + return nil, fmt.Errorf("Unrecognized fetcher type: %s", fetcher_type) } diff --git a/service_definition/fetcher_test.go b/service_definition/fetcher_test.go index 68eef08..2848c58 100644 --- a/service_definition/fetcher_test.go +++ b/service_definition/fetcher_test.go @@ -1,58 +1,19 @@ package service_definition import ( - "os" - "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 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 TestGetPathTypeDetectsGitPathPrefix(t *testing.T) { + actual := GetPathType("git+https://test.com") + if actual != "git" { + t.Errorf("Expected 'git' type, got %s", actual) } } -func TestGetDefinitionDefaultsToFilePathIfNoPrefix(t *testing.T) { - defPath := t.TempDir() + "/service.yml" - - os.WriteFile(defPath, []byte("name: test-service"), 0755) - - 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]) +func TestGetPathTypeDetectsFileNoPrefix(t *testing.T) { + actual := GetPathType("file/path.yml") + if actual != "file" { + t.Errorf("Expected 'file' type, got %s", actual) } } diff --git a/service_definition/file_definition_fetcher.go b/service_definition/file_definition_fetcher.go new file mode 100644 index 0000000..00c78ee --- /dev/null +++ b/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 +} diff --git a/service_definition/git_fetcher.go b/service_definition/git_fetcher.go new file mode 100644 index 0000000..23e730a --- /dev/null +++ b/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")) +} diff --git a/service_definition/git_fetcher_test.go b/service_definition/git_fetcher_test.go new file mode 100644 index 0000000..fbf3d89 --- /dev/null +++ b/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]) + } +} diff --git a/service_definition/paths.go b/service_definition/paths.go new file mode 100644 index 0000000..0e14d0c --- /dev/null +++ b/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" +} diff --git a/service_definition/service_definition_test.go b/service_definition/service_definition_test.go index 5cebc88..f74d454 100644 --- a/service_definition/service_definition_test.go +++ b/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.")