diff --git a/internal/asc/assets_output.go b/internal/asc/assets_output.go index 81da35c3c..c4f9e3050 100644 --- a/internal/asc/assets_output.go +++ b/internal/asc/assets_output.go @@ -93,11 +93,12 @@ type AppScreenshotFanoutUploadResult struct { // AppPreviewUploadResult represents preview upload output. type AppPreviewUploadResult struct { - VersionLocalizationID string `json:"versionLocalizationId"` - SetID string `json:"setId"` - PreviewType string `json:"previewType"` - DryRun bool `json:"dryRun,omitempty"` - Results []AssetUploadResultItem `json:"results"` + VersionLocalizationID string `json:"versionLocalizationId"` + SetID string `json:"setId"` + PreviewType string `json:"previewType"` + DryRun bool `json:"dryRun,omitempty"` + Results []AssetUploadResultItem `json:"results"` + Failures []AssetUploadFailureItem `json:"failures,omitempty"` } // CustomProductPageScreenshotUploadResult represents custom product page screenshot upload output. diff --git a/internal/asc/output_registry_init.go b/internal/asc/output_registry_init.go index 8e59b4323..67415e7de 100644 --- a/internal/asc/output_registry_init.go +++ b/internal/asc/output_registry_init.go @@ -246,6 +246,10 @@ func registerAllOutputRenderers() { ih, ir := assetUploadResultItemRows(v.Results) render(ih, ir) } + if len(v.Failures) > 0 { + fh, fr := assetUploadFailureItemRows(v.Failures) + render(fh, fr) + } return nil }) registerDirect(func(v *CustomProductPageScreenshotUploadResult, render func([]string, [][]string)) error { diff --git a/internal/asc/output_test.go b/internal/asc/output_test.go index 8d0e16560..ad154f48e 100644 --- a/internal/asc/output_test.go +++ b/internal/asc/output_test.go @@ -477,6 +477,21 @@ func TestPrintTable_SkippedAssetUploadResultShowsSkippedState(t *testing.T) { } } +func TestPrintTable_AppPreviewUploadResultRendersPartialFailures(t *testing.T) { + assertRenderedNonJSONContains(t, PrintTable, &AppPreviewUploadResult{ + VersionLocalizationID: "LOC_123", + SetID: "SET_123", + PreviewType: "IPHONE_65", + Results: []AssetUploadResultItem{ + {FileName: "01-first.mov", AssetID: "PREVIEW_1", State: "COMPLETE"}, + {FileName: "02-second.mov", FilePath: "/tmp/02-second.mov", State: "failed"}, + }, + Failures: []AssetUploadFailureItem{ + {FileName: "02-second.mov", FilePath: "/tmp/02-second.mov", Error: "preview reservation failed"}, + }, + }, "SET_123", "IPHONE_65", "01-first.mov", "PREVIEW_1", "failed", "preview reservation failed") +} + func TestPrintTableAndMarkdown_AppScreenshotFanoutUploadResultIncludesFlattenedFileRows(t *testing.T) { resp := &AppScreenshotFanoutUploadResult{ AppID: "123456789", diff --git a/internal/cli/assets/assets_order.go b/internal/cli/assets/assets_order.go new file mode 100644 index 000000000..459d90a19 --- /dev/null +++ b/internal/cli/assets/assets_order.go @@ -0,0 +1,156 @@ +package assets + +import ( + "context" + "fmt" + "strings" + + "github.com/rudrankriyam/App-Store-Connect-CLI/internal/asc" +) + +// collectOrderedLinkageIDs walks every linkage page and returns the linked +// resource IDs in the order App Store Connect reports them. +func collectOrderedLinkageIDs(ctx context.Context, firstPage *asc.LinkagesResponse, next func(context.Context, string) (asc.PaginatedResponse, error)) ([]string, error) { + if firstPage == nil { + return nil, fmt.Errorf("linkage response is required") + } + + orderedIDs := make([]string, 0, len(firstPage.Data)) + err := asc.PaginateEach(ctx, firstPage, next, func(page asc.PaginatedResponse) error { + linkages, ok := page.(*asc.LinkagesResponse) + if !ok { + return fmt.Errorf("unexpected relationship response type %T", page) + } + for _, item := range linkages.Data { + orderedIDs = appendUniqueAssetID(orderedIDs, item.ID) + } + return nil + }) + if err != nil { + return nil, err + } + + return orderedIDs, nil +} + +// orderAssetIDsForLocalFiles orders asset IDs by the local file order of the +// current run and appends any remaining remote IDs in their existing order. +func orderAssetIDsForLocalFiles(currentOrder []string, files []string, skippedResults, uploadedResults []asc.AssetUploadResultItem) []string { + skippedByPath := make(map[string]string, len(skippedResults)) + for _, item := range skippedResults { + if strings.TrimSpace(item.AssetID) == "" { + continue + } + skippedByPath[item.FilePath] = item.AssetID + } + uploadedByPath := make(map[string]string, len(uploadedResults)) + for _, item := range uploadedResults { + if strings.TrimSpace(item.AssetID) == "" { + continue + } + uploadedByPath[item.FilePath] = item.AssetID + } + + orderedIDs := make([]string, 0, len(currentOrder)+len(uploadedResults)) + seen := make(map[string]struct{}, len(currentOrder)+len(uploadedResults)) + for _, filePath := range files { + id := skippedByPath[filePath] + if id == "" { + id = uploadedByPath[filePath] + } + id = strings.TrimSpace(id) + if id == "" { + continue + } + if _, exists := seen[id]; exists { + continue + } + seen[id] = struct{}{} + orderedIDs = append(orderedIDs, id) + } + for _, id := range currentOrder { + id = strings.TrimSpace(id) + if id == "" { + continue + } + if _, exists := seen[id]; exists { + continue + } + seen[id] = struct{}{} + orderedIDs = append(orderedIDs, id) + } + + return orderedIDs +} + +// appendUploadedAssetIDs keeps the remote order of assets that already existed +// before this run and appends the newly uploaded assets in upload order. +func appendUploadedAssetIDs(currentOrder []string, uploadedResults []asc.AssetUploadResultItem) []string { + uploadedIDs := make(map[string]struct{}, len(uploadedResults)) + for _, item := range uploadedResults { + if id := strings.TrimSpace(item.AssetID); id != "" { + uploadedIDs[id] = struct{}{} + } + } + + orderedIDs := make([]string, 0, len(currentOrder)+len(uploadedResults)) + for _, id := range currentOrder { + if _, uploaded := uploadedIDs[strings.TrimSpace(id)]; uploaded { + continue + } + orderedIDs = appendUniqueAssetID(orderedIDs, id) + } + for _, item := range uploadedResults { + orderedIDs = appendUniqueAssetID(orderedIDs, item.AssetID) + } + + return orderedIDs +} + +func sameAssetIDOrder(a, b []string) bool { + a = normalizeAssetIDs(a) + b = normalizeAssetIDs(b) + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func normalizeAssetIDs(ids []string) []string { + if len(ids) == 0 { + return nil + } + + seen := make(map[string]struct{}, len(ids)) + normalized := make([]string, 0, len(ids)) + for _, id := range ids { + id = strings.TrimSpace(id) + if id == "" { + continue + } + if _, exists := seen[id]; exists { + continue + } + seen[id] = struct{}{} + normalized = append(normalized, id) + } + return normalized +} + +func appendUniqueAssetID(ids []string, id string) []string { + id = strings.TrimSpace(id) + if id == "" { + return ids + } + for _, existing := range ids { + if existing == id { + return ids + } + } + return append(ids, id) +} diff --git a/internal/cli/assets/assets_previews.go b/internal/cli/assets/assets_previews.go index 791bef833..eab3e5886 100644 --- a/internal/cli/assets/assets_previews.go +++ b/internal/cli/assets/assets_previews.go @@ -173,11 +173,16 @@ Examples: } result, err := uploadPreviews(ctx, client, locID, previewType, files, *skipExisting, *replace, *dryRun) + if hasAppPreviewUploadResultOutput(result) { + if printErr := shared.PrintOutput(&result, *output.Output, *output.Pretty); printErr != nil { + return printErr + } + } if err != nil { return fmt.Errorf("video-previews upload: %w", err) } - return shared.PrintOutput(&result, *output.Output, *output.Pretty) + return nil }, } } @@ -757,6 +762,10 @@ func uploadPreviewAsset(ctx context.Context, client *asc.Client, setID, filePath if err != nil { return asc.AssetUploadResultItem{}, err } + result := asc.AssetUploadResultItem{ + FileName: info.Name(), + FilePath: filePath, + } checksum, err := asc.ComputeChecksumFromReader(file, asc.ChecksumAlgorithmMD5) if err != nil { @@ -765,31 +774,37 @@ func uploadPreviewAsset(ctx context.Context, client *asc.Client, setID, filePath created, err := client.CreateAppPreview(ctx, setID, info.Name(), info.Size(), mimeType) if err != nil { - return asc.AssetUploadResultItem{}, err + return result, err + } + result.AssetID = created.Data.ID + if created.Data.Attributes.AssetDeliveryState != nil { + result.State = created.Data.Attributes.AssetDeliveryState.State } if len(created.Data.Attributes.UploadOperations) == 0 { - return asc.AssetUploadResultItem{}, fmt.Errorf("no upload operations returned for %q", info.Name()) + return result, fmt.Errorf("no upload operations returned for %q", info.Name()) } if err := asc.UploadAssetFromFile(ctx, file, info.Size(), created.Data.Attributes.UploadOperations); err != nil { - return asc.AssetUploadResultItem{}, err + return result, err } - if _, err := client.UpdateAppPreview(ctx, created.Data.ID, true, checksum.Hash); err != nil { - return asc.AssetUploadResultItem{}, err + updated, err := client.UpdateAppPreview(ctx, created.Data.ID, true, checksum.Hash) + if err != nil { + return result, err + } + if updated.Data.Attributes.AssetDeliveryState != nil { + result.State = updated.Data.Attributes.AssetDeliveryState.State } state, err := waitForPreviewDelivery(ctx, client, created.Data.ID) + if state != "" { + result.State = state + } if err != nil { - return asc.AssetUploadResultItem{}, err + return result, err } - return asc.AssetUploadResultItem{ - FileName: info.Name(), - FilePath: filePath, - AssetID: created.Data.ID, - State: state, - }, nil + return result, nil } // UploadPreviewAsset uploads a preview file to a set. @@ -858,6 +873,7 @@ func uploadPreviews(ctx context.Context, client *asc.Client, localizationID, pre existingPreviews = existingResp.Data } + runFiles := append([]string(nil), files...) skippedResults := make([]asc.AssetUploadResultItem, 0) if skipExisting { files, skippedResults, err = filterExistingPreviewFiles(files, existingPreviews) @@ -904,24 +920,113 @@ func uploadPreviews(ctx context.Context, client *asc.Client, localizationID, pre } } - results := make([]asc.AssetUploadResultItem, 0, len(skippedResults)+len(files)) - if len(files) > 0 { - for _, filePath := range files { - item, err := uploadPreviewAsset(uploadCtx, client, set.ID, filePath) - if err != nil { - return asc.AppPreviewUploadResult{}, err + uploadedResults := make([]asc.AssetUploadResultItem, 0, len(files)) + var ( + uploadErr error + failedResult asc.AssetUploadResultItem + failure asc.AssetUploadFailureItem + ) + for _, filePath := range files { + item, err := uploadPreviewAsset(uploadCtx, client, set.ID, filePath) + if err != nil { + uploadErr = err + failedResult = item + if failedResult.FileName == "" { + failedResult.FileName = filepath.Base(filePath) + } + if failedResult.FilePath == "" { + failedResult.FilePath = filePath } - results = append(results, item) + if failedResult.State == "" { + failedResult.State = "failed" + } + failure = asc.AssetUploadFailureItem{ + FileName: filepath.Base(filePath), + FilePath: filePath, + Error: err.Error(), + } + break } + uploadedResults = append(uploadedResults, item) } - results = append(skippedResults, results...) - return asc.AppPreviewUploadResult{ + result := asc.AppPreviewUploadResult{ VersionLocalizationID: localizationID, SetID: set.ID, PreviewType: set.Attributes.PreviewType, - Results: results, - }, nil + Results: append(append([]asc.AssetUploadResultItem{}, skippedResults...), uploadedResults...), + } + if uploadErr != nil { + result.Results = append(result.Results, failedResult) + result.Failures = append(result.Failures, failure) + return result, uploadErr + } + + if err := syncPreviewOrder(uploadCtx, client, set.ID, runFiles, skippedResults, uploadedResults); err != nil { + return result, err + } + + return result, nil +} + +func hasAppPreviewUploadResultOutput(result asc.AppPreviewUploadResult) bool { + return strings.TrimSpace(result.VersionLocalizationID) != "" || + strings.TrimSpace(result.SetID) != "" || + strings.TrimSpace(result.PreviewType) != "" || + len(result.Results) > 0 || + len(result.Failures) > 0 +} + +// getOrderedAppPreviewIDs returns preview IDs in the current remote order. +func getOrderedAppPreviewIDs(ctx context.Context, client *asc.Client, setID string) ([]string, error) { + if client == nil { + return nil, fmt.Errorf("client is required") + } + + firstPage, err := client.GetAppPreviewSetAppPreviewsRelationships(ctx, setID, asc.WithLinkagesLimit(200)) + if err != nil { + return nil, err + } + + return collectOrderedLinkageIDs(ctx, firstPage, func(ctx context.Context, nextURL string) (asc.PaginatedResponse, error) { + return client.GetAppPreviewSetAppPreviewsRelationships(ctx, "", asc.WithLinkagesNextURL(nextURL)) + }) +} + +// setOrderedAppPreviews replaces the preview relationships for a set in the provided order. +func setOrderedAppPreviews(ctx context.Context, client *asc.Client, setID string, orderedIDs []string) error { + if client == nil { + return fmt.Errorf("client is required") + } + return client.UpdateAppPreviewSetAppPreviewsRelationship(ctx, setID, normalizeAssetIDs(orderedIDs)) +} + +// syncPreviewOrder pins the on-store preview order to the sorted file order of +// this run instead of the order App Store Connect happens to assign. The PATCH +// is skipped when the set is already in the desired order. +func syncPreviewOrder(ctx context.Context, client *asc.Client, setID string, files []string, skippedResults, uploadedResults []asc.AssetUploadResultItem) error { + if client == nil { + return fmt.Errorf("client is required") + } + setID = strings.TrimSpace(setID) + if setID == "" || (len(skippedResults) == 0 && len(uploadedResults) == 0) { + return nil + } + + currentOrder, err := getOrderedAppPreviewIDs(ctx, client, setID) + if err != nil { + return err + } + + orderedIDs := appendUploadedAssetIDs(currentOrder, uploadedResults) + if len(skippedResults) > 0 { + orderedIDs = orderAssetIDsForLocalFiles(currentOrder, files, skippedResults, uploadedResults) + } + if len(orderedIDs) == 0 || sameAssetIDOrder(currentOrder, orderedIDs) { + return nil + } + + return setOrderedAppPreviews(ctx, client, setID, orderedIDs) } func deleteExistingPreviews(ctx context.Context, client *asc.Client, previews []asc.Resource[asc.AppPreviewAttributes]) error { @@ -934,13 +1039,15 @@ func deleteExistingPreviews(ctx context.Context, client *asc.Client, previews [] } func filterExistingPreviewFiles(files []string, previews []asc.Resource[asc.AppPreviewAttributes]) ([]string, []asc.AssetUploadResultItem, error) { - existingChecksums := make(map[string]struct{}, len(previews)) + existingByChecksum := make(map[string]asc.Resource[asc.AppPreviewAttributes], len(previews)) for _, preview := range previews { checksum := strings.TrimSpace(preview.Attributes.SourceFileChecksum) if checksum == "" { continue } - existingChecksums[checksum] = struct{}{} + if _, exists := existingByChecksum[checksum]; !exists { + existingByChecksum[checksum] = preview + } } filtered := make([]string, 0, len(files)) @@ -950,10 +1057,11 @@ func filterExistingPreviewFiles(files []string, previews []asc.Resource[asc.AppP if err != nil { return nil, nil, err } - if _, exists := existingChecksums[checksum]; exists { + if existing, exists := existingByChecksum[checksum]; exists { skipped = append(skipped, asc.AssetUploadResultItem{ FileName: filepath.Base(filePath), FilePath: filePath, + AssetID: existing.ID, State: "skipped", Skipped: true, }) diff --git a/internal/cli/assets/assets_previews_test.go b/internal/cli/assets/assets_previews_test.go index a1b45583a..718782c33 100644 --- a/internal/cli/assets/assets_previews_test.go +++ b/internal/cli/assets/assets_previews_test.go @@ -2,12 +2,15 @@ package assets import ( "context" + "encoding/json" "errors" "flag" + "fmt" "io" "net/http" "os" "path/filepath" + "reflect" "strings" "testing" @@ -184,6 +187,88 @@ func TestUploadPreviewsRejectsUnsupportedFileBeforeRequests(t *testing.T) { } } +func TestUploadPreviewsSkipExistingSyncsSortedOrder(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "01-first.mov") + second := filepath.Join(dir, "02-second.mov") + if err := os.WriteFile(first, []byte("first-preview"), 0o600); err != nil { + t.Fatalf("write first preview: %v", err) + } + if err := os.WriteFile(second, []byte("second-preview"), 0o600); err != nil { + t.Fatalf("write second preview: %v", err) + } + firstChecksum, err := computeFileChecksum(first) + if err != nil { + t.Fatalf("checksum first preview: %v", err) + } + secondChecksum, err := computeFileChecksum(second) + if err != nil { + t.Fatalf("checksum second preview: %v", err) + } + + patched := make([][]string, 0, 1) + client := newAssetsUploadTestServerClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/v1/appStoreVersionLocalizations/LOC_ID/appPreviewSets": + writeAssetsTestJSON(w, http.StatusOK, `{"data":[{"type":"appPreviewSets","id":"set-1","attributes":{"previewType":"IPHONE_65"}}],"links":{}}`) + case r.Method == http.MethodGet && r.URL.Path == "/v1/appPreviewSets/set-1/appPreviews": + writeAssetsTestJSON(w, http.StatusOK, fmt.Sprintf( + `{"data":[{"type":"appPreviews","id":"preview-second","attributes":{"fileName":"02-second.mov","sourceFileChecksum":"%s"}},{"type":"appPreviews","id":"preview-first","attributes":{"fileName":"01-first.mov","sourceFileChecksum":"%s"}}],"links":{}}`, + secondChecksum, firstChecksum, + )) + case r.Method == http.MethodGet && r.URL.Path == "/v1/appPreviewSets/set-1/relationships/appPreviews": + writeAssetsTestJSON(w, http.StatusOK, `{"data":[{"type":"appPreviews","id":"preview-second"},{"type":"appPreviews","id":"preview-first"}],"links":{}}`) + case r.Method == http.MethodPatch && r.URL.Path == "/v1/appPreviewSets/set-1/relationships/appPreviews": + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("read relationship body: %v", err) + } + var payload asc.RelationshipRequest + if err := json.Unmarshal(body, &payload); err != nil { + t.Fatalf("decode relationship body: %v", err) + } + ids := make([]string, 0, len(payload.Data)) + for _, item := range payload.Data { + ids = append(ids, item.ID) + } + patched = append(patched, ids) + w.WriteHeader(http.StatusNoContent) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + writeAssetsTestJSON(w, http.StatusInternalServerError, `{"errors":[{"status":"500","detail":"unexpected request"}]}`) + } + })) + + result, err := uploadPreviews( + context.Background(), + client, + "LOC_ID", + "IPHONE_65", + []string{first, second}, + true, + false, + false, + ) + if err != nil { + t.Fatalf("uploadPreviews() error: %v", err) + } + if len(patched) != 1 { + t.Fatalf("expected exactly one preview order PATCH, got %d (%v)", len(patched), patched) + } + want := []string{"preview-first", "preview-second"} + if !reflect.DeepEqual(patched[0], want) { + t.Fatalf("preview order PATCH = %v, want %v", patched[0], want) + } + if len(result.Results) != 2 { + t.Fatalf("expected 2 skipped results, got %#v", result.Results) + } + for _, item := range result.Results { + if !item.Skipped || strings.TrimSpace(item.AssetID) == "" { + t.Fatalf("expected skipped results to carry the existing preview ID, got %#v", item) + } + } +} + func TestNormalizePreviewTypeCanonicalizesIPhone69Alias(t *testing.T) { testCases := []string{ "IPHONE_69", diff --git a/internal/cli/assets/assets_screenshot_order.go b/internal/cli/assets/assets_screenshot_order.go index 5f492fef9..d643c5ba2 100644 --- a/internal/cli/assets/assets_screenshot_order.go +++ b/internal/cli/assets/assets_screenshot_order.go @@ -51,7 +51,7 @@ func uploadScreenshotsWithOrderState(ctx context.Context, client *asc.Client, se return progress, err } progress.Results = append(progress.Results, item) - progress.OrderedIDs = appendUniqueScreenshotID(progress.OrderedIDs, item.AssetID) + progress.OrderedIDs = appendUniqueAssetID(progress.OrderedIDs, item.AssetID) } if len(progress.OrderedIDs) == 0 { @@ -104,7 +104,7 @@ func resumeScreenshotsWithOrderState(ctx context.Context, client *asc.Client, se } if !retryUpload { progress.Results = append(progress.Results, result) - progress.OrderedIDs = appendUniqueScreenshotID(progress.OrderedIDs, result.AssetID) + progress.OrderedIDs = appendUniqueAssetID(progress.OrderedIDs, result.AssetID) remainingFiles = remainingFiles[1:] } } @@ -216,24 +216,9 @@ func GetOrderedAppScreenshotIDs(ctx context.Context, client *asc.Client, setID s return nil, err } - orderedIDs := make([]string, 0, len(firstPage.Data)) - err = asc.PaginateEach(ctx, firstPage, func(ctx context.Context, nextURL string) (asc.PaginatedResponse, error) { + return collectOrderedLinkageIDs(ctx, firstPage, func(ctx context.Context, nextURL string) (asc.PaginatedResponse, error) { return client.GetAppScreenshotSetAppScreenshotsRelationships(ctx, "", asc.WithLinkagesNextURL(nextURL)) - }, func(page asc.PaginatedResponse) error { - linkages, ok := page.(*asc.LinkagesResponse) - if !ok { - return fmt.Errorf("unexpected screenshot relationship response type %T", page) - } - for _, item := range linkages.Data { - orderedIDs = appendUniqueScreenshotID(orderedIDs, item.ID) - } - return nil }) - if err != nil { - return nil, err - } - - return orderedIDs, nil } // SetOrderedAppScreenshots replaces the screenshot relationships for a set in the provided order. @@ -241,39 +226,5 @@ func SetOrderedAppScreenshots(ctx context.Context, client *asc.Client, setID str if client == nil { return fmt.Errorf("client is required") } - return client.UpdateAppScreenshotSetAppScreenshotsRelationship(ctx, setID, normalizeScreenshotIDs(orderedIDs)) -} - -func normalizeScreenshotIDs(ids []string) []string { - if len(ids) == 0 { - return nil - } - - seen := make(map[string]struct{}, len(ids)) - normalized := make([]string, 0, len(ids)) - for _, id := range ids { - id = strings.TrimSpace(id) - if id == "" { - continue - } - if _, exists := seen[id]; exists { - continue - } - seen[id] = struct{}{} - normalized = append(normalized, id) - } - return normalized -} - -func appendUniqueScreenshotID(ids []string, id string) []string { - id = strings.TrimSpace(id) - if id == "" { - return ids - } - for _, existing := range ids { - if existing == id { - return ids - } - } - return append(ids, id) + return client.UpdateAppScreenshotSetAppScreenshotsRelationship(ctx, setID, normalizeAssetIDs(orderedIDs)) } diff --git a/internal/cli/assets/assets_screenshots_resume.go b/internal/cli/assets/assets_screenshots_resume.go index 22b812844..59ab06dc9 100644 --- a/internal/cli/assets/assets_screenshots_resume.go +++ b/internal/cli/assets/assets_screenshots_resume.go @@ -276,7 +276,7 @@ func executeAppScreenshotUpload(ctx context.Context, cfg screenshotUploadConfig[ orderedIDs := append([]string(nil), progress.OrderedIDs...) if cfg.SkipExisting && len(prepared.SkippedResults) > 0 && (len(prepared.Files) > 0 || strings.TrimSpace(progress.FailedFile) != "") { - desiredIDs := orderScreenshotIDsForLocalFiles(prepared.OrderedIDs, cfg.Files, prepared.SkippedResults, progress.Results) + desiredIDs := orderAssetIDsForLocalFiles(prepared.OrderedIDs, cfg.Files, prepared.SkippedResults, progress.Results) if len(desiredIDs) > 0 { orderedIDs = desiredIDs } @@ -359,7 +359,7 @@ func resumeAppScreenshotUpload(ctx context.Context, client *asc.Client, artifact currentOrder, err := GetOrderedAppScreenshotIDs(uploadCtx, client, artifact.SetID) if err != nil { uploadErr = err - } else if desiredIDs := orderScreenshotIDsForLocalFiles(currentOrder, artifact.Files, skippedResults, uploadedResults); len(desiredIDs) > 0 && !sameScreenshotIDOrder(currentOrder, desiredIDs) { + } else if desiredIDs := orderAssetIDsForLocalFiles(currentOrder, artifact.Files, skippedResults, uploadedResults); len(desiredIDs) > 0 && !sameAssetIDOrder(currentOrder, desiredIDs) { if err := SetOrderedAppScreenshots(uploadCtx, client, artifact.SetID, desiredIDs); err != nil { progress.OrderedIDs = desiredIDs uploadErr = err diff --git a/internal/cli/assets/assets_screenshots_upload.go b/internal/cli/assets/assets_screenshots_upload.go index 125549127..87f72a6c0 100644 --- a/internal/cli/assets/assets_screenshots_upload.go +++ b/internal/cli/assets/assets_screenshots_upload.go @@ -259,75 +259,13 @@ func syncSkippedScreenshotOrder(ctx context.Context, client *asc.Client, setID s return nil, err } - orderedIDs := orderScreenshotIDsForLocalFiles(currentOrder, files, skippedResults, uploadedResults) - if sameScreenshotIDOrder(currentOrder, orderedIDs) { + orderedIDs := orderAssetIDsForLocalFiles(currentOrder, files, skippedResults, uploadedResults) + if sameAssetIDOrder(currentOrder, orderedIDs) { return orderedIDs, nil } return orderedIDs, SetOrderedAppScreenshots(ctx, client, setID, orderedIDs) } -func orderScreenshotIDsForLocalFiles(currentOrder []string, files []string, skippedResults, uploadedResults []asc.AssetUploadResultItem) []string { - skippedByPath := make(map[string]string, len(skippedResults)) - for _, item := range skippedResults { - if strings.TrimSpace(item.AssetID) == "" { - continue - } - skippedByPath[item.FilePath] = item.AssetID - } - uploadedByPath := make(map[string]string, len(uploadedResults)) - for _, item := range uploadedResults { - if strings.TrimSpace(item.AssetID) == "" { - continue - } - uploadedByPath[item.FilePath] = item.AssetID - } - - orderedIDs := make([]string, 0, len(currentOrder)+len(uploadedResults)) - seen := make(map[string]struct{}, len(currentOrder)+len(uploadedResults)) - for _, filePath := range files { - id := skippedByPath[filePath] - if id == "" { - id = uploadedByPath[filePath] - } - id = strings.TrimSpace(id) - if id == "" { - continue - } - if _, exists := seen[id]; exists { - continue - } - seen[id] = struct{}{} - orderedIDs = append(orderedIDs, id) - } - for _, id := range currentOrder { - id = strings.TrimSpace(id) - if id == "" { - continue - } - if _, exists := seen[id]; exists { - continue - } - seen[id] = struct{}{} - orderedIDs = append(orderedIDs, id) - } - - return orderedIDs -} - -func sameScreenshotIDOrder(a, b []string) bool { - a = normalizeScreenshotIDs(a) - b = normalizeScreenshotIDs(b) - if len(a) != len(b) { - return false - } - for i := range a { - if a[i] != b[i] { - return false - } - } - return true -} - func computeFileChecksum(filePath string) (string, error) { file, err := shared.OpenExistingNoFollow(filePath) if err != nil { diff --git a/internal/cli/cmdtest/video_previews_upload_order_test.go b/internal/cli/cmdtest/video_previews_upload_order_test.go new file mode 100644 index 000000000..623f58618 --- /dev/null +++ b/internal/cli/cmdtest/video_previews_upload_order_test.go @@ -0,0 +1,188 @@ +package cmdtest + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/rudrankriyam/App-Store-Connect-CLI/internal/asc" +) + +func writePreviewFile(t *testing.T, path string) int64 { + t.Helper() + + if err := os.WriteFile(path, []byte("preview-bytes-"+filepath.Base(path)), 0o600); err != nil { + t.Fatalf("write preview %s: %v", path, err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat preview %s: %v", path, err) + } + return info.Size() +} + +func previewRelationshipIDs(t *testing.T, req *http.Request) []string { + t.Helper() + + body, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("read relationship body: %v", err) + } + var payload asc.RelationshipRequest + if err := json.Unmarshal(body, &payload); err != nil { + t.Fatalf("decode relationship body: %v\nbody=%s", err, body) + } + ids := make([]string, 0, len(payload.Data)) + for _, item := range payload.Data { + if item.Type != "appPreviews" { + t.Fatalf("relationship type = %q, want appPreviews", item.Type) + } + ids = append(ids, item.ID) + } + return ids +} + +// previewUploadTransport serves a two-file preview upload where App Store +// Connect reports the previews in reverse filename order. +func previewUploadTransport(t *testing.T, sizes map[string]int64, remoteOrder []string, onPatch func([]string)) roundTripFunc { + t.Helper() + + return roundTripFunc(func(req *http.Request) (*http.Response, error) { + switch { + case req.Method == http.MethodGet && req.URL.Path == "/v1/appStoreVersionLocalizations/LOC_123/appPreviewSets": + return statusJSONResponse(`{"data":[{"type":"appPreviewSets","id":"set-1","attributes":{"previewType":"IPHONE_65"}}],"links":{}}`), nil + case req.Method == http.MethodPost && req.URL.Path == "/v1/appPreviews": + body, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("read create preview body: %v", err) + } + id := "preview-first" + fileName := "01-first.mov" + if strings.Contains(string(body), "02-second.mov") { + id = "preview-second" + fileName = "02-second.mov" + } + return statusJSONResponse(fmt.Sprintf( + `{"data":{"type":"appPreviews","id":"%s","attributes":{"fileName":"%s","uploadOperations":[{"method":"PUT","url":"https://upload.example/%s","length":%d,"offset":0}]}}}`, + id, fileName, id, sizes[fileName], + )), nil + case req.Method == http.MethodPut && req.URL.Host == "upload.example": + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader("")), + Header: http.Header{}, + }, nil + case req.Method == http.MethodPatch && strings.HasPrefix(req.URL.Path, "/v1/appPreviews/"): + id := strings.TrimPrefix(req.URL.Path, "/v1/appPreviews/") + return statusJSONResponse(fmt.Sprintf(`{"data":{"type":"appPreviews","id":"%s","attributes":{"uploaded":true}}}`, id)), nil + case req.Method == http.MethodGet && strings.HasPrefix(req.URL.Path, "/v1/appPreviews/"): + id := strings.TrimPrefix(req.URL.Path, "/v1/appPreviews/") + return statusJSONResponse(fmt.Sprintf(`{"data":{"type":"appPreviews","id":"%s","attributes":{"assetDeliveryState":{"state":"COMPLETE"}}}}`, id)), nil + case req.Method == http.MethodGet && req.URL.Path == "/v1/appPreviewSets/set-1/relationships/appPreviews": + linkages := make([]string, 0, len(remoteOrder)) + for _, id := range remoteOrder { + linkages = append(linkages, fmt.Sprintf(`{"type":"appPreviews","id":"%s"}`, id)) + } + return statusJSONResponse(fmt.Sprintf(`{"data":[%s],"links":{}}`, strings.Join(linkages, ","))), nil + case req.Method == http.MethodPatch && req.URL.Path == "/v1/appPreviewSets/set-1/relationships/appPreviews": + onPatch(previewRelationshipIDs(t, req)) + return &http.Response{ + StatusCode: http.StatusNoContent, + Body: io.NopCloser(strings.NewReader("")), + Header: http.Header{}, + }, nil + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.String()) + return nil, nil + } + }) +} + +func TestVideoPreviewsUploadAppliesSortedFileNameOrder(t *testing.T) { + setupAuth(t) + t.Setenv("ASC_CONFIG_PATH", filepath.Join(t.TempDir(), "nonexistent.json")) + t.Setenv("ASC_APP_ID", "") + + pathDir := t.TempDir() + sizes := map[string]int64{ + "01-first.mov": writePreviewFile(t, filepath.Join(pathDir, "01-first.mov")), + "02-second.mov": writePreviewFile(t, filepath.Join(pathDir, "02-second.mov")), + } + + originalTransport := http.DefaultTransport + t.Cleanup(func() { + http.DefaultTransport = originalTransport + }) + + patched := make([][]string, 0, 1) + http.DefaultTransport = previewUploadTransport(t, sizes, []string{"preview-second", "preview-first"}, func(ids []string) { + patched = append(patched, ids) + }) + + stdout, stderr, runErr := runRootCommand(t, []string{ + "video-previews", "upload", + "--version-localization", "LOC_123", + "--path", pathDir, + "--device-type", "IPHONE_65", + "--output", "json", + }) + + if runErr != nil { + t.Fatalf("run error: %v (stderr=%q)", runErr, stderr) + } + if stderr != "" { + t.Fatalf("expected empty stderr, got %q", stderr) + } + if len(patched) != 1 { + t.Fatalf("expected exactly one preview order PATCH, got %d (%v)", len(patched), patched) + } + want := []string{"preview-first", "preview-second"} + if !reflect.DeepEqual(patched[0], want) { + t.Fatalf("preview order PATCH = %v, want %v", patched[0], want) + } + if !strings.Contains(stdout, `"setId":"set-1"`) { + t.Fatalf("expected receipt for set-1, got %s", stdout) + } +} + +func TestVideoPreviewsUploadSkipsOrderPatchWhenAlreadyOrdered(t *testing.T) { + setupAuth(t) + t.Setenv("ASC_CONFIG_PATH", filepath.Join(t.TempDir(), "nonexistent.json")) + t.Setenv("ASC_APP_ID", "") + + pathDir := t.TempDir() + sizes := map[string]int64{ + "01-first.mov": writePreviewFile(t, filepath.Join(pathDir, "01-first.mov")), + "02-second.mov": writePreviewFile(t, filepath.Join(pathDir, "02-second.mov")), + } + + originalTransport := http.DefaultTransport + t.Cleanup(func() { + http.DefaultTransport = originalTransport + }) + + http.DefaultTransport = previewUploadTransport(t, sizes, []string{"preview-first", "preview-second"}, func(ids []string) { + t.Fatalf("expected no preview order PATCH when the set is already ordered, got %v", ids) + }) + + _, stderr, runErr := runRootCommand(t, []string{ + "video-previews", "upload", + "--version-localization", "LOC_123", + "--path", pathDir, + "--device-type", "IPHONE_65", + "--output", "json", + }) + + if runErr != nil { + t.Fatalf("run error: %v (stderr=%q)", runErr, stderr) + } + if stderr != "" { + t.Fatalf("expected empty stderr, got %q", stderr) + } +} diff --git a/internal/cli/cmdtest/video_previews_upload_partial_test.go b/internal/cli/cmdtest/video_previews_upload_partial_test.go new file mode 100644 index 000000000..caca2faae --- /dev/null +++ b/internal/cli/cmdtest/video_previews_upload_partial_test.go @@ -0,0 +1,139 @@ +package cmdtest + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "path/filepath" + "strings" + "testing" + + rootcmd "github.com/rudrankriyam/App-Store-Connect-CLI/cmd" +) + +func TestVideoPreviewsUploadReportsUploadedPreviewsWhenALaterFileFails(t *testing.T) { + setupAuth(t) + t.Setenv("ASC_CONFIG_PATH", filepath.Join(t.TempDir(), "nonexistent.json")) + t.Setenv("ASC_APP_ID", "") + + pathDir := t.TempDir() + firstSize := writePreviewFile(t, filepath.Join(pathDir, "01-first.mov")) + secondSize := writePreviewFile(t, filepath.Join(pathDir, "02-second.mov")) + + originalTransport := http.DefaultTransport + t.Cleanup(func() { + http.DefaultTransport = originalTransport + }) + + http.DefaultTransport = roundTripFunc(func(req *http.Request) (*http.Response, error) { + switch { + case req.Method == http.MethodGet && req.URL.Path == "/v1/appStoreVersionLocalizations/LOC_123/appPreviewSets": + return statusJSONResponse(`{"data":[{"type":"appPreviewSets","id":"set-1","attributes":{"previewType":"IPHONE_65"}}],"links":{}}`), nil + case req.Method == http.MethodPost && req.URL.Path == "/v1/appPreviews": + body, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("read create preview body: %v", err) + } + if strings.Contains(string(body), "02-second.mov") { + return statusJSONResponse(fmt.Sprintf( + `{"data":{"type":"appPreviews","id":"preview-second","attributes":{"fileName":"02-second.mov","assetDeliveryState":{"state":"AWAITING_UPLOAD"},"uploadOperations":[{"method":"PUT","url":"https://upload.example/preview-second","length":%d,"offset":0}]}}}`, + secondSize, + )), nil + } + return statusJSONResponse(fmt.Sprintf( + `{"data":{"type":"appPreviews","id":"preview-first","attributes":{"fileName":"01-first.mov","uploadOperations":[{"method":"PUT","url":"https://upload.example/preview-first","length":%d,"offset":0}]}}}`, + firstSize, + )), nil + case req.Method == http.MethodPut && req.URL.Host == "upload.example" && req.URL.Path == "/preview-second": + return &http.Response{ + StatusCode: http.StatusInternalServerError, + Body: io.NopCloser(strings.NewReader("preview upload failed")), + Header: http.Header{}, + }, nil + case req.Method == http.MethodPut && req.URL.Host == "upload.example": + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader("")), + Header: http.Header{}, + }, nil + case req.Method == http.MethodPatch && req.URL.Path == "/v1/appPreviews/preview-first": + return statusJSONResponse(`{"data":{"type":"appPreviews","id":"preview-first","attributes":{"uploaded":true}}}`), nil + case req.Method == http.MethodGet && req.URL.Path == "/v1/appPreviews/preview-first": + return statusJSONResponse(`{"data":{"type":"appPreviews","id":"preview-first","attributes":{"assetDeliveryState":{"state":"COMPLETE"}}}}`), nil + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.String()) + return nil, nil + } + }) + + exitCode := rootcmd.ExitSuccess + stdout, stderr := captureOutput(t, func() { + exitCode = rootcmd.Run([]string{ + "video-previews", "upload", + "--version-localization", "LOC_123", + "--path", pathDir, + "--device-type", "IPHONE_65", + "--output", "json", + }, "1.2.3") + }) + + if exitCode == rootcmd.ExitSuccess { + t.Fatal("expected a non-zero exit code when a preview upload fails") + } + if !strings.Contains(stderr, "video-previews upload") { + t.Fatalf("expected the failure to be attributed to video-previews upload, got stderr=%q", stderr) + } + + var payload struct { + SetID string `json:"setId"` + Results []struct { + FileName string `json:"fileName"` + AssetID string `json:"assetId"` + State string `json:"state"` + } `json:"results"` + Failures []struct { + FileName string `json:"fileName"` + FilePath string `json:"filePath"` + Error string `json:"error"` + } `json:"failures"` + } + if err := json.Unmarshal([]byte(stdout), &payload); err != nil { + t.Fatalf("decode partial receipt: %v\nstdout=%s", err, stdout) + } + if payload.SetID != "set-1" { + t.Fatalf("setId = %q, want set-1", payload.SetID) + } + + uploaded := make(map[string]string, len(payload.Results)) + for _, item := range payload.Results { + uploaded[item.FileName] = item.AssetID + } + if uploaded["01-first.mov"] != "preview-first" { + t.Fatalf("expected the already uploaded preview in the receipt, got %s", stdout) + } + + if len(payload.Failures) != 1 { + t.Fatalf("expected exactly one failure entry, got %s", stdout) + } + failure := payload.Failures[0] + if failure.FileName != "02-second.mov" { + t.Fatalf("failure fileName = %q, want 02-second.mov", failure.FileName) + } + if !strings.Contains(failure.FilePath, "02-second.mov") { + t.Fatalf("failure filePath = %q, want the failing preview path", failure.FilePath) + } + if strings.TrimSpace(failure.Error) == "" { + t.Fatalf("expected a failure message, got %s", stdout) + } + + failedStates := 0 + for _, item := range payload.Results { + if item.FileName == "02-second.mov" && item.AssetID == "preview-second" && item.State == "AWAITING_UPLOAD" { + failedStates++ + } + } + if failedStates != 1 { + t.Fatalf("expected the failing file to retain its reservation ID and last known state, got %s", stdout) + } +}