refactor: move cache to pkg, simplify handlers (#42)

* refactor: move cache to pkg

* refactor: simplify handlers

Signed-off-by: Aviral Takkar <avtakkar@microsoft.com>

Author: avtakkar

build/package/peerd-helm/Chart.yaml

@@ -21,4 +21,4 @@ version: 0.0.3-alpha
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
 # It is recommended to use it with quotes.
-appVersion: "0.0.6-alpha"
+appVersion: "0.0.7-alpha"

internal/files/store/file_test.go

@@ -10,8 +10,8 @@ import (
 	"testing"
 
 	"github.com/azure/peerd/internal/files"
-	"github.com/azure/peerd/internal/files/cache"
 	remotetests "github.com/azure/peerd/internal/remote/tests"
+	"github.com/azure/peerd/pkg/cache"
 	"github.com/azure/peerd/pkg/discovery/routing/tests"
 )
 

internal/files/store/main_test.go

@@ -9,7 +9,7 @@ import (
 	"os"
 	"testing"
 
-	"github.com/azure/peerd/internal/files/cache"
+	"github.com/azure/peerd/pkg/cache"
 	"github.com/azure/peerd/pkg/metrics"
 )
 

internal/files/store/mockstore.go

@@ -5,7 +5,7 @@ package store
 import (
 	"context"
 
-	"github.com/azure/peerd/internal/files/cache"
+	"github.com/azure/peerd/pkg/cache"
 	"github.com/azure/peerd/pkg/discovery/routing"
 )
 

internal/files/store/store.go

@@ -11,8 +11,8 @@ import (
 
 	p2pcontext "github.com/azure/peerd/internal/context"
 	"github.com/azure/peerd/internal/files"
-	"github.com/azure/peerd/internal/files/cache"
 	"github.com/azure/peerd/internal/remote"
+	"github.com/azure/peerd/pkg/cache"
 	"github.com/azure/peerd/pkg/discovery/routing"
 	"github.com/azure/peerd/pkg/metrics"
 	"github.com/azure/peerd/pkg/urlparser"
@@ -25,7 +25,7 @@ import (
 func NewFilesStore(ctx context.Context, r routing.Router) (FilesStore, error) {
 	fs := &store{
 		metricsRecorder: metrics.FromContext(ctx),
-		cache:           cache.New(ctx),
+		cache:           cache.New(ctx, int64(files.CacheBlockSize)),
 		prefetchChan:    make(chan prefetchableSegment, PrefetchWorkers),
 		prefetchable:    PrefetchWorkers > 0,
 		router:          r,

internal/handlers/files/handler.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package handlers
+package files
 
 import (
 	"context"

internal/handlers/files/handler_test.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package handlers
+package files
 
 import (
 	"context"

internal/handlers/files/main_test.go

@@ -1,14 +1,14 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package handlers
+package files
 
 import (
 	"crypto/rand"
 	"fmt"
 	"os"
 	"testing"
 
-	"github.com/azure/peerd/internal/files/cache"
+	"github.com/azure/peerd/pkg/cache"
 )
 
 func TestMain(m *testing.M) {

internal/handlers/root.go

@@ -9,23 +9,23 @@ import (
 
 	p2pcontext "github.com/azure/peerd/internal/context"
 	filesStore "github.com/azure/peerd/internal/files/store"
-	filesHandler "github.com/azure/peerd/internal/handlers/files"
-	ociHandler "github.com/azure/peerd/internal/handlers/v2"
+	"github.com/azure/peerd/internal/handlers/files"
+	v2 "github.com/azure/peerd/internal/handlers/v2"
 	"github.com/azure/peerd/pkg/containerd"
 	"github.com/azure/peerd/pkg/discovery/routing"
 	"github.com/gin-gonic/gin"
 	"github.com/rs/zerolog"
 )
 
-var fh *filesHandler.FilesHandler
-var v2h *ociHandler.V2Handler
+var fh *files.FilesHandler
+var v2h *v2.V2Handler
 
 // Server creates a new HTTP server.
 func Handler(ctx context.Context, r routing.Router, containerdStore containerd.Store, fs filesStore.FilesStore) (http.Handler, error) {
 	var err error
-	fh = filesHandler.New(ctx, fs)
+	fh = files.New(ctx, fs)
 
-	v2h, err = ociHandler.New(ctx, r, containerdStore)
+	v2h, err = v2.New(ctx, r, containerdStore)
 	if err != nil {
 		return nil, err
 	}

internal/handlers/v2/handler.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package handlers
+package v2
 
 import (
 	"context"
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	p2pcontext "github.com/azure/peerd/internal/context"
-	"github.com/azure/peerd/internal/oci"
 	"github.com/azure/peerd/pkg/containerd"
 	"github.com/azure/peerd/pkg/discovery/routing"
 	"github.com/azure/peerd/pkg/metrics"
@@ -19,8 +18,8 @@ import (
 
 // V2Handler describes a handler for OCI content.
 type V2Handler struct {
-	mirror          *oci.Mirror
-	registry        *oci.Registry
+	mirror          *Mirror
+	registry        *Registry
 	metricsRecorder metrics.Metrics
 }
 
@@ -90,8 +89,8 @@ func (h *V2Handler) fill(c *gin.Context) error {
 // New creates a new OCI content handler.
 func New(ctx context.Context, router routing.Router, containerdStore containerd.Store) (*V2Handler, error) {
 	return &V2Handler{
-		mirror:          oci.NewMirror(router),
-		registry:        oci.NewRegistry(containerdStore),
+		mirror:          NewMirror(router),
+		registry:        NewRegistry(containerdStore),
 		metricsRecorder: metrics.FromContext(ctx),
 	}, nil
 }

internal/handlers/v2/handler_test.go

@@ -1,4 +1,4 @@
-package handlers
+package v2
 
 import (
 	"context"

internal/oci/mirror.go renamed to internal/handlers/v2/mirror.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package oci
+package v2
 
 import (
 	"context"

internal/oci/mirror_test.go renamed to internal/handlers/v2/mirror_test.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package oci
+package v2
 
 import (
 	"fmt"

internal/oci/registry.go renamed to internal/handlers/v2/registry.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package oci
+package v2
 
 import (
 	"fmt"

internal/oci/registry_test.go renamed to internal/handlers/v2/registry_test.go

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-package oci
+package v2
 
 import (
 	"net/http"

internal/remote/interface.go

@@ -20,7 +20,7 @@ type Reader interface {
 	Log() *zerolog.Logger
 }
 
-// Error describes an error that occured during a remote operation.
+// Error describes an error that occurred during a remote operation.
 type Error struct {
 	*http.Response
 	error

internal/files/cache/cache.go renamed to pkg/cache/filecache.go

@@ -13,7 +13,6 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/azure/peerd/internal/files"
 	"github.com/dgraph-io/ristretto"
 	"github.com/rs/zerolog"
 )
@@ -149,7 +148,8 @@ func waitForSet() {
 }
 
 // New creates a new cache of files.
-func New(ctx context.Context) Cache {
+// cacheBlockSize is the fixed size of the cache block in bytes, and is used to evaluate the cost of each item in the cache.
+func New(ctx context.Context, cacheBlockSize int64) Cache {
 	log := zerolog.Ctx(ctx).With().Str("component", "cache").Logger()
 
 	atomic.StoreInt32(&fdCnt, 0)
@@ -176,7 +176,7 @@ func New(ctx context.Context) Cache {
 		},
 
 		Cost: func(val interface{}) int64 {
-			return int64(files.CacheBlockSize)
+			return cacheBlockSize
 		},
 	}); err != nil {
 		// This will call os.Exit(1)

internal/files/cache/cache_test.go renamed to pkg/cache/filecache_test.go

@@ -18,10 +18,14 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
+var (
+	cacheBlockSize = int64(1 * 1024 * 1024)
+)
+
 func TestGetKey(t *testing.T) {
 	name := newRandomStringN(10)
 	offset := int64(100)
-	c := New(context.Background())
+	c := New(context.Background(), cacheBlockSize)
 	got := c.(*fileCache).getKey(name, offset)
 	want := fmt.Sprintf("%v/%v/%v", Path, name, offset)
 	if got != want {
@@ -30,7 +34,7 @@ func TestGetKey(t *testing.T) {
 }
 
 func TestExists(t *testing.T) {
-	c := New(context.Background())
+	c := New(context.Background(), cacheBlockSize)
 
 	filesThatExist := []string{}
 	for i := 0; i < 5; i++ {
@@ -113,7 +117,7 @@ func TestExists(t *testing.T) {
 }
 
 func TestPutAndGetSize(t *testing.T) {
-	c := New(context.Background())
+	c := New(context.Background(), cacheBlockSize)
 	var eg errgroup.Group
 
 	for i := 0; i < 1000; i++ {
@@ -150,7 +154,7 @@ func TestPutAndGetSize(t *testing.T) {
 func TestGetOrCreate(t *testing.T) {
 	zerolog.TimeFieldFormat = time.RFC3339
 	//c := New(zerolog.New(os.Stdout).With().Timestamp().Logger().WithContext(context.Background()))
-	c := New(context.Background())
+	c := New(context.Background(), cacheBlockSize)
 	var eg errgroup.Group
 
 	fileNames := new(sync.Map)

internal/files/cache/interface.go renamed to pkg/cache/interface.go

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 package cache
 
-// Cache describes the cache of files.
+// Cache describes a cache of files.
 type Cache interface {
 	// Size gets the size of the file.
 	Size(path string) (int64, bool)
@@ -18,12 +18,12 @@ type Cache interface {
 }
 
 var (
-	// FilesCacheMaxCost is the capacity of the files cache in any unit.
+	// FilesCacheMaxCost is the capacity of the files cache.
 	FilesCacheMaxCost int64 = 4 * 1024 * 1024 * 1024 // 4 Gib
 
-	// MemoryCacheMaxCost is the capacity of the memory cache in any unit.
+	// MemoryCacheMaxCost is the capacity of the memory cache.
 	MemoryCacheMaxCost int64 = 1 * 1024 * 1024 * 1024 // 1 Gib
 
 	// Path is the path to the cache directory.
-	Path string = "/tmp/distribution/p2p/cache"
+	Path string = "/tmp/distribution/peerd/cache"
 )

internal/files/cache/item.go renamed to pkg/cache/item.go

@@ -48,7 +48,7 @@ type network struct {
 var _ Network = &network{}
 
 // DefaultTLSConfig creates a TLS config to use for this server.
-// This config does not require client certificate verification and is resuable.
+// This config does not require client certificate verification and is reusable.
 func (n *network) DefaultTLSConfig() *tls.Config {
 	return n.defaultTLSConfig
 }