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

Anca Vamanu anca.vamanu at 1and1.ro
Wed Apr 11 16:30:24 CEST 2012


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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20120411/18aacdeb/attachment.htm>


More information about the sr-dev mailing list