Fixed issues with event subscription regarding on_rx_refresh() and unsubscription

[sauwming]

There are several issues with the event subscription, in particular with the on_rx_refresh() callback and the unsubscription handling.

1. The doc specifies that on_rx_refresh() callback is optional, but it will trigger an assertion in pjsip_evsub_create_uas():

  /* Package MUST implement on_rx_refresh */
  PJ_ASSERT_RETURN(user_cb->on_rx_refresh, PJ_EINVALIDOP);

We will fix the doc in this PR, and also add the missing parameter documentations.

2. A flow issue between SUBSCRIBE response and the NOTIFY.
   Upon incoming SUBSCRIBE, on_rx_refresh() will be called, and the doc says that app MUST send NOTIFY request inside the callback. The callback also provides a parameter p_st_code that will be used to send the response to the incoming SUBSCRIBE after the callback. This means that the NOTIFY will be sent first before the SUBSCRIBE response. While this is unlikely to cause any problem to the remote, which should be able to handle out-of-order messages, it's counterintuitive and goes against the RFC 3265's recommendation in section 3.1.6.2:
   Note that a NOTIFY message is always sent immediately after any 200-class response to a SUBSCRIBE request
   Even our sipp pjsua test uac-subscribe.xml will fail since it expects to receive the SUBSCRIBE response first:

  <send retrans="500">
    <![CDATA[
      SUBSCRIBE sip:[service]@[remote_ip]:[remote_port] SIP/2.0
      ....
    ]]>
  </send>
  <recv response="200" rtd="true"> </recv>
  <recv request="NOTIFY" crlf="true"> </recv>

The proposed fix in this PR is to delay sending the NOTIFY request when we're inside the callback.

3. Incorrect evsub state transition during unsubscription.
   Upon receiving unsubscription request, evsub will set the state to TERMINATED before calling on_rx_refresh():

	/* Save old state.
	 * If application respond with non-2xx, revert to old state.
	 */
	old_state = sub->state;
	old_state_str = sub->state_str;

	if (sub->expires->ivalue == 0) {
	    sub->state = PJSIP_EVSUB_STATE_TERMINATED;
 	    sub->state_str = evsub_state_names[sub->state];
        }

This is most likely done so that when enquiring the subscription state via pjsip_evsub_get_state() inside the callback, the app can easily differentiate between unsubscription (state TERMINATED) and refresh (otherwise). And the state setting is done without calling set_state() because there's a possibility the state can be reverted if the app rejects the SUBSCRIBE by specifying non-200 p_st_code. But this causes several issues:
a. The state never transitions smoothly to TERMINATED (the log will ouput"Subscription state changed ACCEPTED --> ACTIVE" followed by "Subscription state changed TERMINATED --> TERMINATED"). This causes timer to be still active, i.e. the following code in set_state() will never be executed:

    if (state == PJSIP_EVSUB_STATE_TERMINATED &&
	prev_state != PJSIP_EVSUB_STATE_TERMINATED) 
    {
	/* Kill any timer. */
	set_timer(sub, TIMER_TYPE_NONE, 0);

As a result, timer, such as timeout timer, will still be triggered later (invoking on_server_timeout() callback), long after the subscription is deemed to have terminated.
b. When app tries to send the (final) NOTIFY request inside the callback, it's not handled properly. Instead it will get the log: Subscription already terminated for NOTIFY, event=presence;id=

It is quite complicated to fix the issue. The proposed fix in this PR is to always reply with success/200 upon unsubscription and therefore, disallow the state reversion. This is consistent with the behaviour of registration and call, where after sending unregistration or hanging up call, user is not going to stick around much longer and wait until it's accepted.
Behaviour change due to the patch: Upon unsubscription, app will now receive callback on_evsub_state() before on_rx_refresh().

The PR also fixes a bug in uac-subscribe.xml missing the To tag that causes the unsubscription request to be treated as new subscription instead.

[TheQue42]

Will pjsip_evsub_get_termination_reason be properly updated in this flow? I am using the PR branch and I receive an empty string in the call? Since the notify is created "later", I am suspecting that the &sub->term_reason; is not set early enough?

At least I would assume being able to check the termination-reason, as soon as the state == TERMINATED ?

[sauwming]

Okay, I put "timeout" as the reason, as hinted by the RFC:
Note that the NOTIFY messages triggered by SUBSCRIBE messages with "Expires" headers of 0 will contain a "Subscription-State" value of "terminated", and a "reason" parameter of "timeout".

Just to clarify, the NOTIFY creation is not created later, only the sending is delayed. So you can change the default reason when calling pjsip_pres_notify( sub, PJSIP_EVSUB_STATE_TERMINATED, ..., reason, ...) and it will be effective immediately.