sipservlet-spec
  1. sipservlet-spec
  2. SIPSERVLET_SPEC-34

Requirement: abiltity to discover (also terminate) SipSessions that are related by forking (aka derived sessions)

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-pr
    • Labels:
      None

      Description

      Forking can lead to 'derived' SipSessions. The label 'derived' only really matters for the act of copying the parent session, as after that, all sessions should be considered to be equivalent: it does not really matter which session was the first, as what really matters is the final response to each session. If session 5 receives a 200 ok and the application wants to terminate the remaining sessions, then it which session is the original session and which are derived does not matter. So, a better label: sibling sessions.

      The requirement: the means to discover sibling SipSessions for a given SipSession. Perhaps a method call SipSession.getSiblingSessions(): Set<SipSession>, returning empty set if there are none.

      A secondary requirement: the means to terminate other SipSessions by calling SipSession.terminateOtherSipSessions(). This is a more complicated requirement: it is relatively straightforward in non-proxy scenarios, but in proxy scenarios all that we can do is to terminate such branches.

      I will have more to add on this topic shortly: this is just an initial bug-report.

        Activity

        Hide
        binod added a comment -

        Current ForkingContext API in the draft currently distributed fixes this issue. Please see section 8.2.6 of the specification.

        Show
        binod added a comment - Current ForkingContext API in the draft currently distributed fixes this issue. Please see section 8.2.6 of the specification.
        Hide
        tomrstrickland added a comment - - edited

        I have been reviewing the Javadocs on ForkingContext and see a new method that seems to go against the originally agreed discussion:

        void setAttributeCloningEnabled(boolean cloneAttributes)
        By default when a SipSession object is created by deriving from another
        SipSession, the attributes from the original session are copied to the derived
        SipSession. Application can control this behavior using this method.
        
        Parameters:
            cloneAttributes - boolean specifying whether the attributes need to cloned or not.
        Throws:
            IllegalStateException - if this method is invoked on an invalid forking context.
        

        The group had originally decided to have this set as NOT cloned by default for JSR-359 apps. I see also that section 8.2.3.2 Derived SipSessions also says that SipSession contents are cloned by default.

        We had originally talked about whether or not this should be switchable at the application-level and/or the SipSession level. I think that we had originally agreed that this would be switchable at the application level, and would default to not cloning attributes if the application is explicitly at least a SipServlet 2.0 app.

        Thoughts
        Regardless of what we said, I think that the old behaviour - cloning attributes - is a horrible thing for app writers to have to put up with in SipServlet 2.0, and to have to remember to switch it off for every new SIP dialog is just irritating fluff for the app writer.

        1. The cloning behaviour should at least be switchable at the application level - to my mind, this seems to be essential.
        2. Maybe the cloning behaviour should be overridable (sp?) at the ForkingContext level. Why though?
        3. If an application is a JSR-359 app, shouldn't the default behaviour be to NOT clone attributes?
        Show
        tomrstrickland added a comment - - edited I have been reviewing the Javadocs on ForkingContext and see a new method that seems to go against the originally agreed discussion: void setAttributeCloningEnabled(boolean cloneAttributes) By default when a SipSession object is created by deriving from another SipSession, the attributes from the original session are copied to the derived SipSession. Application can control this behavior using this method. Parameters: cloneAttributes - boolean specifying whether the attributes need to cloned or not. Throws: IllegalStateException - if this method is invoked on an invalid forking context. The group had originally decided to have this set as NOT cloned by default for JSR-359 apps. I see also that section 8.2.3.2 Derived SipSessions also says that SipSession contents are cloned by default. We had originally talked about whether or not this should be switchable at the application-level and/or the SipSession level. I think that we had originally agreed that this would be switchable at the application level, and would default to not cloning attributes if the application is explicitly at least a SipServlet 2.0 app. Thoughts Regardless of what we said, I think that the old behaviour - cloning attributes - is a horrible thing for app writers to have to put up with in SipServlet 2.0, and to have to remember to switch it off for every new SIP dialog is just irritating fluff for the app writer. The cloning behaviour should at least be switchable at the application level - to my mind, this seems to be essential. Maybe the cloning behaviour should be overridable (sp?) at the ForkingContext level. Why though? If an application is a JSR-359 app, shouldn't the default behaviour be to NOT clone attributes?
        Hide
        binod added a comment -

        Agree that, it is important to be able to switch it the application level. That is missing and we should
        add a element in SipApplication annotation and also in deployment descriptor.

        The method in ForkingContext is a nicety. Its a just a flexibility we can provide to the developer.
        Cant think of a common usecase.

        The default behavior: It might break the backward compatibility for applications ported from 289. But I see
        your point. Hopefully, applications wont be depending on the cloning behavior anyway.

        Show
        binod added a comment - Agree that, it is important to be able to switch it the application level. That is missing and we should add a element in SipApplication annotation and also in deployment descriptor. The method in ForkingContext is a nicety. Its a just a flexibility we can provide to the developer. Cant think of a common usecase. The default behavior: It might break the backward compatibility for applications ported from 289. But I see your point. Hopefully, applications wont be depending on the cloning behavior anyway.
        Hide
        tomrstrickland added a comment -

        Suggestions

        1. For 289 apps, the default (if not set at the application level) session attributes MUST be cloned.
        2. For 359 apps, the default (if not set at the application level) session attributes MUST NOT be cloned.
        3. ForkingContext.setAttributeCloningEnabled needs a use case. However, if it is included, it is used to override the app-level setting and so does not have a meaningful default value.

        In this way:

        1. 359 apps benefit from the new, more sensible, behaviour.
        2. 289 apps are not broken.
        3. Backwards compatability is broken when porting a 289 app to run as a 359 app, but then this is a 2.0 release. Better this small amount of pain for porters, than needless boilerplate for all future app writers.
        Show
        tomrstrickland added a comment - Suggestions For 289 apps, the default (if not set at the application level) session attributes MUST be cloned. For 359 apps, the default (if not set at the application level) session attributes MUST NOT be cloned. ForkingContext.setAttributeCloningEnabled needs a use case. However, if it is included, it is used to override the app-level setting and so does not have a meaningful default value. In this way: 359 apps benefit from the new, more sensible, behaviour. 289 apps are not broken. Backwards compatability is broken when porting a 289 app to run as a 359 app, but then this is a 2.0 release. Better this small amount of pain for porters, than needless boilerplate for all future app writers.

          People

          • Assignee:
            binod
            Reporter:
            tomrstrickland
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: