[Asterisk-code-review] sorcery: Add API to insert/remove a wizard to/from an object... (asterisk[master])

Joshua Colp asteriskteam at digium.com
Thu May 14 15:20:32 CDT 2015


Joshua Colp has submitted this change and it was merged.

Change subject: sorcery: Add API to insert/remove a wizard to/from an object type's list
......................................................................


sorcery: Add API to insert/remove a wizard to/from an object type's list

Currently you can 'apply' a wizard to an object type but the wizard
always goes at the end of the object type's wizard list.  This patch
adds a new ast_sorcery_insert_wizard_mapping function that allows
you to insert a wizard anyplace in the list.  I.E.  You could
add a caching wizard to an object type and place it before all
wizards.

ast_sorcery_get_wizard_mapping_count and
ast_sorcery_get_wizard_mapping were added to allow examination
of the mapping list.

ast_sorcery_remove_mapping was added to remove a mapping by name.

As part of this patch, the object type's wizard list was converted
from an ao2_container to an AST_VECTOR_RW.

A new test was added to test_sorcery for this capability.

ASTERISK-25044 #close

Change-Id: I9d2469a9296b2698082c0989e25e6848dc403b57
---
M include/asterisk/sorcery.h
M main/sorcery.c
M tests/test_sorcery.c
3 files changed, 451 insertions(+), 73 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h
index a5061c6..9d39ce4 100644
--- a/include/asterisk/sorcery.h
+++ b/include/asterisk/sorcery.h
@@ -497,6 +497,125 @@
 #define ast_sorcery_apply_wizard_mapping(sorcery, type, name, data, caching) \
 	__ast_sorcery_apply_wizard_mapping((sorcery), (type), AST_MODULE, (name), (data), (caching));
 
+
+/*!
+ * \brief Pre-defined locations to insert at
+ */
+enum ast_sorcery_wizard_position {
+	AST_SORCERY_WIZARD_POSITION_LAST = -1,
+	AST_SORCERY_WIZARD_POSITION_FIRST = 0,
+};
+
+/*!
+ * \brief Insert an additional object wizard mapping at a specific position
+ * in the wizard list
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param type Type of object to apply to
+ * \param module The name of the module, typically AST_MODULE
+ * \param name Name of the wizard to use
+ * \param data Data to be passed to wizard
+ * \param caching Wizard should cache
+ * \param position An index to insert to or one of ast_sorcery_wizard_position
+ *
+ * \return What occurred when applying the mapping
+ *
+ * \note This should be called *after* applying default mappings
+ * \note Wizards can be retrieved by using ast_sorcery_get_wizard_mapping_count
+ * and iterating over them using ast_sorcery_get_wizard_mapping.
+ *
+ * \since 13.4.0
+ */
+enum ast_sorcery_apply_result __ast_sorcery_insert_wizard_mapping(struct ast_sorcery *sorcery,
+	const char *type, const char *module, const char *name, const char *data,
+	unsigned int caching, int position);
+
+/*!
+ * \brief Insert an additional object wizard mapping at a specific position
+ * in the wizard list
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param type Type of object to apply to
+ * \param module The name of the module, typically AST_MODULE
+ * \param name Name of the wizard to use
+ * \param data Data to be passed to wizard
+ * \param position One of ast_sorcery_wizard_position
+ *
+ * \return What occurred when applying the mapping
+ *
+ * \note This should be called *after* applying default mappings
+ * \since 13.4.0
+ */
+#define ast_sorcery_insert_wizard_mapping(sorcery, type, name, data, caching, position) \
+	__ast_sorcery_insert_wizard_mapping((sorcery), (type), AST_MODULE, (name), (data), \
+		(caching), (position))
+
+/*!
+ * \brief Remove an object wizard mapping
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param type Type of object to remove from
+ * \param module The name of the module, typically AST_MODULE
+ * \param name The name of the wizard to remove
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ *
+ * \since 13.4.0
+ */
+int __ast_sorcery_remove_wizard_mapping(struct ast_sorcery *sorcery,
+	const char *type, const char *module, const char *name);
+
+/*!
+ * \brief Remove an object wizard mapping
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param type Type of object to remove from
+ * \param name The name of the wizard to remove
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ *
+ * \since 13.4.0
+ */
+#define ast_sorcery_remove_wizard_mapping(sorcery, type, name) \
+	__ast_sorcery_remove_wizard_mapping((sorcery), (type), AST_MODULE, (name))
+
+/*!
+ * \brief Return the number of wizards mapped to an object type
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param type Type of object
+ *
+ * \return Number of wizards or -1 for error
+ * \since 13.4.0
+ */
+int ast_sorcery_get_wizard_mapping_count(struct ast_sorcery *sorcery,
+	const char *type);
+
+/*!
+ * \brief By index, return a wizard mapped to an object type
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param type Type of object
+ * \param index Index of the wizard
+ * \param wizard A pointer to receive the wizard pointer
+ * \param data A pointer to receive the data pointer
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ *
+ * \warning The wizard will have its reference count bumped so you must
+ * call ao2_cleanup when you're done with it.
+ *
+ * \note The wizard and data returned are valid only for this object type
+ * and only while the wizard is applied to the object type.
+ *
+ * \since 13.4.0
+ */
+int ast_sorcery_get_wizard_mapping(struct ast_sorcery *sorcery,
+	const char *type, int index, struct ast_sorcery_wizard **wizard, void **data);
+
 /*!
  * \brief Register an object type
  *
diff --git a/main/sorcery.c b/main/sorcery.c
index 1a4b3a0..c779548 100644
--- a/main/sorcery.c
+++ b/main/sorcery.c
@@ -43,6 +43,7 @@
 #include "asterisk/taskprocessor.h"
 #include "asterisk/threadpool.h"
 #include "asterisk/json.h"
+#include "asterisk/vector.h"
 
 /* To prevent DEBUG_FD_LEAKS from interfering with things we undef open and close */
 #undef open
@@ -86,6 +87,35 @@
 /*! \brief Thread pool for observers */
 static struct ast_threadpool *threadpool;
 
+/*! \brief Structure for an internal wizard instance */
+struct ast_sorcery_internal_wizard {
+	/*!
+	 * \brief Wizard interface itself
+	 * \warning Callbacks must always be declared first in this structure
+	 * so an ao2_ref on &callbacks will adjust the ref count on
+	 * internal_wizard.
+	 */
+	struct ast_sorcery_wizard callbacks;
+
+	/*! \brief Observers */
+	struct ao2_container *observers;
+};
+
+/*! \brief Structure for a wizard instance which operates on objects */
+struct ast_sorcery_object_wizard {
+	/*! \brief Wizard interface itself */
+	struct ast_sorcery_internal_wizard *wizard;
+
+	/*! \brief Unique data for the wizard */
+	void *data;
+
+	/*! \brief Wizard is acting as an object cache */
+	unsigned int caching:1;
+};
+
+/*! \brief Interface for a sorcery object type wizards */
+AST_VECTOR_RW(ast_sorcery_object_wizards, struct ast_sorcery_object_wizard *);
+
 /*! \brief Structure for internal sorcery object information */
 struct ast_sorcery_object {
 	/*! \brief Unique identifier of this object */
@@ -119,7 +149,7 @@
 	sorcery_diff_handler diff;
 
 	/*! \brief Wizard instances */
-	struct ao2_container *wizards;
+	struct ast_sorcery_object_wizards wizards;
 
 	/*! \brief Object fields */
 	struct ao2_container *fields;
@@ -174,27 +204,6 @@
 
 	/*! \brief Position of the field */
 	intptr_t args[];
-};
-
-/*! \brief Structure for an internal wizard instance */
-struct ast_sorcery_internal_wizard {
-	/*! \brief Wizard interface itself */
-	struct ast_sorcery_wizard callbacks;
-
-	/*! \brief Observers */
-	struct ao2_container *observers;
-};
-
-/*! \brief Structure for a wizard instance which operates on objects */
-struct ast_sorcery_object_wizard {
-	/*! \brief Wizard interface itself */
-	struct ast_sorcery_internal_wizard *wizard;
-
-	/*! \brief Unique data for the wizard */
-	void *data;
-
-	/*! \brief Wizard is acting as an object cache */
-	unsigned int caching:1;
 };
 
 /*! \brief Full structure for sorcery */
@@ -789,7 +798,10 @@
 {
 	struct ast_sorcery_object_type *object_type = obj;
 
-	ao2_cleanup(object_type->wizards);
+	AST_VECTOR_RW_WRLOCK(&object_type->wizards);
+	AST_VECTOR_CALLBACK_VOID(&object_type->wizards, ao2_cleanup);
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
+	AST_VECTOR_RW_FREE(&object_type->wizards);
 	ao2_cleanup(object_type->fields);
 	ao2_cleanup(object_type->observers);
 
@@ -806,6 +818,7 @@
 /*! \brief Internal function which allocates an object type structure */
 static struct ast_sorcery_object_type *sorcery_object_type_alloc(const char *type, const char *module)
 {
+#define INITIAL_WIZARD_VECTOR_SIZE 5
 	struct ast_sorcery_object_type *object_type;
 	char uuid[AST_UUID_STR_LEN];
 
@@ -814,7 +827,7 @@
 	}
 
 	/* Order matters for object wizards */
-	if (!(object_type->wizards = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 1, NULL, sorcery_wizard_cmp))) {
+	if (AST_VECTOR_RW_INIT(&object_type->wizards, INITIAL_WIZARD_VECTOR_SIZE) != 0) {
 		ao2_ref(object_type, -1);
 		return NULL;
 	}
@@ -864,7 +877,7 @@
 {
 	struct ast_sorcery_object_wizard *object_wizard = obj;
 
-	if (object_wizard->data) {
+	if (object_wizard->data && object_wizard->wizard->callbacks.close) {
 		object_wizard->wizard->callbacks.close(object_wizard->data);
 	}
 
@@ -875,9 +888,73 @@
 	ao2_cleanup(object_wizard->wizard);
 }
 
-/*! \brief Internal function which creates an object type and adds a wizard mapping */
-enum ast_sorcery_apply_result __ast_sorcery_apply_wizard_mapping(struct ast_sorcery *sorcery,
-		const char *type, const char *module, const char *name, const char *data, unsigned int caching)
+/*! \brief Return the number of wizards mapped to an object type */
+int ast_sorcery_get_wizard_mapping_count(struct ast_sorcery *sorcery,
+	const char *type)
+{
+	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
+
+	if (!object_type) {
+		return -1;
+	}
+
+	return AST_VECTOR_SIZE(&object_type->wizards);
+}
+
+int ast_sorcery_get_wizard_mapping(struct ast_sorcery *sorcery,
+	const char *type, int index, struct ast_sorcery_wizard **wizard, void **data)
+{
+	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
+	struct ast_sorcery_object_wizard *owizard;
+
+	if (!object_type) {
+		return -1;
+	}
+
+	if (index < 0 || index >= AST_VECTOR_SIZE(&object_type->wizards)) {
+		return -1;
+	}
+
+	owizard = AST_VECTOR_GET(&object_type->wizards, index);
+
+	if (wizard != NULL) {
+		*wizard = &(owizard->wizard->callbacks);
+		ao2_bump(owizard->wizard);
+	} else {
+		return -1;
+	}
+
+	if (data != NULL) {
+		*data = owizard->data;
+	}
+
+	return 0;
+}
+
+/*! \brief Internal function removes a wizard mapping */
+int __ast_sorcery_remove_wizard_mapping(struct ast_sorcery *sorcery,
+		const char *type, const char *module, const char *name)
+{
+	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
+	int res;
+
+	if (!object_type) {
+		return -1;
+	}
+
+	AST_VECTOR_RW_WRLOCK(&object_type->wizards);
+#define WIZARD_NAME_COMPARE(a, b) (strcmp((a)->wizard->callbacks.name, (b)) == 0)
+	res = AST_VECTOR_REMOVE_CMP_ORDERED(&object_type->wizards, name, WIZARD_NAME_COMPARE, ao2_cleanup);
+#undef WIZARD_NAME_COMPARE
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
+
+	return res;
+}
+
+/*! \brief Internal function which creates an object type and inserts a wizard mapping */
+enum ast_sorcery_apply_result __ast_sorcery_insert_wizard_mapping(struct ast_sorcery *sorcery,
+		const char *type, const char *module, const char *name, const char *data,
+		unsigned int caching, int position)
 {
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
 	RAII_VAR(struct ast_sorcery_internal_wizard *, wizard, ao2_find(wizards, name, OBJ_KEY), ao2_cleanup);
@@ -899,19 +976,23 @@
 		created = 1;
 	}
 
+	AST_VECTOR_RW_WRLOCK(&object_type->wizards);
 	if (!created) {
-		struct ast_sorcery_wizard *found;
+		struct ast_sorcery_object_wizard *found;
 
-		found = ao2_find(object_type->wizards, wizard, OBJ_SEARCH_OBJECT);
+#define WIZARD_COMPARE(a, b) ((a)->wizard == (b))
+		found = AST_VECTOR_GET_CMP(&object_type->wizards, wizard, WIZARD_COMPARE);
+#undef WIZARD_COMPARE
 		if (found) {
 			ast_debug(1, "Wizard %s already applied to object type %s\n",
 					wizard->callbacks.name, object_type->name);
-			ao2_cleanup(found);
+			AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 			return AST_SORCERY_APPLY_DUPLICATE;
 		}
 	}
 
 	if (wizard->callbacks.open && !(object_wizard->data = wizard->callbacks.open(data))) {
+		AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 		return AST_SORCERY_APPLY_FAIL;
 	}
 
@@ -920,7 +1001,16 @@
 	object_wizard->wizard = ao2_bump(wizard);
 	object_wizard->caching = caching;
 
-	ao2_link(object_type->wizards, object_wizard);
+	if (position == AST_SORCERY_WIZARD_POSITION_LAST) {
+		position = AST_VECTOR_SIZE(&object_type->wizards);
+	}
+
+	if (AST_VECTOR_INSERT_AT(&object_type->wizards, position, object_wizard) != 0) {
+		AST_VECTOR_RW_UNLOCK(&object_type->wizards);
+		return AST_SORCERY_APPLY_FAIL;
+	}
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
+	ao2_bump(object_wizard);
 
 	if (created) {
 		ao2_link(sorcery->types, object_type);
@@ -930,6 +1020,14 @@
 		sorcery->module_name, sorcery, type, &wizard->callbacks, data, object_wizard->data);
 
 	return AST_SORCERY_APPLY_SUCCESS;
+}
+
+/*! \brief Internal function which creates an object type and adds a wizard mapping */
+enum ast_sorcery_apply_result __ast_sorcery_apply_wizard_mapping(struct ast_sorcery *sorcery,
+		const char *type, const char *module, const char *name, const char *data, unsigned int caching)
+{
+	return __ast_sorcery_insert_wizard_mapping(sorcery, type, module, name, data,
+		caching, AST_SORCERY_WIZARD_POSITION_LAST);
 }
 
 enum ast_sorcery_apply_result  __ast_sorcery_apply_config(struct ast_sorcery *sorcery, const char *name, const char *module)
@@ -1260,7 +1358,9 @@
 	NOTIFY_INSTANCE_OBSERVERS(details->sorcery->observers, object_type_loading,
 		details->sorcery->module_name, details->sorcery, type->name, details->reload);
 
-	ao2_callback(type->wizards, OBJ_NODATA, sorcery_wizard_load, details);
+	AST_VECTOR_RW_RDLOCK(&type->wizards);
+	AST_VECTOR_CALLBACK(&type->wizards, sorcery_wizard_load, NULL, details, 0);
+	AST_VECTOR_RW_UNLOCK(&type->wizards);
 
 	NOTIFY_INSTANCE_OBSERVERS(details->sorcery->observers, object_type_loaded,
 		details->sorcery->module_name, details->sorcery, type->name, details->reload);
@@ -1700,31 +1800,31 @@
 {
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
 	void *object = NULL;
-	struct ao2_iterator i;
-	struct ast_sorcery_object_wizard *wizard;
+	int i;
 	unsigned int cached = 0;
 
 	if (!object_type || ast_strlen_zero(id)) {
 		return NULL;
 	}
 
-	i = ao2_iterator_init(object_type->wizards, 0);
-	for (; (wizard = ao2_iterator_next(&i)); ao2_ref(wizard, -1)) {
+	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
+	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+		struct ast_sorcery_object_wizard *wizard =
+			AST_VECTOR_GET(&object_type->wizards, i);
+
 		if (wizard->wizard->callbacks.retrieve_id &&
 			!(object = wizard->wizard->callbacks.retrieve_id(sorcery, wizard->data, object_type->name, id))) {
 			continue;
 		}
 
 		cached = wizard->caching;
-
-		ao2_ref(wizard, -1);
 		break;
 	}
-	ao2_iterator_destroy(&i);
 
 	if (!cached && object) {
-		ao2_callback(object_type->wizards, 0, sorcery_cache_create, object);
+		AST_VECTOR_CALLBACK(&object_type->wizards, sorcery_cache_create, NULL, object, 0);
 	}
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object;
 }
@@ -1733,8 +1833,7 @@
 {
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
 	void *object = NULL;
-	struct ao2_iterator i;
-	struct ast_sorcery_object_wizard *wizard;
+	int i;
 	unsigned int cached = 0;
 
 	if (!object_type) {
@@ -1748,9 +1847,11 @@
 		}
 	}
 
-	/* Inquire with the available wizards for retrieval */
-	i = ao2_iterator_init(object_type->wizards, 0);
-	for (; (wizard = ao2_iterator_next(&i)); ao2_ref(wizard, -1)) {
+	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
+	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+		struct ast_sorcery_object_wizard *wizard =
+			AST_VECTOR_GET(&object_type->wizards, i);
+
 		if ((flags & AST_RETRIEVE_FLAG_MULTIPLE)) {
 			if (wizard->wizard->callbacks.retrieve_multiple) {
 				wizard->wizard->callbacks.retrieve_multiple(sorcery, wizard->data, object_type->name, object, fields);
@@ -1767,15 +1868,14 @@
 
 		cached = wizard->caching;
 
-		ao2_ref(wizard, -1);
 		break;
 	}
-	ao2_iterator_destroy(&i);
 
 	/* If we are returning a single object and it came from a non-cache source create it in any caches */
 	if (!(flags & AST_RETRIEVE_FLAG_MULTIPLE) && !cached && object) {
-		ao2_callback(object_type->wizards, 0, sorcery_cache_create, object);
+		AST_VECTOR_CALLBACK(&object_type->wizards, sorcery_cache_create, NULL, object, 0);
 	}
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object;
 }
@@ -1784,22 +1884,24 @@
 {
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
 	struct ao2_container *objects;
-	struct ao2_iterator i;
-	struct ast_sorcery_object_wizard *wizard;
+	int i;
 
 	if (!object_type || !(objects = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 1, NULL, NULL))) {
 		return NULL;
 	}
 
-	i = ao2_iterator_init(object_type->wizards, 0);
-	for (; (wizard = ao2_iterator_next(&i)); ao2_ref(wizard, -1)) {
+	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
+	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+		struct ast_sorcery_object_wizard *wizard =
+			AST_VECTOR_GET(&object_type->wizards, i);
+
 		if (!wizard->wizard->callbacks.retrieve_regex) {
 			continue;
 		}
 
 		wizard->wizard->callbacks.retrieve_regex(sorcery, wizard->data, object_type->name, objects, regex);
 	}
-	ao2_iterator_destroy(&i);
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return objects;
 }
@@ -1846,7 +1948,9 @@
 {
 	const struct ast_sorcery_object_details *details = object;
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, details->object->type, OBJ_KEY), ao2_cleanup);
-	RAII_VAR(struct ast_sorcery_object_wizard *, object_wizard, NULL, ao2_cleanup);
+	struct ast_sorcery_object_wizard *object_wizard = NULL;
+	struct ast_sorcery_object_wizard *found_wizard;
+	int i;
 	struct sorcery_details sdetails = {
 		.sorcery = sorcery,
 		.obj = object,
@@ -1856,14 +1960,21 @@
 		return -1;
 	}
 
-	if ((object_wizard = ao2_callback(object_type->wizards, 0, sorcery_wizard_create, &sdetails)) &&
-		ao2_container_count(object_type->observers)) {
-		struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
+	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
+	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+		found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
+		if (sorcery_wizard_create(found_wizard, &sdetails, 0) == (CMP_MATCH | CMP_STOP)) {
+			object_wizard = found_wizard;
+			if(ao2_container_count(object_type->observers)) {
+				struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
 
-		if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_create, invocation)) {
-			ao2_cleanup(invocation);
+				if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_create, invocation)) {
+					ao2_cleanup(invocation);
+				}
+			}
 		}
 	}
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object_wizard ? 0 : -1;
 }
@@ -1905,7 +2016,9 @@
 {
 	const struct ast_sorcery_object_details *details = object;
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, details->object->type, OBJ_KEY), ao2_cleanup);
-	RAII_VAR(struct ast_sorcery_object_wizard *, object_wizard, NULL, ao2_cleanup);
+	struct ast_sorcery_object_wizard *object_wizard = NULL;
+	struct ast_sorcery_object_wizard *found_wizard;
+	int i;
 	struct sorcery_details sdetails = {
 		.sorcery = sorcery,
 		.obj = object,
@@ -1915,14 +2028,21 @@
 		return -1;
 	}
 
-	if ((object_wizard = ao2_callback(object_type->wizards, 0, sorcery_wizard_update, &sdetails)) &&
-		ao2_container_count(object_type->observers)) {
-		struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
+	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
+	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+		found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
+		if (sorcery_wizard_update(found_wizard, &sdetails, 0) == (CMP_MATCH | CMP_STOP)) {
+			object_wizard = found_wizard;
+			if (ao2_container_count(object_type->observers)) {
+				struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
 
-		if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_update, invocation)) {
-			ao2_cleanup(invocation);
+				if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_update, invocation)) {
+					ao2_cleanup(invocation);
+				}
+			}
 		}
 	}
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object_wizard ? 0 : -1;
 }
@@ -1964,7 +2084,9 @@
 {
 	const struct ast_sorcery_object_details *details = object;
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, details->object->type, OBJ_KEY), ao2_cleanup);
-	RAII_VAR(struct ast_sorcery_object_wizard *, object_wizard, NULL, ao2_cleanup);
+	struct ast_sorcery_object_wizard *object_wizard = NULL;
+	struct ast_sorcery_object_wizard *found_wizard;
+	int i;
 	struct sorcery_details sdetails = {
 		.sorcery = sorcery,
 		.obj = object,
@@ -1974,14 +2096,21 @@
 		return -1;
 	}
 
-	if ((object_wizard = ao2_callback(object_type->wizards, 0, sorcery_wizard_delete, &sdetails)) &&
-		ao2_container_count(object_type->observers)) {
-		struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
+	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
+	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+		found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
+		if (sorcery_wizard_delete(found_wizard, &sdetails, 0) == (CMP_MATCH | CMP_STOP)) {
+			object_wizard = found_wizard;
+			if (ao2_container_count(object_type->observers)) {
+				struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
 
-		if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_delete, invocation)) {
-			ao2_cleanup(invocation);
+				if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_delete, invocation)) {
+					ao2_cleanup(invocation);
+				}
+			}
 		}
 	}
+	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object_wizard ? 0 : -1;
 }
diff --git a/tests/test_sorcery.c b/tests/test_sorcery.c
index ce04c62..b6b3d09 100644
--- a/tests/test_sorcery.c
+++ b/tests/test_sorcery.c
@@ -179,6 +179,19 @@
 /*! \brief Global scope observer structure for testing */
 static struct sorcery_test_observer observer;
 
+static void *wizard2_data;
+
+static void *sorcery_test_open(const char *data)
+{
+	wizard2_data = (void *)data;
+	return wizard2_data;
+}
+
+static void sorcery_test_close(void *data)
+{
+
+}
+
 static int sorcery_test_create(const struct ast_sorcery *sorcery, void *data, void *object)
 {
 	cache.created = 1;
@@ -204,9 +217,19 @@
 	return 0;
 }
 
-/*! \brief Dummy sorcery wizard, not actually used so we only populate the name and nothing else */
+/*! \brief Dummy sorcery wizards, not actually used so we only populate the name and nothing else */
 static struct ast_sorcery_wizard test_wizard = {
 	.name = "test",
+	.create = sorcery_test_create,
+	.retrieve_id = sorcery_test_retrieve_id,
+	.update = sorcery_test_update,
+	.delete = sorcery_test_delete,
+};
+
+static struct ast_sorcery_wizard test_wizard2 = {
+	.name = "test2",
+	.open = sorcery_test_open,
+	.close = sorcery_test_close,
 	.create = sorcery_test_create,
 	.retrieve_id = sorcery_test_retrieve_id,
 	.update = sorcery_test_update,
@@ -3340,6 +3363,111 @@
 	return AST_TEST_PASS;
 }
 
+AST_TEST_DEFINE(wizard_apply_and_insert)
+{
+	RAII_VAR(struct ast_sorcery *, sorcery, NULL, ast_sorcery_unref);
+	RAII_VAR(struct ast_sorcery_wizard *, wizard1, &test_wizard, ast_sorcery_wizard_unregister);
+	RAII_VAR(struct ast_sorcery_wizard *, wizard2, &test_wizard2, ast_sorcery_wizard_unregister);
+	RAII_VAR(struct ast_sorcery_wizard *, wizard, NULL, ao2_cleanup);
+	void *data;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "wizard_apply_and_insert";
+		info->category = "/main/sorcery/";
+		info->summary = "sorcery wizard apply and insert test";
+		info->description =
+			"sorcery wizard apply and insert test";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	wizard1->load = sorcery_test_load;
+	wizard1->reload = sorcery_test_load;
+
+	wizard2->load = sorcery_test_load;
+	wizard2->reload = sorcery_test_load;
+
+	if (!(sorcery = ast_sorcery_open())) {
+		ast_test_status_update(test, "Failed to open a sorcery instance\n");
+		return AST_TEST_FAIL;
+	}
+
+	ast_sorcery_wizard_register(wizard1);
+	ast_sorcery_wizard_register(wizard2);
+
+	/* test_object_type isn't registered yet so count should return error */
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping_count(sorcery, "test_object_type") == -1);
+
+	ast_sorcery_apply_default(sorcery, "test_object_type", "test", NULL);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping_count(sorcery, "test_object_type") == 1);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", 0, &wizard, NULL) == 0);
+	ast_test_validate(test, strcmp("test", wizard->name) == 0);
+	ao2_ref(wizard, -1);
+	wizard = NULL;
+
+	ast_test_validate(test,
+		ast_sorcery_insert_wizard_mapping(sorcery, "test_object_type", "test2", "test2data", 0, 0) == 0);
+
+	ast_test_validate(test,
+		ast_sorcery_insert_wizard_mapping(sorcery, "test_object_type", "test2", "test2data", 0, 0) != 0);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", 0, &wizard, &data) == 0);
+	ast_test_validate(test, strcmp("test2", wizard->name) == 0);
+	ast_test_validate(test, strcmp("test2data", data) == 0);
+	ao2_ref(wizard, -1);
+	wizard = NULL;
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", 1, &wizard, NULL) == 0);
+	ast_test_validate(test, strcmp("test", wizard->name) == 0);
+	ao2_ref(wizard, -1);
+	wizard = NULL;
+
+	/* Test failures */
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "non-existent-type", 0, &wizard, NULL) != 0);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", -1, &wizard, &data) != 0);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", 2, &wizard, NULL) != 0);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", 2, NULL, NULL) != 0);
+
+	/* Test remove */
+	/* Should fail */
+	ast_test_validate(test,
+		ast_sorcery_remove_wizard_mapping(sorcery, "non-existent-type", "somewizard") != 0);
+	ast_test_validate(test,
+		ast_sorcery_remove_wizard_mapping(sorcery, "test_object_type", "somewizard") != 0);
+
+	/* should work */
+	ast_test_validate(test,
+		ast_sorcery_remove_wizard_mapping(sorcery, "test_object_type", "test") == 0);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping_count(sorcery, "test_object_type") == 1);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping(sorcery, "test_object_type", 0, &wizard, &data) == 0);
+	ast_test_validate(test, strcmp("test2", wizard->name) == 0);
+	ast_test_validate(test, strcmp("test2data", data) == 0);
+	ao2_ref(wizard, -1);
+	wizard = NULL;
+
+	return AST_TEST_PASS;
+}
+
 static int unload_module(void)
 {
 	AST_TEST_UNREGISTER(wizard_registration);
@@ -3390,12 +3518,14 @@
 	AST_TEST_UNREGISTER(global_observation);
 	AST_TEST_UNREGISTER(instance_observation);
 	AST_TEST_UNREGISTER(wizard_observation);
+	AST_TEST_UNREGISTER(wizard_apply_and_insert);
 
 	return 0;
 }
 
 static int load_module(void)
 {
+	AST_TEST_REGISTER(wizard_apply_and_insert);
 	AST_TEST_REGISTER(wizard_registration);
 	AST_TEST_REGISTER(sorcery_open);
 	AST_TEST_REGISTER(apply_default);

-- 
To view, visit https://gerrit.asterisk.org/395
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9d2469a9296b2698082c0989e25e6848dc403b57
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>



More information about the asterisk-code-review mailing list