feat(redis): opt-in raw support

[cjpearson]

Resolves #559

This change adds a new saveRawAsBinary option to the redis driver. When enabled, the driver will use the setBuffer and getBuffer methods of ioredis to directly save Buffers rather than base64 encoding and saving as a string. The option is not enabled by default as that would be a backwards incompatible change.

When #528 is implemented, this could be made the default for {type: 'bytes'}. (Perhaps then saveBytesAsBinary is a better name?) It would also be possible to drop the base64 encoding entirely and instead allow users to manually encode and save as { type: 'text' }.

[codecov]

Codecov Report

Attention: Patch coverage is 64.15094% with 19 lines in your changes missing coverage. Please review.

Project coverage is 60.07%. Comparing base (4d61c78) to head (b92591e).
Report is 160 commits behind head on main.

Files with missing lines	Patch %	Lines
src/drivers/redis.ts	64.15%	11 Missing and 8 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
- Coverage   65.05%   60.07%   -4.98%     
==========================================
  Files          39       42       +3     
  Lines        4055     3702     -353     
  Branches      487      615     +128     
==========================================
- Hits         2638     2224     -414     
- Misses       1408     1467      +59     
- Partials        9       11       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cjpearson]

I would suggest something like binary/useBinary here. I think it makes the effect a bit more clear because it changes how raw data is stored, but does not enable/disable the ability to save raw data. Or it could be inverted to base64 so in the future the default value can be false.

[pi0]

I agree that in general binary could be more describing than raw. In the context of unstorage, we call binary-compatible feature raw which was the reason I was thinking to use this.

Considering it is for short-term solution, are you happy we go with raw?

(Also check fb2977a, I have extended normalization)

[cjpearson]

Extending normalization to additional variants of TypedArray makes sense, but it should only allow for binary data and continue to throw when other data is passed. Otherwise it would lead to some unexpected behavior as you save an object but get a Buffer back.

That's also why I think the different naming is helpful. binary means that setItemRaw accepts binary and getItemRaw returns binary. Whereas the existing raw behavior accepts binary and other things.

[pi0]

This is consistent with generic unstorage.*raw* interface behavior -- happy to change it in the future (so ALL calls to setItemRaw will be restricted to accept binary only not objects) but normally, users should use the same key either for raw or non-raw purposes.

[cjpearson]

Hm would it be possible to add an additional option in that case? Either on the driver or the method? What I'm looking for is a way to save binary data and ensure it is binary and saved as binary and when getting, that I get a Buffer in return.

My assumption is that is how getItem(key, { type: 'bytes'})/setItem(key, value, { type: 'bytes'}) would work in the future.

[pi0]

Yes exactly for type: 'bytes' we can get strict.

Currently, if someone passes a string to setItemRaw, getItemRaw gives a Buffer representation of Utf8 bytes which isn't that odd ,is it?

[cjpearson]

Yeah that's what I find a bit unexpected 😅

Do you think there would be an issue if we added type: 'bytes' as a method option here? It would be just for the redis driver at the moment, but done in a forward compatible manner.

Or is it better to use something else and save that wording for later