jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cdami...@apache.org
Subject svn commit: r1465974 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/query/lucene/QueryResultImpl.java test/java/org/apache/jackrabbit/core/query/LimitAndOffsetTest.java
Date Tue, 09 Apr 2013 11:29:24 GMT
Author: cdamioli
Date: Tue Apr  9 11:29:23 2013
New Revision: 1465974

URL: http://svn.apache.org/r1465974
Log:
JCR-3402 getSize() returning too many often -1 

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryResultImpl.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/LimitAndOffsetTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryResultImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryResultImpl.java?rev=1465974&r1=1465973&r2=1465974&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryResultImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/query/lucene/QueryResultImpl.java
Tue Apr  9 11:29:23 2013
@@ -79,9 +79,12 @@ public abstract class QueryResultImpl im
     private final List<ScoreNode[]> resultNodes = new ArrayList<ScoreNode[]>();
 
     /**
-     * This is the raw number of results that matched the query. This number
-     * also includes matches which will not be returned due to access
-     * restrictions. This value is set whenever hits are obtained.
+     * This is the raw number of results that matched the query, ignoring limit and offset.
Only set when accurate.
+     */
+    private int totalResults = -1;
+
+    /**
+     * This is the number of results that matched the query, with limit and offset. Only
set when accurate.
      */
     private int numResults = -1;
 
@@ -254,6 +257,12 @@ public abstract class QueryResultImpl im
         if (log.isDebugEnabled()) {
             log.debug("getResults({}) limit={}", size, limit);
         }
+        
+        // quick check
+        // if numResults is set, all relevant results have been fetched
+        if (numResults != -1) {
+            return;
+        }
 
         long maxResultSize = size;
 
@@ -279,9 +288,10 @@ public abstract class QueryResultImpl im
             // set selector names
             selectorNames = result.getSelectorNames();
 
+            List<ScoreNode[]> offsetNodes = new ArrayList<ScoreNode[]>();
             if (resultNodes.isEmpty() && offset > 0) {
                 // collect result offset into dummy list
-                collectScoreNodes(result, new ArrayList<ScoreNode[]>(), offset);
+                collectScoreNodes(result, offsetNodes, offset);
             } else {
                 int start = resultNodes.size() + invalid + (int) offset;
                 result.skip(start);
@@ -293,8 +303,24 @@ public abstract class QueryResultImpl im
             log.debug("retrieved ScoreNodes in {} ms ({})",
                     System.currentTimeMillis() - time, r3 - r2);
 
-            // update numResults
-            numResults = result.getSize();
+            // update numResults if all results have been fetched 
+            // if resultNodes.getSize() is strictly smaller than maxResultSize, it means
that all results have been fetched
+            int resultSize = resultNodes.size();
+            if (resultSize < maxResultSize) {
+                if (resultNodes.isEmpty()) {
+                    // if there's no result nodes, the actual totalResults if smaller or
equals than the offset
+                    totalResults = offsetNodes.size();
+                    numResults = 0;
+                }
+                else {
+                    totalResults = resultSize + (int) offset;
+                    numResults = resultSize;
+                }
+            }
+            else if (resultSize == limit) {
+                // if there's "limit" results, we can't know the total size (which may be
greater), but the result size is the limit
+                numResults = (int) limit;
+            }
         } catch (IOException e) {
             throw new RepositoryException(e);
         } finally {
@@ -367,18 +393,11 @@ public abstract class QueryResultImpl im
      * will get get if you don't set any limit or offset. This method may return
      * <code>-1</code> if the total size is unknown.
      * <p>
-     * Keep in mind that this number may get smaller if nodes are found in
-     * the result set which the current session has no permission to access.
-     * FIXME: This might be a security problem.
      *
      * @return the total number of hits.
      */
     public int getTotalSize() {
-        if (numResults == -1) {
-            return -1;
-        } else {
-            return numResults - invalid;
-        }
+        return totalResults;
     }
 
     private final class LazyScoreNodeIteratorImpl implements ScoreNodeIterator {
@@ -429,21 +448,9 @@ public abstract class QueryResultImpl im
 
         /**
          * {@inheritDoc}
-         * <p/>
-         * This value may shrink when the query result encounters non-existing
-         * nodes or the session does not have access to a node.
          */
         public long getSize() {
-            int total = getTotalSize();
-            if (total == -1) {
-                return -1;
-            }
-            long size = offset > total ? 0 : total - offset;
-            if (limit >= 0 && size > limit) {
-                return limit;
-            } else {
-                return size;
-            }
+            return numResults;
         }
 
         /**
@@ -497,8 +504,8 @@ public abstract class QueryResultImpl im
             while (next == null) {
                 if (nextPos >= resultNodes.size()) {
                     // quick check if there are more results at all
-                    // this check is only possible if we have numResults
-                    if (numResults != -1 && (nextPos + invalid) >= numResults)
{
+                    // if numResults is set, all relevant results have been fetched
+                    if (numResults != -1) {
                         break;
                     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/LimitAndOffsetTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/LimitAndOffsetTest.java?rev=1465974&r1=1465973&r2=1465974&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/LimitAndOffsetTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/query/LimitAndOffsetTest.java
Tue Apr  9 11:29:23 2013
@@ -148,9 +148,5 @@ public class LimitAndOffsetTest extends 
         result = query.execute();
         nodes = result.getNodes();
         assertEquals(1, nodes.getSize());
-        if (result instanceof JackrabbitQueryResult) {
-            assertEquals(3, ((JackrabbitQueryResult) result).getTotalSize());
-        }
     }
-
 }



Mime
View raw message