### General usage
Most users should only need to use the `evalScript` method. We take the approach recommended by the [redis EVAL docs](https://redis.io/commands/eval):
>>>
The client library implementation can always optimistically send EVALSHA under the hood even when the client actually calls EVAL, in the hope the script was already seen by the server. If the NOSCRIPT error is returned EVAL will be used instead.
>>>
To facilitate this, we introduce a RedisScript struct, which calculates the sha1 hash on the client and stores it alongside the script source. I took inspiration for this approach from the [radix golang library](https://github.com/mediocregopher/radix). The struct makes using scripts a bit easier/nicer, and guides the developer toward the highest performance approach, but I'm curious whether you think it's an unnecessary abstraction.
One disadvantage of the optimistic approach using `EVALSHA` is that the `evalScript` may not be atomic if the script is not cached yet, which will be a problem if we are using it during an explicit pipeline or MULTI/EXEC operation. The redis docs recommend using `SCRIPT LOAD` before a multi/exec operation to avoid this. I think we could leave this up to the application developer.
### Should we implement the basic commands?
The approach above forces the application developer to use the evalScript method instead of exposing either `EVAL` or `EVALSHA` directly. This is to avoid the behavior of only using `EVAL` without caching, which might otherwise seem like the natural choice. If you think this is too opinionated, we can also add basic `eval` and `evalSha` methods.
### SHA1 calculation
I've copied the approach used by the SwiftNIO developers - RediStack already includes the `CNIOSHA1` module required.
There's a comment in [their private implementation](https://github.com/apple/swift-nio/blob/master/Sources/NIOWebSocket/SHA1.swift#L17) which indicates they're not entirely happy with this, but consider it the best approach for server-side Swift for now. I think we could follow their lead and migrate to a standard library method if/when it becomes available.
The `sha1` method is added as an extension property on String. Because it can theoretically fail, the `sha1` property is optional and thus the RedisScript initializer is failable, which is a bit unpleasant. I don't think the sha1 calculation should never actually fail given a String input. A force unwrap in the sha1 property would remove several code smells [like this](https://gitlab.com/liamdon/swift-redi-stack/blob/scripting/Sources/RediStack/Commands/ScriptingCommands.swift#L92), but I left it out in case you have a strong opposition to force unwraps - let me know what you think.
### Not implemented
I have not implemented `SCRIPT DEBUG` or `SCRIPT KILL`, because these seem more like sysadmin tasks that should not be in a client library. But they can easily be added if we want full coverage of the scripting commands.
Motivation:
The SortedSet and List range commands (LTRIM, LRANGE, ZRANGE, etc.) are stringly-based and not flexible with Swift syntax.
Modifications:
- Add overloads of LTRIM that support the gambit of Range Standard Library types
- Rework LRANGE to mirror LTRIM method signatures
- Rework ZScore Range based commands to be more type-safe with `RedisZScoreBound` enum
- Rework ZLex Range based commands to be more type-safe with `RedisZLexBound` enum
- Rework ZCOUNT, ZLEXCOUNT, ZRANGE, ZREVRANGE, ZREMRANGEBYLEX, ZREMRANGEBYRANK, ZREMRANGEBYSCORE methods to be more type-safe and support Swift Range syntax
Result:
Working with SortedSet ranges should be much more type safe, and expressive with Swift's Range syntax.
Motivation:
While reviewing the API, the current design does not read well, and still has room for misunderstanding the actual end result of a ZADD operation.
Modifications:
- Rename `RedisSortedSetAddOption` to `RedisZaddInsertBehavior` and update cases to match desired use site syntax.
- Add `RedisZaddReturnBehavior` enum to define how `zadd` should calculate the return value.
- Update `zadd` and its overloads to support the two new enums in the form of `zadd(_:to:inserting:returning:)`
Result:
The more "Swifty" API will make it much more clear to developers at the call site what the actual behavior of the ZADD command will be.
Motivation:
The goal is to have a strong-typed API for type-safety in arbitrary values, such as trying to use
Int to represent time - as '3' could mean any unit of time, leaving many places for errors and bugs.
Modifications:
Switch all current APIs that accept a `timeout` argument to use `NIO.TimeAmount` instead of a plain `Int`.
Result:
Developers will have an easier time reasoning about their own code as to what values might mean when working with
timeouts in Redis APIs.
Motivation:
Inspired by Swift by Sundell's article on type-safe identifers, the goal of this commit is to have the compiler
assist in preventing incorrect Redis key values from being used in API calls.
See https://www.swiftbysundell.com/articles/type-safe-identifiers-in-swift/ for the inspiration.
Modifications:
- Add new `RedisKey` struct that wraps around a single `String` value that conforms to several expected protocols
(Hashable, Comparable, Codable, etc.)
- Change all command APIs to require `RedisKey` rather than plain strings
Result:
When encountering an API requiring a RedisKey, it should be much more apparant at the use site what form a value should take.
Motivation:
Logging is more dynamic in real world usage than the current static heavy API allows.
Users generally want to be capable of updating connection logger metadata to attach dynamic properties such as an HTTP request ID for log tracing.
Modifications:
- Move all logs to `RedisConnection`
- Add `id: UUID` property to `RedisConnection`
- Add `logging` property and `setLogging(to:)` method requirements to `RedisClient`
- Add chainable `logging(to:)` method extension to `RedisClient`
- Add additional `trace` log statements to `RedisConnection`
- Change when `RedisConnection.init` logging and metric calls are made
- Change some `debug` log statements to `trace in `RedisConnection`
Result:
Users should have infinitely more flexibility in how RedisConnection, and RedisClient implementations in general, behave in regards to logging.
Motivation:
For ergonomics, users sometimes want to provide arguments as a variadic list rather than an array.
Modifications:
- Add variadic overloads for almost all methods that accept lists of homogenous types
Result:
Users should have more flexibility in the way arguments are passed to command methods
Motivation:
It is wrong to always assume that a GET operation is expecting a String response type, as users may be storing other types of data.
Modifications:
- Add `get` generic method with a constraint for types of `RESPValueConvertible` to convert values to the user desired type
- Change existing `get` method to specialize the generic overload
- Fix incorrect doc block regarding the ELF failure condition
Result:
Users should now be able to specialize the return type of a "GET" command
Motivation:
After working with RedisKit with RediStackTestUtils as a dependency, it was realized how opinionated the module is in how RedisConnections can be created in test environments.
Modifications:
Require more information, with reasonable defaults for `RedisConnection.init()`. Provide subclass hooks for `RedisIntegrationTestCase` for implementors to make decisions for themselves at how to connect to Redis.
Result:
Users should have more freedom in how they connect to Redis in their units tests.
Motivation:
For users looking to contribute, and for those looking to validate the library, it was unclear what tests require an actual connection to a Redis instance in order to run.
Modifications:
Add a `RediStackIntegrationTests` that takes all tests that require a Redis instance in order to run.
Result:
Those looking to run just unit tests, or contribute new tests, can now directly point to a specific testTarget as defined in the Package manifest.
Liam Don
Nathan Harris