Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Commit

Permalink
Ensure URLs are folded as well
Browse files Browse the repository at this point in the history
Also add an up-front check for case variant map lookup misses, and
update the map accordingly.
  • Loading branch information
sdboyer committed Sep 11, 2017
1 parent a665a6e commit 9e038e6
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 13 deletions.
26 changes: 18 additions & 8 deletions internal/gps/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand All @@ -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,
},
}

Expand Down
34 changes: 29 additions & 5 deletions internal/gps/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 9e038e6

Please sign in to comment.