[asterisk-dev] [Code Review] 4033: manager/config: Enhancements to support templates and non-unique category names via AMI

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4033/#review13428
-----------------------------------------------------------

Ship it!


My "Ship it!" is a cautious one. Basically, the unit tests are good evidence to me that things are working as expected, but when changes are being made to such a fundamental area of code as the config API, I feel like a second "ship it!" from someone else should occur before actually committing.

Also, I think a discussion needs to occur with regards to this patch's inclusion in Asterisk 12.


branches/12/tests/test_config.c
<https://reviewboard.asterisk.org/r/4033/#comment23952>

    For all of the snprintf() calls in this test, use sizeof(temp) instead of 32.


- Mark Michelson


On Sept. 30, 2014, 9:24 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4033/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2014, 9:24 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch provides the capability to manipulate templates and categories with non-unique names via AMI.
> 
> Summary of changes:
> 
> GetConfig and GetConfigJSON:
> Added "Filter" parameter:  A comma separated list of name_regex=value_regex expressions which will cause only categories whose variables match all expressions to be considered.  The special variable name TEMPLATES can be used to control whether templates are included.  Passing 'include' as the value will include templates along with normal categories. Passing 'restrict' as the value will restrict the operation to ONLY templates.  Not specifying a TEMPLATES expression results in the current default behavior which is to not include templates.
> 
> Examples:
> "GetConfig?Filename=pjsip.conf&Category=myitsp&Filter=type=aor" would return only 'myitsp' categories with a type of 'aor'.
> 
> "GetConfig?Filename=pjsip.conf&Category=itsp-template&Filter=TEMPLATES=restrict,type=aor" would return only 'itsp-template' categories that are actually templates with a type of 'aor'.
> 
> "GetConfig?Filename=pjsip.conf&Filter=type=registration,endpoint=myitsp" would return only registrations whose corresponding endpoint is 'myitsp'.
> 
> The output from GetConfig and GetConfigJSON also includes variables indicating if the returned category is a template and the template names a category inherits from if any.
> 
> UpdateConfig:
> NewCat now includes options for allowing duplicate category names, indicating if the category should be created as a template, and specifying templates the category should inherit from.  The rest of the actions now accept a filter string as defined above.  If there are non-unique category names, you can now update specific ones based on variable values.
> 
> To facilitate the new capabilities in manager, corresponding changes had to be made to config, most notably the addition of filter criteria to many of the APIs.  In some cases it was easy to change the references to use the new prototype but others would have required touching too many files for this patch so a wrapper with the original prototype was created.  Macros couldn't be used in this case because it would break binary compatibility with modules such as res_digium_phone that are linked to real symbols.
> 
> 
> Diffs
> -----
> 
>   branches/12/tests/test_sorcery_realtime.c 424175 
>   branches/12/tests/test_sorcery.c 424175 
>   branches/12/tests/test_config.c 424175 
>   branches/12/res/res_sorcery_realtime.c 424175 
>   branches/12/res/res_sorcery_config.c 424175 
>   branches/12/pbx/pbx_realtime.c 424175 
>   branches/12/main/manager.c 424175 
>   branches/12/main/config.c 424175 
>   branches/12/include/asterisk/config.h 424175 
>   branches/12/apps/app_voicemail.c 424175 
>   branches/12/apps/app_directory.c 424175 
> 
> Diff: https://reviewboard.asterisk.org/r/4033/diff/
> 
> 
> Testing
> -------
> 
> Testsuite before and after runs gave the same results.
> A set of unit tests for config was added to test_config.  They test basic, filtered and template operations.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141001/0d642518/attachment-0001.html>