Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify API by only using non-const connect callbacks #142

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions docs/cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
```
Expand Down
2 changes: 2 additions & 0 deletions docs/migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ The type `sds` is removed from the public API.

* `redisFormatSdsCommandArgv` removed from API. Can be replaced with `valkeyFormatCommandArgv`.
* `redisFreeSdsCommand` removed since the `sds` type is for internal use only.
* `redisAsyncSetConnectCallbackNC` removed and `valkeyAsyncSetConnectCallback` accepts the non-const callback function prototype.
zuiderkwast marked this conversation as resolved.
Show resolved Hide resolved

## Migrating from `hiredis-cluster` 0.14.0

Expand All @@ -42,6 +43,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.
* `redisClusterAsyncSetConnectCallbackNC` removed and `valkeyClusterAsyncSetConnectCallback` accepts the non-const callback function prototype.

### Removed support for splitting multi-key commands per slot

Expand Down
2 changes: 1 addition & 1 deletion examples/async-glib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion examples/async-libev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/async-libevent-tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/async-libevent.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/async-libhv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/async-libsdevent.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/async-libuv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/cluster-async-tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion examples/cluster-async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions examples/cluster-clientside-caching-async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions include/valkey/async.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -101,7 +100,6 @@ typedef struct valkeyAsyncContext {

/* Called when the first write event was received. */
valkeyConnectCallback *onConnect;
valkeyConnectCallbackNC *onConnectNC;

/* Regular command callbacks */
valkeyCallbackList replies;
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions include/valkey/cluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ typedef struct valkeyClusterAsyncContext {

/* Called when the first write event was received. */
valkeyConnectCallback *onConnect;
valkeyConnectCallbackNC *onConnectNC;

} valkeyClusterAsyncContext;

Expand Down Expand Up @@ -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);

Expand Down
37 changes: 7 additions & 30 deletions src/async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
13 changes: 0 additions & 13 deletions src/cluster.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion tests/client_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/clusterclient_async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ";
Expand Down
12 changes: 1 addition & 11 deletions tests/ct_async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/ct_async_glib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ct_async_libev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ct_async_libuv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading
Loading