Preserve page creation outcomes through recovery
This commit is contained in:
@@ -423,16 +423,18 @@ func writeDeliveryReceipt(t *testing.T, repository string, settings cli.BuildSet
|
|||||||
}
|
}
|
||||||
|
|
||||||
type deliveryServerState struct {
|
type deliveryServerState struct {
|
||||||
commit string
|
malformedCreate bool
|
||||||
product string
|
failAssetListing bool
|
||||||
remoteCommit string
|
commit string
|
||||||
apiRequests int
|
product string
|
||||||
pageExists bool
|
remoteCommit string
|
||||||
releaseCreates int
|
apiRequests int
|
||||||
assetUploads int
|
pageExists bool
|
||||||
assetListReads int
|
releaseCreates int
|
||||||
noteBody string
|
assetUploads int
|
||||||
assets map[string][]byte
|
assetListReads int
|
||||||
|
noteBody string
|
||||||
|
assets map[string][]byte
|
||||||
}
|
}
|
||||||
|
|
||||||
func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Server {
|
func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Server {
|
||||||
@@ -483,10 +485,19 @@ func newDeliveryServer(t *testing.T, state *deliveryServerState) *httptest.Serve
|
|||||||
t.Errorf("release source payload = %#v", payload)
|
t.Errorf("release source payload = %#v", payload)
|
||||||
}
|
}
|
||||||
state.pageExists, state.releaseCreates, state.noteBody = true, state.releaseCreates+1, payload.Body
|
state.pageExists, state.releaseCreates, state.noteBody = true, state.releaseCreates+1, payload.Body
|
||||||
|
if state.malformedCreate {
|
||||||
|
w.WriteHeader(http.StatusCreated)
|
||||||
|
_ = json.NewEncoder(w).Encode(map[string]any{"id": 10, "tag_name": "v1.2.3", "name": "inconsistent response", "body": payload.Body})
|
||||||
|
return
|
||||||
|
}
|
||||||
writeDeliveryRelease(w, serverURL, state)
|
writeDeliveryRelease(w, serverURL, state)
|
||||||
case "/api/v1/repos/owner/example/releases/10/assets":
|
case "/api/v1/repos/owner/example/releases/10/assets":
|
||||||
if r.Method == http.MethodGet {
|
if r.Method == http.MethodGet {
|
||||||
state.assetListReads++
|
state.assetListReads++
|
||||||
|
if state.failAssetListing {
|
||||||
|
http.Error(w, "unavailable", 503)
|
||||||
|
return
|
||||||
|
}
|
||||||
list := make([]map[string]any, 0, len(state.assets))
|
list := make([]map[string]any, 0, len(state.assets))
|
||||||
for name, content := range state.assets {
|
for name, content := range state.assets {
|
||||||
list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": serverURL + "/files/" + name})
|
list = append(list, map[string]any{"id": len(list) + 1, "name": name, "size": len(content), "browser_download_url": serverURL + "/files/" + name})
|
||||||
@@ -675,3 +686,21 @@ func TestDeliverRejectsReceiptBuiltFromDirtySourceAfterCleanup(t *testing.T) {
|
|||||||
t.Fatalf("status=%d requests=%d %s", status, state.apiRequests, stderr.String())
|
t.Fatalf("status=%d requests=%d %s", status, state.apiRequests, stderr.String())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRecoveredPageCreatePreservesPartialDeliveryOnInspectionFailure(t *testing.T) {
|
||||||
|
repository := newCLIRepository(t)
|
||||||
|
state := &deliveryServerState{malformedCreate: true, failAssetListing: true}
|
||||||
|
server := newDeliveryServer(t, state)
|
||||||
|
defer server.Close()
|
||||||
|
build := cli.BuildSettings{MainPackage: ".", BinaryName: "helper", Targets: []string{"linux/amd64"}}
|
||||||
|
writeDeliveryProfile(t, repository, cli.Settings{Product: "Example Product", Build: &build, Delivery: &cli.DeliverySettings{BaseURL: server.URL, Owner: "owner", Repository: "example"}})
|
||||||
|
state.commit = cliGitOutput(t, repository, "rev-parse", "HEAD")
|
||||||
|
cliGitRun(t, repository, "tag", "v1.2.3")
|
||||||
|
assets, _ := writeDeliveryReceipt(t, repository, build)
|
||||||
|
t.Setenv("RELEASEKIT_GITEA_TOKEN", "fixture-token")
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
status := run(context.Background(), []string{"deliver", "--repo", repository, "--assets", assets, "v1.2.3"}, &stdout, &stderr)
|
||||||
|
if status != 1 || !strings.Contains(stderr.String(), "state=partial") || state.releaseCreates != 1 || state.assetListReads != 1 || state.assetUploads != 0 {
|
||||||
|
t.Fatalf("status=%d state=%#v %s", status, state, stderr.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -24,11 +24,13 @@ conflicting page.
|
|||||||
If the page is absent, the client posts to `/api/v1/repos/{owner}/{repo}/releases`
|
If the page is absent, the client posts to `/api/v1/repos/{owner}/{repo}/releases`
|
||||||
with the selected tag and commit, the release note's H1 as title, the complete
|
with the selected tag and commit, the release note's H1 as title, the complete
|
||||||
note as body, and draft and prerelease disabled. A create conflict, interrupted
|
note as body, and draft and prerelease disabled. A create conflict, interrupted
|
||||||
response, or malformed create response triggers one lookup. An exact page found
|
response, or malformed create response (including inconsistent decoded fields)
|
||||||
|
triggers one lookup. An exact page found
|
||||||
there confirms the existing content; a differing page is a conflict. If that
|
there confirms the existing content; a differing page is a conflict. If that
|
||||||
lookup cannot determine the result after a possibly successful write, the
|
lookup cannot determine the result after a possibly successful write, the
|
||||||
client returns an unknown outcome. Other HTTP failures retain their structured
|
client returns an unknown outcome. Other HTTP failures retain their structured
|
||||||
failure kind and status code.
|
failure kind and status code. A recovered create retains its write history;
|
||||||
|
a later asset-inspection failure reports partial page progress.
|
||||||
|
|
||||||
For a binary-producing release, the client can build a deterministic
|
For a binary-producing release, the client can build a deterministic
|
||||||
`SHA256SUMS` file from the declared artifact names and SHA-256 digests. Entries
|
`SHA256SUMS` file from the declared artifact names and SHA-256 digests. Entries
|
||||||
|
|||||||
@@ -96,6 +96,8 @@ cleanup. The offline suite and public boundary checks pass.
|
|||||||
|
|
||||||
### R2 — High: Some successful page writes are reported as not attempted
|
### R2 — High: Some successful page writes are reported as not attempted
|
||||||
|
|
||||||
|
Status: Resolved.
|
||||||
|
|
||||||
Category: Failure handling and recovery-state correctness.
|
Category: Failure handling and recovery-state correctness.
|
||||||
|
|
||||||
**Evidence:** [EnsureRelease](../../internal/gitea/release.go), at the
|
**Evidence:** [EnsureRelease](../../internal/gitea/release.go), at the
|
||||||
@@ -121,6 +123,12 @@ unknown. Retain the selected bounded reconciliation and immutable-content rules.
|
|||||||
its matching and conflicting follow-up lookups, and a recovered create followed
|
its matching and conflicting follow-up lookups, and a recovered create followed
|
||||||
by asset-inspection failure. Assert both request counts and returned state.
|
by asset-inspection failure. Assert both request counts and returned state.
|
||||||
|
|
||||||
|
**Resolution:** Semantic create-response failures now use one reconciliation
|
||||||
|
lookup. Recovered page results retain create-request history, preserving partial
|
||||||
|
progress on later asset-inspection failure. Local HTTP tests cover matching,
|
||||||
|
conflicting, and unavailable lookups; a CLI fixture verifies recovered creation
|
||||||
|
followed by failed asset inspection. Both affected suites pass.
|
||||||
|
|
||||||
### R3 — Medium: Read-only asset failures are classified as unknown writes
|
### R3 — Medium: Read-only asset failures are classified as unknown writes
|
||||||
|
|
||||||
Category: Divergence from the selected outcome contract.
|
Category: Divergence from the selected outcome contract.
|
||||||
|
|||||||
@@ -111,7 +111,7 @@ func Deliver(ctx context.Context, request Request) (Result, error) {
|
|||||||
inspection, err := (gitea.Client{}).InspectAssets(ctx, request.Destination, releaseResult.Release.ID, request.Assets)
|
inspection, err := (gitea.Client{}).InspectAssets(ctx, request.Destination, releaseResult.Release.ID, request.Assets)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
result.State = NotAttempted
|
result.State = NotAttempted
|
||||||
if releaseResult.Created {
|
if releaseResult.WriteAttempted {
|
||||||
result.State = Partial
|
result.State = Partial
|
||||||
}
|
}
|
||||||
result.Uncertain = assetIdentities(request.Assets)
|
result.Uncertain = assetIdentities(request.Assets)
|
||||||
|
|||||||
@@ -75,8 +75,9 @@ type Release struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
type Result struct {
|
type Result struct {
|
||||||
Release Release
|
Release Release
|
||||||
Created bool
|
Created bool
|
||||||
|
WriteAttempted bool // a create request was issued, including reconciled creates.
|
||||||
}
|
}
|
||||||
|
|
||||||
type Client struct {
|
type Client struct {
|
||||||
@@ -150,7 +151,12 @@ func (c Client) EnsureRelease(ctx context.Context, request Request) (Result, err
|
|||||||
}
|
}
|
||||||
created, postStatus, postErr := c.postRelease(ctx, client, request.Destination.Token, apiRoot+"/releases", body)
|
created, postStatus, postErr := c.postRelease(ctx, client, request.Destination.Token, apiRoot+"/releases", body)
|
||||||
if postErr == nil {
|
if postErr == nil {
|
||||||
return acceptCreated(created, request)
|
result, verifyErr := acceptCreated(created, request)
|
||||||
|
if verifyErr == nil {
|
||||||
|
result.WriteAttempted = true
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
postErr = verifyErr
|
||||||
}
|
}
|
||||||
if !shouldReconcileWrite(postStatus, postErr) {
|
if !shouldReconcileWrite(postStatus, postErr) {
|
||||||
return Result{}, postErr
|
return Result{}, postErr
|
||||||
@@ -162,6 +168,7 @@ func (c Client) EnsureRelease(ctx context.Context, request Request) (Result, err
|
|||||||
if retryErr == nil {
|
if retryErr == nil {
|
||||||
result, matchErr := acceptExisting(retry, request)
|
result, matchErr := acceptExisting(retry, request)
|
||||||
if matchErr == nil {
|
if matchErr == nil {
|
||||||
|
result.WriteAttempted = true
|
||||||
return result, nil
|
return result, nil
|
||||||
}
|
}
|
||||||
return Result{}, failure(FailureConflict, "resolve release creation", postStatus, matchErr)
|
return Result{}, failure(FailureConflict, "resolve release creation", postStatus, matchErr)
|
||||||
|
|||||||
@@ -339,3 +339,57 @@ func failureKind(t *testing.T, err error) FailureKind {
|
|||||||
func contains(value, substring string) bool {
|
func contains(value, substring string) bool {
|
||||||
return len(substring) > 0 && len(value) >= len(substring) && strings.Contains(value, substring)
|
return len(substring) > 0 && len(value) >= len(substring) && strings.Contains(value, substring)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestEnsureReleaseReconcilesInconsistentSuccessfulCreate(t *testing.T) {
|
||||||
|
for _, mode := range []string{"matching", "conflicting", "unavailable"} {
|
||||||
|
t.Run(mode, func(t *testing.T) {
|
||||||
|
posts, lookups := 0, 0
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
switch {
|
||||||
|
case strings.Contains(r.URL.Path, "/tags/") && !strings.Contains(r.URL.Path, "/releases/"):
|
||||||
|
writeTag(t, w, testCommit)
|
||||||
|
case strings.Contains(r.URL.Path, "/releases/tags/"):
|
||||||
|
lookups++
|
||||||
|
if posts == 0 {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if mode == "unavailable" {
|
||||||
|
http.Error(w, "unavailable", 503)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
body := testNote
|
||||||
|
if mode == "conflicting" {
|
||||||
|
body = "other content"
|
||||||
|
}
|
||||||
|
writeRelease(t, w, releaseFixture{title: "Example v1.2.3", body: body})
|
||||||
|
case r.Method == http.MethodPost:
|
||||||
|
posts++
|
||||||
|
w.WriteHeader(http.StatusCreated)
|
||||||
|
json.NewEncoder(w).Encode(releaseResponse{ID: 10, TagName: "v1.2.3", Name: "inconsistent title", Body: testNote})
|
||||||
|
default:
|
||||||
|
t.Errorf("unexpected request %s", r.URL.Path)
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
result, err := testClient(server).EnsureRelease(context.Background(), testRequest(server.URL))
|
||||||
|
if posts != 1 || lookups != 2 {
|
||||||
|
t.Fatalf("posts=%d lookups=%d", posts, lookups)
|
||||||
|
}
|
||||||
|
switch mode {
|
||||||
|
case "matching":
|
||||||
|
if err != nil || !result.WriteAttempted {
|
||||||
|
t.Fatalf("result=%#v err=%v", result, err)
|
||||||
|
}
|
||||||
|
case "conflicting":
|
||||||
|
if failureKind(t, err) != FailureConflict {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
case "unavailable":
|
||||||
|
if failureKind(t, err) != FailureUnknown {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user