]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
sorcery: Prevent duplicate objects and ensure missing objects are created on update
authorAlexei Gradinari <alex2grad@gmail.com>
Mon, 7 Jul 2025 20:52:13 +0000 (16:52 -0400)
committerAlexei Gradinari <alex2grad@gmail.com>
Wed, 27 Aug 2025 16:56:13 +0000 (16:56 +0000)
This patch resolves two issues in Sorcery objectset handling with multiple
backends:

1. Prevent duplicate objects:
   When an object exists in more than one backend (e.g., a contact in both
   'astdb' and 'realtime'), the objectset previously returned multiple instances
   of the same logical object. This caused logic failures in components like the
   PJSIP registrar, where duplicate contact entries led to overcounting and
   incorrect deletions, when max_contacts=1 and remove_existing=yes.

   This patch ensures only one instance of an object with a given key is added
   to the objectset, avoiding these duplicate-related side effects.

2. Ensure missing objects are created:
   When using multiple writable backends, a temporary backend failure can lead
   to objects missing permanently from that backend.
   Currently, .update() silently fails if the object is not present,
   and no .create() is attempted.
   This results in inconsistent state across backends (e.g. astdb vs. realtime).

   This patch introduces a new global option in sorcery.conf:
     [general]
     update_or_create_on_update_miss = yes|no

   Default: no (preserves existing behavior).

   When enabled: if .update() fails with no data found, .create() is attempted
   in that backend. This ensures that objects missing due to temporary backend
   outages are re-synchronized once the backend is available again.

   Added a new CLI command:
     sorcery show settings
   Displays global Sorcery settings, including the current value of
   update_or_create_on_update_miss.

   Updated tests to validate both flag enabled/disabled behavior.

Fixes: #1289
UserNote: Users relying on Sorcery multiple writable backends configurations
(e.g., astdb + realtime) may now enable update_or_create_on_update_miss = yes
in sorcery.conf to ensure missing objects are recreated after temporary backend
failures. Default behavior remains unchanged unless explicitly enabled.

configs/samples/sorcery.conf.sample
include/asterisk/sorcery.h
main/sorcery.c
res/res_sorcery_astdb.c
res/res_sorcery_memory.c
res/res_sorcery_realtime.c
tests/test_sorcery.c
tests/test_sorcery_astdb.c

index 393015c36b0d7ae6ab78f001095fe20c7c6ebb9f..223a60aabfb7c75ad01b3bed1588b638e2011e65 100644 (file)
@@ -1,5 +1,16 @@
 ; Sample configuration file for Sorcery Data Access Layer
 
+[general]
+
+; When true, writable Sorcery backends may attempt to insert the object
+; if the object is missing.
+; This helps re-populate missing backends when using multiple writable mappings
+; (e.g., AstDB + Realtime) if a backend was temporarily unavailable.
+; NOTE: This is best-effort and does not guarantee atomicity across backends.
+; Default: no (preserves legacy behavior).
+;
+;update_or_create_on_update_miss = no
+
 ;
 ; Wizards
 ;
index ea3da000e69ddb252409694ef2df08e559a258b1..6e8b1c3e682c8951d6106b2b2d2aedb91cd7a585 100644 (file)
@@ -1623,6 +1623,24 @@ int ast_sorcery_is_object_field_registered(const struct ast_sorcery_object_type
  */
 const char *ast_sorcery_get_module(const struct ast_sorcery *sorcery);
 
+/**
+ * \brief Global control for optional update->create fallback in backends.
+ *
+ * When non-zero, writable Sorcery backends may attempt to call create()
+ * if update() indicates the object is missing (e.g., 0 rows affected or
+ * NOT_FOUND). This helps re-populate backends that temporarily missed an
+ * insert.
+ *
+ * Default is 0 (off) to maintain legacy behavior.
+ *
+ * Configured via sorcery.conf:
+ *   [general]
+ *   update_or_create_on_update_miss = yes|no
+ *
+ * NOTE: Backends MUST gate their fallback logic on this variable.
+ */
+extern int ast_sorcery_update_or_create_on_update_miss;
+
 /*!
  * \section AstSorceryConvenienceMacros Simple Sorcery Convenience Macros
  *
index 06a458ab0b3e7640c07c81e2ccacfe155a3f847f..0104437717675abdf79456fafe14719ca08ea36f 100644 (file)
@@ -42,6 +42,7 @@
 #include "asterisk/threadpool.h"
 #include "asterisk/json.h"
 #include "asterisk/vector.h"
+#include "asterisk/cli.h"
 
 /* To prevent DEBUG_FD_LEAKS from interfering with things we undef open and close */
 #undef open
@@ -283,6 +284,9 @@ struct ao2_container *observers;
 /*! \brief Registered sorcery instances */
 static struct ao2_container *instances;
 
+/* \brief Global config flag (declared in sorcery.h) */
+int ast_sorcery_update_or_create_on_update_miss = 0;
+
 static int int_handler_fn(const void *obj, const intptr_t *args, char **buf)
 {
        int *field = (int *)(obj + args[0]);
@@ -365,9 +369,39 @@ AO2_STRING_FIELD_CMP_FN(ast_sorcery_internal_wizard, callbacks.name)
 AO2_STRING_FIELD_HASH_FN(ast_sorcery_object_field, name)
 AO2_STRING_FIELD_CMP_FN(ast_sorcery_object_field, name)
 
+/*!
+ * \internal
+ * \brief CLI command implementation for 'sorcery show settings'
+ */
+static char *cli_show_settings(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
+{
+       switch (cmd) {
+       case CLI_INIT:
+               e->command = "sorcery show settings";
+               e->usage = "Usage: sorcery show settings\n"
+                          "      Show global configuration options\n";
+               return NULL;
+       case CLI_GENERATE:
+               return NULL;
+       }
+
+       ast_cli(a->fd, "\nSorcery global settings\n");
+       ast_cli(a->fd, "-----------------\n");
+       ast_cli(a->fd, "  Update->Create fallback in backends: %s\n",
+               ast_sorcery_update_or_create_on_update_miss ? "enabled" : "disabled");
+
+       return CLI_SUCCESS;
+}
+
+
+static struct ast_cli_entry cli_commands[] = {
+        AST_CLI_DEFINE(cli_show_settings, "Show global configuration options"),
+};
+
 /*! \brief Cleanup function for graceful shutdowns */
 static void sorcery_cleanup(void)
 {
+       ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
        ast_threadpool_shutdown(threadpool);
        threadpool = NULL;
        ao2_cleanup(wizards);
@@ -384,6 +418,29 @@ AO2_STRING_FIELD_CMP_FN(sorcery_proxy, module_name)
 /*! \brief Hashing function for sorcery instances */
 AO2_STRING_FIELD_HASH_FN(sorcery_proxy, module_name)
 
+/*!
+ * \internal
+ * \brief Parse [general] options from sorcery.conf and set globals.
+ */
+static void parse_general_options(void)
+{
+       struct ast_flags flags = { 0 };
+       struct ast_config *cfg = ast_config_load2("sorcery.conf", "sorcery", flags);
+       const struct ast_variable *var;
+
+       if (!cfg || cfg == CONFIG_STATUS_FILEINVALID) {
+               return;
+       }
+
+       for (var = ast_variable_browse(cfg, "general"); var; var = var->next) {
+               if (!strcasecmp(var->name, "update_or_create_on_update_miss")) {
+                       ast_sorcery_update_or_create_on_update_miss = ast_true(var->value);
+               }
+       }
+
+       ast_config_destroy(cfg);
+}
+
 int ast_sorcery_init(void)
 {
        struct ast_threadpool_options options = {
@@ -395,6 +452,8 @@ int ast_sorcery_init(void)
        };
        ast_assert(wizards == NULL);
 
+       parse_general_options();
+
        threadpool = ast_threadpool_create("sorcery", NULL, &options);
        if (!threadpool) {
                return -1;
@@ -417,6 +476,10 @@ int ast_sorcery_init(void)
                return -1;
        }
 
+       if (ast_cli_register_multiple(cli_commands, ARRAY_LEN(cli_commands))) {
+               return -1;
+       }
+
        ast_register_cleanup(sorcery_cleanup);
 
        return 0;
@@ -1907,7 +1970,7 @@ void *ast_sorcery_retrieve_by_fields(const struct ast_sorcery *sorcery, const ch
 
        /* If returning multiple objects create a container to store them in */
        if ((flags & AST_RETRIEVE_FLAG_MULTIPLE)) {
-               object = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, 0, NULL, NULL);
+               object = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT, ast_sorcery_object_id_sort, ast_sorcery_object_id_compare);
                if (!object) {
                        return NULL;
                }
@@ -1961,7 +2024,7 @@ struct ao2_container *ast_sorcery_retrieve_by_regex(const struct ast_sorcery *so
                return NULL;
        }
 
-       objects = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, 0, NULL, NULL);
+       objects = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT, ast_sorcery_object_id_sort, ast_sorcery_object_id_compare);
        if (!objects) {
                return NULL;
        }
@@ -1996,7 +2059,7 @@ struct ao2_container *ast_sorcery_retrieve_by_prefix(const struct ast_sorcery *s
                return NULL;
        }
 
-       objects = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, 0, NULL, NULL);
+       objects = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK, AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT, ast_sorcery_object_id_sort, ast_sorcery_object_id_compare);
        if (!objects) {
                return NULL;
        }
index 87823be0d5644eacff8baa4f9a44b4ff0d3cc427..ccc2b4045105df8d55756edddb680431ffe1873a 100644 (file)
@@ -373,7 +373,7 @@ static int sorcery_astdb_update(const struct ast_sorcery *sorcery, void *data, v
        snprintf(family, sizeof(family), "%s/%s", prefix, ast_sorcery_object_get_type(object));
 
        /* It is okay for the value to be truncated, we are only checking that it exists */
-       if (ast_db_get(family, ast_sorcery_object_get_id(object), value, sizeof(value))) {
+       if (ast_db_get(family, ast_sorcery_object_get_id(object), value, sizeof(value)) && !ast_sorcery_update_or_create_on_update_miss) {
                return -1;
        }
 
index 1e8bef94b7720e14a7c8d98de135c2078b386494..04d4c84d75e96a5ce9d79b277084a495c5070ab8 100644 (file)
@@ -229,12 +229,17 @@ static int sorcery_memory_update(const struct ast_sorcery *sorcery, void *data,
 
        ao2_lock(data);
 
-       if (!(existing = ao2_find(data, ast_sorcery_object_get_id(object), OBJ_KEY | OBJ_UNLINK))) {
+       if (!(existing = ao2_find(data, ast_sorcery_object_get_id(object), OBJ_KEY | OBJ_UNLINK)) && !ast_sorcery_update_or_create_on_update_miss) {
                ao2_unlock(data);
                return -1;
        }
 
-       ao2_link(data, object);
+       if (existing) {
+               ao2_link(data, object);
+       } else {
+               /* Not found: only create if the global flag is enabled */
+               ao2_link_flags(data, object, OBJ_NOLOCK);
+       }
 
        ao2_unlock(data);
 
index 31ba2f55526d8945d9acdc1aa21a53e9ee687822..3d2554f0babedd986c2d03ad88f51e49553d4151 100644 (file)
@@ -286,12 +286,37 @@ static int sorcery_realtime_update(const struct ast_sorcery *sorcery, void *data
 {
        struct sorcery_config *config = data;
        RAII_VAR(struct ast_variable *, fields, ast_sorcery_objectset_create(sorcery, object), ast_variables_destroy);
+       int ret;
+       struct ast_variable *id;
 
        if (!fields) {
                return -1;
        }
 
-       return (ast_update_realtime_fields(config->family, UUID_FIELD, ast_sorcery_object_get_id(object), fields) < 0) ? -1 : 0;
+       ret = ast_update_realtime_fields(config->family, UUID_FIELD, ast_sorcery_object_get_id(object), fields);
+       if (ret < 0) {
+               /* An error occurred */
+               return -1;
+       } else if (ret > 0) {
+               /* The object was updated */
+               return 0;
+       }
+
+       if (!ast_sorcery_update_or_create_on_update_miss) {
+               /* The object does not exist (nothing was updated) and fallback disabled */
+               return -1;
+       }
+
+       id = ast_variable_new(UUID_FIELD, ast_sorcery_object_get_id(object), "");
+       if (!id) {
+               return -1;
+       }
+
+       /* Place the identifier at the front for sanity sake */
+       id->next = fields;
+       fields = id;
+
+       return (ast_store_realtime_fields(config->family, fields) <= 0) ? -1 : 0;
 }
 
 static int sorcery_realtime_delete(const struct ast_sorcery *sorcery, void *data, void *object)
index 1f220407bc5881b8fe28fa3871a455c26c7dc32b..2e808f64b57d9dadd84621705e61becb96c6f95e 100644 (file)
@@ -2141,11 +2141,18 @@ AST_TEST_DEFINE(object_update_uncreated)
                return AST_TEST_FAIL;
        }
 
+       ast_sorcery_update_or_create_on_update_miss = 0;
        if (!ast_sorcery_update(sorcery, obj)) {
                ast_test_status_update(test, "Successfully updated an object which has not been created yet\n");
                return AST_TEST_FAIL;
        }
 
+       ast_sorcery_update_or_create_on_update_miss = 1;
+       if (ast_sorcery_update(sorcery, obj)) {
+               ast_test_status_update(test, "Failed to create object when update() finds no object in a backend\n");
+               return AST_TEST_FAIL;
+       }
+
        return AST_TEST_PASS;
 }
 
index 62895960caf2a5a70c0e871d9b718b07e4c39dc7..714073c7b3ea8740d1cddf641ecff26ec4dadd84 100644 (file)
@@ -511,11 +511,18 @@ AST_TEST_DEFINE(object_update_uncreated)
                return AST_TEST_FAIL;
        }
 
+       ast_sorcery_update_or_create_on_update_miss = 0;
        if (!ast_sorcery_update(sorcery, obj)) {
                ast_test_status_update(test, "Successfully updated an object which has not been created yet\n");
                return AST_TEST_FAIL;
        }
 
+       ast_sorcery_update_or_create_on_update_miss = 1;
+       if (ast_sorcery_update(sorcery, obj)) {
+               ast_test_status_update(test, "Failed to create object when update() finds no object in a backend\n");
+               return AST_TEST_FAIL;
+       }
+
        return AST_TEST_PASS;
 }