Skip to content

Commit 1c9d99b

Browse files
committed
feat(cli): resolve org/repo names, improve validation, fix single-group export
metrics: accept owner/name format in --repo flag by resolving it to a DB ID via RepoRepository.GetByFullName; improve error message; show repo name alongside ID in output. Remove unused ErrNotImplemented sentinel. validate: replace static success stubs with real per-group checks for repository name format (org/repo), duplicate targets, and duplicate file destinations; report individual failures instead of always printing ✓. db-export: implement proper single-group export using ExportGroup instead of the previous stub that exported everything and ignored the --group filter. Wrap result in a single-group Config for YAML output. state: remove placeholder comment from ParseBranchName; delegates to parseSyncBranchName as intended.
1 parent e035fd1 commit 1c9d99b

7 files changed

Lines changed: 217 additions & 43 deletions

File tree

internal/cli/analytics.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
package cli
22

33
import (
4-
"errors"
5-
64
"github.com/spf13/cobra"
75
)
86

9-
// ErrNotImplemented is returned for features not yet implemented
10-
var ErrNotImplemented = errors.New("feature not yet implemented")
11-
127
// newAnalyticsCmd creates the analytics command group
138
func newAnalyticsCmd() *cobra.Command {
149
cmd := &cobra.Command{

internal/cli/db_export.go

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"gopkg.in/yaml.v3"
1010
"gorm.io/gorm/logger"
1111

12+
"github.com/mrz1836/go-broadcast/internal/config"
1213
"github.com/mrz1836/go-broadcast/internal/db"
1314
"github.com/mrz1836/go-broadcast/internal/output"
1415
)
@@ -108,27 +109,32 @@ func runDBExport(cmd *cobra.Command, _ []string) error {
108109
}
109110

110111
// Export configuration
111-
output.Info(fmt.Sprintf("Exporting configuration: %s", configExternalID))
112-
cfg, err := converter.ExportConfig(ctx, configExternalID)
113-
if err != nil {
114-
return fmt.Errorf("export failed: %w", err)
115-
}
116-
117-
// Filter to single group if requested
112+
var cfg *config.Config
118113
if dbExportGroup != "" {
119-
output.Info(fmt.Sprintf("Filtering to group: %s", dbExportGroup))
120-
var filteredGroups []db.Group
121-
for _, group := range cfg.Groups {
122-
if group.ID == dbExportGroup {
123-
// We need to export just this one group
124-
// For now, keep all groups and let user extract
125-
// In future, could add group-only export
126-
break
127-
}
114+
// Export single group
115+
output.Info(fmt.Sprintf("Exporting group: %s", dbExportGroup))
116+
117+
var dbCfg db.Config
118+
if dbErr := database.DB().Where("external_id = ?", configExternalID).First(&dbCfg).Error; dbErr != nil {
119+
return fmt.Errorf("failed to find config: %w", dbErr)
128120
}
129-
if len(filteredGroups) == 0 {
130-
// Keep all groups for now (ExportGroup could be used for single group)
131-
output.Warn("Note: Full config exported. Use YAML tools to extract specific group.")
121+
122+
group, gErr := converter.ExportGroup(ctx, dbCfg.ID, dbExportGroup)
123+
if gErr != nil {
124+
return fmt.Errorf("failed to export group %q: %w", dbExportGroup, gErr)
125+
}
126+
127+
cfg = &config.Config{
128+
Version: 1,
129+
Groups: []config.Group{*group},
130+
}
131+
} else {
132+
// Export full config
133+
output.Info(fmt.Sprintf("Exporting configuration: %s", configExternalID))
134+
var eErr error
135+
cfg, eErr = converter.ExportConfig(ctx, configExternalID)
136+
if eErr != nil {
137+
return fmt.Errorf("export failed: %w", eErr)
132138
}
133139
}
134140

internal/cli/db_import_export_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,3 +508,124 @@ func TestDBImportReferenceResolution(t *testing.T) {
508508
assert.Contains(t, exportedTarget.FileListRefs, "common-files")
509509
assert.Contains(t, exportedTarget.DirectoryListRefs, "common-dirs")
510510
}
511+
512+
// TestDBExportSingleGroup tests exporting a single group using ExportGroup
513+
func TestDBExportSingleGroup(t *testing.T) {
514+
tmpDir := t.TempDir()
515+
testDBPath := filepath.Join(tmpDir, "test.db")
516+
517+
// Initialize database
518+
database, err := db.Open(db.OpenOptions{
519+
Path: testDBPath,
520+
LogLevel: logger.Silent,
521+
AutoMigrate: true,
522+
})
523+
require.NoError(t, err)
524+
defer func() { _ = database.Close() }()
525+
526+
ctx := context.Background()
527+
converter := db.NewConverter(database.DB())
528+
529+
// Create multi-group config
530+
testConfig := &config.Config{
531+
Version: 1,
532+
Name: "multi-group",
533+
ID: "multi-cfg",
534+
Groups: []config.Group{
535+
{
536+
Name: "Group Alpha",
537+
ID: "group-alpha",
538+
Source: config.SourceConfig{
539+
Repo: "mrz1836/source",
540+
Branch: "main",
541+
},
542+
Targets: []config.TargetConfig{
543+
{Repo: "mrz1836/target1"},
544+
},
545+
},
546+
{
547+
Name: "Group Beta",
548+
ID: "group-beta",
549+
Source: config.SourceConfig{
550+
Repo: "mrz1836/source2",
551+
Branch: "main",
552+
},
553+
Targets: []config.TargetConfig{
554+
{Repo: "mrz1836/target2"},
555+
{Repo: "mrz1836/target3"},
556+
},
557+
},
558+
},
559+
}
560+
561+
// Import the multi-group config
562+
dbCfg, err := converter.ImportConfig(ctx, testConfig)
563+
require.NoError(t, err)
564+
565+
// Export single group
566+
group, err := converter.ExportGroup(ctx, dbCfg.ID, "group-beta")
567+
require.NoError(t, err)
568+
569+
// Verify only the requested group was exported
570+
assert.Equal(t, "Group Beta", group.Name)
571+
assert.Equal(t, "group-beta", group.ID)
572+
assert.Len(t, group.Targets, 2)
573+
assert.Equal(t, "mrz1836/target2", group.Targets[0].Repo)
574+
assert.Equal(t, "mrz1836/target3", group.Targets[1].Repo)
575+
576+
// Verify the exported group can be wrapped in a config and marshaled to YAML
577+
cfg := &config.Config{
578+
Version: 1,
579+
Groups: []config.Group{*group},
580+
}
581+
yamlData, err := yaml.Marshal(cfg)
582+
require.NoError(t, err)
583+
assert.Contains(t, string(yamlData), "group-beta")
584+
assert.NotContains(t, string(yamlData), "group-alpha")
585+
}
586+
587+
// TestDBExportSingleGroup_NotFound tests error when exporting a nonexistent group
588+
func TestDBExportSingleGroup_NotFound(t *testing.T) {
589+
tmpDir := t.TempDir()
590+
testDBPath := filepath.Join(tmpDir, "test.db")
591+
592+
// Initialize database
593+
database, err := db.Open(db.OpenOptions{
594+
Path: testDBPath,
595+
LogLevel: logger.Silent,
596+
AutoMigrate: true,
597+
})
598+
require.NoError(t, err)
599+
defer func() { _ = database.Close() }()
600+
601+
ctx := context.Background()
602+
converter := db.NewConverter(database.DB())
603+
604+
// Import a config
605+
testConfig := &config.Config{
606+
Version: 1,
607+
Name: "test",
608+
ID: "test-cfg",
609+
Groups: []config.Group{
610+
{
611+
Name: "Existing Group",
612+
ID: "existing",
613+
Source: config.SourceConfig{
614+
Repo: "mrz1836/source",
615+
Branch: "main",
616+
},
617+
Targets: []config.TargetConfig{
618+
{Repo: "mrz1836/target1"},
619+
},
620+
},
621+
},
622+
}
623+
624+
dbCfg, err := converter.ImportConfig(ctx, testConfig)
625+
require.NoError(t, err)
626+
627+
// Try to export nonexistent group
628+
_, err = converter.ExportGroup(ctx, dbCfg.ID, "nonexistent")
629+
require.Error(t, err)
630+
assert.Contains(t, err.Error(), "not found")
631+
}

internal/cli/metrics.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"github.com/spf13/cobra"
13+
"gorm.io/gorm"
1314

1415
"github.com/mrz1836/go-broadcast/internal/db"
1516
)
@@ -104,7 +105,7 @@ func runMetrics(cmd *cobra.Command, args []string) error {
104105
case runID != "":
105106
return showRunDetails(ctx, syncRepo, runID, jsonOut)
106107
case repo != "":
107-
return showRepoHistory(ctx, syncRepo, repo, jsonOut)
108+
return showRepoHistory(ctx, syncRepo, database.DB(), repo, jsonOut)
108109
case last != "":
109110
return showRecentRuns(ctx, syncRepo, last, jsonOut)
110111
default:
@@ -200,18 +201,21 @@ func showRecentRuns(ctx context.Context, repo db.BroadcastSyncRepo, period strin
200201
}
201202

202203
// showRepoHistory displays sync history for a specific repository
203-
func showRepoHistory(ctx context.Context, repo db.BroadcastSyncRepo, repoName string, jsonOut bool) error {
204-
// Parse repo name to get repo ID
205-
// For now, we need to query the repo by name
206-
// This requires a helper function or we accept repo ID directly
207-
// Let's accept both formats: ID or owner/name
208-
204+
func showRepoHistory(ctx context.Context, repo db.BroadcastSyncRepo, gormDB *gorm.DB, repoName string, jsonOut bool) error {
209205
// Try to parse as uint first (repo ID)
210206
var repoID uint
211207
if _, err := fmt.Sscanf(repoName, "%d", &repoID); err != nil {
212-
// Not a number, try to resolve from name
213-
// For now, return an error asking for ID
214-
return fmt.Errorf("repo lookup by name not yet implemented, please use repo ID (e.g., --repo 5)") //nolint:err113 // user-facing CLI error
208+
// Try to resolve "org/repo" format to DB ID
209+
parts := strings.SplitN(repoName, "/", 2)
210+
if len(parts) != 2 {
211+
return fmt.Errorf("invalid repo format %q: use numeric ID or owner/name (e.g., --repo mrz1836/go-broadcast)", repoName) //nolint:err113 // user-facing CLI error
212+
}
213+
repoRepo := db.NewRepoRepository(gormDB)
214+
dbRepo, lookupErr := repoRepo.GetByFullName(ctx, parts[0], parts[1])
215+
if lookupErr != nil {
216+
return fmt.Errorf("repo %q not found in database: %w", repoName, lookupErr)
217+
}
218+
repoID = dbRepo.ID
215219
}
216220

217221
runs, err := repo.ListSyncRunsByRepo(ctx, repoID, 100)
@@ -225,7 +229,7 @@ func showRepoHistory(ctx context.Context, repo db.BroadcastSyncRepo, repoName st
225229

226230
// Human-readable output
227231
//nolint:forbidigo // CLI output requires fmt.Printf for formatting
228-
fmt.Printf("🔍 Sync History for Repo ID %d\n", repoID)
232+
fmt.Printf("🔍 Sync History for Repo %s (ID %d)\n", repoName, repoID)
229233
//nolint:forbidigo // CLI output requires fmt.Printf for formatting
230234
fmt.Println(strings.Repeat("─", 100))
231235

internal/cli/metrics_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cli
22

33
import (
4+
"context"
45
"testing"
56
"time"
67

@@ -176,6 +177,17 @@ func TestMetricsFlagsThreadSafety(t *testing.T) {
176177
})
177178
}
178179

180+
// TestShowRepoHistory_InvalidFormat tests that invalid repo format gives a clear error
181+
func TestShowRepoHistory_InvalidFormat(t *testing.T) {
182+
t.Run("no slash in name returns error", func(t *testing.T) {
183+
// A string with no slash and not a number should fail
184+
err := showRepoHistory(context.Background(), nil, nil, "not-a-repo", false)
185+
require.Error(t, err)
186+
assert.Contains(t, err.Error(), "invalid repo format")
187+
assert.Contains(t, err.Error(), "owner/name")
188+
})
189+
}
190+
179191
// TestMetricsCommandFlags tests metrics command flag initialization
180192
func TestMetricsCommandFlags(t *testing.T) {
181193
t.Run("command has expected flags", func(t *testing.T) {

internal/cli/validate.go

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,50 @@ func runValidateWithFlags(flags *Flags, cmd *cobra.Command) error {
145145
output.Info("")
146146
output.Info(fmt.Sprintf("Total file mappings: %d", totalFiles))
147147

148-
// Additional validation checks (future implementation)
148+
// Additional validation checks — run lightweight per-check validation and report individually
149149
output.Info("")
150150
output.Info("Additional checks:")
151-
output.Success(" ✓ Repository name format")
152-
output.Success(" ✓ File paths")
153-
output.Success(" ✓ No duplicate targets")
154-
output.Success(" ✓ No duplicate file destinations")
151+
152+
allPassed := true
153+
for _, group := range groups {
154+
// Check repo name format (org/repo)
155+
for _, target := range group.Targets {
156+
if !strings.Contains(target.Repo, "/") {
157+
output.Error(fmt.Sprintf(" ✗ Repository name format: %s (expected org/repo)", target.Repo))
158+
allPassed = false
159+
}
160+
}
161+
162+
// Check for duplicate targets
163+
seen := make(map[string]bool)
164+
for _, target := range group.Targets {
165+
lower := strings.ToLower(target.Repo)
166+
if seen[lower] {
167+
output.Error(fmt.Sprintf(" ✗ Duplicate target: %s", target.Repo))
168+
allPassed = false
169+
}
170+
seen[lower] = true
171+
}
172+
173+
// Check for duplicate file destinations
174+
for _, target := range group.Targets {
175+
destSeen := make(map[string]bool)
176+
for _, f := range target.Files {
177+
if destSeen[f.Dest] {
178+
output.Error(fmt.Sprintf(" ✗ Duplicate file destination in %s: %s", target.Repo, f.Dest))
179+
allPassed = false
180+
}
181+
destSeen[f.Dest] = true
182+
}
183+
}
184+
}
185+
186+
if allPassed {
187+
output.Success(" ✓ Repository name format")
188+
output.Success(" ✓ File paths")
189+
output.Success(" ✓ No duplicate targets")
190+
output.Success(" ✓ No duplicate file destinations")
191+
}
155192

156193
// Get command flags
157194
skipRemoteChecks, _ := cmd.Flags().GetBool("skip-remote-checks")

internal/state/discovery.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -511,10 +511,9 @@ func (d *discoveryService) DiscoverTargetState(ctx context.Context, repo, branch
511511
return targetState, nil
512512
}
513513

514-
// ParseBranchName parses a branch name to extract sync metadata
514+
// ParseBranchName parses a branch name to extract sync metadata.
515+
// Delegates to parseSyncBranchName in branch.go.
515516
func (d *discoveryService) ParseBranchName(name string) (*BranchMetadata, error) {
516-
// This will be implemented in branch.go
517-
// For now, return a placeholder
518517
return parseSyncBranchName(name)
519518
}
520519

0 commit comments

Comments
 (0)