clients: new Endpoint implementation and InstallProtocol function

9 files changed, 129 insertions, 162 deletions

diff --git a/clients/common.go b/clients/common.go
index 8d45a34..1eeacf0 100644
--- a/clients/common.go
+++ b/clients/common.go
@@ -13,52 +13,34 @@ package clients
 
 import (
 	"fmt"
-	"net/url"
 
 	"gopkg.in/src-d/go-git.v4/clients/common"
 	"gopkg.in/src-d/go-git.v4/clients/http"
 	"gopkg.in/src-d/go-git.v4/clients/ssh"
 )
 
-// DefaultProtocols are the protocols supported by default.
-var DefaultProtocols = map[string]common.GitUploadPackService{
-	"http":  http.NewGitUploadPackService(),
-	"https": http.NewGitUploadPackService(),
-	"ssh":   ssh.NewGitUploadPackService(),
-}
-
-// KnownProtocols holds the current set of known protocols. Initially
-// it gets its contents from `DefaultProtocols`. See `InstallProtocol`
-// below to add or modify this variable.
-var KnownProtocols = make(map[string]common.GitUploadPackService, len(DefaultProtocols))
+type GitUploadPackServiceFactory func(common.Endpoint) common.GitUploadPackService
 
-func init() {
-	for k, v := range DefaultProtocols {
-		InstallProtocol(k, v)
-	}
+// Protocols are the protocols supported by default.
+var Protocols = map[string]GitUploadPackServiceFactory{
+	"http":  http.NewGitUploadPackService,
+	"https": http.NewGitUploadPackService,
+	"ssh":   ssh.NewGitUploadPackService,
 }
 
 // InstallProtocol adds or modifies an existing protocol.
-func InstallProtocol(scheme string, service common.GitUploadPackService) {
-	if service == nil {
-		panic("nil service")
-	}
-
-	KnownProtocols[scheme] = service
+func InstallProtocol(scheme string, f GitUploadPackServiceFactory) {
+	Protocols[scheme] = f
 }
 
 // NewGitUploadPackService returns the appropriate upload pack service
 // among of the set of known protocols: HTTP, SSH. See `InstallProtocol`
 // to add or modify protocols.
-func NewGitUploadPackService(repoURL string) (common.GitUploadPackService, error) {
-	u, err := url.Parse(repoURL)
-	if err != nil {
-		return nil, fmt.Errorf("invalid url %q", repoURL)
-	}
-	s, ok := KnownProtocols[u.Scheme]
+func NewGitUploadPackService(endpoint common.Endpoint) (common.GitUploadPackService, error) {
+	f, ok := Protocols[endpoint.Scheme]
 	if !ok {
-		return nil, fmt.Errorf("unsupported scheme %q", u.Scheme)
+		return nil, fmt.Errorf("unsupported scheme %q", endpoint.Scheme)
 	}
 
-	return s, nil
+	return f(endpoint), nil
 }
diff --git a/clients/common/common.go b/clients/common/common.go
index fbd8066..b1a0fe1 100644
--- a/clients/common/common.go
+++ b/clients/common/common.go
@@ -6,27 +6,27 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/url"
 	"strings"
 
 	"gopkg.in/src-d/go-git.v4/core"
 	"gopkg.in/src-d/go-git.v4/formats/pktline"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
-
-	"gopkg.in/sourcegraph/go-vcsurl.v1"
 )
 
 var (
-	NotFoundErr           = errors.New("repository not found")
-	EmptyGitUploadPackErr = errors.New("empty git-upload-pack given")
+	ErrNotFound           = errors.New("repository not found")
+	ErrEmptyGitUploadPack = errors.New("empty git-upload-pack given")
+	ErrInvalidAuthMethod  = errors.New("invalid auth method")
 )
 
 const GitUploadPackServiceName = "git-upload-pack"
 
 type GitUploadPackService interface {
-	Connect(url Endpoint) error
-	ConnectWithAuth(url Endpoint, auth AuthMethod) error
+	Connect() error
+	ConnectWithAuth(AuthMethod) error
 	Info() (*GitUploadPackInfo, error)
-	Fetch(r *GitUploadPackRequest) (io.ReadCloser, error)
+	Fetch(*GitUploadPackRequest) (io.ReadCloser, error)
 }
 
 type AuthMethod interface {
@@ -34,24 +34,26 @@ type AuthMethod interface {
 	String() string
 }
 
-type Endpoint string
+type Endpoint url.URL
 
-func NewEndpoint(url string) (Endpoint, error) {
-	vcs, err := vcsurl.Parse(url)
+func NewEndpoint(endpoint string) (Endpoint, error) {
+	u, err := url.Parse(endpoint)
 	if err != nil {
-		return "", core.NewPermanentError(err)
+		return Endpoint{}, core.NewPermanentError(err)
 	}
 
-	link := vcs.Link()
-	if !strings.HasSuffix(link, ".git") {
-		link += ".git"
+	if !u.IsAbs() {
+		return Endpoint{}, core.NewPermanentError(fmt.Errorf(
+			"invalid endpoint: %s", endpoint,
+		))
 	}
 
-	return Endpoint(link), nil
+	return Endpoint(*u), nil
 }
 
-func (e Endpoint) Service(name string) string {
-	return fmt.Sprintf("%s/info/refs?service=%s", e, name)
+func (e *Endpoint) String() string {
+	u := url.URL(*e)
+	return u.String()
 }
 
 // Capabilities contains all the server capabilities
@@ -190,7 +192,7 @@ func NewGitUploadPackInfo() *GitUploadPackInfo {
 
 func (r *GitUploadPackInfo) Decode(d *pktline.Decoder) error {
 	if err := r.read(d); err != nil {
-		if err == EmptyGitUploadPackErr {
+		if err == ErrEmptyGitUploadPack {
 			return core.NewPermanentError(err)
 		}
 
@@ -223,7 +225,7 @@ func (r *GitUploadPackInfo) read(d *pktline.Decoder) error {
 	}
 
 	if isEmpty {
-		return EmptyGitUploadPackErr
+		return ErrEmptyGitUploadPack
 	}
 
 	return nil
diff --git a/clients/common/common_test.go b/clients/common/common_test.go
index 5be6eba..80934fc 100644
--- a/clients/common/common_test.go
+++ b/clients/common/common_test.go
@@ -17,20 +17,15 @@ type SuiteCommon struct{}
 var _ = Suite(&SuiteCommon{})
 
 func (s *SuiteCommon) TestNewEndpoint(c *C) {
-	e, err := NewEndpoint("git@github.com:user/repository.git")
+	e, err := NewEndpoint("ssh://git@github.com/user/repository.git")
 	c.Assert(err, IsNil)
-	c.Assert(e, Equals, Endpoint("https://github.com/user/repository.git"))
+	c.Assert(e.String(), Equals, "ssh://git@github.com/user/repository.git")
 }
 
 func (s *SuiteCommon) TestNewEndpointWrongForgat(c *C) {
 	e, err := NewEndpoint("foo")
 	c.Assert(err, Not(IsNil))
-	c.Assert(e, Equals, Endpoint(""))
-}
-
-func (s *SuiteCommon) TestEndpointService(c *C) {
-	e, _ := NewEndpoint("git@github.com:user/repository.git")
-	c.Assert(e.Service("foo"), Equals, "https://github.com/user/repository.git/info/refs?service=foo")
+	c.Assert(e.Host, Equals, "")
 }
 
 const CapabilitiesFixture = "6ecf0ef2c2dffb796033e5a02219af86ec6584e5 HEADmulti_ack thin-pack side-band side-band-64k ofs-delta shallow no-progress include-tag multi_ack_detailed no-done symref=HEAD:refs/heads/master agent=git/2:2.4.8~dbussink-fix-enterprise-tokens-compilation-1167-gc7006cf"
diff --git a/clients/common_test.go b/clients/common_test.go
index f27b814..72d8e99 100644
--- a/clients/common_test.go
+++ b/clients/common_test.go
@@ -3,69 +3,68 @@ package clients
 import (
 	"fmt"
 	"io"
-	"os"
 	"testing"
 
 	"gopkg.in/src-d/go-git.v4/clients/common"
 
-	"github.com/alcortesm/tgz"
 	. "gopkg.in/check.v1"
 )
 
 func Test(t *testing.T) { TestingT(t) }
 
-type SuiteCommon struct {
-	dirFixturePath string
-}
+type SuiteCommon struct{}
 
 var _ = Suite(&SuiteCommon{})
 
-const fixtureTGZ = "../storage/filesystem/internal/dotgit/fixtures/spinnaker-gc.tgz"
+func (s *SuiteCommon) TestNewGitUploadPackServiceHTTP(c *C) {
+	e, err := common.NewEndpoint("http://github.com/src-d/go-git")
+	c.Assert(err, IsNil)
+
+	output, err := NewGitUploadPackService(e)
+	c.Assert(err, IsNil)
+	c.Assert(typeAsString(output), Equals, "*http.GitUploadPackService")
+
+	e, err = common.NewEndpoint("https://github.com/src-d/go-git")
+	c.Assert(err, IsNil)
+
+	output, err = NewGitUploadPackService(e)
+	c.Assert(err, IsNil)
+	c.Assert(typeAsString(output), Equals, "*http.GitUploadPackService")
+}
+
+func (s *SuiteCommon) TestNewGitUploadPackServiceSSH(c *C) {
+	e, err := common.NewEndpoint("ssh://github.com/src-d/go-git")
+	c.Assert(err, IsNil)
 
-func (s *SuiteCommon) SetUpSuite(c *C) {
-	var err error
-	s.dirFixturePath, err = tgz.Extract(fixtureTGZ)
+	output, err := NewGitUploadPackService(e)
 	c.Assert(err, IsNil)
+	c.Assert(typeAsString(output), Equals, "*ssh.GitUploadPackService")
 }
 
-func (s *SuiteCommon) TearDownSuite(c *C) {
-	err := os.RemoveAll(s.dirFixturePath)
+func (s *SuiteCommon) TestNewGitUploadPackServiceUnknown(c *C) {
+	e, err := common.NewEndpoint("unknown://github.com/src-d/go-git")
 	c.Assert(err, IsNil)
+
+	_, err = NewGitUploadPackService(e)
+	c.Assert(err, NotNil)
 }
 
-func (s *SuiteCommon) TestNewGitUploadPackService(c *C) {
-	var tests = [...]struct {
-		input string
-		err   bool
-		exp   string
-	}{
-		{"://example.com", true, "<nil>"},
-		{"badscheme://github.com/src-d/go-git", true, "<nil>"},
-		{"http://github.com/src-d/go-git", false, "*http.GitUploadPackService"},
-		{"https://github.com/src-d/go-git", false, "*http.GitUploadPackService"},
-		{"ssh://github.com/src-d/go-git", false, "*ssh.GitUploadPackService"},
-	}
-
-	for i, t := range tests {
-		output, err := NewGitUploadPackService(t.input)
-		c.Assert(err != nil, Equals, t.err,
-			Commentf("%d) %q: wrong error value (was: %s)", i, t.input, err))
-		c.Assert(typeAsString(output), Equals, t.exp,
-			Commentf("%d) %q: wrong type", i, t.input))
-	}
+func (s *SuiteCommon) TestInstallProtocol(c *C) {
+	InstallProtocol("newscheme", newDummyProtocolService)
+	c.Assert(Protocols["newscheme"], NotNil)
 }
 
 type dummyProtocolService struct{}
 
-func newDummyProtocolService() common.GitUploadPackService {
+func newDummyProtocolService(common.Endpoint) common.GitUploadPackService {
 	return &dummyProtocolService{}
 }
 
-func (s *dummyProtocolService) Connect(url common.Endpoint) error {
+func (s *dummyProtocolService) Connect() error {
 	return nil
 }
 
-func (s *dummyProtocolService) ConnectWithAuth(url common.Endpoint, auth common.AuthMethod) error {
+func (s *dummyProtocolService) ConnectWithAuth(auth common.AuthMethod) error {
 	return nil
 }
 
@@ -77,32 +76,6 @@ func (s *dummyProtocolService) Fetch(r *common.GitUploadPackRequest) (io.ReadClo
 	return nil, nil
 }
 
-func (s *SuiteCommon) TestInstallProtocol(c *C) {
-	var tests = [...]struct {
-		scheme  string
-		service common.GitUploadPackService
-		panic   bool
-	}{
-		{"panic", nil, true},
-		{"newscheme", newDummyProtocolService(), false},
-		{"http", newDummyProtocolService(), false},
-	}
-
-	for i, t := range tests {
-		if t.panic {
-			c.Assert(func() { InstallProtocol(t.scheme, t.service) }, PanicMatches, `nil service`)
-			continue
-		}
-
-		InstallProtocol(t.scheme, t.service)
-		c.Assert(typeAsString(KnownProtocols[t.scheme]), Equals, typeAsString(t.service), Commentf("%d) wrong service", i))
-		// reset to default protocols after installing
-		if v, ok := DefaultProtocols[t.scheme]; ok {
-			InstallProtocol(t.scheme, v)
-		}
-	}
-}
-
 func typeAsString(v interface{}) string {
 	return fmt.Sprintf("%T", v)
 }
diff --git a/clients/http/common.go b/clients/http/common.go
index e7c43d5..a4b86bd 100644
--- a/clients/http/common.go
+++ b/clients/http/common.go
@@ -2,7 +2,6 @@
 package http
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
 
@@ -10,8 +9,6 @@ import (
 	"gopkg.in/src-d/go-git.v4/core"
 )
 
-var InvalidAuthMethodErr = errors.New("invalid http auth method: a http.HTTPAuthMethod should be provided.")
-
 type HTTPAuthMethod interface {
 	common.AuthMethod
 	setAuth(r *http.Request)
@@ -53,7 +50,7 @@ func NewHTTPError(r *http.Response) error {
 
 	err := &HTTPError{r}
 	if r.StatusCode == 404 || r.StatusCode == 401 {
-		return core.NewPermanentError(common.NotFoundErr)
+		return core.NewPermanentError(common.ErrNotFound)
 	}
 
 	return core.NewUnexpectedError(err)
diff --git a/clients/http/common_test.go b/clients/http/common_test.go
index 7bd9708..d3e2c9d 100644
--- a/clients/http/common_test.go
+++ b/clients/http/common_test.go
@@ -13,6 +13,13 @@ type SuiteCommon struct{}
 
 var _ = Suite(&SuiteCommon{})
 
+func (s *SuiteCommon) TestNewBasicAuth(c *C) {
+	a := NewBasicAuth("foo", "qux")
+
+	c.Assert(a.Name(), Equals, "http-basic-auth")
+	c.Assert(a.String(), Equals, "http-basic-auth - foo:*******")
+}
+
 func (s *SuiteCommon) TestNewHTTPError200(c *C) {
 	res := &http.Response{StatusCode: 200}
 	res.StatusCode = 200
diff --git a/clients/http/git_upload_pack.go b/clients/http/git_upload_pack.go
index 860318f..96535de 100644
--- a/clients/http/git_upload_pack.go
+++ b/clients/http/git_upload_pack.go
@@ -12,37 +12,38 @@ import (
 )
 
 type GitUploadPackService struct {
-	Client *http.Client
-
+	client   *http.Client
 	endpoint common.Endpoint
 	auth     HTTPAuthMethod
 }
 
-func NewGitUploadPackService() *GitUploadPackService {
+func NewGitUploadPackService(endpoint common.Endpoint) common.GitUploadPackService {
 	return &GitUploadPackService{
-		Client: http.DefaultClient,
+		client:   http.DefaultClient,
+		endpoint: endpoint,
 	}
 }
 
-func (s *GitUploadPackService) Connect(url common.Endpoint) error {
-	s.endpoint = url
+func (s *GitUploadPackService) Connect() error {
 	return nil
 }
 
-func (s *GitUploadPackService) ConnectWithAuth(url common.Endpoint, auth common.AuthMethod) error {
+func (s *GitUploadPackService) ConnectWithAuth(auth common.AuthMethod) error {
 	httpAuth, ok := auth.(HTTPAuthMethod)
 	if !ok {
-		return InvalidAuthMethodErr
+		return common.ErrInvalidAuthMethod
 	}
 
-	s.endpoint = url
 	s.auth = httpAuth
-
 	return nil
 }
 
 func (s *GitUploadPackService) Info() (*common.GitUploadPackInfo, error) {
-	url := fmt.Sprintf("%s/info/refs?service=%s", s.endpoint, common.GitUploadPackServiceName)
+	url := fmt.Sprintf(
+		"%s/info/refs?service=%s",
+		s.endpoint.String(), common.GitUploadPackServiceName,
+	)
+
 	res, err := s.doRequest("GET", url, nil)
 	if err != nil {
 		return nil, err
@@ -55,7 +56,11 @@ func (s *GitUploadPackService) Info() (*common.GitUploadPackInfo, error) {
 }
 
 func (s *GitUploadPackService) Fetch(r *common.GitUploadPackRequest) (io.ReadCloser, error) {
-	url := fmt.Sprintf("%s/%s", s.endpoint, common.GitUploadPackServiceName)
+	url := fmt.Sprintf(
+		"%s/%s",
+		s.endpoint.String(), common.GitUploadPackServiceName,
+	)
+
 	res, err := s.doRequest("POST", url, r.Reader())
 	if err != nil {
 		return nil, err
@@ -83,7 +88,7 @@ func (s *GitUploadPackService) doRequest(method, url string, content *strings.Re
 	s.applyHeadersToRequest(req, content)
 	s.applyAuthToRequest(req)
 
-	res, err := s.Client.Do(req)
+	res, err := s.client.Do(req)
 	if err != nil {
 		return nil, core.NewUnexpectedError(err)
 	}
diff --git a/clients/http/git_upload_pack_test.go b/clients/http/git_upload_pack_test.go
index db4a273..7e8cc36 100644
--- a/clients/http/git_upload_pack_test.go
+++ b/clients/http/git_upload_pack_test.go
@@ -8,22 +8,27 @@ import (
 	"gopkg.in/src-d/go-git.v4/core"
 )
 
-type SuiteRemote struct{}
+type RemoteSuite struct {
+	Endpoint common.Endpoint
+}
 
-var _ = Suite(&SuiteRemote{})
+var _ = Suite(&RemoteSuite{})
 
-const RepositoryFixture = "https://github.com/tyba/git-fixture"
+func (s *RemoteSuite) SetUpSuite(c *C) {
+	var err error
+	s.Endpoint, err = common.NewEndpoint("https://github.com/tyba/git-fixture")
+	c.Assert(err, IsNil)
+}
 
-func (s *SuiteRemote) TestConnect(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.Connect(RepositoryFixture), IsNil)
+func (s *RemoteSuite) TestConnect(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 }
 
-func (s *SuiteRemote) TestConnectWithAuth(c *C) {
+func (s *RemoteSuite) TestConnectWithAuth(c *C) {
 	auth := &BasicAuth{}
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(RepositoryFixture, auth), IsNil)
-	c.Assert(r.auth, Equals, auth)
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.ConnectWithAuth(auth), IsNil)
 }
 
 type mockAuth struct{}
@@ -31,32 +36,32 @@ type mockAuth struct{}
 func (*mockAuth) Name() string   { return "" }
 func (*mockAuth) String() string { return "" }
 
-func (s *SuiteRemote) TestConnectWithAuthWrongType(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.ConnectWithAuth(RepositoryFixture, &mockAuth{}), Equals, InvalidAuthMethodErr)
+func (s *RemoteSuite) TestConnectWithAuthWrongType(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.ConnectWithAuth(&mockAuth{}), Equals, common.ErrInvalidAuthMethod)
 }
 
-func (s *SuiteRemote) TestDefaultBranch(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.Connect(RepositoryFixture), IsNil)
+func (s *RemoteSuite) TestDefaultBranch(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 
 	info, err := r.Info()
 	c.Assert(err, IsNil)
 	c.Assert(info.Capabilities.SymbolicReference("HEAD"), Equals, "refs/heads/master")
 }
 
-func (s *SuiteRemote) TestCapabilities(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.Connect(RepositoryFixture), IsNil)
+func (s *RemoteSuite) TestCapabilities(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 
 	info, err := r.Info()
 	c.Assert(err, IsNil)
 	c.Assert(info.Capabilities.Get("agent").Values, HasLen, 1)
 }
 
-func (s *SuiteRemote) TestFetch(c *C) {
-	r := NewGitUploadPackService()
-	c.Assert(r.Connect(RepositoryFixture), IsNil)
+func (s *RemoteSuite) TestFetch(c *C) {
+	r := NewGitUploadPackService(s.Endpoint)
+	c.Assert(r.Connect(), IsNil)
 
 	req := &common.GitUploadPackRequest{}
 	req.Want(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
diff --git a/clients/ssh/git_upload_pack.go b/clients/ssh/git_upload_pack.go
index be5f7d0..551ab9c 100644
--- a/clients/ssh/git_upload_pack.go
+++ b/clients/ssh/git_upload_pack.go
@@ -15,8 +15,8 @@ import (
 	"gopkg.in/src-d/go-git.v4/clients/common"
 	"gopkg.in/src-d/go-git.v4/formats/pktline"
 
-	"gopkg.in/sourcegraph/go-vcsurl.v1"
 	"golang.org/x/crypto/ssh"
+	"gopkg.in/sourcegraph/go-vcsurl.v1"
 )
 
 // New errors introduced by this package.
@@ -35,6 +35,7 @@ var (
 // TODO: remove NewGitUploadPackService().
 type GitUploadPackService struct {
 	connected bool
+	endpoint  common.Endpoint
 	vcs       *vcsurl.RepoInfo
 	client    *ssh.Client
 	auth      AuthMethod
@@ -42,24 +43,24 @@ type GitUploadPackService struct {
 
 // NewGitUploadPackService initialises a GitUploadPackService.
 // TODO: remove this, as the struct is zero-value safe.
-func NewGitUploadPackService() *GitUploadPackService {
-	return &GitUploadPackService{}
+func NewGitUploadPackService(endpoint common.Endpoint) common.GitUploadPackService {
+	return &GitUploadPackService{endpoint: endpoint}
 }
 
 // Connect cannot be used with SSH clients and always return
 // ErrAuthRequired. Use ConnectWithAuth instead.
-func (s *GitUploadPackService) Connect(ep common.Endpoint) (err error) {
+func (s *GitUploadPackService) Connect() (err error) {
 	return ErrAuthRequired
 }
 
 // ConnectWithAuth connects to ep using SSH. Authentication is handled
 // by auth.
-func (s *GitUploadPackService) ConnectWithAuth(ep common.Endpoint, auth common.AuthMethod) (err error) {
+func (s *GitUploadPackService) ConnectWithAuth(auth common.AuthMethod) (err error) {
 	if s.connected {
 		return ErrAlreadyConnected
 	}
 
-	s.vcs, err = vcsurl.Parse(string(ep))
+	s.vcs, err = vcsurl.Parse(s.endpoint.String())
 	if err != nil {
 		return err
 	}