Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getKeys does not return normalised keys #563

Closed
43081j opened this issue Jan 3, 2025 · 3 comments
Closed

getKeys does not return normalised keys #563

43081j opened this issue Jan 3, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@43081j
Copy link
Member

43081j commented Jan 3, 2025

Environment

N/A (all)

Reproduction

// with the fs driver

// writes `foo/bar`
await storage.setItem("foo:bar", "content");

// returns `foo/bar`
await storage.getKeys();

Describe the bug

When we setItem, we normalise keys by replacing / with :.

This means, in the FS adapters especially, both setItem("foo/bar", v) or setItem("foo:bar", v) will write to the same file.

however, when we later call getKeys(), we don't translate those keys on the way out.

this means we will receive / separated keys (e.g. foo/bar in the previous example).

we probably only want to deal with / separated keys as input, but always expose keys from our side as : separated. that would mean getKeys of each driver needs to replaceAll("/", ":") basically.

Additional context

No response

Logs

No response

@43081j 43081j added the bug Something isn't working label Jan 3, 2025
@43081j 43081j changed the title getKeys returning normalised keys getKeys does not return normalised keys Jan 3, 2025
@pi0
Copy link
Member

pi0 commented Jan 7, 2025

Thanks for the detailed issue (strange we haven't caught it before!).

  • Tests should have ensure getKeys() returns with : notion
  • fs driver should normalize returned keys
  • Core should normalize keys (in case of similar driver issues (lets do it if we see more drivers have this issue)

@43081j
Copy link
Member Author

43081j commented Jan 9, 2025

@pi0 i had another look at this and think i was talking nonsense, somehow

it seems Storage does normalise in getKeys (across all drivers), and individual storage drivers don't

i was probably running getKeys on the driver itself when i saw this behaviour, thats the only explanation i can think of

so we can close this if you like, the current tests already cover it from Storage point of view

@pi0
Copy link
Member

pi0 commented Jan 9, 2025

Okay, okay thanks for confirm. let's close it, if you had any chance to evaluate how many "drivers" currently don't normalize (if majority do, we might remove core overheadperhaps not sure if there is benefit..)

@pi0 pi0 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants