Repository: cxf Updated Branches: refs/heads/master 72a172d84 -> d0c5c3def
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/d0c5c3de Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/d0c5c3de Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/d0c5c3de Branch: refs/heads/master Commit: d0c5c3defab6dbb9619f8fd291f2e06b685cb03c Parents: 72a172d Author: reta <[email protected]> Authored: Sat Aug 30 14:47:45 2014 -0400 Committer: reta <[email protected]> Committed: Sat Aug 30 14:47:45 2014 -0400 ---------------------------------------------------------------------- .../java/demo/jaxrs/search/client/Client.java | 5 +-- .../java/demo/jaxrs/search/server/Catalog.java | 4 ++- .../ext/search/lucene/LuceneQueryVisitor.java | 38 ++++++++++---------- .../lucene/LuceneQueryVisitorFiqlTest.java | 4 ++- 4 files changed, 27 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/d0c5c3de/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/client/Client.java ---------------------------------------------------------------------- diff --git a/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/client/Client.java b/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/client/Client.java index 8eee763..e277132 100644 --- a/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/client/Client.java +++ b/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/client/Client.java @@ -45,9 +45,10 @@ public final class Client { list(url, httpClient); - search(url, httpClient, "ct==java"); - search(url, httpClient, "ct==Java"); + search(url, httpClient, "ct==java"); search(url, httpClient, "ct==websockets"); + + search(url, httpClient, "ct==Java"); search(url, httpClient, "ct==WebSockets"); delete(url, httpClient); http://git-wip-us.apache.org/repos/asf/cxf/blob/d0c5c3de/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/server/Catalog.java ---------------------------------------------------------------------- diff --git a/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/server/Catalog.java b/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/server/Catalog.java index 9de8b65..8e5c4b1 100644 --- a/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/server/Catalog.java +++ b/distribution/src/main/release/samples/jax_rs/search/src/main/java/demo/jaxrs/search/server/Catalog.java @@ -88,11 +88,13 @@ public class Catalog { private final Directory directory = new RAMDirectory(); private final Analyzer analyzer = new StandardAnalyzer(Version.LUCENE_4_9); private final Storage storage; + private final LuceneQueryVisitor<SearchBean> visitor; private final ExecutorService executor = Executors.newFixedThreadPool( Runtime.getRuntime().availableProcessors()); public Catalog(final Storage storage) throws IOException { this.storage = storage; + this.visitor = createVisitor(); initIndex(); } @@ -177,7 +179,7 @@ public class Catalog { final JsonArrayBuilder builder = Json.createArrayBuilder(); try { - final LuceneQueryVisitor<SearchBean> visitor = createVisitor(); + visitor.reset(); visitor.visit(searchContext.getCondition(SearchBean.class)); final Query query = visitor.getQuery(); http://git-wip-us.apache.org/repos/asf/cxf/blob/d0c5c3de/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 3047df2..68abcca 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 @@ -48,14 +48,18 @@ import org.apache.lucene.util.QueryBuilder; import static org.apache.cxf.jaxrs.ext.search.ParamConverterUtils.getString; import static org.apache.cxf.jaxrs.ext.search.ParamConverterUtils.getValue; +/** + * LuceneQueryVisitor implements SearchConditionVisitor and returns corresponding Lucene query. The + * implementations is thread-safe, however if visitor is called multiple times, each call to visit() + * method should be preceded by reset() method call (to properly reset the visitor's internal + * state). + */ public class LuceneQueryVisitor<T> extends AbstractSearchConditionVisitor<T, Query> { private String contentsFieldName; private Map<String, String> contentsFieldMap; private boolean caseInsensitiveMatch; 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() { @@ -98,18 +102,24 @@ public class LuceneQueryVisitor<T> extends AbstractSearchConditionVisitor<T, Que if (analyzer != null) { queryBuilder = new QueryBuilder(analyzer); } + + reset(); } public void setContentsFieldMap(Map<String, String> map) { this.contentsFieldMap = map; } + /** + * Resets visitor's internal state. If the instance of the visitor is intended to be used many times, + * each call to visit() method should be preceded by reset() method call. + */ + public void reset() { + state.set(new Stack<List<Query>>()); + state.get().push(new ArrayList<Query>()); + } + 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) { @@ -120,19 +130,7 @@ public class LuceneQueryVisitor<T> extends AbstractSearchConditionVisitor<T, Que } else { state.get().push(new ArrayList<Query>()); for (SearchCondition<T> condition : sc.getSearchConditions()) { - 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(); - } + condition.accept(this); } boolean orCondition = sc.getConditionType() == ConditionType.OR; List<Query> queries = state.get().pop(); http://git-wip-us.apache.org/repos/asf/cxf/blob/d0c5c3de/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 1b39dc6..8938489 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 @@ -209,6 +209,7 @@ public class LuceneQueryVisitorFiqlTest extends AbstractLuceneQueryVisitorTest { visitor.visit(filter1); assertThat(visitor.getQuery().toString(), equalTo("name:text")); + visitor.reset(); visitor.visit(filter2); assertThat(visitor.getQuery().toString(), equalTo("name:word")); } @@ -226,7 +227,8 @@ public class LuceneQueryVisitorFiqlTest extends AbstractLuceneQueryVisitorTest { executorService.submit(new Runnable() { @Override public void run() { - final SearchCondition<SearchBean> filter = getParser().parse("name==text" + index); + final SearchCondition<SearchBean> filter = getParser().parse("name==text" + index); + visitor.reset(); visitor.visit(filter); assertNotNull("Query should not be null", visitor.getQuery());
