<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/3061/">https://reviewboard.asterisk.org/r/3061/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 10th, 2013, 2:20 p.m. UTC, <b>Joshua Colp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3061/diff/1/?file=49246#file49246line228" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/res/res_sorcery_astdb.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void sorcery_astdb_retrieve_multiple(const struct ast_sorcery *sorcery, void *data, const char *type, struct ao2_container *objects, const struct ast_variable *fields)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">228</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">regex</span><span class="p">[</span><span class="mi">0</span><span class="p">]</span> <span class="o">==</span> <span class="sc">'^'</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">228</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="p">(</span><span class="n">entries</span> <span class="o">=</span> <span class="n">ast_db_gettree</span><span class="p">(</span><span class="n">family</span><span class="p">,</span> <span class="nb">NULL</span><span class="p">))</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">229</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">snprintf</span><span class="p">(</span><span class="n">tree</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">tree</span><span class="p">),</span> <span class="s">"%s%%"</span><span class="p">,</span> <span class="n">regex</span> <span class="o">+</span> <span class="mi">1</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">229</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="o">||</span> <span class="n">regcomp</span><span class="p">(</span><span class="o">&</span><span class="n">expression</span><span class="p">,</span> <span class="n">regex</span><span class="p">,</span> <span class="n">REG_EXTENDED</span> <span class="o">|</span> <span class="n">REG_NOSUB</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">230</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="p">}</span> <span class="k">else</span> <span class="p">{</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">231</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">tree</span><span class="p">[</span><span class="mi">0</span><span class="p">]</span> <span class="o">=</span> <span class="sc">'\0'</span><span class="p">;</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">232</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="p">}</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">233</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">234</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="p">(</span><span class="n">entries</span> <span class="o">=</span> <span class="n">ast_db_gettree</span><span class="p">(</span><span class="n">family</span><span class="p">,</span> <span class="n">tree</span><span class="p">))</span> <span class="o">||</span> <span class="n">regcomp</span><span class="p">(</span><span class="o">&</span><span class="n">expression</span><span class="p">,</span> <span class="n">regex</span><span class="p">,</span> <span class="n">REG_EXTENDED</span> <span class="o">|</span> <span class="n">REG_NOSUB</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">235</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">return</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">230</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">return</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">236</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">231</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'll explain why I disagree with this change.
While you gain full regex functionality it comes at a HUGE cost. Reading in an entire family creates a list of all entries, including the contents. For every retrieve. In something like the PJSIP registrar this can grow to be quite big - and due to the way the registrar works you can retrieve multiple times while processing a REGISTER.
This all adds up.
Fundamentally this is because this is going through an abstraction layer (astdb) instead of directly to the database, but despite that we should keep performance and penalties in mind.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Reverted. Just a behavioral difference I found in the code when working on the regex code. The change *only* affects the use of the '^' anchor character. The anchor character is not a big loss anyway.
I just left it in the second diff because I didn't want to get rid of it right away.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On December 19th, 2013, 6:49 p.m. UTC, rmudgett wrote:</p>
<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 rmudgett.</div>
<p style="color: grey;"><i>Updated Dec. 19, 2013, 6:49 p.m.</i></p>
<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 can be broken into three main components:
1) The core resource support for external MWI providers are in the following files:
apps/app_voicemail.c,
include/asterisk/res_mwi_external.h,
configs/sorcery.conf.sample,
res/res_mwi_external.c, and
res/res_mwi_external.exports.in.
* The core external MWI resource provides for MWI message counts persistence using sorcery. With sorcery, the user is able to configure which sorcery wizzard backend to use if the default astdb is not desired.
* The core external MWI resoruce provides some CLI commands to manually manage the MWI counts if needed.
The new CLI commands are:
"mwi external delete all",
"mwi external delete like <regex>",
"mwi external delete mailbox <mailbox>",
"mwi external list all",
"mwi external list like <regex>",
"mwi external show mailbox <mailbox>", and
"mwi external update mailbox <mailbox> [<new> [<old>]]".
2) The AMI component of external MWI is in:
res/res_mwi_external_ami.c
* The external MWI AMI interface provides a thin wrapper around the core external MWI resource.
The resource adds the following AMI actions:
MWIExternalGet,
MWIExternalDelete, and
MWIExternalUpdate.
3) A consistency fix to sorcery astdb regex record retrieval is in:
res/res_sorcery_astdb.c
* The AstDB wizzard is inconsistent with the other sorcery wizzards with its regex record selection. Any pattern that uses the '^' regex character to anchor the pattern to the beginning of the key *must* be a simple prefix. Any wildcard matching used with the '^' anchor will fail. As a result, a regex of "^.*@context$" will not return anything even if there are mailboxes in "context".</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;">Used the CLI "database show" along with the new CLI commands and AMI actions to test adding/updating, getting, and deleting external MWI counts.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/res/res_sorcery_astdb.c <span style="color: grey">(404386)</span></li>
<li>/trunk/res/res_mwi_external_ami.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/res/res_mwi_external.exports.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/res/res_mwi_external.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/include/asterisk/res_mwi_external.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/configs/sorcery.conf.sample <span style="color: grey">(404386)</span></li>
<li>/trunk/apps/app_voicemail.c <span style="color: grey">(404386)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3061/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>