Simplify API by only using non-const connect callbacks (valkey-io#142)

Remove `valkeyAsyncSetConnectCallbackNC` since it was first added in hiredis for backwards-compatibility reasons, and let `valkeyAsyncSetConnectCallback` use the non-const connect callback prototype. Remove the the cluster version `valkeyClusterAsyncSetConnectCallbackNC` too. Signed-off-by: Björn Svensson <[email protected]>
bjosv · Dec 30, 2024 · ca3dac9 · ca3dac9
1 parent 57774e1
commit ca3dac9
Show file tree

Hide file tree

Showing 26 changed files with 79 additions and 121 deletions.
diff --git a/docs/cluster.md b/docs/cluster.md
@@ -298,19 +298,7 @@ status = valkeyClusterAsyncSetConnectCallback(acc, callbackFn);
 status = valkeyClusterAsyncSetDisconnectCallback(acc, callbackFn);
 ```
 
-The callback functions should have the following prototype, aliased to `valkeyConnectCallback`:
-
-```c
-void(const valkeyAsyncContext *ac, int status);
-```
-
-Alternatively, you set a connect callback that will be passed a non-const `valkeyAsyncContext*` on invocation (e.g. to be able to set a push callback on it).
-
-```c
-status = valkeyClusterAsyncSetConnectCallbackNC(acc, nonConstCallbackFn);
-```
-
-The callback function should have the following prototype, aliased to `valkeyConnectCallbackNC`:
+The connect callback function should have the following prototype, aliased to `valkeyConnectCallback`:
 ```c
 void(valkeyAsyncContext *ac, int status);
 ```

diff --git a/docs/migration-guide.md b/docs/migration-guide.md
@@ -15,17 +15,23 @@ The general actions needed are:
 
 The type `sds` is removed from the public API.
 
+### Renamed API functions
+
+* `redisAsyncSetConnectCallbackNC` is renamed to `valkeyAsyncSetConnectCallback`.
+
 ### Removed API functions
 
 * `redisFormatSdsCommandArgv` removed from API. Can be replaced with `valkeyFormatCommandArgv`.
 * `redisFreeSdsCommand` removed since the `sds` type is for internal use only.
+* `redisAsyncSetConnectCallback` is removed, but can be replaced with `valkeyAsyncSetConnectCallback` which accepts the non-const callback function prototype.
 
 ## Migrating from `hiredis-cluster` 0.14.0
 
 ### Renamed API functions
 
 * `ctx_get_by_node` is renamed to `valkeyClusterGetValkeyContext`.
 * `actx_get_by_node` is renamed to `valkeyClusterGetValkeyAsyncContext`.
+* `redisClusterAsyncSetConnectCallbackNC` is renamed to `valkeyClusterAsyncSetConnectCallback`.
 
 ### Renamed API defines
 
@@ -42,6 +48,7 @@ The type `sds` is removed from the public API.
 * `redisClusterSetOptionConnectNonBlock` removed since it was deprecated.
 * `parse_cluster_nodes` removed from API, for internal use only.
 * `parse_cluster_slots` removed from API, for internal use only.
+* `redisClusterAsyncSetConnectCallback` is removed, but can be replaced with `valkeyClusterAsyncSetConnectCallback` which accepts the non-const callback function prototype.
 
 ### Removed support for splitting multi-key commands per slot
 

diff --git a/examples/async-glib.c b/examples/async-glib.c
@@ -8,7 +8,7 @@
 static GMainLoop *mainloop;
 
 static void
-connect_cb(const valkeyAsyncContext *ac G_GNUC_UNUSED,
+connect_cb(valkeyAsyncContext *ac G_GNUC_UNUSED,
            int status) {
     if (status != VALKEY_OK) {
         g_printerr("Failed to connect: %s\n", ac->errstr);

diff --git a/examples/async-libev.c b/examples/async-libev.c
@@ -18,7 +18,7 @@ void getCallback(valkeyAsyncContext *c, void *r, void *privdata) {
     valkeyAsyncDisconnect(c);
 }
 
-void connectCallback(const valkeyAsyncContext *c, int status) {
+void connectCallback(valkeyAsyncContext *c, int status) {
     if (status != VALKEY_OK) {
         printf("Error: %s\n", c->errstr);
         return;

diff --git a/examples/async-libevent-tls.c b/examples/async-libevent-tls.c
@@ -19,7 +19,7 @@ void getCallback(valkeyAsyncContext *c, void *r, void *privdata) {
     valkeyAsyncDisconnect(c);
 }
 
-void connectCallback(const valkeyAsyncContext *c, int status) {
+void connectCallback(valkeyAsyncContext *c, int status) {
     if (status != VALKEY_OK) {
         printf("Error: %s\n", c->errstr);
         return;

diff --git a/examples/async-libevent.c b/examples/async-libevent.c
@@ -22,7 +22,7 @@ void getCallback(valkeyAsyncContext *c, void *r, void *privdata) {
     valkeyAsyncDisconnect(c);
 }
 
-void connectCallback(const valkeyAsyncContext *c, int status) {
+void connectCallback(valkeyAsyncContext *c, int status) {
     if (status != VALKEY_OK) {
         printf("Error: %s\n", c->errstr);
         return;

diff --git a/examples/async-libhv.c b/examples/async-libhv.c
@@ -30,7 +30,7 @@ void debugCallback(valkeyAsyncContext *c, void *r, void *privdata) {
     valkeyAsyncDisconnect(c);
 }
 
-void connectCallback(const valkeyAsyncContext *c, int status) {
+void connectCallback(valkeyAsyncContext *c, int status) {
     if (status != VALKEY_OK) {
         printf("Error: %s\n", c->errstr);
         return;

diff --git a/examples/async-libsdevent.c b/examples/async-libsdevent.c
@@ -33,7 +33,7 @@ void getCallback(valkeyAsyncContext *c, void *r, void *privdata) {
     valkeyAsyncCommand(c, debugCallback, NULL, "DEBUG SLEEP %f", 1.5);
 }
 
-void connectCallback(const valkeyAsyncContext *c, int status) {
+void connectCallback(valkeyAsyncContext *c, int status) {
     if (status != VALKEY_OK) {
         printf("connect error: %s\n", c->errstr);
         return;

diff --git a/examples/async-libuv.c b/examples/async-libuv.c
@@ -33,7 +33,7 @@ void getCallback(valkeyAsyncContext *c, void *r, void *privdata) {
     valkeyAsyncCommand(c, debugCallback, NULL, "DEBUG SLEEP %f", 1.5);
 }
 
-void connectCallback(const valkeyAsyncContext *c, int status) {
+void connectCallback(valkeyAsyncContext *c, int status) {
     if (status != VALKEY_OK) {
         printf("connect error: %s\n", c->errstr);
         return;

diff --git a/examples/cluster-async-tls.c b/examples/cluster-async-tls.c
@@ -34,7 +34,7 @@ void setCallback(valkeyClusterAsyncContext *cc, void *r, void *privdata) {
     printf("privdata: %s reply: %s\n", (char *)privdata, reply->str);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     if (status != VALKEY_OK) {
         printf("Error: %s\n", ac->errstr);
         return;

diff --git a/examples/cluster-async.c b/examples/cluster-async.c
@@ -30,7 +30,7 @@ void setCallback(valkeyClusterAsyncContext *cc, void *r, void *privdata) {
     printf("privdata: %s reply: %s\n", (char *)privdata, reply->str);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     if (status != VALKEY_OK) {
         printf("Error: %s\n", ac->errstr);
         return;

diff --git a/examples/cluster-clientside-caching-async.c b/examples/cluster-clientside-caching-async.c
@@ -22,10 +22,9 @@ void getCallback1(valkeyClusterAsyncContext *acc, void *r, void *privdata);
 void getCallback2(valkeyClusterAsyncContext *acc, void *r, void *privdata);
 void modifyKey(const char *key, const char *value);
 
-/* The connect callback enables RESP3 and client tracking.
-   The non-const connect callback is used since we want to
-   set the push callback in the libvalkey context. */
-void connectCallbackNC(valkeyAsyncContext *ac, int status) {
+/* The connect callback enables RESP3 and client tracking,
+ * and sets the push callback in the libvalkey context. */
+void connectCallback(valkeyAsyncContext *ac, int status) {
     assert(status == VALKEY_OK);
     valkeyAsyncSetPushCallback(ac, pushCallback);
     valkeyAsyncCommand(ac, NULL, NULL, "HELLO 3");
@@ -147,7 +146,7 @@ int main(int argc, char **argv) {
     assert(acc);
 
     int status;
-    status = valkeyClusterAsyncSetConnectCallbackNC(acc, connectCallbackNC);
+    status = valkeyClusterAsyncSetConnectCallback(acc, connectCallback);
     assert(status == VALKEY_OK);
     status = valkeyClusterAsyncSetDisconnectCallback(acc, disconnectCallback);
     assert(status == VALKEY_OK);

diff --git a/include/valkey/async.h b/include/valkey/async.h
@@ -64,8 +64,7 @@ typedef struct valkeyCallbackList {
 
 /* Connection callback prototypes */
 typedef void(valkeyDisconnectCallback)(const struct valkeyAsyncContext *, int status);
-typedef void(valkeyConnectCallback)(const struct valkeyAsyncContext *, int status);
-typedef void(valkeyConnectCallbackNC)(struct valkeyAsyncContext *, int status);
+typedef void(valkeyConnectCallback)(struct valkeyAsyncContext *, int status);
 typedef void(valkeyTimerCallback)(void *timer, void *privdata);
 
 /* Context for an async connection to Valkey */
@@ -101,7 +100,6 @@ typedef struct valkeyAsyncContext {
 
     /* Called when the first write event was received. */
     valkeyConnectCallback *onConnect;
-    valkeyConnectCallbackNC *onConnectNC;
 
     /* Regular command callbacks */
     valkeyCallbackList replies;
@@ -130,7 +128,6 @@ valkeyAsyncContext *valkeyAsyncConnectBindWithReuse(const char *ip, int port,
                                                     const char *source_addr);
 valkeyAsyncContext *valkeyAsyncConnectUnix(const char *path);
 int valkeyAsyncSetConnectCallback(valkeyAsyncContext *ac, valkeyConnectCallback *fn);
-int valkeyAsyncSetConnectCallbackNC(valkeyAsyncContext *ac, valkeyConnectCallbackNC *fn);
 int valkeyAsyncSetDisconnectCallback(valkeyAsyncContext *ac, valkeyDisconnectCallback *fn);
 
 valkeyAsyncPushFn *valkeyAsyncSetPushCallback(valkeyAsyncContext *ac, valkeyAsyncPushFn *fn);

diff --git a/include/valkey/cluster.h b/include/valkey/cluster.h
@@ -144,7 +144,6 @@ typedef struct valkeyClusterAsyncContext {
 
     /* Called when the first write event was received. */
     valkeyConnectCallback *onConnect;
-    valkeyConnectCallbackNC *onConnectNC;
 
 } valkeyClusterAsyncContext;
 
@@ -274,8 +273,6 @@ void valkeyClusterAsyncFree(valkeyClusterAsyncContext *acc);
 
 int valkeyClusterAsyncSetConnectCallback(valkeyClusterAsyncContext *acc,
                                          valkeyConnectCallback *fn);
-int valkeyClusterAsyncSetConnectCallbackNC(valkeyClusterAsyncContext *acc,
-                                           valkeyConnectCallbackNC *fn);
 int valkeyClusterAsyncSetDisconnectCallback(valkeyClusterAsyncContext *acc,
                                             valkeyDisconnectCallback *fn);
 

diff --git a/src/async.c b/src/async.c
@@ -143,7 +143,6 @@ static valkeyAsyncContext *valkeyAsyncInitialize(valkeyContext *c) {
     ac->ev.scheduleTimer = NULL;
 
     ac->onConnect = NULL;
-    ac->onConnectNC = NULL;
     ac->onDisconnect = NULL;
 
     ac->replies.head = NULL;
@@ -230,18 +229,12 @@ valkeyAsyncContext *valkeyAsyncConnectUnix(const char *path) {
     return valkeyAsyncConnectWithOptions(&options);
 }
 
-static int
-valkeyAsyncSetConnectCallbackImpl(valkeyAsyncContext *ac, valkeyConnectCallback *fn,
-                                  valkeyConnectCallbackNC *fn_nc) {
-    /* If either are already set, this is an error */
-    if (ac->onConnect || ac->onConnectNC)
+int valkeyAsyncSetConnectCallback(valkeyAsyncContext *ac, valkeyConnectCallback *fn) {
+    /* If already set, this is an error */
+    if (ac->onConnect)
         return VALKEY_ERR;
 
-    if (fn) {
-        ac->onConnect = fn;
-    } else if (fn_nc) {
-        ac->onConnectNC = fn_nc;
-    }
+    ac->onConnect = fn;
 
     /* The common way to detect an established connection is to wait for
      * the first write event to be fired. This assumes the related event
@@ -251,14 +244,6 @@ valkeyAsyncSetConnectCallbackImpl(valkeyAsyncContext *ac, valkeyConnectCallback
     return VALKEY_OK;
 }
 
-int valkeyAsyncSetConnectCallback(valkeyAsyncContext *ac, valkeyConnectCallback *fn) {
-    return valkeyAsyncSetConnectCallbackImpl(ac, fn, NULL);
-}
-
-int valkeyAsyncSetConnectCallbackNC(valkeyAsyncContext *ac, valkeyConnectCallbackNC *fn) {
-    return valkeyAsyncSetConnectCallbackImpl(ac, NULL, fn);
-}
-
 int valkeyAsyncSetDisconnectCallback(valkeyAsyncContext *ac, valkeyDisconnectCallback *fn) {
     if (ac->onDisconnect == NULL) {
         ac->onDisconnect = fn;
@@ -324,24 +309,16 @@ static void valkeyRunPushCallback(valkeyAsyncContext *ac, valkeyReply *reply) {
 }
 
 static void valkeyRunConnectCallback(valkeyAsyncContext *ac, int status) {
-    if (ac->onConnect == NULL && ac->onConnectNC == NULL)
+    if (ac->onConnect == NULL)
         return;
 
     if (!(ac->c.flags & VALKEY_IN_CALLBACK)) {
         ac->c.flags |= VALKEY_IN_CALLBACK;
-        if (ac->onConnect) {
-            ac->onConnect(ac, status);
-        } else {
-            ac->onConnectNC(ac, status);
-        }
+        ac->onConnect(ac, status);
         ac->c.flags &= ~VALKEY_IN_CALLBACK;
     } else {
         /* already in callback */
-        if (ac->onConnect) {
-            ac->onConnect(ac, status);
-        } else {
-            ac->onConnectNC(ac, status);
-        }
+        ac->onConnect(ac, status);
     }
 }
 

diff --git a/src/cluster.c b/src/cluster.c
@@ -2784,8 +2784,6 @@ valkeyClusterGetValkeyAsyncContext(valkeyClusterAsyncContext *acc,
 
     if (acc->onConnect) {
         valkeyAsyncSetConnectCallback(ac, acc->onConnect);
-    } else if (acc->onConnectNC) {
-        valkeyAsyncSetConnectCallbackNC(ac, acc->onConnectNC);
     }
 
     if (acc->onDisconnect) {
@@ -2849,21 +2847,10 @@ int valkeyClusterAsyncSetConnectCallback(valkeyClusterAsyncContext *acc,
                                          valkeyConnectCallback *fn) {
     if (acc->onConnect != NULL)
         return VALKEY_ERR;
-    if (acc->onConnectNC != NULL)
-        return VALKEY_ERR;
     acc->onConnect = fn;
     return VALKEY_OK;
 }
 
-int valkeyClusterAsyncSetConnectCallbackNC(valkeyClusterAsyncContext *acc,
-                                           valkeyConnectCallbackNC *fn) {
-    if (acc->onConnectNC != NULL || acc->onConnect != NULL) {
-        return VALKEY_ERR;
-    }
-    acc->onConnectNC = fn;
-    return VALKEY_OK;
-}
-
 int valkeyClusterAsyncSetDisconnectCallback(valkeyClusterAsyncContext *acc,
                                             valkeyDisconnectCallback *fn) {
     if (acc->onDisconnect == NULL) {

diff --git a/tests/client_test.c b/tests/client_test.c
@@ -2173,7 +2173,7 @@ static valkeyAsyncContext *do_aconnect(struct config config, astest_no testno) {
     c->data = &astest;
     c->dataCleanup = asCleanup;
     valkeyPollAttach(c);
-    valkeyAsyncSetConnectCallbackNC(c, connectCallback);
+    valkeyAsyncSetConnectCallback(c, connectCallback);
     valkeyAsyncSetDisconnectCallback(c, disconnectCallback);
     return c;
 }

diff --git a/tests/clusterclient_async.c b/tests/clusterclient_async.c
@@ -207,7 +207,7 @@ void eventCallback(const valkeyClusterContext *cc, int event, void *privdata) {
     printf("Event: %s\n", e);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     const char *s = "";
     if (status != VALKEY_OK)
         s = "failed to ";

diff --git a/tests/ct_async.c b/tests/ct_async.c
@@ -23,19 +23,11 @@ void setCallback(valkeyClusterAsyncContext *acc, void *r, void *privdata) {
     ASSERT_MSG(reply != NULL, acc->errstr);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     ASSERT_MSG(status == VALKEY_OK, ac->errstr);
     printf("Connected to %s:%d\n", ac->c.tcp.host, ac->c.tcp.port);
 }
 
-void connectCallbackNC(valkeyAsyncContext *ac, int status) {
-    UNUSED(ac);
-    UNUSED(status);
-    /* The testcase expects a failure during registration of this
-       non-const connect callback and it should never be called. */
-    assert(0);
-}
-
 void disconnectCallback(const valkeyAsyncContext *ac, int status) {
     ASSERT_MSG(status == VALKEY_OK, ac->errstr);
     printf("Disconnected from %s:%d\n", ac->c.tcp.host, ac->c.tcp.port);
@@ -77,8 +69,6 @@ int main(void) {
     assert(status == VALKEY_OK);
     status = valkeyClusterAsyncSetConnectCallback(acc, connectCallback);
     assert(status == VALKEY_ERR); /* Re-registration not accepted */
-    status = valkeyClusterAsyncSetConnectCallbackNC(acc, connectCallbackNC);
-    assert(status == VALKEY_ERR); /* Re-registration not accepted */
 
     status = valkeyClusterAsyncSetDisconnectCallback(acc, disconnectCallback);
     assert(status == VALKEY_OK);

diff --git a/tests/ct_async_glib.c b/tests/ct_async_glib.c
@@ -24,7 +24,7 @@ void getCallback(valkeyClusterAsyncContext *acc, void *r, void *privdata) {
     g_main_loop_quit(mainloop);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     ASSERT_MSG(status == VALKEY_OK, ac->errstr);
     printf("Connected to %s:%d\n", ac->c.tcp.host, ac->c.tcp.port);
 }

diff --git a/tests/ct_async_libev.c b/tests/ct_async_libev.c
@@ -21,7 +21,7 @@ void getCallback(valkeyClusterAsyncContext *acc, void *r, void *privdata) {
     valkeyClusterAsyncDisconnect(acc);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     ASSERT_MSG(status == VALKEY_OK, ac->errstr);
     printf("Connected to %s:%d\n", ac->c.tcp.host, ac->c.tcp.port);
 }

diff --git a/tests/ct_async_libuv.c b/tests/ct_async_libuv.c
@@ -22,7 +22,7 @@ void getCallback(valkeyClusterAsyncContext *acc, void *r, void *privdata) {
     valkeyClusterAsyncDisconnect(acc);
 }
 
-void connectCallback(const valkeyAsyncContext *ac, int status) {
+void connectCallback(valkeyAsyncContext *ac, int status) {
     ASSERT_MSG(status == VALKEY_OK, ac->errstr);
     printf("Connected to %s:%d\n", ac->c.tcp.host, ac->c.tcp.port);
 }