Responded to Tim's comments.

Switched to using bufio.scanner, some other cleanup and added some tests.

diff --git a/services/datamanager/collection/collection.go b/services/datamanager/collection/collection.go
index 03b2f59317b4e55c945170811d5c5771f75c1d18..48db7bdafa463d5ac760772ba51c08dc387f6859 100644 (file)
--- a/services/datamanager/collection/collection.go
+++ b/services/datamanager/collection/collection.go
@@ -66,7 +66,7 @@ func GetCollections(arv arvadosclient.ArvadosClient) (results readCollections) {
                                OwnerUuid: item_map["owner_uuid"].(string),
                                BlockDigestToSize: make(map[string]int)}
                        manifest := manifest.Manifest{item_map["manifest_text"].(string)}
-                       blockChannel := manifest.DuplicateBlockIter()
+                       blockChannel := manifest.BlockIterWithDuplicates()
                        for block := range blockChannel {
                                if stored_size, stored := collection.BlockDigestToSize[block.Digest];
                                stored && stored_size != block.Size {

diff --git a/services/datamanager/manifest/manifest.go b/services/datamanager/manifest/manifest.go
index 11b96d8d76c1345445b94b63a01c3ee2f578951d..ad0948847e03778be0f3d8b854c10d5fa7146a90 100644 (file)
--- a/services/datamanager/manifest/manifest.go
+++ b/services/datamanager/manifest/manifest.go
@@ -5,6 +5,7 @@
 package manifest
 
 import (
+       "bufio"
        "fmt"
        "log"
        "regexp"
@@ -56,9 +57,8 @@ func parseManifestLine(s string) (m ManifestLine) {
        m.StreamName = tokens[0]
        tokens = tokens[1:]
        var i int
-       var token string
-       for i, token = range tokens {
-               if !LocatorPattern.MatchString(token) {
+       for i = range tokens {
+               if !LocatorPattern.MatchString(tokens[i]) {
                        break
                }
        }
@@ -69,16 +69,12 @@ func parseManifestLine(s string) (m ManifestLine) {
 
 func (m *Manifest) LineIter() <-chan ManifestLine {
        ch := make(chan ManifestLine)
-       go func(remaining string) {
-               for {
+       go func(input string) {
+               scanner := bufio.NewScanner(strings.NewReader(input))
+               for scanner.Scan() {
                        // We parse one line at a time, to save effort if we only need
                        // the first few lines.
-                       splitsies := strings.SplitN(remaining, "\n", 2)
-                       ch <- parseManifestLine(splitsies[0])
-                       if len(splitsies) == 1 {
-                               break
-                       }
-                       remaining = splitsies[1]
+                       ch <- parseManifestLine(scanner.Text())
                }
                close(ch)
        }(m.Text)
@@ -89,7 +85,7 @@ func (m *Manifest) LineIter() <-chan ManifestLine {
 // 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) DuplicateBlockIter() <-chan BlockLocator {
+func (m *Manifest) BlockIterWithDuplicates() <-chan BlockLocator {
        blockChannel := make(chan BlockLocator)
        go func(lineChannel <-chan ManifestLine) {
                for m := range lineChannel {

diff --git a/services/datamanager/manifest/manifest_test.go b/services/datamanager/manifest/manifest_test.go
index 2bf8cdaa7504780d28c9f7c89e54292b6a73e20f..6cd16bccc61dbe866948de8e1bc5ceda470ec9cd 100644 (file)
--- a/services/datamanager/manifest/manifest_test.go
+++ b/services/datamanager/manifest/manifest_test.go
@@ -74,8 +74,36 @@ func expectLocatorPatternMatch(t *testing.T, s string) {
        }
 }
 
+func expectLocatorPatternFail(t *testing.T, s string) {
+       if LocatorPattern.MatchString(s) {
+               t.Fatalf("Expected \"%s\" to fail locator pattern but it passed.",
+                       s)
+       }
+}
+
 func TestLocatorPatternBasic(t *testing.T) {
        expectLocatorPatternMatch(t, "12345678901234567890123456789012+12345")
+       expectLocatorPatternMatch(t, "A2345678901234abcdefababdeffdfdf+12345")
+       expectLocatorPatternMatch(t, "12345678901234567890123456789012+12345+A1")
+       expectLocatorPatternMatch(t,
+               "12345678901234567890123456789012+12345+A1+B123wxyz@_-")
+       expectLocatorPatternMatch(t,
+               "12345678901234567890123456789012+12345+A1+B123wxyz@_-+C@")
+
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+")
+       expectLocatorPatternFail(t,  "1234567890123456789012345678901+12345")
+       expectLocatorPatternFail(t,  "123456789012345678901234567890123+12345")
+       expectLocatorPatternFail(t,  "g2345678901234abcdefababdeffdfdf+12345")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345 ")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+1")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+1A")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+A")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+a1")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+A1+")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+A1+B")
+       expectLocatorPatternFail(t,  "12345678901234567890123456789012+12345+A+B2")
 }
 
 func TestParseManifestLineSimple(t *testing.T) {