7253: update BlockIterWithDuplicates to return any errors through Manifest, rather...

diff --git a/sdk/go/manifest/manifest.go b/sdk/go/manifest/manifest.go
index 49faa01b4d56b2eb32ab0c60d7e68a0c4966df87..6deb88f3c146daa9c82dd57caeb7cb46f0a6012d 100644 (file)
--- a/sdk/go/manifest/manifest.go
+++ b/sdk/go/manifest/manifest.go
@@ -20,6 +20,7 @@ var LocatorPattern = regexp.MustCompile(
 
 type Manifest struct {
        Text string
+       Err  error
 }
 
 type BlockLocator struct {
@@ -246,28 +247,24 @@ func (m *Manifest) FileSegmentIterByName(filepath string) <-chan *FileSegment {
        return ch
 }
 
-type ManifestBlockLocator struct {
-       Locator blockdigest.BlockLocator
-       Err     error
-}
-
 // Blocks may appear mulitple times within the same manifest if they
 // are used by multiple files. In that case this Iterator will output
 // the same block multiple times.
-func (m *Manifest) BlockIterWithDuplicates() <-chan ManifestBlockLocator {
-       blockChannel := make(chan ManifestBlockLocator)
+//
+// In order to detect parse errors, caller must check m.Err after the returned channel closes.
+func (m *Manifest) BlockIterWithDuplicates() <-chan blockdigest.BlockLocator {
+       blockChannel := make(chan blockdigest.BlockLocator)
        go func(streamChannel <-chan ManifestStream) {
                for ms := range streamChannel {
                        if ms.Err != nil {
-                               blockChannel <- ManifestBlockLocator{Locator: blockdigest.BlockLocator{}, Err: ms.Err}
+                               m.Err = ms.Err
                                continue
                        }
                        for _, block := range ms.Blocks {
-                               b, err := blockdigest.ParseBlockLocator(block)
-                               if err == nil {
-                                       blockChannel <- ManifestBlockLocator{b, nil}
+                               if b, err := blockdigest.ParseBlockLocator(block); err == nil {
+                                       blockChannel <- b
                                } else {
-                                       blockChannel <- ManifestBlockLocator{Locator: blockdigest.BlockLocator{}, Err: err}
+                                       m.Err = err
                                }
                        }
                }

diff --git a/sdk/go/manifest/manifest_test.go b/sdk/go/manifest/manifest_test.go
index 65ea7f1e471c6276c9b0b74b97079d78781a1843..53d4a15af60a7dd0bb03ece18f16ecb7e1a3a9bf 100644 (file)
--- a/sdk/go/manifest/manifest_test.go
+++ b/sdk/go/manifest/manifest_test.go
@@ -96,7 +96,7 @@ func TestStreamIterShortManifestWithBlankStreams(t *testing.T) {
        if err != nil {
                t.Fatalf("Unexpected error reading manifest from file: %v", err)
        }
-       manifest := Manifest{string(content)}
+       manifest := Manifest{Text: string(content)}
        streamIter := manifest.StreamIter()
 
        firstStream := <-streamIter
@@ -118,25 +118,25 @@ func TestBlockIterLongManifest(t *testing.T) {
        if err != nil {
                t.Fatalf("Unexpected error reading manifest from file: %v", err)
        }
-       manifest := Manifest{string(content)}
+       manifest := Manifest{Text: string(content)}
        blockChannel := manifest.BlockIterWithDuplicates()
 
        firstBlock := <-blockChannel
 
        expectBlockLocator(t,
-               firstBlock.Locator,
+               firstBlock,
                blockdigest.BlockLocator{Digest: blockdigest.AssertFromString("b746e3d2104645f2f64cd3cc69dd895d"),
                        Size:  15693477,
                        Hints: []string{"E2866e643690156651c03d876e638e674dcd79475@5441920c"}})
        blocksRead := 1
-       var lastBlock ManifestBlockLocator
+       var lastBlock blockdigest.BlockLocator
        for lastBlock = range blockChannel {
                blocksRead++
        }
        expectEqual(t, blocksRead, 853)
 
        expectBlockLocator(t,
-               lastBlock.Locator,
+               lastBlock,
                blockdigest.BlockLocator{Digest: blockdigest.AssertFromString("f9ce82f59e5908d2d70e18df9679b469"),
                        Size:  31367794,
                        Hints: []string{"E53f903684239bcc114f7bf8ff9bd6089f33058db@5441920c"}})
@@ -211,24 +211,21 @@ func TestBlockIterWithBadManifest(t *testing.T) {
        }
 
        for _, testCase := range testCases {
-               manifest := Manifest{string(testCase[0])}
+               manifest := Manifest{Text: string(testCase[0])}
                blockChannel := manifest.BlockIterWithDuplicates()
 
-               var err error
                for block := range blockChannel {
-                       if block.Err != nil {
-                               err = block.Err
-                       }
+                       _ = block
                }
 
                // completed reading from blockChannel; now check for errors
-               if err == nil {
+               if manifest.Err == nil {
                        t.Errorf("Expected error")
                }
 
-               matched, _ := regexp.MatchString(testCase[1], err.Error())
+               matched, _ := regexp.MatchString(testCase[1], manifest.Err.Error())
                if !matched {
-                       t.Errorf("Expected error not found. Expected: %v; Found: %v", testCase[1], err.Error())
+                       t.Errorf("Expected error not found. Expected: %v; Found: %v", testCase[1], manifest.Err.Error())
                }
        }
 }

diff --git a/services/datamanager/collection/collection.go b/services/datamanager/collection/collection.go
index 0d583c2c99845ffcbd0841b73e28a78ab90d1592..df6852687a621fccbb9606b99ae30cfa81bf3dca 100644 (file)
--- a/services/datamanager/collection/collection.go
+++ b/services/datamanager/collection/collection.go
@@ -274,7 +274,7 @@ func ProcessCollections(arvLogger *logger.Logger,
                        collection.ReplicationLevel = defaultReplicationLevel
                }
 
-               manifest := manifest.Manifest{sdkCollection.ManifestText}
+               manifest := manifest.Manifest{Text: sdkCollection.ManifestText}
                manifestSize := uint64(len(sdkCollection.ManifestText))
 
                if _, alreadySeen := UUIDToCollection[collection.UUID]; !alreadySeen {
@@ -286,21 +286,22 @@ func ProcessCollections(arvLogger *logger.Logger,
 
                blockChannel := manifest.BlockIterWithDuplicates()
                for block := range blockChannel {
-                       if block.Err != nil {
-                               err = block.Err
-                               return
-                       }
-                       if storedSize, stored := collection.BlockDigestToSize[block.Locator.Digest]; stored && storedSize != block.Locator.Size {
+                       if storedSize, stored := collection.BlockDigestToSize[block.Digest]; stored && storedSize != block.Size {
                                err = fmt.Errorf(
                                        "Collection %s contains multiple sizes (%d and %d) for block %s",
                                        collection.UUID,
                                        storedSize,
-                                       block.Locator.Size,
-                                       block.Locator.Digest)
+                                       block.Size,
+                                       block.Digest)
                                return
                        }
-                       collection.BlockDigestToSize[block.Locator.Digest] = block.Locator.Size
+                       collection.BlockDigestToSize[block.Digest] = block.Size
                }
+               if manifest.Err != nil {
+                       err = manifest.Err
+                       return
+               }
+
                collection.TotalSize = 0
                for _, size := range collection.BlockDigestToSize {
                        collection.TotalSize += size