caio.co/de/cerberus

Get rid of Searcher.Builder

Whilst the pattern is super useful for things like SearchQuery, there's
zero advantages to using it here and then the type-checking loss becomes
kinda inexcusable :-)
Id
6935ad40bbce0da7484ef239ad93b5b9d1379e05
Author
Caio
Commit time
2019-05-28T22:10:44+02:00

Modified src/main/java/co/caio/cerberus/search/IndexConfiguration.java

@@ -24,6 +24,11

private final FacetsConfig facetsConfig;
private final Analyzer analyzer;
+
+ Path getBaseDirectory() {
+ return baseDirectory;
+ }
+
private final Path baseDirectory;

IndexConfiguration(Path baseDirectory, Set<String> multiValuedDimensions) {

Modified src/main/java/co/caio/cerberus/search/Indexer.java

@@ -238,8 +238,7

@Override
public Searcher buildSearcher() {
- return Searcher.Builder.fromOpened(
- indexConfiguration, indexWriter.getDirectory(), taxonomyWriter.getDirectory());
+ return Searcher.Factory.open(indexConfiguration.getBaseDirectory());
}
}
}

Modified src/main/java/co/caio/cerberus/search/Searcher.java

@@ -1,14 +1,8
package co.caio.cerberus.search;

import co.caio.cerberus.model.SearchQuery;
import co.caio.cerberus.model.SearchResult;
-import java.io.IOException;
import java.nio.file.Path;
-import org.apache.lucene.facet.taxonomy.TaxonomyReader;
-import org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyReader;
-import org.apache.lucene.index.DirectoryReader;
-import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.store.Directory;

public interface Searcher {

@@ -18,85 +12,23

int numDocs();

- class Builder {
+ class Factory {

- private IndexReader indexReader;
- private TaxonomyReader taxonomyReader;
- private IndexConfiguration indexConfiguration;
- private SearchPolicy searchPolicy;
- private Path dataDirectory;
+ public static Searcher open(Path dir) {

- public Builder dataDirectory(Path dir) {
- dataDirectory = dir;
- return this;
- }
-
- public Builder searchPolicy(SearchPolicy policy) {
- searchPolicy = policy;
- return this;
- }
-
- static Searcher fromOpened(
- IndexConfiguration configuration, Directory indexDir, Directory taxonomyDir) {
-
- var builder = new Searcher.Builder();
- builder.indexConfiguration = configuration;
try {
- builder.indexReader = DirectoryReader.open(indexDir);
- builder.taxonomyReader = new DirectoryTaxonomyReader(taxonomyDir);
- } catch (IOException wrapped) {
- throw new SearcherBuilderException(wrapped);
- }
-
- return builder.build();
- }
-
- public Searcher build() {
-
- if (dataDirectory != null && taxonomyReader == null && indexReader == null) {
- indexConfiguration = IndexConfiguration.fromBaseDirectory(dataDirectory);
- try {
- indexReader = DirectoryReader.open(indexConfiguration.openIndexDirectory());
- taxonomyReader = new DirectoryTaxonomyReader(indexConfiguration.openTaxonomyDirectory());
- } catch (IOException wrapped) {
- throw new SearcherBuilderException(wrapped);
- }
- }
-
- if (indexReader == null || taxonomyReader == null || indexConfiguration == null) {
- throw new SearcherBuilderException("This should never happen");
- }
-
- if (searchPolicy != null) {
- return new SearcherWithPolicy(this);
- } else {
- return new SearcherImpl(this);
+ return new SearcherImpl(dir);
+ } catch (Exception wrapped) {
+ throw new SearcherException(wrapped);
}
}

- IndexReader getIndexReader() {
- return indexReader;
- }
+ public static Searcher open(Path dir, SearchPolicy policy) {

- TaxonomyReader getTaxonomyReader() {
- return taxonomyReader;
- }
-
- IndexConfiguration getIndexConfiguration() {
- return indexConfiguration;
- }
-
- SearchPolicy getSearchPolicy() {
- return searchPolicy;
- }
-
- static class SearcherBuilderException extends RuntimeException {
- SearcherBuilderException(Throwable throwable) {
- super(throwable);
- }
-
- SearcherBuilderException(String message) {
- super(message);
+ try {
+ return new SearcherWithPolicy(dir, policy);
+ } catch (Exception wrapped) {
+ throw new SearcherException(wrapped);
}
}
}

Modified src/main/java/co/caio/cerberus/search/SearcherImpl.java

@@ -8,6 +8,7
import co.caio.cerberus.model.SearchResult;
import java.io.IOException;
import java.io.StringReader;
+import java.nio.file.Path;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.FloatPoint;
import org.apache.lucene.document.IntPoint;
@@ -15,6 +16,8
import org.apache.lucene.facet.FacetsCollector;
import org.apache.lucene.facet.taxonomy.FastTaxonomyFacetCounts;
import org.apache.lucene.facet.taxonomy.TaxonomyReader;
+import org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyReader;
+import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.queries.mlt.MoreLikeThis;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
@@ -39,13 +42,16
private final FulltextQueryParser queryParser;
private final MoreLikeThis moreLikeThis;

- SearcherImpl(Builder builder) {
- indexSearcher = new IndexSearcher(builder.getIndexReader());
- taxonomyReader = builder.getTaxonomyReader();
- indexConfiguration = builder.getIndexConfiguration();
- queryParser = new FulltextQueryParser(indexConfiguration.getAnalyzer());
+ SearcherImpl(Path dir) throws IOException {
+ indexConfiguration = IndexConfiguration.fromBaseDirectory(dir);

- moreLikeThis = new MoreLikeThis(builder.getIndexReader());
+ var indexReader = DirectoryReader.open(indexConfiguration.openIndexDirectory());
+
+ this.indexSearcher = new IndexSearcher(indexReader);
+ this.taxonomyReader = new DirectoryTaxonomyReader(indexConfiguration.openTaxonomyDirectory());
+
+ queryParser = new FulltextQueryParser(indexConfiguration.getAnalyzer());
+ moreLikeThis = new MoreLikeThis(indexReader);
moreLikeThis.setAnalyzer(indexConfiguration.getAnalyzer());
}

Modified src/main/java/co/caio/cerberus/search/SearcherWithPolicy.java

@@ -1,14 +1,16
package co.caio.cerberus.search;

+import java.io.IOException;
+import java.nio.file.Path;
import org.apache.lucene.search.Query;

class SearcherWithPolicy extends SearcherImpl implements Searcher {

private final SearchPolicy searchPolicy;

- SearcherWithPolicy(Builder builder) {
- super(builder);
- searchPolicy = builder.getSearchPolicy();
+ SearcherWithPolicy(Path dir, SearchPolicy policy) throws IOException {
+ super(dir);
+ searchPolicy = policy;
}

@Override

Modified src/test/java/co/caio/cerberus/search/SearcherTest.java

@@ -14,8 +14,7
import co.caio.cerberus.model.Recipe;
import co.caio.cerberus.model.SearchQuery;
import co.caio.cerberus.model.SearchQuery.SortOrder;
-import co.caio.cerberus.search.IndexConfiguration.IndexConfigurationException;
-import co.caio.cerberus.search.Searcher.Builder.SearcherBuilderException;
+import co.caio.cerberus.search.Searcher.SearcherException;
import java.nio.file.Path;
import java.util.List;
import java.util.OptionalInt;
@@ -36,13 +35,11
}

@Test
- void builder() {
- // Missing indexReader
- assertThrows(SearcherBuilderException.class, () -> new Searcher.Builder().build());
- // Bad directory
+ void throwsOnInvalidDir() {
+ assertThrows(SearcherException.class, () -> Searcher.Factory.open(Path.of("/does/not/exist")));
assertThrows(
- IndexConfigurationException.class,
- () -> new Searcher.Builder().dataDirectory(Path.of("/this/doesnt/exist")).build());
+ SearcherException.class,
+ () -> Searcher.Factory.open(Path.of("/does/not/exist"), mock(SearchPolicy.class)));
}

@Test
@@ -265,11 +262,7

given(policyMock.rewriteParsedFulltextQuery(any())).willReturn(new MatchAllDocsQuery());

- var searcherWithPolicy =
- new Searcher.Builder()
- .searchPolicy(policyMock)
- .dataDirectory(Util.getTestDataDir())
- .build();
+ var searcherWithPolicy = Searcher.Factory.open(Util.getTestDataDir(), policyMock);

searcherWithPolicy.search(new SearchQuery.Builder().fulltext("unused").build());

@@ -282,11 +275,7

given(policyMock.rewriteParsedFulltextQuery(any())).willReturn(new MatchAllDocsQuery());

- var searcherWithPolicy =
- new Searcher.Builder()
- .searchPolicy(policyMock)
- .dataDirectory(Util.getTestDataDir())
- .build();
+ var searcherWithPolicy = Searcher.Factory.open(Util.getTestDataDir(), policyMock);

// maxFacets is set to zero, policy shouldn't be called
searcherWithPolicy.search(new SearchQuery.Builder().fulltext("unused").maxFacets(0).build());
@@ -305,11 +294,7
// Policy will rewrite to MatchNoDocsQuery()
given(policyMock.rewriteParsedFulltextQuery(any())).willReturn(new MatchNoDocsQuery());

- var searcherWithPolicy =
- new Searcher.Builder()
- .searchPolicy(policyMock)
- .dataDirectory(Util.getTestDataDir())
- .build();
+ var searcherWithPolicy = Searcher.Factory.open(Util.getTestDataDir(), policyMock);

// So even when searching for all docs, we should
// get zero results
@@ -321,11 +306,7
void throwingFromPolicyIsAllowed() {
var policyMock = mock(SearchPolicy.class);

- var searcherWithPolicy =
- new Searcher.Builder()
- .searchPolicy(policyMock)
- .dataDirectory(Util.getTestDataDir())
- .build();
+ var searcherWithPolicy = Searcher.Factory.open(Util.getTestDataDir(), policyMock);

class CustomTestException extends RuntimeException {
CustomTestException() {
@@ -346,11 +327,7

given(policyMock.rewriteParsedFulltextQuery(any())).willReturn(new MatchAllDocsQuery());

- var searcherWithPolicy =
- new Searcher.Builder()
- .searchPolicy(policyMock)
- .dataDirectory(Util.getTestDataDir())
- .build();
+ var searcherWithPolicy = Searcher.Factory.open(Util.getTestDataDir(), policyMock);

var queryWithFacets = new SearchQuery.Builder().fulltext("oil").maxFacets(4).build();

@@ -411,11 +388,7

given(policyMock.rewriteParsedSimilarityQuery(any())).willReturn(new MatchNoDocsQuery());

- var searcherWithPolicy =
- new Searcher.Builder()
- .searchPolicy(policyMock)
- .dataDirectory(Util.getTestDataDir())
- .build();
+ var searcherWithPolicy = Searcher.Factory.open(Util.getTestDataDir(), policyMock);

var text = recipeText(Util.getSampleRecipes().skip(10).findFirst().get());