[dna-commits] DNA SVN: r1557 - in trunk: extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query and 1 other directory.

[dna-commits at lists.jboss.org]

Author: rhauch
Date: 2010-01-08 00:45:54 -0500 (Fri, 08 Jan 2010)
New Revision: 1557

Modified:
   trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
   trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/CompareQuery.java
   trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/HasValueQuery.java
   trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/IdsQuery.java
Log:
DNA-630 DNA-613 This was found while trying to fix DNA-613.

I logged this because this was so tricky. LuceneSearchEngine works by building up a (complex) Lucene Query object and executing that, then collecting the tuples from the Lucene documents that are returned in the results.

In this case, the AQM query included a constraint (among others) that LuceneSearchEngine converted into a custom HasValueQuery class (which extends Query). HasValueQuery creates a custom ExistsScorer class (which subclasses org.apache.lucene.search.Scorer), and the problem was that the 'nextDoc()' method was implemented incorrectly. It uses an internal counter to walk through the documents (handed to it in the constructor), and was not (correctly) checking whether the docId was less than the maxDoc, which is obtained from the IndexReader.maxDoc().

Essentially, the ExistsScorer was iterating through more documents than in the reader, and thus the 'read past EOF'. (The IndexReader uses a random-access mechanism to seek to the desired document, but if the document ID is too big, the seek goes beyond the EOF.)

I checked the other Query subclasses in org.jboss.dna.search.lucene.query, and found a few other classes that were also implemented incorrectly. These were fixed as well, and all tests pass.

Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java	2010-01-08 05:45:17 UTC (rev 1556)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java	2010-01-08 05:45:54 UTC (rev 1557)
@@ -177,7 +177,7 @@
             // These might not all be level one tests
             addTestSuite(org.apache.jackrabbit.test.api.query.XPathPosIndexTest.class);
             // addTestSuite(org.apache.jackrabbit.test.api.query.XPathDocOrderTest.class);
-            // addTestSuite(org.apache.jackrabbit.test.api.query.XPathOrderByTest.class);
+            addTestSuite(org.apache.jackrabbit.test.api.query.XPathOrderByTest.class);
             addTestSuite(org.apache.jackrabbit.test.api.query.XPathJcrPathTest.class);
             addTestSuite(org.apache.jackrabbit.test.api.query.DerefQueryLevel1Test.class);
             addTestSuite(org.apache.jackrabbit.test.api.query.GetLanguageTest.class);

Modified: trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/CompareQuery.java
===================================================================
--- trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/CompareQuery.java	2010-01-08 05:45:17 UTC (rev 1556)
+++ trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/CompareQuery.java	2010-01-08 05:45:54 UTC (rev 1557)
@@ -267,14 +267,14 @@
         public int nextDoc() throws IOException {
             do {
                 ++docId;
+                if (docId >= maxDocId) return Scorer.NO_MORE_DOCS;
                 if (reader.isDeleted(docId)) {
                     // We should skip this document ...
                     continue;
                 }
                 ValueType value = readFromDocument(reader, docId);
                 if (evaluator.satisfiesConstraint(value, constraintValue)) return docId;
-            } while (docId < maxDocId);
-            return Scorer.NO_MORE_DOCS;
+            } while (true);
         }
 
         /**

Modified: trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/HasValueQuery.java
===================================================================
--- trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/HasValueQuery.java	2010-01-08 05:45:17 UTC (rev 1556)
+++ trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/HasValueQuery.java	2010-01-08 05:45:54 UTC (rev 1557)
@@ -219,13 +219,13 @@
         public int nextDoc() throws IOException {
             do {
                 ++docId;
+                if (docId >= maxDocId) return Scorer.NO_MORE_DOCS;
                 if (reader.isDeleted(docId)) {
                     // We should skip this document ...
                     continue;
                 }
                 if (hasValue(reader, docId)) return docId;
-            } while (docId < maxDocId);
-            return Scorer.NO_MORE_DOCS;
+            } while (true);
         }
 
         /**

Modified: trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/IdsQuery.java
===================================================================
--- trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/IdsQuery.java	2010-01-08 05:45:17 UTC (rev 1556)
+++ trunk/extensions/dna-search-lucene/src/main/java/org/jboss/dna/search/lucene/query/IdsQuery.java	2010-01-08 05:45:54 UTC (rev 1557)
@@ -221,13 +221,13 @@
         public int nextDoc() throws IOException {
             do {
                 ++docId;
+                if (docId >= maxDocId) return Scorer.NO_MORE_DOCS;
                 if (reader.isDeleted(docId)) {
                     // We should skip this document ...
                     continue;
                 }
                 if (includeDocument(reader, docId)) return docId;
-            } while (docId < maxDocId);
-            return Scorer.NO_MORE_DOCS;
+            } while (true);
         }
 
         /**