Honor cancellation during comparison replacement
This commit is contained in:
@@ -88,6 +88,10 @@ case-variant, or duplicate fields; multiple JSON values; extra entries;
|
|||||||
symlinks; and future or otherwise unsupported versions. Treat a bundle that
|
symlinks; and future or otherwise unsupported versions. Treat a bundle that
|
||||||
fails recognition as an ordinary directory, not as a compatible bundle.
|
fails recognition as an ordinary directory, not as a compatible bundle.
|
||||||
|
|
||||||
|
When replacing a recognized bundle, cancellation observed before the new
|
||||||
|
bundle is installed preserves the prior bundle rather than committing the
|
||||||
|
replacement.
|
||||||
|
|
||||||
The manifest contains safe operational provenance, but `data-package.yml` and
|
The manifest contains safe operational provenance, but `data-package.yml` and
|
||||||
the generated Markdown can contain sensitive weather or location context. Do
|
the generated Markdown can contain sensitive weather or location context. Do
|
||||||
not assume these artifacts are safe for public distribution. Handle retention,
|
not assume these artifacts are safe for public distribution. Handle retention,
|
||||||
|
|||||||
@@ -18,8 +18,9 @@ symlinks and non-directories, accepts a missing or empty directory, and permits
|
|||||||
replacement only for a recognized current bundle. Publication rechecks that
|
replacement only for a recognized current bundle. Publication rechecks that
|
||||||
authorization immediately before it writes a private sibling staging directory.
|
authorization immediately before it writes a private sibling staging directory.
|
||||||
For replacement, it moves the prior bundle to a private sibling backup,
|
For replacement, it moves the prior bundle to a private sibling backup,
|
||||||
reauthorizes that moved entry, and restores it if installing the new bundle
|
reauthorizes that moved entry, checks for cancellation, and restores it if
|
||||||
fails.
|
cancellation or installing the new bundle prevents replacement. If guarded
|
||||||
|
restoration fails, the error retains the prior bundle's recovery path.
|
||||||
|
|
||||||
Planning also validates the final component and the bounded fixed names used
|
Planning also validates the final component and the bounded fixed names used
|
||||||
for private staging and backup siblings. A destination that cannot form those
|
for private staging and backup siblings. A destination that cannot form those
|
||||||
|
|||||||
@@ -123,9 +123,12 @@ rechecked immediately before an atomic publish. A missing or empty directory
|
|||||||
is usable. A nonempty directory can be replaced only when `--replace` is given
|
is usable. A nonempty directory can be replaced only when `--replace` is given
|
||||||
and it is recognized as a current Weatherreporter comparison bundle; ordinary
|
and it is recognized as a current Weatherreporter comparison bundle; ordinary
|
||||||
directories, symlinks, and unsafe destinations are rejected. Cancellation and
|
directories, symlinks, and unsafe destinations are rejected. Cancellation and
|
||||||
all failures before publication preserve an existing bundle. Profile failures
|
all failures before publication preserve an existing bundle, including a
|
||||||
are different: the command publishes a complete partial bundle, with failed
|
cancellation observed while a replacement is being prepared. If guarded
|
||||||
profiles represented in the manifest and no Markdown file for those profiles.
|
restoration cannot complete, the error names the retained sibling bundle for
|
||||||
|
manual recovery. Profile failures are different: the command publishes a
|
||||||
|
complete partial bundle, with failed profiles represented in the manifest and
|
||||||
|
no Markdown file for those profiles.
|
||||||
Comparison preflight also checks that private publication siblings can be
|
Comparison preflight also checks that private publication siblings can be
|
||||||
formed. An infeasible destination name is rejected before a missing parent
|
formed. An infeasible destination name is rejected before a missing parent
|
||||||
directory is created.
|
directory is created.
|
||||||
|
|||||||
@@ -550,6 +550,9 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
|
|||||||
if err := authorizeMovedDestination(currentPlan, backupDirectory); err != nil {
|
if err := authorizeMovedDestination(currentPlan, backupDirectory); err != nil {
|
||||||
return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err)
|
return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err)
|
||||||
}
|
}
|
||||||
|
if err := ctx.Err(); err != nil {
|
||||||
|
return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err)
|
||||||
|
}
|
||||||
if err := operations.rename(temporaryDirectory, currentPlan.Target); err != nil {
|
if err := operations.rename(temporaryDirectory, currentPlan.Target); err != nil {
|
||||||
return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, fmt.Errorf("replace comparison destination %q: %w", currentPlan.Target, err))
|
return PublicationResult{}, restoreMovedDestination(operations, backupDirectory, currentPlan.Target, fmt.Errorf("replace comparison destination %q: %w", currentPlan.Target, err))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -685,6 +685,154 @@ func TestPublishCancellationBeforeCommitLeavesNoDestination(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestPublishRestoresPriorDestinationWhenCanceledAfterBackup(t *testing.T) {
|
||||||
|
for _, test := range []struct {
|
||||||
|
name string
|
||||||
|
prepare func(t *testing.T, workingDirectory, target string)
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "empty directory",
|
||||||
|
prepare: func(t *testing.T, _, target string) {
|
||||||
|
t.Helper()
|
||||||
|
if err := os.Mkdir(target, 0o700); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "recognized bundle",
|
||||||
|
prepare: func(t *testing.T, workingDirectory, target string) {
|
||||||
|
t.Helper()
|
||||||
|
plan, err := PlanDestination(workingDirectory, target, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, err := Publish(context.Background(), plan, testBundle()); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
workingDirectory := t.TempDir()
|
||||||
|
target := filepath.Join(workingDirectory, "comparison-daily")
|
||||||
|
test.prepare(t, workingDirectory, target)
|
||||||
|
before := directorySnapshot(t, target)
|
||||||
|
|
||||||
|
plan, err := PlanDestination(workingDirectory, target, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
next := testBundle()
|
||||||
|
next.Reports[0].Markdown = []byte("# Replacement\n")
|
||||||
|
publication, err := publish(ctx, plan, next, publishOperations{rename: func(oldPath, newPath string) error {
|
||||||
|
if err := os.Rename(oldPath, newPath); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if oldPath == target {
|
||||||
|
cancel()
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}})
|
||||||
|
if publication.Committed || !errors.Is(err, context.Canceled) {
|
||||||
|
t.Fatalf("publish() result/error = %#v/%v, want canceled uncommitted publication", publication, err)
|
||||||
|
}
|
||||||
|
if after := directorySnapshot(t, target); !equalSnapshots(before, after) {
|
||||||
|
t.Fatalf("restored destination = %#v, want %#v", after, before)
|
||||||
|
}
|
||||||
|
assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(target))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPublishRetainsBackupWhenCancellationRestorationFails(t *testing.T) {
|
||||||
|
workingDirectory := t.TempDir()
|
||||||
|
target := filepath.Join(workingDirectory, "comparison-daily")
|
||||||
|
initialPlan, err := PlanDestination(workingDirectory, target, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, err := Publish(context.Background(), initialPlan, testBundle()); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
plan, err := PlanDestination(workingDirectory, target, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
restoreCause := errors.New("restore failed")
|
||||||
|
var backupPath string
|
||||||
|
calls := 0
|
||||||
|
publication, err := publish(ctx, plan, testBundle(), publishOperations{rename: func(oldPath, newPath string) error {
|
||||||
|
calls++
|
||||||
|
if calls == 2 {
|
||||||
|
return restoreCause
|
||||||
|
}
|
||||||
|
if err := os.Rename(oldPath, newPath); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if calls == 1 {
|
||||||
|
backupPath = newPath
|
||||||
|
cancel()
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}})
|
||||||
|
if publication.Committed || !errors.Is(err, context.Canceled) || !errors.Is(err, restoreCause) {
|
||||||
|
t.Fatalf("publish() result/error = %#v/%v, want cancellation and restoration failure", publication, err)
|
||||||
|
}
|
||||||
|
if info, statErr := os.Stat(backupPath); statErr != nil || !info.IsDir() {
|
||||||
|
t.Fatalf("backup stat = %v, %v, want retained directory", info, statErr)
|
||||||
|
}
|
||||||
|
if _, statErr := os.Lstat(target); !errors.Is(statErr, os.ErrNotExist) {
|
||||||
|
t.Fatalf("destination stat error = %v, want not exist", statErr)
|
||||||
|
}
|
||||||
|
assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(backupPath))
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPublishReturnsCommittedBundleWhenCanceledAfterInstall(t *testing.T) {
|
||||||
|
workingDirectory := t.TempDir()
|
||||||
|
target := filepath.Join(workingDirectory, "comparison-daily")
|
||||||
|
initialPlan, err := PlanDestination(workingDirectory, target, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, err := Publish(context.Background(), initialPlan, testBundle()); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
next := testBundle()
|
||||||
|
next.Reports[0].Markdown = []byte("# Next\n")
|
||||||
|
plan, err := PlanDestination(workingDirectory, target, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
publication, err := publish(ctx, plan, next, publishOperations{rename: func(oldPath, newPath string) error {
|
||||||
|
if err := os.Rename(oldPath, newPath); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if newPath == target && oldPath != target {
|
||||||
|
cancel()
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}})
|
||||||
|
if !publication.Committed || err != nil {
|
||||||
|
t.Fatalf("publish() result/error = %#v/%v, want committed bundle", publication, err)
|
||||||
|
}
|
||||||
|
if err := ctx.Err(); !errors.Is(err, context.Canceled) {
|
||||||
|
t.Fatalf("context error = %v, want cancellation", err)
|
||||||
|
}
|
||||||
|
if got := string(readFile(t, filepath.Join(target, next.Reports[0].Path))); got != string(next.Reports[0].Markdown) {
|
||||||
|
t.Fatalf("published report = %q, want %q", got, next.Reports[0].Markdown)
|
||||||
|
}
|
||||||
|
if _, err := RecognizeBundle(target); err != nil {
|
||||||
|
t.Fatalf("committed bundle recognition error = %v", err)
|
||||||
|
}
|
||||||
|
assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(target))
|
||||||
|
}
|
||||||
|
|
||||||
func assertDestinationErrorKind(t *testing.T, workingDirectory, target string, replace bool, want DestinationErrorKind) {
|
func assertDestinationErrorKind(t *testing.T, workingDirectory, target string, replace bool, want DestinationErrorKind) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
_, err := PlanDestination(workingDirectory, target, replace)
|
_, err := PlanDestination(workingDirectory, target, replace)
|
||||||
|
|||||||
Reference in New Issue
Block a user