[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