feat: add has and readAll methods to StorageAdapter interface

[roziscoding]

These methods will be used mainly by the chat-members and the broadcast
plugins

[codecov-commenter]

Codecov Report

Base: 36.70% // Head: 36.65% // Decreases project coverage by -0.05% ⚠️

Coverage data is based on head (ececf38) compared to base (bcb1fbe).
Patch coverage: 26.66% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
- Coverage   36.70%   36.65%   -0.05%     
==========================================
  Files          18       18              
  Lines        4588     4602      +14     
  Branches      166      166              
==========================================
+ Hits         1684     1687       +3     
- Misses       2902     2913      +11     
  Partials        2        2              

Impacted Files	Coverage Δ
src/convenience/session.ts	96.88% <26.66%> (-3.12%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[KnorpelSenf]

It does not list all the keys. But perhaps we should consider doing this instead. I think only retrieving values without the corresponding keys is rarely useful, so I see three alternatives here:

1. Provide readAllKeys only
2. Provide readAllKeys and readAllEntries which returns [key, value] pairs
3. Provide readAllKeys and readAllEntries and readAllValues

Also, we have to think about readAll from the current memory sessions. Should we align the name in the adapter for 2.0 (deprecate until then) or rather align the name in the interface to readAll?

[roziscoding]

It does not list all the keys. But perhaps we should consider doing this instead. I think only retrieving values without the corresponding keys is rarely useful, so I see three alternatives here:

You're right.
I haven't thought about the implementation of the broadcast plugin enough to be able to tell if we'll need keys, entries, or values, so I'd say we go with 3. to be future proof.

Should we align the name in the adapter for 2.0 (deprecate until then)

I'm assuming by this you mean we add the three new methods and have readAll call readValues (current behavior), then remove it on 2.0, right? If so, I think this is the way to go

[KnorpelSenf]

Yes, sounds good then!

[KnorpelSenf]

@Satont I'm very curious what your opinion is on this

[roziscoding]

@Satont I'm very curious what your opinion is on this

I'll wait for this before I move on

[Satont]

I'm wondering about implementation of something like that. Already expect to "crutches".

I'm interested about use case such methods, can you explain a little more about what this is for??

[roziscoding]

I'm interested about use case such methods, can you explain a little more about what this is for??

Initially, for the chat-members and the broadcast plugins.
The chat-members plugin stores information about users in the storage, and the broadcast plugin will need to list all those members in order to send messages to them.
chat-members need has to check wether a member has already been saved, and broadcast need readAll / readEntries

[Satont]

I'm interested about use case such methods, can you explain a little more about what this is for??

Initially, for the chat-members and the broadcast plugins.
The chat-members plugin stores information about users in the storage, and the broadcast plugin will need to list all those members in order to send messages to them.
chat-members need has to check wether a member has already been saved, and broadcast need readAll / readEntries

Ok. Do you plan implement all those methods in storages?

[roziscoding]

Ok. Do you plan implement all those methods in storages?

I can try to, yes. But the idea of havin the optional properties is so we don't necessarily have to do that.

[Satont]

I can try to, yes. But the idea of havin the optional properties is so we don't necessarily have to do that.

Ok, then go on.

[KnorpelSenf]

LGTM.

Please implement these changes for the built-in memory sessions. Then we can merge.

[KnorpelSenf]

One last question. When using the new storage adapter, do we ever need to differentiate between sync and async iterables? If yes, how could this be done?

[roziscoding]

One last question. When using the new storage adapter, do we ever need to differentiate between sync and async iterables?

We shouldn't need to, because sync iterables can be consumed with await the same way async ones can.

[KnorpelSenf]

Awesome, that's what I was hoping for!

[KnorpelSenf]

LGTM, thanks!