Author: julien(a)jboss.com Date: 2007-06-26 20:58:53 -0400 (Tue, 26 Jun 2007) New Revision: 7571 Modified: tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/core/impl/model/portal/PortalObjectImpl.java tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/test/core/model/portal/PortalObjectContainerTestCase.java Log: improve implementation of aggregated properties state management Modified: tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/core/impl/model/portal/PortalObjectImpl.java =================================================================== --- tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/core/impl/model/portal/PortalObjectImpl.java 2007-06-26 23:21:11 UTC (rev 7570) +++ tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/core/impl/model/portal/PortalObjectImpl.java 2007-06-27 00:58:53 UTC (rev 7571) @@ -69,6 +69,7 @@ private Map childrenMap; private Map properties; private Map unmodifiableProperties; + private boolean childrenAccessed; public PortalObjectImpl() { @@ -79,6 +80,12 @@ { this.declaredPropertyMap = new HashMap(); this.listener = null; + + // + this.childrenMap = null; + this.properties = null; + this.unmodifiableProperties = null; + this.childrenAccessed = false; } public Long getKey() @@ -301,6 +308,7 @@ if (childrenMap == null) { childrenMap = new HashMap(); + childrenAccessed = true; } else { @@ -347,7 +355,8 @@ ObjectNode childNode = (ObjectNode)objectNode.getChildren().get(name); if (childNode != null) { - return childNode.getObject(); + childrenAccessed = true; + return childNode.getObject(); } else { @@ -355,6 +364,9 @@ } } + /** + * Get the aggregated properties in a lazy manner. + */ public Map getProperties() { // Lazy compute properties @@ -376,6 +388,8 @@ // this.unmodifiableProperties = Collections.unmodifiableMap(properties); } + + // return unmodifiableProperties; } @@ -412,51 +426,55 @@ if (value == null) { declaredPropertyMap.remove(name); - - removeProperty(name); + String parentValue = getParent().getProperty(name); + propagatePropertyUpdate(name, parentValue, true); } else { declaredPropertyMap.put(name, value); - - setProperty(name, value); + propagatePropertyUpdate(name, value, true); } } - private void removeProperty(String name) + /** + * This method propagates a property value update to descendants which have been *loaded* from the database. + * It considers that if the <code>properties</code> field of the runtime state is null then it means that + * the object is loaded but children have not made an attempt to read the properties of this object. Indeed + * if a child is loaded any attempt to access its aggregated properties will trigger the computation of + * the aggregated properties of this object. + * + * Null property values are considered as removal + * + * @param name the property name + * @param value the new property value + * @param force the update + */ + private void propagatePropertyUpdate(String name, String value, boolean force) { - if (properties != null) - { - properties.remove(name); - } - - Iterator it = this.getChildren().iterator(); - while (it.hasNext()) - { - PortalObject object = (PortalObject) it.next(); - if (object instanceof PortalObjectImpl) - { - ((PortalObjectImpl)object).removeProperty(name); - } - } - } + if (properties != null) + { + if (force || !declaredPropertyMap.containsKey(name)) + { + if (value == null) + { + properties.remove(name); + } + else + { + properties.put(name, value); + } - private void setProperty(String name, String value) - { - if (properties != null) - { - properties.put(name, value); - } - - Iterator it = this.getChildren().iterator(); - while (it.hasNext()) - { - PortalObject object = (PortalObject) it.next(); - if (object instanceof PortalObjectImpl) - { - ((PortalObjectImpl)object).setProperty(name, value); - } - } + // + if (childrenAccessed) + { + for (Iterator i = this.getChildren().iterator();i.hasNext();) + { + PortalObjectImpl child = (PortalObjectImpl)i.next(); + child.propagatePropertyUpdate(name, value, false); + } + } + } + } } public String getProperty(String name) @@ -473,11 +491,6 @@ return (String)properties.get(name); } -// public final boolean isDashboard() -// { -// return getDashboardContext() != null; -// } - public abstract int getType(); public String toString() @@ -579,6 +592,7 @@ protected final void addChild(String name, PortalObjectImpl childObject) throws DuplicatePortalObjectException, IllegalArgumentException { objectNode.addChild(name, childObject); + childrenAccessed = true; } private PortalObjectImpl copy(PortalObjectImpl parent, String name, boolean deep) throws DuplicatePortalObjectException Modified: tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/test/core/model/portal/PortalObjectContainerTestCase.java =================================================================== --- tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/test/core/model/portal/PortalObjectContainerTestCase.java 2007-06-26 23:21:11 UTC (rev 7570) +++ tags/JBoss_Portal_2_6_0/core/src/main/org/jboss/portal/test/core/model/portal/PortalObjectContainerTestCase.java 2007-06-27 00:58:53 UTC (rev 7571) @@ -39,10 +39,7 @@ import org.jboss.portal.test.core.model.content.SimpleContent; import org.jboss.portal.test.framework.AbstractPortalTestCase; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; +import java.util.*; /** * Portal Object Container Test Cases based on the microcontainer architecture @@ -68,6 +65,96 @@ TransactionAssert.commitTransaction(); } + /** + * + */ + public void testPropertyUpdateCascadeToDescendantsWhenTheyDoNotDeclareIt() throws Exception + { + TransactionAssert.beginTransaction(); + PortalContainer ctx = container.getContext(); + Portal n1 = ctx.createPortal("default"); + Page n2 = n1.createPage("default"); + Page n3 = n2.createPage("default"); + Map p1 = n1.getProperties(); + Map p2 = n2.getProperties(); + Map p3 = n3.getProperties(); + + // + n1.setDeclaredProperty("foo", "bar1"); + assertEquals("bar1", p1.get("foo")); + assertEquals("bar1", p2.get("foo")); + assertEquals("bar1", p3.get("foo")); + + // + n2.setDeclaredProperty("foo", "bar2"); + assertEquals("bar1", p1.get("foo")); + assertEquals("bar2", p2.get("foo")); + assertEquals("bar2", p3.get("foo")); + + TransactionAssert.commitTransaction(); + } + + /** + * + */ + public void testPropertyUpdateDoesNotCascadeToDescendantsWhenTheyDeclareIt() throws Exception + { + TransactionAssert.beginTransaction(); + PortalContainer ctx = container.getContext(); + Portal n1 = ctx.createPortal("default"); + Page n2 = n1.createPage("default"); + Page n3 = n2.createPage("default"); + Map p1 = n1.getProperties(); + Map p2 = n2.getProperties(); + Map p3 = n3.getProperties(); + + // + n2.setDeclaredProperty("foo", "bar1"); + assertEquals(null, p1.get("foo")); + assertEquals("bar1", p2.get("foo")); + assertEquals("bar1", p3.get("foo")); + + // + n1.setDeclaredProperty("foo", "bar2"); + assertEquals("bar2", p1.get("foo")); + assertEquals("bar1", p2.get("foo")); + assertEquals("bar1", p3.get("foo")); + + // + TransactionAssert.commitTransaction(); + } + + /** + * + */ + public void testOverridenPropertyRemovalPropagateParentValueToDescendants() throws Exception + { + TransactionAssert.beginTransaction(); + PortalContainer ctx = container.getContext(); + Portal n1 = ctx.createPortal("default"); + Page n2 = n1.createPage("default"); + Page n3 = n2.createPage("default"); + Map p1 = n1.getProperties(); + Map p2 = n2.getProperties(); + Map p3 = n3.getProperties(); + + // + n1.setDeclaredProperty("foo", "bar1"); + n2.setDeclaredProperty("foo", "bar2"); + assertEquals("bar1", p1.get("foo")); + assertEquals("bar2", p2.get("foo")); + assertEquals("bar2", p3.get("foo")); + + // + n2.setDeclaredProperty("foo", null); + assertEquals("bar1", p1.get("foo")); + assertEquals("bar1", p2.get("foo")); + assertEquals("bar1", p3.get("foo")); + + // + TransactionAssert.commitTransaction(); + } + public void testCRUD() throws Exception { TransactionAssert.beginTransaction();