Return-Path: 
 <jackrabbit-commits-return-1219-apmail-incubator-jackrabbit-commits-archive=www.apache.org@incubator.apache.org>
Delivered-To: apmail-incubator-jackrabbit-commits-archive@www.apache.org
Received: (qmail 11809 invoked from network); 13 Jul 2005 15:45:38 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199)
  by minotaur.apache.org with SMTP; 13 Jul 2005 15:45:38 -0000
Received: (qmail 90840 invoked by uid 500); 13 Jul 2005 15:45:37 -0000
Mailing-List: contact jackrabbit-commits-help@incubator.apache.org;
 run by ezmlm
Precedence: bulk
List-Help: <mailto:jackrabbit-commits-help@incubator.apache.org>
List-Unsubscribe: <mailto:jackrabbit-commits-unsubscribe@incubator.apache.org>
List-Post: <mailto:jackrabbit-commits@incubator.apache.org>
List-Id: <jackrabbit-commits.incubator.apache.org>
Reply-To: jackrabbit-dev@incubator.apache.org
Delivered-To: mailing list jackrabbit-commits@incubator.apache.org
Received: (qmail 90827 invoked by uid 500); 13 Jul 2005 15:45:37 -0000
Delivered-To: apmail-incubator-jackrabbit-cvs@incubator.apache.org
Received: (qmail 90824 invoked by uid 99); 13 Jul 2005 15:45:37 -0000
X-ASF-Spam-Status: No, hits=-9.8 required=10.0
	tests=ALL_TRUSTED,NO_REAL_NAME
X-Spam-Check-By: apache.org
Received: from [209.237.227.194] (HELO minotaur.apache.org) (209.237.227.194)
    by apache.org (qpsmtpd/0.29) with SMTP; Wed, 13 Jul 2005 08:45:26 -0700
Received: (qmail 11693 invoked by uid 65534); 13 Jul 2005 15:45:24 -0000
Message-ID: <20050713154524.11690.qmail@minotaur.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r216177 - in
 /incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core:
 LazyItemIterator.java NodeImpl.java version/VersionHistoryImpl.java
 version/VersionImpl.java
Date: Wed, 13 Jul 2005 15:45:22 -0000
To: jackrabbit-cvs@incubator.apache.org
From: stefan@apache.org
X-Mailer: svnmailer-1.0.2
X-Virus-Checked: Checked by ClamAV on apache.org
X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N

Author: stefan
Date: Wed Jul 13 08:45:20 2005
New Revision: 216177

URL: http://svn.apache.org/viewcvs?rev=216177&view=rev
Log:
optimized LazyItemIterator and made it more forgiving

Modified:
    incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/LazyItemIterator.java
    incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/NodeImpl.java
    incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionHistoryImpl.java
    incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionImpl.java

Modified: incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/LazyItemIterator.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/LazyItemIterator.java?rev=216177&r1=216176&r2=216177&view=diff
==============================================================================
--- incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/LazyItemIterator.java (original)
+++ incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/LazyItemIterator.java Wed Jul 13 08:45:20 2005
@@ -18,38 +18,46 @@
 
 import org.apache.log4j.Logger;
 
+import javax.jcr.Item;
 import javax.jcr.Node;
 import javax.jcr.NodeIterator;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
-import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
+import java.util.ArrayList;
 
 /**
  * <code>LazyItemIterator</code> is an id-based iterator that instantiates
  * the <code>Item</code>s only when they are requested.
+ * <p/>
+ * <strong>Important:</strong> <code>Item</code>s that appear to be nonexistent
+ * for some reason (e.g. because of insufficient access rights or because they
+ * have been removed since the iterator has been retrieved) are silently
+ * skipped. As a result the size of the iterator as reported by
+ * {@link #getSize()} might appear to be shrinking while iterating over the
+ * items.
+ * todo should getSize() better always return -1?
+ *
+ * @see #getSize()
  */
 class LazyItemIterator implements NodeIterator, PropertyIterator {
 
+    /** Logger instance for this class */
     private static Logger log = Logger.getLogger(LazyItemIterator.class);
 
-    /**
-     * the item manager that is used to fetch the items
-     */
+    /** the item manager that is used to lazily fetch the items */
     private final ItemManager itemMgr;
 
-    /**
-     * the list of item ids
-     */
+    /** the list of item ids */
     private final List idList;
 
-    /**
-     * the position of the next item
-     */
-    private int pos = 0;
+    /** the position of the next item */
+    private int pos;
+
+    /** prefetched item to be returned on <code>{@link #next()}</code> */
+    private Item next;
 
     /**
      * Creates a new <code>LazyItemIterator</code> instance.
@@ -58,35 +66,40 @@
      * @param idList  list of item id's
      */
     public LazyItemIterator(ItemManager itemMgr, List idList) {
-        this(itemMgr, idList, false);
+        this.itemMgr = itemMgr;
+        this.idList = new ArrayList(idList);
+        // prefetch first item
+        pos = 0;
+        prefetchNext();
     }
 
     /**
-     * Creates a new <code>LazyItemIterator</code> instance.
-     *
-     * @param itemMgr        item manager
-     * @param idList         list of item id's
-     * @param skipInexistent if <code>true</code> the id's of those items
-     *                       that appear to be non-existent will be filtered
-     *                       out silently; otherwise such entries will cause
-     *                       a <code>NoSuchElementException</code> on
-     *                       <code>{@link #next()}</code> .
-     */
-    public LazyItemIterator(ItemManager itemMgr, List idList,
-                            boolean skipInexistent) {
-        this.itemMgr = itemMgr;
-        if (skipInexistent) {
-            // check existence of all items first
-            this.idList = new ArrayList();
-            Iterator iter = idList.iterator();
-            while (iter.hasNext()) {
-                ItemId id = (ItemId) iter.next();
-                if (itemMgr.itemExists(id)) {
-                    this.idList.add(id);
-                }
+     * Prefetches next item.
+     * <p/>
+     * {@link #next} is set to the next available item in this iterator or to
+     * <code>null</code> in case there are no more items.
+     */
+    private void prefetchNext() {
+        // reset
+        next = null;
+        while (next == null && pos < idList.size()) {
+            ItemId id = (ItemId) idList.get(pos);
+            if (!itemMgr.itemExists(id)) {
+                log.debug("ignoring nonexistent item " + id);
+                // remove invalid id
+                idList.remove(pos);
+                // try next
+                continue;
+            }
+            try {
+                next = itemMgr.getItem(id);
+            } catch (RepositoryException e) {
+                // should never get here since existence has already been checked...
+                log.error("failed to fetch item " + id + ", skipping...", e);
+                // remove invalid id
+                idList.remove(pos);
+                // try next
             }
-        } else {
-            this.idList = idList;
         }
     }
 
@@ -106,7 +119,7 @@
         return (Property) next();
     }
 
-    //------------------------------------------------------< RangeIterator >---
+    //--------------------------------------------------------< RangeIterator >
     /**
      * {@inheritDoc}
      */
@@ -116,6 +129,13 @@
 
     /**
      * {@inheritDoc}
+     * <p/>
+     * Note that the size of the iterator as reported by {@link #getSize()}
+     * might appear to be shrinking while iterating because items that for
+     * some reason cannot be retrieved through this iterator are silently
+     * skipped, thus reducing the size of this iterator.
+     *
+     * todo better to always return -1?
      */
     public long getSize() {
         return idList.size();
@@ -126,12 +146,42 @@
      */
     public void skip(long skipNum) {
         if (skipNum < 0) {
-            throw new IllegalArgumentException("skipNum must be a positive number");
+            throw new IllegalArgumentException("skipNum must not be negative");
+        }
+        if (skipNum == 0) {
+            return;
+        }
+        if (next == null) {
+            throw new NoSuchElementException();
         }
-        if (pos + skipNum > idList.size()) {
-            throw new NoSuchElementException("skipNum + pos greater than size");
+
+        // reset
+        next = null;
+        // skip the first (skipNum - 1) items without actually retrieving them
+        while (--skipNum > 0) {
+            pos++;
+            if (pos >= idList.size()) {
+                // skipped past last item
+                throw new NoSuchElementException();
+            }
+            ItemId id = (ItemId) idList.get(pos);
+            // eliminate invalid items from this iterator
+            while (!itemMgr.itemExists(id)) {
+                log.debug("ignoring nonexistent item " + id);
+                // remove invalid id
+                idList.remove(pos);
+                if (pos >= idList.size()) {
+                    // skipped past last item
+                    throw new NoSuchElementException();
+                }
+                id = (ItemId) idList.get(pos);
+                // try next
+                continue;
+            }
         }
-        pos += skipNum;
+        // prefetch final item (the one to be returned on next())
+        pos++;
+        prefetchNext();
     }
 
     //-------------------------------------------------------------< Iterator >
@@ -139,23 +189,20 @@
      * {@inheritDoc}
      */
     public boolean hasNext() {
-        return pos < idList.size();
+        return next != null;
     }
 
     /**
      * {@inheritDoc}
      */
     public Object next() {
-        if (pos >= idList.size()) {
+        if (next == null) {
             throw new NoSuchElementException();
         }
-        ItemId id = (ItemId) idList.get(pos++);
-        try {
-            return itemMgr.getItem(id);
-        } catch (RepositoryException e) {
-            log.debug("failed to fetch item " + id, e);
-            throw new NoSuchElementException(e.getMessage());
-        }
+        Item item = next;
+        pos++;
+        prefetchNext();
+        return item;
     }
 
     /**

Modified: incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/NodeImpl.java?rev=216177&r1=216176&r2=216177&view=diff
==============================================================================
--- incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/NodeImpl.java (original)
+++ incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/NodeImpl.java Wed Jul 13 08:45:20 2005
@@ -925,28 +925,6 @@
     }
 
     /**
-     * Same as <code>{@link Node#getReferences()}</code> except that
-     * this method also filters out the references that appear to be non-existent
-     * in this workspace if <code>skipInexistent</code> is set to <code>true</code>.
-     *
-     * @param skipInexistent if set to <code>true</code> inexistent items are skipped
-     */
-    protected PropertyIterator getReferences(boolean skipInexistent)
-            throws RepositoryException {
-        try {
-            NodeReferencesId targetId = new NodeReferencesId(((NodeId) id).getUUID());
-            NodeReferences refs = getOrCreateNodeReferences(targetId);
-            // refs.getReferences returns a list of PropertyId's
-            List idList = refs.getReferences();
-            return new LazyItemIterator(itemMgr, idList, skipInexistent);
-        } catch (ItemStateException e) {
-            String msg = "Unable to retrieve node references for: " + id;
-            log.debug(msg);
-            throw new RepositoryException(msg, e);
-        }
-    }
-
-    /**
      * Same as {@link Node#addMixin(String)}, but takes a <code>QName</code>
      * instad of a <code>String</code>.
      *
@@ -2532,7 +2510,17 @@
         // check state of this instance
         sanityCheck();
 
-        return getReferences(false);
+        try {
+            NodeReferencesId targetId = new NodeReferencesId(((NodeId) id).getUUID());
+            NodeReferences refs = getOrCreateNodeReferences(targetId);
+            // refs.getReferences() returns a list of PropertyId's
+            List idList = refs.getReferences();
+            return new LazyItemIterator(itemMgr, idList);
+        } catch (ItemStateException e) {
+            String msg = "Unable to retrieve REFERENCE properties that refer to " + id;
+            log.debug(msg);
+            throw new RepositoryException(msg, e);
+        }
     }
 
     /**

Modified: incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionHistoryImpl.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionHistoryImpl.java?rev=216177&r1=216176&r2=216177&view=diff
==============================================================================
--- incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionHistoryImpl.java (original)
+++ incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionHistoryImpl.java Wed Jul 13 08:45:20 2005
@@ -28,7 +28,6 @@
 import org.apache.jackrabbit.name.UnknownPrefixException;
 
 import javax.jcr.Item;
-import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
 import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.nodetype.NodeDefinition;
@@ -277,17 +276,6 @@
             throw new VersionException("Specified version not contained in this history.");
         }
     }
-
-    /**
-     * {@inheritDoc}
-     * <p/>
-     * In addition to the normal behaviour, this method also filters out the
-     * references that do not exist in this workspace.
-     */
-    public PropertyIterator getReferences() throws RepositoryException {
-        return getReferences(true);
-    }
-
 
     /**
      * Returns the internal version history

Modified: incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionImpl.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionImpl.java?rev=216177&r1=216176&r2=216177&view=diff
==============================================================================
--- incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionImpl.java (original)
+++ incubator/jackrabbit/trunk/core/src/java/org/apache/jackrabbit/core/version/VersionImpl.java Wed Jul 13 08:45:20 2005
@@ -24,7 +24,6 @@
 import org.apache.jackrabbit.core.state.NodeState;
 
 import javax.jcr.Item;
-import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
 import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.nodetype.NodeDefinition;
@@ -139,15 +138,4 @@
             return false;
         }
     }
-
-    /**
-     * {@inheritDoc}
-     * <p/>
-     * In addition to the normal behaviour, this method also filters out the
-     * references that do not exist in this workspace.
-     */
-    public PropertyIterator getReferences() throws RepositoryException {
-        return getReferences(true);
-    }
-
 }