From 559bf930732291e0ffb010d2889117ec6008c691 Mon Sep 17 00:00:00 2001 From: Thibaut Etienne Date: Thu, 19 Mar 2026 12:33:01 +0100 Subject: [PATCH] feat(cli): add --cache flag to phrase pull for conditional requests Store ETag and Last-Modified response headers locally and send them as If-None-Match / If-Modified-Since on subsequent pulls. When the server returns 304 Not Modified, the locale file is skipped. Cache is stored at os.UserCacheDir()/phrase/download_cache.json. Only supported in sync mode; --cache with --async prints a warning and falls back to normal behavior. Fixes phrase/phrase-cli#164 --- clients/cli/cmd/internal/download_cache.go | 152 +++++++++++ .../cli/cmd/internal/download_cache_test.go | 256 ++++++++++++++++++ clients/cli/cmd/internal/pull.go | 80 +++++- clients/cli/cmd/pull.go | 2 + clients/cli/go.sum | 2 + 5 files changed, 481 insertions(+), 11 deletions(-) create mode 100644 clients/cli/cmd/internal/download_cache.go create mode 100644 clients/cli/cmd/internal/download_cache_test.go diff --git a/clients/cli/cmd/internal/download_cache.go b/clients/cli/cmd/internal/download_cache.go new file mode 100644 index 000000000..4efc76e38 --- /dev/null +++ b/clients/cli/cmd/internal/download_cache.go @@ -0,0 +1,152 @@ +package internal + +import ( + "crypto/sha256" + "encoding/json" + "fmt" + "os" + "path/filepath" + "reflect" + + "github.com/antihax/optional" + "github.com/phrase/phrase-go/v4" +) + +const ( + cacheVersion = 1 + cacheFileName = "download_cache.json" + cacheDirName = "phrase" +) + +type CacheEntry struct { + ETag string `json:"etag,omitempty"` + LastModified string `json:"last_modified,omitempty"` +} + +type DownloadCache struct { + Version int `json:"version"` + Entries map[string]CacheEntry `json:"entries"` + path string + dirty bool +} + +func LoadDownloadCache() *DownloadCache { + return loadFromPath(cachePath()) +} + +func loadFromPath(path string) *DownloadCache { + dc := &DownloadCache{ + Version: cacheVersion, + Entries: make(map[string]CacheEntry), + path: path, + } + + data, err := os.ReadFile(path) + if err != nil { + if !os.IsNotExist(err) { + fmt.Fprintf(os.Stderr, "Warning: could not read download cache %s: %v\n", path, err) + } + return dc + } + + var loaded DownloadCache + if err := json.Unmarshal(data, &loaded); err != nil { + fmt.Fprintf(os.Stderr, "Warning: corrupt download cache %s, starting fresh\n", path) + return dc + } + if loaded.Version != cacheVersion { + return dc + } + loaded.path = path + if loaded.Entries == nil { + loaded.Entries = make(map[string]CacheEntry) + } + return &loaded +} + +func (dc *DownloadCache) Get(key string) (CacheEntry, bool) { + e, ok := dc.Entries[key] + return e, ok +} + +func (dc *DownloadCache) Set(key string, entry CacheEntry) { + dc.Entries[key] = entry + dc.dirty = true +} + +func (dc *DownloadCache) Save() error { + if !dc.dirty { + return nil + } + if err := os.MkdirAll(filepath.Dir(dc.path), 0o700); err != nil { + return err + } + data, err := json.Marshal(dc) + if err != nil { + return err + } + if err := os.WriteFile(dc.path, data, 0o600); err != nil { + return err + } + dc.dirty = false + return nil +} + +// CacheKey builds a deterministic key by hashing the full download parameters. +// It uses reflection to extract actual values from optional fields since +// antihax/optional types don't serialize meaningfully via json.Marshal. +func CacheKey(projectID, localeID string, opts phrase.LocaleDownloadOpts) string { + // Zero out conditional request fields so they don't affect the key. + opts.IfNoneMatch = optional.String{} + opts.IfModifiedSince = optional.String{} + + raw := fmt.Sprintf("%s/%s/%s", projectID, localeID, serializeOpts(opts)) + h := sha256.Sum256([]byte(raw)) + return fmt.Sprintf("%x", h[:12]) +} + +// serializeOpts extracts set values from optional fields into a deterministic map. +// It assumes all fields in LocaleDownloadOpts are either slices or antihax/optional +// types with IsSet()/Value() methods. Fields with other types are silently excluded. +func serializeOpts(opts phrase.LocaleDownloadOpts) string { + v := reflect.ValueOf(opts) + t := v.Type() + m := make(map[string]interface{}) + + for i := 0; i < t.NumField(); i++ { + field := v.Field(i) + name := t.Field(i).Name + + // Handle slices directly + if field.Kind() == reflect.Slice { + if field.Len() > 0 { + m[name] = field.Interface() + } + continue + } + + // For optional types, check IsSet and extract Value + isSetMethod := field.MethodByName("IsSet") + valueMethod := field.MethodByName("Value") + if isSetMethod.IsValid() && valueMethod.IsValid() { + results := isSetMethod.Call(nil) + if len(results) > 0 && results[0].Bool() { + m[name] = valueMethod.Call(nil)[0].Interface() + } + } + } + + data, err := json.Marshal(m) + if err != nil { + return fmt.Sprintf("%v", m) + } + return string(data) +} + +func cachePath() string { + dir, err := os.UserCacheDir() + if err != nil { + dir = os.TempDir() + } + return filepath.Join(dir, cacheDirName, cacheFileName) +} diff --git a/clients/cli/cmd/internal/download_cache_test.go b/clients/cli/cmd/internal/download_cache_test.go new file mode 100644 index 000000000..32875acbb --- /dev/null +++ b/clients/cli/cmd/internal/download_cache_test.go @@ -0,0 +1,256 @@ +package internal + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/antihax/optional" + "github.com/phrase/phrase-go/v4" +) + +func TestLoadFromPath_Missing(t *testing.T) { + dc := loadFromPath("/nonexistent/path/cache.json") + if dc == nil { + t.Fatal("expected non-nil cache") + } + if dc.Version != cacheVersion { + t.Errorf("expected version %d, got %d", cacheVersion, dc.Version) + } + if len(dc.Entries) != 0 { + t.Errorf("expected empty entries, got %d", len(dc.Entries)) + } +} + +func TestLoadFromPath_Corrupt(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, cacheFileName) + os.WriteFile(path, []byte("not json"), 0o600) + + dc := loadFromPath(path) + if dc.Version != cacheVersion { + t.Errorf("expected version %d, got %d", cacheVersion, dc.Version) + } + if len(dc.Entries) != 0 { + t.Errorf("expected empty entries on corrupt file, got %d", len(dc.Entries)) + } +} + +func TestLoadFromPath_VersionMismatch(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, cacheFileName) + data, _ := json.Marshal(DownloadCache{ + Version: cacheVersion + 99, + Entries: map[string]CacheEntry{"key": {ETag: "old"}}, + }) + os.WriteFile(path, data, 0o600) + + dc := loadFromPath(path) + if dc.Version != cacheVersion { + t.Errorf("expected fresh cache with version %d, got %d", cacheVersion, dc.Version) + } + if len(dc.Entries) != 0 { + t.Errorf("expected empty entries on version mismatch, got %d", len(dc.Entries)) + } +} + +func TestLoadFromPath_Valid(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, cacheFileName) + data, _ := json.Marshal(DownloadCache{ + Version: cacheVersion, + Entries: map[string]CacheEntry{ + "k1": {ETag: `"e1"`, LastModified: "mod1"}, + }, + }) + os.WriteFile(path, data, 0o600) + + dc := loadFromPath(path) + if dc.Version != cacheVersion { + t.Errorf("expected version %d, got %d", cacheVersion, dc.Version) + } + entry, ok := dc.Get("k1") + if !ok { + t.Fatal("expected entry k1 to exist") + } + if entry.ETag != `"e1"` || entry.LastModified != "mod1" { + t.Errorf("unexpected entry: %+v", entry) + } +} + +func TestDownloadCache_RoundTrip(t *testing.T) { + dc := &DownloadCache{ + Version: cacheVersion, + Entries: make(map[string]CacheEntry), + } + + dc.Set("abc", CacheEntry{ETag: `"etag123"`, LastModified: "Thu, 01 Jan 2025 00:00:00 GMT"}) + + entry, ok := dc.Get("abc") + if !ok { + t.Fatal("expected entry to exist") + } + if entry.ETag != `"etag123"` { + t.Errorf("expected ETag %q, got %q", `"etag123"`, entry.ETag) + } + if entry.LastModified != "Thu, 01 Jan 2025 00:00:00 GMT" { + t.Errorf("unexpected LastModified: %s", entry.LastModified) + } + + _, ok = dc.Get("missing") + if ok { + t.Error("expected missing key to return false") + } +} + +func TestDownloadCache_SaveLoad(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "sub", cacheFileName) + + dc := &DownloadCache{ + Version: cacheVersion, + Entries: make(map[string]CacheEntry), + path: path, + } + dc.Set("key1", CacheEntry{ETag: `"e1"`, LastModified: "mod1"}) + dc.Set("key2", CacheEntry{ETag: `"e2"`}) + + if err := dc.Save(); err != nil { + t.Fatalf("Save failed: %v", err) + } + + loaded := loadFromPath(path) + if loaded.Version != cacheVersion { + t.Errorf("expected version %d, got %d", cacheVersion, loaded.Version) + } + if len(loaded.Entries) != 2 { + t.Errorf("expected 2 entries, got %d", len(loaded.Entries)) + } + if loaded.Entries["key1"].ETag != `"e1"` { + t.Errorf("unexpected ETag for key1: %s", loaded.Entries["key1"].ETag) + } + if loaded.Entries["key1"].LastModified != "mod1" { + t.Errorf("unexpected LastModified for key1: %s", loaded.Entries["key1"].LastModified) + } + if loaded.Entries["key2"].ETag != `"e2"` { + t.Errorf("unexpected ETag for key2: %s", loaded.Entries["key2"].ETag) + } +} + +func TestDownloadCache_SaveSkipsWhenClean(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, cacheFileName) + + dc := &DownloadCache{ + Version: cacheVersion, + Entries: make(map[string]CacheEntry), + path: path, + } + + if err := dc.Save(); err != nil { + t.Fatalf("Save failed: %v", err) + } + + if _, err := os.Stat(path); err == nil { + t.Error("expected no file to be written when cache is clean") + } +} + +func TestDownloadCache_ETagOnly(t *testing.T) { + dc := &DownloadCache{ + Version: cacheVersion, + Entries: make(map[string]CacheEntry), + } + dc.Set("k", CacheEntry{ETag: `"abc"`}) + + entry, ok := dc.Get("k") + if !ok { + t.Fatal("expected entry") + } + if entry.ETag != `"abc"` { + t.Errorf("unexpected ETag: %s", entry.ETag) + } + if entry.LastModified != "" { + t.Errorf("expected empty LastModified, got: %s", entry.LastModified) + } +} + +func TestCacheKey_Deterministic(t *testing.T) { + opts := phrase.LocaleDownloadOpts{ + FileFormat: optional.NewString("json"), + Tags: optional.NewString("web"), + } + + k1 := CacheKey("proj1", "locale1", opts) + k2 := CacheKey("proj1", "locale1", opts) + + if k1 != k2 { + t.Errorf("expected deterministic keys, got %s and %s", k1, k2) + } +} + +func TestCacheKey_ZeroOpts(t *testing.T) { + k := CacheKey("proj", "locale", phrase.LocaleDownloadOpts{}) + if k == "" { + t.Error("expected non-empty key for zero opts") + } + if len(k) != 24 { + t.Errorf("expected 24-char hex key, got %d chars: %s", len(k), k) + } +} + +func TestCacheKey_DifferentParams(t *testing.T) { + opts1 := phrase.LocaleDownloadOpts{ + FileFormat: optional.NewString("json"), + } + opts2 := phrase.LocaleDownloadOpts{ + FileFormat: optional.NewString("yml"), + } + + k1 := CacheKey("proj1", "locale1", opts1) + k2 := CacheKey("proj1", "locale1", opts2) + + if k1 == k2 { + t.Error("expected different keys for different file formats") + } + + // Different project + k3 := CacheKey("proj2", "locale1", opts1) + if k1 == k3 { + t.Error("expected different keys for different projects") + } + + // Different locale + k4 := CacheKey("proj1", "locale2", opts1) + if k1 == k4 { + t.Error("expected different keys for different locales") + } +} + +func TestCacheKey_IgnoresConditionalHeaders(t *testing.T) { + base := phrase.LocaleDownloadOpts{ + FileFormat: optional.NewString("json"), + } + + withETag := phrase.LocaleDownloadOpts{ + FileFormat: optional.NewString("json"), + IfNoneMatch: optional.NewString(`"abc123"`), + } + + withLastMod := phrase.LocaleDownloadOpts{ + FileFormat: optional.NewString("json"), + IfModifiedSince: optional.NewString("Thu, 01 Jan 2025 00:00:00 GMT"), + } + + kBase := CacheKey("p", "l", base) + kETag := CacheKey("p", "l", withETag) + kLastMod := CacheKey("p", "l", withLastMod) + + if kBase != kETag { + t.Errorf("IfNoneMatch should not affect cache key: %s vs %s", kBase, kETag) + } + if kBase != kLastMod { + t.Errorf("IfModifiedSince should not affect cache key: %s vs %s", kBase, kLastMod) + } +} diff --git a/clients/cli/cmd/internal/pull.go b/clients/cli/cmd/internal/pull.go index 24ca0b675..f33d97446 100644 --- a/clients/cli/cmd/internal/pull.go +++ b/clients/cli/cmd/internal/pull.go @@ -3,6 +3,7 @@ package internal import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -26,13 +27,17 @@ const ( asyncRetryCount = 360 // 30 minutes ) -var Config *phrase.Config +var ( + Config *phrase.Config + errNotModified = errors.New("not modified") +) type PullCommand struct { phrase.Config Branch string UseLocalBranchName bool Async bool + Cache bool } var Auth context.Context @@ -81,8 +86,22 @@ func (cmd *PullCommand) Run(config *phrase.Config) error { target.RemoteLocales = val } + var cache *DownloadCache + if cmd.Cache { + if cmd.Async { + print.Warn("--cache is not supported with --async, ignoring cache") + } else { + cache = LoadDownloadCache() + defer func() { + if err := cache.Save(); err != nil { + print.Warn("Failed to save download cache: %s. Check directory permissions.", err) + } + }() + } + } + for _, target := range targets { - err := target.Pull(client, cmd.Async) + err := target.Pull(client, cmd.Async, cache) if err != nil { return err } @@ -110,7 +129,7 @@ type PullParams struct { LocaleID string `json:"locale_id"` } -func (target *Target) Pull(client *phrase.APIClient, async bool) error { +func (target *Target) Pull(client *phrase.APIClient, async bool, cache *DownloadCache) error { if err := target.CheckPreconditions(); err != nil { return err } @@ -131,8 +150,10 @@ func (target *Target) Pull(client *phrase.APIClient, async bool) error { return err } - err = target.DownloadAndWriteToFile(client, localeFile, async) - if err != nil { + err = target.DownloadAndWriteToFile(client, localeFile, async, cache) + if errors.Is(err, errNotModified) { + print.Success("Not modified %s", localeFile.RelPath()) + } else if err != nil { if openapiError, ok := err.(phrase.GenericOpenAPIError); ok { print.Warn("API response: %s", openapiError.Body()) } @@ -146,7 +167,7 @@ func (target *Target) Pull(client *phrase.APIClient, async bool) error { return nil } -func (target *Target) DownloadAndWriteToFile(client *phrase.APIClient, localeFile *LocaleFile, async bool) error { +func (target *Target) DownloadAndWriteToFile(client *phrase.APIClient, localeFile *LocaleFile, async bool, cache *DownloadCache) error { localVarOptionals := phrase.LocaleDownloadOpts{} if target.Params != nil { @@ -182,9 +203,8 @@ func (target *Target) DownloadAndWriteToFile(client *phrase.APIClient, localeFil if async { return target.downloadAsynchronously(client, localeFile, localVarOptionals) - } else { - return target.downloadSynchronously(client, localeFile, localVarOptionals) } + return target.downloadSynchronously(client, localeFile, localVarOptionals, cache) } func (target *Target) downloadAsynchronously(client *phrase.APIClient, localeFile *LocaleFile, downloadOpts phrase.LocaleDownloadOpts) error { @@ -216,12 +236,40 @@ func (target *Target) downloadAsynchronously(client *phrase.APIClient, localeFil return fmt.Errorf("download failed: %s", asyncDownload.Error) } -func (target *Target) downloadSynchronously(client *phrase.APIClient, localeFile *LocaleFile, downloadOpts phrase.LocaleDownloadOpts) error { +func (target *Target) downloadSynchronously(client *phrase.APIClient, localeFile *LocaleFile, downloadOpts phrase.LocaleDownloadOpts, cache *DownloadCache) error { + // Compute cache key before mutating opts + var cacheKey string + if cache != nil { + cacheKey = CacheKey(target.ProjectID, localeFile.ID, downloadOpts) + if entry, ok := cache.Get(cacheKey); ok { + debugFprintln("Cache hit for", localeFile.ID, "- sending conditional request") + if entry.ETag != "" { + downloadOpts.IfNoneMatch = optional.NewString(entry.ETag) + } + if entry.LastModified != "" { + downloadOpts.IfModifiedSince = optional.NewString(entry.LastModified) + } + } else { + debugFprintln("Cache miss for", localeFile.ID) + } + } + file, response, err := client.LocalesApi.LocaleDownload(Auth, target.ProjectID, localeFile.ID, &downloadOpts) + + // The SDK treats status >= 300 as an error, including 304 Not Modified. + // Check response.StatusCode before err to intercept the 304 case. + if response != nil && response.StatusCode == 304 { + debugFprintln("Not modified (304), skipping", localeFile.Path) + return errNotModified + } + if err != nil { - if response.Rate.Remaining == 0 { + if response != nil && response.Rate.Remaining == 0 { waitForRateLimit(response.Rate) - file, _, err = client.LocalesApi.LocaleDownload(Auth, target.ProjectID, localeFile.ID, &downloadOpts) + // Strip conditional headers on retry to get a full response + downloadOpts.IfNoneMatch = optional.String{} + downloadOpts.IfModifiedSince = optional.String{} + file, response, err = client.LocalesApi.LocaleDownload(Auth, target.ProjectID, localeFile.ID, &downloadOpts) if err != nil { return err } @@ -229,6 +277,16 @@ func (target *Target) downloadSynchronously(client *phrase.APIClient, localeFile return err } } + + // Update cache from response headers + if cache != nil && response != nil { + etag := response.Header.Get("ETag") + lastMod := response.Header.Get("Last-Modified") + if etag != "" || lastMod != "" { + cache.Set(cacheKey, CacheEntry{ETag: etag, LastModified: lastMod}) + } + } + return copyToDestination(file, localeFile.Path) } diff --git a/clients/cli/cmd/pull.go b/clients/cli/cmd/pull.go index 83d759231..3511cca51 100644 --- a/clients/cli/cmd/pull.go +++ b/clients/cli/cmd/pull.go @@ -21,6 +21,7 @@ func initPull() { Branch: params.GetString("branch"), UseLocalBranchName: params.GetBool("use-local-branch-name"), Async: params.GetBool("async"), + Cache: params.GetBool("cache"), } err := cmdPull.Run(Config) if err != nil { @@ -33,5 +34,6 @@ func initPull() { AddFlag(pullCmd, "string", "branch", "b", "branch", false) AddFlag(pullCmd, "bool", "use-local-branch-name", "", "use local branch name", false) AddFlag(pullCmd, "bool", "async", "a", "use asynchronous locale downloads (recommended for large number of keys)", false) + AddFlag(pullCmd, "bool", "cache", "", "cache ETags locally to skip unchanged downloads (sync mode only)", false) params.BindPFlags(pullCmd.Flags()) } diff --git a/clients/cli/go.sum b/clients/cli/go.sum index e4552cef1..6b1a9c91a 100644 --- a/clients/cli/go.sum +++ b/clients/cli/go.sum @@ -219,6 +219,8 @@ github.com/phrase/phrase-go/v4 v4.18.1 h1:y1sv4z8ufEQB+kJA8ymSiH8nRAvH8gGoVSB5/7 github.com/phrase/phrase-go/v4 v4.18.1/go.mod h1:4XplKvrbHS2LDaXfFp9xrVDtO5xk2WHFm0htutwwd8c= github.com/phrase/phrase-go/v4 v4.19.0 h1:tNliCxO/0SMu2viLE9idzADBUoY9C6CqrDmp3ntgpQI= github.com/phrase/phrase-go/v4 v4.19.0/go.mod h1:4XplKvrbHS2LDaXfFp9xrVDtO5xk2WHFm0htutwwd8c= +github.com/phrase/phrase-go/v4 v4.20.0 h1:UTgos4elHzv83XbC6hK2ulDVEvvi1215r8NzE8jC3bc= +github.com/phrase/phrase-go/v4 v4.20.0/go.mod h1:4XplKvrbHS2LDaXfFp9xrVDtO5xk2WHFm0htutwwd8c= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=