cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r...@apache.org
Subject [1/2] git commit: CXF-5938: LuceneQueryVisitor is not reusable / not thread-safe
Date Fri, 29 Aug 2014 14:14:23 GMT
Repository: cxf
Updated Branches:
  refs/heads/master fa8a61427 -> 6bec79650


CXF-5938: LuceneQueryVisitor is not reusable / not thread-safe


Project: http://git-wip-us.apache.org/repos/asf/cxf/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/eee2a947
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/eee2a947
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/eee2a947

Branch: refs/heads/master
Commit: eee2a947d867c403c212677cf942b3d07fa06fd7
Parents: c56913c
Author: reta <drreta@gmail.com>
Authored: Fri Aug 29 10:12:02 2014 -0400
Committer: reta <drreta@gmail.com>
Committed: Fri Aug 29 10:12:02 2014 -0400

----------------------------------------------------------------------
 .../ext/search/lucene/LuceneQueryVisitor.java   | 39 +++++++++----
 .../lucene/AbstractLuceneQueryVisitorTest.java  |  1 -
 .../lucene/LuceneQueryVisitorFiqlTest.java      | 58 ++++++++++++++++++++
 3 files changed, 87 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf/blob/eee2a947/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitor.java
----------------------------------------------------------------------
diff --git a/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitor.java
b/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitor.java
index ec4a9a5..3047df2 100644
--- a/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitor.java
+++ b/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitor.java
@@ -29,6 +29,8 @@ import org.apache.cxf.jaxrs.ext.search.ConditionType;
 import org.apache.cxf.jaxrs.ext.search.PrimitiveStatement;
 import org.apache.cxf.jaxrs.ext.search.SearchCondition;
 import org.apache.cxf.jaxrs.ext.search.visitor.AbstractSearchConditionVisitor;
+import org.apache.cxf.jaxrs.ext.search.visitor.ThreadLocalVisitorState;
+import org.apache.cxf.jaxrs.ext.search.visitor.VisitorState;
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.document.DateTools;
 import org.apache.lucene.document.DateTools.Resolution;
@@ -51,7 +53,9 @@ public class LuceneQueryVisitor<T> extends AbstractSearchConditionVisitor<T,
Que
     private String contentsFieldName;
     private Map<String, String> contentsFieldMap;
     private boolean caseInsensitiveMatch;
-    private Stack<List<Query>> queryStack = new Stack<List<Query>>();
+    private VisitorState< Stack< List< Query > > > state = new ThreadLocalVisitorState<
Stack< List< Query > > >();
+    private VisitorState< Stack< SearchCondition< ? > > > conditions =

+        new ThreadLocalVisitorState< Stack< SearchCondition< ? > > >();
     private QueryBuilder queryBuilder;
     
     public LuceneQueryVisitor() {
@@ -93,9 +97,7 @@ public class LuceneQueryVisitor<T> extends AbstractSearchConditionVisitor<T,
Que
         
         if (analyzer != null) {
             queryBuilder = new QueryBuilder(analyzer);
-        }
-        
-        queryStack.push(new ArrayList<Query>());
+        }                
     }
     
     public void setContentsFieldMap(Map<String, String> map) {
@@ -103,26 +105,43 @@ public class LuceneQueryVisitor<T> extends AbstractSearchConditionVisitor<T,
Que
     }
     
     public void visit(SearchCondition<T> sc) {
+        if (conditions.get() == null || conditions.get().isEmpty()) {
+            state.set(new Stack<List<Query>>());
+            state.get().push(new ArrayList<Query>());
+        }
+        
         PrimitiveStatement statement = sc.getStatement();
         if (statement != null) {
             if (statement.getProperty() != null) {
-                queryStack.peek().add(buildSimpleQuery(sc.getConditionType(), 
+                state.get().peek().add(buildSimpleQuery(sc.getConditionType(), 
                                          statement.getProperty(), 
                                          statement.getValue()));
             }
         } else {
-            queryStack.push(new ArrayList<Query>());
+            state.get().push(new ArrayList<Query>());
             for (SearchCondition<T> condition : sc.getSearchConditions()) {
-                condition.accept(this);
+                try {
+                    // There could me multiple recursive calls to the visit() method.
+                    // The conditions stack keeps track of every call down the call chain
+                    // in order to understand when visitor's state should be reset.
+                    if (conditions.get() == null) {
+                        conditions.set(new Stack<SearchCondition<?>>());
+                    }
+                    
+                    conditions.get().push(condition);
+                    condition.accept(this);
+                } finally {
+                    conditions.get().pop();
+                }
             }
             boolean orCondition = sc.getConditionType() == ConditionType.OR;
-            List<Query> queries = queryStack.pop();
-            queryStack.peek().add(createCompositeQuery(queries, orCondition));
+            List<Query> queries = state.get().pop();
+            state.get().peek().add(createCompositeQuery(queries, orCondition));
         }    
     }
     
     public Query getQuery() {
-        List<Query> queries = queryStack.peek();
+        List<Query> queries = state.get().peek();
         return queries.isEmpty() ? null : queries.get(0);
     }
     

http://git-wip-us.apache.org/repos/asf/cxf/blob/eee2a947/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/AbstractLuceneQueryVisitorTest.java
----------------------------------------------------------------------
diff --git a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/AbstractLuceneQueryVisitorTest.java
b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/AbstractLuceneQueryVisitorTest.java
index bc745d3..70a4952 100644
--- a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/AbstractLuceneQueryVisitorTest.java
+++ b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/AbstractLuceneQueryVisitorTest.java
@@ -76,7 +76,6 @@ public abstract class AbstractLuceneQueryVisitorTest extends Assert {
         directory.close();
     }
     
-
     protected abstract SearchConditionParser<SearchBean> getParser();
  
     protected void doTestTextContentMatch(String expression) throws Exception {

http://git-wip-us.apache.org/repos/asf/cxf/blob/eee2a947/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitorFiqlTest.java
----------------------------------------------------------------------
diff --git a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitorFiqlTest.java
b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitorFiqlTest.java
index 8cf4581..1b39dc6 100644
--- a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitorFiqlTest.java
+++ b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/lucene/LuceneQueryVisitorFiqlTest.java
@@ -18,12 +18,23 @@
  */
 package org.apache.cxf.jaxrs.ext.search.lucene;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.cxf.jaxrs.ext.search.SearchBean;
+import org.apache.cxf.jaxrs.ext.search.SearchCondition;
 import org.apache.cxf.jaxrs.ext.search.SearchConditionParser;
 import org.apache.cxf.jaxrs.ext.search.fiql.FiqlParser;
 import org.apache.lucene.search.Query;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+
 public class LuceneQueryVisitorFiqlTest extends AbstractLuceneQueryVisitorTest {
     @Test
     public void testTextContentMatchEqual() throws Exception {
@@ -189,6 +200,53 @@ public class LuceneQueryVisitorFiqlTest extends AbstractLuceneQueryVisitorTest
{
         doTestTextContentMatchWithQuery(query);
     }
     
+    @Test
+    public void testThatMultipleQueriesForTheSameFieldAreHandledProperly() {
+        final SearchCondition<SearchBean> filter1 = getParser().parse("name==text");
+        final SearchCondition<SearchBean> filter2 = getParser().parse("name==word");
      
+        final LuceneQueryVisitor<SearchBean> visitor = new LuceneQueryVisitor<SearchBean>();
+        
+        visitor.visit(filter1);        
+        assertThat(visitor.getQuery().toString(), equalTo("name:text"));
+        
+        visitor.visit(filter2);        
+        assertThat(visitor.getQuery().toString(), equalTo("name:word"));
+    }
+    
+    @Test
+    public void testThatMultipleQueriesForTheSameFieldAreThreadSafe() throws InterruptedException,
ExecutionException {
+        final LuceneQueryVisitor<SearchBean> visitor = new LuceneQueryVisitor<SearchBean>();
+        final ExecutorService executorService = Executors.newFixedThreadPool(5);
+        
+        final Collection< Future< ? > > futures = new ArrayList< Future<
? > >();
+        for (int i = 0; i < 5; ++i) {
+            final int index = i;
+            
+            futures.add(
+                executorService.submit(new Runnable() {                
+                    @Override
+                    public void run() {
+                        final SearchCondition<SearchBean> filter = getParser().parse("name==text"
+ index);            
+                        visitor.visit(filter);        
+                        
+                        assertNotNull("Query should not be null", visitor.getQuery());
+                        assertThat(visitor.getQuery().toString(), equalTo("name:text" + index));
+                    }                
+                }) 
+            );
+        }
+        
+        executorService.shutdown();
+        assertTrue("All threads should be terminated", 
+            executorService.awaitTermination(5, TimeUnit.SECONDS));
+        
+        for (final Future< ? > future: futures) {
+            // The exception will be raised if queries are messed up
+            future.get(); 
+        }
+    }
+
+    
     @Override
     protected SearchConditionParser<SearchBean> getParser() {
         return new FiqlParser<SearchBean>(SearchBean.class);


Mime
View raw message