[sr-dev] git:master: modules_k/presence: RFC 4827 (presence hard-state) support

[Peter Dunkley]

Hi Anca,

Did you get any time to look at this?

If you didn't I might have a chance over the next couple of days.

Regards,

Peter

On Wed, 2012-04-11 at 17:30 +0300, Anca Vamanu wrote:

> Hi Peter,
> 
> 
> Thanks for the feedback. 
> 
> 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.
> 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.
> 
> 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. 
> 
> Regards,
> Anca
> 
> 
> On 04/11/2012 05:13 PM, Peter Dunkley wrote: 
> 
> > Hi Anca,
> > 
> > I hadn't realised this was already in presence_xml.  I have added
> > some comments below.
> > 
> > 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?
> > 
> > On Wed, 2012-04-11 at 16:55 +0300, Anca Vamanu wrote:
> > 
> > > The good think about my solution was that I left the XCAP document
> > > fetch part in presence_xml module. I think it is better like this
> > > because the presence 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). 
> > > 
> > 
> > I can see that presence_xml is a more logical place to put some of
> > this implementation.
> > 
> > 
> > > Here I like better that you have fetched the document and stored
> > > it in presentity table with expires -1.
> > > 
> > > 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 pres_refresh_watchers function. If you look in the
> > > documentation, actually when type!= 0 , a pidf manipulation
> > > document change might be expected:
> > > 
> > > 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
> > > 
> > > 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.
> > 
> > I do agree with this.
> > 
> > 
> > > When pres_refresh_watchers is called with type=2 , 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
> > > get_rules_doc 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).
> > > 
> > > Then continue with the query_db_notify(pres, ev, NULL) function
> > > which will also be called if type=1.
> > > 
> > > 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 
> > 
> > This makes sense to me.
> > 
> > 
> > > 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?
> > 
> > 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.
> > 
> > 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.
> > 
> > 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).
> > 
> > Could this ETag stuff be supported by giving an extra (optional)
> > parameter to pres_refresh_watchers() called ETag?
> > 
> > Peter
> > 
> > -- 
> > Peter Dunkley
> > Technical Director
> > Crocodile RCS Ltd
> 
> 


-- 
Peter Dunkley
Technical Director
Crocodile RCS Ltd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20120531/e99ebd90/attachment.htm>