Remove APIs for separate init and connect of async cluster contexts (#…

…165)

Embed a `valkeyClusterContext` in a created `valkeyClusterAsyncContext`
to be able to cast between these types. This enables users to access the
async context in an event callback without using `privdata`, and we can
remove the APIs previously kept just for this scenario:

valkeyClusterAsyncContext *valkeyClusterAsyncContextInit(const valkeyClusterOptions *options);
int valkeyClusterAsyncConnect(valkeyClusterAsyncContext *acc);
int valkeyClusterAsyncSetEventCallback(valkeyClusterAsyncContext *acc,
                                                                  void(fn)(const valkeyClusterContext *cc,
                                                                  int event, void *privdata),

Also removing the testcase based on `clusterclient_reconnect_async.c`
since it was using the removed APIs.

There is now a symmetry to how standalone contexts works, i.e.
`valkeyAsyncContext` embeds the `valkeyContext`.

Signed-off-by: Björn Svensson <[email protected]>

docs/cluster.md

@@ -193,8 +193,9 @@ The callback is called with `event` set to one of the following values:
 * `VALKEYCLUSTER_EVENT_SLOTMAP_UPDATED` when the slot mapping has been updated;
 * `VALKEYCLUSTER_EVENT_READY` when the slot mapping has been fetched for the first
   time and the client is ready to accept commands, useful when initiating the
-  client with `valkeyClusterAsyncConnect` where a client is not immediately
-  ready after a successful call;
+  client using `valkeyClusterAsyncConnectWithOptions` without enabling the option
+  `VALKEY_OPT_BLOCKING_INITIAL_UPDATE` where a client is not immediately ready
+  after a successful call;
 * `VALKEYCLUSTER_EVENT_FREE_CONTEXT` when the cluster context is being freed, so
   that the user can free the event `privdata`.
 
@@ -255,26 +256,6 @@ When enabled `valkeyClusterAsyncConnectWithOptions` will initially connect to th
 Any command sent by the user thereafter will create a new non-blocking connection, unless a non-blocking connection already exists to the destination.
 The function returns a pointer to a newly created `valkeyClusterAsyncContext` struct and its `err` field should be checked to make sure the initial slot map update was successful.
 
-There is also a separate API to perform the context initiation and initial connect in separate steps.
-This is useful when there is a need to provide an event callback with the current `valkeyClusterAsyncContext`.
-The context is first initiated using `valkeyClusterAsyncContextInit` and then `valkeyClusterAsyncConnect` will initiate connection attempts.
-
-```c
-valkeyClusterOptions options = {
-   .initial_nodes = "127.0.0.1:7000";
-};
-valkeyClusterOptionsUseLibev(&options, EV_DEFAULT);
-
-// Initiate the context.
-valkeyClusterAsyncContext *acc = valkeyClusterAsyncContextInit(&options);
-
-// Set the event callback using the context as privdata.
-valkeyClusterAsyncSetEventCallback(acc, eventCallback, acc);
-
-// Start connecting to the initial nodes.
-valkeyClusterAsyncConnect(acc)
-```
-
 ### Connection options
 
 There is a variety of options you can specify using the `valkeyClusterOptions` struct when connecting to a cluster.
@@ -339,9 +320,14 @@ After this, the disconnection callback is executed with the `VALKEY_OK` status a
 
 Use [`event_callback` in `valkeyClusterOptions`](#events-per-cluster-context) to get notified when certain events occur.
 
-When the callback function requires the current `valkeyClusterAsyncContext` it can be provided in the `privdata`.
-In this case initiate the context using `valkeyClusterAsyncContextInit`, set the callback and `privdata` using `valkeyClusterAsyncSetEventCallback`,
-and initiate connection attempts using `valkeyClusterAsyncConnect` as described under the [Connecting](#connecting-1) section.
+When the callback function requires the current `valkeyClusterAsyncContext`, it can typecast the given `valkeyClusterContext` to a `valkeyClusterAsyncContext`.
+The `valkeyClusterAsyncContext` struct is an extension of the `valkeyClusterContext` struct.
+
+```c
+void eventCallback(const valkeyClusterContext *cc, int event, void *privdata) {
+   valkeyClusterAsyncContext *acc = (valkeyClusterAsyncContext *)cc;
+}
+```
 
 #### Events per connection
 

docs/migration-guide.md

@@ -36,6 +36,8 @@ The type `sds` is removed from the public API.
   initiation examples that might be helpful.
 * The default command to update the internal slot map is changed to `CLUSTER SLOTS`.
   `CLUSTER NODES` can be re-enabled through options using `VALKEY_OPT_USE_CLUSTER_NODES`.
+* A `valkeyClusterAsyncContext` now embeds a `valkeyClusterContext` instead of
+  holding a pointer to it. Replace any use of `acc->cc` with `&acc->cc` or similar.
 
 ### Renamed API functions
 
@@ -61,10 +63,13 @@ The type `sds` is removed from the public API.
 * `redisClusterSetOptionConnectNonBlock` removed since it was deprecated.
 * `redisClusterSetOptionConnectTimeout` removed, use `valkeyClusterOptions.connect_timeout`.
 * `redisClusterSetOptionMaxRetry` removed, use `valkeyClusterOptions.max_retry`.
-* `redisClusterSetOptionParseSlaves` removed, use `valkeyClusterOptions.flags` and `VALKEY_OPT_USE_REPLICAS`.
+* `redisClusterSetOptionParseSlaves` removed, use `valkeyClusterOptions.options` and `VALKEY_OPT_USE_REPLICAS`.
 * `redisClusterSetOptionPassword` removed, use `valkeyClusterOptions.password`.
-* `redisClusterSetOptionRouteUseSlots` removed, the use of `CLUSTER SLOTS` is enabled by default.
+* `redisClusterSetOptionRouteUseSlots` removed, `CLUSTER SLOTS` is used by default.
 * `redisClusterSetOptionUsername` removed, use `valkeyClusterOptions.username`.
+* `redisClusterAsyncConnect` removed, use `valkeyClusterAsyncConnectWithOptions` with options flag `VALKEY_OPT_BLOCKING_INITIAL_UPDATE`.
+* `redisClusterAsyncConnect2` removed, use `valkeyClusterAsyncConnectWithOptions`.
+* `redisClusterAsyncContextInit` removed, `valkeyClusterAsyncConnectWithOptions` will initiate the context.
 * `redisClusterAsyncSetConnectCallback` removed, but `valkeyClusterOptions.async_connect_callback` can be used which accepts a non-const callback function prototype.
 * `redisClusterAsyncSetConnectCallbackNC` removed, use `valkeyClusterOptions.async_connect_callback`.
 * `redisClusterAsyncSetDisconnectCallback` removed, use `valkeyClusterOptions.async_disconnect_callback`.

examples/cluster-clientside-caching-async.c

@@ -36,7 +36,10 @@ void connectCallback(valkeyAsyncContext *ac, int status) {
    commands. A reply is expected via a call to 'setCallback()' */
 void eventCallback(const valkeyClusterContext *cc, int event, void *privdata) {
     (void)cc;
-    valkeyClusterAsyncContext *acc = (valkeyClusterAsyncContext *)privdata;
+    (void)privdata;
+    /* Get the async context by a simple cast since in the Async API a
+     * valkeyClusterAsyncContext is an extension of the valkeyClusterContext. */
+    valkeyClusterAsyncContext *acc = (valkeyClusterAsyncContext *)cc;
 
     /* We send our commands when the client is ready to accept commands. */
     if (event == VALKEYCLUSTER_EVENT_READY) {
@@ -145,17 +148,14 @@ int main(int argc, char **argv) {
     options.initial_nodes = CLUSTER_NODE;
     options.async_connect_callback = connectCallback;
     options.async_disconnect_callback = disconnectCallback;
+    options.event_callback = eventCallback;
     valkeyClusterOptionsUseLibevent(&options, base);
 
-    valkeyClusterAsyncContext *acc = valkeyClusterAsyncContextInit(&options);
-    assert(acc);
-
-    int status;
-    status = valkeyClusterAsyncSetEventCallback(acc, eventCallback, acc);
-    assert(status == VALKEY_OK);
-
-    status = valkeyClusterAsyncConnect(acc);
-    assert(status == VALKEY_OK);
+    valkeyClusterAsyncContext *acc = valkeyClusterAsyncConnectWithOptions(&options);
+    if (acc == NULL || acc->err != 0) {
+        printf("Connect error: %s\n", acc ? acc->errstr : "OOM");
+        exit(2);
+    }
 
     event_base_dispatch(base);
 

include/valkey/cluster.h

@@ -115,10 +115,13 @@ typedef struct valkeyClusterContext {
 
 /* Context for accessing a Valkey Cluster asynchronously */
 typedef struct valkeyClusterAsyncContext {
-    valkeyClusterContext *cc;
+    /* Hold the regular context. */
+    valkeyClusterContext cc;
 
-    int err;          /* Error flags, 0 when there is no error */
-    char errstr[128]; /* String representation of error when applicable */
+    int err;      /* Error flag, 0 when there is no error,
+                   * a copy of cc->err for convenience. */
+    char *errstr; /* String representation of error when applicable,
+                   * always pointing to cc->errstr[] */
 
     int64_t lastSlotmapUpdateAttempt; /* Timestamp */
 
@@ -289,17 +292,6 @@ valkeyClusterAsyncContext *valkeyClusterAsyncConnectWithOptions(const valkeyClus
 void valkeyClusterAsyncDisconnect(valkeyClusterAsyncContext *acc);
 void valkeyClusterAsyncFree(valkeyClusterAsyncContext *acc);
 
-/* Initiate and connect as separate steps. */
-valkeyClusterAsyncContext *valkeyClusterAsyncContextInit(const valkeyClusterOptions *options);
-int valkeyClusterAsyncConnect(valkeyClusterAsyncContext *acc);
-
-/* Callback option configurable after a context initiation, enabling that the
- * valkeyClusterAsyncContext pointer can be given as privdata in the callback. */
-int valkeyClusterAsyncSetEventCallback(valkeyClusterAsyncContext *acc,
-                                       void(fn)(const valkeyClusterContext *cc,
-                                                int event, void *privdata),
-                                       void *privdata);
-
 /* Commands */
 int valkeyClusterAsyncCommand(valkeyClusterAsyncContext *acc,
                               valkeyClusterCallbackFn *fn, void *privdata,