bug fix and basic unit tests

Signed-off-by: Xiaoxuan Wang <[email protected]>

internal/graph/deletablememory.go

@@ -108,7 +108,7 @@ func (m *DeletableMemory) Remove(ctx context.Context, node ocispec.Descriptor) e
 	defer m.lock.Unlock()
 	// remove the node from its successors' predecessor list
 	for successorKey := range m.successors[nodeKey] {
-		m.predecessors[successorKey].Delete(successorKey)
+		m.predecessors[successorKey].Delete(nodeKey)
 	}
 	delete(m.successors, nodeKey)
 	return nil
@@ -133,7 +133,7 @@ func (m *DeletableMemory) index(ctx context.Context, fetcher content.Fetcher, no
 	for _, successor := range successors {
 		successorKey := descriptor.FromOCI(successor)
 		successorSet.Add(successorKey)
-		predecessorSet, exists := m.predecessors[nodeKey]
+		predecessorSet, exists := m.predecessors[successorKey]
 		if !exists {
 			predecessorSet = set.New[descriptor.Descriptor]()
 			m.predecessors[successorKey] = predecessorSet

internal/graph/deletablememory_test.go

@@ -0,0 +1,379 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package graph
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"io"
+	"reflect"
+	"testing"
+
+	"github.com/opencontainers/go-digest"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"oras.land/oras-go/v2/internal/cas"
+	"oras.land/oras-go/v2/internal/descriptor"
+)
+
+/*
+A(manifest)-------------------+
+
+	|                         |
+	|                         |
+	|                         |
+	v                         |
+
+B(manifest)-----------------+ |
+
+	|                       | |
+	|                       | |
+	v                       v v
+
+C(layer)                  D(layer)
+*/
+func TestDeletableMemory_IndexAndRemove(t *testing.T) {
+	testFetcher := cas.NewMemory()
+	testDeletableMemory := NewDeletableMemory()
+	ctx := context.Background()
+
+	// generate test content
+	var blobs [][]byte
+	var descs []ocispec.Descriptor
+	appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor {
+		blobs = append(blobs, blob)
+		descs = append(descs, ocispec.Descriptor{
+			MediaType: mediaType,
+			Digest:    digest.FromBytes(blob),
+			Size:      int64(len(blob)),
+		})
+		return descs[len(descs)-1]
+	}
+	generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor {
+		manifest := ocispec.Manifest{
+			Layers: layers,
+		}
+		manifestJSON, err := json.Marshal(manifest)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON)
+	}
+	descC := appendBlob("layer node C", []byte("Node C is a layer")) // blobs[0], layer "C"
+	descD := appendBlob("layer node D", []byte("Node D is a layer")) // blobs[1], layer "D"
+	descB := generateManifest(descs[0:2]...)                         // blobs[2], manifest "B"
+	descA := generateManifest(descs[1:3]...)                         // blobs[3], manifest "A"
+	testFetcher.Push(ctx, descA, bytes.NewReader(blobs[3]))
+	testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2]))
+	testFetcher.Push(ctx, descC, bytes.NewReader(blobs[0]))
+	testFetcher.Push(ctx, descD, bytes.NewReader(blobs[1]))
+
+	// make sure that testFetcher works
+	rc, err := testFetcher.Fetch(ctx, descA)
+	if err != nil {
+		t.Errorf("testFetcher.Fetch() error = %v", err)
+	}
+	got, err := io.ReadAll(rc)
+	if err != nil {
+		t.Errorf("testFetcher.Fetch().Read() error = %v", err)
+	}
+	err = rc.Close()
+	if err != nil {
+		t.Errorf("testFetcher.Fetch().Close() error = %v", err)
+	}
+	if !bytes.Equal(got, blobs[3]) {
+		t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4])
+	}
+
+	// index test content into graph.DeletableMemory
+	testDeletableMemory.Index(ctx, testFetcher, descA)
+	testDeletableMemory.Index(ctx, testFetcher, descB)
+	testDeletableMemory.Index(ctx, testFetcher, descC)
+	testDeletableMemory.Index(ctx, testFetcher, descD)
+
+	// check that the content of testDeletableMemory.predecessors and successors
+	// are correct
+	nodeKeyA := descriptor.FromOCI(descA)
+	nodeKeyB := descriptor.FromOCI(descB)
+	nodeKeyC := descriptor.FromOCI(descC)
+	nodeKeyD := descriptor.FromOCI(descD)
+
+	// check the information of node A
+	predecessorsA := testDeletableMemory.predecessors[nodeKeyA]
+	successorsA := testDeletableMemory.successors[nodeKeyA]
+	for range predecessorsA {
+		t.Errorf("predecessors of %s should be empty", "A")
+	}
+	if !successorsA.Contains(nodeKeyB) {
+		t.Errorf("successors of %s should contain %s", "A", "B")
+	}
+	if !successorsA.Contains(nodeKeyD) {
+		t.Errorf("successors of %s should contain %s", "A", "D")
+	}
+
+	// check the information of node B
+	predecessorsB := testDeletableMemory.predecessors[nodeKeyB]
+	successorsB := testDeletableMemory.successors[nodeKeyB]
+	if !predecessorsB.Contains(nodeKeyA) {
+		t.Errorf("predecessors of %s should contain %s", "B", "A")
+	}
+	if !successorsB.Contains(nodeKeyC) {
+		t.Errorf("successors of %s should contain %s", "B", "C")
+	}
+	if !successorsB.Contains(nodeKeyD) {
+		t.Errorf("successors of %s should contain %s", "B", "D")
+	}
+
+	// check the information of node C
+	predecessorsC := testDeletableMemory.predecessors[nodeKeyC]
+	successorsC := testDeletableMemory.successors[nodeKeyC]
+	if !predecessorsC.Contains(nodeKeyB) {
+		t.Errorf("predecessors of %s should contain %s", "C", "B")
+	}
+	for range successorsC {
+		t.Errorf("successors of %s should be empty", "C")
+	}
+
+	// check the information of node D
+	predecessorsD := testDeletableMemory.predecessors[nodeKeyD]
+	successorsD := testDeletableMemory.successors[nodeKeyD]
+	if !predecessorsD.Contains(nodeKeyB) {
+		t.Errorf("predecessors of %s should contain %s", "D", "B")
+	}
+	if !predecessorsD.Contains(nodeKeyA) {
+		t.Errorf("predecessors of %s should contain %s", "D", "A")
+	}
+	for range successorsD {
+		t.Errorf("successors of %s should be empty", "C")
+	}
+
+	// remove node B and check the stored information
+	testDeletableMemory.Remove(ctx, descB)
+	if predecessorsC.Contains(nodeKeyB) {
+		t.Errorf("predecessors of %s should not contain %s", "C", "B")
+	}
+	if predecessorsD.Contains(nodeKeyB) {
+		t.Errorf("predecessors of %s should not contain %s", "D", "B")
+	}
+	if !successorsA.Contains(nodeKeyB) {
+		t.Errorf("successors of %s should still contain %s", "A", "B")
+	}
+	if _, exists := testDeletableMemory.successors[nodeKeyB]; exists {
+		t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B")
+	}
+
+	// remove node A and check the stored information
+	testDeletableMemory.Remove(ctx, descA)
+	if predecessorsD.Contains(nodeKeyA) {
+		t.Errorf("predecessors of %s should not contain %s", "D", "A")
+	}
+	if _, exists := testDeletableMemory.successors[nodeKeyA]; exists {
+		t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A")
+	}
+}
+
+func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) {
+	testFetcher := cas.NewMemory()
+	testDeletableMemory := NewDeletableMemory()
+	ctx := context.Background()
+
+	// generate test content
+	var blobs [][]byte
+	var descriptors []ocispec.Descriptor
+	appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor {
+		blobs = append(blobs, blob)
+		descriptors = append(descriptors, ocispec.Descriptor{
+			MediaType: mediaType,
+			Digest:    digest.FromBytes(blob),
+			Size:      int64(len(blob)),
+		})
+		return descriptors[len(descriptors)-1]
+	}
+	generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor {
+		manifest := ocispec.Manifest{
+			Layers: layers,
+		}
+		manifestJSON, err := json.Marshal(manifest)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON)
+	}
+	generateIndex := func(manifests ...ocispec.Descriptor) ocispec.Descriptor {
+		index := ocispec.Index{
+			Manifests: manifests,
+		}
+		indexJSON, err := json.Marshal(index)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return appendBlob(ocispec.MediaTypeImageIndex, indexJSON)
+	}
+	descE := appendBlob("layer node E", []byte("Node E is a layer")) // blobs[0], layer "E"
+	descF := appendBlob("layer node F", []byte("Node F is a layer")) // blobs[1], layer "F"
+	descB := generateManifest(descriptors[0:1]...)                   // blobs[2], manifest "B"
+	descC := generateManifest(descriptors[0:2]...)                   // blobs[3], manifest "C"
+	descD := generateManifest(descriptors[1:2]...)                   // blobs[4], manifest "D"
+	descA := generateIndex(descriptors[2:5]...)                      // blobs[5], index "A"
+	testFetcher.Push(ctx, descA, bytes.NewReader(blobs[5]))
+	testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2]))
+	testFetcher.Push(ctx, descC, bytes.NewReader(blobs[3]))
+	testFetcher.Push(ctx, descD, bytes.NewReader(blobs[4]))
+	testFetcher.Push(ctx, descE, bytes.NewReader(blobs[0]))
+	testFetcher.Push(ctx, descF, bytes.NewReader(blobs[1]))
+
+	// make sure that testFetcher works
+	rc, err := testFetcher.Fetch(ctx, descA)
+	if err != nil {
+		t.Errorf("testFetcher.Fetch() error = %v", err)
+	}
+	got, err := io.ReadAll(rc)
+	if err != nil {
+		t.Errorf("testFetcher.Fetch().Read() error = %v", err)
+	}
+	err = rc.Close()
+	if err != nil {
+		t.Errorf("testFetcher.Fetch().Close() error = %v", err)
+	}
+	if !bytes.Equal(got, blobs[5]) {
+		t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4])
+	}
+
+	// index node A into graph.DeletableMemory using IndexAll
+	testDeletableMemory.IndexAll(ctx, testFetcher, descA)
+
+	// check that the content of testDeletableMemory.predecessors and successors
+	// are correct
+	nodeKeyA := descriptor.FromOCI(descA)
+	nodeKeyB := descriptor.FromOCI(descB)
+	nodeKeyC := descriptor.FromOCI(descC)
+	nodeKeyD := descriptor.FromOCI(descD)
+	nodeKeyE := descriptor.FromOCI(descE)
+	nodeKeyF := descriptor.FromOCI(descF)
+
+	// check the information of node A
+	predecessorsA := testDeletableMemory.predecessors[nodeKeyA]
+	successorsA := testDeletableMemory.successors[nodeKeyA]
+	for range predecessorsA {
+		t.Errorf("predecessors of %s should be empty", "A")
+	}
+	if !successorsA.Contains(nodeKeyB) {
+		t.Errorf("successors of %s should contain %s", "A", "B")
+	}
+	if !successorsA.Contains(nodeKeyC) {
+		t.Errorf("successors of %s should contain %s", "A", "C")
+	}
+	if !successorsA.Contains(nodeKeyD) {
+		t.Errorf("successors of %s should contain %s", "A", "D")
+	}
+
+	// check the information of node B
+	predecessorsB := testDeletableMemory.predecessors[nodeKeyB]
+	successorsB := testDeletableMemory.successors[nodeKeyB]
+	if !predecessorsB.Contains(nodeKeyA) {
+		t.Errorf("predecessors of %s should contain %s", "B", "A")
+	}
+	if !successorsB.Contains(nodeKeyE) {
+		t.Errorf("successors of %s should contain %s", "B", "E")
+	}
+
+	// check the information of node C
+	predecessorsC := testDeletableMemory.predecessors[nodeKeyC]
+	successorsC := testDeletableMemory.successors[nodeKeyC]
+	if !predecessorsC.Contains(nodeKeyA) {
+		t.Errorf("predecessors of %s should contain %s", "C", "A")
+	}
+	if !successorsC.Contains(nodeKeyE) {
+		t.Errorf("successors of %s should contain %s", "C", "E")
+	}
+	if !successorsC.Contains(nodeKeyF) {
+		t.Errorf("successors of %s should contain %s", "C", "F")
+	}
+
+	// check the information of node D
+	predecessorsD := testDeletableMemory.predecessors[nodeKeyD]
+	successorsD := testDeletableMemory.successors[nodeKeyD]
+	if !predecessorsD.Contains(nodeKeyA) {
+		t.Errorf("predecessors of %s should contain %s", "D", "A")
+	}
+	if !successorsD.Contains(nodeKeyF) {
+		t.Errorf("successors of %s should contain %s", "D", "F")
+	}
+
+	// check the information of node E
+	predecessorsE := testDeletableMemory.predecessors[nodeKeyE]
+	successorsE := testDeletableMemory.successors[nodeKeyE]
+	if !predecessorsE.Contains(nodeKeyB) {
+		t.Errorf("predecessors of %s should contain %s", "E", "B")
+	}
+	if !predecessorsE.Contains(nodeKeyC) {
+		t.Errorf("predecessors of %s should contain %s", "E", "C")
+	}
+	for range successorsE {
+		t.Errorf("successors of %s should be empty", "E")
+	}
+
+	// check the information of node F
+	predecessorsF := testDeletableMemory.predecessors[nodeKeyF]
+	successorsF := testDeletableMemory.successors[nodeKeyF]
+	if !predecessorsF.Contains(nodeKeyC) {
+		t.Errorf("predecessors of %s should contain %s", "F", "C")
+	}
+	if !predecessorsF.Contains(nodeKeyD) {
+		t.Errorf("predecessors of %s should contain %s", "F", "D")
+	}
+	for range successorsF {
+		t.Errorf("successors of %s should be empty", "E")
+	}
+
+	// check the Predecessors
+	predsC, err := testDeletableMemory.Predecessors(ctx, descC)
+	if err != nil {
+		t.Errorf("testFetcher.Predecessors() error = %v", err)
+	}
+	if len(predsC) != 1 {
+		t.Errorf("%s should have length %d", "predsC", 1)
+	}
+	if !reflect.DeepEqual(predsC[0], descA) {
+		t.Errorf("incorrect predecessor result")
+	}
+
+	// TODO: change it to G
+
+	// // remove node B and check the stored information
+	// testDeletableMemory.Remove(ctx, descB)
+	// if predecessorsC.Contains(nodeKeyB) {
+	// 	t.Errorf("predecessors of %s should not contain %s", "C", "B")
+	// }
+	// if predecessorsD.Contains(nodeKeyB) {
+	// 	t.Errorf("predecessors of %s should not contain %s", "D", "B")
+	// }
+	// if !successorsA.Contains(nodeKeyB) {
+	// 	t.Errorf("successors of %s should still contain %s", "A", "B")
+	// }
+	// if _, exists := testDeletableMemory.successors[nodeKeyB]; exists {
+	// 	t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B")
+	// }
+
+	// // remove node A and check the stored information
+	// testDeletableMemory.Remove(ctx, descA)
+	// if predecessorsD.Contains(nodeKeyA) {
+	// 	t.Errorf("predecessors of %s should not contain %s", "D", "A")
+	// }
+	// if _, exists := testDeletableMemory.successors[nodeKeyA]; exists {
+	// 	t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A")
+	// }
+}