plumbing/transport: allow AdvertisedReferences being called multiple times. (#165)

* AdvertisedReferences is now part of transport.Session.
* It is allowed to be called more than once.
* It is allowed to be called before and after FetchPack/SendPack.
* Implementations cache its result.

5 files changed, 23 insertions, 31 deletions

diff --git a/plumbing/transport/common.go b/plumbing/transport/common.go
index a20fa14..41813bf 100644
--- a/plumbing/transport/common.go
+++ b/plumbing/transport/common.go
@@ -30,8 +30,6 @@ var (
 	ErrAuthorizationRequired  = errors.New("authorization required")
 	ErrEmptyUploadPackRequest = errors.New("empty git-upload-pack given")
 	ErrInvalidAuthMethod      = errors.New("invalid auth method")
-
-	ErrAdvertistedReferencesAlreadyCalled = errors.New("cannot call AdvertisedReference twice")
 )
 
 const (
@@ -49,6 +47,9 @@ type Client interface {
 
 type Session interface {
 	SetAuth(auth AuthMethod) error
+	// AdvertisedReferences retrieves the advertised references for a
+	// repository.
+	AdvertisedReferences() (*packp.AdvRefs, error)
 	io.Closer
 }
 
@@ -63,10 +64,6 @@ type AuthMethod interface {
 // In that order.
 type FetchPackSession interface {
 	Session
-	// AdvertisedReferences retrieves the advertised references for a
-	// repository. It should be called before FetchPack, and it cannot be
-	// called after FetchPack.
-	AdvertisedReferences() (*packp.AdvRefs, error)
 	// FetchPack takes a request and returns a reader for the packfile
 	// received from the server.
 	FetchPack(*packp.UploadPackRequest) (*packp.UploadPackResponse, error)
@@ -78,10 +75,6 @@ type FetchPackSession interface {
 // In that order.
 type SendPackSession interface {
 	Session
-	// AdvertisedReferences retrieves the advertised references for a
-	// repository. It should be called before FetchPack, and it cannot be
-	// called after FetchPack.
-	AdvertisedReferences() (*packp.AdvRefs, error)
 	// UpdateReferences sends an update references request and returns a
 	// writer to be used for packfile writing.
 	//TODO: Complete signature.
diff --git a/plumbing/transport/http/common.go b/plumbing/transport/http/common.go
index a1b085b..aa3425e 100644
--- a/plumbing/transport/http/common.go
+++ b/plumbing/transport/http/common.go
@@ -6,6 +6,7 @@ import (
 	"net/http"
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
 )
 
@@ -50,6 +51,7 @@ type session struct {
 	auth     AuthMethod
 	client   *http.Client
 	endpoint transport.Endpoint
+	advRefs  *packp.AdvRefs
 }
 
 func (s *session) SetAuth(auth transport.AuthMethod) error {
diff --git a/plumbing/transport/http/fetch_pack.go b/plumbing/transport/http/fetch_pack.go
index 1548110..0c85be4 100644
--- a/plumbing/transport/http/fetch_pack.go
+++ b/plumbing/transport/http/fetch_pack.go
@@ -17,7 +17,6 @@ import (
 
 type fetchPackSession struct {
 	*session
-	advRefsRun bool
 }
 
 func newFetchPackSession(c *http.Client,
@@ -33,12 +32,10 @@ func newFetchPackSession(c *http.Client,
 }
 
 func (s *fetchPackSession) AdvertisedReferences() (*packp.AdvRefs, error) {
-	if s.advRefsRun {
-		return nil, transport.ErrAdvertistedReferencesAlreadyCalled
+	if s.advRefs != nil {
+		return s.advRefs, nil
 	}
 
-	defer func() { s.advRefsRun = true }()
-
 	url := fmt.Sprintf(
 		"%s/info/refs?service=%s",
 		s.endpoint.String(), transport.UploadPackServiceName,
@@ -72,6 +69,7 @@ func (s *fetchPackSession) AdvertisedReferences() (*packp.AdvRefs, error) {
 	}
 
 	transport.FilterUnsupportedCapabilities(ar.Capabilities)
+	s.advRefs = ar
 	return ar, nil
 }
 
diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go
index 11c86ef..12f4995 100644
--- a/plumbing/transport/internal/common/common.go
+++ b/plumbing/transport/internal/common/common.go
@@ -99,10 +99,10 @@ type session struct {
 	Stdout  io.Reader
 	Command Command
 
-	advRefsRun bool
-	packRun    bool
-	finished   bool
-	errLines   chan string
+	advRefs  *packp.AdvRefs
+	packRun  bool
+	finished bool
+	errLines chan string
 }
 
 func (c *client) newSession(s string, ep transport.Endpoint) (*session, error) {
@@ -154,12 +154,10 @@ func (s *session) SetAuth(auth transport.AuthMethod) error {
 
 // AdvertisedReferences retrieves the advertised references from the server.
 func (s *session) AdvertisedReferences() (*packp.AdvRefs, error) {
-	if s.advRefsRun {
-		return nil, transport.ErrAdvertistedReferencesAlreadyCalled
+	if s.advRefs != nil {
+		return s.advRefs, nil
 	}
 
-	s.advRefsRun = true
-
 	ar := packp.NewAdvRefs()
 	if err := ar.Decode(s.Stdout); err != nil {
 		// If repository is not found, we get empty stdout and server
@@ -187,6 +185,7 @@ func (s *session) AdvertisedReferences() (*packp.AdvRefs, error) {
 	}
 
 	transport.FilterUnsupportedCapabilities(ar.Capabilities)
+	s.advRefs = ar
 	return ar, nil
 }
 
@@ -201,10 +200,8 @@ func (s *session) FetchPack(req *packp.UploadPackRequest) (*packp.UploadPackResp
 		return nil, err
 	}
 
-	if !s.advRefsRun {
-		if _, err := s.AdvertisedReferences(); err != nil {
-			return nil, err
-		}
+	if _, err := s.AdvertisedReferences(); err != nil {
+		return nil, err
 	}
 
 	s.packRun = true
diff --git a/plumbing/transport/test/common.go b/plumbing/transport/test/common.go
index 3b7f05f..2984154 100644
--- a/plumbing/transport/test/common.go
+++ b/plumbing/transport/test/common.go
@@ -49,13 +49,15 @@ func (s *FetchPackSuite) TestInfoNotExists(c *C) {
 	c.Assert(reader, IsNil)
 }
 
-func (s *FetchPackSuite) TestCannotCallAdvertisedReferenceTwice(c *C) {
+func (s *FetchPackSuite) TestCallAdvertisedReferenceTwice(c *C) {
 	r, err := s.Client.NewFetchPackSession(s.Endpoint)
 	c.Assert(err, IsNil)
-	_, err = r.AdvertisedReferences()
+	ar1, err := r.AdvertisedReferences()
 	c.Assert(err, IsNil)
-	_, err = r.AdvertisedReferences()
-	c.Assert(err, Equals, transport.ErrAdvertistedReferencesAlreadyCalled)
+	c.Assert(ar1, NotNil)
+	ar2, err := r.AdvertisedReferences()
+	c.Assert(err, IsNil)
+	c.Assert(ar2, DeepEquals, ar1)
 }
 
 func (s *FetchPackSuite) TestDefaultBranch(c *C) {