Skip to content

fix: exact matching#60

Closed
jcmfernandes wants to merge 5 commits intotespkg:mainfrom
bckground:main
Closed

fix: exact matching#60
jcmfernandes wants to merge 5 commits intotespkg:mainfrom
bckground:main

Conversation

@jcmfernandes
Copy link

@jcmfernandes jcmfernandes commented Jun 23, 2025

I noticed that exact matching is broken because I was seeing an old cache being restored, which can happen because:

  1. The call to listObjects() can return multiple objects
  2. We aren't looking for an exact match
  3. We do not sort the return from listObjects()

Changes

  • Make exact matching work as expected: it's exact.

Fix Issues

None, as I worked on the fix before reporting the issue.

This isn't perfect, since using `startsWith` doesn't really guarantee
an exact match, but it's good enough and certainly better than what I found.
jackieli-tes added a commit that referenced this pull request Mar 11, 2026
listObjects uses MinIO prefix matching, so key "foo" also returns
"foo-bar/cache.tzst". The previous fix (PR #60) used startsWith(key)
which still has the same problem. Use startsWith(key + "/") to ensure
the key matches as a full path segment.

Add a workflow test that saves a decoy cache with a longer key sharing
the same prefix, then verifies exact restore does not match it.
jackieli-tes added a commit that referenced this pull request Mar 11, 2026
* fix: exact match should check key as path segment, not string prefix

listObjects uses MinIO prefix matching, so key "foo" also returns
"foo-bar/cache.tzst". The previous fix (PR #60) used startsWith(key)
which still has the same problem. Use startsWith(key + "/") to ensure
the key matches as a full path segment.

Add a workflow test that saves a decoy cache with a longer key sharing
the same prefix, then verifies exact restore does not match it.

* fix: use path.sep instead of hardcoded "/" for Windows compatibility

On Windows, path.join produces backslash separators, so MinIO objects
have "\" in their names. The exact match check must use path.sep to
match the platform's separator.
@jackieli-tes
Copy link
Contributor

Thanks for your contribution. fixed by #65

@jcmfernandes
Copy link
Author

Thanks for your contribution. fixed by #65

I saw your fix, and you're absolutely right that this is still broken. Thanks for addressing the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants