chore: refactor act/artifactcache Handler to an interface (#934)

- the Handler struct becomes handler (lowercase)
- the Handler interface is defined to be the existing methods
- isClosed() is added and used only in tests
- setgcAt() is added and used only in tests

---

This is to allow mocking the Handler interface for testing.

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- other
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/934): <!--number 934 --><!--line 0 --><!--description Y2hvcmU6IHJlZmFjdG9yIGFjdC9hcnRpZmFjdGNhY2hlIEhhbmRsZXIgdG8gYW4gaW50ZXJmYWNl-->chore: refactor act/artifactcache Handler to an interface<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/934
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

act/artifactcache/handler.go

@@ -28,7 +28,26 @@ const (
 	urlBase = "/_apis/artifactcache"
 )
 
-type Handler struct {
+type Handler interface {
+	ExternalURL() string
+	Close() error
+	isClosed() bool
+	openDB() (*bolthold.Store, error)
+	find(w http.ResponseWriter, r *http.Request, params httprouter.Params)
+	reserve(w http.ResponseWriter, r *http.Request, params httprouter.Params)
+	upload(w http.ResponseWriter, r *http.Request, params httprouter.Params)
+	commit(w http.ResponseWriter, r *http.Request, params httprouter.Params)
+	get(w http.ResponseWriter, r *http.Request, params httprouter.Params)
+	clean(w http.ResponseWriter, r *http.Request, _ httprouter.Params)
+	middleware(handler httprouter.Handle) httprouter.Handle
+	readCache(id uint64) (*Cache, error)
+	useCache(id uint64) error
+	setgcAt(at time.Time)
+	gcCache()
+	responseJSON(w http.ResponseWriter, r *http.Request, code int, v ...any)
+}
+
+type handler struct {
 	dir      string
 	storage  *Storage
 	router   *httprouter.Router
@@ -43,8 +62,8 @@ type Handler struct {
 	outboundIP string
 }
 
-func StartHandler(dir, outboundIP string, port uint16, secret string, logger logrus.FieldLogger) (*Handler, error) {
+func StartHandler(dir, outboundIP string, port uint16, secret string, logger logrus.FieldLogger) (Handler, error) {
-	h := &Handler{
+	h := &handler{
 		secret: secret,
 	}
 
@@ -114,14 +133,14 @@ func StartHandler(dir, outboundIP string, port uint16, secret string, logger log
 	return h, nil
 }
 
-func (h *Handler) ExternalURL() string {
+func (h *handler) ExternalURL() string {
 	port := strconv.Itoa(h.listener.Addr().(*net.TCPAddr).Port)
 
 	// TODO: make the external url configurable if necessary
 	return fmt.Sprintf("http://%s", net.JoinHostPort(h.outboundIP, port))
 }
 
-func (h *Handler) Close() error {
+func (h *handler) Close() error {
 	if h == nil {
 		return nil
 	}
@@ -146,7 +165,11 @@ func (h *Handler) Close() error {
 	return retErr
 }
 
-func (h *Handler) openDB() (*bolthold.Store, error) {
+func (h *handler) isClosed() bool {
+	return h.listener == nil && h.server == nil
+}
+
+func (h *handler) openDB() (*bolthold.Store, error) {
 	return bolthold.Open(filepath.Join(h.dir, "bolt.db"), 0o644, &bolthold.Options{
 		Encoder: json.Marshal,
 		Decoder: json.Unmarshal,
@@ -159,7 +182,7 @@ func (h *Handler) openDB() (*bolthold.Store, error) {
 }
 
 // GET /_apis/artifactcache/cache
-func (h *Handler) find(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
+func (h *handler) find(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
 	rundata := runDataFromHeaders(r)
 	repo, err := h.validateMac(rundata)
 	if err != nil {
@@ -216,7 +239,7 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, params httprouter
 }
 
 // POST /_apis/artifactcache/caches
-func (h *Handler) reserve(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
+func (h *handler) reserve(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
 	rundata := runDataFromHeaders(r)
 	repo, err := h.validateMac(rundata)
 	if err != nil {
@@ -255,7 +278,7 @@ func (h *Handler) reserve(w http.ResponseWriter, r *http.Request, params httprou
 }
 
 // PATCH /_apis/artifactcache/caches/:id
-func (h *Handler) upload(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
+func (h *handler) upload(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
 	rundata := runDataFromHeaders(r)
 	repo, err := h.validateMac(rundata)
 	if err != nil {
@@ -310,7 +333,7 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request, params httprout
 }
 
 // POST /_apis/artifactcache/caches/:id
-func (h *Handler) commit(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
+func (h *handler) commit(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
 	rundata := runDataFromHeaders(r)
 	repo, err := h.validateMac(rundata)
 	if err != nil {
@@ -374,7 +397,7 @@ func (h *Handler) commit(w http.ResponseWriter, r *http.Request, params httprout
 }
 
 // GET /_apis/artifactcache/artifacts/:id
-func (h *Handler) get(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
+func (h *handler) get(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
 	rundata := runDataFromHeaders(r)
 	repo, err := h.validateMac(rundata)
 	if err != nil {
@@ -417,7 +440,7 @@ func (h *Handler) get(w http.ResponseWriter, r *http.Request, params httprouter.
 }
 
 // POST /_apis/artifactcache/clean
-func (h *Handler) clean(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+func (h *handler) clean(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	rundata := runDataFromHeaders(r)
 	_, err := h.validateMac(rundata)
 	if err != nil {
@@ -430,7 +453,7 @@ func (h *Handler) clean(w http.ResponseWriter, r *http.Request, _ httprouter.Par
 	h.responseJSON(w, r, 200)
 }
 
-func (h *Handler) middleware(handler httprouter.Handle) httprouter.Handle {
+func (h *handler) middleware(handler httprouter.Handle) httprouter.Handle {
 	return func(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
 		h.logger.Debugf("%s %s", r.Method, r.RequestURI)
 		handler(w, r, params)
@@ -488,7 +511,7 @@ func insertCache(db *bolthold.Store, cache *Cache) error {
 	return nil
 }
 
-func (h *Handler) readCache(id uint64) (*Cache, error) {
+func (h *handler) readCache(id uint64) (*Cache, error) {
 	db, err := h.openDB()
 	if err != nil {
 		return nil, err
@@ -501,7 +524,7 @@ func (h *Handler) readCache(id uint64) (*Cache, error) {
 	return cache, nil
 }
 
-func (h *Handler) useCache(id uint64) error {
+func (h *handler) useCache(id uint64) error {
 	db, err := h.openDB()
 	if err != nil {
 		return err
@@ -522,7 +545,11 @@ const (
 	keepOld    = 5 * time.Minute
 )
 
-func (h *Handler) gcCache() {
+func (h *handler) setgcAt(at time.Time) {
+	h.gcAt = at
+}
+
+func (h *handler) gcCache() {
 	if h.gcing.Load() {
 		return
 	}
@@ -629,7 +656,7 @@ func (h *Handler) gcCache() {
 	}
 }
 
-func (h *Handler) responseJSON(w http.ResponseWriter, r *http.Request, code int, v ...any) {
+func (h *handler) responseJSON(w http.ResponseWriter, r *http.Request, code int, v ...any) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	var data []byte
 	if len(v) == 0 || v[0] == nil {

act/artifactcache/handler_test.go

@@ -76,8 +76,7 @@ func TestHandler(t *testing.T) {
 		})
 		t.Run("close", func(t *testing.T) {
 			require.NoError(t, handler.Close())
-			assert.Nil(t, handler.server)
+			assert.True(t, handler.isClosed())
-			assert.Nil(t, handler.listener)
 			_, err := httpClient.Post(fmt.Sprintf("%s/caches/%d", base, 1), "", nil)
 			assert.Error(t, err)
 		})
@@ -983,7 +982,7 @@ func TestHandler_gcCache(t *testing.T) {
 	}
 	require.NoError(t, db.Close())
 
-	handler.gcAt = time.Time{} // ensure gcCache will not skip
+	handler.setgcAt(time.Time{}) // ensure gcCache will not skip
 	handler.gcCache()
 
 	db, err = handler.openDB()
@@ -1010,8 +1009,7 @@ func TestHandler_ExternalURL(t *testing.T) {
 
 		assert.Equal(t, handler.ExternalURL(), "http://127.0.0.1:34567")
 		require.NoError(t, handler.Close())
-		assert.Nil(t, handler.server)
+		assert.True(t, handler.isClosed())
-		assert.Nil(t, handler.listener)
 	})
 
 	t.Run("reports correct URL on IPv6 zero host", func(t *testing.T) {
@@ -1021,8 +1019,7 @@ func TestHandler_ExternalURL(t *testing.T) {
 
 		assert.Equal(t, handler.ExternalURL(), "http://[2001:db8::]:34567")
 		require.NoError(t, handler.Close())
-		assert.Nil(t, handler.server)
+		assert.True(t, handler.isClosed())
-		assert.Nil(t, handler.listener)
 	})
 
 	t.Run("reports correct URL on IPv6", func(t *testing.T) {
@@ -1032,7 +1029,6 @@ func TestHandler_ExternalURL(t *testing.T) {
 
 		assert.Equal(t, handler.ExternalURL(), "http://[2001:db8::1:2:3:4]:34567")
 		require.NoError(t, handler.Close())
-		assert.Nil(t, handler.server)
+		assert.True(t, handler.isClosed())
-		assert.Nil(t, handler.listener)
 	})
 }

act/artifactcache/mac.go

@@ -16,7 +16,7 @@ import (
 
 var ErrValidation = errors.New("validation error")
 
-func (h *Handler) validateMac(rundata cacheproxy.RunData) (string, error) {
+func (h *handler) validateMac(rundata cacheproxy.RunData) (string, error) {
 	// TODO: allow configurable max age
 	if !validateAge(rundata.Timestamp) {
 		return "", ErrValidation

act/artifactcache/mac_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func TestMac(t *testing.T) {
-	handler := &Handler{
+	handler := &handler{
 		secret: "secret for testing",
 	}
 

internal/app/cmd/exec.go

@@ -54,7 +54,7 @@ type executeArgs struct {
 	debug                 bool
 	dryrun                bool
 	image                 string
-	cacheHandler          *artifactcache.Handler
+	cacheHandler          artifactcache.Handler
 	network               string
 	enableIPv6            bool
 	githubInstance        string