Make res_sorcery_realtime filter unknown retrieved results.
authorMark Michelson <mmichelson@digium.com>
Fri, 7 Mar 2014 21:23:39 +0000 (21:23 +0000)
committerMark Michelson <mmichelson@digium.com>
Fri, 7 Mar 2014 21:23:39 +0000 (21:23 +0000)
When retrieving data from a database or other realtime backend, it's quite
possible to retrieve variables that Asterisk does not care about but that
are legitimate to exist. Asterisk does not need to throw a hissy fit when
these variables are encountered but rather just filter them out.

Review: https://reviewboard.asterisk.org/r/3305
........

Merged revisions 410187 from http://svn.asterisk.org/svn/asterisk/branches/12

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@410207 65c4cc65-6c06-0410-ace0-fbb531ad65f3

include/asterisk/sorcery.h
main/sorcery.c
res/res_sorcery_realtime.c
tests/test_sorcery_realtime.c

index 873d322..a110336 100644 (file)
@@ -928,6 +928,26 @@ int ast_sorcery_object_set_extended(const void *object, const char *name, const
  */
 int ast_sorcery_object_id_compare(const void *obj_left, const void *obj_right, int flags);
 
+/*!
+ * \brief Get the sorcery object type given a type name.
+ *
+ * \param sorcery The sorcery from which to retrieve the object type
+ * \param type The type name
+ */
+struct ast_sorcery_object_type *ast_sorcery_get_object_type(const struct ast_sorcery *sorcery,
+               const char *type);
+
+/*!
+ * \brief Determine if a particular object field has been registered with sorcery
+ *
+ * \param object_type The object type to check against
+ * \param field_name The name of the field to check
+ *
+ * \retval 0 The field is not registered for this sorcery type
+ * \retval 1 The field is registered for this sorcery type
+ */
+int ast_sorcery_is_object_field_registered(const struct ast_sorcery_object_type *object_type,
+               const char *field_name);
 
 #if defined(__cplusplus) || defined(c_plusplus)
 }
index 1d0aec7..2bb925e 100644 (file)
@@ -58,6 +58,9 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 /*! \brief Number of buckets for instances (should be prime for performance reasons) */
 #define INSTANCE_BUCKETS 17
 
+/*! \brief Number of buckets for object fields (should be prime for performance reasons) */
+#define OBJECT_FIELD_BUCKETS 29
+
 /*! \brief Thread pool for observers */
 static struct ast_threadpool *threadpool;
 
@@ -258,22 +261,22 @@ static sorcery_field_handler sorcery_field_default_handler(enum aco_option_type
 /*! \brief Hashing function for sorcery wizards */
 static int sorcery_wizard_hash(const void *obj, const int flags)
 {
-    const struct ast_sorcery_wizard *object;
-    const char *key;
+       const struct ast_sorcery_wizard *object;
+       const char *key;
 
-    switch (flags & OBJ_SEARCH_MASK) {
-    case OBJ_SEARCH_KEY:
-        key = obj;
-        break;
-    case OBJ_SEARCH_OBJECT:
-        object = obj;
-        key = object->name;
-        break;
-    default:
-        ast_assert(0);
-        return 0;
-    }
-    return ast_str_hash(key);
+       switch (flags & OBJ_SEARCH_MASK) {
+       case OBJ_SEARCH_KEY:
+               key = obj;
+               break;
+       case OBJ_SEARCH_OBJECT:
+               object = obj;
+               key = object->name;
+               break;
+       default:
+               ast_assert(0);
+               return 0;
+       }
+       return ast_str_hash(key);
 }
 
 /*! \brief Comparator function for sorcery wizards */
@@ -304,6 +307,54 @@ static int sorcery_wizard_cmp(void *obj, void *arg, int flags)
        return CMP_MATCH;
 }
 
+/*! \brief Hashing function for sorcery wizards */
+static int object_type_field_hash(const void *obj, const int flags)
+{
+       const struct ast_sorcery_object_field *object_field;
+       const char *key;
+
+       switch (flags & OBJ_SEARCH_MASK) {
+       case OBJ_SEARCH_KEY:
+               key = obj;
+               break;
+       case OBJ_SEARCH_OBJECT:
+               object_field = obj;
+               key = object_field->name;
+               break;
+       default:
+               ast_assert(0);
+               return 0;
+       }
+       return ast_str_hash(key);
+}
+
+static int object_type_field_cmp(void *obj, void *arg, int flags)
+{
+       const struct ast_sorcery_object_field *field_left = obj;
+       const struct ast_sorcery_object_field *field_right = arg;
+       const char *right_key = arg;
+       int cmp;
+
+       switch (flags & OBJ_SEARCH_MASK) {
+       case OBJ_SEARCH_OBJECT:
+               right_key = field_right->name;
+               /* Fall through */
+       case OBJ_SEARCH_KEY:
+               cmp = strcmp(field_left->name, right_key);
+               break;
+       case OBJ_SEARCH_PARTIAL_KEY:
+               cmp = strncmp(field_left->name, right_key, strlen(right_key));
+               break;
+       default:
+               cmp = 0;
+               break;
+       }
+       if (cmp) {
+               return 0;
+       }
+       return CMP_MATCH;
+}
+
 /*! \brief Cleanup function */
 static void sorcery_exit(void)
 {
@@ -351,22 +402,22 @@ static int sorcery_instance_cmp(void *obj, void *arg, int flags)
 /*! \brief Hashing function for sorcery instances */
 static int sorcery_instance_hash(const void *obj, const int flags)
 {
-    const struct ast_sorcery *object;
-    const char *key;
+       const struct ast_sorcery *object;
+       const char *key;
 
-    switch (flags & OBJ_SEARCH_MASK) {
-    case OBJ_SEARCH_KEY:
-        key = obj;
-        break;
-    case OBJ_SEARCH_OBJECT:
-        object = obj;
-        key = object->module_name;
-        break;
-    default:
-        ast_assert(0);
-        return 0;
-    }
-    return ast_str_hash(key);
+       switch (flags & OBJ_SEARCH_MASK) {
+       case OBJ_SEARCH_KEY:
+               key = obj;
+               break;
+       case OBJ_SEARCH_OBJECT:
+               object = obj;
+               key = object->module_name;
+               break;
+       default:
+               ast_assert(0);
+               return 0;
+       }
+       return ast_str_hash(key);
 }
 
 int ast_sorcery_init(void)
@@ -461,22 +512,22 @@ static void sorcery_destructor(void *obj)
 /*! \brief Hashing function for sorcery types */
 static int sorcery_type_hash(const void *obj, const int flags)
 {
-    const struct ast_sorcery_object_type *object;
-    const char *key;
+       const struct ast_sorcery_object_type *object;
+       const char *key;
 
-    switch (flags & OBJ_SEARCH_MASK) {
-    case OBJ_SEARCH_KEY:
-        key = obj;
-        break;
-    case OBJ_SEARCH_OBJECT:
-        object = obj;
-        key = object->name;
-        break;
-    default:
-        ast_assert(0);
-        return 0;
-    }
-    return ast_str_hash(key);
+       switch (flags & OBJ_SEARCH_MASK) {
+       case OBJ_SEARCH_KEY:
+               key = obj;
+               break;
+       case OBJ_SEARCH_OBJECT:
+               object = obj;
+               key = object->name;
+               break;
+       default:
+               ast_assert(0);
+               return 0;
+       }
+       return ast_str_hash(key);
 }
 
 /*! \brief Comparator function for sorcery types */
@@ -577,7 +628,8 @@ static struct ast_sorcery_object_type *sorcery_object_type_alloc(const char *typ
                return NULL;
        }
 
-       if (!(object_type->fields = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 1, NULL, NULL))) {
+       if (!(object_type->fields = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, OBJECT_FIELD_BUCKETS,
+                                       object_type_field_hash, object_type_field_cmp))) {
                ao2_ref(object_type, -1);
                return NULL;
        }
@@ -1787,3 +1839,26 @@ int ast_sorcery_object_id_compare(const void *obj_left, const void *obj_right, i
        }
        return strcmp(ast_sorcery_object_get_id(obj_left), ast_sorcery_object_get_id(obj_right));
 }
+
+struct ast_sorcery_object_type *ast_sorcery_get_object_type(const struct ast_sorcery *sorcery,
+               const char *type)
+{
+       return ao2_find(sorcery->types, type, OBJ_SEARCH_KEY);
+}
+
+int ast_sorcery_is_object_field_registered(const struct ast_sorcery_object_type *object_type,
+               const char *field_name)
+{
+       struct ast_sorcery_object_field *object_field;
+       int res = 1;
+
+       ast_assert(object_type != NULL);
+
+       object_field = ao2_find(object_type->fields, field_name, OBJ_SEARCH_KEY);
+       if (!object_field) {
+               res = 0;
+       }
+
+       ao2_cleanup(object_field);
+       return res;
+}
index 29517c8..bc4c7f9 100644 (file)
@@ -82,14 +82,47 @@ static int sorcery_realtime_create(const struct ast_sorcery *sorcery, void *data
        return ast_store_realtime_fields(family, fields) ? -1 : 0;
 }
 
-/*! \brief Internal helper function which returns a filtered objectset, basically removes the id field */
-static struct ast_variable *sorcery_realtime_filter_objectset(struct ast_variable *objectset, struct ast_variable **id)
+/*! \brief Internal helper function which returns a filtered objectset. 
+ *
+ * The following are filtered out of the objectset:
+ * \li The id field. This is returned to the caller in an out parameter.
+ * \li Fields that are not registered with sorcery.
+ *
+ * \param objectset Objectset to filter.
+ * \param[out] id The ID of the sorcery object, as found in the objectset.
+ * \param sorcery The sorcery instance that is requesting an objectset.
+ * \param type The object type
+ *
+ * \return The filtered objectset
+ */
+static struct ast_variable *sorcery_realtime_filter_objectset(struct ast_variable *objectset, struct ast_variable **id,
+               const struct ast_sorcery *sorcery, const char *type)
 {
-       struct ast_variable *previous = NULL, *field;
+       struct ast_variable *previous = NULL, *field = objectset;
+       struct ast_sorcery_object_type *object_type;
+
+       object_type = ast_sorcery_get_object_type(sorcery, type);
+       if (!object_type) {
+               ast_log(LOG_WARNING, "Unknown sorcery object type %s. Expect errors\n", type);
+               /* Continue since we still want to filter out the id */
+       }
+
+       while (field) {
+               int remove_field = 0;
+               int delete_field = 0;
 
-       for (field = objectset; field; field = field->next) {
                if (!strcmp(field->name, UUID_FIELD)) {
                        *id = field;
+                       remove_field = 1;
+               } else if (object_type &&
+                               !ast_sorcery_is_object_field_registered(object_type, field->name)) {
+                       ast_debug(1, "Filtering out realtime field '%s' from retrieval\n", field->name);
+                       remove_field = 1;
+                       delete_field = 1;
+               }
+
+               if (remove_field) {
+                       struct ast_variable *removed;
 
                        if (previous) {
                                previous->next = field->next;
@@ -97,10 +130,16 @@ static struct ast_variable *sorcery_realtime_filter_objectset(struct ast_variabl
                                objectset = field->next;
                        }
 
-                       field->next = NULL;
-                       break;
+                       removed = field;
+                       field = field->next;
+                       removed->next = NULL;
+                       if (delete_field) {
+                               ast_variables_destroy(removed);
+                       }
+               } else {
+                       previous = field;
+                       field = field->next;
                }
-               previous = field;
        }
 
        return objectset;
@@ -117,7 +156,7 @@ static void *sorcery_realtime_retrieve_fields(const struct ast_sorcery *sorcery,
                return NULL;
        }
 
-       objectset = sorcery_realtime_filter_objectset(objectset, &id);
+       objectset = sorcery_realtime_filter_objectset(objectset, &id, sorcery, type);
 
        if (!id || !(object = ast_sorcery_alloc(sorcery, type, id->value)) || ast_sorcery_objectset_apply(sorcery, object, objectset)) {
                return NULL;
@@ -159,20 +198,18 @@ static void sorcery_realtime_retrieve_multiple(const struct ast_sorcery *sorcery
        }
 
        while ((row = ast_category_browse(rows, row))) {
-               struct ast_variable *objectset = ast_category_root(rows, row);
+               struct ast_category *cat = ast_category_get(rows, row);
+               struct ast_variable *objectset = ast_category_detach_variables(cat);
                RAII_VAR(struct ast_variable *, id, NULL, ast_variables_destroy);
                RAII_VAR(void *, object, NULL, ao2_cleanup);
 
-               objectset = sorcery_realtime_filter_objectset(objectset, &id);
+               objectset = sorcery_realtime_filter_objectset(objectset, &id, sorcery, type);
 
                if (id && (object = ast_sorcery_alloc(sorcery, type, id->value)) && !ast_sorcery_objectset_apply(sorcery, object, objectset)) {
                        ao2_link(objects, object);
                }
 
-               /* If the id is the root of the row it will be destroyed during ast_config_destroy */
-               if (id == ast_category_root(rows, row)) {
-                       id = NULL;
-               }
+               ast_variables_destroy(objectset);
        }
 }
 
index 8a2140b..a3642d4 100644 (file)
@@ -753,6 +753,108 @@ AST_TEST_DEFINE(object_delete_uncreated)
        return AST_TEST_PASS;
 }
 
+AST_TEST_DEFINE(object_allocate_on_retrieval)
+{
+       RAII_VAR(struct ast_sorcery *, sorcery, NULL, deinitialize_sorcery);
+       RAII_VAR(struct test_sorcery_object *, obj, NULL, ao2_cleanup);
+       struct ast_category *cat;
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "object_allocate_on_retrieval";
+               info->category = "/res/sorcery_realtime/";
+               info->summary = "sorcery object allocation upon retrieval unit test";
+               info->description =
+                       "This test creates data in a realtime backend, not through sorcery. Sorcery is then\n"
+                       "instructed to retrieve an object with the id of the object that was created in the\n"
+                       "realtime backend. Sorcery should be able to allocate the object appropriately";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       if (!(sorcery = alloc_and_initialize_sorcery())) {
+               ast_test_status_update(test, "Failed to open sorcery structure\n");
+               return AST_TEST_FAIL;
+       }
+
+       cat = ast_category_new("blah", "", 0);
+       ast_variable_append(cat, ast_variable_new("id", "blah", ""));
+       ast_variable_append(cat, ast_variable_new("bob", "42", ""));
+       ast_variable_append(cat, ast_variable_new("joe", "93", ""));
+       ast_category_append(realtime_objects, cat);
+
+       if (!(obj = ast_sorcery_retrieve_by_id(sorcery, "test", "blah"))) {
+               ast_test_status_update(test, "Failed to allocate object 'blah' base on realtime data\n");
+               return AST_TEST_FAIL;
+       }
+
+       if (obj->bob != 42) {
+               ast_test_status_update(test, "Object's 'bob' field does not have expected value: %d != 42\n",
+                               obj->bob);
+               return AST_TEST_FAIL;
+       } else if (obj->joe != 93) {
+               ast_test_status_update(test, "Object's 'joe' field does not have expected value: %d != 93\n",
+                               obj->joe);
+               return AST_TEST_FAIL;
+       }
+
+       return AST_TEST_PASS;
+}
+
+
+AST_TEST_DEFINE(object_filter)
+{
+       RAII_VAR(struct ast_sorcery *, sorcery, NULL, deinitialize_sorcery);
+       RAII_VAR(struct test_sorcery_object *, obj, NULL, ao2_cleanup);
+       struct ast_category *cat;
+
+       switch (cmd) {
+       case TEST_INIT:
+               info->name = "object_filter";
+               info->category = "/res/sorcery_realtime/";
+               info->summary = "sorcery object field filter unit test";
+               info->description =
+                       "This test creates data in a realtime backend, not through sorcery. In addition to\n"
+                       "the object fields that have been registered with sorcery, there is data in the\n"
+                       "realtime backend that is unknown to sorcery. When sorcery attempts to retrieve\n"
+                       "the object from the realtime backend, the data unknown to sorcery should be\n"
+                       "filtered out of the returned objectset, and the object should be successfully\n"
+                       "allocated by sorcery\n";
+               return AST_TEST_NOT_RUN;
+       case TEST_EXECUTE:
+               break;
+       }
+
+       if (!(sorcery = alloc_and_initialize_sorcery())) {
+               ast_test_status_update(test, "Failed to open sorcery structure\n");
+               return AST_TEST_FAIL;
+       }
+
+       cat = ast_category_new("blah", "", 0);
+       ast_variable_append(cat, ast_variable_new("id", "blah", ""));
+       ast_variable_append(cat, ast_variable_new("bob", "42", ""));
+       ast_variable_append(cat, ast_variable_new("joe", "93", ""));
+       ast_variable_append(cat, ast_variable_new("fred", "50", ""));
+       ast_category_append(realtime_objects, cat);
+
+       if (!(obj = ast_sorcery_retrieve_by_id(sorcery, "test", "blah"))) {
+               ast_test_status_update(test, "Failed to retrieve properly created object using id of 'blah'\n");
+               return AST_TEST_FAIL;
+       }
+
+       if (obj->bob != 42) {
+               ast_test_status_update(test, "Object's 'bob' field does not have expected value: %d != 42\n",
+                               obj->bob);
+               return AST_TEST_FAIL;
+       } else if (obj->joe != 93) {
+               ast_test_status_update(test, "Object's 'joe' field does not have expected value: %d != 93\n",
+                               obj->joe);
+               return AST_TEST_FAIL;
+       }
+       return AST_TEST_PASS;
+}
+
 static int unload_module(void)
 {
        ast_config_engine_deregister(&sorcery_config_engine);
@@ -766,6 +868,8 @@ static int unload_module(void)
        AST_TEST_UNREGISTER(object_update_uncreated);
        AST_TEST_UNREGISTER(object_delete);
        AST_TEST_UNREGISTER(object_delete_uncreated);
+       AST_TEST_UNREGISTER(object_allocate_on_retrieval);
+       AST_TEST_UNREGISTER(object_filter);
 
        return 0;
 }
@@ -784,6 +888,8 @@ static int load_module(void)
        AST_TEST_REGISTER(object_update_uncreated);
        AST_TEST_REGISTER(object_delete);
        AST_TEST_REGISTER(object_delete_uncreated);
+       AST_TEST_REGISTER(object_allocate_on_retrieval);
+       AST_TEST_REGISTER(object_filter);
 
        return AST_MODULE_LOAD_SUCCESS;
 }