refactor: Move domain extra log fields to domain package

4 files changed, 95 insertions, 35 deletions

diff --git a/internal/domain/logging.go b/internal/domain/logging.go
new file mode 100644
index 00000000..ae2b44e3
--- /dev/null
+++ b/internal/domain/logging.go
@@ -0,0 +1,24 @@
+package domain
+
+import (
+	"net/http"
+
+	"gitlab.com/gitlab-org/labkit/log"
+)
+
+func (d *Domain) LogFields(r *http.Request) log.Fields {
+	if d == nil {
+		return log.Fields{}
+	}
+
+	lp, err := d.GetLookupPath(r)
+	if err != nil {
+		return log.Fields{"error": err.Error()}
+	}
+
+	return log.Fields{
+		"pages_project_serving_type": lp.ServingType,
+		"pages_project_prefix":       lp.Prefix,
+		"pages_project_id":           lp.ProjectID,
+	}
+}
diff --git a/internal/domain/logging_test.go b/internal/domain/logging_test.go
new file mode 100644
index 00000000..2a1902c2
--- /dev/null
+++ b/internal/domain/logging_test.go
@@ -0,0 +1,68 @@
+package domain
+
+import (
+	"net/http"
+	"testing"
+
+	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
+)
+
+type resolver struct {
+	err error
+	f   func(*http.Request) *serving.LookupPath
+}
+
+func (r *resolver) Resolve(req *http.Request) (*serving.Request, error) {
+	if r.f != nil {
+		return &serving.Request{LookupPath: r.f(req)}, nil
+	}
+
+	return nil, r.err
+}
+
+func TestDomainLogFields(t *testing.T) {
+	domainWithResolver := New("", "", "", &resolver{f: func(*http.Request) *serving.LookupPath {
+		return &serving.LookupPath{
+			ServingType: "file",
+			ProjectID:   100,
+			Prefix:      "/prefix",
+		}
+	}})
+
+	tests := map[string]struct {
+		domain         *Domain
+		host           string
+		expectedFields log.Fields
+	}{
+		"nil_domain_returns_empty_fields": {
+			domain:         nil,
+			host:           "gitlab.io",
+			expectedFields: log.Fields{},
+		},
+		"unresolved_domain_returns_error": {
+			domain:         New("githost.io", "", "", &resolver{err: ErrDomainDoesNotExist}),
+			host:           "gitlab.io",
+			expectedFields: log.Fields{"error": ErrDomainDoesNotExist.Error()},
+		},
+		"domain_with_fields": {
+			domain: domainWithResolver,
+			host:   "gitlab.io",
+			expectedFields: log.Fields{
+				"pages_project_id":           uint64(100),
+				"pages_project_prefix":       "/prefix",
+				"pages_project_serving_type": "file",
+			},
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			r, err := http.NewRequest("GET", "/", nil)
+			require.NoError(t, err)
+
+			require.Equal(t, tt.expectedFields, tt.domain.LogFields(r))
+		})
+	}
+}
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
index ffa42370..f485b20b 100644
--- a/internal/logging/logging.go
+++ b/internal/logging/logging.go
@@ -59,16 +59,8 @@ func getExtraLogFields(r *http.Request) log.Fields {
 		"pages_host":  request.GetHost(r),
 	}
 
-	if d := request.GetDomain(r); d != nil {
-		lp, err := d.GetLookupPath(r)
-		if err != nil {
-			logFields["error"] = err.Error()
-			return logFields
-		}
-
-		logFields["pages_project_serving_type"] = lp.ServingType
-		logFields["pages_project_prefix"] = lp.Prefix
-		logFields["pages_project_id"] = lp.ProjectID
+	for name, value := range request.GetDomain(r).LogFields(r) {
+		logFields[name] = value
 	}
 
 	return logFields
diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go
index f2b344a7..df8c3013 100644
--- a/internal/logging/logging_test.go
+++ b/internal/logging/logging_test.go
@@ -37,7 +37,6 @@ func TestGetExtraLogFields(t *testing.T) {
 		name                  string
 		scheme                string
 		host                  string
-		domain                *domain.Domain
 		expectedHTTPS         interface{}
 		expectedHost          interface{}
 		expectedProjectID     interface{}
@@ -49,7 +48,6 @@ func TestGetExtraLogFields(t *testing.T) {
 			name:                  "https",
 			scheme:                request.SchemeHTTPS,
 			host:                  "githost.io",
-			domain:                domainWithResolver,
 			expectedHTTPS:         true,
 			expectedHost:          "githost.io",
 			expectedProjectID:     uint64(100),
@@ -60,34 +58,12 @@ func TestGetExtraLogFields(t *testing.T) {
 			name:                  "http",
 			scheme:                request.SchemeHTTP,
 			host:                  "githost.io",
-			domain:                domainWithResolver,
 			expectedHTTPS:         false,
 			expectedHost:          "githost.io",
 			expectedProjectID:     uint64(100),
 			expectedProjectPrefix: "/prefix",
 			expectedServingType:   "file",
 		},
-		{
-			name:                "domain_not_configured",
-			scheme:              request.SchemeHTTP,
-			host:                "githost.io",
-			domain:              nil,
-			expectedHTTPS:       false,
-			expectedHost:        "githost.io",
-			expectedProjectID:   nil,
-			expectedServingType: nil,
-		},
-		{
-			name:                "no_domain",
-			scheme:              request.SchemeHTTP,
-			host:                "githost.io",
-			domain:              domain.New("githost.io", "", "", &resolver{err: domain.ErrDomainDoesNotExist}),
-			expectedHTTPS:       false,
-			expectedHost:        "githost.io",
-			expectedProjectID:   nil,
-			expectedServingType: nil,
-			expectedErrMsg:      domain.ErrDomainDoesNotExist.Error(),
-		},
 	}
 
 	for _, tt := range tests {
@@ -96,7 +72,7 @@ func TestGetExtraLogFields(t *testing.T) {
 			require.NoError(t, err)
 
 			req.URL.Scheme = tt.scheme
-			req = request.WithHostAndDomain(req, tt.host, tt.domain)
+			req = request.WithHostAndDomain(req, tt.host, domainWithResolver)
 
 			got := getExtraLogFields(req)
 			require.Equal(t, tt.expectedHTTPS, got["pages_https"])