Fixed broken Lucene blob delete

diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd
index bed23288f7ab6601567e6b478c06f96b816b3751..43a2db0d901e33e9d886ea42fa1aceb75f8e9f77 100644 (file)
--- a/docs/04_releases.mkd
+++ b/docs/04_releases.mkd
@@ -6,8 +6,9 @@
 \r
 #### fixes\r
 \r
+- Fixed bug in Lucene search where old/stale blobs were never properly deleted prior to reindexing.  This resulted in duplicate blob entries in the index.\r
 - Fixed intermittent bug in identifying line numbers in Lucene search (issue 105)\r
-- Adjust repository search to handle foo.git and foo/bar.git (issue 104)\r
+- Adjust repository identification to handle foo.git and foo/bar.git (issue 104)\r
 - Fixed bug where a repository set as authenticated push did not have anonymous clone access (issue 96)\r
 - Fixed bug in Basic authentication if passwords had a colon (Github/peterloron)\r
 - Fixed bug where the Gitblit Manager could not update a setting that was not referenced in reference.properties (issue 85)\r

diff --git a/groovy/.gitignore b/groovy/.gitignore
new file mode 100644 (file)
index 0000000..e58dc47
--- /dev/null
+++ b/groovy/.gitignore
@@ -0,0 +1 @@
+/grape

diff --git a/src/com/gitblit/LuceneExecutor.java b/src/com/gitblit/LuceneExecutor.java
index c702dcc7156f5f199a7cc3db09a7556bc0e3426f..f7a739056066e47df1b627bcf07e29902d277af8 100644 (file)
--- a/src/com/gitblit/LuceneExecutor.java
+++ b/src/com/gitblit/LuceneExecutor.java
@@ -105,7 +105,7 @@ import com.gitblit.utils.StringUtils;
 public class LuceneExecutor implements Runnable {\r
        \r
                \r
-       private static final int INDEX_VERSION = 3;\r
+       private static final int INDEX_VERSION = 4;\r
 \r
        private static final String FIELD_OBJECT_TYPE = "type";\r
        private static final String FIELD_ISSUE = "issue";\r
@@ -301,7 +301,6 @@ public class LuceneExecutor implements Runnable {
                        throw new RuntimeException(e);\r
                }\r
        }\r
-\r
        \r
        /**\r
         * Returns the author for the commit, if this information is available.\r
@@ -728,8 +727,9 @@ public class LuceneExecutor implements Runnable {
         * @param repositoryName\r
         * @param issueId\r
         * @throws Exception\r
+        * @return true, if deleted, false if no record was deleted\r
         */\r
-       private void deleteIssue(String repositoryName, String issueId) throws Exception {\r
+       private boolean deleteIssue(String repositoryName, String issueId) throws Exception {\r
                BooleanQuery query = new BooleanQuery();\r
                Term objectTerm = new Term(FIELD_OBJECT_TYPE, SearchObjectType.issue.name());\r
                query.add(new TermQuery(objectTerm), Occur.MUST);\r
@@ -737,8 +737,17 @@ public class LuceneExecutor implements Runnable {
                query.add(new TermQuery(issueidTerm), Occur.MUST);\r
                \r
                IndexWriter writer = getIndexWriter(repositoryName);\r
+               int numDocsBefore = writer.numDocs();\r
                writer.deleteDocuments(query);\r
                writer.commit();\r
+               int numDocsAfter = writer.numDocs();\r
+               if (numDocsBefore == numDocsAfter) {\r
+                       logger.debug(MessageFormat.format("no records found to delete {0}", query.toString()));\r
+                       return false;\r
+               } else {\r
+                       logger.debug(MessageFormat.format("deleted {0} records with {1}", numDocsBefore - numDocsAfter, query.toString()));\r
+                       return true;\r
+               }\r
        }\r
        \r
        /**\r
@@ -748,19 +757,29 @@ public class LuceneExecutor implements Runnable {
         * @param branch\r
         * @param path\r
         * @throws Exception\r
+        * @return true, if deleted, false if no record was deleted\r
         */\r
-       private void deleteBlob(String repositoryName, String branch, String path) throws Exception {\r
-               BooleanQuery query = new BooleanQuery();\r
-               Term objectTerm = new Term(FIELD_OBJECT_TYPE, SearchObjectType.blob.name());\r
-               query.add(new TermQuery(objectTerm), Occur.MUST);\r
-               Term branchTerm = new Term(FIELD_BRANCH, branch);\r
-               query.add(new TermQuery(branchTerm), Occur.MUST);\r
-               Term pathTerm = new Term(FIELD_PATH, path);\r
-               query.add(new TermQuery(pathTerm), Occur.MUST);\r
+       public boolean deleteBlob(String repositoryName, String branch, String path) throws Exception {\r
+               String pattern = MessageFormat.format("{0}:'{'0} AND {1}:\"'{'1'}'\" AND {2}:\"'{'2'}'\"", FIELD_OBJECT_TYPE, FIELD_BRANCH, FIELD_PATH);\r
+               String q = MessageFormat.format(pattern, SearchObjectType.blob.name(), branch, path);\r
                \r
+               BooleanQuery query = new BooleanQuery();\r
+               StandardAnalyzer analyzer = new StandardAnalyzer(LUCENE_VERSION);\r
+               QueryParser qp = new QueryParser(LUCENE_VERSION, FIELD_SUMMARY, analyzer);\r
+               query.add(qp.parse(q), Occur.MUST);\r
+\r
                IndexWriter writer = getIndexWriter(repositoryName);\r
-               writer.deleteDocuments(query);\r
+               int numDocsBefore = writer.numDocs();\r
+               writer.deleteDocuments(query);          \r
                writer.commit();\r
+               int numDocsAfter = writer.numDocs();\r
+               if (numDocsBefore == numDocsAfter) {\r
+                       logger.debug(MessageFormat.format("no records found to delete {0}", query.toString()));\r
+                       return false;\r
+               } else {\r
+                       logger.debug(MessageFormat.format("deleted {0} records with {1}", numDocsBefore - numDocsAfter, query.toString()));\r
+                       return true;\r
+               }\r
        }\r
 \r
        /**\r
@@ -881,7 +900,9 @@ public class LuceneExecutor implements Runnable {
                                                IssueModel issue = IssueUtils.getIssue(repository, issueId);\r
                                                if (issue == null) {\r
                                                        // issue was deleted, remove from index\r
-                                                       deleteIssue(model.name, issueId);\r
+                                                       if (!deleteIssue(model.name, issueId)) {\r
+                                                               logger.error(MessageFormat.format("Failed to delete issue {0} from Lucene index!", issueId));\r
+                                                       }\r
                                                } else {\r
                                                        // issue was updated\r
                                                        index(model.name, issue);\r
@@ -1119,7 +1140,7 @@ public class LuceneExecutor implements Runnable {
                        qp = new QueryParser(LUCENE_VERSION, FIELD_CONTENT, analyzer);\r
                        qp.setAllowLeadingWildcard(true);\r
                        query.add(qp.parse(text), Occur.SHOULD);\r
-\r
+                       \r
                        IndexSearcher searcher;\r
                        if (repositories.length == 1) {\r
                                // single repository search\r
@@ -1135,7 +1156,10 @@ public class LuceneExecutor implements Runnable {
                                MultiSourceReader reader = new MultiSourceReader(rdrs);\r
                                searcher = new IndexSearcher(reader);\r
                        }\r
+                       \r
                        Query rewrittenQuery = searcher.rewrite(query);\r
+                       logger.debug(rewrittenQuery.toString());\r
+\r
                        TopScoreDocCollector collector = TopScoreDocCollector.create(5000, true);\r
                        searcher.search(rewrittenQuery, collector);\r
                        int offset = Math.max(0, (page - 1) * pageSize);\r

diff --git a/tests/com/gitblit/tests/IssuesTest.java b/tests/com/gitblit/tests/IssuesTest.java
index 11f955146575c8917ea0a78bc9ce6a6e858c670f..54cac335f1e4ca7901faed9e33e3c4766c83c0be 100644 (file)
--- a/tests/com/gitblit/tests/IssuesTest.java
+++ b/tests/com/gitblit/tests/IssuesTest.java
@@ -134,7 +134,7 @@ public class IssuesTest {
                        lucene.index(name, anIssue);\r
                }\r
                List<SearchResult> hits = lucene.search("working", 1, 10, name);\r
-               assertTrue(hits.size() > 0);\r
+               assertTrue(hits.size() == 1);\r
                \r
                // reindex an issue\r
                issue = allIssues.get(0);\r
@@ -144,6 +144,10 @@ public class IssuesTest {
                issue = IssueUtils.getIssue(repository, issue.id);\r
                lucene.index(name, issue);\r
 \r
+               hits = lucene.search("working", 1, 10, name);\r
+               assertTrue(hits.size() == 1);\r
+\r
+               \r
                // delete all issues\r
                for (IssueModel anIssue : allIssues) {\r
                        assertTrue(IssueUtils.deleteIssue(repository, anIssue.id, "D"));\r

diff --git a/tests/com/gitblit/tests/LuceneExecutorTest.java b/tests/com/gitblit/tests/LuceneExecutorTest.java
index ec81fd8ec366b38a1a1af213a31ec8e64feaf08a..21454fe4bdb4abd448f7c0fb45f3d32555dd8c55 100644 (file)
--- a/tests/com/gitblit/tests/LuceneExecutorTest.java
+++ b/tests/com/gitblit/tests/LuceneExecutorTest.java
@@ -16,6 +16,8 @@
 package com.gitblit.tests;\r
 \r
 import static org.junit.Assert.assertEquals;\r
+import static org.junit.Assert.assertFalse;\r
+import static org.junit.Assert.assertTrue;\r
 \r
 import java.util.ArrayList;\r
 import java.util.List;\r
@@ -23,14 +25,12 @@ import java.util.List;
 import org.eclipse.jgit.lib.Repository;\r
 import org.junit.Test;\r
 \r
-import com.gitblit.GitBlit;\r
 import com.gitblit.LuceneExecutor;\r
 import com.gitblit.models.RefModel;\r
 import com.gitblit.models.RepositoryModel;\r
 import com.gitblit.models.SearchResult;\r
 import com.gitblit.utils.FileUtils;\r
 import com.gitblit.utils.JGitUtils;\r
-import com.gitblit.utils.StringUtils;\r
 \r
 /**\r
  * Tests Lucene indexing and querying.\r
@@ -160,4 +160,17 @@ public class LuceneExecutorTest {
                lucene.close();\r
                assertEquals(10, results.size());\r
        }\r
+       \r
+       @Test\r
+       public void testDeleteBlobFromIndex() throws Exception {\r
+               // start with a fresh reindex of entire repository\r
+               LuceneExecutor lucene = new LuceneExecutor(null, GitBlitSuite.REPOSITORIES);\r
+               Repository repository = GitBlitSuite.getHelloworldRepository();\r
+               RepositoryModel model = newRepositoryModel(repository);\r
+               lucene.reindex(model, repository);\r
+               \r
+               // now delete a blob\r
+               assertTrue(lucene.deleteBlob(model.name, "refs/heads/master", "java.java"));\r
+               assertFalse(lucene.deleteBlob(model.name, "refs/heads/master", "java.java"));\r
+       }\r
 }
\ No newline at end of file