10 Apr
2012
10 Apr
'12
7:09 p.m.
Module: sip-router
Branch: master
Commit: 37812cef5fb1ee2022592de24bbae48352e17524
URL:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=37812ce…
Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com>
Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com>
Date: Tue Apr 10 18:05:10 2012 +0100
modules_k/presence: RFC 4827 (presence hard-state) support
- Hard-state presence documents are stored as pidf-manipulation
documents in the integrated XCAP server.
- When one of these documents is put/deleted/changed it can
be "published" using the new pres_update_presentity() exported
function.
- Because the original document is in XCAP a client can download
it and manipulate it directly.
- Hard-state documents have an expiry time of -1 and never expire
(the clean function in presence has been updated to make sure of
this).
- The filename of the document is used as the ETag value in the
presentity table. This enables multiple hard-state documents
(with different filenames) to be uploaded for each subscriber.
- Hard-state is useful for permanently setting an avatar, or an
out-of-office message, etc.
---
modules_k/presence/README | 302 ++++++++++++++++++-----------
modules_k/presence/doc/presence_admin.xml | 99 ++++++++++
modules_k/presence/presence.c | 128 ++++++++++++-
modules_k/presence/presence.h | 5 +
modules_k/presence/presentity.c | 87 ++++++---
modules_k/presence/publish.c | 200 +++++++++++++++++++-
modules_k/presence/publish.h | 2 +-
7 files changed, 679 insertions(+), 144 deletions(-)
Diff:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=378…
11 Apr
11 Apr
3:55 p.m.
New subject: git:master: modules_k/presence: RFC 4827 (presence hard-state) support
Hi Peter,
Nice work with this patch. I have looked through it and I have some
observations.
First of all, an implementation of this RFC already existed even in
Kamailio :) . I knew I coded it, but was not sure if before or after the
split, but I looked now and it is present in kamailio also. In the
README check after pidf_manipulation:
http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_…
The implementation that I've done is very different from yours. In some
aspects I like your solution better. Analyzing both, I thought at a way
of combining them and making the best solution for this. In any case, I
don't think it is ok to keep them both in this state as it will be a bit
confusing for someone reading the documentation.
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).
The code for fetching the file is at line *473*:
http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_…
The bad thing about my implementation was that I put the fetching of
this file when aggregating the presence documents. So a query in xcap
table ( for integrated xcap server) was actually performed each time a
Notify was sent out.
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_…
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.
Considering this the solution that I see best is the following:
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
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?
Let me know your thoughts about this.
Regards,
Anca
On 04/10/2012 08:09 PM, Peter Dunkley wrote:
Module: sip-router
Branch: master
Commit: 37812cef5fb1ee2022592de24bbae48352e17524
URL:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=37812ce…
Author: Peter Dunkley<peter.dunkley(a)crocodile-rcs.com>
Committer: Peter Dunkley<peter.dunkley(a)crocodile-rcs.com>
Date: Tue Apr 10 18:05:10 2012 +0100
modules_k/presence: RFC 4827 (presence hard-state) support
- Hard-state presence documents are stored as pidf-manipulation
documents in the integrated XCAP server.
- When one of these documents is put/deleted/changed it can
be "published" using the new pres_update_presentity() exported
function.
- Because the original document is in XCAP a client can download
it and manipulate it directly.
- Hard-state documents have an expiry time of -1 and never expire
(the clean function in presence has been updated to make sure of
this).
- The filename of the document is used as the ETag value in the
presentity table. This enables multiple hard-state documents
(with different filenames) to be uploaded for each subscriber.
- Hard-state is useful for permanently setting an avatar, or an
out-of-office message, etc.
---
modules_k/presence/README | 302 ++++++++++++++++++-----------
modules_k/presence/doc/presence_admin.xml | 99 ++++++++++
modules_k/presence/presence.c | 128 ++++++++++++-
modules_k/presence/presence.h | 5 +
modules_k/presence/presentity.c | 87 ++++++---
modules_k/presence/publish.c | 200 +++++++++++++++++++-
modules_k/presence/publish.h | 2 +-
7 files changed, 679 insertions(+), 144 deletions(-)
Diff:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=378…
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
4:13 p.m.
New subject: git:master: modules_k/presence: RFC 4827 (presence hard-state) support
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_…
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
4:30 p.m.
New subject: git:master: modules_k/presence: RFC 4827 (presence hard-state) support
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_…
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
31 May
31 May
11:21 a.m.
New subject: git:master: modules_k/presence: RFC 4827 (presence hard-state) support
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:
--
Peter Dunkley
Technical Director
Crocodile RCS Ltd
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_…
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
2:24 p.m.
New subject: git:master: modules_k/presence: RFC 4827 (presence hard-state) support
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:
--
Peter Dunkley
Technical Director
Crocodile RCS Ltd
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_…
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
2:29 p.m.
New subject: git:master: modules_k/presence: RFC 4827 (presence hard-state) support
I'm looking at it now. Should be able to get it done today or tomorrow.
Peter
On Thu, 2012-05-31 at 15:24 +0300, Anca Vamanu wrote:
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:
--
Peter Dunkley
Technical Director
Crocodile RCS Ltd
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:
--
Peter Dunkley
Technical Director
Crocodile RCS Ltd 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_…
>
> 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
4348
days inactive
4399
days old
6 comments
2 participants
participants (2)
-
Anca Vamanu -
Peter Dunkley