<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3254/">https://reviewboard.asterisk.org/r/3254/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By George Joseph.</div>
<p style="color: grey;"><i>Updated Feb. 27, 2014, 1:19 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Addressed 6 of opticron's 7 comments.
opticron, look at the last one and let me know if that makes sense.
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-22537">ASTERISK-22537</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/12/tests/test_sorcery.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip_outbound_registration.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip_endpoint_identifier_ip.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip_acl.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip/pjsip_configuration.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip/location.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip/config_transport.c <span style="color: grey">(409132)</span></li>
<li>branches/12/res/res_pjsip/config_auth.c <span style="color: grey">(409132)</span></li>
<li>branches/12/main/sorcery.c <span style="color: grey">(409132)</span></li>
<li>branches/12/main/config.c <span style="color: grey">(409132)</span></li>
<li>branches/12/main/bucket.c <span style="color: grey">(409132)</span></li>
<li>branches/12/include/asterisk/sorcery.h <span style="color: grey">(409132)</span></li>
<li>branches/12/include/asterisk/config.h <span style="color: grey">(409132)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3254/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>