Simon Lessard said the following On 5/17/2009 6:19 AM PT:
Hi Blake,

/Prefer the same what?  Are you talking about the ExternalContext?/

I meant placing register the listeners the Window itself, not the Manager, same as isNew.
The difference between isNew() and the addition of the listeners is that the the application doesn't have a good place to add a listener on a window instance except in some form of listener callback itself. The reason is that the application often isn't directly involved in the creation of new windows--for example the user hits control-N in IE and a new window is created. Other than using listener injection, which would be applied to all windows anyway, it isn't clear how an application could add a listener when a particular window instance is created. It seemed easier for the application to add a listener that applies to all windows and let the application ignore events on any windows it doesn't care about.

-- Blake Sullivan



~ Simon

On Sat, May 16, 2009 at 1:18 AM, Blake Sullivan <[email protected] <mailto:[email protected]>> wrote:

    Simon Lessard said the following On 5/15/2009 12:35 PM PT:
    Hi Blake,

    I'm + 1 with the idea and the general API, but I have some concerns:

       1. I don't really like the API to expose a read only map
          through WindowManager.getWindows, I would prefer
          WindowManager.getWindowIds(ExternalContext) and
          WindowManager.getWindow(ExternalContext, String);

    Is your worry over that the Map is read only?  The two separate
    methods prevent the developer from leveraging the Map interface.

       1. WindowListener should be an interface extending
          EventListener one of its subclasses;

    Fair enough, the rationale for making this an abstract class was
    so we could add more event types later.  However, since the
    WindowManager class is itself an abstract class we could add more
    event listener interfaces later to the WindowManager later, so I
    agree that the interface is fine

       1. Either WindowListener or WindowLifecycleEvent is wrongly
          named from a JSF point of view (althoguh it could be
          potentially correctly named in Swing). All JSF Listener
          handle events with the exact same name as the listener, but
          with Listener changed to Event, so it should either be
          WindowLifecycleListener or WindowEvent for the API to be
          coherent with the usual JSF nomenclature.

    See above.  I agree that if we use an interface then the listener
    class must be WindowLifecycleListener

       1. WindowListener method is not properly named. Pretty much as
          above, in JSF all listener methods are called
          process<evenType> so it should either be public void
          processWindowEvent or processWindowLifecycleEventdepending
          on the resolution of the previous point;
       2. The process method parameters do not match the usual
          listener convention to receive a single event object. Would
          it be possible to place the ExternalContext instance in the
          event?

    I guess we could put the ExternalContext in the event, though the
    field will need to be transient and getExternalContext() method on
    the event would need to document that it might return null if the
    event has been serialized.  All of which is pretty gross.  In this
    case the extra parameter is much cleaner.  I believe that the
    convention is really to aid Java Beans design tools using
    introspection and would prefer cleanliness in this case.

       1. I would prefer to see WindowManager.isCurrentWindowNew as
          Window.isNew, it's more OOP correct;

    OK.  The original implementation only tracked the new status of
    the current window, which was tracked by the WindowManager (which
    is why the WindowManager).  However, due to the need to answer
    this question in the face of redirects and other weirdness, the
    Windows in the implementation I have been testing do maintain
    their "new" state, so I agree that it makes more sense for the
    Window to answer this question.

       1. Actually I would prefer the same for the
          add/get/removeWindowListener

    Prefer the same what?  Are you talking about the ExternalContext?

    Thanks for the helpful feedback.

    --Blake Sullivan



    Regards,

    ~ Simon


    On Fri, May 15, 2009 at 3:09 PM, Blake Sullivan
    <[email protected] <mailto:[email protected]>> wrote:

        Here is the proposed api:

        package org.apache.myfaces.trinidad.context;


        /**
        * Represents a Window in the current user's Session.  Windows
        are created and vended
        * by the Session's WindowManager and the Window for the
        current request is
        * available from <code>WindowManager.getCurrentWindow</code>
        * @see WindowManager#getCurrentWindow
        */
        public abstract class Window implements Serializable
        {
         /**
         * <p>
         * Represents the current state of the Window.  Windows start
        out <code>OPEN</code>,
         * when the current window's document is being unloaded, they
        move to the <code>UNLOADING</code>
         * state and then either move back to the <code>OPEN</code>
        state if the Window's content
         * is populated with a new document from the same
        application, or to the <code>CLOSED</code>
         * state if it is not.
         * </p><p>
         * This represents the framework's best guess at the current
        status of the Window.
         * </p>
         */
         public enum LifecycleState
         {
          /** The Window is currently open */
          OPEN,

          /** The Window is being unloaded */
          UNLOADING,

          /** The Window is believed to be closed, either because the
        window was explicitly closed
           *  or because the window is suspected to have been closed
           */
          CLOSED
         }

         /**
         * Represents how the window is used in the application
         */
         public enum Usage
         {
          /** Used as a top-level application window */
          FRAME,

          /** Used as a dialog */
          DIALOG
         }

         /**
         * @return The unique identifier for this Window within the
        Session
         */
         public abstract String getId();

         /**
         * @return The current state of the Window
         */
         public abstract LifecycleState getLifecycleState();

         /**
         * Returns the Usage of the Window--either a top-level frame
        or a dialog
         * @return how the window is used
         */
         public abstract Usage getUsage();
        }

        /**
        * <p>
        * Manages the set of Windows currently in the Session and
        allows listeners on the Windows'
        * lifecycles to be registered.
        * </p>
        * @see
        org.apache.myfaces.trinidad.context.RequestContext#getWindowManager
        */
        abstract public class WindowManager
        {
         /**
         * @param extContext ExternalContext so that the
        WindowManager may be called before the
         * FacesContext is available
         * @return The Window that contains the document making the
        current request
         */
         public abstract Window getCurrentWindow(ExternalContext
        extContext);

         /**
         * @param extContext ExternalContext so that the
        WindowManager may be called before the
         * FacesContext is available
         * @return <code>true</code> if the Window making the current
        request is newly created.
         */
         public abstract boolean isCurrentWindowNew(ExternalContext
        extContext);

         /**
         * @param extContext ExternalContext so that the
        WindowManager may be called before the
         * FacesContext is available
         * @return The Unmodifiable Map of WindowIds to Windows
         */
         public abstract Map<String, ? extends Window>
        getWindows(ExternalContext extContext);

         /**
         * <p>
         * Registers a listener that will be informed of changes to
        the Lifecylce state of any of
         * the known Windows.
         * </p>
         * <p>
         * Window listeners may be registered automatically by adding
        a file
         * containing the names of the classes implementing the
        WindowListener in a file named
         *
        <code>org.apache.myfaces.trinidad.event.WindowListener</code>
        inside of
         * the <code>META_INF/services</code> directory.
         * </p>
         * @param extContext ExternalContext so that the
        WindowManager may be called before the
         * FacesContext is available
         * @param windowListener
         */
         public abstract void addWindowListener(ExternalContext
        extContext, WindowListener windowListener);

         /**
         * Removes a listener that will be informed of changes to the
        Lifecylce state of any of
         * the known Windows
         * @param extContext ExternalContext so that the
        WindowManager may be called before the
         * FacesContext is available
         * @param windowListener
         */
         public abstract void removeWindowListener(ExternalContext
        extContext, WindowListener windowListener);

         /**
         * Performs any necessary action to embed the current window
        identifier into the output
         * @param context FacesContext to use to write the output
         * @throws IOException if an output exception occurs
         */
         public abstract void writeState(FacesContext context) throws
        IOException;
        }

        /**
        * <p>
        * Application-scoped factory for creating per-Session
        WindowManager instances.  It is the
        * WindowManagerFactory implementation's responsibility to
        ensure that only one
        * WindowManager instance is created per-session.  The
        WindowManagerFactory is also responsible
        * for ensuring that any mutable state in the WindowManager
        instances will be successfully failed
        * over.
        * </p>
        * <p>
        * The factory is usually specified by placing the name of the
        WindowManagerFactory
        * implementation class in a file named
        *
        <code>org.apache.myfaces.trinidad.context.WindowManagerFactory</code>
        * in the <code>META-INF/services</code> directory
        * </p>
        * @see org.apache.myfaces.trinidad.context.WindowManager
        * @see
        org.apache.myfaces.trinidad.context.RequestContext#getWindowManager
        */
        abstract public class WindowManagerFactory
        {
         /**
         * Returns the WindowManager to use for this session,
        creating a new instance if one doesn't
         * already exist.
         * @param extContext ExternalContext
         * @return WindowManager to use for this Session
         */
         public abstract WindowManager
        getWindowManager(ExternalContext extContext);
        }

        To RequestContext add;

         /**
         * <p>
         * Returns the WindowManager for this request.  A non-null
        WindowManager
         * will always be returned.
         * </p><p>
         * The default implementation uses the first
        WindowManagerFactory specified
         * implementation class in a file named
         *
        <code>org.apache.myfaces.trinidad.context.WindowManagerFactory</code>
         * in the <code>META-INF/services</code> directory and uses
        the WindowManagerFactory
         * to create the WindowManager for this Session.  If no
        WindowManagerFactory is
         * found, a default WindowManager that never returns any
        Windows is used.
         * </p>
         * @return the WindowManager used for this Session.
         */
         public WindowManager getWindowManager()

        In package:

        package org.apache.myfaces.trinidad.event;


        /**
        * Represents an event delivered with a Window as the source.
        * @see org.apache.myfaces.trinidad.context.Window
        * @see org.apache.myfaces.trinidad.event.WindowListener
        */
        public abstract class WindowEvent extends EventObject
        {  /**
         * @return the Window that this event ocurred on.
         */
         public Window getSource()
        }

        /**
        * Event delivered when the LifecycleState of a Window
        changes.  The <code>cause</code>
        * indicates the cause ot the state change.  The state diagram
        for theWindow LifecycleStates
        * is
        <pre>
                            +-----------load---------------+
| | ---expire--- V /---unload----\ | / \ <start> ---open--->OPEN----- ----->UNLOADED-- -->CLOSED | \--navigate---/ ^ \ / | | ---close----
                            +---------closing--------------+

        </pre>
        * The new LifecycleStates can be retrieved by calling
        <code>getLifecycleState</code> on the
        * source Window or by calling the
        <code>getNewLifecycleState</code> convenience function
        * on the WindowLifecycleEvent
        * @see org.apache.myfaces.trinidad.context.Window
        * @see org.apache.myfaces.trinidad.context.Window.LifecycleState
        */
        public class WindowLifecycleEvent extends WindowEvent
        {
         /**
         * What caused the delivery of the WindowLifecycleEvent.
         */
         public enum Cause
         {
          /**
           * Delivered when a new Window is open
           */
          OPEN,
          /**
           * Delivered when the content of a Window have been
        unloaded but cause of the unloading
           * isn't known.
           */
          UNLOAD,
          /**
           * Delivered when the content of a Window have been
        unloaded as a result of
           * navigating within the application
           */
          NAVIGATE,
          /**
           * Delivered when the content of a Window have been
        unloaded in order to
           * close the window
           */
          CLOSING,
            /**
           * The contents of an existing Window are being reloaded
           */
          RELOAD,
            /**
           * The Window is believed to have been closed by the user
           */
          EXPIRE,

          /**
           * The Window is believed to have been closed by the user
           */
          CLOSE
         }

         /**
         * Creates a WindowOpenEvent event for the specified Window
        and cause.
         */
         public WindowLifecycleEvent(Window source, Cause cause)

         /**
         * @return the cause of the WindowOpen event.
         */
         public Cause getCause()

         /**
         * Returns the new LifecycleState that the Window has moved to.
         */
         public final LifecycleState getNewLifecycleState()
        }

        /**
        * WindowLifecycleEvent delivered when the current window is
        being unloaded
        * in order to navigate to a new location
        */
        public final class WindowNavigateEvent extends
        WindowLifecycleEvent
        {
         public WindowNavigateEvent(Window source, String destination)

         /**
         * Returns the URL to which the page is navigating.
         * <p>
         * The destination is not guaranteed to be normalized;  it may
         * be absolute, page-relative, or server-relative.  It is also
         * not guaranteed to be correct, as a browser
         * may be redirected to an alternate destination.
         */
         public String getDestination()
        }

        /**
        * <p>
        * A listener called when the Lifecyle of a Window changes.
        * </p>
        * <p>
        * Window listeners may be registered automatically by adding
        a file
        * containing the names of the classes implementing the
        WindowListener in a file named
        *
        <code>org.apache.myfaces.trinidad.event.WindowListener</code>
        inside of
        * the <code>META_INF/services</code> directory or manually by
        calling
        * <code>WindowManager.addWindowListener</code>
        * @see org.apache.myfaces.trinidad.context.WindowManager
        */
        public abstract class WindowListener
        {
         /**
         * <p>
         * Called when the LifecycleState of a Window changes.
         * </p>
         * <p>
         * The current lifecycle state of a Window is the framework's
        best guess and may not be accurate.
         * In particular, the last remaining open window may never
        move into the <code>CLOSED</code> state
         * once it has moved into the <code>UNLOADED</code> state.
         In addition, no Window lifecycle events
         * are delivered if the Session ceases to exist.
         * </p>
         * <p>
         * The FacesContext may not be available at the time that
        this event is delivered.
         * </p>
         * @param extContext ExternalContext available for this event
         * @param event WindowLifecycleEvent indicating the cause of
        the change to the Window's
         * LifecycleState
         */
         public abstract void processLifecylceEvent(ExternalContext
        extContext, WindowLifecycleEvent event);
        }

        Blake Sullivan (JIRA) said the following On 5/15/2009 11:41
        AM PT:

            Add Window abstraction to Trinidad
            ----------------------------------

                            Key: TRINIDAD-1474
                            URL:
            https://issues.apache.org/jira/browse/TRINIDAD-1474
                        Project: MyFaces Trinidad
                     Issue Type: New Feature
                     Components: Archetype
               Affects Versions:  1.2.12-core
                    Environment: All
                       Reporter: Blake Sullivan


            Add Window abstraction to Trinidad.  Currently, Trinidad
            knows nothing of the separate browser Windows that make
            up a browser session.  This causes weird problems.  For
            example, the state management token cache is shared
            across all of the active windows with a simple LRU.  If
            the user opens up two windows and operates on one window
            long enough, he will cause the token state for the
            original window to be purged.  When the user switches
            back to the original window and POSTs back, the token
            won't be found, Trinidad will assume that this is because
            the session expired, and the user will be given an error.

            Adding the concept of a Window and a Window lifecyle
            opens up the following capabilities:
            1) Correct handling of per-window UI state by segregating
            tokens by window
            2) Early clean-up of UI state by aggressively purging
            state for closed windows
            3) Applications can manager per-window state by listening
            for window lifecycle events
            4) Sessions can be cleaned up earlier by terminating the
            session when the last window in the session is closed
            5) A window scope can be implemented to ease using
            per-window state with EL
            6) A window manager implementation can hide the details
            of handling control-N in the browser






Reply via email to