fix: update

Signed-off-by: Junjie Gao <[email protected]>

revocation/crl/cache/errors_test.go

@@ -0,0 +1,26 @@
+package cache
+
+import (
+	"errors"
+	"testing"
+	"time"
+)
+
+func TestCacheExpiredError(t *testing.T) {
+	expirationTime := time.Now()
+	err := &CacheExpiredError{Expires: expirationTime}
+
+	expectedMessage := "cache expired at " + expirationTime.String()
+	if err.Error() != expectedMessage {
+		t.Errorf("expected %q, got %q", expectedMessage, err.Error())
+	}
+}
+
+func TestBrokenFileError(t *testing.T) {
+	innerErr := errors.New("inner error")
+	err := &BrokenFileError{Err: innerErr}
+
+	if err.Error() != innerErr.Error() {
+		t.Errorf("expected %q, got %q", innerErr.Error(), err.Error())
+	}
+}

revocation/crl/cache/file.go

@@ -90,12 +90,13 @@ func (c *fileCache) Set(ctx context.Context, key string, bundle *Bundle) error {
 	}
 	tempFile.Close()
 
-	// rename is atomic on UNIX platforms
+	// rename is atomic on UNIX-like platforms
 	return os.Rename(tempFile.Name(), filepath.Join(c.dir, key))
 }
 
 // Delete removes the CRL bundle file from file system
 func (c *fileCache) Delete(ctx context.Context, key string) error {
+	// remove is atomic on UNIX-like platforms
 	return os.Remove(filepath.Join(c.dir, key))
 }
 
@@ -109,6 +110,7 @@ func (c *fileCache) Flush(ctx context.Context) error {
 			return nil
 		}
 
+		// remove is atomic on UNIX-like platforms
 		return os.Remove(path)
 	})
 }

revocation/crl/cache/memory.go

@@ -47,8 +47,9 @@ func (c *memoryCache) Get(ctx context.Context, key string) (*Bundle, error) {
 		return nil, fmt.Errorf("invalid type: %T", value)
 	}
 
-	if c.maxAge > 0 && time.Now().After(bundle.Metadata.CreateAt.Add(c.maxAge)) {
-		return nil, os.ErrNotExist
+	expires := bundle.Metadata.CreateAt.Add(c.maxAge)
+	if c.maxAge > 0 && time.Now().After(expires) {
+		return nil, &CacheExpiredError{Expires: expires}
 	}
 
 	return bundle, nil

revocation/internal/crl/fetcher.go → revocation/crl/fetcher/fetcher.go

@@ -1,4 +1,4 @@
-package crl
+package fetcher
 
 import (
 	"context"
@@ -20,71 +20,76 @@ const maxCRLSize = 64 * 1024 * 1024 // 64 MiB
 
 // Fetcher is an interface that specifies methods used for fetching CRL
 // from the given URL
-//
-// The interface is useful for pre-loading CRLs cache before the verification
 type Fetcher interface {
 	Fetch(ctx context.Context, crlURL string) (bundle *cache.Bundle, fromCache bool, err error)
 }
 
-type fetcher struct {
+type cachedFetcher struct {
 	httpClient  *http.Client
 	cacheClient cache.Cache
 }
 
-// NewFetcher creates a new Fetcher with the given HTTP client and cache client
+// NewCachedFetcher creates a new Fetcher with the given HTTP client and cache client
 //   - if httpClient is nil, http.DefaultClient will be used
 //   - if cacheClient is nil, no cache will be used
-func NewFetcher(httpClient *http.Client, cacheClient cache.Cache) Fetcher {
+func NewCachedFetcher(httpClient *http.Client, cacheClient cache.Cache) (Fetcher, error) {
 	if httpClient == nil {
 		httpClient = http.DefaultClient
 	}
 
-	return &fetcher{
+	if cacheClient == nil {
+		return nil, errors.New("cache client is nil")
+	}
+
+	return &cachedFetcher{
 		httpClient:  httpClient,
 		cacheClient: cacheClient,
-	}
+	}, nil
 }
 
 // Fetch retrieves the CRL from the given URL
-//   - if the cache is enabled, it will try to get the CRL from the cache first
-//   - if the CRL is not in the cache or expired, it will download the CRL from
-//     the URL
-func (f *fetcher) Fetch(ctx context.Context, crlURL string) (bundle *cache.Bundle, fromCache bool, err error) {
+//
+// Steps:
+//  1. Try to get from cache
+//  2. If not exist or broken, download and save to cache
+func (f *cachedFetcher) Fetch(ctx context.Context, crlURL string) (bundle *cache.Bundle, fromCache bool, err error) {
 	if crlURL == "" {
 		return nil, false, errors.New("CRL URL is empty")
 	}
 
-	if f.cacheClient == nil {
-		// no cache, download directly
-		return f.downloadAndCache(ctx, crlURL)
-	}
-
 	// try to get from cache
 	bundle, err = f.cacheClient.Get(ctx, tarStoreName(crlURL))
 	if err != nil {
 		var cacheBrokenError *cache.BrokenFileError
 		if os.IsNotExist(err) || errors.As(err, &cacheBrokenError) {
 			// download if not exist or broken
-			return f.downloadAndCache(ctx, crlURL)
+			bundle, err = f.Download(ctx, crlURL)
+			if err != nil {
+				return nil, false, err
+			}
+			return bundle, false, nil
 		}
+
 		return nil, false, err
 	}
 
 	return bundle, true, nil
 }
 
-func (f *fetcher) downloadAndCache(ctx context.Context, crlURL string) (bundle *cache.Bundle, fromCache bool, err error) {
+// Download downloads the CRL from the given URL and saves it to the
+// cache
+func (f *cachedFetcher) Download(ctx context.Context, crlURL string) (bundle *cache.Bundle, err error) {
 	bundle, err = download(ctx, crlURL, f.httpClient)
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 
 	// save to cache
 	if err := f.cacheClient.Set(ctx, tarStoreName(crlURL), bundle); err != nil {
-		return nil, false, fmt.Errorf("failed to save to cache: %w", err)
+		return nil, fmt.Errorf("failed to save to cache: %w", err)
 	}
 
-	return bundle, false, nil
+	return bundle, nil
 }
 
 func download(ctx context.Context, crlURL string, client *http.Client) (bundle *cache.Bundle, err error) {
@@ -111,8 +116,7 @@ func download(ctx context.Context, crlURL string, client *http.Client) (bundle *
 		return nil, fmt.Errorf("failed to download with status code: %d", resp.StatusCode)
 	}
 	// read with size limit
-	limitedReader := io.LimitReader(resp.Body, maxCRLSize)
-	data, err := io.ReadAll(limitedReader)
+	data, err := io.ReadAll(io.LimitReader(resp.Body, maxCRLSize))
 	if err != nil {
 		return nil, fmt.Errorf("failed to read CRL response: %w", err)
 	}
@@ -125,11 +129,7 @@ func download(ctx context.Context, crlURL string, client *http.Client) (bundle *
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse CRL: %w", err)
 	}
-	bundle, err = cache.NewBundle(crl, crlURL)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create bundle: %w", err)
-	}
-	return bundle, nil
+	return cache.NewBundle(crl, crlURL)
 }
 
 func tarStoreName(url string) string {