jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ste...@apache.org
Subject svn commit: r106099 - /incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemManager.java /incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java /incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
Date Sun, 21 Nov 2004 17:16:51 GMT
Author: stefan
Date: Sun Nov 21 09:16:49 2004
New Revision: 106099

Modified:
   incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemManager.java
   incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java
   incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
Log:
fixed bug that occured when calling Node.setProperty, specifying a non-existing property and
an illegal value: 

exception was thrown but property had nevertheless been created with null value

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemManager.java
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemManager.java	(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemManager.java	Sun Nov
21 09:16:49 2004
@@ -24,7 +24,9 @@
 import javax.jcr.nodetype.NodeDef;
 import javax.jcr.nodetype.PropertyDef;
 import java.io.PrintStream;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Map;
 
 /**
  * There's one <code>ItemManager</code> instance per <code>Session</code>
@@ -43,7 +45,7 @@
  * <code>Node</code> or <code>Property</code> associated with the
same
  * <code>Session</code> instance.
  * <li>maintaining a cache of the item instances it created.
- * <li>checking access rights of associated <code>Session</code> in all
methods.
+ * <li>respecting access rights of associated <code>Session</code> in all
methods.
  * </ul>
  * <p/>
  * If the parent <code>Session</code> is an <code>XASession</code>,
there is
@@ -143,36 +145,96 @@
 
     //--------------------------------------------------< item access methods >
     /**
-     * @param path
-     * @return
+     * Checks if the item with the given path exists.
+     *
+     * @param path path to the item to be checked
+     * @return true if the specified item exists
      */
     boolean itemExists(Path path) {
+/*
         try {
             getItem(path);
             return true;
         } catch (PathNotFoundException pnfe) {
             return false;
         } catch (AccessDeniedException ade) {
+            // item exists but the session has not been granted read access
+            return false;
+        } catch (RepositoryException re) {
+            return false;
+        }
+*/
+        try {
+            // check sanity of session
+            session.sanityCheck();
+
+            ItemId id = hierMgr.resolvePath(path);
+
+            // check if state exists for the given item
+            if (!itemStateProvider.hasItemState(id)) {
+                return false;
+            }
+
+            // check privileges
+            if (!session.getAccessManager().isGranted(id, AccessManager.READ)) {
+                // clear cache
+                if (isCached(id)) {
+                    evictItem(id);
+                }
+                // item exists but the session has not been granted read access
+                return false;
+            }
             return true;
+        } catch (PathNotFoundException pnfe) {
+            return false;
+        } catch (ItemNotFoundException infe) {
+            return false;
         } catch (RepositoryException re) {
             return false;
         }
     }
 
     /**
-     * Checks if the item with the given id exists
+     * Checks if the item with the given id exists.
      *
-     * @param id
-     * @return
+     * @param id id of the item to be checked
+     * @return true if the specified item exists
      */
     boolean itemExists(ItemId id) {
+/*
         try {
             getItem(id);
             return true;
         } catch (ItemNotFoundException infe) {
             return false;
         } catch (AccessDeniedException ade) {
+            // item exists but the session has not been granted read access
+            return false;
+        } catch (RepositoryException re) {
+            return false;
+        }
+*/
+        try {
+            // check sanity of session
+            session.sanityCheck();
+
+            // check if state exists for the given item
+            if (!itemStateProvider.hasItemState(id)) {
+                return false;
+            }
+
+            // check privileges
+            if (!session.getAccessManager().isGranted(id, AccessManager.READ)) {
+                // clear cache
+                if (isCached(id)) {
+                    evictItem(id);
+                }
+                // item exists but the session has not been granted read access
+                return false;
+            }
             return true;
+        } catch (ItemNotFoundException infe) {
+            return false;
         } catch (RepositoryException re) {
             return false;
         }

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java	(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java	Sun Nov 21
09:16:49 2004
@@ -45,6 +45,9 @@
 
     protected NodeDef definition;
 
+    // flag set in status passed to getOrCreateProperty if property was created
+    protected static final short CREATED = 0;
+
     /**
      * Package private constructor.
      *
@@ -164,15 +167,10 @@
         return genValues;
     }
 
-    protected PropertyImpl getOrCreateProperty(String name, int type, boolean multiValued)
+    protected PropertyImpl getOrCreateProperty(String name, int type,
+                                               boolean multiValued,
+                                               BitSet status)
             throws RepositoryException {
-        try {
-            return (PropertyImpl) getProperty(name);
-        } catch (PathNotFoundException pnfe) {
-            // fall through
-        }
-
-        // property does not exist yet...
         QName qName;
         try {
             qName = QName.fromJCRName(name, session.getNamespaceResolver());
@@ -181,21 +179,25 @@
         } catch (UnknownPrefixException upe) {
             throw new RepositoryException("invalid property name: " + name, upe);
         }
-        // find definition for the specified property and create property
-        PropertyDefImpl def = getApplicablePropertyDef(qName, type, multiValued);
-        return createChildProperty(qName, type, def);
+        return getOrCreateProperty(qName, type, multiValued, status);
     }
 
-    protected PropertyImpl getOrCreateProperty(QName name, int type, boolean multiValued)
+    protected synchronized PropertyImpl getOrCreateProperty(QName name, int type,
+                                                            boolean multiValued,
+                                                            BitSet status)
             throws RepositoryException {
-        try {
-            return (PropertyImpl) getProperty(name);
-        } catch (ItemNotFoundException e) {
-            // does not exist yet:
-            // find definition for the specified property and create property
-            PropertyDefImpl def = getApplicablePropertyDef(name, type, multiValued);
-            return createChildProperty(name, type, def);
+        status.clear();
+
+        PropertyId propId = new PropertyId(((NodeState) state).getUUID(), name);
+        if (itemMgr.itemExists(propId)) {
+            return getProperty(name);
         }
+        // does not exist yet:
+        // find definition for the specified property and create property
+        PropertyDefImpl def = getApplicablePropertyDef(name, type, multiValued);
+        PropertyImpl prop = createChildProperty(name, type, def);
+        status.set(CREATED);
+        return prop;
     }
 
     protected synchronized PropertyImpl createChildProperty(QName name, int type, PropertyDefImpl
def)
@@ -319,6 +321,19 @@
         return (NodeImpl) itemMgr.getItem(targetId);
     }
 
+
+    protected void removeChildProperty(String propName) throws RepositoryException {
+        QName qName;
+        try {
+            qName = QName.fromJCRName(propName, session.getNamespaceResolver());
+        } catch (IllegalNameException ine) {
+            throw new RepositoryException("invalid property name: " + propName, ine);
+        } catch (UnknownPrefixException upe) {
+            throw new RepositoryException("invalid property name: " + propName, upe);
+        }
+        removeChildProperty(qName);
+    }
+
     protected void removeChildProperty(QName propName) throws RepositoryException {
         // modify the state of 'this', i.e. the parent node
         NodeState thisState = (NodeState) getOrCreateTransientItemState();
@@ -689,11 +704,22 @@
         } else {
             type = value.getType();
         }
-        PropertyImpl prop = getOrCreateProperty(name, type, false);
-        if (value == null) {
-            prop.internalSetValue((InternalValue[]) null, type);
-        } else {
-            prop.internalSetValue(new InternalValue[]{value}, type);
+
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, type, false, status);
+        try {
+            if (value == null) {
+                prop.internalSetValue((InternalValue[]) null, type);
+            } else {
+                prop.internalSetValue(new InternalValue[]{value}, type);
+            }
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
         }
         return prop;
     }
@@ -722,8 +748,19 @@
         } else {
             type = values[0].getType();
         }
-        PropertyImpl prop = getOrCreateProperty(name, type, true);
-        prop.internalSetValue(values, type);
+
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, type, true, status);
+        try {
+            prop.internalSetValue(values, type);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -899,8 +936,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.NAME, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.NAME, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -919,8 +966,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.NAME, true);
-        prop.setValue(values);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.NAME, true, status);
+        try {
+            prop.setValue(values);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -946,8 +1003,18 @@
         } else {
             type = values[0].getType();
         }
-        PropertyImpl prop = getOrCreateProperty(name, type, true);
-        prop.setValue(values);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, type, true, status);
+        try {
+            prop.setValue(values);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -968,8 +1035,19 @@
         sanityCheck();
 
         int type = (value == null) ? PropertyType.STRING : value.getType();
-        PropertyImpl prop = getOrCreateProperty(name, type, false);
-        prop.setValue(value);
+
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, type, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1204,8 +1282,19 @@
         } else {
             type = values[0].getType();
         }
-        PropertyImpl prop = getOrCreateProperty(name, type, true);
-        prop.setValue(values);
+
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, type, true, status);
+        try {
+            prop.setValue(values);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1217,8 +1306,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.STRING, true);
-        prop.setValue(values);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.STRING, true, status);
+        try {
+            prop.setValue(values);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1229,8 +1328,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.STRING, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.STRING, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1243,8 +1352,19 @@
         sanityCheck();
 
         int type = (value == null) ? PropertyType.STRING : value.getType();
-        PropertyImpl prop = getOrCreateProperty(name, type, false);
-        prop.setValue(value);
+
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, type, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1256,8 +1376,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.BINARY, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.BINARY, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1269,8 +1399,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.BOOLEAN, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.BOOLEAN, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1282,8 +1422,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.DOUBLE, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.DOUBLE, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1295,8 +1445,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.LONG, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.LONG, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1308,8 +1468,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.DATE, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.DATE, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 
@@ -1321,8 +1491,18 @@
         // check state of this instance
         sanityCheck();
 
-        PropertyImpl prop = getOrCreateProperty(name, PropertyType.REFERENCE, false);
-        prop.setValue(value);
+        BitSet status = new BitSet();
+        PropertyImpl prop = getOrCreateProperty(name, PropertyType.REFERENCE, false, status);
+        try {
+            prop.setValue(value);
+        } catch (RepositoryException re) {
+            if (status.get(CREATED)) {
+                // setting value failed, get rid of newly created property
+                removeChildProperty(name);
+            }
+            // rethrow
+            throw re;
+        }
         return prop;
     }
 

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
Sun Nov 21 09:16:49 2004
@@ -248,10 +248,34 @@
      * @see ItemStateProvider#hasItemState(ItemId)
      */
     public boolean hasItemState(ItemId id) {
-        return transientStateMgr.hasItemStateInAttic(id)
-                || transientStateMgr.hasItemState(id)
-                || persistentStateMgr.hasItemState(id)
-                || hasVirtualItemState(id);
+	// first check if the specified item has been transiently removed
+	if (transientStateMgr.hasItemStateInAttic(id)) {
+	    /**
+	     * check if there's new transient state for the specified item
+	     * (e.g. if a property with name 'x' has been removed and a new
+	     * property with same name has been created);
+	     */
+	    return transientStateMgr.hasItemState(id);
+	}
+
+	// check if there's transient state for the specified item
+	if (transientStateMgr.hasItemState(id)) {
+	    return true;
+	}
+
+	// check if there is a virtual state for the specified item
+	for (int i = 0; i < virtualProviders.length; i++) {
+	    if (virtualProviders[i].hasItemState(id)) {
+		return true;
+	    }
+	}
+
+	// check if there's persistent state for the specified item
+	if (persistentStateMgr.hasItemState(id)) {
+	    return true;
+	}
+
+	return false;
     }
 
     /**

Mime
View raw message