From 9e038e69dc5accf451d32bd1b8742f83eabe9b9e Mon Sep 17 00:00:00 2001
From: sam boyer <tech@samboyer.org>
Date: Sun, 10 Sep 2017 23:22:07 -0400
Subject: [PATCH] Ensure URLs are folded as well

Also add an up-front check for case variant map lookup misses, and
update the map accordingly.
---
 internal/gps/manager_test.go | 26 ++++++++++++++++++--------
 internal/gps/source.go       | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/internal/gps/manager_test.go b/internal/gps/manager_test.go
index 5dec172d7b..5e3c13e8e8 100644
--- a/internal/gps/manager_test.go
+++ b/internal/gps/manager_test.go
@@ -349,8 +349,8 @@ func TestMgrMethodsFailWithBadPath(t *testing.T) {
 }
 
 type sourceCreationTestFixture struct {
-	roots              []ProjectIdentifier
-	urlcount, srccount int
+	roots               []ProjectIdentifier
+	namecount, srccount int
 }
 
 func (f sourceCreationTestFixture) run(t *testing.T) {
@@ -365,8 +365,8 @@ func (f sourceCreationTestFixture) run(t *testing.T) {
 		}
 	}
 
-	if len(sm.srcCoord.nameToURL) != f.urlcount {
-		t.Errorf("want %v names in the name->url map, but got %v. contents: \n%v", f.urlcount, len(sm.srcCoord.nameToURL), sm.srcCoord.nameToURL)
+	if len(sm.srcCoord.nameToURL) != f.namecount {
+		t.Errorf("want %v names in the name->url map, but got %v. contents: \n%v", f.namecount, len(sm.srcCoord.nameToURL), sm.srcCoord.nameToURL)
 	}
 
 	if len(sm.srcCoord.srcs) != f.srccount {
@@ -390,16 +390,26 @@ func TestSourceCreationCounts(t *testing.T) {
 				mkPI("gopkg.in/sdboyer/gpkt.v2"),
 				mkPI("gopkg.in/sdboyer/gpkt.v3"),
 			},
-			urlcount: 6,
-			srccount: 3,
+			namecount: 6,
+			srccount:  3,
 		},
 		"gopkgin separation from github": {
 			roots: []ProjectIdentifier{
 				mkPI("gopkg.in/sdboyer/gpkt.v1"),
 				mkPI("github.com/sdboyer/gpkt"),
 			},
-			urlcount: 4,
-			srccount: 2,
+			namecount: 4,
+			srccount:  2,
+		},
+		"case variance across path and URL-based access": {
+			roots: []ProjectIdentifier{
+				ProjectIdentifier{ProjectRoot: ProjectRoot("github.com/sdboyer/gpkt"), Source: "https://github.com/Sdboyer/gpkt"},
+				mkPI("github.com/sdboyer/gpkt"),
+				ProjectIdentifier{ProjectRoot: ProjectRoot("github.com/sdboyer/gpkt"), Source: "https://github.com/sdboyeR/gpkt"},
+				mkPI("github.com/sdboyeR/gpkt"),
+			},
+			namecount: 5,
+			srccount:  1,
 		},
 	}
 
diff --git a/internal/gps/source.go b/internal/gps/source.go
index bb683a1fc4..9cb89ce4ac 100644
--- a/internal/gps/source.go
+++ b/internal/gps/source.go
@@ -83,7 +83,6 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 		}
 		panic(fmt.Sprintf("%q was URL for %q in nameToURL, but no corresponding srcGate in srcs map", url, normalizedName))
 	}
-	sc.srcmut.RUnlock()
 
 	// Without a direct match, we must fold the input name to a generally
 	// stable, caseless variant and primarily work from that. This ensures that
@@ -103,6 +102,31 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	// systems. So we follow this path, which is both a vastly simpler solution
 	// and one that seems quite likely to work in practice.
 	foldedNormalName := toFold(normalizedName)
+	if foldedNormalName != normalizedName {
+		// If the folded name differs from the input name, then there may
+		// already be an entry for it in the nameToURL map, so check again.
+		if url, has := sc.nameToURL[foldedNormalName]; has {
+			// There was a match on the canonical folded variant. Upgrade to a
+			// write lock, so that future calls on this name don't need to
+			// burn cycles on folding.
+			sc.srcmut.RUnlock()
+			sc.srcmut.Lock()
+			// It may be possible that another goroutine could interleave
+			// between the unlock and re-lock. Even if they do, though, they'll
+			// only have recorded the same url value as we have here. In other
+			// words, these operations commute, so we can safely write here
+			// without checking again.
+			sc.nameToURL[normalizedName] = url
+
+			srcGate, has := sc.srcs[url]
+			sc.srcmut.Unlock()
+			if has {
+				return srcGate, nil
+			}
+			panic(fmt.Sprintf("%q was URL for %q in nameToURL, but no corresponding srcGate in srcs map", url, normalizedName))
+		}
+	}
+	sc.srcmut.RUnlock()
 
 	// No gateway exists for this path yet; set up a proto, being careful to fold
 	// together simultaneous attempts on the same case-folded path.
@@ -140,7 +164,7 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 		sc.psrcmut.Unlock()
 	}
 
-	pd, err := sc.deducer.deduceRootPath(ctx, normalizedName)
+	pd, err := sc.deducer.deduceRootPath(ctx, foldedNormalName)
 	if err != nil {
 		// As in the deducer, don't cache errors so that externally-driven retry
 		// strategies can be constructed.
@@ -189,15 +213,15 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
 	defer sc.srcmut.Unlock()
 	// Record the name -> URL mapping, making sure that we also get the
 	// self-mapping.
-	sc.nameToURL[normalizedName] = url
-	if url != normalizedName {
+	sc.nameToURL[foldedNormalName] = url
+	if url != foldedNormalName {
 		sc.nameToURL[url] = url
 	}
 
 	// Make sure we have both the folded and unfolded names recorded in the map,
 	// should they differ.
 	if normalizedName != foldedNormalName {
-		sc.nameToURL[foldedNormalName] = url
+		sc.nameToURL[normalizedName] = url
 	}
 
 	if sa, has := sc.srcs[url]; has {