From 043e2d7fa61b4954f0d84c6e4281e37a4f6f865f Mon Sep 17 00:00:00 2001
From: mishaz <misha@curoverse.com>
Date: Thu, 25 Sep 2014 21:12:37 +0000
Subject: [PATCH] Responded to Tim's comments. Switched to using bufio.scanner,
 some other cleanup and added some tests.

---
 services/datamanager/collection/collection.go |  2 +-
 services/datamanager/manifest/manifest.go     | 20 ++++++-------
 .../datamanager/manifest/manifest_test.go     | 28 +++++++++++++++++++
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/services/datamanager/collection/collection.go b/services/datamanager/collection/collection.go
index 03b2f59317..48db7bdafa 100644
--- 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 11b96d8d76..ad0948847e 100644
--- 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 2bf8cdaa75..6cd16bccc6 100644
--- 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) {
-- 
2.30.2