----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3254/#review10971 -----------------------------------------------------------
branches/12/funcs/func_sorcery.c <https://reviewboard.asterisk.org/r/3254/#comment20599> This might not be correct. branches/12/funcs/func_sorcery.c <https://reviewboard.asterisk.org/r/3254/#comment20600> Same here. branches/12/funcs/func_sorcery.c <https://reviewboard.asterisk.org/r/3254/#comment20601> "separator | occurrence_number" is an odd name for a parameter. How about "retrieval_details"? branches/12/funcs/func_sorcery.c <https://reviewboard.asterisk.org/r/3254/#comment20602> Move the setting of defaults for separator and field_number to the variable definitions. branches/12/funcs/func_sorcery.c <https://reviewboard.asterisk.org/r/3254/#comment20603> This should bail since "occurrence_number must be a positive integer" and can be combined with the previous conditional. branches/12/main/config.c <https://reviewboard.asterisk.org/r/3254/#comment20606> Put this in the init section of the for loop. branches/12/main/sorcery.c <https://reviewboard.asterisk.org/r/3254/#comment20608> Why not use the newly created ast_variable_list_append() here? - opticron On Feb. 23, 2014, 3:48 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3254/ > ----------------------------------------------------------- > > (Updated Feb. 23, 2014, 3:48 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-22537 > https://issues.asterisk.org/jira/browse/ASTERISK-22537 > > > Repository: Asterisk > > > Description > ------- > > This patch creates the AST_SORCERY dialplan function which allows someone to > retrieve any value from a sorcery-based config file. It's similar to > AST_CONFIG. > > The creation of the function itself was fairly straightforward but it > required changes to the underlying sorcery infrastructure that rippled into > individual sorcery objects. The changes stemmed from inconsistencies in how > sorcery created ast_variable objectsets from sorcery objects and the > inconsistency in how individual objects used that feature especially when it > came to parameters that can be specified multiple times like contact in aor > and match in identify. You can read more here... > http://lists.digium.com/pipermail/asterisk-dev/2014-February/065202.html > > So, what this patch does, besides actually creating the AST_SORCERY function, > is the following... > > Creates ast_variable_list_append which is a helper to append one ast_variable > list to another. > Modifies the ast_sorcery_object_field_register functions to accept the > already-defined sorcery_fields_handler callback. > Modifies ast_sorcery_objectset_create to accept a parameter indicating return > type preference...a single ast_variable with all values concatenated or an > ast_variable list with multiple entries. Also fixed a few bugs. > Modifies individual sorcery object implementations to use the new function > definition of the ast_sorcery_object_field_register functions. > Modifies location.c and res_pjsip_endpoint_identifier_ip.c to implement > sorcery_fields_handler handlers so they return multiple occurrences as an > ast_variable_list. > Added a whole bunch of tests to test_sorcery. > > NOTES: > > If you read through the email chain, I had a question on whether we > should/could change "Contacts" to "Contact" in AMI output. Joshua though we > should, which I agree with, but there is the issue of backward compatibility. > What is did therefore was to put a little translation code in to keep it > plural for 12, then if you guys agree, remove that code in trunk/13 so it's > singular. I'll have to do a new patch for trunk anyway because > ast_sip_auth_array changed to a vector in trunk. > > > Diffs > ----- > > branches/12/tests/test_sorcery.c 408869 > branches/12/res/res_pjsip_outbound_registration.c 408869 > branches/12/res/res_pjsip_endpoint_identifier_ip.c 408869 > branches/12/res/res_pjsip_acl.c 408869 > branches/12/res/res_pjsip/pjsip_configuration.c 408869 > branches/12/res/res_pjsip/location.c 408869 > branches/12/res/res_pjsip/config_transport.c 408869 > branches/12/res/res_pjsip/config_auth.c 408869 > branches/12/main/sorcery.c 408869 > branches/12/main/config.c 408869 > branches/12/main/bucket.c 408869 > branches/12/include/asterisk/sorcery.h 408869 > branches/12/include/asterisk/config.h 408869 > branches/12/funcs/func_sorcery.c PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/3254/diff/ > > > Testing > ------- > > Created a new test in test_sorcery for the dialplan function which has the > side effect of also checking the changes to sorcery for multiple values. > Ran testsuite tests/channels/pjsip to check sorcery functions and AMI output. > > *CLI> test execute category /main/sorcery/ > Running all available tests matching category /main/sorcery/ > START /main/sorcery/ - dialplan_function > END /main/sorcery/ - dialplan_function Time: <1ms Result: PASS > START /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all > <snip> > 44 Test(s) Executed 44 Passed 0 Failed > > > testsuite$ python -u ./runtests.py -t tests/channels/pjsip | ./pretty_print > Running tests for Asterisk 12.1.0 > Test > Run/Pass/Fail [ Status ] > tests/channels/pjsip/handle_options_request > 1/ 1/ 0 [ Passed ] > tests/channels/pjsip/incoming_calls_without_auth > 2/ 2/ 0 [ Passed ] > <snip> > tests/channels/pjsip/ami/show_endpoints > 57/ 57/ 0 [ Passed ] > tests/channels/pjsip/ami/show_endpoint > 58/ 58/ 0 [ Passed ] > tests/channels/pjsip/ami/show_registrations_inbound > 59/ 59/ 0 [ Passed ] > tests/channels/pjsip/ami/show_registrations_outbound > 60/ 60/ 0 [ Passed ] > tests/channels/pjsip/ami/show_subscriptions > 61/ 61/ 0 [ Passed ] > tests/channels/pjsip/set_var > 62/ 62/ 0 [ Passed ] > tests/channels/pjsip/hold/run-test > 63/ 62/ 1 [ Failed ] > tests/channels/pjsip/presence_pidf > 64/ 63/ 1 [ Passed ] > tests/channels/pjsip/presence_xpidf > 65/ 64/ 1 [ Passed ] > tests/channels/pjsip/mwi > 66/ 65/ 1 [ Passed ] > Tests: 66 Passed: 65 Failed: 1 > FAILED: tests/channels/pjsip/hold/run-test > > The failed one was failing before any changes. Not sure why. > > > Thanks, > > George Joseph > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
