diff --git a/clients/cli/cmd/internal/download_cache.go b/clients/cli/cmd/internal/download_cache.go index 4efc76e3..c3b7adc8 100644 --- a/clients/cli/cmd/internal/download_cache.go +++ b/clients/cli/cmd/internal/download_cache.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "reflect" + "sync" "github.com/antihax/optional" "github.com/phrase/phrase-go/v4" @@ -28,6 +29,7 @@ type DownloadCache struct { Entries map[string]CacheEntry `json:"entries"` path string dirty bool + mu sync.Mutex `json:"-"` } func LoadDownloadCache() *DownloadCache { @@ -65,16 +67,22 @@ func loadFromPath(path string) *DownloadCache { } func (dc *DownloadCache) Get(key string) (CacheEntry, bool) { + dc.mu.Lock() + defer dc.mu.Unlock() e, ok := dc.Entries[key] return e, ok } func (dc *DownloadCache) Set(key string, entry CacheEntry) { + dc.mu.Lock() + defer dc.mu.Unlock() dc.Entries[key] = entry dc.dirty = true } func (dc *DownloadCache) Save() error { + dc.mu.Lock() + defer dc.mu.Unlock() if !dc.dirty { return nil } @@ -133,6 +141,9 @@ func serializeOpts(opts phrase.LocaleDownloadOpts) string { if len(results) > 0 && results[0].Bool() { m[name] = valueMethod.Call(nil)[0].Interface() } + } else if !field.IsZero() { + // plain field (string, bool, int, etc.) — include if non-zero + m[name] = field.Interface() } } diff --git a/clients/cli/cmd/internal/pull.go b/clients/cli/cmd/internal/pull.go index 216cf5de..b609eeea 100644 --- a/clients/cli/cmd/internal/pull.go +++ b/clients/cli/cmd/internal/pull.go @@ -105,14 +105,14 @@ func (cmd *PullCommand) Run(config *phrase.Config) error { } } + if cmd.Parallel && cmd.Async { + print.Warn("--parallel is not supported with --async, ignoring parallel") + } for _, target := range targets { var err error if cmd.Parallel && !cmd.Async { - err = target.PullParallel(client) + err = target.PullParallel(client, cache) } else { - if cmd.Parallel && cmd.Async { - print.Warn("--parallel is not supported with --async, ignoring parallel") - } err = target.Pull(client, cmd.Async, cache) } if err != nil { @@ -181,30 +181,16 @@ func (target *Target) Pull(client *phrase.APIClient, async bool, cache *Download } type downloadResult struct { - message string - path string - errMsg string + message string + path string + errMsg string + notModified bool } func (target *Target) DownloadAndWriteToFile(client *phrase.APIClient, localeFile *LocaleFile, async bool, cache *DownloadCache) error { - localVarOptionals := phrase.LocaleDownloadOpts{} - - if target.Params != nil { - localVarOptionals = target.Params.LocaleDownloadOpts - translationKeyPrefix, err := placeholders.ResolveTranslationKeyPrefix(target.Params.TranslationKeyPrefix, localeFile.Path) - if err != nil { - return err - } - localVarOptionals.TranslationKeyPrefix = translationKeyPrefix - } - - if localVarOptionals.FileFormat.Value() == "" { - localVarOptionals.FileFormat = optional.NewString(localeFile.FileFormat) - } - - if localeFile.Tag != "" { - localVarOptionals.Tags = optional.NewString(localeFile.Tag) - localVarOptionals.Tag = optional.EmptyString() + localVarOptionals, err := target.buildDownloadOpts(localeFile) + if err != nil { + return err } debugFprintln("Target file pattern:", target.File) @@ -226,7 +212,7 @@ func (target *Target) DownloadAndWriteToFile(client *phrase.APIClient, localeFil return target.downloadSynchronously(client, localeFile, localVarOptionals, cache) } -func (target *Target) PullParallel(client *phrase.APIClient) error { +func (target *Target) PullParallel(client *phrase.APIClient, cache *DownloadCache) error { if err := target.CheckPreconditions(); err != nil { return err } @@ -264,11 +250,34 @@ func (target *Target) PullParallel(client *phrase.APIClient) error { return err } - err = target.downloadWithRateGate(client, lf, opts, &rateMu) - if err != nil { - if openapiError, ok := err.(phrase.GenericOpenAPIError); ok { + var cacheKey string + if cache != nil { + cacheKey = CacheKey(target.ProjectID, lf.ID, opts) + applyCacheHeaders(cache, cacheKey, &opts) + } + + file, response, dlErr := target.downloadWithRateGate(client, lf, opts, &rateMu) + + if response != nil && response.StatusCode == 304 { + debugFprintln("Not modified (304), skipping", lf.Path) + results[i] = downloadResult{message: lf.Message(), path: lf.RelPath(), notModified: true} + return nil + } + + if dlErr != nil { + if openapiError, ok := dlErr.(phrase.GenericOpenAPIError); ok { print.Warn("API response: %s", openapiError.Body()) } + dlErr = fmt.Errorf("%s for %s", dlErr, lf.Path) + results[i] = downloadResult{errMsg: dlErr.Error()} + return dlErr + } + + if cache != nil { + updateCache(cache, cacheKey, response) + } + + if err := copyToDestination(file, lf.Path); err != nil { err = fmt.Errorf("%s for %s", err, lf.Path) results[i] = downloadResult{errMsg: err.Error()} return err @@ -285,19 +294,16 @@ func (target *Target) PullParallel(client *phrase.APIClient) error { waitErr := g.Wait() // Print results in original order: successes and failures - var skipCount int for _, r := range results { - if r.path != "" { + switch { + case r.notModified && r.path != "": + print.Success("Not modified %s", r.path) + case r.path != "": print.Success("Downloaded %s to %s", r.message, r.path) - } else if r.errMsg != "" { + case r.errMsg != "": print.Failure("Failed %s", r.errMsg) - } else { - skipCount++ } } - if skipCount > 0 { - print.Warn("%d download(s) skipped due to earlier failure", skipCount) - } return waitErr } @@ -305,34 +311,33 @@ func (target *Target) PullParallel(client *phrase.APIClient) error { // downloadWithRateGate downloads a locale file with rate-limit coordination. // Uses RWMutex as a broadcast gate: workers take a read lock (cheap, concurrent), // and a rate-limited worker takes the write lock to pause everyone until reset. -func (target *Target) downloadWithRateGate(client *phrase.APIClient, localeFile *LocaleFile, opts phrase.LocaleDownloadOpts, gate *sync.RWMutex) error { - // Read-lock gate: blocks only when a writer (rate-limited worker) holds it - gate.RLock() - gate.RUnlock() +func (target *Target) downloadWithRateGate(client *phrase.APIClient, localeFile *LocaleFile, opts phrase.LocaleDownloadOpts, gate *sync.RWMutex) (*os.File, *phrase.APIResponse, error) { + waitForGate := func() { gate.RLock(); gate.RUnlock() } + waitForGate() file, response, err := client.LocalesApi.LocaleDownload(Auth, target.ProjectID, localeFile.ID, &opts) if err != nil { + if response != nil && response.StatusCode == 304 { + return nil, response, nil + } if response != nil && response.Rate.Remaining == 0 { - // TryLock ensures only one worker handles the rate limit pause. - // Others will block on their next RLock until the pause is over. if gate.TryLock() { waitForRateLimit(response.Rate) gate.Unlock() } else { - // Another worker is already pausing; wait for it - gate.RLock() - gate.RUnlock() + waitForGate() } - - file, _, err = client.LocalesApi.LocaleDownload(Auth, target.ProjectID, localeFile.ID, &opts) + opts.IfNoneMatch = optional.String{} + opts.IfModifiedSince = optional.String{} + file, response, err = client.LocalesApi.LocaleDownload(Auth, target.ProjectID, localeFile.ID, &opts) if err != nil { - return err + return nil, response, err } } else { - return err + return nil, response, err } } - return copyToDestination(file, localeFile.Path) + return file, response, nil } // buildDownloadOpts prepares the LocaleDownloadOpts for a locale file download. @@ -370,7 +375,7 @@ func (target *Target) downloadAsynchronously(client *phrase.APIClient, localeFil return err } - for i := 0; asyncDownload.Status == "processing"; i++ { + for i := 0; asyncDownload.Status == "processing" && i <= asyncRetryCount; i++ { debugFprintln("Waiting for the files to be exported...") time.Sleep(asyncWaitTime) debugFprintln("Checking if the download is ready...") @@ -379,9 +384,9 @@ func (target *Target) downloadAsynchronously(client *phrase.APIClient, localeFil if err != nil { return err } - if i > asyncRetryCount { - return fmt.Errorf("download is taking too long") - } + } + if asyncDownload.Status == "processing" { + return fmt.Errorf("download is taking too long") } if asyncDownload.Status == "completed" { return downloadExportedLocale(asyncDownload.Result.Url, localeFile.Path) @@ -389,34 +394,43 @@ func (target *Target) downloadAsynchronously(client *phrase.APIClient, localeFil return fmt.Errorf("download failed: %s", asyncDownload.Error) } +func applyCacheHeaders(cache *DownloadCache, cacheKey string, opts *phrase.LocaleDownloadOpts) { + if entry, ok := cache.Get(cacheKey); ok { + debugFprintln("Cache hit for", cacheKey, "- sending conditional request") + if entry.ETag != "" { + opts.IfNoneMatch = optional.NewString(entry.ETag) + } + if entry.LastModified != "" { + opts.IfModifiedSince = optional.NewString(entry.LastModified) + } + } else { + debugFprintln("Cache miss for", cacheKey) + } +} + +func updateCache(cache *DownloadCache, cacheKey string, response *phrase.APIResponse) { + if response == nil { + return + } + etag, lastMod := response.Header.Get("ETag"), response.Header.Get("Last-Modified") + if etag != "" || lastMod != "" { + cache.Set(cacheKey, CacheEntry{ETag: etag, LastModified: lastMod}) + } +} + 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) - } + applyCacheHeaders(cache, cacheKey, &downloadOpts) } 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 != nil && response.StatusCode == 304 { + debugFprintln("Not modified (304), skipping", localeFile.Path) + return errNotModified + } if response != nil && response.Rate.Remaining == 0 { waitForRateLimit(response.Rate) // Strip conditional headers on retry to get a full response @@ -431,30 +445,25 @@ func (target *Target) downloadSynchronously(client *phrase.APIClient, localeFile } } - // 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}) - } + if cache != nil { + updateCache(cache, cacheKey, response) } return copyToDestination(file, localeFile.Path) } func copyToDestination(file *os.File, path string) error { + if file == nil { + return fmt.Errorf("no content to write to %s", path) + } + defer file.Close() destFile, err := os.Create(path) if err != nil { return err } defer destFile.Close() - if file != nil { - defer file.Close() - _, err = io.Copy(destFile, file) - return err - } - return nil + _, err = io.Copy(destFile, file) + return err } func downloadExportedLocale(url string, localName string) error { @@ -474,8 +483,8 @@ func downloadExportedLocale(url string, localName string) error { return err } defer response.Body.Close() - io.Copy(file, response.Body) - return nil + _, err = io.Copy(file, response.Body) + return err } // asyncDownloadParams converts the optional parameters from the Pull command into a LocaleDownloadCreateParameters struct. diff --git a/clients/cli/go.sum b/clients/cli/go.sum index 76a512db..6960c6ae 100644 --- a/clients/cli/go.sum +++ b/clients/cli/go.sum @@ -215,12 +215,8 @@ github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pelletier/go-toml v1.2.0 h1:T5zMGML61Wp+FlcbWjRDT7yAxhJNAiPPLOFECq181zc= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= -github.com/phrase/phrase-go/v4 v4.18.1 h1:y1sv4z8ufEQB+kJA8ymSiH8nRAvH8gGoVSB5/7jvYEQ= -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/phrase/phrase-go/v4 v4.21.0 h1:rYTuU6126RHpu0NPI6Owe9pJOpJmYXEY2bbFlHtZ/+8= +github.com/phrase/phrase-go/v4 v4.21.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=