Attachment 137668 Details for Bug 278258 – Proposed solution

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Proposed solution

clipboard.txt (text/plain), 15.11 KB, created by Rüdiger Herrmann

on 2009-05-29 11:54:24 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Rüdiger Herrmann

Created: 2009-05-29 11:54:24 EDT

Size: 15.11 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.rap.rwt
>Index: src/org/eclipse/rwt/service/ISessionStore.java
>===================================================================
>RCS file: /cvsroot/rt/org.eclipse.rap/runtime.rwt/org.eclipse.rap.rwt/src/org/eclipse/rwt/service/ISessionStore.java,v
>retrieving revision 1.5
>diff -u -r1.5 ISessionStore.java
>--- src/org/eclipse/rwt/service/ISessionStore.java	30 Jun 2008 07:30:59 -0000	1.5
>+++ src/org/eclipse/rwt/service/ISessionStore.java	29 May 2009 15:54:13 -0000
>@@ -53,12 +53,11 @@
>    *             cannot be null
>    *            
>    * @param value the object to be bound
>-   *
>-   * @exception IllegalStateException If this method is called on an
>-   *                                  invalidated session
>-   *
>+   * @return <code>true</code> if the attribute was set or <code>false</code>
>+   *         if the attribute could not be set because the session was 
>+   *         invalidated.
>    */
>-  void setAttribute( String name, Object value );
>+  boolean setAttribute( String name, Object value );
> 
>   /**
>    * Returns the object bound with the specified name in this
>@@ -66,9 +65,8 @@
>    * under the name.
>    * 
>    * @param name a string specifying the name of the object
>-   * @return the object with the specified name
>-   * @exception IllegalStateException If this method is called on an invalidated
>-   *                                  session.
>+   * @return the object with the specified name or <code>null</code> if the
>+   *         underlying session was invalidated.
>    */
>   Object getAttribute( String name );
> 
>@@ -83,10 +81,11 @@
>    * 
>    * @param name The name of the object to remove from this 
>    *             <code>ISessionStore</code>.
>-   * @exception IllegalStateException If this method is called on an invalidated
>-   *                                  session.
>+   * @return <code>true</code> if the attribute was removed or 
>+   *         <code>false</code> if the attribute could not be removed because 
>+   *         the session was invalidated.
>    */
>-  void removeAttribute( String name );
>+  boolean removeAttribute( String name );
> 
>   /**
>    * Returns an <code>Enumeration</code> of <code>String</code> objects
>@@ -95,9 +94,8 @@
>    * 
>    * @return An <code>Enumeration</code> of <code>String</code> objects
>    *         specifying the names of all the objects bound to this
>-   *         <code>ISessionStore</code>.
>-   * @exception IllegalStateException If this method is called on an invalidated
>-   *                                  session.
>+   *         <code>ISessionStore</code> or <code>null</code> if the underlying
>+   *         session was invalidated.
>    */
>   Enumeration getAttributeNames();
> 
>@@ -109,8 +107,6 @@
>    * 
>    * @return A string specifying the identifier assigned to the
>    *         underlying <code>HttpSession</code>.
>-   * @exception IllegalStateException If this method is called on an invalidated
>-   *                                  session.
>    */
>   String getId();
> 
>@@ -121,10 +117,11 @@
>    * If the given listenern is already added the method has no effect.
>    *  
>    * @param listener the SessionStoreListener to be added
>-   * @exception IllegalStateException If this method is called on an invalidated
>-   *                                  session.
>+   * @return <code>true</code> if the listener was added or <code>false</code>
>+   *         if the listener could not be added because the session was 
>+   *         invalidated.
>    */
>-  void addSessionStoreListener( SessionStoreListener listener );
>+  boolean addSessionStoreListener( SessionStoreListener listener );
>   
>   /**
>    * Removes an instance of <code>SessionStoreListener</code> to this
>@@ -134,10 +131,11 @@
>    * has no effect.
>    * 
>    * @param listener the SessionStoreListener to be removed
>-   * @exception IllegalStateException If this method is called on an invalidated
>-   *                                  session.
>+   * @return <code>true</code> if the listener was removed or <code>false</code>
>+   *         if the listener could not be removed because the session was 
>+   *         invalidated.
>    */
>-  void removeSessionStoreListener( SessionStoreListener listener );
>+  boolean removeSessionStoreListener( SessionStoreListener listener );
> 
>   /**
>    * Returns the underlying HttpSession instance.
>Index: src/org/eclipse/rwt/internal/lifecycle/UICallBackManager.java
>===================================================================
>RCS file: /cvsroot/rt/org.eclipse.rap/runtime.rwt/org.eclipse.rap.rwt/src/org/eclipse/rwt/internal/lifecycle/UICallBackManager.java,v
>retrieving revision 1.18
>diff -u -r1.18 UICallBackManager.java
>--- src/org/eclipse/rwt/internal/lifecycle/UICallBackManager.java	29 May 2009 14:24:17 -0000	1.18
>+++ src/org/eclipse/rwt/internal/lifecycle/UICallBackManager.java	29 May 2009 15:54:13 -0000
>@@ -211,14 +211,7 @@
>       } finally {
>         locked.remove( currentThread );
>         if( !result ) {
>-          // TODO [rh] remove the try/catch block once this bug 278258 is fixed
>-          //      (Rework ISessionStore#add/removeSessionStoreListener)
>-          try {
>-            ContextProvider.getSession().removeSessionStoreListener( listener );
>-          } catch( IllegalStateException e ) {
>-            // ignore - the session store is (about to be) unbound, this means
>-            // the listener is/will be removed anyway
>-          }
>+          ContextProvider.getSession().removeSessionStoreListener( listener );
>         }
>       }
>       waitForUIThread = true;
>Index: src/org/eclipse/rwt/internal/service/SessionStoreImpl.java
>===================================================================
>RCS file: /cvsroot/rt/org.eclipse.rap/runtime.rwt/org.eclipse.rap.rwt/src/org/eclipse/rwt/internal/service/SessionStoreImpl.java,v
>retrieving revision 1.9
>diff -u -r1.9 SessionStoreImpl.java
>--- src/org/eclipse/rwt/internal/service/SessionStoreImpl.java	12 Jul 2008 16:02:34 -0000	1.9
>+++ src/org/eclipse/rwt/internal/service/SessionStoreImpl.java	29 May 2009 15:54:13 -0000
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2002, 2008 Innoopract Informationssysteme GmbH.
>+ * Copyright (c) 2002, 2009 Innoopract Informationssysteme GmbH.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -7,6 +7,7 @@
>  *
>  * Contributors:
>  *     Innoopract Informationssysteme GmbH - initial API and implementation
>+ *     EclipseSource - ongoing development
>  ******************************************************************************/
> package org.eclipse.rwt.internal.service;
> 
>@@ -26,9 +27,10 @@
>   public static final String ID_SESSION_STORE
>     = SessionStoreImpl.class.getName();
>   
>-  private final Map attributes = new HashMap();
>-  private final Set listeners = new HashSet();
>+  private final Map attributes;
>+  private final Set listeners;
>   private final HttpSession session;
>+  private final String id;
>   private boolean bound;
>   private boolean aboutUnbound;
>   private ISessionShutdownAdapter shutdownAdapter;
>@@ -36,6 +38,9 @@
>   
>   public SessionStoreImpl( final HttpSession session ) {
>     ParamCheck.notNull( session, "session" );
>+    attributes = new HashMap();
>+    listeners = new HashSet();
>+    this.id = session.getId();
>     this.session = session;
>     this.session.setAttribute( ID_SESSION_STORE, this );
>     bound = true;
>@@ -52,57 +57,72 @@
>       } );
>     }
>   }
>-
>   
>   //////////////////////////
>   // interface ISessionStore
>   
>   public Object getAttribute( final String name ) {
>-    checkBound();
>-    synchronized( attributes ) {
>-      return attributes.get( name );
>+    Object result = null;
>+    if( bound ) {
>+      synchronized( attributes ) {
>+        result = attributes.get( name );
>+      }
>     }
>+    return result;
>   }
> 
>-  public void setAttribute( final String name, final Object value ) {
>-    checkBound();
>-    if( value == null ) {
>-      removeAttribute( name );
>-    } else {
>-      Object removed = null;
>-      synchronized( attributes ) {
>-        if( attributes.containsKey( name ) ) {
>-          removed = removeAttributeInternal( name );
>+  public boolean setAttribute( final String name, final Object value ) {
>+    boolean result = false;
>+    if( bound ) {
>+      result = true;
>+      if( value == null ) {
>+        removeAttribute( name );
>+      } else {
>+        Object removed = null;
>+        synchronized( attributes ) {
>+          if( attributes.containsKey( name ) ) {
>+            removed = removeAttributeInternal( name );
>+          }
>+          attributes.put( name, value );
>         }
>-        attributes.put( name, value );        
>-      }
>-      if( removed != null ) {
>-        fireValueUnbound( name, removed );
>+        if( removed != null ) {
>+          fireValueUnbound( name, removed );
>+        }
>+        fireValueBound( name, value );
>       }
>-      fireValueBound( name, value );
>     }
>+    return result;
>   }
>   
>-  public void removeAttribute( final String name ) {
>-    checkBound();
>-    fireValueUnbound( name, removeAttributeInternal( name ) );
>+  public boolean removeAttribute( final String name ) {
>+    boolean result = false;
>+    if( bound ) {
>+      result = true;
>+      fireValueUnbound( name, removeAttributeInternal( name ) );
>+    }
>+    return result;
>   }
> 
>   public Enumeration getAttributeNames() {
>-    checkBound();
>-    final Iterator iterator = attributes.keySet().iterator();
>-    return new Enumeration() {
>-      public boolean hasMoreElements() {
>-        return iterator.hasNext();
>-      }
>-      public Object nextElement() {
>-        return iterator.next();
>-      }
>-    };
>+    Enumeration result = null;
>+    if( bound ) {
>+      final Iterator iterator = attributes.keySet().iterator();
>+      result = new Enumeration() {
>+
>+        public boolean hasMoreElements() {
>+          return iterator.hasNext();
>+        }
>+
>+        public Object nextElement() {
>+          return iterator.next();
>+        }
>+      };
>+    }
>+    return result;
>   }
>   
>   public String getId() {
>-    return session.getId();
>+    return id;
>   }
>   
>   public HttpSession getHttpSession() {
>@@ -113,20 +133,26 @@
>     return bound;
>   }
> 
>-  public void addSessionStoreListener( final SessionStoreListener lsnr ) {
>-    checkAboutUnbound();
>-    checkBound();
>-    synchronized( listeners ) {
>-      listeners.add( lsnr );
>+  public boolean addSessionStoreListener( final SessionStoreListener lsnr ) {
>+    boolean result = false;
>+    if( bound && !aboutUnbound ) {
>+      result = true;
>+      synchronized( listeners ) {
>+        listeners.add( lsnr );
>+      }
>     }
>+    return result;
>   }
> 
>-  public void removeSessionStoreListener( final SessionStoreListener lsnr ) {
>-    checkAboutUnbound();
>-    checkBound();
>-    synchronized( listeners ) {
>-      listeners.remove( lsnr );
>+  public boolean removeSessionStoreListener( final SessionStoreListener lsnr ) {
>+    boolean result = false;
>+    if( bound && !aboutUnbound ) {
>+      result = true;
>+      synchronized( listeners ) {
>+        listeners.remove( lsnr );
>+      }
>     }
>+    return result;
>   }
> 
>   
>@@ -239,19 +265,6 @@
>     return result;
>   }
>   
>-  private void checkBound() {
>-    if( !bound ) {
>-      throw new IllegalStateException( "The session store has been unbound." );
>-    }
>-  }
>-  
>-  private void checkAboutUnbound() {
>-    if( aboutUnbound ) {
>-      String msg = "The session store is about to be unbound.";
>-      throw new IllegalStateException( msg );
>-    }
>-  }
>-    
>   private void fireValueBound( final String name, final Object value ) {
>     if( value instanceof HttpSessionBindingListener ) {
>       HttpSessionBindingListener listener
>#P org.eclipse.rap.rwt.test
>Index: src/org/eclipse/rwt/service/SessionStore_Test.java
>===================================================================
>RCS file: /cvsroot/rt/org.eclipse.rap/runtime.rwt.test/org.eclipse.rap.rwt.test/src/org/eclipse/rwt/service/SessionStore_Test.java,v
>retrieving revision 1.5
>diff -u -r1.5 SessionStore_Test.java
>--- src/org/eclipse/rwt/service/SessionStore_Test.java	1 Jul 2008 17:16:03 -0000	1.5
>+++ src/org/eclipse/rwt/service/SessionStore_Test.java	29 May 2009 15:54:17 -0000
>@@ -104,60 +104,36 @@
>     assertSame( httpSession, sessionLog[ 0 ] );
>     assertSame( attr2, valueLog[ 0 ] );
>     assertSame( session, storeLog[ 0 ] );
>-    try {
>-      session.setAttribute( ATTR1, null );
>-      fail();
>-    } catch( final IllegalStateException ise ) {
>-    }
>-    try {
>-      session.getAttribute( ATTR1 );
>-      fail();
>-    } catch( final IllegalStateException ise ) {
>-    }
>-    try {
>-      session.removeAttribute( ATTR1 );
>-      fail();
>-    } catch( final IllegalStateException ise ) {
>-    }
>-    try {
>-      session.getAttributeNames();
>-      fail();
>-    } catch( final IllegalStateException ise ) {
>-    }
>-    try {
>-      session.addSessionStoreListener( null );
>-      fail();
>-    } catch( final IllegalStateException ise ) {
>-    }
>-    try {
>-      session.removeSessionStoreListener( null );
>-      fail();
>-    } catch( final IllegalStateException ise ) {
>-    }
>-
>-
>+    // check method calls for unbound session store
>+    boolean setAttribute = session.setAttribute( ATTR1, null );
>+    assertFalse( setAttribute );
>+    Object attribute = session.getAttribute( ATTR1 );
>+    assertNull( attribute );
>+    boolean removeAttribute = session.removeAttribute( ATTR1 );
>+    assertFalse( removeAttribute );
>+    attributeNames = session.getAttributeNames();
>+    assertNull( attributeNames );
>+    boolean addSessionStoreListener = session.addSessionStoreListener( null );
>+    assertFalse( addSessionStoreListener );
>+    boolean removeSessionStoreListener = session.removeSessionStoreListener( null );
>+    assertFalse( removeSessionStoreListener );
>+    // check that listener cannot be added when about to be unbound
>+    final boolean[] aboutUnboundListener = { true };
>     TestSession checkAboutUnboundHttpSession = new TestSession();
>     final SessionStoreImpl checkAboutUnbound
>       = new SessionStoreImpl( checkAboutUnboundHttpSession );
>     checkAboutUnbound.addSessionStoreListener( new SessionStoreListener() {
>       public void beforeDestroy( final SessionStoreEvent event ) {
>-        checkAboutUnbound.addSessionStoreListener( new SessionStoreListener() {
>+        SessionStoreListener lsnr = new SessionStoreListener() {
>           public void beforeDestroy( final SessionStoreEvent event ) {
>           }
>-        } );
>-      }
>-    } );
>-    TestServletContext servletContext
>-      = ( TestServletContext )checkAboutUnboundHttpSession.getServletContext();
>-    final Set problems = new HashSet();
>-    servletContext.setLogger( new TestLogger() {
>-      public void log( final String message, final Throwable throwable ) {
>-        problems.add( throwable );
>+        };
>+        aboutUnboundListener[ 0 ]
>+          = checkAboutUnbound.addSessionStoreListener( lsnr );
>       }
>     } );
>     checkAboutUnbound.getHttpSession().invalidate();
>-    assertEquals( 1, problems.size() );
>-    servletContext.setLogger( null );
>+    assertFalse( aboutUnboundListener[ 0 ] );
> 
>     final boolean[] hasContext = { false };
>     SessionStoreImpl checkContext = new SessionStoreImpl( new TestSession() );

Actions: View | Diff

Attachments on bug 278258: 137668