diff --git a/docs/cli.md b/docs/cli.md index cb4a153..41efd44 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -293,6 +293,7 @@ Text output is optimized for direct operator use. JSON output is optimized for a - `run --format json` emits a JSON result for partial destination failures, sets `ok` to `false`, includes result details and errors, and exits non-zero. - Warnings are included in JSON output and are printed in text output when relevant. - `run` summaries include `publish_new`, `upsert_additive`, `replace_catalog`, `skip_same`, `force_replace`, `fail_unmanaged`, and `fail_conflict` counters. Destination action records use the same stable action values and include the resolved `destination_path`. +- `skip_same` means the planned outputs already match valid catalog metadata, so `run` does not write outputs, rewrite `.distributor.json`, delete files, or notify. The decision is based on catalog metadata and does not read destination file bytes. ## Diagnostics And Recovery diff --git a/docs/internal/publish.md b/docs/internal/publish.md index d8192f3..a61ae0d 100644 --- a/docs/internal/publish.md +++ b/docs/internal/publish.md @@ -33,6 +33,7 @@ Planning inspects destination state through `internal/state` and maps catalog co - `publish_new`: no valid state exists and the destination bundle path is empty. - `upsert_additive`: additive workflow writes planned outputs and retains unrelated catalog outputs. - `replace_catalog`: replacement workflow writes planned outputs and deletes omitted outputs for the current owner. +- `skip_same`: valid catalog metadata already matches every planned output. - `force_replace`: explicit bounded replacement selected by `Force`. - `fail_unmanaged`: unmanaged destination content blocks publication. - `fail_conflict`: invalid or unsupported state blocks publication. @@ -43,9 +44,11 @@ Execution writes destination state after selected outputs are written. Catalog o ## Workflow Behavior -Additive workflow computes a write set for the planned outputs and preserves catalog outputs for unplanned paths. Existing catalog records for planned paths are replaced by the current owner and source identity. +Additive workflow computes a write set for the planned outputs and preserves catalog outputs for unplanned paths. Existing catalog records for planned paths are replaced by the current owner and source identity. When every planned output already matches catalog metadata, planning returns `skip_same` and ignores unrelated retained catalog outputs for the no-op decision. -Replacement workflow computes a write set for the planned outputs, preserves other-owner outputs, and deletes omitted outputs owned by the current pipeline and destination. It does not need `Force`. +Replacement workflow computes a write set for the planned outputs, preserves other-owner outputs, and deletes omitted outputs owned by the current pipeline and destination. It does not need `Force`. Matching planned outputs return `skip_same` only when replacement would not delete omitted outputs for the current owner. + +Catalog skip comparison is metadata-only. It checks pipeline id, destination id, source id, source digest, source creation timestamp, output path, kind, digest, size, generated output source path, generated output transform, and output URL metadata. It does not read destination file bytes, and `skip_same` execution does not write outputs, rewrite `.distributor.json`, delete files, or notify. Forced replacement is explicit per request. It deletes the bounded destination bundle path before writing planned outputs and schema version `4` catalog state. Catalog planning selects `force_replace` only when `Force` is true and normal planning would otherwise fail for a non-empty no-state destination, a planned path collision with unmanaged storage content, invalid destination state, or unsupported future destination state. diff --git a/docs/operations.md b/docs/operations.md index 7a49a61..39b7b2b 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -144,7 +144,7 @@ Review these action labels before publishing: - `publish_new`: destination state is absent and the destination bundle path is empty. - `upsert_additive`: additive workflow will write planned outputs into valid catalog state. - `replace_catalog`: replacement workflow will write planned outputs and remove omitted outputs for the current owner. -- `skip_same`: no-op action value in the run output vocabulary. +- `skip_same`: planned outputs already match valid catalog metadata, so publication will not write outputs, rewrite `.distributor.json`, delete files, or notify. - `force_replace`: destructive catalog replacement selected because `--force` is present for unmanaged content, a planned unmanaged path collision, invalid state, or unsupported future state. - `fail_unmanaged`: unmanaged destination content prevents publication. - `fail_conflict`: invalid or unsupported destination state prevents publication. @@ -152,6 +152,8 @@ Review these action labels before publishing: Text and JSON summaries count `publish_new`, `upsert_additive`, `replace_catalog`, `skip_same`, `force_replace`, `fail_unmanaged`, `fail_conflict`, and failed destinations separately. JSON output includes warnings, pipeline summaries, destination action records, output records, URLs when configured, final counters, and partial failure details. Fatal setup failures such as unreadable config or invalid secrets do not produce a JSON result document. +The `skip_same` optimization trusts valid catalog metadata. It compares owner identity, source identity, output path, kind, digest, size, generated output metadata, and URL metadata recorded in `.distributor.json`; it does not read destination file bytes to detect bitrot. + Fixed destinations add fixed-path warnings during dry runs, including the selected source bundle and replacement warnings when the destination root would be replaced. For fixed destinations, the resolved destination bundle path is the backend root. ## Forced Replacement Workflow diff --git a/internal/app/run_test.go b/internal/app/run_test.go index 8dcb756..e1c9b1c 100644 --- a/internal/app/run_test.go +++ b/internal/app/run_test.go @@ -781,6 +781,13 @@ func TestRunNotifiesAfterReplacement(t *testing.T) { if err := Run(context.Background(), RunOptions{ConfigPath: configPath}); err != nil { t.Fatalf("first Run() error = %v", err) } + writeSourceBundle(t, sourceRoot, "", testBundleOptions{ + Created: testutil.DefaultCreated.Add(time.Hour), + Files: []testFile{ + {Path: "report.md", Data: "# Report\nNew.\n"}, + {Path: "summary.txt", Data: "New summary\n"}, + }, + }) notifier := &recordingNotifier{} err := Run(context.Background(), RunOptions{ @@ -1063,6 +1070,13 @@ func TestRunNotifiesForAdditiveUpsert(t *testing.T) { if err := Run(context.Background(), RunOptions{ConfigPath: configPath}); err != nil { t.Fatalf("first Run() error = %v", err) } + writeSourceBundle(t, sourceRoot, "", testBundleOptions{ + Created: testutil.DefaultCreated.Add(time.Hour), + Files: []testFile{ + {Path: "report.md", Data: "# Report\nNew.\n"}, + {Path: "summary.txt", Data: "New summary\n"}, + }, + }) notifier := &recordingNotifier{} err := Run(context.Background(), RunOptions{ConfigPath: configPath, Notifier: notifier}) @@ -1366,7 +1380,6 @@ func TestRunReplacesHTMLIndexOutput(t *testing.T) { } func TestRunSkipsWhenDestinationStateMatches(t *testing.T) { - t.Skip("idempotent write is allowed for matching catalog outputs") sourceRoot := t.TempDir() destinationRoot := t.TempDir() writeSourceBundle(t, sourceRoot, "", testBundleOptions{}) @@ -1374,6 +1387,55 @@ func TestRunSkipsWhenDestinationStateMatches(t *testing.T) { if err := Run(context.Background(), RunOptions{ConfigPath: configPath}); err != nil { t.Fatalf("first Run() error = %v", err) } + statePath := filepath.Join(destinationRoot, storage.StateFileName) + stateBefore, err := os.ReadFile(statePath) + if err != nil { + t.Fatalf("read state before second run: %v", err) + } + reportBefore, err := os.ReadFile(filepath.Join(destinationRoot, "report.md")) + if err != nil { + t.Fatalf("read report before second run: %v", err) + } + notifier := &recordingNotifier{} + + var stdout bytes.Buffer + err = Run(context.Background(), RunOptions{ConfigPath: configPath, Stdout: &stdout, Notifier: notifier}) + if err != nil { + t.Fatalf("second Run() error = %v", err) + } + if !strings.Contains(stdout.String(), "action=skip_same") { + t.Fatalf("stdout = %q, want skip_same", stdout.String()) + } + if !strings.Contains(stdout.String(), "Final status: ok planned=1 publish_new=0 upsert_additive=0 replace_catalog=0 skip_same=1") { + t.Fatalf("stdout = %q, want skip_same summary", stdout.String()) + } + stateAfter, err := os.ReadFile(statePath) + if err != nil { + t.Fatalf("read state after second run: %v", err) + } + if string(stateAfter) != string(stateBefore) { + t.Fatalf("state changed during skip") + } + reportAfter, err := os.ReadFile(filepath.Join(destinationRoot, "report.md")) + if err != nil { + t.Fatalf("read report after second run: %v", err) + } + if string(reportAfter) != string(reportBefore) { + t.Fatalf("report changed during skip") + } + if got, want := len(notifier.events), 0; got != want { + t.Fatalf("notification count = %d, want %d", got, want) + } +} + +func TestRunReplacementSkipsWhenDestinationStateMatches(t *testing.T) { + sourceRoot := t.TempDir() + destinationRoot := t.TempDir() + writeSourceBundle(t, sourceRoot, "", testBundleOptions{}) + configPath := writeLocalConfigWithWorkflow(t, sourceRoot, destinationRoot, config.PathMappingPreserveRelative, config.WorkflowReplacement) + if err := Run(context.Background(), RunOptions{ConfigPath: configPath}); err != nil { + t.Fatalf("first Run() error = %v", err) + } var stdout bytes.Buffer err := Run(context.Background(), RunOptions{ConfigPath: configPath, Stdout: &stdout}) @@ -1561,8 +1623,8 @@ func TestRunExercisesRemoteBackendShapesThroughCommonPath(t *testing.T) { if err := runConfigWithBackendFactory(context.Background(), cfg, RunOptions{Stdout: &repeatOutput}, provider); err != nil { t.Fatalf("repeat error = %v", err) } - if got := strings.Count(repeatOutput.String(), "action=upsert_additive"); got != 4 { - t.Fatalf("repeat output = %q, upsert_additive count = %d, want 4", repeatOutput.String(), got) + if got := strings.Count(repeatOutput.String(), "action=skip_same"); got != 4 { + t.Fatalf("repeat output = %q, skip_same count = %d, want 4", repeatOutput.String(), got) } } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 1f7a900..75f010d 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -693,6 +693,42 @@ func TestExecuteRunJSONDryRun(t *testing.T) { } } +func TestExecuteRunJSONReportsSkipSame(t *testing.T) { + sourceRoot := t.TempDir() + destinationRoot := t.TempDir() + testutil.WriteSourceBundle(t, sourceRoot, "", testutil.BundleOptions{}) + configPath := testutil.WriteMinimalLocalConfig(t, sourceRoot, destinationRoot) + + var firstStdout, firstStderr bytes.Buffer + if code := Execute(context.Background(), []string{"run", "--config", configPath}, &firstStdout, &firstStderr); code != exitOK { + t.Fatalf("first exit code = %d, want %d; stderr = %q", code, exitOK, firstStderr.String()) + } + + var stdout, stderr bytes.Buffer + code := Execute(context.Background(), []string{"run", "--config", configPath, "--format", "json"}, &stdout, &stderr) + + if code != exitOK { + t.Fatalf("exit code = %d, want %d; stderr = %q", code, exitOK, stderr.String()) + } + envelope := decodeEnvelope(t, &stdout) + result := envelopeResult(t, envelope) + actions, ok := result["actions"].([]any) + if !ok || len(actions) != 1 { + t.Fatalf("actions = %#v, want one action", result["actions"]) + } + action, ok := actions[0].(map[string]any) + if !ok || action["action"] != "skip_same" { + t.Fatalf("action = %#v, want skip_same", actions[0]) + } + summary, ok := result["summary"].(map[string]any) + if !ok || summary["skip_same"] != float64(1) || summary["publish_new"] != float64(0) { + t.Fatalf("summary = %#v, want skip_same counter", result["summary"]) + } + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } +} + func TestExecuteRunJSONDryRunReportsFixedPathMapping(t *testing.T) { sourceRoot := t.TempDir() destinationRoot := t.TempDir() diff --git a/internal/publish/plan.go b/internal/publish/plan.go index 28267d6..fd52985 100644 --- a/internal/publish/plan.go +++ b/internal/publish/plan.go @@ -231,6 +231,7 @@ func planExistingCatalog(ctx context.Context, req Request, catalog state.Catalog Action: actionForWorkflow(workflow), CatalogOutputsToWrite: catalogOutputsForPlan(req, outputs, catalog.Outputs, scope, now), } + allPlannedOutputsMatch := catalogContainsMatchingOutputs(req, catalog.Outputs, outputs, scope) for _, output := range catalog.Outputs { if _, exists := planned[output.Path]; exists { continue @@ -241,9 +242,43 @@ func planExistingCatalog(ctx context.Context, req Request, catalog state.Catalog } details.CatalogOutputsToRetain = append(details.CatalogOutputsToRetain, output) } + if allPlannedOutputsMatch && (workflow == config.WorkflowAdditive || len(details.CatalogOutputsToDelete) == 0) { + details.Action = ActionSkipSame + details.CatalogOutputsToWrite = nil + details.CatalogOutputsToDelete = nil + } return details, nil } +func catalogContainsMatchingOutputs(req Request, existing []state.CatalogOutputFile, outputs []Output, scope state.OwnerScope) bool { + for _, output := range outputs { + catalogOutput, ok := state.FindCatalogOutputByPath(existing, output.DestinationPath) + if !ok || !catalogOutputMatchesPlan(req, catalogOutput, output, scope) { + return false + } + } + return true +} + +func catalogOutputMatchesPlan(req Request, catalogOutput state.CatalogOutputFile, output Output, scope state.OwnerScope) bool { + if catalogOutput.PipelineID != scope.PipelineID || + catalogOutput.DestinationID != scope.DestinationID || + catalogOutput.Source.ID != req.SourceBundle.Manifest.ID || + catalogOutput.Source.Digest != req.SourceBundle.Manifest.Digest || + !catalogOutput.Source.Created.Equal(req.SourceBundle.Manifest.Created) || + catalogOutput.Path != output.DestinationPath || + catalogOutput.Kind != output.Kind || + catalogOutput.URL != output.URL || + catalogOutput.SHA256 != output.SHA256 || + catalogOutput.Size != output.Size { + return false + } + if output.Kind == state.OutputKindGenerated { + return catalogOutput.SourcePath == output.SourcePath && catalogOutput.Transform == output.Transform + } + return catalogOutput.SourcePath == "" && catalogOutput.Transform == "" +} + func planSupersededLegacy(req Request, outputs []Output, workflow string, scope state.OwnerScope, now time.Time) catalogPlanDetails { details := catalogPlanDetails{ Action: actionForWorkflow(workflow), diff --git a/internal/publish/plan_test.go b/internal/publish/plan_test.go index 6ae270b..87c45fe 100644 --- a/internal/publish/plan_test.go +++ b/internal/publish/plan_test.go @@ -7,11 +7,13 @@ import ( "testing" "time" + "gitea.maximumdirect.net/eric/distributor/internal/bundle" "gitea.maximumdirect.net/eric/distributor/internal/config" "gitea.maximumdirect.net/eric/distributor/internal/state" "gitea.maximumdirect.net/eric/distributor/internal/storage" "gitea.maximumdirect.net/eric/distributor/internal/storage/fake" "gitea.maximumdirect.net/eric/distributor/internal/testutil" + "gitea.maximumdirect.net/eric/distributor/internal/transform" ) var ( @@ -108,6 +110,189 @@ func TestBuildReplacementDeletesCurrentOwnerAndRetainsOtherOwners(t *testing.T) } } +func TestBuildAdditiveSkipsMatchingCatalogOutputs(t *testing.T) { + _, destinationBackend, req := catalogPlanRequest(t, config.WorkflowAdditive) + existing := matchingCatalogForRequest(t, req) + existing.Outputs = append(existing.Outputs, catalogOutput(req, "reports", "web", "old.txt", state.OutputKindSource, planCreatedAt)) + writeCatalogState(t, destinationBackend, "", existing) + + plan, err := Build(context.Background(), req) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + if plan.Action != ActionSkipSame { + t.Fatalf("plan action = %s, want %s", plan.Action, ActionSkipSame) + } + if len(plan.CatalogOutputsToWrite) != 0 || len(plan.CatalogOutputsToDelete) != 0 { + t.Fatalf("catalog write=%d delete=%d, want no writes or deletes", len(plan.CatalogOutputsToWrite), len(plan.CatalogOutputsToDelete)) + } + if len(plan.CatalogOutputsToRetain) != 1 || plan.CatalogOutputsToRetain[0].Path != "old.txt" { + t.Fatalf("retained outputs = %#v, want old.txt", plan.CatalogOutputsToRetain) + } +} + +func TestBuildReplacementSkipsMatchingCatalogOutputs(t *testing.T) { + _, destinationBackend, req := catalogPlanRequest(t, config.WorkflowReplacement) + existing := matchingCatalogForRequest(t, req) + existing.Outputs = append(existing.Outputs, catalogOutput(req, "reports", "web", "shared.txt", state.OutputKindSource, planCreatedAt)) + writeCatalogState(t, destinationBackend, "", existing) + + plan, err := Build(context.Background(), req) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + if plan.Action != ActionSkipSame { + t.Fatalf("plan action = %s, want %s", plan.Action, ActionSkipSame) + } + if len(plan.CatalogOutputsToWrite) != 0 || len(plan.CatalogOutputsToDelete) != 0 { + t.Fatalf("catalog write=%d delete=%d, want no writes or deletes", len(plan.CatalogOutputsToWrite), len(plan.CatalogOutputsToDelete)) + } + if len(plan.CatalogOutputsToRetain) != 1 || plan.CatalogOutputsToRetain[0].Path != "shared.txt" { + t.Fatalf("retained outputs = %#v, want shared.txt", plan.CatalogOutputsToRetain) + } +} + +func TestBuildReplacementDoesNotSkipWhenCurrentOwnerOutputWouldBeDeleted(t *testing.T) { + _, destinationBackend, req := catalogPlanRequest(t, config.WorkflowReplacement) + existing := matchingCatalogForRequest(t, req) + existing.Outputs = append(existing.Outputs, catalogOutput(req, "reports", "archive", "stale.txt", state.OutputKindSource, planCreatedAt)) + writeCatalogState(t, destinationBackend, "", existing) + + plan, err := Build(context.Background(), req) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + if plan.Action != ActionReplaceCatalog { + t.Fatalf("plan action = %s, want %s", plan.Action, ActionReplaceCatalog) + } + if len(plan.CatalogOutputsToDelete) != 1 || plan.CatalogOutputsToDelete[0].Path != "stale.txt" { + t.Fatalf("delete outputs = %#v, want stale.txt", plan.CatalogOutputsToDelete) + } +} + +func TestBuildDoesNotSkipWhenCatalogMetadataDiffers(t *testing.T) { + tests := []struct { + name string + configure func(*Request) + mutate func(*state.CatalogState) + }{ + { + name: "pipeline id", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].PipelineID = "other" + }, + }, + { + name: "destination id", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].DestinationID = "web" + }, + }, + { + name: "source id", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].Source.ID = "reports.other" + }, + }, + { + name: "source digest", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].Source.Digest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + }, + }, + { + name: "source created", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].Source.Created = catalog.Outputs[0].Source.Created.Add(time.Second) + }, + }, + { + name: "output kind", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].Kind = state.OutputKindGenerated + catalog.Outputs[0].SourcePath = "report.md" + catalog.Outputs[0].Transform = transform.MarkdownToHTML + }, + }, + { + name: "output digest", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].SHA256 = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + }, + }, + { + name: "output size", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].Size++ + }, + }, + { + name: "url", + configure: func(req *Request) { + req.Links = &config.Links{BaseURL: "https://reports.example.com/archive", Primary: config.LinkPrimaryAuto} + }, + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].URL = "https://reports.example.com/archive/old-report.md" + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, destinationBackend, req := catalogPlanRequest(t, config.WorkflowAdditive) + if tt.configure != nil { + tt.configure(&req) + } + existing := matchingCatalogForRequest(t, req) + tt.mutate(&existing) + writeCatalogState(t, destinationBackend, "", existing) + + plan, err := Build(context.Background(), req) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + if plan.Action == ActionSkipSame { + t.Fatalf("plan action = %s, want write action after metadata change", plan.Action) + } + }) + } +} + +func TestBuildDoesNotSkipWhenGeneratedCatalogMetadataDiffers(t *testing.T) { + tests := []struct { + name string + mutate func(*state.CatalogState) + }{ + { + name: "source path", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].SourcePath = "summary.md" + }, + }, + { + name: "transform", + mutate: func(catalog *state.CatalogState) { + catalog.Outputs[0].Transform = "markdown_to_html_index" + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, destinationBackend, req := catalogGeneratedPlanRequest(t, config.WorkflowAdditive) + existing := matchingCatalogForRequest(t, req) + tt.mutate(&existing) + writeCatalogState(t, destinationBackend, "", existing) + + plan, err := Build(context.Background(), req) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + if plan.Action == ActionSkipSame { + t.Fatalf("plan action = %s, want write action after generated metadata change", plan.Action) + } + }) + } +} + func TestBuildTransfersManagedPathOwnership(t *testing.T) { _, destinationBackend, req := catalogPlanRequest(t, config.WorkflowReplacement) existing := baseCatalog(req) @@ -342,6 +527,41 @@ func catalogPlanRequest(t *testing.T, workflow string) (*fake.Backend, *fake.Bac } } +func catalogGeneratedPlanRequest(t *testing.T, workflow string) (*fake.Backend, *fake.Backend, Request) { + t.Helper() + sourceBackend, destinationBackend, req := catalogPlanRequest(t, workflow) + data := []byte("
Generated
\n") + req.Publish = config.PublishPolicy{HTML: true} + req.Transform = config.Transform{MarkdownToHTML: &config.MarkdownToHTML{ + Enabled: true, + Mode: config.TransformModeSidecar, + }} + req.Transformers = testResolver{transform.MarkdownToHTML: testTransformer{outputs: []transform.Output{{ + Path: "report.html", + SourcePath: "report.md", + Transform: transform.MarkdownToHTML, + Data: data, + SHA256: bundle.FileDigest(data), + Size: int64(len(data)), + }}}} + return sourceBackend, destinationBackend, req +} + +func matchingCatalogForRequest(t *testing.T, req Request) state.CatalogState { + t.Helper() + outputs, err := PlanOutputs(context.Background(), req) + if err != nil { + t.Fatalf("PlanOutputs() error = %v", err) + } + outputs, _, err = PlanLinks(req, outputs) + if err != nil { + t.Fatalf("PlanLinks() error = %v", err) + } + catalog := baseCatalog(req) + catalog.Outputs = catalogOutputsForPlan(req, outputs, nil, state.CurrentOwnerScope(req.PipelineID, req.DestinationID), planCreatedAt) + return catalog +} + func baseCatalog(req Request) state.CatalogState { return state.CatalogState{ SchemaVersion: state.CatalogSchemaVersion,