[OpenSIPS-Devel] [ opensips-Patches-3162656 ] Presence Dialog-Info enhancements

[SourceForge.net]

Patches item #3162656, was opened at 2011-01-20 13:14
Message generated for change (Comment added) made by nobody
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=3162656&group_id=232389

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: trunk
Status: Closed
Resolution: Accepted
Priority: 5
Private: No
Submitted By: Vallimamod Abdullah (vabdulla)
Assigned to: Anca Vamanu (anca_vamanu)
Summary: Presence Dialog-Info enhancements

Initial Comment:
Here is a patch that enhance the presence_dialoginfo behaviour as we have encountered
severe limitations when we deployed the BLF feature to our customers. By now, the
presence_dialoginfo module sends a PUBLISH on a per-dialog basis. So we have no
global visibility of the entity's status. For example if the entity is involved
in multiple calls, only the last call status is PUBLISHed.

I have already reported this issue on the past and the proposed fix was to set
the dialog-info state attribute to "partial" in the NOTIFY. But this is not
handled correctly by some phones.

So the solution I have found is to retrieve all dialogs where the entity is
involved and to build the PUBLISH dialog-info body with all these dialogs
instead of only the current one.

My patch is in 3 parts:

1) An extension to the dialog module API to add get_dlg_lst_by_uri() function
that traverses the dialog hash to retrieve all dialogs where the uri appears as
caller or callee.

2) A change in pua_dialoginfo in the way the PUBLISH body is built: a
dialog node is added for each dialog retrieved by the previous api function.
This simplifies greatly the dialog callback function. I have also added a
process_body function to set the version attribute. The function was originally
written by Anca Vamanu.

3) Finally, I have refactored the agregate_xmls() function in
presence_dialoginfo module to simplify the way the NOTIFY body is built and to
correct the force_single_dialog handling.

This patch is in early stage. I have done some tests and it looks stable but I
have no idea how it will work under load as we need to traverse all dialog list
everytime a state is PUBLISHed. I am available to discuss on all potential
issues that I am not currently aware of.

Regards,
Vallimamod Abdullah
.



----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2011-06-18 21:27

Message:
9A4FDe  <a href="http://hgkqglvifdjf.com/">hgkqglvifdjf</a>,
[url=http://fgvmjdnbfdow.com/]fgvmjdnbfdow[/url],
[link=http://lqtnxkphoykv.com/]lqtnxkphoykv[/link],
http://izyfynwpoucu.com/

----------------------------------------------------------------------

Comment By: Anca Vamanu (anca_vamanu)
Date: 2011-01-25 08:41

Message:
Hi Vallimamod,

Yes, for the case of using another presence server it might not like that
strange version. However, it should also be able to update the version per
subscription. 
I am thinking to make this configurable - so if OpenSIPS presence server
is used, not to do that unnecessary processing. I will make this change and
commit your patch.
Thanks you very much.

Regards,
Anca

----------------------------------------------------------------------

Comment By: Vallimamod Abdullah (vabdulla)
Date: 2011-01-24 17:42

Message:
Hi Anca,

You are right, the actual version of the NOTIFY is set in
presence_dialoginfo, so it is not really necessary to set it in pua. But I
was thinking of the case when the presence server is completely
independant. In this case, all PUBLISH sent by the pua module contain
version="00000000000", which is not really helpful ;-)

Regards,
Vallimamod
.


----------------------------------------------------------------------

Comment By: Anca Vamanu (anca_vamanu)
Date: 2011-01-24 14:21

Message:
Hi Vallimamod,

That's great, thanks!
I have looked through the patch again and there is still something that I
fully understand - why do you need an aux body processing function to set
the version in pua module? The design was made so that there is a function
to set the version define in presence_dialoginfo and this is called before
each Notify is sent and the version is taken from the subscribe dialog - in
this way a correctly increasing version is received by each UA. 
Why have you put also a function in pua? Did you observe a case where it
helped doing this?

Thanks and regards,
Anca

----------------------------------------------------------------------

Comment By: Vallimamod Abdullah (vabdulla)
Date: 2011-01-21 21:43

Message:
Hi Anca,

You were right, the change you said solved my problem !
I have reworked my original patch to strip the unecessary dialog api
related stuff. You'll find attached the updated version that include
correct version setting of publish messages and the refactoring of
agregate_xmls().

Regards,
Vallimamod
.



----------------------------------------------------------------------

Comment By: Anca Vamanu (anca_vamanu)
Date: 2011-01-21 14:09

Message:
Hi Vallimamod,

I have found where the problem is. The pua_dialoginfo module actually
publishes all the dialogs for a certain entity in the same publication
record. SO, in presence server we will only have the last updated dialog.
See pua_dialoginfo/dialog_publish.c +316. The id of the publication is a
static string used for all - "dialog_publish". To have more records for an
entity - one record per dialog we should set this id ( that is used by pua
module for matching) to something dialog specific - callid per example.
Change line: 
"publ.id = dlginfo_id;"
with 
"publ.id=*callid;"
and check if this solves your problem.

Regards,
Anca

----------------------------------------------------------------------

Comment By: Vallimamod Abdullah (vabdulla)
Date: 2011-01-20 18:27

Message:
Hi Anca,

In all the tests I have made, only the last published information was
available to agregate_xmls() function to build the notify for a given
entity. Maybe I have overlooked something and there is an actual bug there
?

Regards,
Vallimamod


----------------------------------------------------------------------

Comment By: Anca Vamanu (anca_vamanu)
Date: 2011-01-20 17:36

Message:
Hi Vallimamod,

I don't understand why you need point 1) and 2) from your patch. Why do
you want to send Publish with all the info? You already should have the
other dialogs published when they were modified. Aggregation when sending
the notify should be enough, isn't it?

Regards,
Anca

----------------------------------------------------------------------

Comment By: Anca Vamanu (anca_vamanu)
Date: 2011-01-20 13:18

Message:
Hi Vallimamod,

Thank you, I will review your patch.

Regads,
Anca

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=3162656&group_id=232389