GitLab

Motivation:

To make it a little more generic, and to avoid turnover during renames (such as the planned rebranding in issue #61), `RedisClientError` more accurately reflects the source of the errors, as well as the responsibility of causing the bug.

Modifications:

- Rename `RedisNIOError` to `RedisClientError`
- Rename `RedisError` file to `RedisErrors`
- Add documentation of `RedisClientError`
- Remove no longer used `.unsupportedOperation(method:message:)` value
- Rename `.responseConversion(to:)` to `.failedRESPConversion(to:)`

Result:

Names of `RedisClientError` should be more descriptive, less prone to turnover, and more documented for users to understand the issues related to these thrown errors.

47 -- SSWG Review Feedback

Closes #55, #57, #54, #49, and #47

See merge request Mordil/swift-redis-nio-client!53

Refactor `RedisConnection`

See merge request Mordil/swift-redis-nio-client!71

Motivation:

During proposal review, and while working within the codebase, several issues were identified with how `RedisConnection` was architectured.

Modifications:

- Change implementation of `RedisConnection` in some areas for new logic of internal `ConnectionState`
- Change behavior of logging in a few places
- The initializer for `RedisConnection` is now **internal**
- How users can override the default `ClientBootstrap` for a connection is by passing an instance to the `.connect` static method
- Change unit tests to inherit from a common XCTestCase class that handles creation and cleanup of `RedisConnection` in tests
- Remove Redis namespace enum

Result:

The API for `RedisConnection` should be much simpler, with the implementation being less buggy.

This resolves issues #49, #54, and #57.

60 -- Provide Strong Option Types in SortedSet Commands

See merge request Mordil/swift-redis-nio-client!70

Motivation:

While working through issue #59, it was noticed just how "stringly" the SortedSet command options for `zadd`, `zinterstore`, and `zunionstore` were, and Swift provides ways of having strong type safety for these options.

Modifications:

- Add `RedisSortedSetAddOption` and `RedisSortedSetAggregateMethod` to replace the String API in `zadd`, `zinterstore`, and `zunionstore`
- Fix an implication of how `overestimatedCountBeingAdded` documentation for `Array where Element == RESPValue` for `add(contentsOf:overestimatedCountBeingAdded:_:)`

Result:

Users should have a more discoverable and straightforward way that isn't error prone for calling `zadd`, `zinterstore`, and `zunionstore` with Redis supported options.

59 -- Use `RESPValueConvertible` as Generic Constraint

See merge request Mordil/swift-redis-nio-client!69

Motivation:

Johannes continues to provide great insight, and correctly pointed out that `RESPValueConvertible` was being used as an "existential" in all cases.

This can cause unexpected type-erasure and introduce unnecessary cost overhead with dynamic dispatch when in most cases we know the exact value we want for `RESPValue` to execute commands.

Modifications:

- Add new extensions to `Array where Element == RESPValue` for appending and adding elements into them
- Change `RedisClient.send(command:with:)` to require `[RESPValue]` instead of `[RESPValueConvertible]` as the `with` argument type
- Change all instances of `RESPValueConvertible` being an "existential" type for method arguments to instead be a generic constraint

Result:

The library should be safeguarded from a class of bugs, with the use of `send` being a bit more straight forward, with some new convenience methods for `[RESPValue]` types.

Revisit `RESPValue` and `RESPValueConvertible` implementations.

See merge request Mordil/swift-redis-nio-client!67

Motivation:

Johannes provided a fair code review of the project and summarized his findings in issue #48, and one of the prime offenders was all of the `unsafe*` APIs (pointers, buffers, bytes)
that were used with `RESPValue` and `RESPValueConvertible`.

He also provided great feedback and pointed out good points of confusion with the API design of `RESPValue` and `RESPValueConvertible`.

Modifications:

- Return to using `Array` instead of `ContiguousArray` for `RESPValue.array` storage
- Update all documentation to be more thorough in explaining how the types should be used and conformed to.
- Remove all uses of `unsafe*` APIs where possible
- Change implementations to be a lot more type and memory safe, double checking assumptions
- Remove conformance to `ExpressibleBy*Literal` as it is too easy for users to shoot themselves in the foot and saves only a few characters over `.init(bulk:)`
- Create new `RedisNIOTestUtils` target for common test extensions, making them public
- Move most almost all implementations of `RESPValue` computed properties into the `RESPValueConvertible` conformances

Result:

Users should be more safeguarded by the API against unknowingly getting incorrect `RESPValue` representations, the API design of `RESPValue` and `RESPValueConvertible` should be much clearer,
and memory safety should be at a higher bar from these changes.

This resolves issues #55 & #48, and contributes to issue #47.

Rename `RedisCommandContext` to `RedisCommand`, improve `RedisCommandHandler`...

See merge request Mordil/swift-redis-nio-client!66

Update `RedisClient.expire` to no longer use `deadline` terminology.

See merge request Mordil/swift-redis-nio-client!65

Rename `RedisCommandContext` to `RedisCommand`, improve `RedisCommandHandler` usage semantics, and cleanup documentation.

Motivation:

During proposal review, it was noted that `RedisCommandContext` was a bit misleading, and the hidden reference semantics worrisome.

In addition, several of parts of the documentation around `RedisCommandHandler` were weak or also misleading.

Modifications:

- Rename `RedisCommandContext` to just `RedisCommand`
- Update documentation to be more explicit about the module who owns the types being referenced
- Update documentation to call out explicit usage semantics and behavior
- Change `RedisCommandHandler` to close the socket connection on error thrown
- Rename the "base" Redis Channel Handlers to be more explicitly named

Result:

Users should have clearer documentation on what happens when using `RedisCommandHandler` and `RedisCommand` without hidden semantics.

This contributes to issue #47.

Motivation:

During proposal review, it was appropriately pointed out that `RedisClient.expire` incorrectly mixes 'deadline' and 'timeout' terminology.

Modifications:

- Change references of 'deadline' to 'timeout' to follow Redis' established semantics for 'EXPIRE'
- Add additional unit test for `RedisClient.expire`

Result:

`RedisClient.expire` should now be more clear as to its semantics and not mix terminology incorrectly.

This contributes to issue #47

Change `Redis.makeDefaultClientBootstrap` to `ClientBootstrap.makeRedisTCPClient`

See merge request Mordil/swift-redis-nio-client!64

Motivation:

During proposal review, feedback was provided that the discoverability of the factory method for building a standard RESP `ChannelPipeline` was poor outside of documentation.

Modifications:

- Move `Redis.makeDefaultClientBootstrap` to `ClientBootstrap.makeRedisTCPClient`.
- Move the `channelInitializer` implementation into a new `Channel.addBaseRedisHandlers()` instance method.

Result:

Users should have an easier time discovering how to easily create baseline RESP `ChannelPipelines`.

This contributes to #47.

Refactor `RESPTranslator` to mutate the passed `ByteBuffer` directly.

See merge request Mordil/swift-redis-nio-client!63

Add `fromRESP` label to `RESPValueConvertible.init`

See merge request Mordil/swift-redis-nio-client!56

Motivation:

During proposal review, it was pointed out that the code with a position index was redundant and error prone over relying on `ByteBuffer`'s `readerIndex`.

Modifications:

Refactored `RESPTranslator` to rely on `ByteBuffer.readerIndex` for position of current parsing cursor,
and eliminated `ParsingResult` enum to instead return `RESPValue?`.

The implementation for writing out `RESPValue` has been expanded to `RESPValueConvertible` and moved to an extension of `ByteBuffer`.

Result:

Parsing `RESPValue` into and out of `ByteBuffer` should be less error-prone and more straight forward.

This contributes to issues #47 and #55

Make `RedisCommandHandler` final

See merge request Mordil/swift-redis-nio-client!55

Motivation:

During SSWG review, feedback was provided on the API design of the `RESPValueConvertible.init` signature and how it should appropriately follow Swift Design Guidelines regarding labels.

Modifications:

`RESPValueConvertible.init(_:` is now `RESPValueConvertible.init(fromRESP:)`.

Result:

There should be more clarity at the call site when initializing a type from a `RESPValue`.

This contributes to #47

Motivation:

During SSWG review, feedback was provided that forced a re-evaluation of early design feedback interpretation on composability of RedisNIO.

First understanding was that the desire was to have "customization points" to gain benefits of implementation of RedisCommandHandler, while new understanding is that it just needs to be public in order for users to include it in their own custom ChannelPipeline schemes.

Modifications:

`RedisCommandHandler` is now a final class.

Result:

Users will no longer be able to subclass `RedisCommandHandler`, but gain a super slight performance increase.

This contributes to #47.

58 -- Add Swift 5.1 and Nightly Build Testing

Closes #58

See merge request Mordil/swift-redis-nio-client!68

Swap `append(_🇹🇴)` parameters in signature

See merge request Mordil/swift-redis-nio-client!62

Adding append

See merge request Mordil/swift-redis-nio-client!61

50 -- Rename `EventLoopFuture.mapFromRESP` to `convertFromRESPValue`

Closes #50

See merge request Mordil/swift-redis-nio-client!60

53 -- Use `CircularBuffer` for `RedisCommandHandler` queue

Closes #53

See merge request Mordil/swift-redis-nio-client!59

51 -- Use `UInt8(ascii:)` for RESP Tokens

Closes #51

See merge request Mordil/swift-redis-nio-client!58

52 -- Remove `@_exported` Usage

Closes #52

See merge request Mordil/swift-redis-nio-client!57

Add static property for default RedisConnection port

See merge request Mordil/swift-redis-nio-client!51

Motivation:

The default port for Redis is well published to be 6379, and it is common to want to pass this value around or use it as a static default value in methods and initializers.

Modifications:

Add `RedisConnection.defaultPort` static property for all users to use, and update references of the `6379` literals to use new  the new property.

Result:

Users should have a reliable default defined to use everywhere to avoid bugs.

Fix `Data` Conversion to `RESPValue`

See merge request Mordil/swift-redis-nio-client!50