Let platform manifests go only on a confirmed collect, and copy again what a sweep took

An unrecorded index or a copy in progress can name a platform the records do not see, so
only a person's collect, after its dry run, takes an index's platforms, and only once every
kept index of each repository it touches was read. A copy missing a platform is copied
again, and a copy a build holds again is no longer recorded as collected (review of #144).

internal/artifacts/platforms_test.go

@@ -87,7 +87,11 @@ func TestAnIndexGoesWithItsPlatformsAndAKeptIndexKeepsItsOwn(t *testing.T) {
 	repository := "upstream/docker.io/library/golang"
 	old := catalogue.ArtifactStoreScheme + repository + "@" + digestN(1)
 	kept := catalogue.ArtifactStoreScheme + repository + "@" + digestN(2)
-	store.Spare = store.SpareKeptIndexes([]string{kept})
+	spare, err := store.SpareKeptIndexes(context.Background(), []string{kept}, []string{old})
+	if err != nil {
+		t.Fatal(err)
+	}
+	store.Spare = spare
 
 	if err := store.LetGo(context.Background(), old); err != nil {
 		t.Fatal(err)
@@ -113,7 +117,11 @@ func TestWithoutSparingAnIndexGoesAloneAndAnImageNamesNoPlatforms(t *testing.T)
 	}
 	// An image is one delete, with a Spare or without.
 	s.deleted = nil
-	store.Spare = store.SpareKeptIndexes(nil)
+	spare, err := store.SpareKeptIndexes(context.Background(), nil, []string{catalogue.ArtifactStoreScheme + "web/app@" + digestN(5)})
+	if err != nil {
+		t.Fatal(err)
+	}
+	store.Spare = spare
 	if err := store.LetGo(context.Background(), catalogue.ArtifactStoreScheme+"web/app@"+digestN(5)); err != nil {
 		t.Fatal(err)
 	}
@@ -122,7 +130,7 @@ func TestWithoutSparingAnIndexGoesAloneAndAnImageNamesNoPlatforms(t *testing.T)
 	}
 	// An index the store no longer holds is Gone, and nothing is deleted on its account.
 	s.deleted = nil
-	err := store.LetGo(context.Background(), catalogue.ArtifactStoreScheme+"web/base@"+digestN(9))
+	err = store.LetGo(context.Background(), catalogue.ArtifactStoreScheme+"web/base@"+digestN(9))
 	if !errors.Is(err, Gone) || len(s.deleted) != 0 {
 		t.Fatalf("an index the store does not hold: %v, deleted %v", err, s.deleted)
 	}
@@ -140,3 +148,29 @@ func TestASpareThatCannotBeReadKeepsTheIndex(t *testing.T) {
 		t.Fatalf("deleted %v without knowing what a kept index names", s.deleted)
 	}
 }
+
+// The spare list is read whole before the sweep: a kept index that cannot be read is an error before
+// anything is deleted, and a repository not read before keeps its index.
+func TestASpareListIsReadWholeBeforeAnythingGoes(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	t.Cleanup(server.Close)
+	store := Store{Address: strings.TrimPrefix(server.URL, "http://")}
+	kept := catalogue.ArtifactStoreScheme + "web/base@" + digestN(2)
+	old := catalogue.ArtifactStoreScheme + "web/base@" + digestN(1)
+	if _, err := store.SpareKeptIndexes(context.Background(), []string{kept}, []string{old}); err == nil {
+		t.Fatal("a kept index the store would not answer for was taken as naming nothing")
+	}
+
+	s := &indexStore{indexes: map[string][]string{digestN(1): {digestN(11)}}, images: map[string]bool{digestN(11): true}}
+	good := s.serve(t)
+	spare, err := good.SpareKeptIndexes(context.Background(), nil, []string{catalogue.ArtifactStoreScheme + "other/repo@" + digestN(3)})
+	if err != nil {
+		t.Fatal(err)
+	}
+	good.Spare = spare
+	if err := good.LetGo(context.Background(), old); err == nil || len(s.deleted) != 0 {
+		t.Fatalf("an index in a repository not read before the sweep was let go of: %v, deleted %v", err, s.deleted)
+	}
+}

internal/artifacts/store.go

@@ -169,44 +169,54 @@ func (s Store) Platforms(ctx context.Context, repository, digest string) ([]stri
 	return out, nil
 }
 
-// SpareKeptIndexes is a Spare for a sweep: the platform manifests the kept references of each
-// repository name, read from the store once per repository and remembered for the sweep. A kept
-// reference itself is spared too. A kept index the store does not hold names nothing.
-func (s Store) SpareKeptIndexes(kept []string) func(ctx context.Context, repository string) (map[string]bool, error) {
-	byRepository := map[string][]string{}
-	for _, reference := range kept {
+// SpareKeptIndexes is a Spare for a sweep, read whole before it starts: for every repository an
+// image reference of `touching` is in, each kept manifest there and every platform manifest a kept
+// index there names. Any read that fails is an error, and the sweep lets nothing go. A repository not
+// read here is answered with an error, so an index in it is kept. A kept index the store no longer
+// holds names nothing.
+func (s Store) SpareKeptIndexes(ctx context.Context, kept, touching []string) (func(context.Context, string) (map[string]bool, error), error) {
+	imageIn := func(reference string) (repository, digest string, ok bool) {
 		path, ours := catalogue.InArtifactStore(reference)
 		if !ours {
-			continue
+			return "", "", false
 		}
 		repository, kind, digest, err := split(path)
 		if err != nil || kind != "manifests" {
-			continue
+			return "", "", false
 		}
-		byRepository[repository] = append(byRepository[repository], digest)
+		return repository, digest, true
 	}
 	read := map[string]map[string]bool{}
-	return func(ctx context.Context, repository string) (map[string]bool, error) {
-		if spared, done := read[repository]; done {
-			return spared, nil
+	for _, reference := range touching {
+		if repository, _, ok := imageIn(reference); ok {
+			read[repository] = map[string]bool{}
 		}
-		spared := map[string]bool{}
-		for _, digest := range byRepository[repository] {
-			spared[digest] = true
-			children, err := s.Platforms(ctx, repository, digest)
-			if errors.Is(err, Gone) {
-				continue
-			}
-			if err != nil {
-				return nil, err
-			}
-			for _, c := range children {
-				spared[c] = true
-			}
-		}
-		read[repository] = spared
-		return spared, nil
 	}
+	for _, reference := range kept {
+		repository, digest, ok := imageIn(reference)
+		spared, touched := read[repository]
+		if !ok || !touched {
+			continue
+		}
+		spared[digest] = true
+		children, err := s.Platforms(ctx, repository, digest)
+		if errors.Is(err, Gone) {
+			continue
+		}
+		if err != nil {
+			return nil, fmt.Errorf("reading the kept %s@%s: %w", repository, digest, err)
+		}
+		for _, c := range children {
+			spared[c] = true
+		}
+	}
+	return func(_ context.Context, repository string) (map[string]bool, error) {
+		spared, done := read[repository]
+		if !done {
+			return nil, fmt.Errorf("%s was not read before the sweep began", repository)
+		}
+		return spared, nil
+	}, nil
 }
 
 // remove asks the store to delete what is at url. Gone when it has no such thing.