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

[Anca Vamanu]

Hi Peter,

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.

Regards,
Anca



On 05/31/2012 12:21 PM, Peter Dunkley wrote:
> 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/8c1d6c07/attachment-0001.htm>