From 858ccc0ee531a007345a187bb8eaed2186a28c9e Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Wed, 14 Oct 2020 16:44:39 +1100
Subject: [PATCH 1/2] WIP: use url path as key

---
 internal/vfs/zip/vfs.go | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go
index 78a77e1cb..3b06f70d4 100644
--- a/internal/vfs/zip/vfs.go
+++ b/internal/vfs/zip/vfs.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/url"
+	"strings"
 	"sync"
 	"time"
 
@@ -101,14 +102,15 @@ func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArc
 	fs.cacheLock.Lock()
 	defer fs.cacheLock.Unlock()
 
-	archive, expiry, found := fs.cache.GetWithExpiration(path)
+	key := strings.TrimRight(path, "?")
+	archive, expiry, found := fs.cache.GetWithExpiration(key)
 	if found {
 		metrics.ZipCacheRequests.WithLabelValues("archive", "hit").Inc()
 
 		// TODO: do not refreshed errored archives https://gitlab.com/gitlab-org/gitlab-pages/-/merge_requests/351
 		if time.Until(expiry) < defaultCacheRefreshInterval {
 			// refresh item
-			fs.cache.SetDefault(path, archive)
+			fs.cache.SetDefault(key, archive)
 		}
 	} else {
 		archive = newArchive(fs, path, DefaultOpenTimeout)
@@ -120,7 +122,7 @@ func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArc
 
 		// if adding the archive to the cache fails it means it's already been added before
 		// this is done to find concurrent additions.
-		if fs.cache.Add(path, archive, cache.DefaultExpiration) != nil {
+		if fs.cache.Add(key, archive, cache.DefaultExpiration) != nil {
 			return nil, errAlreadyCached
 		}
 
-- 
GitLab


From b05139acee33d23743ff2211de2d3bd847a86d13 Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Wed, 14 Oct 2020 17:28:12 +1100
Subject: [PATCH 2/2] WIP: Use etag as archive hash

---
 internal/httprange/http_reader.go |  7 +++++++
 internal/vfs/zip/archive.go       |  8 ++++++++
 internal/vfs/zip/vfs.go           | 20 ++++++++++++++++++--
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/internal/httprange/http_reader.go b/internal/httprange/http_reader.go
index 44694e858..c57378139 100644
--- a/internal/httprange/http_reader.go
+++ b/internal/httprange/http_reader.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httputil"
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httptransport"
@@ -76,12 +77,18 @@ func (r *Reader) ensureResponse() error {
 
 	metrics.HTTPRangeOpenRequests.Inc()
 
+	dreq, err := httputil.DumpRequestOut(req, true)
+	fmt.Printf("req: %s err: %+v\n", dreq, err)
+
 	res, err := httpClient.Do(req)
 	if err != nil {
 		metrics.HTTPRangeOpenRequests.Dec()
 		return err
 	}
 
+	dres, err := httputil.DumpResponse(res, false)
+	fmt.Printf("res: %s err: %+v\n", dres, err)
+
 	err = r.setResponse(res)
 	if err != nil {
 		metrics.HTTPRangeOpenRequests.Dec()
diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go
index 548ba6518..7846c8788 100644
--- a/internal/vfs/zip/archive.go
+++ b/internal/vfs/zip/archive.go
@@ -247,3 +247,11 @@ func (a *zipArchive) Readlink(ctx context.Context, name string) (string, error)
 func (a *zipArchive) onEvicted() {
 	metrics.ZipArchiveEntriesCached.Sub(float64(len(a.files)))
 }
+
+func (a *zipArchive) Hash() string {
+	if a.resource == nil {
+		return ""
+	}
+
+	return a.resource.ETag
+}
diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go
index 3b06f70d4..82e63d721 100644
--- a/internal/vfs/zip/vfs.go
+++ b/internal/vfs/zip/vfs.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/patrickmn/go-cache"
 
+	"gitlab.com/gitlab-org/gitlab-pages/internal/httprange"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
 )
@@ -109,8 +110,23 @@ func (fs *zipVFS) findOrCreateArchive(ctx context.Context, path string) (*zipArc
 
 		// TODO: do not refreshed errored archives https://gitlab.com/gitlab-org/gitlab-pages/-/merge_requests/351
 		if time.Until(expiry) < defaultCacheRefreshInterval {
-			// refresh item
-			fs.cache.SetDefault(key, archive)
+
+			zipArchive := archive.(*zipArchive)
+
+			res, err := httprange.NewResource(ctx, path)
+			if err != nil {
+				err := zipArchive.openArchive(ctx)
+				if err != nil {
+					return nil, err
+				}
+				// do not refresh on error
+				return zipArchive, nil
+			}
+
+			if res.ETag != zipArchive.Hash() {
+				// refresh item with new path
+				fs.cache.SetDefault(key, archive)
+			}
 		}
 	} else {
 		archive = newArchive(fs, path, DefaultOpenTimeout)
-- 
GitLab