diff --git a/internal/app/command_profile.go b/internal/app/command_profile.go index 7bdc6d3..56c218c 100644 --- a/internal/app/command_profile.go +++ b/internal/app/command_profile.go @@ -73,29 +73,38 @@ func runProfileList(stdout io.Writer) error { } func runProfileCreate(cfg cli.Config, stdout io.Writer) error { - if cfg.Provider == "" { + if strings.TrimSpace(cfg.Provider) == "" { return fmt.Errorf("--provider is required. Specify the TTS provider (gcp, aws, azure, ibm, alibaba)") } - if cfg.ProfileName == "" { + if strings.TrimSpace(cfg.ProfileName) == "" { return fmt.Errorf("--name is required. Choose a unique name for this profile") } if cfg.APIKey == "" { return fmt.Errorf("--api-key is required. Get your API key from the provider's console") } + normalizedProvider := strings.ToLower(strings.TrimSpace(cfg.Provider)) + + profileKey, providerName, profileName, err := config.BuildProfileKey(normalizedProvider, cfg.ProfileName) + if err != nil { + return err + } + if err := validateProfileCreateProvider(providerName); err != nil { + return err + } + appCfg, err := loadConfig() if err != nil { return fmt.Errorf("load config: %w", err) } - profileKey := cfg.Provider + ":" + cfg.ProfileName if _, exists := appCfg.Profiles[profileKey]; exists { - return fmt.Errorf("profile '%s' already exists. Choose a different name or delete it first", profileKey) + return fmt.Errorf("profile %q already exists. Choose a different name or delete it first", profileKey) } profile := config.Profile{ - Provider: cfg.Provider, - Name: cfg.ProfileName, + Provider: providerName, + Name: profileName, Credentials: map[string]interface{}{ "apiKey": cfg.APIKey, }, @@ -146,19 +155,35 @@ func runProfileCreate(cfg cli.Config, stdout io.Writer) error { return nil } +func validateProfileCreateProvider(provider string) error { + switch provider { + case "gcp": + return nil + case "aws", "azure", "ibm", "alibaba": + return fmt.Errorf("provider %q is not yet implemented. Available today: gcp", provider) + default: + return fmt.Errorf("unsupported provider %q. Supported providers: gcp", provider) + } +} + func runProfileDelete(cfg cli.Config, stdout io.Writer) error { appCfg, err := loadConfig() if err != nil { return fmt.Errorf("load config: %w", err) } - if _, exists := appCfg.Profiles[cfg.Profile]; !exists { - return fmt.Errorf("profile '%s' not found. Run 'ttscli profile list' to see available profiles", cfg.Profile) + profileKey, _, _, err := config.ParseProfileKey(cfg.Profile) + if err != nil { + return err + } + + if _, exists := appCfg.Profiles[profileKey]; !exists { + return fmt.Errorf("profile %q not found. Run 'ttscli profile list' to see available profiles", profileKey) } - delete(appCfg.Profiles, cfg.Profile) + delete(appCfg.Profiles, profileKey) - if appCfg.ActiveProvider+":"+appCfg.ActiveProfile == cfg.Profile { + if appCfg.ActiveProvider+":"+appCfg.ActiveProfile == profileKey { appCfg.ActiveProvider = "" appCfg.ActiveProfile = "" for key, profile := range appCfg.Profiles { @@ -176,7 +201,7 @@ func runProfileDelete(cfg cli.Config, stdout io.Writer) error { return fmt.Errorf("save config: %w", err) } - fmt.Fprintf(stdout, "✓ Profile deleted: %s\n", cfg.Profile) + fmt.Fprintf(stdout, "✓ Profile deleted: %s\n", profileKey) return nil } @@ -186,17 +211,13 @@ func runProfileUse(cfg cli.Config, stdout io.Writer) error { return fmt.Errorf("load config: %w", err) } - parts := strings.Split(cfg.Profile, ":") - if len(parts) != 2 { - return fmt.Errorf("invalid profile format. Expected 'provider:name' (e.g., gcp:default)") + profileKey, provider, name, err := config.ParseProfileKey(cfg.Profile) + if err != nil { + return err } - provider := parts[0] - name := parts[1] - profileKey := cfg.Profile - if _, exists := appCfg.Profiles[profileKey]; !exists { - return fmt.Errorf("profile '%s' not found. Run 'ttscli profile list' to see available profiles", profileKey) + return fmt.Errorf("profile %q not found. Run 'ttscli profile list' to see available profiles", profileKey) } appCfg.ActiveProvider = provider @@ -220,11 +241,16 @@ func runProfileGet(cfg cli.Config, stdout io.Writer) error { return fmt.Errorf("load config: %w", err) } - profile, err := getProfile(appCfg, cfg.Profile) + profileKey, _, _, err := config.ParseProfileKey(cfg.Profile) if err != nil { return err } + profile, exists := appCfg.Profiles[profileKey] + if !exists { + return fmt.Errorf("profile %q not found. Run 'ttscli profile list' to see available profiles", profileKey) + } + isActive := "" if profile.Provider == appCfg.ActiveProvider && profile.Name == appCfg.ActiveProfile { isActive = " (active)" @@ -232,7 +258,7 @@ func runProfileGet(cfg cli.Config, stdout io.Writer) error { fmt.Fprintln(stdout, "Profile Details") fmt.Fprintln(stdout, "───────────────") - fmt.Fprintf(stdout, "Profile Key: %s%s\n", cfg.Profile, isActive) + fmt.Fprintf(stdout, "Profile Key: %s%s\n", profileKey, isActive) fmt.Fprintf(stdout, "Provider: %s\n", profile.Provider) fmt.Fprintf(stdout, "Name: %s\n", profile.Name) fmt.Fprintf(stdout, "API Key: %s\n", maskAPIKey(resolveProfileAPIKey(profile))) @@ -248,7 +274,7 @@ func runProfileGet(cfg cli.Config, stdout io.Writer) error { } fmt.Fprintln(stdout) fmt.Fprintln(stdout, "Usage:") - fmt.Fprintf(stdout, " ttscli speak --text \"Hello\" --profile %s\n", cfg.Profile) - fmt.Fprintf(stdout, " ttscli save --text \"Hello\" --out speech.mp3 --profile %s\n", cfg.Profile) + fmt.Fprintf(stdout, " ttscli speak --text \"Hello\" --profile %s\n", profileKey) + fmt.Fprintf(stdout, " ttscli save --text \"Hello\" --out speech.mp3 --profile %s\n", profileKey) return nil } diff --git a/internal/app/command_profile_test.go b/internal/app/command_profile_test.go index 4abe16e..056602d 100644 --- a/internal/app/command_profile_test.go +++ b/internal/app/command_profile_test.go @@ -98,6 +98,46 @@ func TestRunProfileCreateSuccess(t *testing.T) { } } +func TestRunProfileCreateNormalizesProviderName(t *testing.T) { + reset := stubAppDeps() + defer reset() + + loadConfig = func() (config.Config, error) { + return config.Config{Profiles: map[string]config.Profile{}}, nil + } + newProvider = func(profile config.Profile) (tts.Provider, error) { + if profile.Provider != "gcp" { + t.Fatalf("expected normalized provider gcp, got %q", profile.Provider) + } + return &fakeTTSClient{ + listVoicesFn: func(ctx context.Context, lang string) ([]tts.Voice, error) { + return []tts.Voice{{Name: "en-US-Neural2-F"}}, nil + }, + synthesizeFn: func(ctx context.Context, text, lang, voice, enc string) ([]byte, error) { + return nil, nil + }, + }, nil + } + + var saved config.Config + saveConfig = func(cfg config.Config) error { + saved = cfg + return nil + } + + var stdout bytes.Buffer + cfg := cli.Config{Provider: " GCP ", ProfileName: "work", APIKey: "test-key"} + if err := runProfileCreate(cfg, &stdout); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, exists := saved.Profiles["gcp:work"]; !exists { + t.Fatalf("expected normalized profile key gcp:work in saved config, got: %#v", saved.Profiles) + } + if !strings.Contains(stdout.String(), "Profile created: gcp:work") { + t.Errorf("expected normalized creation message, got: %q", stdout.String()) + } +} + func TestRunProfileCreateAlreadyExists(t *testing.T) { reset := stubAppDeps() defer reset() @@ -105,8 +145,9 @@ func TestRunProfileCreateAlreadyExists(t *testing.T) { // stubAppDeps has gcp:default — try to create it again. cfg := cli.Config{Provider: "gcp", ProfileName: "default", APIKey: "test-key"} err := runProfileCreate(cfg, &bytes.Buffer{}) - if err == nil || !strings.Contains(err.Error(), "already exists") { - t.Fatalf("expected already exists error, got: %v", err) + want := `profile "gcp:default" already exists. Choose a different name or delete it first` + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) } } @@ -132,6 +173,81 @@ func TestRunProfileCreateMissingName(t *testing.T) { } } +func TestRunProfileCreateRejectsInvalidProvider(t *testing.T) { + reset := stubAppDeps() + defer reset() + + cfg := cli.Config{Provider: "gc:p", ProfileName: "work", APIKey: "test-key"} + err := runProfileCreate(cfg, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "invalid provider") { + t.Fatalf("expected invalid provider error, got: %v", err) + } +} + +func TestRunProfileCreateRejectsUnimplementedProvider(t *testing.T) { + reset := stubAppDeps() + defer reset() + + providerCalled := false + newProvider = func(profile config.Profile) (tts.Provider, error) { + providerCalled = true + return nil, nil + } + + cfg := cli.Config{Provider: "aws", ProfileName: "work", APIKey: "test-key"} + err := runProfileCreate(cfg, &bytes.Buffer{}) + want := `provider "aws" is not yet implemented. Available today: gcp` + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } + if providerCalled { + t.Fatal("expected provider initialization to be skipped for unimplemented provider") + } +} + +func TestRunProfileCreateRejectsUnknownProvider(t *testing.T) { + reset := stubAppDeps() + defer reset() + + providerCalled := false + newProvider = func(profile config.Profile) (tts.Provider, error) { + providerCalled = true + return nil, nil + } + + cfg := cli.Config{Provider: "openai", ProfileName: "work", APIKey: "test-key"} + err := runProfileCreate(cfg, &bytes.Buffer{}) + want := `unsupported provider "openai". Supported providers: gcp` + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } + if providerCalled { + t.Fatal("expected provider initialization to be skipped for unsupported provider") + } +} + +func TestRunProfileCreateRejectsInvalidProfileName(t *testing.T) { + reset := stubAppDeps() + defer reset() + + cfg := cli.Config{Provider: "gcp", ProfileName: "wo:rk", APIKey: "test-key"} + err := runProfileCreate(cfg, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "invalid profile name") { + t.Fatalf("expected invalid profile name error, got: %v", err) + } +} + +func TestRunProfileCreateRejectsWhitespaceOnlyName(t *testing.T) { + reset := stubAppDeps() + defer reset() + + cfg := cli.Config{Provider: "gcp", ProfileName: " ", APIKey: "test-key"} + err := runProfileCreate(cfg, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "--name is required") { + t.Fatalf("expected name required error, got: %v", err) + } +} + func TestRunProfileCreateMissingAPIKey(t *testing.T) { reset := stubAppDeps() defer reset() @@ -170,8 +286,9 @@ func TestRunProfileDeleteNotFound(t *testing.T) { defer reset() err := runProfileDelete(cli.Config{Profile: "gcp:nonexistent"}, &bytes.Buffer{}) - if err == nil || !strings.Contains(err.Error(), "not found") { - t.Fatalf("expected not found error, got: %v", err) + want := `profile "gcp:nonexistent" not found. Run 'ttscli profile list' to see available profiles` + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) } } @@ -279,18 +396,29 @@ func TestRunProfileUseInvalidFormat(t *testing.T) { defer reset() err := runProfileUse(cli.Config{Profile: "invalid-format"}, &bytes.Buffer{}) - if err == nil || !strings.Contains(err.Error(), "invalid profile format") { + if err == nil || !strings.Contains(err.Error(), "invalid profile key") { t.Fatalf("expected invalid format error, got: %v", err) } } +func TestRunProfileDeleteInvalidFormat(t *testing.T) { + reset := stubAppDeps() + defer reset() + + err := runProfileDelete(cli.Config{Profile: "gcp"}, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "invalid profile key") { + t.Fatalf("expected invalid profile key error, got: %v", err) + } +} + func TestRunProfileUseNotFound(t *testing.T) { reset := stubAppDeps() defer reset() err := runProfileUse(cli.Config{Profile: "gcp:nonexistent"}, &bytes.Buffer{}) - if err == nil || !strings.Contains(err.Error(), "not found") { - t.Fatalf("expected not found error, got: %v", err) + want := `profile "gcp:nonexistent" not found. Run 'ttscli profile list' to see available profiles` + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) } } @@ -313,8 +441,19 @@ func TestRunProfileGetNotFound(t *testing.T) { defer reset() err := runProfileGet(cli.Config{Profile: "gcp:nonexistent"}, &bytes.Buffer{}) - if err == nil { - t.Fatal("expected error for missing profile") + want := `profile "gcp:nonexistent" not found. Run 'ttscli profile list' to see available profiles` + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } +} + +func TestRunProfileGetInvalidFormat(t *testing.T) { + reset := stubAppDeps() + defer reset() + + err := runProfileGet(cli.Config{Profile: "gcp:"}, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "invalid profile key") { + t.Fatalf("expected invalid profile key error, got: %v", err) } } diff --git a/internal/app/command_setup.go b/internal/app/command_setup.go index dab21e4..3187f3f 100644 --- a/internal/app/command_setup.go +++ b/internal/app/command_setup.go @@ -37,7 +37,11 @@ func runSetupCommand(stdout, stderr io.Writer) error { profileName = "default" } - profileKey = "gcp:" + profileName + var buildErr error + profileKey, _, profileName, buildErr = config.BuildProfileKey("gcp", profileName) + if buildErr != nil { + return buildErr + } if _, exists := appCfg.Profiles[profileKey]; exists { fmt.Fprintf(stdout, "Profile '%s' already exists.\n", profileKey) fmt.Fprintf(stdout, "Tip: Use 'ttscli profile use %s' to activate it.\n", profileKey) @@ -144,7 +148,7 @@ func runSetupCommand(stdout, stderr io.Writer) error { if err != nil { return fmt.Errorf("resolve config path: %w", err) } - + fmt.Fprintln(stdout) fmt.Fprintln(stdout, "✓ Setup complete!") fmt.Fprintln(stdout) diff --git a/internal/app/command_setup_test.go b/internal/app/command_setup_test.go index 8a85261..1bb5592 100644 --- a/internal/app/command_setup_test.go +++ b/internal/app/command_setup_test.go @@ -77,6 +77,18 @@ func TestRunSetupCommandProfileAlreadyExists(t *testing.T) { } } +func TestRunSetupCommandRejectsInvalidProfileName(t *testing.T) { + reset := stubAppDeps() + defer reset() + + setupInput = strings.NewReader("bad:name\n") + + err := runSetupCommand(&bytes.Buffer{}, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "invalid profile name") { + t.Fatalf("expected invalid profile name error, got: %v", err) + } +} + func TestRunSetupCommandVoiceNotAvailable(t *testing.T) { reset := stubAppDeps() defer reset() diff --git a/internal/app/run.go b/internal/app/run.go index 63fdd82..6266c2e 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -5,6 +5,7 @@ import ( "io" "github.com/ppikrorngarn/ttscli/internal/cli" + "github.com/ppikrorngarn/ttscli/internal/config" "github.com/ppikrorngarn/ttscli/internal/tts" ) @@ -42,7 +43,11 @@ func Run(args []string, stdout, stderr io.Writer) error { // Determine which profile to use var profileKey string if cfg.Profile != "" { - profileKey = cfg.Profile + var err error + profileKey, _, _, err = config.ParseProfileKey(cfg.Profile) + if err != nil { + return err + } } else { // Use active profile from config if appCfg.ActiveProvider == "" || appCfg.ActiveProfile == "" { diff --git a/internal/app/run_modes_test.go b/internal/app/run_modes_test.go index 5b53526..4734930 100644 --- a/internal/app/run_modes_test.go +++ b/internal/app/run_modes_test.go @@ -69,6 +69,20 @@ func TestRunProfileFlag(t *testing.T) { } } +func TestRunProfileFlagInvalidFormat(t *testing.T) { + reset := stubAppDeps() + defer reset() + + parseArgs = func(args []string, stderr io.Writer) (cli.Config, error) { + return cli.Config{Mode: cli.ModeVoices, ListVoices: true, Lang: "en-US", Profile: "gcp"}, nil + } + + err := Run([]string{"voices", "--profile", "gcp"}, &bytes.Buffer{}, &bytes.Buffer{}) + if err == nil || !strings.Contains(err.Error(), "invalid profile key") { + t.Fatalf("expected invalid profile key error, got: %v", err) + } +} + func TestRunSynthesizeError(t *testing.T) { reset := stubAppDeps() defer reset() diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 7081f3c..65e38df 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -16,20 +16,14 @@ const ( helpUsageSpeak = ` ttscli speak --text "Hello world"` helpUsageSave = ` ttscli save --text "Hello world" --out output.mp3` helpUsageVoices = " ttscli voices --lang en-GB" - helpUsageSetup = " ttscli setup" - helpUsageDoctor = " ttscli doctor" - helpUsageCompletion = " ttscli completion " - helpUsageProfile = " ttscli profile [flags]" helpExampleSpeak = ` ttscli speak --text "Hello world, this is a test."` helpExampleSave = ` ttscli save --text "Save this to a file." --out output.mp3` helpExampleVoices = " ttscli voices --lang en-GB" helpExampleSetup = " ttscli setup" helpExampleDoctor = " ttscli doctor" - helpExampleCompletion = " ttscli completion zsh" helpExampleProfile = " ttscli profile create --provider gcp --name work --api-key YOUR_API_KEY" helpExampleSpeakAlias = ` ttscli speak -t "Quick test" -l en-GB -v en-GB-Neural2-B` helpExampleSaveAlias = ` ttscli save -t "Save this" -o speech.mp3` - helpAliases = "Aliases: -t (text), -o (out), -l (lang), -v (voice), -p (profile)" ModeSpeak = "speak" ModeSave = "save" ModeVoices = "voices" @@ -62,7 +56,6 @@ type Config struct { ListVoices bool HasVoiceFlag bool HasLangFlag bool - HasAPIKeyFlag bool DefaultSubcommand string Profile string Provider string @@ -130,6 +123,40 @@ func parseCompletionCommand(args []string) (Config, error) { return cfg, nil } +func addLangFlag(fs *flag.FlagSet, target *string, usage string) { + fs.StringVar(target, "lang", DefaultLanguage, usage) + fs.StringVar(target, "l", DefaultLanguage, "Language code (shorthand)") +} + +func addVoiceFlag(fs *flag.FlagSet, target *string, usage string) { + fs.StringVar(target, "voice", DefaultVoice, usage) + fs.StringVar(target, "v", DefaultVoice, "Voice name (shorthand)") +} + +func addProfileFlag(fs *flag.FlagSet, target *string) { + fs.StringVar(target, "profile", "", "Profile to use (e.g., gcp:default)") + fs.StringVar(target, "p", "", "Profile to use (shorthand)") +} + +func markExplicitSpeechFlags(fs *flag.FlagSet, cfg *Config) { + fs.Visit(func(f *flag.Flag) { + switch f.Name { + case "voice", "v": + cfg.HasVoiceFlag = true + case "lang", "l": + cfg.HasLangFlag = true + } + }) +} + +func markExplicitVoiceListFlags(fs *flag.FlagSet, cfg *Config) { + fs.Visit(func(f *flag.Flag) { + if f.Name == "lang" || f.Name == "l" { + cfg.HasLangFlag = true + } + }) +} + func parseSpeakCommand(args []string, stderr io.Writer) (Config, error) { cfg := Config{Mode: ModeSpeak} fs := flag.NewFlagSet(appName+" speak", flag.ContinueOnError) @@ -137,24 +164,14 @@ func parseSpeakCommand(args []string, stderr io.Writer) (Config, error) { fs.StringVar(&cfg.Text, "text", "", "Text to convert to speech") fs.StringVar(&cfg.Text, "t", "", "Text to convert to speech (shorthand)") - fs.StringVar(&cfg.Lang, "lang", DefaultLanguage, "Language code (e.g., en-US, en-GB, fr-FR)") - fs.StringVar(&cfg.Lang, "l", DefaultLanguage, "Language code (shorthand)") - fs.StringVar(&cfg.Voice, "voice", DefaultVoice, "Voice name for synthesis") - fs.StringVar(&cfg.Voice, "v", DefaultVoice, "Voice name (shorthand)") - fs.StringVar(&cfg.Profile, "profile", "", "Profile to use (e.g., gcp:default)") - fs.StringVar(&cfg.Profile, "p", "", "Profile to use (shorthand)") + addLangFlag(fs, &cfg.Lang, "Language code (e.g., en-US, en-GB, fr-FR)") + addVoiceFlag(fs, &cfg.Voice, "Voice name for synthesis") + addProfileFlag(fs, &cfg.Profile) if err := fs.Parse(args); err != nil { return cfg, err } - fs.Visit(func(f *flag.Flag) { - switch f.Name { - case "voice", "v": - cfg.HasVoiceFlag = true - case "lang", "l": - cfg.HasLangFlag = true - } - }) + markExplicitSpeechFlags(fs, &cfg) if fs.NArg() > 0 { return cfg, fmt.Errorf("unexpected arguments: %s. Use --text or -t to provide text", strings.Join(fs.Args(), " ")) @@ -175,24 +192,14 @@ func parseSaveCommand(args []string, stderr io.Writer) (Config, error) { fs.StringVar(&cfg.Text, "t", "", "Text to convert to speech (shorthand)") fs.StringVar(&cfg.SavePath, "out", "", "Path to save the output MP3 file (e.g., output.mp3)") fs.StringVar(&cfg.SavePath, "o", "", "Path to save the output MP3 file (shorthand)") - fs.StringVar(&cfg.Lang, "lang", DefaultLanguage, "Language code (e.g., en-US, en-GB, fr-FR)") - fs.StringVar(&cfg.Lang, "l", DefaultLanguage, "Language code (shorthand)") - fs.StringVar(&cfg.Voice, "voice", DefaultVoice, "Voice name for synthesis") - fs.StringVar(&cfg.Voice, "v", DefaultVoice, "Voice name (shorthand)") - fs.StringVar(&cfg.Profile, "profile", "", "Profile to use (e.g., gcp:default)") - fs.StringVar(&cfg.Profile, "p", "", "Profile to use (shorthand)") + addLangFlag(fs, &cfg.Lang, "Language code (e.g., en-US, en-GB, fr-FR)") + addVoiceFlag(fs, &cfg.Voice, "Voice name for synthesis") + addProfileFlag(fs, &cfg.Profile) if err := fs.Parse(args); err != nil { return cfg, err } - fs.Visit(func(f *flag.Flag) { - switch f.Name { - case "voice", "v": - cfg.HasVoiceFlag = true - case "lang", "l": - cfg.HasLangFlag = true - } - }) + markExplicitSpeechFlags(fs, &cfg) if fs.NArg() > 0 { return cfg, fmt.Errorf("unexpected arguments: %s. Use --text/-t and --out/-o", strings.Join(fs.Args(), " ")) @@ -210,19 +217,13 @@ func parseVoicesCommand(args []string, stderr io.Writer) (Config, error) { cfg := Config{Mode: ModeVoices} fs := flag.NewFlagSet(appName+" voices", flag.ContinueOnError) fs.SetOutput(stderr) - fs.StringVar(&cfg.Lang, "lang", DefaultLanguage, "Language code to filter voices (e.g., en-US, en-GB)") - fs.StringVar(&cfg.Lang, "l", DefaultLanguage, "Language code (shorthand)") - fs.StringVar(&cfg.Profile, "profile", "", "Profile to use (e.g., gcp:default)") - fs.StringVar(&cfg.Profile, "p", "", "Profile to use (shorthand)") + addLangFlag(fs, &cfg.Lang, "Language code to filter voices (e.g., en-US, en-GB)") + addProfileFlag(fs, &cfg.Profile) if err := fs.Parse(args); err != nil { return cfg, err } - fs.Visit(func(f *flag.Flag) { - if f.Name == "lang" || f.Name == "l" { - cfg.HasLangFlag = true - } - }) + markExplicitVoiceListFlags(fs, &cfg) if fs.NArg() > 0 { return cfg, fmt.Errorf("unexpected arguments: %s. Use --lang or -l to filter by language", strings.Join(fs.Args(), " ")) diff --git a/internal/config/config.go b/internal/config/config.go index be0bebc..0fcdb89 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strings" ) const ( @@ -113,7 +114,7 @@ func LoadConfig() (Config, error) { func SaveConfig(cfg Config) error { path, err := Path() if err != nil { - return err + return fmt.Errorf("resolve config path: %w", err) } if err := mkdirAll(filepath.Dir(path), dirPermission); err != nil { return fmt.Errorf("create config dir (%s): %w", filepath.Dir(path), err) @@ -138,9 +139,40 @@ func SaveConfig(cfg Config) error { } func GetProfile(cfg Config, profileKey string) (Profile, error) { + if _, _, _, err := ParseProfileKey(profileKey); err != nil { + return Profile{}, err + } profile, ok := cfg.Profiles[profileKey] if !ok { return Profile{}, fmt.Errorf("profile %q not found", profileKey) } return profile, nil } + +func ParseProfileKey(raw string) (key, provider, name string, err error) { + parts := strings.Split(raw, ":") + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return "", "", "", fmt.Errorf("invalid profile key %q. Expected 'provider:name' (e.g., gcp:default)", raw) + } + return raw, parts[0], parts[1], nil +} + +func BuildProfileKey(provider, name string) (key, normalizedProvider, normalizedName string, err error) { + normalizedProvider = strings.TrimSpace(provider) + normalizedName = strings.TrimSpace(name) + + if normalizedProvider == "" { + return "", "", "", fmt.Errorf("provider is required") + } + if normalizedName == "" { + return "", "", "", fmt.Errorf("profile name is required") + } + if strings.Contains(normalizedProvider, ":") { + return "", "", "", fmt.Errorf("invalid provider %q. Provider names must not contain ':'", normalizedProvider) + } + if strings.Contains(normalizedName, ":") { + return "", "", "", fmt.Errorf("invalid profile name %q. Profile names must not contain ':'", normalizedName) + } + + return normalizedProvider + ":" + normalizedName, normalizedProvider, normalizedName, nil +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a335496..9e8da30 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -234,6 +234,72 @@ func TestLoadConfigParseError(t *testing.T) { } } +func TestParseProfileKeyValid(t *testing.T) { + key, provider, name, err := ParseProfileKey("gcp:default") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if key != "gcp:default" || provider != "gcp" || name != "default" { + t.Fatalf("unexpected parse result: key=%q provider=%q name=%q", key, provider, name) + } +} + +func TestParseProfileKeyInvalid(t *testing.T) { + tests := []string{"", "gcp", ":default", "gcp:", "gcp:work:extra"} + for _, raw := range tests { + _, _, _, err := ParseProfileKey(raw) + if err == nil { + t.Fatalf("expected error for %q", raw) + } + if !strings.Contains(err.Error(), "invalid profile key") { + t.Fatalf("expected invalid profile key error for %q, got: %v", raw, err) + } + } +} + +func TestBuildProfileKeyValid(t *testing.T) { + key, provider, name, err := BuildProfileKey(" gcp ", " work ") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if key != "gcp:work" || provider != "gcp" || name != "work" { + t.Fatalf("unexpected build result: key=%q provider=%q name=%q", key, provider, name) + } +} + +func TestBuildProfileKeyInvalid(t *testing.T) { + tests := []struct { + provider string + name string + want string + }{ + {provider: "", name: "work", want: "provider is required"}, + {provider: "gcp", name: "", want: "profile name is required"}, + {provider: "gc:p", name: "work", want: "invalid provider"}, + {provider: "gcp", name: "wo:rk", want: "invalid profile name"}, + } + + for _, tt := range tests { + _, _, _, err := BuildProfileKey(tt.provider, tt.name) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("expected %q error for provider=%q name=%q, got: %v", tt.want, tt.provider, tt.name, err) + } + } +} + +func TestGetProfileRejectsInvalidProfileKey(t *testing.T) { + cfg := Config{ + Profiles: map[string]Profile{ + "gcp:default": {Provider: "gcp", Name: "default"}, + }, + } + + _, err := GetProfile(cfg, "gcp") + if err == nil || !strings.Contains(err.Error(), "invalid profile key") { + t.Fatalf("expected invalid profile key error, got: %v", err) + } +} + // SaveConfig tests func TestSaveConfigSuccess(t *testing.T) { @@ -319,6 +385,20 @@ func TestSaveConfigMkdirError(t *testing.T) { } } +func TestSaveConfigPathError(t *testing.T) { + reset := stubConfigDeps() + defer reset() + + userConfigDir = func() (string, error) { + return "", errors.New("config dir failed") + } + + err := SaveConfig(Config{Profiles: map[string]Profile{}}) + if err == nil || !strings.Contains(err.Error(), "resolve config path") { + t.Fatalf("expected resolve config path error, got: %v", err) + } +} + func TestSaveConfigWriteError(t *testing.T) { reset := stubConfigDeps() defer reset() diff --git a/internal/player/player.go b/internal/player/player.go index c0f3220..2581777 100644 --- a/internal/player/player.go +++ b/internal/player/player.go @@ -44,7 +44,7 @@ func PlayAudio(audioBytes []byte, stdout, stderr io.Writer) error { cmd, err := playCommand(currentGOOS(), tmpFilePath, lookPathCmd) if err != nil { - return err + return fmt.Errorf("build player command: %w", err) } cmd.Stdout = stdout cmd.Stderr = stderr diff --git a/internal/player/player_test.go b/internal/player/player_test.go index ed57c84..eb15400 100644 --- a/internal/player/player_test.go +++ b/internal/player/player_test.go @@ -158,8 +158,8 @@ func TestPlayAudioBuildCommandError(t *testing.T) { } err := PlayAudio([]byte("audio"), &bytes.Buffer{}, &bytes.Buffer{}) - if err == nil { - t.Fatal("expected build command error") + if err == nil || !strings.Contains(err.Error(), "build player command") { + t.Fatalf("expected build player command error, got: %v", err) } } diff --git a/internal/tts/client.go b/internal/tts/client.go index cbed326..b78f014 100644 --- a/internal/tts/client.go +++ b/internal/tts/client.go @@ -193,11 +193,60 @@ func (c *Client) do(req *http.Request) ([]byte, error) { return nil, fmt.Errorf("read response: %w", err) } if resp.StatusCode < 200 || resp.StatusCode > 299 { - return nil, fmt.Errorf("status=%d body=%s", resp.StatusCode, string(body)) + return nil, formatAPIError(resp.StatusCode, body) } return body, nil } +func formatAPIError(statusCode int, body []byte) error { + message := extractAPIErrorMessage(body) + if message == "" { + return fmt.Errorf("tts request failed with status %d", statusCode) + } + return fmt.Errorf("tts request failed with status %d: %s", statusCode, message) +} + +func extractAPIErrorMessage(body []byte) string { + var parsed struct { + Error json.RawMessage `json:"error"` + Message string `json:"message"` + } + if err := json.Unmarshal(body, &parsed); err != nil { + return "" + } + + if message := extractNestedErrorMessage(parsed.Error); message != "" { + return message + } + for _, candidate := range []string{ + parsed.Message, + stringValue(parsed.Error), + } { + if message := strings.TrimSpace(candidate); message != "" { + return message + } + } + return "" +} + +func extractNestedErrorMessage(raw json.RawMessage) string { + var nested struct { + Message string `json:"message"` + } + if err := json.Unmarshal(raw, &nested); err != nil { + return "" + } + return strings.TrimSpace(nested.Message) +} + +func stringValue(raw json.RawMessage) string { + var value string + if err := json.Unmarshal(raw, &value); err != nil { + return "" + } + return strings.TrimSpace(value) +} + func PrintVoices(w io.Writer, langCode string, voices []Voice) { fmt.Fprintf(w, "Available voices for language: %s\n", langCode) fmt.Fprintf(w, "Found %d voice(s)\n", len(voices)) diff --git a/internal/tts/client_test.go b/internal/tts/client_test.go index 7b2c127..9e7c7d7 100644 --- a/internal/tts/client_test.go +++ b/internal/tts/client_test.go @@ -84,8 +84,28 @@ func TestTTSClientSynthesizeErrorStatus(t *testing.T) { c.baseURL = srv.URL _, err := c.Synthesize(context.Background(), "hello", "en-US", "en-US-Neural2-F", AudioEncodingMP3) - if err == nil || !strings.Contains(err.Error(), "status=400") { - t.Fatalf("expected status error, got: %v", err) + want := "tts request failed with status 400" + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } +} + +func TestTTSClientSynthesizeErrorStatusStructuredMessage(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assertAPIKeyHeader(t, r) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, `{"error":{"message":"API key not valid"}}`) + })) + defer srv.Close() + + c := NewClient("k", srv.Client()) + c.baseURL = srv.URL + + _, err := c.Synthesize(context.Background(), "hello", "en-US", "en-US-Neural2-F", AudioEncodingMP3) + want := "tts request failed with status 400: API key not valid" + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) } } @@ -286,8 +306,72 @@ func TestTTSClientListVoicesErrorStatus(t *testing.T) { c.baseURL = srv.URL _, err := c.ListVoices(context.Background(), "en-US") - if err == nil || !strings.Contains(err.Error(), "status=401") { - t.Fatalf("expected status error, got: %v", err) + want := "tts request failed with status 401" + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } +} + +func TestTTSClientListVoicesErrorStatusTopLevelMessage(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assertAPIKeyHeader(t, r) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + fmt.Fprint(w, `{"message":"quota exceeded"}`) + })) + defer srv.Close() + + c := NewClient("k", srv.Client()) + c.baseURL = srv.URL + + _, err := c.ListVoices(context.Background(), "en-US") + want := "tts request failed with status 429: quota exceeded" + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } +} + +func TestTTSClientListVoicesErrorStatusStringError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assertAPIKeyHeader(t, r) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, `{"error":"unauthorized"}`) + })) + defer srv.Close() + + c := NewClient("k", srv.Client()) + c.baseURL = srv.URL + + _, err := c.ListVoices(context.Background(), "en-US") + want := "tts request failed with status 401: unauthorized" + if err == nil || err.Error() != want { + t.Fatalf("expected %q, got: %v", want, err) + } +} + +func TestTTSClientSynthesizeErrorStatusDoesNotLeakResponseBody(t *testing.T) { + const secret = "tts-sentinel-secret-98765" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assertAPIKeyHeader(t, r) + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, secret) + })) + defer srv.Close() + + c := NewClient("k", srv.Client()) + c.baseURL = srv.URL + + _, err := c.Synthesize(context.Background(), "hello", "en-US", "en-US-Neural2-F", AudioEncodingMP3) + if err == nil { + t.Fatal("expected status error") + } + if strings.Contains(err.Error(), secret) { + t.Fatalf("response body leaked into error: %v", err) + } + if err.Error() != "tts request failed with status 403" { + t.Fatalf("expected status-only fallback, got: %v", err) } }