<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/3305/">https://reviewboard.asterisk.org/r/3305/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Minor issue below.</pre>
<br />
<div>
<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/3305/diff/3/?file=55576#file55576line311" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/main/sorcery.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 int sorcery_wizard_cmp(void *obj, void *arg, int flags)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">311</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">static</span> <span class="kt">int</span> <span class="nf">object_type_field_hash</span><span class="p">(</span><span class="k">const</span> <span class="kt">void</span> <span class="o">*</span><span class="n">obj</span><span class="p">,</span> <span class="k">const</span> <span class="kt">int</span> <span class="n">flags</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This function is using spaces and not tabs.
Copying directly from the wiki pages always give spaces even though I put it up there with tabs.</pre>
</div>
<br />
<p>- rmudgett</p>
<br />
<p>On March 6th, 2014, 2:17 p.m. CST, Mark Michelson 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 and Joshua Colp.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated March 6, 2014, 2:17 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;">When testing out realtime for PJSIP, I encountered a problem. In my database, I had defined additional columns beyond the ones that Asterisk cared about. As a result, when data was pulled from the database, the config framework barfed, saying it didn't know what these object fields were. For config files, this practice is fantastic. It makes sure to raise a giant red flag to people that they are attempting to configure something that is not recognized by Asterisk. For realtime, on the other hand, it is legitimate for a database to contain supplemental data alongside the data Asterisk cares about.
What I have done is to modify res_sorcery_realtime.c to filter out fields from the retrieved objectset that have not been registered with sorcery. In addition, I have created two realtime sorcery unit tests. The first one simply creates data in a realtime backend and then has sorcery atttempt to retrieve the data, ensuring that an object is allocated with the expected data. The second test is similar, except that in addition to the data that has been registered with sorcery, an additional field is created in the realtime backend. This test ensures that the data can still be retrieved and successfully allocated by sorcery.
Of all things in this review, the thing I'm concerned about most is the log message that has been added to indicate that an object has been filtered out of the resultset retrieved from realtime. I initally wanted to make this a NOTICE-level message, but I was afraid that with the frequency with which objects are retrieved from realtime, this could spam the logs a little too much. On the other hand, leaving it as a level 1 debug message I have now could make it difficult for someone that has misconfigured their database (say they accidentally have a typo in a column name) from seeing that data they expect to be relevant is being ignored by Asterisk. Any suggestions on how to handle this?</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;">All realtime sorcery unit tests pass with flying colors.</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>/branches/12/tests/test_sorcery_realtime.c <span style="color: grey">(410010)</span></li>
<li>/branches/12/res/res_sorcery_realtime.c <span style="color: grey">(410010)</span></li>
<li>/branches/12/main/sorcery.c <span style="color: grey">(410010)</span></li>
<li>/branches/12/include/asterisk/sorcery.h <span style="color: grey">(410010)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3305/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>