Skip to content

Commit f825e42

Browse files
fix: cache adjust restore order of exact key matches (#2267)
* wip: adjust restore order * fixup * add tests * cleanup * fix typo --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent d9a19c8 commit f825e42

File tree

2 files changed

+115
-0
lines changed

2 files changed

+115
-0
lines changed

pkg/artifactcache/handler.go

+11
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,17 @@ func (h *Handler) middleware(handler httprouter.Handle) httprouter.Handle {
352352
func findCache(db *bolthold.Store, keys []string, version string) (*Cache, error) {
353353
cache := &Cache{}
354354
for _, prefix := range keys {
355+
// if a key in the list matches exactly, don't return partial matches
356+
if err := db.FindOne(cache,
357+
bolthold.Where("Key").Eq(prefix).
358+
And("Version").Eq(version).
359+
And("Complete").Eq(true).
360+
SortBy("CreatedAt").Reverse()); err == nil || !errors.Is(err, bolthold.ErrNotFound) {
361+
if err != nil {
362+
return nil, fmt.Errorf("find cache: %w", err)
363+
}
364+
return cache, nil
365+
}
355366
prefixPattern := fmt.Sprintf("^%s", regexp.QuoteMeta(prefix))
356367
re, err := regexp.Compile(prefixPattern)
357368
if err != nil {

pkg/artifactcache/handler_test.go

+104
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,110 @@ func TestHandler(t *testing.T) {
422422
assert.Equal(t, key+"_abc", got.CacheKey)
423423
}
424424
})
425+
426+
t.Run("exact keys are preferred (key 0)", func(t *testing.T) {
427+
version := "c19da02a2bd7e77277f1ac29ab45c09b7d46a4ee758284e26bb3045ad11d9d20"
428+
key := strings.ToLower(t.Name())
429+
keys := [3]string{
430+
key + "_a",
431+
key + "_a_b_c",
432+
key + "_a_b",
433+
}
434+
contents := [3][]byte{
435+
make([]byte, 100),
436+
make([]byte, 200),
437+
make([]byte, 300),
438+
}
439+
for i := range contents {
440+
_, err := rand.Read(contents[i])
441+
require.NoError(t, err)
442+
uploadCacheNormally(t, base, keys[i], version, contents[i])
443+
time.Sleep(time.Second) // ensure CreatedAt of caches are different
444+
}
445+
446+
reqKeys := strings.Join([]string{
447+
key + "_a",
448+
key + "_a_b",
449+
}, ",")
450+
451+
resp, err := http.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, reqKeys, version))
452+
require.NoError(t, err)
453+
require.Equal(t, 200, resp.StatusCode)
454+
455+
/*
456+
Expect `key_a` because:
457+
- `key_a` matches `key_a`, `key_a_b` and `key_a_b_c`, but `key_a` is an exact match.
458+
- `key_a_b` matches `key_a_b` and `key_a_b_c`, but previous key had a match
459+
*/
460+
expect := 0
461+
462+
got := struct {
463+
ArchiveLocation string `json:"archiveLocation"`
464+
CacheKey string `json:"cacheKey"`
465+
}{}
466+
require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
467+
assert.Equal(t, keys[expect], got.CacheKey)
468+
469+
contentResp, err := http.Get(got.ArchiveLocation)
470+
require.NoError(t, err)
471+
require.Equal(t, 200, contentResp.StatusCode)
472+
content, err := io.ReadAll(contentResp.Body)
473+
require.NoError(t, err)
474+
assert.Equal(t, contents[expect], content)
475+
})
476+
477+
t.Run("exact keys are preferred (key 1)", func(t *testing.T) {
478+
version := "c19da02a2bd7e77277f1ac29ab45c09b7d46a4ee758284e26bb3045ad11d9d20"
479+
key := strings.ToLower(t.Name())
480+
keys := [3]string{
481+
key + "_a",
482+
key + "_a_b_c",
483+
key + "_a_b",
484+
}
485+
contents := [3][]byte{
486+
make([]byte, 100),
487+
make([]byte, 200),
488+
make([]byte, 300),
489+
}
490+
for i := range contents {
491+
_, err := rand.Read(contents[i])
492+
require.NoError(t, err)
493+
uploadCacheNormally(t, base, keys[i], version, contents[i])
494+
time.Sleep(time.Second) // ensure CreatedAt of caches are different
495+
}
496+
497+
reqKeys := strings.Join([]string{
498+
"------------------------------------------------------",
499+
key + "_a",
500+
key + "_a_b",
501+
}, ",")
502+
503+
resp, err := http.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, reqKeys, version))
504+
require.NoError(t, err)
505+
require.Equal(t, 200, resp.StatusCode)
506+
507+
/*
508+
Expect `key_a` because:
509+
- `------------------------------------------------------` doesn't match any caches.
510+
- `key_a` matches `key_a`, `key_a_b` and `key_a_b_c`, but `key_a` is an exact match.
511+
- `key_a_b` matches `key_a_b` and `key_a_b_c`, but previous key had a match
512+
*/
513+
expect := 0
514+
515+
got := struct {
516+
ArchiveLocation string `json:"archiveLocation"`
517+
CacheKey string `json:"cacheKey"`
518+
}{}
519+
require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
520+
assert.Equal(t, keys[expect], got.CacheKey)
521+
522+
contentResp, err := http.Get(got.ArchiveLocation)
523+
require.NoError(t, err)
524+
require.Equal(t, 200, contentResp.StatusCode)
525+
content, err := io.ReadAll(contentResp.Body)
526+
require.NoError(t, err)
527+
assert.Equal(t, contents[expect], content)
528+
})
425529
}
426530

427531
func uploadCacheNormally(t *testing.T, base, key, version string, content []byte) {

0 commit comments

Comments
 (0)