Reverts changes that added headers to the Drop function.

- After reconsideration we came to the conclusion that it does not make sense to support custom headers at this time and they unnecessarily dirty the API. - We consider downloading to only be part of Drop() main functionality the other half being detecting on disk depenencies as well, the headers change only affected half of Drop() functionality which is a weird API to enforce.
paketo-buildpacks · Jun 30, 2020 · f099442 · f099442
1 parent c0bc35a
commit f099442
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 65 deletions.
diff --git a/cargo/dependency_cacher.go b/cargo/dependency_cacher.go
@@ -3,7 +3,6 @@ package cargo
 import (
 	"fmt"
 	"io"
-	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -13,7 +12,7 @@ import (
 
 //go:generate faux --interface Downloader --output fakes/downloader.go
 type Downloader interface {
-	Drop(root, uri string, header http.Header) (io.ReadCloser, error)
+	Drop(root, uri string) (io.ReadCloser, error)
 }
 
 type DependencyCacher struct {
@@ -41,7 +40,7 @@ func (dc DependencyCacher) Cache(root string, deps []ConfigMetadataDependency) (
 		dc.logger.Subprocess("%s (%s) [%s]", dep.ID, dep.Version, strings.Join(dep.Stacks, ", "))
 		dc.logger.Action("↳  dependencies/%s", dep.SHA256)
 
-		source, err := dc.downloader.Drop("", dep.URI, nil)
+		source, err := dc.downloader.Drop("", dep.URI)
 		if err != nil {
 			return nil, fmt.Errorf("failed to download dependency: %s", err)
 		}

diff --git a/cargo/dependency_cacher_test.go b/cargo/dependency_cacher_test.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -35,7 +34,7 @@ func testDependencyCacher(t *testing.T, context spec.G, it spec.S) {
 		Expect(err).NotTo(HaveOccurred())
 
 		downloader = &fakes.Downloader{}
-		downloader.DropCall.Stub = func(root, uri string, header http.Header) (io.ReadCloser, error) {
+		downloader.DropCall.Stub = func(root, uri string) (io.ReadCloser, error) {
 			switch uri {
 			case "http://dep1-uri":
 				return ioutil.NopCloser(strings.NewReader("dep1-contents")), nil

diff --git a/cargo/fakes/downloader.go b/cargo/fakes/downloader.go
@@ -2,7 +2,6 @@ package fakes
 
 import (
 	"io"
-	"net/http"
 	"sync"
 )
 
@@ -11,27 +10,25 @@ type Downloader struct {
 		sync.Mutex
 		CallCount int
 		Receives  struct {
-			Root   string
-			Uri    string
-			Header http.Header
+			Root string
+			Uri  string
 		}
 		Returns struct {
 			ReadCloser io.ReadCloser
 			Error      error
 		}
-		Stub func(string, string, http.Header) (io.ReadCloser, error)
+		Stub func(string, string) (io.ReadCloser, error)
 	}
 }
 
-func (f *Downloader) Drop(param1 string, param2 string, param3 http.Header) (io.ReadCloser, error) {
+func (f *Downloader) Drop(param1 string, param2 string) (io.ReadCloser, error) {
 	f.DropCall.Lock()
 	defer f.DropCall.Unlock()
 	f.DropCall.CallCount++
 	f.DropCall.Receives.Root = param1
 	f.DropCall.Receives.Uri = param2
-	f.DropCall.Receives.Header = param3
 	if f.DropCall.Stub != nil {
-		return f.DropCall.Stub(param1, param2, param3)
+		return f.DropCall.Stub(param1, param2)
 	}
 	return f.DropCall.Returns.ReadCloser, f.DropCall.Returns.Error
 }
diff --git a/cargo/transport.go b/cargo/transport.go
@@ -9,15 +9,13 @@ import (
 	"strings"
 )
 
-type Transport struct{
-	header http.Header
-}
+type Transport struct{}
 
 func NewTransport() Transport {
 	return Transport{}
 }
 
-func (t Transport) Drop(root, uri string, header http.Header) (io.ReadCloser, error) {
+func (t Transport) Drop(root, uri string) (io.ReadCloser, error) {
 	if strings.HasPrefix(uri, "file://") {
 		file, err := os.Open(filepath.Join(root, strings.TrimPrefix(uri, "file://")))
 		if err != nil {
@@ -32,15 +30,10 @@ func (t Transport) Drop(root, uri string, header http.Header) (io.ReadCloser, er
 		return nil, fmt.Errorf("failed to parse request uri: %s", err)
 	}
 
-	if header != nil {
-		request.Header = header
-	}
-
 	response, err := http.DefaultClient.Do(request)
 	if err != nil {
 		return nil, fmt.Errorf("failed to make request: %s", err)
 	}
 
 	return response.Body, nil
 }
-
diff --git a/cargo/transport_test.go b/cargo/transport_test.go
@@ -30,20 +30,11 @@ func testTransport(t *testing.T, context spec.G, it spec.S) {
 
 			it.Before(func() {
 				server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-					if req.Header.Get("header") == "some-header" {
-						switch req.URL.Path {
-						case "/some-bundle-with-header":
-							w.Write([]byte("some-bundle-with-header-contents"))
-						default:
-							http.NotFound(w, req)
-						}
-					} else {
-						switch req.URL.Path {
-						case "/some-bundle":
-							w.Write([]byte("some-bundle-contents"))
-						default:
-							http.NotFound(w, req)
-						}
+					switch req.URL.Path {
+					case "/some-bundle":
+						w.Write([]byte("some-bundle-contents"))
+					default:
+						http.NotFound(w, req)
 					}
 				}))
 			})
@@ -53,7 +44,7 @@ func testTransport(t *testing.T, context spec.G, it spec.S) {
 			})
 
 			it("downloads the file from a URI", func() {
-				bundle, err := transport.Drop("", fmt.Sprintf("%s/some-bundle", server.URL), nil)
+				bundle, err := transport.Drop("", fmt.Sprintf("%s/some-bundle", server.URL))
 				Expect(err).NotTo(HaveOccurred())
 
 				contents, err := ioutil.ReadAll(bundle)
@@ -63,23 +54,10 @@ func testTransport(t *testing.T, context spec.G, it spec.S) {
 				Expect(bundle.Close()).To(Succeed())
 			})
 
-			context("when there are request headers", func() {
-				it("downloads the file from a URI", func() {
-					bundle, err := transport.Drop("", fmt.Sprintf("%s/some-bundle-with-header", server.URL), http.Header{"header": []string{"some-header"}})
-					Expect(err).NotTo(HaveOccurred())
-
-					contents, err := ioutil.ReadAll(bundle)
-					Expect(err).NotTo(HaveOccurred())
-					Expect(string(contents)).To(Equal("some-bundle-with-header-contents"))
-
-					Expect(bundle.Close()).To(Succeed())
-				})
-			})
-
 			context("failure cases", func() {
 				context("when the uri is malformed", func() {
 					it("returns an error", func() {
-						_, err := transport.Drop("", "%%%%", nil)
+						_, err := transport.Drop("", "%%%%")
 						Expect(err).To(MatchError(ContainSubstring("failed to parse request uri")))
 						Expect(err).To(MatchError(ContainSubstring("invalid URL escape")))
 					})
@@ -91,7 +69,7 @@ func testTransport(t *testing.T, context spec.G, it spec.S) {
 					})
 
 					it("returns an error", func() {
-						_, err := transport.Drop("", fmt.Sprintf("%s/some-bundle", server.URL), nil)
+						_, err := transport.Drop("", fmt.Sprintf("%s/some-bundle", server.URL))
 						Expect(err).To(MatchError(ContainSubstring("failed to make request")))
 						Expect(err).To(MatchError(ContainSubstring("connection refused")))
 					})
@@ -121,7 +99,7 @@ func testTransport(t *testing.T, context spec.G, it spec.S) {
 			})
 
 			it("returns the file descriptor", func() {
-				bundle, err := transport.Drop(dir, fmt.Sprintf("file://%s", path), nil)
+				bundle, err := transport.Drop(dir, fmt.Sprintf("file://%s", path))
 				Expect(err).NotTo(HaveOccurred())
 
 				contents, err := ioutil.ReadAll(bundle)
@@ -138,7 +116,7 @@ func testTransport(t *testing.T, context spec.G, it spec.S) {
 
 				context("when the file does not exist", func() {
 					it("returns an error", func() {
-						_, err := transport.Drop(dir, fmt.Sprintf("file://%s", path), nil)
+						_, err := transport.Drop(dir, fmt.Sprintf("file://%s", path))
 						Expect(err).To(MatchError(ContainSubstring("failed to open file")))
 						Expect(err).To(MatchError(ContainSubstring("no such file or directory")))
 					})

diff --git a/postal/fakes/transport.go b/postal/fakes/transport.go
@@ -2,7 +2,6 @@ package fakes
 
 import (
 	"io"
-	"net/http"
 	"sync"
 )
 
@@ -11,27 +10,25 @@ type Transport struct {
 		sync.Mutex
 		CallCount int
 		Receives  struct {
-			Root   string
-			Uri    string
-			Header http.Header
+			Root string
+			Uri  string
 		}
 		Returns struct {
 			ReadCloser io.ReadCloser
 			Error      error
 		}
-		Stub func(string, string, http.Header) (io.ReadCloser, error)
+		Stub func(string, string) (io.ReadCloser, error)
 	}
 }
 
-func (f *Transport) Drop(param1 string, param2 string, param3 http.Header) (io.ReadCloser, error) {
+func (f *Transport) Drop(param1 string, param2 string) (io.ReadCloser, error) {
 	f.DropCall.Lock()
 	defer f.DropCall.Unlock()
 	f.DropCall.CallCount++
 	f.DropCall.Receives.Root = param1
 	f.DropCall.Receives.Uri = param2
-	f.DropCall.Receives.Header = param3
 	if f.DropCall.Stub != nil {
-		return f.DropCall.Stub(param1, param2, param3)
+		return f.DropCall.Stub(param1, param2)
 	}
 	return f.DropCall.Returns.ReadCloser, f.DropCall.Returns.Error
 }
diff --git a/postal/service.go b/postal/service.go
@@ -3,7 +3,6 @@ package postal
 import (
 	"fmt"
 	"io"
-	"net/http"
 	"sort"
 
 	"github.com/Masterminds/semver"
@@ -16,7 +15,7 @@ import (
 // Transport serves as the interface for types that can fetch dependencies
 // given a location uri using either the http:// or file:// scheme.
 type Transport interface {
-	Drop(root, uri string, header http.Header) (io.ReadCloser, error)
+	Drop(root, uri string) (io.ReadCloser, error)
 }
 
 // Service provides a mechanism for resolving and installing dependencies given
@@ -97,7 +96,7 @@ func (s Service) Resolve(path, id, version, stack string) (Dependency, error) {
 // Dependency and will error if there are inconsistencies in the fetched
 // result.
 func (s Service) Install(dependency Dependency, cnbPath, layerPath string) error {
-	bundle, err := s.transport.Drop(cnbPath, dependency.URI, nil)
+	bundle, err := s.transport.Drop(cnbPath, dependency.URI)
 	if err != nil {
 		return fmt.Errorf("failed to fetch dependency: %s", err)
 	}