From dcf90064e3671d4eccc5f0f59182a4afe23c95f9 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 28 May 2026 10:02:03 +0300 Subject: refactor(forge): unify repo lifecycle across GitHub and Codeberg (dq) --- internal/cli/forge_client.go | 21 ++++++++ internal/cli/forge_client_test.go | 52 +++++++++++++++++++ internal/cli/handlers.go | 26 ++-------- internal/codeberg/codeberg.go | 33 +++--------- internal/forge/repo_ops.go | 51 ++++++++++++++++++ internal/forge/repo_ops_test.go | 105 ++++++++++++++++++++++++++++++++++++++ internal/github/github.go | 33 +++--------- 7 files changed, 250 insertions(+), 71 deletions(-) create mode 100644 internal/cli/forge_client.go create mode 100644 internal/cli/forge_client_test.go create mode 100644 internal/forge/repo_ops.go create mode 100644 internal/forge/repo_ops_test.go diff --git a/internal/cli/forge_client.go b/internal/cli/forge_client.go new file mode 100644 index 0000000..4d8eb60 --- /dev/null +++ b/internal/cli/forge_client.go @@ -0,0 +1,21 @@ +package cli + +import ( + "codeberg.org/snonux/gitsyncer/internal/codeberg" + "codeberg.org/snonux/gitsyncer/internal/config" + "codeberg.org/snonux/gitsyncer/internal/forge" + "codeberg.org/snonux/gitsyncer/internal/github" +) + +func newRepoClientForOrg(org config.Organization) (forge.RepoClient, bool) { + switch { + case org.IsGitHub(): + client := github.NewClient(org.GitHubToken, org.Name) + return &client, true + case org.IsCodeberg(): + client := codeberg.NewClient(org.Name, org.CodebergToken) + return &client, true + default: + return nil, false + } +} diff --git a/internal/cli/forge_client_test.go b/internal/cli/forge_client_test.go new file mode 100644 index 0000000..84f819b --- /dev/null +++ b/internal/cli/forge_client_test.go @@ -0,0 +1,52 @@ +package cli + +import ( + "testing" + + "codeberg.org/snonux/gitsyncer/internal/config" +) + +func TestNewRepoClientForOrg(t *testing.T) { + t.Parallel() + + t.Run("github", func(t *testing.T) { + client, ok := newRepoClientForOrg(config.Organization{ + Host: "git@github.com", + Name: "acme", + GitHubToken: "token", + }) + if !ok { + t.Fatal("expected supported github client") + } + if !client.HasToken() { + t.Fatal("expected github client token to be loaded") + } + }) + + t.Run("codeberg", func(t *testing.T) { + client, ok := newRepoClientForOrg(config.Organization{ + Host: "git@codeberg.org", + Name: "acme", + CodebergToken: "token", + }) + if !ok { + t.Fatal("expected supported codeberg client") + } + if !client.HasToken() { + t.Fatal("expected codeberg client token to be loaded") + } + }) + + t.Run("unsupported", func(t *testing.T) { + client, ok := newRepoClientForOrg(config.Organization{ + Host: "ssh://example.org", + Name: "acme", + }) + if ok { + t.Fatal("expected unsupported host") + } + if client != nil { + t.Fatal("expected nil client for unsupported host") + } + }) +} diff --git a/internal/cli/handlers.go b/internal/cli/handlers.go index aa43a86..242bc53 100644 --- a/internal/cli/handlers.go +++ b/internal/cli/handlers.go @@ -7,7 +7,6 @@ import ( "path/filepath" "strings" - "codeberg.org/snonux/gitsyncer/internal/codeberg" "codeberg.org/snonux/gitsyncer/internal/config" "codeberg.org/snonux/gitsyncer/internal/github" "codeberg.org/snonux/gitsyncer/internal/version" @@ -175,20 +174,12 @@ func HandleDeleteRepo(cfg *config.Config, repoName string) int { } for _, org := range cfg.Organizations { - var exists bool - var err error - - switch org.Host { - case "git@github.com": - client := github.NewClient(org.GitHubToken, org.Name) - exists, err = client.RepoExists(repoName) - case "git@codeberg.org": - client := codeberg.NewClient(org.Name, org.CodebergToken) - exists, err = client.RepoExists(repoName) - default: + client, supported := newRepoClientForOrg(org) + if !supported { fmt.Printf("Skipping unsupported host: %s\n", org.Host) continue } + exists, err := client.RepoExists(repoName) orgsWithRepo = append(orgsWithRepo, struct { org config.Organization @@ -240,15 +231,8 @@ func HandleDeleteRepo(cfg *config.Config, repoName string) int { fmt.Printf(" Deleting from %s... ", info.org.GetGitURL()) - var deleteErr error - switch info.org.Host { - case "git@github.com": - client := github.NewClient(info.org.GitHubToken, info.org.Name) - deleteErr = client.DeleteRepo(repoName) - case "git@codeberg.org": - client := codeberg.NewClient(info.org.Name, info.org.CodebergToken) - deleteErr = client.DeleteRepo(repoName) - } + client, _ := newRepoClientForOrg(info.org) + deleteErr := client.DeleteRepo(repoName) if deleteErr != nil { fmt.Printf("FAILED: %v\n", deleteErr) diff --git a/internal/codeberg/codeberg.go b/internal/codeberg/codeberg.go index 9ecb3d8..4b5e552 100644 --- a/internal/codeberg/codeberg.go +++ b/internal/codeberg/codeberg.go @@ -10,6 +10,7 @@ import ( "path/filepath" "time" + "codeberg.org/snonux/gitsyncer/internal/forge" "codeberg.org/snonux/gitsyncer/internal/httpclient" ) @@ -37,6 +38,8 @@ type Client struct { token string } +var _ forge.RepoClient = (*Client)(nil) + // NewClient creates a new Codeberg API client func NewClient(org, token string) Client { c := Client{ @@ -266,9 +269,9 @@ func (c *Client) RepoExists(repoName string) (bool, error) { // CreateRepo creates a new repository on Codeberg func (c *Client) CreateRepo(repoName, description string, private bool) error { - exists, err := c.RepoExists(repoName) + exists, err := forge.CheckRepoExists(repoName, c.RepoExists) if err != nil { - return fmt.Errorf("failed to check if repo exists: %w", err) + return err } if exists { return nil // Repository already exists @@ -333,14 +336,8 @@ func (c *Client) DeleteRepo(repoName string) error { return fmt.Errorf("Codeberg token required to delete repository") } - // First check if the repo exists - exists, err := c.RepoExists(repoName) - if err != nil { - return fmt.Errorf("failed to check if repo exists: %w", err) - } - if !exists { - // Repo doesn't exist, nothing to delete - return fmt.Errorf("repository %s/%s does not exist", c.org, repoName) + if err := forge.EnsureRepoExists(c.org, repoName, c.RepoExists); err != nil { + return err } url := fmt.Sprintf("%s/repos/%s/%s", c.baseURL, c.org, repoName) @@ -359,20 +356,6 @@ func (c *Client) DeleteRepo(repoName string) error { } defer resp.Body.Close() - if resp.StatusCode == 204 { - // Successfully deleted - return nil - } else if resp.StatusCode == 404 { - // Already gone, consider it a success - return nil - } else if resp.StatusCode == 403 { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("permission denied (403): %s", string(body)) - } else if resp.StatusCode == 401 { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("authentication failed (401): %s", string(body)) - } - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to delete repository: status %d: %s", resp.StatusCode, string(body)) + return forge.DeleteStatusError(resp.StatusCode, string(body)) } diff --git a/internal/forge/repo_ops.go b/internal/forge/repo_ops.go new file mode 100644 index 0000000..8334a16 --- /dev/null +++ b/internal/forge/repo_ops.go @@ -0,0 +1,51 @@ +package forge + +import "fmt" + +// RepoClient defines shared repository lifecycle operations across forges. +type RepoClient interface { + HasToken() bool + RepoExists(repoName string) (bool, error) + CreateRepo(repoName, description string, private bool) error + DeleteRepo(repoName string) error +} + +// RepoExistsFunc checks if a repository exists. +type RepoExistsFunc func(repoName string) (bool, error) + +// CheckRepoExists normalizes repo-existence check error handling. +func CheckRepoExists(repoName string, existsFn RepoExistsFunc) (bool, error) { + exists, err := existsFn(repoName) + if err != nil { + return false, fmt.Errorf("failed to check if repo exists: %w", err) + } + + return exists, nil +} + +// EnsureRepoExists validates that a repository exists before destructive actions. +func EnsureRepoExists(org, repoName string, existsFn RepoExistsFunc) error { + exists, err := CheckRepoExists(repoName, existsFn) + if err != nil { + return err + } + if !exists { + return fmt.Errorf("repository %s/%s does not exist", org, repoName) + } + + return nil +} + +// DeleteStatusError maps common forge delete status codes into stable errors. +func DeleteStatusError(statusCode int, body string) error { + switch statusCode { + case 204, 404: + return nil + case 403: + return fmt.Errorf("permission denied (403): %s", body) + case 401: + return fmt.Errorf("authentication failed (401): %s", body) + default: + return fmt.Errorf("failed to delete repository: status %d: %s", statusCode, body) + } +} diff --git a/internal/forge/repo_ops_test.go b/internal/forge/repo_ops_test.go new file mode 100644 index 0000000..8a269d5 --- /dev/null +++ b/internal/forge/repo_ops_test.go @@ -0,0 +1,105 @@ +package forge + +import ( + "errors" + "strings" + "testing" +) + +func TestCheckRepoExists(t *testing.T) { + t.Run("exists", func(t *testing.T) { + exists, err := CheckRepoExists("repo", func(string) (bool, error) { + return true, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !exists { + t.Fatal("expected repo to exist") + } + }) + + t.Run("missing", func(t *testing.T) { + exists, err := CheckRepoExists("repo", func(string) (bool, error) { + return false, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if exists { + t.Fatal("expected repo to be missing") + } + }) + + t.Run("wraps error", func(t *testing.T) { + original := errors.New("boom") + _, err := CheckRepoExists("repo", func(string) (bool, error) { + return false, original + }) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "failed to check if repo exists") { + t.Fatalf("expected wrapped message, got %q", err.Error()) + } + if !errors.Is(err, original) { + t.Fatal("expected wrapped original error") + } + }) +} + +func TestEnsureRepoExists(t *testing.T) { + t.Run("exists", func(t *testing.T) { + err := EnsureRepoExists("org", "repo", func(string) (bool, error) { + return true, nil + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("missing", func(t *testing.T) { + err := EnsureRepoExists("org", "repo", func(string) (bool, error) { + return false, nil + }) + if err == nil { + t.Fatal("expected an error") + } + if got, want := err.Error(), "repository org/repo does not exist"; got != want { + t.Fatalf("expected %q, got %q", want, got) + } + }) +} + +func TestDeleteStatusError(t *testing.T) { + tests := []struct { + name string + status int + body string + errMsg string + }{ + {name: "deleted", status: 204, body: ""}, + {name: "already gone", status: 404, body: ""}, + {name: "forbidden", status: 403, body: "nope", errMsg: "permission denied (403): nope"}, + {name: "unauthorized", status: 401, body: "bad creds", errMsg: "authentication failed (401): bad creds"}, + {name: "unexpected", status: 500, body: "oops", errMsg: "failed to delete repository: status 500: oops"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := DeleteStatusError(tc.status, tc.body) + if tc.errMsg == "" { + if err != nil { + t.Fatalf("expected nil, got %v", err) + } + return + } + if err == nil { + t.Fatal("expected an error") + } + if got := err.Error(); got != tc.errMsg { + t.Fatalf("expected %q, got %q", tc.errMsg, got) + } + }) + } +} diff --git a/internal/github/github.go b/internal/github/github.go index 2667658..a8909a8 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" + "codeberg.org/snonux/gitsyncer/internal/forge" "codeberg.org/snonux/gitsyncer/internal/httpclient" ) @@ -19,6 +20,8 @@ type Client struct { org string } +var _ forge.RepoClient = (*Client)(nil) + // NewClient creates a new GitHub API client func NewClient(token, org string) Client { return Client{ @@ -124,9 +127,9 @@ func (c *Client) CreateRepo(repoName, description string, private bool) error { fmt.Printf(" Checking if GitHub repo %s/%s exists...\n", c.org, repoName) // First check if it already exists - exists, err := c.RepoExists(repoName) + exists, err := forge.CheckRepoExists(repoName, c.RepoExists) if err != nil { - return fmt.Errorf("failed to check if repo exists: %w", err) + return err } if exists { fmt.Printf(" GitHub repo already exists, skipping creation\n") @@ -356,14 +359,8 @@ func (c *Client) DeleteRepo(repoName string) error { return fmt.Errorf("GitHub token required to delete repository") } - // First check if the repo exists - exists, err := c.RepoExists(repoName) - if err != nil { - return fmt.Errorf("failed to check if repo exists: %w", err) - } - if !exists { - // Repo doesn't exist, nothing to delete - return fmt.Errorf("repository %s/%s does not exist", c.org, repoName) + if err := forge.EnsureRepoExists(c.org, repoName, c.RepoExists); err != nil { + return err } url := fmt.Sprintf("https://api.github.com/repos/%s/%s", c.org, repoName) @@ -383,20 +380,6 @@ func (c *Client) DeleteRepo(repoName string) error { } defer resp.Body.Close() - if resp.StatusCode == 204 { - // Successfully deleted - return nil - } else if resp.StatusCode == 404 { - // Already gone, consider it a success - return nil - } else if resp.StatusCode == 403 { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("permission denied (403): %s", string(body)) - } else if resp.StatusCode == 401 { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("authentication failed (401): %s", string(body)) - } - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to delete repository: status %d: %s", resp.StatusCode, string(body)) + return forge.DeleteStatusError(resp.StatusCode, string(body)) } -- cgit v1.2.3