Implement catalog idempotent publish skips

2026-06-19 16:57:38 +00:00
parent 484fda2514
commit 2e0d903626
7 changed files with 365 additions and 6 deletions
--- 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)
 	}
 }

--- 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()
--- 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),
--- 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("<p>Generated</p>\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,