Make it possible to fabricate serving per request

11 files changed, 131 insertions, 100 deletions

diff --git a/internal/domain/domain.go b/internal/domain/domain.go
index 06f99914..af238668 100644
--- a/internal/domain/domain.go
+++ b/internal/domain/domain.go
@@ -8,7 +8,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
-	"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk"
 )
 
 // Domain is a domain that gitlab-pages can serve.
@@ -37,35 +36,23 @@ func (d *Domain) isUnconfigured() bool {
 	return d.Resolver == nil
 }
 
-func (d *Domain) resolve(r *http.Request) (*serving.LookupPath, string) {
-	lookupPath, subpath, _ := d.Resolver.Resolve(r)
-
+func (d *Domain) resolve(r *http.Request) *serving.Request {
 	// Current implementation does not return errors in any case
-	if lookupPath == nil {
-		return nil, ""
-	}
+	request, _ := d.Resolver.Resolve(r)
 
-	return lookupPath, subpath
+	return request
 }
 
-// GetLookupPath returns a project details based on the request
+// GetLookupPath returns a project details based on the request. If LookupPath
+// is nil it means that a project does not exist.
 func (d *Domain) GetLookupPath(r *http.Request) *serving.LookupPath {
-	lookupPath, _ := d.resolve(r)
-
-	return lookupPath
-}
+	request := d.resolve(r)
 
-// Handler returns a serving handler for this request
-func (d *Domain) Handler(w http.ResponseWriter, r *http.Request) serving.Handler {
-	lookup, subpath := d.resolve(r)
-
-	return serving.Handler{
-		Writer:     w,
-		Request:    r,
-		LookupPath: lookup,
-		SubPath:    subpath,
-		Serving:    disk.New(),
+	if request == nil {
+		return nil
 	}
+
+	return d.resolve(r).LookupPath
 }
 
 // IsHTTPSOnly figures out if the request should be handled with HTTPS
@@ -159,8 +146,9 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
 		return true
 	}
 
-	return d.Handler(w, r).ServeFileHTTP()
-	//d.Serving().ServeFileHTTP(d.toHandler(w, r))
+	request := d.resolve(r)
+
+	return request.ServeFileHTTP(w, r)
 }
 
 // ServeNotFoundHTTP serves the not found pages from the projects.
@@ -170,6 +158,7 @@ func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	d.Handler(w, r).ServeNotFoundHTTP()
-	//d.Serving().ServeNotFoundHTTP(d.toHandler(w, r))
+	request := d.resolve(r)
+
+	request.ServeNotFoundHTTP(w, r)
 }
diff --git a/internal/domain/domain_test.go b/internal/domain/domain_test.go
index a43d3c9a..aa95eb95 100644
--- a/internal/domain/domain_test.go
+++ b/internal/domain/domain_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
 )
 
@@ -21,8 +22,12 @@ type stubbedResolver struct {
 	err     error
 }
 
-func (resolver *stubbedResolver) Resolve(*http.Request) (*serving.LookupPath, string, error) {
-	return resolver.project, resolver.subpath, resolver.err
+func (resolver *stubbedResolver) Resolve(*http.Request) (*serving.Request, error) {
+	return &serving.Request{
+		Serving:    disk.New(),
+		LookupPath: resolver.project,
+		SubPath:    resolver.subpath,
+	}, resolver.err
 }
 
 func serveFileOrNotFound(domain *Domain) http.HandlerFunc {
diff --git a/internal/domain/resolver.go b/internal/domain/resolver.go
index 6bc10f8c..4b93baed 100644
--- a/internal/domain/resolver.go
+++ b/internal/domain/resolver.go
@@ -6,9 +6,9 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
 )
 
-// Resolver represents an interface responsible for resolving a project
-// per-request
+// Resolver represents an interface responsible for resolving a pages serving
+// request for each HTTP request
 type Resolver interface {
-	// Resolve returns a project with a file path and an error if it occurred
-	Resolve(*http.Request) (*serving.LookupPath, string, error)
+	// Resolve returns a serving request and an error if it occurred
+	Resolve(*http.Request) (*serving.Request, error)
 }
diff --git a/internal/serving/handler.go b/internal/serving/handler.go
index a18fce7b..a0d66ecb 100644
--- a/internal/serving/handler.go
+++ b/internal/serving/handler.go
@@ -8,16 +8,5 @@ type Handler struct {
 	Writer     http.ResponseWriter
 	Request    *http.Request
 	LookupPath *LookupPath
-	Serving    Serving
 	SubPath    string
 }
-
-// ServeFileHTTP passes the handler itself to a serving function
-func (h Handler) ServeFileHTTP() bool {
-	return h.Serving.ServeFileHTTP(h)
-}
-
-// ServeNotFoundHTTP passes the handler itself to a serving function
-func (h Handler) ServeNotFoundHTTP() {
-	h.Serving.ServeNotFoundHTTP(h)
-}
diff --git a/internal/serving/lookup_path.go b/internal/serving/lookup_path.go
index 492d3995..4360358b 100644
--- a/internal/serving/lookup_path.go
+++ b/internal/serving/lookup_path.go
@@ -1,11 +1,5 @@
 package serving
 
-import (
-	"strings"
-
-	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
-)
-
 // LookupPath holds a domain project configuration needed to handle a request
 type LookupPath struct {
 	Prefix             string // Project prefix, for example, /my/project in group.gitlab.io/my/project/index.html
@@ -15,17 +9,3 @@ type LookupPath struct {
 	HasAccessControl   bool
 	ProjectID          uint64
 }
-
-// NewLookupPath fabricates a new serving lookup path based on a API response.
-// `lookups` argument is a temporary workaround for
-// https://gitlab.com/gitlab-org/gitlab-pages/issues/272
-func NewLookupPath(lookups int, lookup api.LookupPath) *LookupPath {
-	return &LookupPath{
-		Prefix:             lookup.Prefix,
-		Path:               strings.TrimPrefix(lookup.Source.Path, "/"),
-		IsNamespaceProject: (lookup.Prefix == "/" && lookups > 1),
-		IsHTTPSOnly:        lookup.HTTPSOnly,
-		HasAccessControl:   lookup.AccessControl,
-		ProjectID:          uint64(lookup.ProjectID),
-	}
-}
diff --git a/internal/serving/request.go b/internal/serving/request.go
new file mode 100644
index 00000000..019939d6
--- /dev/null
+++ b/internal/serving/request.go
@@ -0,0 +1,35 @@
+package serving
+
+import "net/http"
+
+// Request is a type that aggregates a serving itself, project lookup path and
+// a request subpath based on an incoming request to serve page.
+type Request struct {
+	Serving    Serving     // Serving choosen to serve this request
+	LookupPath *LookupPath // LookupPath contains pages project details
+	SubPath    string      // Subpath is a URL path subcomponent for this request
+}
+
+// ServeFileHTTP forwards serving request handler to the serving itself
+func (s Request) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool {
+	handler := Handler{
+		Writer:     w,
+		Request:    r,
+		LookupPath: s.LookupPath,
+		SubPath:    s.SubPath,
+	}
+
+	return s.Serving.ServeFileHTTP(handler)
+}
+
+// ServeNotFoundHTTP forwards serving request handler to the serving itself
+func (s Request) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) {
+	handler := Handler{
+		Writer:     w,
+		Request:    r,
+		LookupPath: s.LookupPath,
+		SubPath:    s.SubPath,
+	}
+
+	s.Serving.ServeNotFoundHTTP(handler)
+}
diff --git a/internal/source/disk/custom.go b/internal/source/disk/custom.go
index cc4f3f4c..2668ed81 100644
--- a/internal/source/disk/custom.go
+++ b/internal/source/disk/custom.go
@@ -4,6 +4,7 @@ import (
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk"
 )
 
 type customProjectResolver struct {
@@ -12,7 +13,7 @@ type customProjectResolver struct {
 	path string
 }
 
-func (p *customProjectResolver) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
+func (p *customProjectResolver) Resolve(r *http.Request) (*serving.Request, error) {
 	lookupPath := &serving.LookupPath{
 		Prefix:             "/",
 		Path:               p.path,
@@ -22,5 +23,9 @@ func (p *customProjectResolver) Resolve(r *http.Request) (*serving.LookupPath, s
 		ProjectID:          p.config.ID,
 	}
 
-	return lookupPath, r.URL.Path, nil
+	return &serving.Request{
+		Serving:    disk.New(),
+		LookupPath: lookupPath,
+		SubPath:    r.URL.Path,
+	}, nil
 }
diff --git a/internal/source/disk/group.go b/internal/source/disk/group.go
index 9f466bc4..e0365bbd 100644
--- a/internal/source/disk/group.go
+++ b/internal/source/disk/group.go
@@ -8,6 +8,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/host"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk"
 )
 
 const (
@@ -77,11 +78,13 @@ func (g *Group) getProjectConfigWithSubpath(r *http.Request) (*projectConfig, st
 
 // Resolve tries to find project and its config recursively for a given request
 // to a group domain
-func (g *Group) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
+func (g *Group) Resolve(r *http.Request) (*serving.Request, error) {
 	projectConfig, prefix, projectPath, subPath := g.getProjectConfigWithSubpath(r)
 
 	if projectConfig == nil {
-		return nil, "", nil // it is not an error when project does not exist
+		// it is not an error when project does not exist, in that case
+		// serving.Request.LookupPath is nil.
+		return &serving.Request{Serving: disk.New()}, nil
 	}
 
 	lookupPath := &serving.LookupPath{
@@ -93,5 +96,9 @@ func (g *Group) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
 		ProjectID:          projectConfig.ID,
 	}
 
-	return lookupPath, subPath, nil
+	return &serving.Request{
+		Serving:    disk.New(),
+		LookupPath: lookupPath,
+		SubPath:    subPath,
+	}, nil
 }
diff --git a/internal/source/gitlab/api/lookup.go b/internal/source/gitlab/api/lookup.go
index 73a3ce43..ff1b1dae 100644
--- a/internal/source/gitlab/api/lookup.go
+++ b/internal/source/gitlab/api/lookup.go
@@ -2,6 +2,7 @@ package api
 
 // Lookup defines an API lookup action with a response that GitLab sends
 type Lookup struct {
+	ETag   string // TODO
 	Name   string
 	Error  error
 	Domain *VirtualDomain
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index 79b58501..9a5cb7b1 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -6,10 +6,14 @@ import (
 	"net/http"
 	"path"
 	"strings"
+	"time"
+
+	store "github.com/patrickmn/go-cache"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/cache"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/client"
@@ -19,6 +23,7 @@ import (
 // information about domains from GitLab instance.
 type Gitlab struct {
 	client api.Resolver
+	store  *store.Cache
 }
 
 // New returns a new instance of gitlab domain source.
@@ -28,7 +33,10 @@ func New(config client.Config) (*Gitlab, error) {
 		return nil, err
 	}
 
-	return &Gitlab{client: cache.NewCache(client)}, nil
+	return &Gitlab{
+		client: cache.NewCache(client),
+		store:  store.New(time.Hour, time.Minute),
+	}, nil
 }
 
 // GetDomain return a representation of a domain that we have fetched from
@@ -55,33 +63,45 @@ func (g *Gitlab) GetDomain(name string) (*domain.Domain, error) {
 	return &domain, nil
 }
 
-// Resolve is supposed to get the serving lookup path based on the request from
-// the GitLab source
-func (g *Gitlab) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
+// Resolve is supposed to return the serving request containing lookup path,
+// subpath for a given lookup and the serving itself created based on a request
+// from GitLab pages domains source
+func (g *Gitlab) Resolve(r *http.Request) (*serving.Request, error) {
 	host := request.GetHostWithoutPort(r)
 
 	response := g.client.Resolve(r.Context(), host)
 	if response.Error != nil {
-		return nil, "", response.Error
+		return nil, response.Error
 	}
 
 	urlPath := path.Clean(r.URL.Path)
+	lookups := len(response.Domain.LookupPaths)
 
 	for _, lookup := range response.Domain.LookupPaths {
 		isSubPath := strings.HasPrefix(urlPath, lookup.Prefix)
 		isRootPath := urlPath == path.Clean(lookup.Prefix)
 
 		if isSubPath || isRootPath {
-			lookupPath := serving.NewLookupPath(len(response.Domain.LookupPaths), lookup)
+			lookupPath := &serving.LookupPath{
+				Prefix:             lookup.Prefix,
+				Path:               strings.TrimPrefix(lookup.Source.Path, "/"),
+				IsNamespaceProject: (lookup.Prefix == "/" && lookups > 1),
+				IsHTTPSOnly:        lookup.HTTPSOnly,
+				HasAccessControl:   lookup.AccessControl,
+				ProjectID:          uint64(lookup.ProjectID),
+			}
 
 			subPath := ""
 			if isSubPath {
 				subPath = strings.TrimPrefix(urlPath, lookup.Prefix)
 			}
 
-			return lookupPath, subPath, nil
+			return &serving.Request{
+				Serving:    disk.New(),
+				LookupPath: lookupPath,
+				SubPath:    subPath}, nil
 		}
 	}
 
-	return nil, "", errors.New("could not match lookup path")
+	return &serving.Request{Serving: disk.New()}, errors.New("could not match lookup path")
 }
diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go
index 0e855f10..e6f194ee 100644
--- a/internal/source/gitlab/gitlab_test.go
+++ b/internal/source/gitlab/gitlab_test.go
@@ -39,62 +39,62 @@ func TestResolve(t *testing.T) {
 		target := "https://test.gitlab.io:443/my/pages/project/"
 		request := httptest.NewRequest("GET", target, nil)
 
-		lookup, subpath, err := source.Resolve(request)
+		response, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/my/pages/project/", lookup.Prefix)
-		require.Equal(t, "some/path/to/project/", lookup.Path)
-		require.Equal(t, "", subpath)
-		require.False(t, lookup.IsNamespaceProject)
+		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
+		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
+		require.Equal(t, "", response.SubPath)
+		require.False(t, response.LookupPath.IsNamespaceProject)
 	})
 
 	t.Run("when requesting a nested group project with full path", func(t *testing.T) {
 		target := "https://test.gitlab.io:443/my/pages/project/path/index.html"
 		request := httptest.NewRequest("GET", target, nil)
 
-		lookup, subpath, err := source.Resolve(request)
+		response, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/my/pages/project/", lookup.Prefix)
-		require.Equal(t, "some/path/to/project/", lookup.Path)
-		require.Equal(t, "path/index.html", subpath)
-		require.False(t, lookup.IsNamespaceProject)
+		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
+		require.Equal(t, "some/path/to/project/", response.LookupPath.Path)
+		require.Equal(t, "path/index.html", response.SubPath)
+		require.False(t, response.LookupPath.IsNamespaceProject)
 	})
 
 	t.Run("when requesting the group root project with root path", func(t *testing.T) {
 		target := "https://test.gitlab.io:443/"
 		request := httptest.NewRequest("GET", target, nil)
 
-		lookup, subpath, err := source.Resolve(request)
+		response, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/", lookup.Prefix)
-		require.Equal(t, "some/path/to/project-3/", lookup.Path)
-		require.Equal(t, "", subpath)
-		require.True(t, lookup.IsNamespaceProject)
+		require.Equal(t, "/", response.LookupPath.Prefix)
+		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
+		require.Equal(t, "", response.SubPath)
+		require.True(t, response.LookupPath.IsNamespaceProject)
 	})
 
 	t.Run("when requesting the group root project with full path", func(t *testing.T) {
 		target := "https://test.gitlab.io:443/path/to/index.html"
 		request := httptest.NewRequest("GET", target, nil)
 
-		lookup, subpath, err := source.Resolve(request)
+		response, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/", lookup.Prefix)
-		require.Equal(t, "path/to/index.html", subpath)
-		require.Equal(t, "some/path/to/project-3/", lookup.Path)
-		require.True(t, lookup.IsNamespaceProject)
+		require.Equal(t, "/", response.LookupPath.Prefix)
+		require.Equal(t, "path/to/index.html", response.SubPath)
+		require.Equal(t, "some/path/to/project-3/", response.LookupPath.Path)
+		require.True(t, response.LookupPath.IsNamespaceProject)
 	})
 
 	t.Run("when request path has not been sanitized", func(t *testing.T) {
 		target := "https://test.gitlab.io:443/something/../something/../my/pages/project/index.html"
 		request := httptest.NewRequest("GET", target, nil)
 
-		lookup, subpath, err := source.Resolve(request)
+		response, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/my/pages/project/", lookup.Prefix)
-		require.Equal(t, "index.html", subpath)
+		require.Equal(t, "/my/pages/project/", response.LookupPath.Prefix)
+		require.Equal(t, "index.html", response.SubPath)
 	})
 }