Skip to content

Commit

Permalink
refactor(pkg/cache/upstream): use context logger instead of struct lo…
Browse files Browse the repository at this point in the history 
…gger (#142)
  • Loading branch information
@kalbasit
kalbasit authored Dec 22, 2024
1 parent f64686c commit 7203da8
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 152 deletions.
6 changes: 3 additions & 3 deletions cmd/serve.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func serveAction() cli.ActionFunc {
return autoMaxProcs(ctx, 30*time.Second, logger)
})

ucs, err := getUpstreamCaches(ctx, logger, cmd)
ucs, err := getUpstreamCaches(ctx, cmd)
if err != nil {
return fmt.Errorf("error computing the upstream caches: %w", err)
}
Expand Down Expand Up @@ -167,7 +167,7 @@ func serveAction() cli.ActionFunc {
}
}

func getUpstreamCaches(_ context.Context, logger zerolog.Logger, cmd *cli.Command) ([]upstream.Cache, error) {
func getUpstreamCaches(ctx context.Context, cmd *cli.Command) ([]upstream.Cache, error) {
ucSlice := cmd.StringSlice("upstream-cache")

ucs := make([]upstream.Cache, 0, len(ucSlice))
Expand All @@ -188,7 +188,7 @@ func getUpstreamCaches(_ context.Context, logger zerolog.Logger, cmd *cli.Comman
}
}

uc, err := upstream.New(logger, u, pubKeys)
uc, err := upstream.New(ctx, u, pubKeys)
if err != nil {
return nil, fmt.Errorf("error creating a new upstream cache: %w", err)
}
Expand Down
51 changes: 24 additions & 27 deletions pkg/cache/cache_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ import (

const cacheName = "cache.example.com"

//nolint:gochecknoglobals
var logger = zerolog.New(io.Discard)

func TestAddUpstreamCaches(t *testing.T) {
t.Run("upstream caches added at once", func(t *testing.T) {
t.Parallel()
Expand All @@ -51,7 +48,7 @@ func TestAddUpstreamCaches(t *testing.T) {
for _, idx := range randomOrder {
ts := testServers[idx]

uc, err := upstream.New(logger, testhelper.MustParseURL(t, ts.URL), nil)
uc, err := upstream.New(newContext(), testhelper.MustParseURL(t, ts.URL), nil)
require.NoError(t, err)

ucs = append(ucs, uc)
Expand All @@ -67,15 +64,13 @@ func TestAddUpstreamCaches(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.AddUpstreamCaches(ctx, ucs...)
c.AddUpstreamCaches(newContext(), ucs...)

for idx, uc := range c.upstreamCaches {
//nolint:gosec
Expand Down Expand Up @@ -106,7 +101,7 @@ func TestAddUpstreamCaches(t *testing.T) {
for _, idx := range randomOrder {
ts := testServers[idx]

uc, err := upstream.New(logger, testhelper.MustParseURL(t, ts.URL), nil)
uc, err := upstream.New(newContext(), testhelper.MustParseURL(t, ts.URL), nil)
require.NoError(t, err)

ucs = append(ucs, uc)
Expand All @@ -122,16 +117,14 @@ func TestAddUpstreamCaches(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

for _, uc := range ucs {
c.AddUpstreamCaches(ctx, uc)
c.AddUpstreamCaches(newContext(), uc)
}

for idx, uc := range c.upstreamCaches {
Expand All @@ -155,21 +148,19 @@ func TestRunLRU(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

ts := testdata.NewTestServer(t, 40)
defer ts.Close()

uc, err := upstream.New(logger, testhelper.MustParseURL(t, ts.URL), nil)
uc, err := upstream.New(newContext(), testhelper.MustParseURL(t, ts.URL), nil)
require.NoError(t, err)

c.AddUpstreamCaches(ctx, uc)
c.AddUpstreamCaches(newContext(), uc)
c.SetRecordAgeIgnoreTouch(0)

// NOTE: For this test, any nar that's explicitly testing the zstd
Expand Down Expand Up @@ -220,7 +211,7 @@ func TestRunLRU(t *testing.T) {

for _, narEntry := range allEntries {
nu := nar.URL{Hash: narEntry.NarHash, Compression: narEntry.NarCompression}
assert.True(t, c.narStore.HasNar(ctx, nu), "confirm all nars are in the store")
assert.True(t, c.narStore.HasNar(newContext(), nu), "confirm all nars are in the store")
}

// ensure time has moved by one sec for the last_accessed_at work
Expand Down Expand Up @@ -255,23 +246,23 @@ func TestRunLRU(t *testing.T) {
require.NoError(t, err)
}

c.runLRU(ctx)()
c.runLRU(newContext())()

// confirm all narinfos except the last one are in the store
for _, nar := range entries {
assert.True(t, c.narInfoStore.HasNarInfo(ctx, nar.NarInfoHash))
assert.True(t, c.narInfoStore.HasNarInfo(newContext(), nar.NarInfoHash))
}

assert.False(t, c.narInfoStore.HasNarInfo(ctx, lastEntry.NarInfoHash))
assert.False(t, c.narInfoStore.HasNarInfo(newContext(), lastEntry.NarInfoHash))

// confirm all nars except the last one are in the store
for _, narEntry := range entries {
nu := nar.URL{Hash: narEntry.NarHash, Compression: narEntry.NarCompression}
assert.True(t, c.narStore.HasNar(ctx, nu))
assert.True(t, c.narStore.HasNar(newContext(), nu))
}

nu := nar.URL{Hash: lastEntry.NarHash, Compression: lastEntry.NarCompression}
assert.False(t, c.narStore.HasNar(ctx, nu))
assert.False(t, c.narStore.HasNar(newContext(), nu))

// all narinfo records except the last one are in the database
for _, narEntry := range entries {
Expand All @@ -292,3 +283,9 @@ func TestRunLRU(t *testing.T) {
_, err = c.db.GetNarByHash(context.Background(), lastEntry.NarHash)
require.ErrorIs(t, sql.ErrNoRows, err)
}

func newContext() context.Context {
return zerolog.
New(io.Discard).
WithContext(context.Background())
}
83 changes: 32 additions & 51 deletions pkg/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ import (

const cacheName = "cache.example.com"

//nolint:gochecknoglobals
var logger = zerolog.New(io.Discard)

func TestNew(t *testing.T) {
t.Parallel()

Expand All @@ -54,12 +51,10 @@ func TestNew(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

_, err = cache.New(ctx, "", db, localStore, localStore, localStore)
_, err = cache.New(newContext(), "", db, localStore, localStore, localStore)
assert.ErrorIs(t, err, cache.ErrHostnameRequired)
})

Expand All @@ -74,12 +69,10 @@ func TestNew(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

_, err = cache.New(ctx, "https://cache.example.com", db, localStore, localStore, localStore)
_, err = cache.New(newContext(), "https://cache.example.com", db, localStore, localStore, localStore)
assert.ErrorIs(t, err, cache.ErrHostnameMustNotContainScheme)
})

Expand All @@ -94,12 +87,10 @@ func TestNew(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

_, err = cache.New(ctx, "cache.example.com/path/to", db, localStore, localStore, localStore)
_, err = cache.New(newContext(), "cache.example.com/path/to", db, localStore, localStore, localStore)
assert.ErrorIs(t, err, cache.ErrHostnameMustNotContainPath)
})

Expand All @@ -114,12 +105,10 @@ func TestNew(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

_, err = cache.New(ctx, cacheName, db, localStore, localStore, localStore)
_, err = cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)
})
})
Expand All @@ -138,12 +127,10 @@ func TestPublicKey(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

pubKey := c.PublicKey().String()
Expand Down Expand Up @@ -173,7 +160,7 @@ func TestGetNarInfo(t *testing.T) {
require.NoError(t, err)
defer os.RemoveAll(dir) // clean up

uc, err := upstream.New(logger, testhelper.MustParseURL(t, ts.URL), testdata.PublicKeys())
uc, err := upstream.New(newContext(), testhelper.MustParseURL(t, ts.URL), testdata.PublicKeys())
require.NoError(t, err)

dbFile := filepath.Join(dir, "var", "ncps", "db", "db.sqlite")
Expand All @@ -182,15 +169,13 @@ func TestGetNarInfo(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.AddUpstreamCaches(ctx, uc)
c.AddUpstreamCaches(newContext(), uc)
c.SetRecordAgeIgnoreTouch(0)

t.Run("narinfo does not exist upstream", func(t *testing.T) {
Expand Down Expand Up @@ -563,12 +548,10 @@ func TestPutNarInfo(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.SetRecordAgeIgnoreTouch(0)
Expand Down Expand Up @@ -705,12 +688,10 @@ func TestDeleteNarInfo(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.SetRecordAgeIgnoreTouch(0)
Expand Down Expand Up @@ -768,7 +749,7 @@ func TestGetNar(t *testing.T) {
require.NoError(t, err)
defer os.RemoveAll(dir) // clean up

uc, err := upstream.New(logger, testhelper.MustParseURL(t, ts.URL), testdata.PublicKeys())
uc, err := upstream.New(newContext(), testhelper.MustParseURL(t, ts.URL), testdata.PublicKeys())
require.NoError(t, err)

dbFile := filepath.Join(dir, "var", "ncps", "db", "db.sqlite")
Expand All @@ -777,15 +758,13 @@ func TestGetNar(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.AddUpstreamCaches(ctx, uc)
c.AddUpstreamCaches(newContext(), uc)
c.SetRecordAgeIgnoreTouch(0)

t.Run("nar does not exist upstream", func(t *testing.T) {
Expand Down Expand Up @@ -984,12 +963,10 @@ func TestPutNar(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.SetRecordAgeIgnoreTouch(0)
Expand Down Expand Up @@ -1031,12 +1008,10 @@ func TestDeleteNar(t *testing.T) {
db, err := database.Open("sqlite:" + dbFile)
require.NoError(t, err)

ctx := context.Background()

localStore, err := local.New(ctx, dir)
localStore, err := local.New(newContext(), dir)
require.NoError(t, err)

c, err := cache.New(ctx, cacheName, db, localStore, localStore, localStore)
c, err := cache.New(newContext(), cacheName, db, localStore, localStore, localStore)
require.NoError(t, err)

c.SetRecordAgeIgnoreTouch(0)
Expand Down Expand Up @@ -1085,3 +1060,9 @@ func TestDeleteNar(t *testing.T) {
})
})
}

func newContext() context.Context {
return zerolog.
New(io.Discard).
WithContext(context.Background())
}
Loading

0 comments on commit 7203da8

Please sign in to comment.