Menu
▾
▴
[Aplaws-commit] r166 - in ccm-user-preferences/trunk: . src/com/arsdigita/london/userprefs src/com/arsdigita/london/userprefs/ui
|
From: <mb...@re...> - 2004-12-22 17:36:03
|
Author: mbooth
Date: 2004-12-22 18:27:08 +0100 (Wed, 22 Dec 2004)
New Revision: 166
Modified:
ccm-user-preferences/trunk/application.xml
ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/PersistentUserPrefs.java
ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/UserPrefs.java
ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/ui/UserPrefsComponent.java
Log:
Fix bug owning to useless httpsessions.
Modified: ccm-user-preferences/trunk/application.xml
===================================================================
--- ccm-user-preferences/trunk/application.xml 2004-12-22 14:04:21 UTC (rev 165)
+++ ccm-user-preferences/trunk/application.xml 2004-12-22 17:27:08 UTC (rev 166)
@@ -3,7 +3,7 @@
<ccm:application name="ccm-user-preferences"
prettyName="User Preferences"
version="1.0.0"
- release="2"
+ release="3"
xmlns:ccm="http://ccm.redhat.com/ccm-project">
<ccm:dependencies>
Modified: ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/PersistentUserPrefs.java
===================================================================
--- ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/PersistentUserPrefs.java 2004-12-22 14:04:21 UTC (rev 165)
+++ ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/PersistentUserPrefs.java 2004-12-22 17:27:08 UTC (rev 166)
@@ -85,8 +85,11 @@
PersistentUserPrefs prefs = null;
if( pups.next() ) {
prefs = new PersistentUserPrefs( pups.getDataObject() );
- s_log.warn( "User " + user.getOID() + " has multiple user " +
- "preferences" );
+
+ if( pups.next() ) {
+ s_log.warn( "User " + user.getOID() + " has multiple user " +
+ "preferences" );
+ }
}
pups.close();
Modified: ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/UserPrefs.java
===================================================================
--- ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/UserPrefs.java 2004-12-22 14:04:21 UTC (rev 165)
+++ ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/UserPrefs.java 2004-12-22 17:27:08 UTC (rev 166)
@@ -16,6 +16,7 @@
package com.arsdigita.london.userprefs;
+import com.arsdigita.caching.CacheTable;
import com.arsdigita.domain.DataObjectNotFoundException;
import com.arsdigita.domain.DomainCollection;
import com.arsdigita.kernel.Kernel;
@@ -34,8 +35,7 @@
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import javax.servlet.http.HttpSessionEvent;
-import javax.servlet.http.HttpSessionActivationListener;
+import javax.servlet.http.HttpSession;
import org.apache.log4j.Logger;
@@ -55,33 +55,6 @@
* @author Matthew Booth <mb...@re...>
*/
-// LAZY PERSISTENCE
-
-// My original intention was that save would not save immediately, but rather
-// only save when the session expires. This is not terribly safe for the data,
-// but these are only user preferences and I didn't want to unnecessarily hit
-// the database. As long as data is not usually lost, that's fine.
-
-// I was originally going to achieve this by implementing
-// HttpSessionBindingListener in the understanding that objects would be unbound
-// before the session is destroyed. They're not. Instead I tried
-// HttpSessionActivationListener which is called when a Session is migrated
-// between servers. This isn't used either, but I left that in because it
-// seems like a good idea.
-
-// Finally I tried HttpSessionListener. It looks like I should have tried this
-// first, as it appears to be the correct interface for the job. However, the
-// sessionDestroyed() method is only called *after* the Session object is
-// invalidated. This means that not only can you not retrieve attributes from
-// it, but you can't even get the session ID. You know that a session was just
-// destroyed, but you have no way of knowing which one. In fact, it's utterly,
-// utterly, utterly useless. It's completely without merit. A waste of bits.
-// Who on earth came up with that?
-
-// There seems to be some talk of this being fixed in the Servlet 2.4 spec.
-
-// Matt <mb...@re...>
-
public class UserPrefs {
private static final Logger s_log = Logger.getLogger( UserPrefs.class );
@@ -94,9 +67,16 @@
private HashMap m_prefs = new HashMap();
+ private static final CacheTable s_prefsCache =
+ new CacheTable( "user_preferences" );
+
private UserPrefs() {};
private UserPrefs( PersistentUserPrefs persistentPrefs ) {
+ init( persistentPrefs );
+ }
+
+ private void init( PersistentUserPrefs persistentPrefs ) {
m_persistentPrefs = persistentPrefs.getOID();
m_cookie = persistentPrefs.getCookie();
m_user = persistentPrefs.getUser() == null ?
@@ -124,68 +104,83 @@
*/
public static UserPrefs retrieve( HttpServletRequest req,
HttpServletResponse res ) {
- UserPrefs prefs = (UserPrefs)
- req.getSession().getAttribute( SESSION_ATTRIBUTE );
+ HttpSession session = req.getSession();
- if( null != prefs ) return prefs;
+ // Yes, this seems like a silly use of an HttpSession, but they really
+ // are broken in almost every useful way.
+ UserPrefs prefs = (UserPrefs) s_prefsCache.get( session.getId() );
+ if( null != prefs ) {
+ if( s_log.isDebugEnabled() ) {
+ s_log.debug( "Retrieved user prefs for session " +
+ session.getId() );
+ }
+ return prefs;
+ }
+
+ PersistentUserPrefs persistentPrefs = retrievePersistent( req, res );
+ if( null != persistentPrefs ) {
+ prefs = new UserPrefs( persistentPrefs );
+ } else {
+ prefs = new UserPrefs();
+
+ User user = (User) Kernel.getContext().getParty();
+ if( null != user ) {
+ prefs.m_user = user.getOID();
+ } else {
+ Long cookie;
+ try {
+ SecureRandom random = SecureRandom.getInstance( "SHA1PRNG" );
+ cookie = new Long( random.nextLong() );
+ } catch( NoSuchAlgorithmException ex ) {
+ s_log.warn( "Unable to get SecureRandom for SHA1PRNG. " +
+ "Falling back to insecure random generator." );
+ cookie = new Long( new Random().nextLong() );
+ }
+
+ prefs.m_cookie = cookie;
+ setCookie( res, cookie.toString() );
+ }
+
+ s_log.debug( "Created new prefs" );
+ }
+
+ s_prefsCache.put( session.getId(), prefs );
+ return prefs;
+ }
+
+ private static PersistentUserPrefs retrievePersistent
+ ( HttpServletRequest req, HttpServletResponse res )
+ {
+ PersistentUserPrefs persistentPrefs = null;
+
User user = (User) Kernel.getContext().getParty();
if( null != user ) {
- PersistentUserPrefs persistentPrefs =
- PersistentUserPrefs.retrieveForUser( user );
- if( null != persistentPrefs ) {
- prefs = new UserPrefs( persistentPrefs );
- }
+ persistentPrefs = PersistentUserPrefs.retrieveForUser( user );
}
- if( null != prefs ) {
+ if( null != persistentPrefs ) {
s_log.debug( "Got prefs for user" );
- req.getSession().setAttribute( SESSION_ATTRIBUTE, prefs );
- return prefs;
+ return persistentPrefs;
}
Long cookie = getCookie( req );
if( null != cookie ) {
- PersistentUserPrefs persistentPrefs =
- PersistentUserPrefs.retrieveForCookie( cookie );
+ persistentPrefs = PersistentUserPrefs.retrieveForCookie( cookie );
// Remove a bogus cookie
if( null == persistentPrefs ) {
removeCookie( res );
}
-
- else {
- prefs = new UserPrefs( persistentPrefs );
- }
}
- if( null != prefs ) {
+ if( null != persistentPrefs ) {
s_log.debug( "Got prefs for cookie" );
- req.getSession().setAttribute( SESSION_ATTRIBUTE, prefs );
- return prefs;
+ return persistentPrefs;
}
- prefs = new UserPrefs();
-
- if( null != user ) {
- prefs.m_user = user.getOID();
- } else {
- try {
- SecureRandom random = SecureRandom.getInstance( "SHA1PRNG" );
- cookie = new Long( random.nextLong() );
- } catch( NoSuchAlgorithmException ex ) {
- s_log.warn( "Unable to get SecureRandom for SHA1PRNG. " +
- "Falling back to insecure random generator." );
- cookie = new Long( new Random().nextLong() );
- }
-
- prefs.m_cookie = cookie;
- setCookie( res, cookie.toString() );
- }
-
- s_log.debug( "Created new prefs" );
- req.getSession().setAttribute( SESSION_ATTRIBUTE, prefs );
- return prefs;
+ s_log.debug( "No existing prefs" );
+ return null;
}
/**
@@ -214,9 +209,13 @@
* @param key The identifier of the preference to be stored
* @param value The value of the preference to be stored
*/
- public void set( String key, String value ) {
+ public void set( String key, String value,
+ HttpServletRequest req, HttpServletResponse res ) {
m_prefs.put( key, value );
- getPersistent().setValue( key, value );
+
+ PersistentUserPrefs prefs = getPersistent();
+ if( null == prefs ) prefs = createPersistent( req, res );
+ prefs.setValue( key, value );
}
/**
@@ -224,23 +223,31 @@
*
* @param key The identifier of the preference to be removed
*/
- public void remove( String key ) {
+ public void remove( String key, HttpServletRequest req ) {
m_prefs.remove( key );
+ PersistentUserPrefs persistent = getPersistent();
+
if( !m_prefs.isEmpty() ) {
- getPersistent().removeValue( key );
+ if( null != persistent ) persistent.removeValue( key );
} else {
- getPersistent().delete();
+ if( null != persistent ) persistent.delete();
+
+ req.getSession().setAttribute( SESSION_ATTRIBUTE, null );
+ m_persistentPrefs = null;
}
}
/**
* Save user preferences to the database
*/
- public void persist() {
+ public void persist( HttpServletRequest req,
+ HttpServletResponse res ) {
s_log.info( "Persisting session" );
PersistentUserPrefs prefs = getPersistent();
+ if( null == prefs ) prefs = createPersistent( req, res );
+
prefs.setAllValues( m_prefs );
prefs.save();
@@ -251,22 +258,36 @@
* Get a PersistentUserPrefs object. Create one if necessary.
*/
private PersistentUserPrefs getPersistent() {
- PersistentUserPrefs prefs;
+ PersistentUserPrefs prefs = null;
- if( null == m_persistentPrefs ) {
- prefs = new PersistentUserPrefs();
- } else {
+ if( null != m_persistentPrefs ) {
try {
prefs = new PersistentUserPrefs( m_persistentPrefs );
} catch( DataObjectNotFoundException ex ) {
s_log.warn( "User preferences object contained bogus " +
"persistent preferences OID" );
- prefs = new PersistentUserPrefs();
}
}
- // If this is newly persisted, set appropriately
- if( prefs.isNew() ) {
+ // If we're saving something and we have a user object now, use that
+ // in preference
+ if( null == m_user ) {
+ User user = (User) Kernel.getContext().getParty();
+ if( null != user ) {
+ prefs.setUser( user );
+ prefs.setCookie( null );
+ }
+ }
+
+ return prefs;
+ }
+
+ private PersistentUserPrefs createPersistent( HttpServletRequest req,
+ HttpServletResponse res ) {
+ PersistentUserPrefs prefs = retrievePersistent( req, res );
+ if( null == prefs ) {
+ prefs = new PersistentUserPrefs();
+
if( s_log.isDebugEnabled() ) {
s_log.debug( "Initializing new user preferences: " +
prefs.getOID() );
@@ -283,20 +304,17 @@
else {
throw new UncheckedWrapperException ( "User preferences object doesn't contain either a user or a cookie object" );
}
- }
- // If we're saving something and we have a user object now, use that
- // in preference
- if( null == m_user ) {
- User user = (User) Kernel.getContext().getParty();
- if( null != user ) {
- prefs.setUser( user );
- prefs.setCookie( null );
+ m_persistentPrefs = prefs.getOID();
+ } else {
+ if( s_log.isDebugEnabled() ) {
+ s_log.debug( "Reusing existing persistent preferences " +
+ prefs.getOID() );
}
+
+ init( prefs );
}
- m_persistentPrefs = prefs.getOID();
-
return prefs;
}
Modified: ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/ui/UserPrefsComponent.java
===================================================================
--- ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/ui/UserPrefsComponent.java 2004-12-22 14:04:21 UTC (rev 165)
+++ ccm-user-preferences/trunk/src/com/arsdigita/london/userprefs/ui/UserPrefsComponent.java 2004-12-22 17:27:08 UTC (rev 166)
@@ -94,12 +94,25 @@
boolean acted = false;
if( null != setKey ) {
- prefs.set( setKey, setValue );
+ if( s_log.isDebugEnabled() ) {
+ s_log.debug( "Set in " +
+ ps.getRequest().getRequestURI() + '?' +
+ ps.getRequest().getQueryString() );
+ }
+
+ prefs.set( setKey, setValue,
+ ps.getRequest(), ps.getResponse() );
acted = true;
}
if( null != remove ) {
- prefs.remove( remove );
+ if( s_log.isDebugEnabled() ) {
+ s_log.debug( "Remove in " +
+ ps.getRequest().getRequestURI() + '?' +
+ ps.getRequest().getQueryString() );
+ }
+
+ prefs.remove( remove, ps.getRequest() );
acted = true;
}
|
