Skip to main content

Jonas comments [Re: EDR draft updated.]

  • From: binod pg < >
  • To:
  • Subject: Jonas comments [Re: EDR draft updated.]
  • Date: Wed, 17 Apr 2013 17:15:06 +0530
  • Organization: Oracle Corporation

Hi Jonas,

Thanks for the comments.
On 4/17/2013 2:13 AM, Jonas Borjesson wrote:
" type="cite">
My comments on the latest draft (v0.4)

=== Chapter 4 ===

4.2.1 @SipMethod

“an emtpy string matches any SIP method”

Personally I think that an empty string only should match an empty string and not everything. If you have specified a @SipMethod then it must match. If you want to match everything, simply leave it out. Especially since @AnyMethod exists...

4.3 Method specific annotations

Perhaps also want to point out that if new methods will be defined in some currently unknown RFC you can always use the @SipMethod annotation. In fact, perhaps we want to point out (if I understand this correctly) that all of these more explicit annotations such as @Invite are just “short-hand” for using the @SipMethod? It may not be obvious to everyone...

4.3.1 @AnyMethod

Already pointed out in @SipMethod but personally I like being explicit and therefore I like the @AnyMethod over having @SipMethod with an empty string meaning the same thing. Hence, yes to @AnyMethod but no to @SipMethod(“”) meaning the same thing. In fact, I would argue that constructing a @SipMethod with an empty string should lead to an error.

I am fine with that. That would make @AnyMethod not to depend on @SipMethod annotation. Unless anyone disagree, I will
make this change in the EDR version.

" type="cite">

4.4 Response Filtering

Very nice and can’t wait to use it but why not define all “families”. I agree that a generic 4xx-6xx error response annotation is useful but quite often I find myself doing different things even for 4xx vs 5xx etc. Hence, I would suggest to define three new annotations like so:



@GlobalFailureResponse (or perhaps it should be named GlobalError since everything else is called error)

Still keep the @ErrorResponse though to catch all or, depending what people think, remove @ErrorResponse and allow people to use mutiple annotations like so:

@ClientErrorResponse @GlobalFailureResponse

public void handleCertainErrors(SipServletResponse resp) {}

I am fine with adding additional annotations. May be we should also provide
an annotation each for each error response.

However, since the time is short, I would prefer, we add this after EDR, unless anyone disagree.

" type="cite">

4.6 Method Selection Procedure

“as decided in F2F” - remove this comment

Thanks. Fixed in my local version.

" type="cite">

General comment on this section is that even though specifying the selection rule as defined in 4.6.2 is great, it is too complicated and I suspect that most people (including myself) just got a headache reading it (perhaps I don’t have enough brain power but so be it). So, I think explaining what’s going on in plain English with a few examples could be a good thing. I can already envision the amount of confusion from people when it doesn’t evaluate the way they think and how hard it would be to debug this. Also, this goes for container implementors as well and I would suggest a bunch of tests should go into the TCK to ensure that all containers evaluate this chain the same way.

4.6.3 Conflict Resolution

I think I follow and it make sense but once again, I think we need to come up with some examples of how this is done. Not that it matters for this specification but I urge all container implementors to really remember that for a user this will be confusing so to have a clear logging of this selection procedure is going to be super important.

Sure, we can add more examples and more text to explain this. I am not sure, this change will make EDR (by monday) though.

" type="cite">

=== Chapter 7 ===

7.2.5 Dialog Termination

Suggesting to change the names of the AutomaticProcessingListener to “onXXXX”. I don’t feel strongly about it, I just think it reads better...

Also, the name “AutomaticProcessingListener” doesn’t say much what it is supposed to do. Perhaps the following name is a little too long but at least it is a little more obvious to what it is supposed to do: “AutomaticDialogTerminationListener”. But like above, I don’t feel strongly about it...

Regarding allowing the user to modify a message in place before it goes out. Are there any modifications that are not allowed? Just the same stuff that would apply for any other sub-sequent request? NO matter what, let's call it out.

AFAIK, there is no specific restriction. Eric might have some opinion on this.

" type="cite"> Subscribe Dialog

Would like more detail here since we do that for INVITE. How about adding the following:

* The UA acting as a Subscriber cannot explicitly terminate a subscription but can ask the Notifier to delete the subscription by sending a un-subscribe request (sub-sequent subscribe with expires=0). The container is responsible for doing so when asked to terminate the dialog.

* The UA acting as a Notifier will issue a NOTIFY with the Subscription-State header set to “terminated”.

Question: For the Subscriber case, should there be a timeout of how long this dialog will live on for before it gets killed? I.e., even if the container sent a un-subscribe request but the Notifier never (for whatever reason, but or what not) issues a terminating-notify, how long should we wait? Wait for the last known good expire value to expire?

So far, we only mandated the notifier side. But it is a good point, since we should clarify this one way or other. Given the current logic does not really involve timeouts, I guess, the only
thing the container can do is to send the un-subscribe request. If the notify does not come back, the app would rely on usual sipsession invalidation semantics to end the session.

Unless someone disagree that we will make this change in EDR.
" type="cite">

7.4.2 Threading Issues

The text in this section goes against what is later stated in 16.3 SIP Servlet Concurrency. I guess this section needs to be re-written to reflect these new changes and reference section 16.3 for further reading.

Good point. Made that change in my local version.
" type="cite">

=== Chapter 8 ===

8 SIP Servlet Applications

Just a general question: since we are (I think) pushing for not using the sip.xml ala 1.0 wouldn’t it make sense to show all examples in this section using annotations only? And merely point out that sip.xml is still supported etc? If you show sip.xml, every newcomer to Sip Servlets will use it...

If you are mentioning the sip.xml snippet in section 8.6, it indicates how to override the application name.  But you are right in the general sense.
I cut that example down to simply show the module name.
" type="cite">

=== Chapter 13 ===

13.2.6 SipWebSocketContextListener

If we find it useful to know the life-cycle of a socket (which I guess what this essentially is), would this be something we would like to expose for other transports as well? I.e., should we have similar functionality on a SipServletContextListener (which doesn’t exist today)?

Probably. We are planning to revisit this after EDR with more feedback we get from the community.
" type="cite">

16.3.1 Specifying Concurrency Mode

I think that default value of the concurrency mode should explicitly be defined and not left up to the container. I suggest that the default mode should be APPLICATIONSESSION but as long as we define a behavior (so either none or sas) I’m happy :-)

Also missing the NONE concurrency enum in the table (suggestion):

NONE - Indicates that the container will not synchronize access to shared resources such as SipSession and SipApplicationSession. It is up to the application developer to synchronize these resources in order to guarantee a thread safe application.

I think the confusion comes from the fact that we renamed NONE to UNSPECIFIED (from another review comment earlier) and i had some text which was not corrected. The default is UNSPECIFIED (which is essentially NONE). I fixed
that. The default is UNSPECIFIED, which provide the backward compatibility for the applications.
" type="cite">

=== Chapter 16.4 SIP Outbound Support ===

On the Flow interface we have two methods for retrieving the local and remote URI. However, are these really URI’s? A SipURI can have parameters, user portion etc but we are really just talking about an address here. Compare with SipServletMessage.getLocalAddr and getLocalPort (and the remote counterpart) so I would suggest to follow the same logic here or return a SocketAddress instead.

Jean, what do you think? Again, I would defer changes (if any) on this to after EDR. This is more of a style issue.

" type="cite">

=== General Comment ===

When it comes to showing examples I think it is important that we show the preferred way of doing things. E.g., I believe that we are trying to get away from sip.xml and instead encourage users to use annotations. If that is so, then all examples, where applicable, should make use of annotations.

Similarly, if we are pushing for POJOs (perhaps we are not but just to highlight another example) then all examples should use POJO’s. E.g. the examples section are all extending SipServlet but perhaps we want to change these into POJO’s?\

I agree. I modified the to follow POJO model in my local version. And will keep this in mind going forward.

" type="cite">

Anyway, I think examples are super important and whatever code snippets we show, that’s what people will start using first and foremost (at least that is how I work).

Thanks again for the comments.

- Binod.

" type="cite">

On Tue, Apr 16, 2013 at 8:05 AM, binod pg < " target="_blank"> > wrote:

- Made sip/websocket and outbound support depend on servletcontext supportedrfc/supported properties.
- Made editorial changes for handling some comments.


EDR draft updated.

binod pg 04/16/2013

Re: EDR draft updated.

Jonas Borjesson 04/16/2013

Jonas comments [Re: EDR draft updated.]

binod pg 04/17/2013

Re: EDR draft updated - dialog termination section

Eric Cheung 04/17/2013

Re: EDR draft updated - dialog termination section

Jonas Borjesson 04/17/2013

Re: EDR draft updated - dialog termination section

Nitzan Nissim 04/21/2013

Re: EDR draft updated - dialog termination section

Eric Cheung 04/22/2013

Re: EDR draft updated - dialog termination section

binod pg 04/22/2013

Re: EDR draft updated - dialog termination section

Eric Cheung 04/22/2013

Re: EDR draft updated.

Eric Cheung 04/17/2013

Eric's comments [Re: EDR draft updated.]

binod pg 04/17/2013

Re: EDR draft updated.

Strickland, Tom 04/17/2013

Re: EDR draft updated.

binod pg 04/18/2013
Please Confirm