From ae3c331c7dfb5e3641b8f13100f7a402eb61683e Mon Sep 17 00:00:00 2001 From: hns Date: Mon, 7 Apr 2008 15:24:11 +0000 Subject: [PATCH] * Fix several bugs in grouped collections: --- src/helma/objectmodel/db/DbMapping.java | 22 ++++- src/helma/objectmodel/db/Node.java | 122 +++++++++++++++--------- 2 files changed, 95 insertions(+), 49 deletions(-) diff --git a/src/helma/objectmodel/db/DbMapping.java b/src/helma/objectmodel/db/DbMapping.java index 4aa9ff58..be2f199a 100644 --- a/src/helma/objectmodel/db/DbMapping.java +++ b/src/helma/objectmodel/db/DbMapping.java @@ -124,7 +124,10 @@ public final class DbMapping { HashSet dependentMappings = new HashSet(); // does this DbMapping describe a virtual node (collection, mountpoint, groupnode)? - private boolean virtual = false; + private boolean isVirtual = false; + + // does this Dbmapping describe a group node? + private boolean isGroup = false; /** * Create an internal DbMapping used for "virtual" mappings aka collections, mountpoints etc. @@ -132,7 +135,7 @@ public final class DbMapping { public DbMapping(Application app, String parentTypeName) { this(app, parentTypeName, null); // DbMappings created with this constructor always define virtual nodes - virtual = true; + isVirtual = true; if (parentTypeName != null) { parentMapping = app.getDbMapping(parentTypeName); if (parentMapping == null) { @@ -756,9 +759,9 @@ public final class DbMapping { * db-mapping with the right relations to create the group-by nodes */ public synchronized DbMapping getGroupbyMapping() { - if ((subRelation == null) && (parentMapping != null)) { + if ((subRelation == null) && (parentMapping != null)) { return parentMapping.getGroupbyMapping(); - } else if (subRelation.groupby == null) { + } else if (subRelation == null || subRelation.groupby == null) { return null; } else if (groupbyMapping == null) { initGroupbyMapping(); @@ -774,6 +777,7 @@ public final class DbMapping { // if a prototype is defined for groupby nodes, use that // if mapping doesn' exist or isn't defined, create a new (anonymous internal) one groupbyMapping = new DbMapping(app, subRelation.groupbyPrototype); + groupbyMapping.isGroup = true; // set subnode and property relations groupbyMapping.subRelation = subRelation.getGroupbySubnodeRelation(); @@ -1596,6 +1600,14 @@ public final class DbMapping { * @return true if this instance describes a virtual node. */ public boolean isVirtual() { - return virtual; + return isVirtual; + } + + /** + * Find if this DbMapping describes a group node. + * @return true if this instance describes a group node. + */ + public boolean isGroup() { + return isGroup; } } \ No newline at end of file diff --git a/src/helma/objectmodel/db/Node.java b/src/helma/objectmodel/db/Node.java index 12006247..4e9a7b60 100644 --- a/src/helma/objectmodel/db/Node.java +++ b/src/helma/objectmodel/db/Node.java @@ -869,26 +869,10 @@ public final class Node implements INode, Serializable { loadNodes(); // check if this node has a group-by subnode-relation - if (dbmap != null) { - Relation srel = dbmap.getSubnodeRelation(); - - if ((srel != null) && (srel.groupby != null)) { - Relation groupbyRel = srel.otherType.columnNameToRelation(srel.groupby); - String groupbyProp = (groupbyRel != null) ? groupbyRel.propName - : srel.groupby; - String groupbyValue = node.getString(groupbyProp); - INode groupbyNode = (INode) getChildElement(groupbyValue); - - // if group-by node doesn't exist, we'll create it - if (groupbyNode == null) { - groupbyNode = getGroupbySubnode(groupbyValue, true); - } else { - groupbyNode.setDbMapping(dbmap.getGroupbyMapping()); - } - - groupbyNode.addNode(node); - return node; - } + INode groupbyNode = getGroupbySubnode(node, true); + if (groupbyNode != null) { + groupbyNode.addNode(node); + return node; } NodeHandle nhandle = node.getHandle(); @@ -1198,6 +1182,38 @@ public final class Node implements INode, Serializable { return retval; } + protected Node getGroupbySubnode(Node node, boolean create) { + if (node.dbmap != null && node.dbmap.isGroup()) { + return null; + } + + if (dbmap != null) { + Relation srel = dbmap.getSubnodeRelation(); + + if ((srel != null) && (srel.groupby != null)) { + Relation groupbyRel = srel.otherType.columnNameToRelation(srel.groupby); + String groupbyProp = (groupbyRel != null) ? groupbyRel.propName + : srel.groupby; + String groupbyValue = node.getString(groupbyProp); + Node groupbyNode = (Node) getChildElement(groupbyValue); + + // if group-by node doesn't exist, we'll create it + if (groupbyNode == null) { + groupbyNode = getGroupbySubnode(groupbyValue, create); + // mark subnodes as changed as we have a new group node + if (create && groupbyNode != null) { + Transactor.getInstance().visitParentNode(this); + } + } else { + groupbyNode.setDbMapping(dbmap.getGroupbyMapping()); + } + + return groupbyNode; + } + } + return null; + } + /** * * @@ -1211,10 +1227,7 @@ public final class Node implements INode, Serializable { throw new IllegalArgumentException("Can't create group by null"); } - if (state == TRANSIENT) { - throw new RuntimeException("Can't add grouped child on transient node. "+ - "Make parent persistent before adding grouped nodes."); - } + boolean persistent = state != TRANSIENT; loadNodes(); @@ -1228,34 +1241,40 @@ public final class Node implements INode, Serializable { boolean relational = groupbyMapping.getSubnodeMapping().isRelational(); if (relational || create) { - Node node = relational ? new Node(this, sid, nmgr, null) - : new Node(sid, null, nmgr); + Node node = relational && persistent ? + new Node(this, sid, nmgr, null) : + new Node(sid, null, nmgr); // set "groupname" property to value of groupby field node.setString("groupname", sid); - + // Set the dbmapping on the group node node.setDbMapping(groupbyMapping); + node.setPrototype(groupbyMapping.getTypeName()); - if (!relational) { - // if we're not transient, make new node persistable - if (state != TRANSIENT) { - node.makePersistable(); - node.checkWriteLock(); - } - subnodes.add(node.getHandle()); + // if we're relational and persistent, make new node persistable + if (!relational && persistent) { + node.makePersistable(); + node.checkWriteLock(); + } + + // if we created a new node, check if we need to add it to subnodes + if (create) { + NodeHandle handle = node.getHandle(); + if (!subnodes.contains(handle)) + subnodes.add(handle); } - // Set the dbmapping on the group node - node.setPrototype(groupbyMapping.getTypeName()); // If we created the group node, we register it with the // nodemanager. Otherwise, we just evict whatever was there before - if (create) { - // register group node with transactor - Transactor tx = Transactor.getInstanceOrFail(); - tx.visitCleanNode(node); - nmgr.registerNode(node); - } else { - nmgr.evictKey(node.getKey()); + if (persistent) { + if (create) { + // register group node with transactor + Transactor tx = Transactor.getInstanceOrFail(); + tx.visitCleanNode(node); + nmgr.registerNode(node); + } else { + nmgr.evictKey(node.getKey()); + } } return node; @@ -1299,6 +1318,13 @@ public final class Node implements INode, Serializable { * {@link #removeNode(INode)}. */ protected void releaseNode(Node node) { + + Node groupNode = getGroupbySubnode(node, false); + if (groupNode != null) { + groupNode.releaseNode(node); + return; + } + INode parent = node.getParent(); checkWriteLock(); @@ -1314,7 +1340,9 @@ public final class Node implements INode, Serializable { synchronized (subnodes) { removed = subnodes.remove(node.getHandle()); } - if (removed) { + if (dbmap != null && dbmap.isGroup() && subnodes.size() == 0) { + remove(); + } else if (removed) { registerSubnodeChange(); } } @@ -1341,6 +1369,12 @@ public final class Node implements INode, Serializable { nmgr.evictKey(new SyntheticKey(getKey(), prop)); } } + } else if (prel.groupby != null) { + String prop = node.getString("groupname"); + if (prop != null && state != TRANSIENT) { + nmgr.evictKey(new SyntheticKey(getKey(), prop)); + } + } // TODO: We should unset constraints to actually remove subnodes here, // but omit it by convention and to keep backwards compatible.