You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by gc...@apache.org on 2008/02/01 20:25:06 UTC
svn commit: r617615 - in /myfaces/trinidad/trunk_1.2.x/trinidad-api/src:
main/java/org/apache/myfaces/trinidad/bean/util/StateUtils.java
test/java/org/apache/myfaces/trinidad/bean/FacesBeanImplTest.java
Author: gcrawford
Date: Fri Feb 1 11:24:50 2008
New Revision: 617615
URL: http://svn.apache.org/viewvc?rev=617615&view=rev
Log:
TRINIDAD-891 Locking issues coming from restore state
store a map of name->class on map returned from requestContext.getApplicationScopedAppMap()
Modified:
myfaces/trinidad/trunk_1.2.x/trinidad-api/src/main/java/org/apache/myfaces/trinidad/bean/util/StateUtils.java
myfaces/trinidad/trunk_1.2.x/trinidad-api/src/test/java/org/apache/myfaces/trinidad/bean/FacesBeanImplTest.java
Modified: myfaces/trinidad/trunk_1.2.x/trinidad-api/src/main/java/org/apache/myfaces/trinidad/bean/util/StateUtils.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk_1.2.x/trinidad-api/src/main/java/org/apache/myfaces/trinidad/bean/util/StateUtils.java?rev=617615&r1=617614&r2=617615&view=diff
==============================================================================
--- myfaces/trinidad/trunk_1.2.x/trinidad-api/src/main/java/org/apache/myfaces/trinidad/bean/util/StateUtils.java (original)
+++ myfaces/trinidad/trunk_1.2.x/trinidad-api/src/main/java/org/apache/myfaces/trinidad/bean/util/StateUtils.java Fri Feb 1 11:24:50 2008
@@ -6,9 +6,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -21,16 +21,19 @@
import java.io.Serializable;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
+
import javax.faces.component.StateHolder;
import javax.faces.context.FacesContext;
-
import org.apache.myfaces.trinidad.bean.FacesBean;
import org.apache.myfaces.trinidad.bean.PropertyKey;
import org.apache.myfaces.trinidad.bean.PropertyMap;
+import org.apache.myfaces.trinidad.context.RequestContext;
import org.apache.myfaces.trinidad.logging.TrinidadLogger;
@@ -222,8 +225,8 @@
Object[] array = new Object[size];
// 2006-08-01: -= Simon Lessard =-
- // Inefficient loop if the list implementation
- // ever change to a linked data structure. Use
+ // Inefficient loop if the list implementation
+ // ever change to a linked data structure. Use
// iterators instead
//for (int i = 0; i < size; i++)
// array[i] = saveStateHolder(context, list.get(i));
@@ -232,7 +235,7 @@
{
array[index++] = saveStateHolder(context, object);
}
-
+
return array;
}
@@ -279,10 +282,48 @@
public Object restoreState(FacesContext context)
{
- ClassLoader cl = _getClassLoader();
+ // we don't need to use concurrent map methods like putIfAbsent. If someone happens to
+ // add a name/value pair again it's fine because as the doc for put in HashMap says
+ // "If the map previously contained a mapping for this key, the old value is replaced."
+ ConcurrentMap<String, Object> appMap =
+ RequestContext.getCurrentInstance().getApplicationScopedConcurrentMap();
+
+
+ Map<String, Class> classMap = (Map<String, Class>) appMap.get(_CLASS_MAP_KEY);
+
+ if (classMap == null)
+ {
+ // the classMap doesn't need to worry about synchronization,
+ // if the Class is loaded twice that's fine.
+ Map<String, Class> newClassMap = new HashMap<String, Class>();
+ Map<String, Class> oldClassMap =
+ (Map<String, Class>) appMap.putIfAbsent(_CLASS_MAP_KEY, newClassMap);
+
+ if (oldClassMap != null)
+ classMap = oldClassMap;
+ else
+ classMap = newClassMap;
+ }
+
+ Class clazz = classMap.get(_name);
+
+ if (clazz == null)
+ {
+ try
+ {
+ ClassLoader cl = _getClassLoader();
+ clazz = cl.loadClass(_name);
+ classMap.put(_name, clazz);
+ }
+ catch (Throwable t)
+ {
+ _LOG.severe(t);
+ return null;
+ }
+ }
+
try
{
- Class<?> clazz = cl.loadClass(_name);
return clazz.newInstance();
}
catch (Throwable t)
@@ -337,5 +378,10 @@
}
static private final TrinidadLogger _LOG = TrinidadLogger.createTrinidadLogger(StateUtils.class);
+
+ private static final String _CLASS_MAP_KEY =
+ "org.apache.myfaces.trinidad.bean.util.CLASS_MAP_KEY";
+
+
}
Modified: myfaces/trinidad/trunk_1.2.x/trinidad-api/src/test/java/org/apache/myfaces/trinidad/bean/FacesBeanImplTest.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk_1.2.x/trinidad-api/src/test/java/org/apache/myfaces/trinidad/bean/FacesBeanImplTest.java?rev=617615&r1=617614&r2=617615&view=diff
==============================================================================
--- myfaces/trinidad/trunk_1.2.x/trinidad-api/src/test/java/org/apache/myfaces/trinidad/bean/FacesBeanImplTest.java (original)
+++ myfaces/trinidad/trunk_1.2.x/trinidad-api/src/test/java/org/apache/myfaces/trinidad/bean/FacesBeanImplTest.java Fri Feb 1 11:24:50 2008
@@ -6,9 +6,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -34,6 +34,7 @@
import junit.framework.TestCase;
import junit.framework.TestSuite;
+import org.apache.myfaces.trinidad.context.MockRequestContext;
import org.apache.myfaces.trinidadbuild.test.FacesTestCase;
public class FacesBeanImplTest extends FacesTestCase
@@ -42,7 +43,7 @@
{
return new TestSuite(FacesBeanImplTest.class);
}
-
+
public static void main(String[] args) throws Throwable
{
junit.textui.TestRunner.run(suite());
@@ -54,15 +55,21 @@
super(testName);
}
+
+ private MockRequestContext _mafct;
+
@Override
protected void setUp() throws Exception
{
super.setUp();
+ _mafct = new MockRequestContext();
}
@Override
protected void tearDown() throws Exception
{
+ _mafct.release();
+ _mafct = null;
super.tearDown();
}
@@ -166,7 +173,7 @@
bean.setFirst("first");
assertEquals("first", bean.getFirst());
-
+
bean.setFirst(null);
assertEquals("vbFirst", bean.getFirst());
}
@@ -176,7 +183,7 @@
TestBean bean = new TestBean();
assertTrue(bean.keySet().isEmpty());
assertTrue(bean.bindingKeySet().isEmpty());
-
+
bean.setFirst("first");
bean.setSecond("second");
@@ -185,9 +192,9 @@
bean.setSecond("newSecond");
assertEquals(2, bean.keySet().size());
-
+
bean.setSecond(null);
-
+
// This test is somewhat dubious...
assertEquals(1, bean.keySet().size());
@@ -202,8 +209,8 @@
bean.setValueBinding(thirdKey, new TestValueBinding());
assertEquals(2, bean.bindingKeySet().size());
-
- assertTrue(bean.bindingKeySet().contains(thirdKey));
+
+ assertTrue(bean.bindingKeySet().contains(thirdKey));
assertTrue(bean.bindingKeySet().contains(TestBean.FIRST_KEY));
assertTrue(!bean.bindingKeySet().contains(TestBean.SECOND_KEY));
}
@@ -222,7 +229,7 @@
bean.addItem(new Integer(2));
assertEquals(2, bean.getItems().length);
-
+
array = bean.getItems();
assertEquals(array[0], new Integer(1));
assertEquals(array[1], new Integer(2));
@@ -257,7 +264,7 @@
bean.setProperty(TestBean.ITEMS_KEY, "Shouldn't work");
fail();
}
- catch (IllegalArgumentException iae)
+ catch (IllegalArgumentException iae)
{
// expected
}
@@ -319,7 +326,7 @@
bean.removeEntry(TestBean.FIRST_KEY, null);
fail();
}
- catch (IllegalArgumentException iae)
+ catch (IllegalArgumentException iae)
{
// expected
}
@@ -436,7 +443,7 @@
assertEquals("second", newBean.getSecond());
assertNull(newBean.getValueBinding(SubTypeBean.SECOND_KEY));
assertEquals("sub", newBean.getSub());
-
+
// Verify that our "silly" object has had its state restored
assertEquals("2", newBean.getProperty(SubTypeBean.SILLY_KEY).toString());
@@ -445,7 +452,7 @@
assertEquals(new Integer(1), array[0]);
assertEquals(new Integer(2), array[1]);
-
+
// Make sure the transient value is now null
assertNull(newBean.getTransient());
@@ -455,26 +462,26 @@
assertTrue(vb instanceof TestValueBinding);
assertTrue(vb != vb1);
assertEquals(vb.getValue(null), "vbFirst");
-
+
// Now change the value binding, and verify the original
// bean is unchanged
vb.setValue(null, "changedVB");
assertEquals("changedVB", newBean.getFirst());
assertEquals("vbFirst", bean.getFirst());
-
+
// Now, verify that if we mark the initial state and save, that we get
// a non-null value
newBean.markInitialState();
assertNull(newBean.saveState(null));
-
+
// Now, we'll set a value, so we should get a non-null state
-
+
// Our current delta support *does not* keep track of the original value.
// If it does, add this test
// String oldFirst = newBean.getFirst();
newBean.setFirst("foo");
assertNotNull(newBean.saveState(null));
-
+
// Our current delta support *does not* keep track of the original value.
// If it does, add this test
// newBean.setFirst(oldFirst);