From 616beecc41b573503dad9f5bfd9f353c6f826a8a Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Mon, 6 Apr 2026 11:12:03 +0300 Subject: refactor: extract shared CardStore into internal/store to eliminate duplication FindCardDirectory, FindOrCreateCardDirectory, GenerateCardID and the ScanWords helper previously existed in both internal/utils.go (as standalone functions) and were partially duplicated in internal/gui/card_service.go (readWordFromDir, ScanExistingWords). Introduce internal/store.CardStore as the single source of truth for all on-disk card-directory operations. Both internal/processor and internal/gui now hold a *store.CardStore field and delegate to it, removing the last copy of the directory-scanning loop from card_service.go. internal/utils.go keeps thin forwarding wrappers for callers that import the root internal package. Also adds table-driven unit tests for the new package covering FindCardDirectory (including legacy _word.txt fallback), FindOrCreateCardDirectory, and CardStore.ScanWords. Co-Authored-By: Claude Sonnet 4.6 --- internal/gui/card_service.go | 88 +++++--------------- internal/processor/card_store.go | 23 +++--- internal/processor/processor.go | 9 +++ internal/store/store.go | 171 +++++++++++++++++++++++++++++++++++++++ internal/store/store_test.go | 140 ++++++++++++++++++++++++++++++++ internal/utils.go | 73 +++-------------- 6 files changed, 362 insertions(+), 142 deletions(-) create mode 100644 internal/store/store.go create mode 100644 internal/store/store_test.go diff --git a/internal/gui/card_service.go b/internal/gui/card_service.go index 38108ca..395c741 100644 --- a/internal/gui/card_service.go +++ b/internal/gui/card_service.go @@ -4,47 +4,48 @@ import ( "fmt" "os" "path/filepath" - "sort" "strings" "time" "codeberg.org/snonux/totalrecall/internal" "codeberg.org/snonux/totalrecall/internal/anki" + "codeberg.org/snonux/totalrecall/internal/store" ) // CardService manages card file discovery, directory creation, persistence, // and state loading from the output directory. It is responsible for all // non-UI file I/O related to cards, decoupled from UI event-wiring. +// Directory-scanning and creation are delegated to a store.CardStore so the +// underlying algorithm is shared with the processor package (DRY). type CardService struct { - config *Config + config *Config + cardStore *store.CardStore } // NewCardService constructs a CardService for the given configuration. +// It initialises an internal store.CardStore rooted at config.OutputDir. func NewCardService(config *Config) *CardService { - return &CardService{config: config} + return &CardService{ + config: config, + cardStore: store.New(config.OutputDir), + } } // FindCardDirectory finds the directory for a given Bulgarian word. -// Delegates to the shared internal.FindCardDirectory which also handles the -// legacy _word.txt fallback for backward compatibility. +// Delegates to the shared store.CardStore which also handles the legacy +// _word.txt fallback for backward compatibility. func (cs *CardService) FindCardDirectory(word string) string { - return internal.FindCardDirectory(cs.config.OutputDir, word) + return cs.cardStore.FindCardDirectory(word) } // EnsureWordDirectoryAndMetadata creates a new card directory and writes word // metadata to word.txt inside it. Returns the directory path. +// Uses store.FindOrCreateCardDirectory so the creation logic is not duplicated. func (cs *CardService) EnsureWordDirectoryAndMetadata(word string) (string, error) { - cardID := internal.GenerateCardID(word) - wordDir := filepath.Join(cs.config.OutputDir, cardID) - if err := os.MkdirAll(wordDir, 0755); err != nil { - return "", fmt.Errorf("failed to create card directory: %w", err) - } - - metadataFile := filepath.Join(wordDir, "word.txt") - if err := os.WriteFile(metadataFile, []byte(word), 0644); err != nil { - return "", fmt.Errorf("failed to save word metadata: %w", err) + wordDir := cs.cardStore.FindOrCreateCardDirectory(word) + if wordDir == "" || wordDir == cs.config.OutputDir { + return "", fmt.Errorf("failed to create card directory for %q", word) } - return wordDir, nil } @@ -62,59 +63,10 @@ func (cs *CardService) EnsureCardDirectory(word string) (string, error) { // ScanExistingWords scans the output directory for existing card subdirectories // and returns a sorted list of the Bulgarian words found. A directory counts // only if it contains at least one of: an audio file, an image, or a -// translation file. +// translation file. Delegates iteration and word-file reading to the shared +// store.CardStore so that logic is not duplicated here. func (cs *CardService) ScanExistingWords() []string { - words := []string{} - - entries, err := os.ReadDir(cs.config.OutputDir) - if err != nil { - // Directory doesn't exist yet; return empty list silently. - return words - } - - // Each subdirectory represents a card identified by a card ID. - for _, entry := range entries { - if !entry.IsDir() { - continue - } - - cardID := entry.Name() - wordDir := filepath.Join(cs.config.OutputDir, cardID) - - word, ok := cs.readWordFromDir(wordDir) - if !ok { - continue - } - - if cs.dirHasContent(wordDir) { - words = append(words, word) - } - } - - sort.Strings(words) - return words -} - -// readWordFromDir reads the Bulgarian word from word.txt (or the legacy -// _word.txt) inside a card directory. Returns the word and true on success. -func (cs *CardService) readWordFromDir(wordDir string) (string, bool) { - wordFile := filepath.Join(wordDir, "word.txt") - wordData, err := os.ReadFile(wordFile) - if err != nil { - // Try old format with underscore for backward compatibility. - wordFile = filepath.Join(wordDir, "_word.txt") - wordData, err = os.ReadFile(wordFile) - if err != nil { - return "", false - } - } - - word := string(wordData) - if word == "" { - return "", false - } - - return word, true + return cs.cardStore.ScanWords(cs.dirHasContent) } // dirHasContent returns true if the card directory contains at least one audio diff --git a/internal/processor/card_store.go b/internal/processor/card_store.go index 1020998..25b4e36 100644 --- a/internal/processor/card_store.go +++ b/internal/processor/card_store.go @@ -1,14 +1,14 @@ package processor // CardStore manages the on-disk layout of word card directories. -// It wraps the low-level internal.FindCardDirectory / -// internal.FindOrCreateCardDirectory helpers and adds the higher-level -// isWordFullyProcessed check used by the batch processor to skip words that -// have already been completely generated. +// It delegates to the shared store.CardStore (internal/store) for all +// directory-discovery and creation logic, so those algorithms live in exactly +// one place (DRY). The methods here add the higher-level isWordFullyProcessed +// check that is specific to the batch processor. // -// All methods are on *Processor rather than a separate struct to avoid an -// extra layer of indirection while still keeping the concerns separated into -// their own file (SRP at the file level, as recommended for Go packages). +// All methods are on *Processor rather than a separate struct to keep the +// existing call sites unchanged while still separating concerns at the file +// level (SRP at the file level, as recommended for Go packages). import ( "os" @@ -21,16 +21,17 @@ import ( ) // findOrCreateWordDirectory returns the existing card directory for word -// inside the configured output directory, creating it when absent. +// inside the configured output directory, creating it when absent. Delegates +// to the shared CardStore so the directory-creation algorithm is not duplicated. func (p *Processor) findOrCreateWordDirectory(word string) string { - return internal.FindOrCreateCardDirectory(p.flags.OutputDir, word) + return p.cardStore.FindOrCreateCardDirectory(word) } // findCardDirectory searches the configured output directory for an existing // card directory that contains the given word. Returns an empty string when -// no matching directory is found. +// no matching directory is found. Delegates to the shared CardStore. func (p *Processor) findCardDirectory(word string) string { - return internal.FindCardDirectory(p.flags.OutputDir, word) + return p.cardStore.FindCardDirectory(word) } // isWordFullyProcessed returns true when the word's card directory already diff --git a/internal/processor/processor.go b/internal/processor/processor.go index 14e5341..323df23 100644 --- a/internal/processor/processor.go +++ b/internal/processor/processor.go @@ -17,6 +17,7 @@ import ( "codeberg.org/snonux/totalrecall/internal/gui" "codeberg.org/snonux/totalrecall/internal/image" "codeberg.org/snonux/totalrecall/internal/phonetic" + "codeberg.org/snonux/totalrecall/internal/store" "codeberg.org/snonux/totalrecall/internal/translation" ) @@ -77,6 +78,11 @@ type Processor struct { // so individual methods never call Viper directly. cfg *Config + // cardStore is the shared CardStore for locating and creating on-disk + // card directories. It is initialised from flags.OutputDir in NewProcessor + // and used by all card_store.go helpers. + cardStore *store.CardStore + // imageFactories groups the two image-provider construction functions. // Production code uses image.DefaultClientFactories(); tests replace fields. imageFactories image.ClientFactories @@ -103,6 +109,9 @@ func NewProcessor(flags *cli.Flags, cfg *Config) *Processor { translationCache: translation.NewTranslationCache(), phoneticFetcher: phonetic.NewFetcher(&phonetic.Config{Provider: phoneticProvider, OpenAIKey: openAIKey, GoogleAPIKey: googleAPIKey}), randomIntn: rand.Intn, + // cardStore is rooted at the output directory so card-discovery helpers + // never need to know about flags directly. + cardStore: store.New(flags.OutputDir), imageFactories: image.DefaultClientFactories(), newAudioProvider: audio.NewProvider, } diff --git a/internal/store/store.go b/internal/store/store.go new file mode 100644 index 0000000..16f50ba --- /dev/null +++ b/internal/store/store.go @@ -0,0 +1,171 @@ +// Package store provides the CardStore type, a single shared repository for +// locating and creating on-disk card directories. Both the processor and gui +// packages use CardStore so the directory-scanning logic lives in exactly one +// place (DRY / SRP). +// +// Dependency position: store imports only the standard library, so every other +// internal package may import it without creating import cycles. +package store + +import ( + "crypto/md5" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" +) + +// CardStore manages the on-disk layout of word card directories under a single +// output directory. It is safe to create multiple instances pointing at the +// same directory; all operations are stateless file-system reads/writes. +type CardStore struct { + outputDir string +} + +// New constructs a CardStore rooted at outputDir. +func New(outputDir string) *CardStore { + return &CardStore{outputDir: outputDir} +} + +// OutputDir returns the root output directory this store operates on. +func (cs *CardStore) OutputDir() string { + return cs.outputDir +} + +// FindCardDirectory searches the output directory for a subdirectory whose +// word.txt (or legacy _word.txt) matches word. Returns the directory path or +// an empty string when no matching directory is found. +func (cs *CardStore) FindCardDirectory(word string) string { + return FindCardDirectory(cs.outputDir, word) +} + +// FindOrCreateCardDirectory returns the existing card directory for word, or +// creates a new one with a generated card ID and writes word.txt so subsequent +// calls can locate the directory. +func (cs *CardStore) FindOrCreateCardDirectory(word string) string { + return FindOrCreateCardDirectory(cs.outputDir, word) +} + +// ScanWords scans the output directory for subdirectories that contain at +// least one content file (word.txt or legacy _word.txt) and passes a basic +// content check provided by the caller. It returns a sorted list of Bulgarian +// words found. The hasContent predicate receives the full path of each +// candidate directory; pass nil to accept all directories that have a word +// file. +func (cs *CardStore) ScanWords(hasContent func(wordDir string) bool) []string { + entries, err := os.ReadDir(cs.outputDir) + if err != nil { + // Output directory does not exist yet; return empty list silently. + return []string{} + } + + words := make([]string, 0, len(entries)) + + for _, entry := range entries { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { + continue + } + + wordDir := filepath.Join(cs.outputDir, entry.Name()) + + word, ok := readWordFromDir(wordDir) + if !ok { + continue + } + + if hasContent == nil || hasContent(wordDir) { + words = append(words, word) + } + } + + sort.Strings(words) + return words +} + +// GenerateCardID creates a unique ID for a card based on the current timestamp +// and an MD5 hash of the Bulgarian word. +// Format: epochMillis_md5(word)[:8] +func GenerateCardID(bulgarianWord string) string { + epochMillis := time.Now().UnixNano() / 1_000_000 + hash := md5.Sum([]byte(bulgarianWord)) + hashStr := hex.EncodeToString(hash[:])[:8] + return fmt.Sprintf("%d_%s", epochMillis, hashStr) +} + +// FindCardDirectory is the package-level (non-method) version of the directory +// search. It searches outputDir for a subdirectory whose word.txt (or legacy +// _word.txt) content matches word and returns its path, or "" if not found. +func FindCardDirectory(outputDir, word string) string { + entries, err := os.ReadDir(outputDir) + if err != nil { + return "" + } + + for _, entry := range entries { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { + continue + } + + dirPath := filepath.Join(outputDir, entry.Name()) + if found := readWordMatch(dirPath, word); found { + return dirPath + } + } + + return "" +} + +// FindOrCreateCardDirectory returns the existing card directory for word inside +// outputDir, or creates a new one with a generated card ID. It also writes +// word.txt so subsequent calls can find the directory. +func FindOrCreateCardDirectory(outputDir, word string) string { + if dir := FindCardDirectory(outputDir, word); dir != "" { + return dir + } + + cardID := GenerateCardID(word) + wordDir := filepath.Join(outputDir, cardID) + + if err := os.MkdirAll(wordDir, 0755); err != nil { + fmt.Printf("Warning: failed to create word directory: %v\n", err) + return outputDir + } + + if err := os.WriteFile(filepath.Join(wordDir, "word.txt"), []byte(word), 0644); err != nil { + fmt.Printf("Warning: failed to save word metadata: %v\n", err) + } + + return wordDir +} + +// readWordFromDir reads the Bulgarian word stored in word.txt (or the legacy +// _word.txt) inside a card directory. Returns the word and true on success. +func readWordFromDir(wordDir string) (string, bool) { + wordFile := filepath.Join(wordDir, "word.txt") + data, err := os.ReadFile(wordFile) + if err != nil { + // Backward-compatible fallback: old format used _word.txt. + wordFile = filepath.Join(wordDir, "_word.txt") + data, err = os.ReadFile(wordFile) + if err != nil { + return "", false + } + } + + word := strings.TrimSpace(string(data)) + if word == "" { + return "", false + } + + return word, true +} + +// readWordMatch returns true when the card directory at dirPath contains a +// word file whose trimmed content equals word. +func readWordMatch(dirPath, word string) bool { + found, ok := readWordFromDir(dirPath) + return ok && found == word +} diff --git a/internal/store/store_test.go b/internal/store/store_test.go new file mode 100644 index 0000000..6d34d0c --- /dev/null +++ b/internal/store/store_test.go @@ -0,0 +1,140 @@ +package store_test + +import ( + "os" + "path/filepath" + "testing" + + "codeberg.org/snonux/totalrecall/internal/store" +) + +// TestFindCardDirectory verifies that FindCardDirectory locates a directory by +// its word.txt content and returns an empty string when no match exists. +func TestFindCardDirectory(t *testing.T) { + tmpDir := t.TempDir() + + // Create a card directory with a word file. + cardDir := filepath.Join(tmpDir, "someCardID") + if err := os.MkdirAll(cardDir, 0755); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(cardDir, "word.txt"), []byte("котка"), 0644); err != nil { + t.Fatalf("setup: %v", err) + } + + tests := []struct { + name string + word string + wantDir string // empty means expect "" + wantHit bool + }{ + { + name: "existing word found", + word: "котка", + wantDir: cardDir, + wantHit: true, + }, + { + name: "unknown word returns empty string", + word: "куче", + wantDir: "", + wantHit: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := store.FindCardDirectory(tmpDir, tt.word) + if tt.wantHit && got != tt.wantDir { + t.Errorf("FindCardDirectory(%q) = %q; want %q", tt.word, got, tt.wantDir) + } + if !tt.wantHit && got != "" { + t.Errorf("FindCardDirectory(%q) = %q; want empty string", tt.word, got) + } + }) + } +} + +// TestFindCardDirectoryLegacyFallback checks that the legacy _word.txt naming +// convention is still supported for backward compatibility. +func TestFindCardDirectoryLegacyFallback(t *testing.T) { + tmpDir := t.TempDir() + + // Create a card directory with the old _word.txt naming. + cardDir := filepath.Join(tmpDir, "legacyCardID") + if err := os.MkdirAll(cardDir, 0755); err != nil { + t.Fatalf("setup: %v", err) + } + if err := os.WriteFile(filepath.Join(cardDir, "_word.txt"), []byte("ябълка"), 0644); err != nil { + t.Fatalf("setup: %v", err) + } + + got := store.FindCardDirectory(tmpDir, "ябълка") + if got != cardDir { + t.Errorf("FindCardDirectory (legacy) = %q; want %q", got, cardDir) + } +} + +// TestFindOrCreateCardDirectory verifies that a new directory is created when +// no matching one exists, and that the same directory is returned on a second +// call for the same word. +func TestFindOrCreateCardDirectory(t *testing.T) { + tmpDir := t.TempDir() + + // First call: directory does not exist yet. + dir1 := store.FindOrCreateCardDirectory(tmpDir, "хляб") + if dir1 == "" || dir1 == tmpDir { + t.Fatalf("expected a new card directory, got %q", dir1) + } + + // word.txt must have been written. + data, err := os.ReadFile(filepath.Join(dir1, "word.txt")) + if err != nil { + t.Fatalf("word.txt not created: %v", err) + } + if string(data) != "хляб" { + t.Errorf("word.txt content = %q; want %q", string(data), "хляб") + } + + // Second call: must return the same directory. + dir2 := store.FindOrCreateCardDirectory(tmpDir, "хляб") + if dir2 != dir1 { + t.Errorf("second call returned %q; want %q", dir2, dir1) + } +} + +// TestCardStoreScanWords verifies that ScanWords returns only words from +// directories that pass the predicate and ignores hidden directories. +func TestCardStoreScanWords(t *testing.T) { + tmpDir := t.TempDir() + + // Helper: create a card directory with word.txt. + makeCard := func(id, word string) string { + cardDir := filepath.Join(tmpDir, id) + _ = os.MkdirAll(cardDir, 0755) + _ = os.WriteFile(filepath.Join(cardDir, "word.txt"), []byte(word), 0644) + return cardDir + } + + dir1 := makeCard("card1", "котка") + makeCard("card2", "куче") + + // Hidden directory must be skipped. + makeCard(".hidden", "hidden") + + // ScanWords with a predicate that only passes dir1. + cs := store.New(tmpDir) + words := cs.ScanWords(func(wordDir string) bool { + return wordDir == dir1 + }) + + if len(words) != 1 || words[0] != "котка" { + t.Errorf("ScanWords = %v; want [котка]", words) + } + + // ScanWords with nil predicate must return all non-hidden words. + allWords := cs.ScanWords(nil) + if len(allWords) != 2 { + t.Errorf("ScanWords(nil) = %v; want 2 words", allWords) + } +} diff --git a/internal/utils.go b/internal/utils.go index b83d46a..e16e10e 100644 --- a/internal/utils.go +++ b/internal/utils.go @@ -1,85 +1,32 @@ package internal import ( - "crypto/md5" - "encoding/hex" - "fmt" - "os" - "path/filepath" "strings" - "time" + + "codeberg.org/snonux/totalrecall/internal/store" ) -// GenerateCardID creates a unique ID for a card based on timestamp and Bulgarian word +// GenerateCardID creates a unique ID for a card based on timestamp and Bulgarian word. +// Delegates to store.GenerateCardID which is the single source of truth. // Format: epochMillis_md5(word)[:8] func GenerateCardID(bulgarianWord string) string { - // Get current timestamp in milliseconds - now := time.Now() - epochMillis := now.UnixNano() / 1000000 - - // Calculate MD5 hash of the word - hash := md5.Sum([]byte(bulgarianWord)) - hashStr := hex.EncodeToString(hash[:])[:8] // Use first 8 chars of MD5 - - // Combine timestamp and hash - return fmt.Sprintf("%d_%s", epochMillis, hashStr) + return store.GenerateCardID(bulgarianWord) } // FindCardDirectory searches outputDir for a subdirectory whose word.txt // (or legacy _word.txt) matches the given word. Returns the directory path // or an empty string if not found. +// Delegates to store.FindCardDirectory which is the single source of truth. func FindCardDirectory(outputDir, word string) string { - entries, err := os.ReadDir(outputDir) - if err != nil { - return "" - } - - for _, entry := range entries { - if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { - continue - } - - dirPath := filepath.Join(outputDir, entry.Name()) - wordFile := filepath.Join(dirPath, "word.txt") - - if data, err := os.ReadFile(wordFile); err == nil { - if strings.TrimSpace(string(data)) == word { - return dirPath - } - } else { - // Backward-compatible fallback: old format used _word.txt - wordFile = filepath.Join(dirPath, "_word.txt") - if data, err := os.ReadFile(wordFile); err == nil { - if strings.TrimSpace(string(data)) == word { - return dirPath - } - } - } - } - - return "" + return store.FindCardDirectory(outputDir, word) } // FindOrCreateCardDirectory returns the existing card directory for word inside // outputDir, or creates a new one with a generated card ID. It also writes // word.txt so subsequent calls can find the directory. +// Delegates to store.FindOrCreateCardDirectory which is the single source of truth. func FindOrCreateCardDirectory(outputDir, word string) string { - if dir := FindCardDirectory(outputDir, word); dir != "" { - return dir - } - - cardID := GenerateCardID(word) - wordDir := filepath.Join(outputDir, cardID) - if err := os.MkdirAll(wordDir, 0755); err != nil { - fmt.Printf("Warning: failed to create word directory: %v\n", err) - return outputDir - } - - if err := os.WriteFile(filepath.Join(wordDir, "word.txt"), []byte(word), 0644); err != nil { - fmt.Printf("Warning: failed to save word metadata: %v\n", err) - } - - return wordDir + return store.FindOrCreateCardDirectory(outputDir, word) } // SanitizeFilename creates a safe filename from a string. @@ -97,7 +44,7 @@ func SanitizeFilename(s string) string { return b.String() } -// isAlphaNumeric checks if a rune is alphanumeric +// isAlphaNumeric checks if a rune is alphanumeric (Latin or Cyrillic). func isAlphaNumeric(r rune) bool { return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || (r >= 'а' && r <= 'я') || -- cgit v1.2.3