From cb8749ea2b83627a23cbecc8afdd45886573b14a Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Wed, 20 Sep 2023 11:09:56 +0000 Subject: [PATCH] bug fix and basic unit tests Signed-off-by: Xiaoxuan Wang --- internal/graph/deletablememory.go | 4 +- internal/graph/deletablememory_test.go | 379 +++++++++++++++++++++++++ 2 files changed, 381 insertions(+), 2 deletions(-) create mode 100644 internal/graph/deletablememory_test.go diff --git a/internal/graph/deletablememory.go b/internal/graph/deletablememory.go index 49ef64dc..54aa02a5 100644 --- a/internal/graph/deletablememory.go +++ b/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 diff --git a/internal/graph/deletablememory_test.go b/internal/graph/deletablememory_test.go new file mode 100644 index 00000000..ef2ee164 --- /dev/null +++ b/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") + // } +}