Add config delete

Author: aws-kevinrickard

configtest/bin/configtest.c

@@ -291,6 +291,50 @@ static void test_get(
     }
 }
 
+static void test_delete(GglList key_path, GglError expected_result) {
+    GGL_LOGD(
+        "test_delete %s, expecting %s",
+        print_key_path(&key_path),
+        ggl_strerror(expected_result)
+    );
+    GglBuffer server = GGL_STR("gg_config");
+
+    GglMap params = GGL_MAP({ GGL_STR("key_path"), GGL_OBJ_LIST(key_path) }, );
+
+    GglError remote_error = GGL_ERR_OK;
+    GglError error = ggl_call(
+        server, GGL_STR("delete"), params, &remote_error, NULL, NULL
+    );
+    if (expected_result != GGL_ERR_OK && error != GGL_ERR_REMOTE) {
+        GGL_LOGE(
+            "delete key %s expected result %s but there was not a remote error",
+            print_key_path(&key_path),
+            ggl_strerror(expected_result)
+        );
+        assert(0);
+    }
+    if (expected_result == GGL_ERR_OK && error != GGL_ERR_OK) {
+        GGL_LOGE(
+            "delete key %s did not expect error but got error %s and remote "
+            "error %s",
+            print_key_path(&key_path),
+            ggl_strerror(error),
+            ggl_strerror(remote_error)
+        );
+        assert(0);
+    }
+    if (remote_error != expected_result) {
+        GGL_LOGE(
+            "delete key %s expected result %s but got %s",
+            print_key_path(&key_path),
+            ggl_strerror(expected_result),
+            ggl_strerror(remote_error)
+        );
+        assert(0);
+        return;
+    }
+}
+
 static GglError subscription_callback(
     void *ctx, unsigned int handle, GglObject data
 ) {
@@ -899,6 +943,68 @@ int main(int argc, char **argv) {
         GGL_ERR_OK
     );
 
+    // Test to ensure a key can be deleted, not affecting its parent
+    test_insert(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component13")), GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_OBJ_BUF(GGL_STR("value")),
+        -1,
+        GGL_ERR_OK
+    );
+    test_delete(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component13")), GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_ERR_OK
+    );
+    test_get(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component13")), GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_OBJ_NULL(),
+        GGL_ERR_NOENTRY
+    );
+    test_get(
+        GGL_LIST(GGL_OBJ_BUF(GGL_STR("component13"))),
+        GGL_OBJ_MAP(GGL_MAP()),
+        GGL_ERR_OK
+    );
+
+    // Test to ensure deletes are recursive
+    test_insert(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component14")),
+            GGL_OBJ_BUF(GGL_STR("foo")),
+            GGL_OBJ_BUF(GGL_STR("bar"))
+        ),
+        GGL_OBJ_BUF(GGL_STR("value")),
+        -1,
+        GGL_ERR_OK
+    );
+    test_delete(GGL_LIST(GGL_OBJ_BUF(GGL_STR("component14"))), GGL_ERR_OK);
+    test_get(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component14")),
+            GGL_OBJ_BUF(GGL_STR("foo")),
+            GGL_OBJ_BUF(GGL_STR("bar"))
+        ),
+        GGL_OBJ_NULL(),
+        GGL_ERR_NOENTRY
+    );
+    test_get(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component14")), GGL_OBJ_BUF(GGL_STR("foo"))
+        ),
+        GGL_OBJ_NULL(),
+        GGL_ERR_NOENTRY
+    );
+    test_get(
+        GGL_LIST(GGL_OBJ_BUF(GGL_STR("component14"))),
+        GGL_OBJ_NULL(),
+        GGL_ERR_NOENTRY
+    );
+
     // Test to ensure an empty map can be written and read
     test_insert(
         GGL_LIST(GGL_OBJ_BUF(GGL_STR("component15"))),
@@ -1004,6 +1110,55 @@ int main(int argc, char **argv) {
         GGL_ERR_FAILURE
     );
 
+    // Test to check subscriber behavior on deleted keys
+    test_insert(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component20")),
+            GGL_OBJ_BUF(GGL_STR("foo")),
+            GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_OBJ_BUF(GGL_STR("value1")),
+        -1,
+        GGL_ERR_OK
+    );
+    test_subscribe(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component20")),
+            GGL_OBJ_BUF(GGL_STR("foo")),
+            GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_ERR_OK
+    );
+    test_subscribe(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component20")), GGL_OBJ_BUF(GGL_STR("foo"))
+        ),
+        GGL_ERR_OK
+    );
+    test_delete(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component20")),
+            GGL_OBJ_BUF(GGL_STR("foo")),
+            GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_ERR_OK
+    );
+    test_insert(
+        GGL_LIST(
+            GGL_OBJ_BUF(GGL_STR("component20")),
+            GGL_OBJ_BUF(GGL_STR("foo")),
+            GGL_OBJ_BUF(GGL_STR("key"))
+        ),
+        GGL_OBJ_BUF(GGL_STR("value2")),
+        -1,
+        GGL_ERR_OK
+    ); // Should see one `read component20/foo/key` on the callback handle
+       // created for component20/foo
+    // Currently, the other subscription callback for component20/foo/key is not
+    // notified. In the future, it would be good to have that behavior too. See
+    // the docs/design/ggconfigd.md section "Subscription behavior for keys
+    // which become deleted" for more info.
+
     // test_insert(
     //     GGL_LIST(GGL_OBJ_BUF(GGL_STR("component")),
     //     GGL_OBJ_BUF(GGL_STR("bar"))), GGL_OBJ_MAP(GGL_MAP({ GGL_STR("foo"),

core-bus-gg-config/include/ggl/core_bus/gg_config.h

@@ -28,6 +28,9 @@ GglError ggl_gg_config_write(
     GglBufList key_path, GglObject value, const int64_t *timestamp
 );
 
+/// Wrapper for core-bus `gg_config` `delete`
+GglError ggl_gg_config_delete(GglBufList key_path);
+
 /// Wrapper for core-bus `gg_config` `subscribe`
 GglError ggl_gg_config_subscribe(
     GglBufList key_path,

core-bus-gg-config/src/gg_config.c

@@ -44,6 +44,34 @@ GglError ggl_gg_config_read(
     return err;
 }
 
+GglError ggl_gg_config_delete(GglBufList key_path) {
+    if (key_path.len > GGL_MAX_OBJECT_DEPTH) {
+        GGL_LOGE("Key path depth exceeds maximum handled.");
+        return GGL_ERR_UNSUPPORTED;
+    }
+
+    GglObject path_obj[GGL_MAX_OBJECT_DEPTH] = { 0 };
+    for (size_t i = 0; i < key_path.len; i++) {
+        path_obj[i] = GGL_OBJ_BUF(key_path.bufs[i]);
+    }
+
+    GglMap args = GGL_MAP(
+        { GGL_STR("key_path"),
+          GGL_OBJ_LIST((GglList) { .items = path_obj, .len = key_path.len }) },
+    );
+
+    GglError remote_err = GGL_ERR_OK;
+    GglError err = ggl_call(
+        GGL_STR("gg_config"), GGL_STR("delete"), args, &remote_err, NULL, NULL
+    );
+
+    if ((err == GGL_ERR_REMOTE) && (remote_err != GGL_ERR_OK)) {
+        err = remote_err;
+    }
+
+    return err;
+}
+
 GglError ggl_gg_config_read_str(GglBufList key_path, GglBuffer *result) {
     GglObject result_obj;
     GglBumpAlloc alloc = ggl_bump_alloc_init(*result);

docs/spec/core-bus-interface/gg_config.md

@@ -64,8 +64,9 @@ The `write` method does not have a response value.
 
 ## delete
 
-The `delete` method removes the key and value associated with `key_path`. If the
-value is a map with subkeys, everything under it is also deleted recursively.
+The `delete` method removes the key and value associated with `key_path`, if
+present. If the value is a map with subkeys, everything under it is also deleted
+recursively. If the key does not exist, nothing happens.
 
 - [gg-config-delete-1] `delete` can be invoked with call or notify.
 
@@ -80,7 +81,8 @@ value is a map with subkeys, everything under it is also deleted recursively.
 The `delete` method does not have a response value.
 
 - [gg-config-delete-resp-1] If the method returns without an error, the
-  configuration has been successfully deleted.
+  configuration key does not exist, either because it was deleted or it didn't
+  exist when the call was made.
 
 ## subscribe
 

ggconfigd/include/ggconfigd.h

@@ -20,6 +20,7 @@ GglError ggconfig_write_value_at_key(
     GglList *key_path, GglBuffer *value, int64_t timestamp
 );
 GglError ggconfig_write_empty_map(GglList *key_path);
+GglError ggconfig_delete_key(GglList *key_path);
 GglError ggconfig_get_value_from_key(GglList *key_path, GglObject *value);
 GglError ggconfig_get_key_notification(GglList *key_path, uint32_t handle);
 GglError ggconfig_open(void);

ggconfigd/src/db_corebus.c

@@ -133,10 +133,37 @@ static GglError rpc_read(void *ctx, GglMap params, uint32_t handle) {
     return GGL_ERR_OK;
 }
 
-static GglError rpc_subscribe(void *ctx, GglMap params, uint32_t handle) {
+static GglError rpc_delete(void *ctx, GglMap params, uint32_t handle) {
     (void) ctx;
 
-    GGL_LOGD("subscribing");
+    GglObject *key_path;
+    if (!ggl_map_get(params, GGL_STR("key_path"), &key_path)
+        || (key_path->type != GGL_TYPE_LIST)) {
+        GGL_LOGE("read received invalid key_path argument.");
+        return GGL_ERR_INVALID;
+    }
+
+    GglError err = ggl_list_type_check(key_path->list, GGL_TYPE_BUF);
+    if (err != GGL_ERR_OK) {
+        GGL_LOGE("key_path elements must be strings.");
+        return GGL_ERR_RANGE;
+    }
+
+    GGL_LOGD(
+        "Processing request to delete key %s (recursively)",
+        print_key_path(&key_path->list)
+    );
+    err = ggconfig_delete_key(&key_path->list);
+    if (err != GGL_ERR_OK) {
+        return err;
+    }
+
+    ggl_respond(handle, GGL_OBJ_NULL());
+    return GGL_ERR_OK;
+}
+
+static GglError rpc_subscribe(void *ctx, GglMap params, uint32_t handle) {
+    (void) ctx;
 
     GglObject *key_path;
     if (!ggl_map_get(params, GGL_STR("key_path"), &key_path)
@@ -301,6 +328,7 @@ void ggconfigd_start_server(void) {
     GglRpcMethodDesc handlers[]
         = { { GGL_STR("read"), false, rpc_read, NULL },
             { GGL_STR("write"), false, rpc_write, NULL },
+            { GGL_STR("delete"), false, rpc_delete, NULL },
             { GGL_STR("subscribe"), true, rpc_subscribe, NULL } };
     size_t handlers_len = sizeof(handlers) / sizeof(handlers[0]);