<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
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:
<blockquote cite="mid:1334153590.12925.17.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>
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>
</body>
</html>