From 58987db21c5fcd29e7c2192c4404501b43035e4e Mon Sep 17 00:00:00 2001 From: rick olson Date: Wed, 25 Oct 2017 16:05:08 -0600 Subject: [PATCH] lfsapi: teach endpointGitFinder about Context to get *git.Configuration --- lfsapi/auth_test.go | 4 +-- lfsapi/endpoint_finder.go | 46 +++++++++++++------------ lfsapi/endpoint_finder_test.go | 50 ++++++++++++++-------------- lfsapi/lfsapi.go | 30 +++++++---------- test/git-lfs-test-server-api/main.go | 2 +- 5 files changed, 65 insertions(+), 67 deletions(-) diff --git a/lfsapi/auth_test.go b/lfsapi/auth_test.go index e0fb12cc..daedcdc1 100644 --- a/lfsapi/auth_test.go +++ b/lfsapi/auth_test.go @@ -143,7 +143,7 @@ func TestDoWithAuthReject(t *testing.T) { c, _ := NewClient(nil) c.Credentials = creds - c.Endpoints = NewEndpointFinder(testEnv(map[string]string{ + c.Endpoints = NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.url": srv.URL, })) @@ -531,7 +531,7 @@ func TestGetCreds(t *testing.T) { req.Header.Set(key, value) } - ef := NewEndpointFinder(testEnv(test.Config)) + ef := NewEndpointFinder(NewContext(nil, nil, test.Config)) endpoint, access, creds, credsURL, err := getCreds(credHelper, netrcFinder, ef, test.Remote, req) if !assert.Nil(t, err) { continue diff --git a/lfsapi/endpoint_finder.go b/lfsapi/endpoint_finder.go index feef0d35..40baa3ee 100644 --- a/lfsapi/endpoint_finder.go +++ b/lfsapi/endpoint_finder.go @@ -37,7 +37,8 @@ type EndpointFinder interface { } type endpointGitFinder struct { - git config.Environment + gitConfig *git.Configuration + gitEnv config.Environment gitProtocol string aliasMu sync.Mutex @@ -48,21 +49,24 @@ type endpointGitFinder struct { urlConfig *config.URLConfig } -func NewEndpointFinder(git config.Environment) EndpointFinder { +func NewEndpointFinder(ctx Context) EndpointFinder { + if ctx == nil { + ctx = NewContext(nil, nil, nil) + } + e := &endpointGitFinder{ + gitConfig: ctx.GitConfig(), + gitEnv: ctx.GitEnv(), gitProtocol: "https", aliases: make(map[string]string), urlAccess: make(map[string]Access), } - if git != nil { - e.git = git - e.urlConfig = config.NewURLConfig(e.git) - if v, ok := git.Get("lfs.gitprotocol"); ok { - e.gitProtocol = v - } - initAliases(e, git) + e.urlConfig = config.NewURLConfig(e.gitEnv) + if v, ok := e.gitEnv.Get("lfs.gitprotocol"); ok { + e.gitProtocol = v } + initAliases(e, e.gitEnv) return e } @@ -74,17 +78,17 @@ func (e *endpointGitFinder) Endpoint(operation, remote string) Endpoint { } func (e *endpointGitFinder) getEndpoint(operation, remote string) Endpoint { - if e.git == nil { + if e.gitEnv == nil { return Endpoint{} } if operation == "upload" { - if url, ok := e.git.Get("lfs.pushurl"); ok { + if url, ok := e.gitEnv.Get("lfs.pushurl"); ok { return e.NewEndpoint(url) } } - if url, ok := e.git.Get("lfs.url"); ok { + if url, ok := e.gitEnv.Get("lfs.url"); ok { return e.NewEndpoint(url) } @@ -98,7 +102,7 @@ func (e *endpointGitFinder) getEndpoint(operation, remote string) Endpoint { } func (e *endpointGitFinder) RemoteEndpoint(operation, remote string) Endpoint { - if e.git == nil { + if e.gitEnv == nil { return Endpoint{} } @@ -108,11 +112,11 @@ func (e *endpointGitFinder) RemoteEndpoint(operation, remote string) Endpoint { // Support separate push URL if specified and pushing if operation == "upload" { - if url, ok := e.git.Get("remote." + remote + ".lfspushurl"); ok { + if url, ok := e.gitEnv.Get("remote." + remote + ".lfspushurl"); ok { return e.NewEndpoint(url) } } - if url, ok := e.git.Get("remote." + remote + ".lfsurl"); ok { + if url, ok := e.gitEnv.Get("remote." + remote + ".lfsurl"); ok { return e.NewEndpoint(url) } @@ -125,14 +129,14 @@ func (e *endpointGitFinder) RemoteEndpoint(operation, remote string) Endpoint { } func (e *endpointGitFinder) GitRemoteURL(remote string, forpush bool) string { - if e.git != nil { + if e.gitEnv != nil { if forpush { - if u, ok := e.git.Get("remote." + remote + ".pushurl"); ok { + if u, ok := e.gitEnv.Get("remote." + remote + ".pushurl"); ok { return u } } - if u, ok := e.git.Get("remote." + remote + ".url"); ok { + if u, ok := e.gitEnv.Get("remote." + remote + ".url"); ok { return u } } @@ -187,7 +191,7 @@ func (e *endpointGitFinder) NewEndpoint(rawurl string) Endpoint { } func (e *endpointGitFinder) AccessFor(rawurl string) Access { - if e.git == nil { + if e.gitEnv == nil { return NoneAccess } @@ -214,10 +218,10 @@ func (e *endpointGitFinder) SetAccess(rawurl string, access Access) { switch access { case emptyAccess, NoneAccess: - git.Config.UnsetLocalKey("", key) + e.gitConfig.UnsetLocalKey("", key) e.urlAccess[accessurl] = NoneAccess default: - git.Config.SetLocal("", key, string(access)) + e.gitConfig.SetLocal("", key, string(access)) e.urlAccess[accessurl] = access } } diff --git a/lfsapi/endpoint_finder_test.go b/lfsapi/endpoint_finder_test.go index 0663dc09..0af7696f 100644 --- a/lfsapi/endpoint_finder_test.go +++ b/lfsapi/endpoint_finder_test.go @@ -7,7 +7,7 @@ import ( ) func TestEndpointDefaultsToOrigin(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.lfsurl": "abc", })) @@ -18,7 +18,7 @@ func TestEndpointDefaultsToOrigin(t *testing.T) { } func TestEndpointOverridesOrigin(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.url": "abc", "remote.origin.lfsurl": "def", })) @@ -30,7 +30,7 @@ func TestEndpointOverridesOrigin(t *testing.T) { } func TestEndpointNoOverrideDefaultRemote(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.lfsurl": "abc", "remote.other.lfsurl": "def", })) @@ -42,7 +42,7 @@ func TestEndpointNoOverrideDefaultRemote(t *testing.T) { } func TestEndpointUseAlternateRemote(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.lfsurl": "abc", "remote.other.lfsurl": "def", })) @@ -54,7 +54,7 @@ func TestEndpointUseAlternateRemote(t *testing.T) { } func TestEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "https://example.com/foo/bar", })) @@ -65,7 +65,7 @@ func TestEndpointAddsLfsSuffix(t *testing.T) { } func TestBareEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "https://example.com/foo/bar.git", })) @@ -76,7 +76,7 @@ func TestBareEndpointAddsLfsSuffix(t *testing.T) { } func TestEndpointSeparateClonePushUrl(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "https://example.com/foo/bar.git", "remote.origin.pushurl": "https://readwrite.com/foo/bar.git", })) @@ -93,7 +93,7 @@ func TestEndpointSeparateClonePushUrl(t *testing.T) { } func TestEndpointOverriddenSeparateClonePushLfsUrl(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "https://example.com/foo/bar.git", "remote.origin.pushurl": "https://readwrite.com/foo/bar.git", "remote.origin.lfsurl": "https://examplelfs.com/foo/bar", @@ -112,7 +112,7 @@ func TestEndpointOverriddenSeparateClonePushLfsUrl(t *testing.T) { } func TestEndpointGlobalSeparateLfsPush(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.url": "https://readonly.com/foo/bar", "lfs.pushurl": "https://write.com/foo/bar", })) @@ -129,7 +129,7 @@ func TestEndpointGlobalSeparateLfsPush(t *testing.T) { } func TestSSHEndpointOverridden(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "git@example.com:foo/bar", "remote.origin.lfsurl": "lfs", })) @@ -142,7 +142,7 @@ func TestSSHEndpointOverridden(t *testing.T) { } func TestSSHEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "ssh://git@example.com/foo/bar", })) @@ -154,7 +154,7 @@ func TestSSHEndpointAddsLfsSuffix(t *testing.T) { } func TestSSHCustomPortEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "ssh://git@example.com:9000/foo/bar", })) @@ -166,7 +166,7 @@ func TestSSHCustomPortEndpointAddsLfsSuffix(t *testing.T) { } func TestBareSSHEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "git@example.com:foo/bar.git", })) @@ -178,7 +178,7 @@ func TestBareSSHEndpointAddsLfsSuffix(t *testing.T) { } func TestSSHEndpointFromGlobalLfsUrl(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.url": "git@example.com:foo/bar.git", })) @@ -190,7 +190,7 @@ func TestSSHEndpointFromGlobalLfsUrl(t *testing.T) { } func TestHTTPEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "http://example.com/foo/bar", })) @@ -202,7 +202,7 @@ func TestHTTPEndpointAddsLfsSuffix(t *testing.T) { } func TestBareHTTPEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "http://example.com/foo/bar.git", })) @@ -214,7 +214,7 @@ func TestBareHTTPEndpointAddsLfsSuffix(t *testing.T) { } func TestGitEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "git://example.com/foo/bar", })) @@ -226,7 +226,7 @@ func TestGitEndpointAddsLfsSuffix(t *testing.T) { } func TestGitEndpointAddsLfsSuffixWithCustomProtocol(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "git://example.com/foo/bar", "lfs.gitprotocol": "http", })) @@ -239,7 +239,7 @@ func TestGitEndpointAddsLfsSuffixWithCustomProtocol(t *testing.T) { } func TestBareGitEndpointAddsLfsSuffix(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "remote.origin.url": "git://example.com/foo/bar.git", })) @@ -266,7 +266,7 @@ func TestAccessConfig(t *testing.T) { } for value, expected := range tests { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.url": "http://example.com", "lfs.http://example.com.access": value, "lfs.https://example.com.access": "bad", @@ -285,7 +285,7 @@ func TestAccessConfig(t *testing.T) { // Test again but with separate push url for value, expected := range tests { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.url": "http://example.com", "lfs.pushurl": "http://examplepush.com", "lfs.http://example.com.access": value, @@ -312,7 +312,7 @@ func TestAccessAbsentConfig(t *testing.T) { } func TestSetAccess(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{})) + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{})) assert.Equal(t, NoneAccess, finder.AccessFor("http://example.com")) finder.SetAccess("http://example.com", NTLMAccess) @@ -320,7 +320,7 @@ func TestSetAccess(t *testing.T) { } func TestChangeAccess(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.http://example.com.access": "basic", })) @@ -330,7 +330,7 @@ func TestChangeAccess(t *testing.T) { } func TestDeleteAccessWithNone(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.http://example.com.access": "basic", })) @@ -340,7 +340,7 @@ func TestDeleteAccessWithNone(t *testing.T) { } func TestDeleteAccessWithEmptyString(t *testing.T) { - finder := NewEndpointFinder(testEnv(map[string]string{ + finder := NewEndpointFinder(NewContext(nil, nil, map[string]string{ "lfs.http://example.com.access": "basic", })) diff --git a/lfsapi/lfsapi.go b/lfsapi/lfsapi.go index de81a34a..898391dd 100644 --- a/lfsapi/lfsapi.go +++ b/lfsapi/lfsapi.go @@ -43,8 +43,6 @@ type Client struct { ntlmSessions map[string]ntlm.ClientSession ntlmMu sync.Mutex - gitConfig *git.Configuration - httpLogger *syncLogger LoggingStats bool // DEPRECATED @@ -63,22 +61,11 @@ type Context interface { func NewClient(ctx Context) (*Client, error) { if ctx == nil { - return newClient(nil, nil, nil) - } - return newClient(ctx.GitConfig(), ctx.OSEnv(), ctx.GitEnv()) -} - -func newClient(gitConf *git.Configuration, osEnv, gitEnv config.Environment) (*Client, error) { - if gitConf == nil { - gitConf = git.NewConfig("", "") - } - if osEnv == nil { - osEnv = make(testEnv) - } - if gitEnv == nil { - gitEnv = make(testEnv) + ctx = NewContext(nil, nil, nil) } + gitEnv := ctx.GitEnv() + osEnv := ctx.OSEnv() netrc, netrcfile, err := ParseNetrc(osEnv) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("bad netrc file %s", netrcfile)) @@ -95,7 +82,7 @@ func newClient(gitConf *git.Configuration, osEnv, gitEnv config.Environment) (*C } c := &Client{ - Endpoints: NewEndpointFinder(gitEnv), + Endpoints: NewEndpointFinder(ctx), Credentials: creds, SSH: sshResolver, Netrc: netrc, @@ -106,7 +93,6 @@ func newClient(gitConf *git.Configuration, osEnv, gitEnv config.Environment) (*C SkipSSLVerify: !gitEnv.Bool("http.sslverify", true) || osEnv.Bool("GIT_SSL_NO_VERIFY", false), Verbose: osEnv.Bool("GIT_CURL_VERBOSE", false), DebuggingVerbose: osEnv.Bool("LFS_DEBUG_HTTP", false), - gitConfig: gitConf, gitEnv: gitEnv, osEnv: osEnv, uc: config.NewURLConfig(gitEnv), @@ -174,11 +160,19 @@ func (c *testContext) GitEnv() config.Environment { func NewContext(gitConf *git.Configuration, osEnv, gitEnv map[string]string) Context { c := &testContext{gitConfig: gitConf} + if c.gitConfig == nil { + c.gitConfig = git.NewConfig("", "") + } if osEnv != nil { c.osEnv = testEnv(osEnv) + } else { + c.osEnv = make(testEnv) } + if gitEnv != nil { c.gitEnv = testEnv(gitEnv) + } else { + c.gitEnv = make(testEnv) } return c } diff --git a/test/git-lfs-test-server-api/main.go b/test/git-lfs-test-server-api/main.go index efcb6b5c..068d9dd8 100644 --- a/test/git-lfs-test-server-api/main.go +++ b/test/git-lfs-test-server-api/main.go @@ -139,7 +139,7 @@ func (*testDataCallback) Errorf(format string, args ...interface{}) { func buildManifest(r *test.Repo) (*tq.Manifest, error) { // Configure the endpoint manually - finder := lfsapi.NewEndpointFinder(r.GitEnv()) + finder := lfsapi.NewEndpointFinder(r) var endp lfsapi.Endpoint if len(cloneUrl) > 0 {