[Asterisk-code-review] sorcery.c: Sorcery enhancements for wizard management (...asterisk[13])

George Joseph asteriskteam at digium.com
Thu Mar 14 12:02:06 CDT 2019


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/11154


Change subject: sorcery.c: Sorcery enhancements for wizard management
......................................................................

sorcery.c: Sorcery enhancements for wizard management

Added ability to specifiy a wizard is read-only when applying
it to a specific object type.  This allows you to specify
create, update and delete callbacks for the wizard but limit
which object types can use them.

Added 2 new sorcery wizard functions:

* ast_sorcery_object_type_insert_wizard which does the same thing
  as the existing ast_sorcery_insert_wizard_mapping function but
  accepts the new read-only flag and also returns the
  ast_sorcery_wizard structure used and it's internal data structure.
  This allows immediate use of the wizard's callbacks without
  having to register a "wizard mapped" observer.

* ast_sorcery_object_type_apply_wizard which does the same
  thing as the existing ast_sorcery_apply_wizard_mapping function
  but has the added same capabilities of
  ast_sorcery_object_type_insert_wizard.

* The original logic in __ast_sorcery_insert_wizard_mapping was moved
  to __ast_sorcery_object_type_insert_wizard and enhanced for the
  new capabilities, then __ast_sorcery_insert_wizard_mapping was
  refactored to just call __ast_sorcery_insert_wizard_mapping.

* Added a unit test to test_sorcery.c to test the read-only
  capability.

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



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/54/11154/1

diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h
index 73ed060..dab6853 100644
--- a/include/asterisk/sorcery.h
+++ b/include/asterisk/sorcery.h
@@ -570,6 +570,61 @@
 		(caching), (position))
 
 /*!
+ * \brief Insert an additional object wizard mapping at a specific position
+ * in the wizard list returning wizard information
+ * \since 13.26.0
+ * \since 16.3.0
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param object_type_name Name of the object type to apply to
+ * \param module The name of the module, typically AST_MODULE
+ * \param wizard_name Name of the wizard to use
+ * \param wizard_args Opaque string to be passed to the wizard
+ * \param caching Wizard should cache
+ * \param read_only Wizard should be read-only
+ * \param position An index to insert to or one of ast_sorcery_wizard_position
+ * \param[out] wizard A variable to receive a pointer to the ast_sorcery_wizard structure.
+ * 				May be NULL if not needed.
+ * \param[out] wizard_data A variable to receive a pointer to the wizard's internal data.
+ * 				May be NULL if not needed.
+ *
+ * \return What occurred when applying the mapping
+ *
+ * \note This should be called *after* applying default mappings
+ */
+enum ast_sorcery_apply_result __ast_sorcery_object_type_insert_wizard(struct ast_sorcery *sorcery,
+	const char *object_type_name, const char *module, const char *wizard_name,
+	const char *wizard_args, unsigned int caching, unsigned int read_only, int position,
+	struct ast_sorcery_wizard **wizard, void **wizard_data);
+
+/*!
+ * \brief Apply additional object wizard mappings returning wizard information
+ * \since 13.26.0
+ * \since 16.3.0
+ *
+ * \param sorcery Pointer to a sorcery structure
+ * \param object_type_name Name of the object type to apply to
+ * \param wizard_name Name of the wizard to use
+ * \param wizard_args Opaque string to be passed to the wizard
+ * \param caching Wizard should cache
+ * \param read_only Wizard should be read-only
+ * \param[out] wizard A variable to receive a pointer to the ast_sorcery_wizard structure.
+ * 				May be NULL if not needed.
+ * \param[out] wizard_data A variable to receive a pointer to the wizard's internal data.
+ * 				May be NULL if not needed.
+ *
+ * \return What occurred when applying the mapping
+ *
+ * \note This should be called *after* applying default mappings
+ */
+#define ast_sorcery_object_type_apply_wizard(sorcery, \
+	object_type_name, wizard_name, wizard_args, caching, read_only, \
+	wizard, wizard_data) \
+	__ast_sorcery_object_type_insert_wizard((sorcery), \
+		(object_type_name), AST_MODULE, (wizard_name), (wizard_args), (caching), (read_only), \
+		AST_SORCERY_WIZARD_POSITION_LAST, (wizard), (wizard_data))
+
+/*!
  * \brief Remove an object wizard mapping
  *
  * \param sorcery Pointer to a sorcery structure
diff --git a/main/sorcery.c b/main/sorcery.c
index 3a43fa7..712ac80 100644
--- a/main/sorcery.c
+++ b/main/sorcery.c
@@ -111,6 +111,9 @@
 
 	/*! \brief Wizard is acting as an object cache */
 	unsigned int caching:1;
+
+	/*! \brief Wizard is read_only */
+	unsigned int read_only:1;
 };
 
 /*! \brief Interface for a sorcery object type wizards */
@@ -812,26 +815,27 @@
 	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)
+enum ast_sorcery_apply_result __ast_sorcery_object_type_insert_wizard(struct ast_sorcery *sorcery,
+	const char *object_type_name, const char *module, const char *wizard_name,
+	const char *wizard_args, unsigned int caching, unsigned int read_only, int position,
+	struct ast_sorcery_wizard **wizard, void **wizard_data)
 {
-	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);
+	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, object_type_name, OBJ_KEY), ao2_cleanup);
+	RAII_VAR(struct ast_sorcery_internal_wizard *, internal_wizard, ao2_find(wizards, wizard_name, OBJ_KEY), ao2_cleanup);
 	RAII_VAR(struct ast_sorcery_object_wizard *, object_wizard, ao2_alloc(sizeof(*object_wizard), sorcery_object_wizard_destructor), ao2_cleanup);
 	int created = 0;
 
-	if (!wizard) {
-		ast_log(LOG_ERROR, "Wizard '%s' could not be applied to object type '%s' as it was not found\n",
-			name, type);
+	if (!internal_wizard) {
+		ast_log(LOG_ERROR,
+			"Wizard '%s' could not be applied to object type '%s' as it was not found\n",
+			wizard_name, object_type_name);
 		return AST_SORCERY_APPLY_FAIL;
 	} else if (!object_wizard) {
 		return AST_SORCERY_APPLY_FAIL;
 	}
 
 	if (!object_type) {
-		if (!(object_type = sorcery_object_type_alloc(type, module))) {
+		if (!(object_type = sorcery_object_type_alloc(object_type_name, module))) {
 			return AST_SORCERY_APPLY_FAIL;
 		}
 		created = 1;
@@ -842,29 +846,35 @@
 		struct ast_sorcery_object_wizard *found;
 
 #define WIZARD_COMPARE(a, b) ((a)->wizard == (b))
-		found = AST_VECTOR_GET_CMP(&object_type->wizards, wizard, WIZARD_COMPARE);
+		found = AST_VECTOR_GET_CMP(&object_type->wizards, internal_wizard, WIZARD_COMPARE);
 #undef WIZARD_COMPARE
-		if (found) {
+		/*
+		 * We will allow multiple memory wizards because they typically
+		 * are written to only by the backend implementer and are
+		 * usually inserted at the end of the wizard vector.
+		 */
+		if (found && strcmp("memory", wizard_name)) {
 			ast_debug(1, "Wizard %s already applied to object type %s\n",
-					wizard->callbacks.name, object_type->name);
+				internal_wizard->callbacks.name, object_type->name);
 			AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 			return AST_SORCERY_APPLY_DUPLICATE;
 		}
 	}
 
 	ast_debug(5, "Calling wizard %s open callback on object type %s\n",
-		name, object_type->name);
-	if (wizard->callbacks.open && !(object_wizard->data = wizard->callbacks.open(data))) {
+		wizard_name, object_type->name);
+	if (internal_wizard->callbacks.open && !(object_wizard->data = internal_wizard->callbacks.open(wizard_args))) {
 		ast_log(LOG_WARNING, "Wizard '%s' failed to open mapping for object type '%s' with data: %s\n",
-			name, object_type->name, S_OR(data, ""));
+			wizard_name, object_type->name, S_OR(wizard_args, ""));
 		AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 		return AST_SORCERY_APPLY_FAIL;
 	}
 
-	ast_module_ref(wizard->callbacks.module);
+	ast_module_ref(internal_wizard->callbacks.module);
 
-	object_wizard->wizard = ao2_bump(wizard);
+	object_wizard->wizard = ao2_bump(internal_wizard);
 	object_wizard->caching = caching;
+	object_wizard->read_only = read_only;
 
 	if (position == AST_SORCERY_WIZARD_POSITION_LAST) {
 		position = AST_VECTOR_SIZE(&object_type->wizards);
@@ -882,11 +892,28 @@
 	}
 
 	NOTIFY_INSTANCE_OBSERVERS(sorcery->observers, wizard_mapped,
-		sorcery->module_name, sorcery, type, &wizard->callbacks, data, object_wizard->data);
+		sorcery->module_name, sorcery, object_type_name, &internal_wizard->callbacks, wizard_args,
+		object_wizard->data);
+
+	if (wizard) {
+		*wizard = &internal_wizard->callbacks;
+	}
+	if (wizard_data) {
+		*wizard_data = object_wizard->data;
+	}
 
 	return AST_SORCERY_APPLY_SUCCESS;
 }
 
+/*! \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)
+{
+	return __ast_sorcery_object_type_insert_wizard(sorcery, type, module, name,
+		data, caching, 0, position, NULL, NULL);
+}
+
 /*! \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)
@@ -1873,7 +1900,7 @@
 /*! \brief Internal function which returns if the wizard has created the object */
 static int sorcery_wizard_create(const struct ast_sorcery_object_wizard *object_wizard, const struct sorcery_details *details)
 {
-	if (!object_wizard->wizard->callbacks.create) {
+	if (!object_wizard->wizard->callbacks.create || object_wizard->read_only) {
 		ast_debug(5, "Sorcery wizard '%s' does not support creation\n", object_wizard->wizard->callbacks.name);
 		return 0;
 	}
@@ -1984,7 +2011,7 @@
 /*! \brief Internal function which returns if a wizard has updated the object */
 static int sorcery_wizard_update(const struct ast_sorcery_object_wizard *object_wizard, const struct sorcery_details *details)
 {
-	if (!object_wizard->wizard->callbacks.update) {
+	if (!object_wizard->wizard->callbacks.update || object_wizard->read_only) {
 		ast_debug(5, "Sorcery wizard '%s' does not support updating\n", object_wizard->wizard->callbacks.name);
 		return 0;
 	}
@@ -2072,7 +2099,7 @@
 /*! \brief Internal function which returns if a wizard has deleted the object */
 static int sorcery_wizard_delete(const struct ast_sorcery_object_wizard *object_wizard, const struct sorcery_details *details)
 {
-	if (!object_wizard->wizard->callbacks.delete) {
+	if (!object_wizard->wizard->callbacks.delete || object_wizard->read_only) {
 		ast_debug(5, "Sorcery wizard '%s' does not support deletion\n", object_wizard->wizard->callbacks.name);
 		return 0;
 	}
diff --git a/tests/test_sorcery.c b/tests/test_sorcery.c
index 521d6e8..0c6dfa2 100644
--- a/tests/test_sorcery.c
+++ b/tests/test_sorcery.c
@@ -3468,6 +3468,73 @@
 	return AST_TEST_PASS;
 }
 
+static struct ast_sorcery_wizard test_read_only_wizard = {
+	.name = "test-read-only",
+	.retrieve_id = sorcery_test_retrieve_id,
+};
+
+AST_TEST_DEFINE(wizard_read_only)
+{
+	RAII_VAR(struct ast_sorcery *, sorcery, NULL, ast_sorcery_unref);
+	RAII_VAR(struct ast_sorcery_wizard *, wizard_read_only, &test_read_only_wizard, ast_sorcery_wizard_unregister);
+	RAII_VAR(struct ast_sorcery_wizard *, wizard1, &test_wizard, ast_sorcery_wizard_unregister);
+	RAII_VAR(struct test_sorcery_object *, obj, NULL, ao2_cleanup);
+	struct ast_sorcery_wizard *wizard;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "wizard_read_only";
+		info->category = "/main/sorcery/";
+		info->summary = "sorcery wizard read-only test";
+		info->description =
+			"sorcery wizard read-only test";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	wizard1->load = sorcery_test_load;
+	wizard1->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(wizard_read_only);
+	ast_sorcery_wizard_register(wizard1);
+
+	if ((ast_sorcery_apply_default(sorcery, "test_object_type", "test-read-only", NULL) != AST_SORCERY_APPLY_SUCCESS) ||
+		ast_sorcery_internal_object_register(sorcery, "test_object_type", test_sorcery_object_alloc, NULL, NULL)) {
+		ast_test_status_update(test, "Failed to apply object defaults\n");
+		return AST_TEST_FAIL;
+	}
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping_count(sorcery, "test_object_type") == 1);
+
+	ast_test_validate(test,
+		ast_sorcery_object_type_apply_wizard(sorcery, "test_object_type",
+			"test", "test2data", 0, 1, &wizard, NULL) == 0);
+
+	ast_test_validate(test, strcmp(wizard->name, wizard1->name) == 0);
+
+	ast_test_validate(test,
+		ast_sorcery_get_wizard_mapping_count(sorcery, "test_object_type") == 2);
+
+	if (!(obj = ast_sorcery_alloc(sorcery, "test_object_type", "blah"))) {
+		ast_test_status_update(test, "Failed to allocate a known object type\n");
+		return AST_TEST_FAIL;
+	}
+
+	if (ast_sorcery_create(sorcery, obj) == 0) {
+		ast_test_status_update(test, "Should not have created object using read-only wizard\n");
+		return AST_TEST_FAIL;
+	}
+
+	return AST_TEST_PASS;
+}
+
 static int unload_module(void)
 {
 	AST_TEST_UNREGISTER(wizard_registration);
@@ -3519,6 +3586,7 @@
 	AST_TEST_UNREGISTER(instance_observation);
 	AST_TEST_UNREGISTER(wizard_observation);
 	AST_TEST_UNREGISTER(wizard_apply_and_insert);
+	AST_TEST_UNREGISTER(wizard_read_only);
 
 	return 0;
 }
@@ -3574,6 +3642,7 @@
 	AST_TEST_REGISTER(global_observation);
 	AST_TEST_REGISTER(instance_observation);
 	AST_TEST_REGISTER(wizard_observation);
+	AST_TEST_REGISTER(wizard_read_only);
 
 	return AST_MODULE_LOAD_SUCCESS;
 }

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11154
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I40f35840252e4313d99e11dbd80e270a3aa10605
Gerrit-Change-Number: 11154
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190314/6cef2909/attachment-0001.html>


More information about the asterisk-code-review mailing list