<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Peter,<br>
<br>
Unfortunately I have not worked at this until now. If you can do it
in the next period, it would be great. If I can help you with
anything, please ask.<br>
<br>
Regards,<br>
Anca<br>
<br>
<br>
<br>
On 05/31/2012 12:21 PM, Peter Dunkley wrote:
<blockquote cite="mid:1338456066.2508.1.camel@pd-laptop-linux"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="GENERATOR" content="GtkHTML/4.2.3">
Hi Anca,<br>
<br>
Did you get any time to look at this?<br>
<br>
If you didn't I might have a chance over the next couple of days.<br>
<br>
Regards,<br>
<br>
Peter<br>
<br>
On Wed, 2012-04-11 at 17:30 +0300, Anca Vamanu wrote:<br>
<blockquote type="CITE"> Hi Peter,<br>
<br>
<br>
Thanks for the feedback. <br>
<br>
I agree with extra parameters for pres_refresh_watchers(), so if
type is 2 it can receive two optional parameters: file URL and
file name.<br>
And also get_rules_doc function in presence_xml module has to be
changed to be able to do include int the query file URL is
given.<br>
<br>
I also have a busy schedule in the next period (with Easter and
some holidays :) ), but I will try to implement this change in
the next period. I don't think we need to worry about the
freeze, as this is actually a fix, not a new feature. <br>
<br>
Regards,<br>
Anca<br>
<br>
<br>
On 04/11/2012 05:13 PM, Peter Dunkley wrote: <br>
<blockquote type="CITE"> Hi Anca,<br>
<br>
I hadn't realised this was already in presence_xml. I have
added some comments below.<br>
<br>
I would like to make these changes, but it could be a couple
of weeks before I have time. Given that there is a freeze
coming up in two weeks what do you think should be done in the
short-term?<br>
<br>
On Wed, 2012-04-11 at 16:55 +0300, Anca Vamanu wrote:<br>
<blockquote type="CITE"> The good think about my solution was
that I left the XCAP document fetch part in <b>presence_xml</b>
module. I think it is better like this because the <b>presence</b>
module remains independent of the XCAP part. And more than
this, it works for both integrated and not integrated XCAP
server ( because it uses the generic API of fetching XCAP
documents) and even for any event (not only presence). <br>
<br>
</blockquote>
I can see that presence_xml is a more logical place to put
some of this implementation.<br>
<br>
<blockquote type="CITE"> Here I like better that you have
fetched the document and stored it in presentity table with
expires -1.<br>
<br>
As for the fact that you added a new function to be called
from the script when to fetch and store this document
(pres_update_presentity), I think it would have been better
to use the existing <b>pres_refresh_watchers</b> function.
If you look in the documentation, actually when type!= 0 , a
pidf manipulation document change might be expected:<br>
<br>
<a moz-do-not-send="true"
href="http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_k/presence/README;h=25de125997927321c63938e5a013f93c10b86379;hb=refs/heads/master">http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_k/presence/README;h=25de125997927321c63938e5a013f93c10b86379;hb=refs/heads/master</a><br>
<br>
The reason why it is better to use this function is that it
is actually called by the refereshWatchers MI command and we
will maintain the compatibility with other external XCAP
servers that use this MI command. We can also put a specific
type, let's say type=2 for pidf manipulation document change
and type=1 for presentity table change.<br>
</blockquote>
I do agree with this.<br>
<br>
<blockquote type="CITE"> When <b>pres_refresh_watchers</b> is
called with<b> </b>type=<b>2</b> , try to fetch the pidf
manipulation document from xcap server and if found, store
it in presentity table with expires=-1 (as you have done).
The only thing that I would change is to use a wrapper over
<b>get_rules_doc</b> function from presence_xml module. We
can do this adding a new field in the pres_ev_t structure
(exactly as get_rules_doc field).<br>
<br>
Then continue with the query_db_notify(pres, ev, NULL)
function which will also be called if type=1.<br>
<br>
And to maintain compatibility for refereshWatchers function
we can call from here pres_referesh_watchers with type=2,
considering that also a pidf manipulation document <br>
</blockquote>
This makes sense to me.<br>
<br>
<blockquote type="CITE"> The only thing that I don't know is
how to keep from your implementation is the possibility for
a user to have multiple pidf documents. I saw that you use
the URL to fetch from xcap table and the file name as ETAG
to offer the possibility of storing multiple pidf documents.
However my question is actually whether this use case is
actually possible? Why would a user have more permanent
state documents at the same time?<br>
</blockquote>
I did this because I know (from experience) that many
different clients use slightly different file names for the
documents they store in XCAP. This doesn't matter so much for
pres-rules, avatars, user-profiles, and resource-lists because
the XML content of the documents is still the same regardless
of name. With presentities it is a bit different. This is
because different clients can (and do) use different XML nodes
for things like <busy/>, <available/>,
<vacation/>, as well as supporting and displaying
different fields like emoticons, notes, URLs. This means that
although different clients that support hard-state will both
generate PIDF XML the fields within those XML documents could
vary widely.<br>
<br>
Supporting multiple documents for a subscriber at least leaves
open the possibility that someone who uses different clients
(one on Windows at work, one on Linux at home, perhaps)
wouldn't be in the situation of having one client simply
dropping stuff the other had PUBLISHed.<br>
<br>
For example, the presence client I tested this development
with (which doesn't support hard-state anyway - I had to use
curl to upload the document) didn't support the
<vacation/> status from RFC 4827, and therefore didn't
display it (I had to confirm the NOTIFYs were correct with a
Wireshark trace).<br>
<br>
Could this ETag stuff be supported by giving an extra
(optional) parameter to pres_refresh_watchers() called ETag?<br>
<br>
Peter<br>
<br>
<table cellpadding="0" cellspacing="0" width="100%">
<tbody>
<tr>
<td>
<pre>--
Peter Dunkley
Technical Director
Crocodile RCS Ltd
</pre>
</td>
</tr>
</tbody>
</table>
</blockquote>
<br>
</blockquote>
<br>
<table cellpadding="0" cellspacing="0" width="100%">
<tbody>
<tr>
<td>
<pre>--
Peter Dunkley
Technical Director
Crocodile RCS Ltd
</pre>
</td>
</tr>
</tbody>
</table>
</blockquote>
<br>
</body>
</html>