Skip to main content

Re: EDR draft updated.

  • From: Jonas Borjesson < >
  • To:
  • Subject: Re: EDR draft updated.
  • Date: Tue, 16 Apr 2013 13:43:51 -0700

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.


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:


@ClientErrorResponse

@ServerErrorResponse

@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) {}


4.6 Method Selection Procedure

“as decided in F2F” - remove this comment


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.


=== 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.


7.2.5.1.2 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?


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.


=== 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...


=== 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)?


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.


=== 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.


=== 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 16.3.2.2 are all extending SipServlet but perhaps we want to change these into POJO’s?


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).




On Tue, Apr 16, 2013 at 8:05 AM, binod pg < " target="_blank"> > wrote:
http://java.net/projects/sipservlet-spec/downloads/directory/v0.4

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

thanks,
Binod.



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
 
 
Close
loading
Please Confirm
Close