From 7e77815ad1f88d2edb076e4786af7a43198740a2 Mon Sep 17 00:00:00 2001 From: George Joseph Date: Thu, 14 Mar 2019 10:46:53 -0600 Subject: [PATCH] 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 the ability to allow an object type to have multiple wizards of the same type. This is indicated when a wizard is added to a specific object type. Added 3 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 and allot-duplicates flags 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 capabilities of ast_sorcery_object_type_insert_wizard. * ast_sorcery_object_type_remove_wizard which removes a wizard matching both its name and its original argument string. * 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 --- include/asterisk/sorcery.h | 163 +++++++++++++++++++++++++++++++++++++ main/sorcery.c | 126 +++++++++++++++++++++------- tests/test_sorcery.c | 76 +++++++++++++++++ 3 files changed, 336 insertions(+), 29 deletions(-) diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h index bafca5fb6b..3a0e1b81a5 100644 --- a/include/asterisk/sorcery.h +++ b/include/asterisk/sorcery.h @@ -561,6 +561,169 @@ enum ast_sorcery_apply_result __ast_sorcery_insert_wizard_mapping(struct ast_sor __ast_sorcery_insert_wizard_mapping((sorcery), (type), AST_MODULE, (name), (data), \ (caching), (position)) +/*! + * \brief Wizard Apply Flags + * + * These flags apply only to a wizard/object-type combination. + * The same wizard may be applied to a different object-type with + * different flags and behavior. If ALLOW_DUPLICATE is set + * the wizard could even be applied to the same object-type + * with different flags. + */ +enum ast_sorcery_wizard_apply_flags { + /*! Apply no special behavior */ + AST_SORCERY_WIZARD_APPLY_NONE = (0 << 0), + /*! This wizard will cache this object type's entries */ + AST_SORCERY_WIZARD_APPLY_CACHING = (1 << 0), + /*! This wizard won't allow Create, Update or Delete operations on this object type */ + AST_SORCERY_WIZARD_APPLY_READONLY = (1 << 1), + /*! This wizard will allow other instances of itself on the same object type */ + AST_SORCERY_WIZARD_APPLY_ALLOW_DUPLICATE = (1 << 2) +}; + +/*! + * \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_type_name Name of the wizard type to use + * \param wizard_args Opaque string to be passed to the wizard + * May be NULL but see note below + * \param flags One or more of enum ast_sorcery_wizard_apply_flags + * \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 + * + * \note Although \ref wizard_args is an optional parameter it is highly + * recommended to supply one. If you use the AST_SORCERY_WIZARD_APPLY_ALLOW_DUPLICATE + * flag, and you intend to ever remove a wizard mapping, you'll need wizard_args + * to remove specific instances of a wizard type. + */ +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_type_name, + const char *wizard_args, enum ast_sorcery_wizard_apply_flags flags, int position, + struct ast_sorcery_wizard **wizard, void **wizard_data); + +/*! + * \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 wizard_type_name Name of the wizard type to use + * \param wizard_args Opaque string to be passed to the wizard + * May be NULL but see note below + * \param flags One or more of enum ast_sorcery_wizard_apply_flags + * \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 + * + * \note Although \ref wizard_args is an optional parameter it is highly + * recommended to supply one. If you use the AST_SORCERY_WIZARD_APPLY_ALLOW_DUPLICATE + * flag, and you intend to ever remove a wizard mapping, you'll need wizard_args + * to remove specific instances. + */ +#define ast_sorcery_object_type_insert_wizard(sorcery, \ + object_type_name, wizard_type_name, wizard_args, flags, \ + position, wizard, wizard_data) \ + __ast_sorcery_object_type_insert_wizard((sorcery), \ + (object_type_name), AST_MODULE, (wizard_type_name), (wizard_args), (flags), \ + position, (wizard), (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_type_name Name of the wizard type to use + * \param wizard_args Opaque string to be passed to the wizard + * May be NULL but see note below + * \param flags One or more of enum ast_sorcery_wizard_apply_flags + * \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 + * + * \note Although \ref wizard_args is an optional parameter it is highly + * recommended to supply one. If you use the AST_SORCERY_WIZARD_APPLY_ALLOW_DUPLICATE + * flag, and you intend to ever remove a wizard mapping, you'll need wizard_args + * to remove specific instances. + */ +#define ast_sorcery_object_type_apply_wizard(sorcery, \ + object_type_name, wizard_type_name, wizard_args, flags, \ + wizard, wizard_data) \ + __ast_sorcery_object_type_insert_wizard((sorcery), \ + (object_type_name), AST_MODULE, (wizard_type_name), (wizard_args), (flags), \ + AST_SORCERY_WIZARD_POSITION_LAST, (wizard), (wizard_data)) + +/*! + * \brief Remove an object wizard mapping + * \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 remove from + * \param module The name of the module, typically AST_MODULE + * \param wizard_type_name The name of the of the wizard type to remove + * \param wizard_args Opaque string originally passed to the wizard + * + * \retval 0 success + * \retval -1 failure + * + * \note If there were multiple instances of the same wizard type + * added to this object type without using \ref wizard_args, then + * only the first wizard matching wizard_type will be removed. + */ +int __ast_sorcery_object_type_remove_wizard(struct ast_sorcery *sorcery, + const char *object_type_name, const char *module, const char *wizard_type_name, + const char *wizard_args); + +/*! + * \brief Remove an object wizard mapping + * \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 remove from + * \param wizard_type_name The name of the of the wizard type to remove + * \param wizard_args Opaque string originally passed to the wizard + * + * \retval 0 success + * \retval -1 failure + * + * \note If there were multiple instances of the same wizard type + * added to this object type without using \ref wizard_args, then + * only the first wizard matching wizard_type will be removed. + */ +#define ast_sorcery_object_type_remove_wizard(sorcery, object_type_name, \ + wizard_type_name, wizard_args) \ + __ast_sorcery_object_type_remove_wizard((sorcery), (object_type_name), \ + AST_MODULE, (wizard_type_name), (wizard_args)) + /*! * \brief Remove an object wizard mapping * diff --git a/main/sorcery.c b/main/sorcery.c index 8e14881998..d837845711 100644 --- a/main/sorcery.c +++ b/main/sorcery.c @@ -109,6 +109,15 @@ struct ast_sorcery_object_wizard { /*! \brief Wizard is acting as an object cache */ unsigned int caching:1; + + /*! \brief Wizard is read_only */ + unsigned int read_only:1; + + /*! \brief Wizard allows others of the same type */ + unsigned int allow_duplicates:1; + + /*! \brief Wizard arguments */ + char wizard_args[0]; }; /*! \brief Interface for a sorcery object type wizards */ @@ -802,6 +811,35 @@ int ast_sorcery_get_wizard_mapping(struct ast_sorcery *sorcery, return 0; } +int __ast_sorcery_object_type_remove_wizard(struct ast_sorcery *sorcery, + const char *object_type_name, const char *module, const char *wizard_type_name, + const char *wizard_args) +{ + RAII_VAR(struct ast_sorcery_object_type *, object_type, + ao2_find(sorcery->types, object_type_name, OBJ_SEARCH_KEY), ao2_cleanup); + int res = -1; + int i; + + if (!object_type) { + return res; + } + + AST_VECTOR_RW_WRLOCK(&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 (strcmp(wizard->wizard->callbacks.name, wizard_type_name) == 0 + && strcmp(S_OR(wizard->wizard_args, ""), S_OR(wizard_args, "")) == 0) { + ao2_cleanup(AST_VECTOR_REMOVE_ORDERED(&object_type->wizards, i)); + res = 0; + break; + } + } + AST_VECTOR_RW_UNLOCK(&object_type->wizards); + + return res; +} + /*! \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) @@ -822,29 +860,35 @@ int __ast_sorcery_remove_wizard_mapping(struct ast_sorcery *sorcery, 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_type_name, + const char *wizard_args, enum ast_sorcery_wizard_apply_flags flags, 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_wizard *, object_wizard, ao2_alloc(sizeof(*object_wizard), sorcery_object_wizard_destructor), 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_type_name, OBJ_KEY), ao2_cleanup); + RAII_VAR(struct ast_sorcery_object_wizard *, object_wizard, NULL, ao2_cleanup); int created = 0; + object_wizard = ao2_alloc(sizeof(*object_wizard) + + (ast_strlen_zero(wizard_args) ? 0 : strlen(wizard_args) + 1), + sorcery_object_wizard_destructor); + if (!object_wizard) { return AST_SORCERY_APPLY_FAIL; } - if (!wizard || wizard->callbacks.module != ast_module_running_ref(wizard->callbacks.module)) { - 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 + || internal_wizard->callbacks.module != ast_module_running_ref(internal_wizard->callbacks.module)) { + ast_log(LOG_ERROR, + "Wizard '%s' could not be applied to object type '%s' as it was not found\n", + wizard_type_name, object_type_name); return AST_SORCERY_APPLY_FAIL; } if (!object_type) { - if (!(object_type = sorcery_object_type_alloc(type, module))) { - ast_module_unref(wizard->callbacks.module); + if (!(object_type = sorcery_object_type_alloc(object_type_name, module))) { + ast_module_unref(internal_wizard->callbacks.module); return AST_SORCERY_APPLY_FAIL; } created = 1; @@ -855,29 +899,34 @@ enum ast_sorcery_apply_result __ast_sorcery_insert_wizard_mapping(struct ast_sor 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) { + if (found + && !((flags & AST_SORCERY_WIZARD_APPLY_ALLOW_DUPLICATE) || found->allow_duplicates)) { 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); - ast_module_unref(wizard->callbacks.module); + ast_module_unref(internal_wizard->callbacks.module); 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_type_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_type_name, object_type->name, S_OR(wizard_args, "")); AST_VECTOR_RW_UNLOCK(&object_type->wizards); - ast_module_unref(wizard->callbacks.module); + ast_module_unref(internal_wizard->callbacks.module); return AST_SORCERY_APPLY_FAIL; } - object_wizard->wizard = ao2_bump(wizard); - object_wizard->caching = caching; + object_wizard->wizard = ao2_bump(internal_wizard); + object_wizard->caching = !!(flags & AST_SORCERY_WIZARD_APPLY_CACHING); + object_wizard->read_only = !!(flags & AST_SORCERY_WIZARD_APPLY_READONLY); + if (wizard_args) { + strcpy(object_wizard->wizard_args, wizard_args); /* Safe */ + } if (position == AST_SORCERY_WIZARD_POSITION_LAST) { position = AST_VECTOR_SIZE(&object_type->wizards); @@ -895,17 +944,36 @@ enum ast_sorcery_apply_result __ast_sorcery_insert_wizard_mapping(struct ast_sor } 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 *object_type_name, const char *module, const char *wizard_type_name, + const char *wizard_args, unsigned int caching, int position) +{ + return __ast_sorcery_object_type_insert_wizard(sorcery, object_type_name, module, wizard_type_name, + wizard_args, caching ? AST_SORCERY_WIZARD_APPLY_CACHING : AST_SORCERY_WIZARD_APPLY_NONE, + 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) + const char *object_type_name, const char *module, const char *wizard_type_name, + const char *wizard_args, unsigned int caching) { - return __ast_sorcery_insert_wizard_mapping(sorcery, type, module, name, data, - caching, AST_SORCERY_WIZARD_POSITION_LAST); + return __ast_sorcery_insert_wizard_mapping(sorcery, object_type_name, module, wizard_type_name, + wizard_args, 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) @@ -1904,7 +1972,7 @@ struct ao2_container *ast_sorcery_retrieve_by_prefix(const struct ast_sorcery *s /*! \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; } @@ -2015,7 +2083,7 @@ static int sorcery_observers_notify_update(void *data) /*! \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; } @@ -2103,7 +2171,7 @@ static int sorcery_observers_notify_delete(void *data) /*! \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 0662336957..9f5cc7e8e8 100644 --- a/tests/test_sorcery.c +++ b/tests/test_sorcery.c @@ -3504,6 +3504,13 @@ AST_TEST_DEFINE(wizard_apply_and_insert) ast_test_validate(test, ast_sorcery_insert_wizard_mapping(sorcery, "test_object_type", "test2", "test2data", 0, 0) != 0); + ast_test_validate(test, + ast_sorcery_object_type_insert_wizard(sorcery, "test_object_type", "test2", "test2data2", + AST_SORCERY_WIZARD_APPLY_ALLOW_DUPLICATE, 0, NULL, NULL) == 0); + + ast_test_validate(test, + ast_sorcery_object_type_remove_wizard(sorcery, "test_object_type", "test2", "test2data2") == 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); @@ -3554,6 +3561,73 @@ AST_TEST_DEFINE(wizard_apply_and_insert) 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", AST_SORCERY_WIZARD_APPLY_READONLY, &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); @@ -3606,6 +3680,7 @@ static int unload_module(void) 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; } @@ -3662,6 +3737,7 @@ static int load_module(void) 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; }