From 41d4869c01527f8e0a9be903e7bceec5478fb715 Mon Sep 17 00:00:00 2001 From: qaate47 Date: Mon, 28 Oct 2024 15:47:40 +0100 Subject: [PATCH 1/4] GH-5149 Add search:numDocs property and maxQueryDocuments param in LuceneSail query --- .../elasticsearch/ElasticsearchIndex.java | 43 +++++++++- .../sail/lucene/AbstractSearchIndex.java | 3 + .../eclipse/rdf4j/sail/lucene/LuceneSail.java | 7 ++ .../rdf4j/sail/lucene/LuceneSailSchema.java | 3 + .../eclipse/rdf4j/sail/lucene/QuerySpec.java | 35 ++++++++ .../rdf4j/sail/lucene/QuerySpecBuilder.java | 15 +++- .../sail/lucene/QuerySpecBuilderTest.java | 8 ++ .../rdf4j/sail/lucene/impl/LuceneIndex.java | 49 ++++++++++- .../eclipse/rdf4j/sail/solr/SolrIndex.java | 51 ++++++++++- .../sail/lucene/AbstractLuceneSailTest.java | 86 +++++++++++++++++-- 10 files changed, 281 insertions(+), 19 deletions(-) diff --git a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java index 910c6eb385b..29a2d1e74c5 100644 --- a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java +++ b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java @@ -577,10 +577,19 @@ protected Iterable query(Resource subject, QuerySpec sp } SearchHits hits; + Integer numDocs = spec.getNumDocs(); if (subject != null) { - hits = search(subject, request, qb); + if (numDocs != null) { + hits = search(subject, request, qb, numDocs); + } else { + hits = search(subject, request, qb); + } } else { - hits = search(request, qb); + if (numDocs != null) { + hits = search(request, qb, numDocs); + } else { + hits = search(request, qb); + } } return Iterables.transform(hits, new Function<>() { @@ -600,11 +609,24 @@ public DocumentScore apply(SearchHit hit) { * @return search hits */ public SearchHits search(Resource resource, SearchRequestBuilder request, QueryBuilder query) { + return search(resource, request, query, -1); + } + + /** + * Evaluates the given query only for the given resource. + * + * @param resource + * @param request + * @param query + * @param numDocs + * @return search hits + */ + public SearchHits search(Resource resource, SearchRequestBuilder request, QueryBuilder query, int numDocs) { // rewrite the query QueryBuilder idQuery = QueryBuilders.termQuery(SearchFields.URI_FIELD_NAME, SearchFields.getResourceID(resource)); QueryBuilder combinedQuery = QueryBuilders.boolQuery().must(idQuery).must(query); - return search(request, combinedQuery); + return search(request, combinedQuery, numDocs); } @Override @@ -712,9 +734,22 @@ private ShapeRelation toSpatialOp(String relation) { * Evaluates the given query and returns the results as a TopDocs instance. */ public SearchHits search(SearchRequestBuilder request, QueryBuilder query) { + return search(request, query, -1); + } + + /** + * Evaluates the given query and returns the results as a TopDocs instance. + */ + public SearchHits search(SearchRequestBuilder request, QueryBuilder query, int numDocs) { String[] types = getTypes(); int nDocs; - if (maxDocs > 0) { + if (numDocs > 0) { + if (maxQueryDocs > 0 && maxQueryDocs < numDocs) { + nDocs = maxQueryDocs; + } else { + nDocs = numDocs; + } + } else if (maxDocs > 0) { nDocs = maxDocs; } else { long docCount = client.prepareSearch(indexName) diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java index d8f85a4ff58..9423e9dd95f 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java @@ -66,6 +66,7 @@ public abstract class AbstractSearchIndex implements SearchIndex { } protected int maxDocs; + protected int maxQueryDocs; protected Set wktFields = Collections.singleton(SearchFields.getPropertyField(GEO.AS_WKT)); @@ -77,6 +78,8 @@ public abstract class AbstractSearchIndex implements SearchIndex { public void initialize(Properties parameters) throws Exception { String maxDocParam = parameters.getProperty(LuceneSail.MAX_DOCUMENTS_KEY); maxDocs = (maxDocParam != null) ? Integer.parseInt(maxDocParam) : -1; + String maxQueryDocParam = parameters.getProperty(LuceneSail.MAX_QUERY_DOCUMENTS_KEY); + maxQueryDocs = (maxQueryDocParam != null) ? Integer.parseInt(maxQueryDocParam) : maxDocs; String wktFieldParam = parameters.getProperty(LuceneSail.WKT_FIELDS); if (wktFieldParam != null) { diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java index 37e6e8c0db8..c3b0d90a665 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java @@ -296,6 +296,13 @@ public class LuceneSail extends NotifyingSailWrapper { */ public static final String MAX_DOCUMENTS_KEY = "maxDocuments"; + /** + * Set the key "maxQueryDocuments=<n>" as sail parameter to limit the maximum number of documents the user can + * query at a time to return from a search query. The default is the value of the {@link #MAX_DOCUMENTS_KEY} + * parameter. + */ + public static final String MAX_QUERY_DOCUMENTS_KEY = "maxQueryDocuments"; + /** * Set this key to configure which fields contain WKT and should be spatially indexed. The value should be a * space-separated list of URIs. Default is http://www.opengis.net/ont/geosparql#asWKT. diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSailSchema.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSailSchema.java index 13e5d891818..78d56b24734 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSailSchema.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSailSchema.java @@ -52,6 +52,8 @@ public class LuceneSailSchema { public static final IRI CONTEXT; + public static final IRI NUM_DOCS; + static { ValueFactory factory = SimpleValueFactory.getInstance(); // compatible with beta4: // creating a new factory @@ -73,5 +75,6 @@ public class LuceneSailSchema { WITHIN_DISTANCE = factory.createIRI(NAMESPACE + "withinDistance"); DISTANCE = factory.createIRI(NAMESPACE + "distance"); CONTEXT = factory.createIRI(NAMESPACE + "context"); + NUM_DOCS = factory.createIRI(NAMESPACE + "numDocs"); } } diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpec.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpec.java index c6886ccd084..0e647c592f7 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpec.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpec.java @@ -16,7 +16,9 @@ import java.util.stream.Collectors; import org.eclipse.rdf4j.model.IRI; +import org.eclipse.rdf4j.model.Literal; import org.eclipse.rdf4j.model.Resource; +import org.eclipse.rdf4j.model.Value; import org.eclipse.rdf4j.query.algebra.QueryModelNode; import org.eclipse.rdf4j.query.algebra.SingletonSet; import org.eclipse.rdf4j.query.algebra.StatementPattern; @@ -67,21 +69,43 @@ private static void append(Var var, StringBuilder buffer) { private final StatementPattern idPattern; + private final StatementPattern numDocsPattern; + private final Resource subject; private final String matchesVarName; private final String scoreVarName; + private final Integer numDocs; + public QuerySpec(StatementPattern matchesPattern, Collection queryPatterns, StatementPattern scorePattern, StatementPattern typePattern, StatementPattern idPattern, Resource subject) { + this(matchesPattern, queryPatterns, scorePattern, typePattern, idPattern, null, subject); + } + + public QuerySpec(StatementPattern matchesPattern, Collection queryPatterns, + StatementPattern scorePattern, StatementPattern typePattern, + StatementPattern idPattern, StatementPattern numDocsPattern, Resource subject) { this.matchesPattern = matchesPattern; this.queryPatterns = queryPatterns; this.scorePattern = scorePattern; this.typePattern = typePattern; this.idPattern = idPattern; + this.numDocsPattern = numDocsPattern; this.subject = subject; + if (numDocsPattern != null) { + Value val = numDocsPattern.getObjectVar().getValue(); + if (val != null && val.isLiteral()) { + this.numDocs = ((Literal) val).intValue(); + } else { + throw new IllegalArgumentException("numDocs should be constant literal value"); + } + } else { + this.numDocs = null; + } + if (matchesPattern != null) { this.matchesVarName = matchesPattern.getSubjectVar().getName(); } else { @@ -101,9 +125,11 @@ public QuerySpec(String matchesVarName, String propertyVarName, String scoreVarN this.matchesPattern = null; this.scorePattern = null; this.typePattern = null; + this.numDocsPattern = null; this.queryPatterns = Set.of(); this.idPattern = null; this.subject = subject; + this.numDocs = null; } @Override @@ -121,6 +147,7 @@ public QueryModelNode removeQueryPatterns() { replace(getScorePattern(), replacement); replace(getTypePattern(), replacement); replace(getIdPattern(), replacement); + replace(getNumDocsPattern(), replacement); final QueryModelNode placeholder = new SingletonSet(); @@ -154,6 +181,10 @@ public StatementPattern getScorePattern() { return scorePattern; } + public StatementPattern getNumDocsPattern() { + return numDocsPattern; + } + /** * The variable name associated with the query score * @@ -163,6 +194,10 @@ public String getScoreVariableName() { return scoreVarName; } + public Integer getNumDocs() { + return numDocs; + } + public StatementPattern getTypePattern() { return typePattern; } diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilder.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilder.java index c62a30266ab..73c88dc10f6 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilder.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilder.java @@ -15,6 +15,7 @@ import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.INDEXID; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.LUCENE_QUERY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.MATCHES; +import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.NUM_DOCS; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.PROPERTY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.QUERY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.SCORE; @@ -152,7 +153,7 @@ public void process(TupleExpr tupleExpr, BindingSet bindings, Collection queryPatterns; try { @@ -161,6 +162,7 @@ public void process(TupleExpr tupleExpr, BindingSet bindings, Collection boostPatterns = new ArrayList<>(); + public ArrayList numDocsPatterns = new ArrayList<>(); + /** * Method implementing the visitor pattern that gathers all statements using a predicate from the LuceneSail's * namespace. @@ -487,6 +496,8 @@ public void meet(StatementPattern node) { idPatterns.add(node); } else if (BOOST.equals(predicate)) { boostPatterns.add(node); + } else if (NUM_DOCS.equals(predicate)) { + numDocsPatterns.add(node); } else if (TYPE.equals(predicate)) { Value object = node.getObjectVar().getValue(); if (LUCENE_QUERY.equals(object)) { diff --git a/core/sail/lucene-api/src/test/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilderTest.java b/core/sail/lucene-api/src/test/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilderTest.java index 89c474e0d44..d7009f4f909 100644 --- a/core/sail/lucene-api/src/test/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilderTest.java +++ b/core/sail/lucene-api/src/test/java/org/eclipse/rdf4j/sail/lucene/QuerySpecBuilderTest.java @@ -14,6 +14,7 @@ import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.BOOST; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.LUCENE_QUERY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.MATCHES; +import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.NUM_DOCS; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.QUERY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.SCORE; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.SNIPPET; @@ -55,6 +56,7 @@ public void testQueryInterpretation() { "<" + TYPE + "> <" + LUCENE_QUERY + ">; " + "<" + QUERY + "> \"my Lucene query\"; " + "<" + SCORE + "> ?Score; " + + "<" + NUM_DOCS + "> 76; " + "<" + SNIPPET + "> ?Snippet ]. } "; ParsedQuery query = parser.parseQuery(buffer, null); TupleExpr tupleExpr = query.getTupleExpr(); @@ -69,6 +71,8 @@ public void testQueryInterpretation() { assertEquals("Score", querySpec.getScorePattern().getObjectVar().getName()); assertEquals("Snippet", param.getSnippetPattern().getObjectVar().getName()); assertEquals(LUCENE_QUERY, querySpec.getTypePattern().getObjectVar().getValue()); + assertEquals(76, querySpec.getNumDocs()); + assertEquals(76, ((Literal) querySpec.getNumDocsPattern().getObjectVar().getValue()).intValue()); assertEquals("my Lucene query", param.getQuery()); assertNull(querySpec.getSubject()); } @@ -80,11 +84,13 @@ public void testMultipleQueriesInterpretation() { "<" + TYPE + "> <" + LUCENE_QUERY + ">; " + "<" + QUERY + "> \"my Lucene query\"; " + "<" + SCORE + "> ?score1; " + + "<" + NUM_DOCS + "> 86; " + "<" + SNIPPET + "> ?snippet1 ]. " + " ?sub2 <" + MATCHES + "> [ " + "<" + TYPE + "> <" + LUCENE_QUERY + ">; " + "<" + QUERY + "> \"second lucene query\"; " + "<" + SCORE + "> ?score2; " + + "<" + NUM_DOCS + "> 13; " + "<" + SNIPPET + "> ?snippet2 ]. " + // and connect them both via any X in between, just as salt to make the // parser do something @@ -103,6 +109,7 @@ public void testMultipleQueriesInterpretation() { // Matched the first assertEquals("sub1", querySpec.getMatchesPattern().getSubjectVar().getName()); assertEquals(1, querySpec.getQueryPatterns().size()); + assertEquals(86, querySpec.getNumDocs()); QuerySpec.QueryParam param = querySpec.getQueryPatterns().iterator().next(); assertEquals("my Lucene query", ((Literal) param.getQueryPattern().getObjectVar().getValue()).getLabel()); @@ -116,6 +123,7 @@ public void testMultipleQueriesInterpretation() { // and the second assertEquals("sub2", querySpec.getMatchesPattern().getSubjectVar().getName()); assertEquals(1, querySpec.getQueryPatterns().size()); + assertEquals(13, querySpec.getNumDocs()); QuerySpec.QueryParam param = querySpec.getQueryPatterns().iterator().next(); assertEquals("second lucene query", ((Literal) param.getQueryPattern().getObjectVar().getValue()).getLabel()); diff --git a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java index 42174324308..5a4965b088d 100644 --- a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java +++ b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java @@ -751,11 +751,21 @@ protected Iterable query(Resource subject, QuerySpec sp highlighter = null; } + Integer numDocs = spec.getNumDocs(); + TopDocs docs; if (subject != null) { - docs = search(subject, q); + if (numDocs != null) { + docs = search(subject, q, numDocs); + } else { + docs = search(subject, q); + } } else { - docs = search(q); + if (numDocs != null) { + docs = search(q, numDocs); + } else { + docs = search(q); + } } return Iterables.transform(Arrays.asList(docs.scoreDocs), (ScoreDoc doc) -> new LuceneDocumentScore(doc, highlighter, LuceneIndex.this)); @@ -960,12 +970,25 @@ public synchronized String getSnippet(String fieldName, String text, Highlighter * @throws IOException */ public synchronized TopDocs search(Resource resource, Query query) throws IOException { + return search(resource, query, -1); + } + + /** + * Evaluates the given query only for the given resource. + * + * @param resource + * @param query + * @param numDocs + * @return top documents + * @throws IOException + */ + public synchronized TopDocs search(Resource resource, Query query, int numDocs) throws IOException { // rewrite the query TermQuery idQuery = new TermQuery(new Term(SearchFields.URI_FIELD_NAME, SearchFields.getResourceID(resource))); BooleanQuery.Builder combinedQuery = new BooleanQuery.Builder(); combinedQuery.add(idQuery, Occur.MUST); combinedQuery.add(query, Occur.MUST); - return search(combinedQuery.build()); + return search(combinedQuery.build(), numDocs); } /** @@ -976,8 +999,26 @@ public synchronized TopDocs search(Resource resource, Query query) throws IOExce * @throws IOException */ public synchronized TopDocs search(Query query) throws IOException { + return search(query, -1); + } + + /** + * Evaluates the given query and returns the results as a TopDocs instance. + * + * @param query + * @param numDocs + * @return top documents + * @throws IOException + */ + public synchronized TopDocs search(Query query, int numDocs) throws IOException { int nDocs; - if (maxDocs > 0) { + if (numDocs > 0) { + if (maxQueryDocs > 0 && maxQueryDocs < numDocs) { + nDocs = maxQueryDocs; + } else { + nDocs = numDocs; + } + } else if (maxDocs > 0) { nDocs = maxDocs; } else { nDocs = Math.max(getIndexReader().numDocs(), 1); diff --git a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java index 31cdfb3a440..34ea7b799a8 100644 --- a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java +++ b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java @@ -317,11 +317,20 @@ protected Iterable query(Resource subject, QuerySpec sp q.addField(SearchFields.URI_FIELD_NAME); } q.addField("score"); + Integer numDocs = spec.getNumDocs(); try { if (subject != null) { - response = search(subject, q); + if (numDocs != null) { + response = search(subject, q, numDocs); + } else { + response = search(subject, q); + } } else { - response = search(q); + if (numDocs != null) { + response = search(q, numDocs); + } else { + response = search(q); + } } } catch (SolrServerException e) { throw new IOException(e); @@ -346,10 +355,25 @@ protected Iterable query(Resource subject, QuerySpec sp * @throws IOException */ public QueryResponse search(Resource resource, SolrQuery query) throws SolrServerException, IOException { + return search(resource, query, -1); + } + + /** + * Evaluates the given query only for the given resource. + * + * @param resource + * @param query + * @param numDocs + * @return response + * @throws SolrServerException + * @throws IOException + */ + public QueryResponse search(Resource resource, SolrQuery query, int numDocs) + throws SolrServerException, IOException { // rewrite the query String idQuery = termQuery(SearchFields.URI_FIELD_NAME, SearchFields.getResourceID(resource)); query.setQuery(query.getQuery() + " AND " + idQuery); - return search(query); + return search(query, numDocs); } @Override @@ -553,8 +577,27 @@ public double getY() { * @throws IOException */ public QueryResponse search(SolrQuery query) throws SolrServerException, IOException { + return search(query, -1); + } + + /** + * Evaluates the given query and returns the results as a TopDocs instance. + * + * @param query + * @param numDocs + * @return query response + * @throws SolrServerException + * @throws IOException + */ + public QueryResponse search(SolrQuery query, int numDocs) throws SolrServerException, IOException { int nDocs; - if (maxDocs > 0) { + if (numDocs > 0) { + if (maxQueryDocs > 0 && maxQueryDocs < numDocs) { + nDocs = maxQueryDocs; + } else { + nDocs = numDocs; + } + } else if (maxDocs > 0) { nDocs = maxDocs; } else { long docCount = client.query(query.setRows(0)).getResults().getNumFound(); diff --git a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java index d17d17b353a..84391587d15 100644 --- a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java +++ b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java @@ -11,6 +11,7 @@ package org.eclipse.testsuite.rdf4j.sail.lucene; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.MATCHES; +import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.NUM_DOCS; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.PROPERTY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.QUERY; import static org.eclipse.rdf4j.sail.lucene.LuceneSailSchema.SCORE; @@ -21,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -33,6 +35,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; import org.eclipse.rdf4j.model.IRI; import org.eclipse.rdf4j.model.Literal; @@ -53,6 +56,7 @@ import org.eclipse.rdf4j.repository.RepositoryConnection; import org.eclipse.rdf4j.repository.RepositoryException; import org.eclipse.rdf4j.repository.sail.SailRepository; +import org.eclipse.rdf4j.repository.util.Repositories; import org.eclipse.rdf4j.sail.lucene.LuceneSail; import org.eclipse.rdf4j.sail.lucene.LuceneSailSchema; import org.eclipse.rdf4j.sail.memory.MemoryStore; @@ -107,17 +111,19 @@ public abstract class AbstractLuceneSailTest { protected abstract void configure(LuceneSail sail); - @BeforeEach - public void setUp() { - // set logging, uncomment this to get better logging for debugging - // org.apache.log4j.BasicConfigurator.configure(); - + private void createTestSail(Consumer config) throws IOException { + if (repository != null) { + repository.shutDown(); + repository = null; + } // setup a LuceneSail MemoryStore memoryStore = new MemoryStore(); // enable lock tracking org.eclipse.rdf4j.common.concurrent.locks.Properties.setLockTrackingEnabled(true); sail = new LuceneSail(); + configure(sail); + config.accept(sail); sail.setBaseSail(memoryStore); // create a Repository wrapping the LuceneSail @@ -139,10 +145,19 @@ public void setUp() { } } + @BeforeEach + public void setUp() throws Exception { + // set logging, uncomment this to get better logging for debugging + // org.apache.log4j.BasicConfigurator.configure(); + createTestSail(lc -> { + }); + } + @AfterEach public void tearDown() throws RepositoryException { if (repository != null) { repository.shutDown(); + repository = null; } org.eclipse.rdf4j.common.concurrent.locks.Properties.setLockTrackingEnabled(false); } @@ -1081,6 +1096,67 @@ public void run() { assertEquals(0, exceptions.size(), "Exceptions occurred during testMultithreadedAdd, see stacktraces above"); } + @Test + public void testMaxNumDocsResult() throws IOException { + for (int i = 1; i <= 3; i++) { + final int j = i; + createTestSail(lc -> lc.setParameter(LuceneSail.MAX_QUERY_DOCUMENTS_KEY, String.valueOf(j))); + Repositories.consumeNoTransaction(repository, conn -> { + try (TupleQueryResult res = conn.prepareTupleQuery( + "SELECT ?Resource {\n" + + " ?Resource <" + MATCHES + "> [\n " + + " <" + QUERY + "> \"one\";\n " + + " <" + NUM_DOCS + "> 3;\n " + + " ]. } " + ).evaluate()) { + for (int k = 0; k < j; k++) { + assertTrue(res.hasNext(), "missing result #" + k); + res.next(); + } + if (res.hasNext()) { + StringBuilder b = new StringBuilder(); + int r = 0; + do { + b.append("\n#").append(r++).append(res.next()); + } while (res.hasNext()); + fail("can't have more than " + j + " result(s)" + b); + } + } + ; + }); + } + } + + @Test + public void testNumDocsResult() { + for (int i = 1; i <= 3; i++) { + final int j = i; + Repositories.consumeNoTransaction(repository, conn -> { + try (TupleQueryResult res = conn.prepareTupleQuery( + "SELECT ?Resource {\n" + + " ?Resource <" + MATCHES + "> [\n " + + " <" + QUERY + "> \"one\";\n " + + " <" + NUM_DOCS + "> " + j + ";\n " + + " ]. } " + ).evaluate()) { + for (int k = 0; k < j; k++) { + assertTrue(res.hasNext(), "missing result #" + k); + res.next(); + } + if (res.hasNext()) { + StringBuilder b = new StringBuilder(); + int r = 0; + do { + b.append("\n#").append(r++).append(res.next()); + } while (res.hasNext()); + fail("can't have more than " + j + " result(s)" + b); + } + } + ; + }); + } + } + protected void assertQueryResult(String literal, IRI predicate, Resource resultUri) { try (RepositoryConnection connection = repository.getConnection()) { // fire a query for all subjects with a given term From 3e6454efc2d87a5934a3f1b9fe3beb59e9814d3d Mon Sep 17 00:00:00 2001 From: qaate47 Date: Tue, 5 Nov 2024 14:05:30 +0100 Subject: [PATCH 2/4] GH-5149 Use ParameterizedTest and Objects.requireNonNull --- .../elasticsearch/ElasticsearchIndex.java | 15 +-- .../rdf4j/sail/lucene/impl/LuceneIndex.java | 15 +-- .../eclipse/rdf4j/sail/solr/SolrIndex.java | 15 +-- testsuites/lucene/pom.xml | 5 + .../sail/lucene/AbstractLuceneSailTest.java | 109 +++++++++--------- 5 files changed, 69 insertions(+), 90 deletions(-) diff --git a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java index 29a2d1e74c5..65e5e8a1502 100644 --- a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java +++ b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java @@ -18,6 +18,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; @@ -577,19 +578,11 @@ protected Iterable query(Resource subject, QuerySpec sp } SearchHits hits; - Integer numDocs = spec.getNumDocs(); + int numDocs = Objects.requireNonNullElse(spec.getNumDocs(), -1); if (subject != null) { - if (numDocs != null) { - hits = search(subject, request, qb, numDocs); - } else { - hits = search(subject, request, qb); - } + hits = search(subject, request, qb, numDocs); } else { - if (numDocs != null) { - hits = search(request, qb, numDocs); - } else { - hits = search(request, qb); - } + hits = search(request, qb, numDocs); } return Iterables.transform(hits, new Function<>() { diff --git a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java index 5a4965b088d..16885229230 100644 --- a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java +++ b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java @@ -25,6 +25,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -751,21 +752,13 @@ protected Iterable query(Resource subject, QuerySpec sp highlighter = null; } - Integer numDocs = spec.getNumDocs(); + int numDocs = Objects.requireNonNullElse(spec.getNumDocs(), -1); TopDocs docs; if (subject != null) { - if (numDocs != null) { - docs = search(subject, q, numDocs); - } else { - docs = search(subject, q); - } + docs = search(subject, q, numDocs); } else { - if (numDocs != null) { - docs = search(q, numDocs); - } else { - docs = search(q); - } + docs = search(q, numDocs); } return Iterables.transform(Arrays.asList(docs.scoreDocs), (ScoreDoc doc) -> new LuceneDocumentScore(doc, highlighter, LuceneIndex.this)); diff --git a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java index 34ea7b799a8..ab9e7282324 100644 --- a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java +++ b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java @@ -16,6 +16,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; @@ -317,20 +318,12 @@ protected Iterable query(Resource subject, QuerySpec sp q.addField(SearchFields.URI_FIELD_NAME); } q.addField("score"); - Integer numDocs = spec.getNumDocs(); + int numDocs = Objects.requireNonNullElse(spec.getNumDocs(), -1); try { if (subject != null) { - if (numDocs != null) { - response = search(subject, q, numDocs); - } else { - response = search(subject, q); - } + response = search(subject, q, numDocs); } else { - if (numDocs != null) { - response = search(q, numDocs); - } else { - response = search(q); - } + response = search(q, numDocs); } } catch (SolrServerException e) { throw new IOException(e); diff --git a/testsuites/lucene/pom.xml b/testsuites/lucene/pom.xml index cf0c02226d8..66488c8e819 100644 --- a/testsuites/lucene/pom.xml +++ b/testsuites/lucene/pom.xml @@ -35,5 +35,10 @@ junit-vintage-engine compile + + org.junit.jupiter + junit-jupiter-params + compile + diff --git a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java index 84391587d15..9d3bda29f33 100644 --- a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java +++ b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java @@ -22,7 +22,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -64,6 +63,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Timeout(value = 10, unit = TimeUnit.MINUTES) public abstract class AbstractLuceneSailTest { @@ -111,7 +112,7 @@ public abstract class AbstractLuceneSailTest { protected abstract void configure(LuceneSail sail); - private void createTestSail(Consumer config) throws IOException { + private void createTestSail(Consumer config) { if (repository != null) { repository.shutDown(); repository = null; @@ -1096,65 +1097,59 @@ public void run() { assertEquals(0, exceptions.size(), "Exceptions occurred during testMultithreadedAdd, see stacktraces above"); } - @Test - public void testMaxNumDocsResult() throws IOException { - for (int i = 1; i <= 3; i++) { - final int j = i; - createTestSail(lc -> lc.setParameter(LuceneSail.MAX_QUERY_DOCUMENTS_KEY, String.valueOf(j))); - Repositories.consumeNoTransaction(repository, conn -> { - try (TupleQueryResult res = conn.prepareTupleQuery( - "SELECT ?Resource {\n" - + " ?Resource <" + MATCHES + "> [\n " - + " <" + QUERY + "> \"one\";\n " - + " <" + NUM_DOCS + "> 3;\n " - + " ]. } " - ).evaluate()) { - for (int k = 0; k < j; k++) { - assertTrue(res.hasNext(), "missing result #" + k); - res.next(); - } - if (res.hasNext()) { - StringBuilder b = new StringBuilder(); - int r = 0; - do { - b.append("\n#").append(r++).append(res.next()); - } while (res.hasNext()); - fail("can't have more than " + j + " result(s)" + b); - } + @ParameterizedTest + @ValueSource(ints = { 1, 2, 3 }) + public void testMaxNumDocsResult(int numDoc) { + createTestSail(lc -> lc.setParameter(LuceneSail.MAX_QUERY_DOCUMENTS_KEY, String.valueOf(numDoc))); + Repositories.consumeNoTransaction(repository, conn -> { + try (TupleQueryResult res = conn.prepareTupleQuery( + "SELECT ?Resource {\n" + + " ?Resource <" + MATCHES + "> [\n " + + " <" + QUERY + "> \"one\";\n " + + " <" + NUM_DOCS + "> 3;\n " + + " ]. } " + ).evaluate()) { + for (int k = 0; k < numDoc; k++) { + assertTrue(res.hasNext(), "missing result #" + k); + res.next(); } - ; - }); - } + if (res.hasNext()) { + StringBuilder b = new StringBuilder(); + int r = 0; + do { + b.append("\n#").append(r++).append(res.next()); + } while (res.hasNext()); + fail("can't have more than " + numDoc + " result(s)" + b); + } + } + }); } - @Test - public void testNumDocsResult() { - for (int i = 1; i <= 3; i++) { - final int j = i; - Repositories.consumeNoTransaction(repository, conn -> { - try (TupleQueryResult res = conn.prepareTupleQuery( - "SELECT ?Resource {\n" - + " ?Resource <" + MATCHES + "> [\n " - + " <" + QUERY + "> \"one\";\n " - + " <" + NUM_DOCS + "> " + j + ";\n " - + " ]. } " - ).evaluate()) { - for (int k = 0; k < j; k++) { - assertTrue(res.hasNext(), "missing result #" + k); - res.next(); - } - if (res.hasNext()) { - StringBuilder b = new StringBuilder(); - int r = 0; - do { - b.append("\n#").append(r++).append(res.next()); - } while (res.hasNext()); - fail("can't have more than " + j + " result(s)" + b); - } + @ParameterizedTest + @ValueSource(ints = { 1, 2, 3 }) + public void testNumDocsResult(int numDoc) { + Repositories.consumeNoTransaction(repository, conn -> { + try (TupleQueryResult res = conn.prepareTupleQuery( + "SELECT ?Resource {\n" + + " ?Resource <" + MATCHES + "> [\n " + + " <" + QUERY + "> \"one\";\n " + + " <" + NUM_DOCS + "> " + numDoc + ";\n " + + " ]. } " + ).evaluate()) { + for (int k = 0; k < numDoc; k++) { + assertTrue(res.hasNext(), "missing result #" + k); + res.next(); } - ; - }); - } + if (res.hasNext()) { + StringBuilder b = new StringBuilder(); + int r = 0; + do { + b.append("\n#").append(r++).append(res.next()); + } while (res.hasNext()); + fail("can't have more than " + numDoc + " result(s)" + b); + } + } + }); } protected void assertQueryResult(String literal, IRI predicate, Resource resultUri) { From 42a331d31c13c6d265fdb8f982dd5c1f3b9f3b0a Mon Sep 17 00:00:00 2001 From: qaate47 Date: Thu, 14 Nov 2024 12:01:31 +0100 Subject: [PATCH 3/4] GH-5149 Rename MAX_QUERY_DOCUMENTS_KEY to MAX_DOCUMENTS_KEY and MAX_DOCUMENTS_KEY to DEFAULT_NUM_DOCS_KEY --- .../elasticsearch/ElasticsearchIndex.java | 8 ++--- .../sail/lucene/AbstractSearchIndex.java | 10 +++---- .../eclipse/rdf4j/sail/lucene/LuceneSail.java | 12 ++++---- .../rdf4j/sail/lucene/impl/LuceneIndex.java | 8 ++--- .../eclipse/rdf4j/sail/solr/SolrIndex.java | 8 ++--- .../sail/lucene/AbstractLuceneSailTest.java | 29 ++++++++++++++++++- 6 files changed, 51 insertions(+), 24 deletions(-) diff --git a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java index 65e5e8a1502..19846bfc827 100644 --- a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java +++ b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java @@ -737,13 +737,13 @@ public SearchHits search(SearchRequestBuilder request, QueryBuilder query, int n String[] types = getTypes(); int nDocs; if (numDocs > 0) { - if (maxQueryDocs > 0 && maxQueryDocs < numDocs) { - nDocs = maxQueryDocs; + if (maxDocs > 0 && maxDocs < numDocs) { + nDocs = maxDocs; } else { nDocs = numDocs; } - } else if (maxDocs > 0) { - nDocs = maxDocs; + } else if (defaultNumDocs > 0) { + nDocs = defaultNumDocs; } else { long docCount = client.prepareSearch(indexName) .setTypes(types) diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java index 9423e9dd95f..e52fc63ab81 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java @@ -65,8 +65,8 @@ public abstract class AbstractSearchIndex implements SearchIndex { REJECTED_DATATYPES.add("http://www.w3.org/2001/XMLSchema#float"); } + protected int defaultNumDocs; protected int maxDocs; - protected int maxQueryDocs; protected Set wktFields = Collections.singleton(SearchFields.getPropertyField(GEO.AS_WKT)); @@ -76,10 +76,10 @@ public abstract class AbstractSearchIndex implements SearchIndex { @Override public void initialize(Properties parameters) throws Exception { - String maxDocParam = parameters.getProperty(LuceneSail.MAX_DOCUMENTS_KEY); - maxDocs = (maxDocParam != null) ? Integer.parseInt(maxDocParam) : -1; - String maxQueryDocParam = parameters.getProperty(LuceneSail.MAX_QUERY_DOCUMENTS_KEY); - maxQueryDocs = (maxQueryDocParam != null) ? Integer.parseInt(maxQueryDocParam) : maxDocs; + String maxDocumentsParam = parameters.getProperty(LuceneSail.MAX_DOCUMENTS_KEY); + maxDocs = (maxDocumentsParam != null) ? Integer.parseInt(maxDocumentsParam) : -1; + String defaultNumDocsParam = parameters.getProperty(LuceneSail.DEFAULT_NUM_DOCS_KEY); + defaultNumDocs = (defaultNumDocsParam != null) ? Integer.parseInt(defaultNumDocsParam) : defaultNumDocs; String wktFieldParam = parameters.getProperty(LuceneSail.WKT_FIELDS); if (wktFieldParam != null) { diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java index c3b0d90a665..f2d96ecc435 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/LuceneSail.java @@ -290,18 +290,18 @@ public class LuceneSail extends NotifyingSailWrapper { public static final String LUCENE_RAMDIR_KEY = "useramdir"; /** - * Set the key "maxDocuments=<n>" as sail parameter to limit the maximum number of documents to return from a - * search query. The default is to return all documents. NB: this may involve extra cost for some SearchIndex + * Set the key "defaultNumDocs=<n>" as sail parameter to limit the maximum number of documents to return from + * a search query. The default is to return all documents. NB: this may involve extra cost for some SearchIndex * implementations as they may have to determine this number. */ - public static final String MAX_DOCUMENTS_KEY = "maxDocuments"; + public static final String DEFAULT_NUM_DOCS_KEY = "defaultNumDocs"; /** - * Set the key "maxQueryDocuments=<n>" as sail parameter to limit the maximum number of documents the user can - * query at a time to return from a search query. The default is the value of the {@link #MAX_DOCUMENTS_KEY} + * Set the key "maxDocuments=<n>" as sail parameter to limit the maximum number of documents the user can + * query at a time to return from a search query. The default is the value of the {@link #DEFAULT_NUM_DOCS_KEY} * parameter. */ - public static final String MAX_QUERY_DOCUMENTS_KEY = "maxQueryDocuments"; + public static final String MAX_DOCUMENTS_KEY = "maxDocuments"; /** * Set this key to configure which fields contain WKT and should be spatially indexed. The value should be a diff --git a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java index 16885229230..5e17fa2b36d 100644 --- a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java +++ b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java @@ -1006,13 +1006,13 @@ public synchronized TopDocs search(Query query) throws IOException { public synchronized TopDocs search(Query query, int numDocs) throws IOException { int nDocs; if (numDocs > 0) { - if (maxQueryDocs > 0 && maxQueryDocs < numDocs) { - nDocs = maxQueryDocs; + if (maxDocs > 0 && maxDocs < numDocs) { + nDocs = maxDocs; } else { nDocs = numDocs; } - } else if (maxDocs > 0) { - nDocs = maxDocs; + } else if (defaultNumDocs > 0) { + nDocs = defaultNumDocs; } else { nDocs = Math.max(getIndexReader().numDocs(), 1); } diff --git a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java index ab9e7282324..a7c54396cf2 100644 --- a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java +++ b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java @@ -585,13 +585,13 @@ public QueryResponse search(SolrQuery query) throws SolrServerException, IOExcep public QueryResponse search(SolrQuery query, int numDocs) throws SolrServerException, IOException { int nDocs; if (numDocs > 0) { - if (maxQueryDocs > 0 && maxQueryDocs < numDocs) { - nDocs = maxQueryDocs; + if (maxDocs > 0 && maxDocs < numDocs) { + nDocs = maxDocs; } else { nDocs = numDocs; } - } else if (maxDocs > 0) { - nDocs = maxDocs; + } else if (defaultNumDocs > 0) { + nDocs = defaultNumDocs; } else { long docCount = client.query(query.setRows(0)).getResults().getNumFound(); nDocs = Math.max((int) Math.min(docCount, Integer.MAX_VALUE), 1); diff --git a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java index 9d3bda29f33..fa01c118a38 100644 --- a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java +++ b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java @@ -1097,10 +1097,37 @@ public void run() { assertEquals(0, exceptions.size(), "Exceptions occurred during testMultithreadedAdd, see stacktraces above"); } + @ParameterizedTest + @ValueSource(ints = { 1, 2, 3 }) + public void testDefaultNumDocsResult(int numDoc) { + createTestSail(lc -> lc.setParameter(LuceneSail.DEFAULT_NUM_DOCS_KEY, String.valueOf(numDoc))); + Repositories.consumeNoTransaction(repository, conn -> { + try (TupleQueryResult res = conn.prepareTupleQuery( + "SELECT ?Resource {\n" + + " ?Resource <" + MATCHES + "> [\n " + + " <" + QUERY + "> \"one\"\n " + + " ]. } " + ).evaluate()) { + for (int k = 0; k < numDoc; k++) { + assertTrue(res.hasNext(), "missing result #" + k); + res.next(); + } + if (res.hasNext()) { + StringBuilder b = new StringBuilder(); + int r = 0; + do { + b.append("\n#").append(r++).append(res.next()); + } while (res.hasNext()); + fail("can't have more than " + numDoc + " result(s)" + b); + } + } + }); + } + @ParameterizedTest @ValueSource(ints = { 1, 2, 3 }) public void testMaxNumDocsResult(int numDoc) { - createTestSail(lc -> lc.setParameter(LuceneSail.MAX_QUERY_DOCUMENTS_KEY, String.valueOf(numDoc))); + createTestSail(lc -> lc.setParameter(LuceneSail.MAX_DOCUMENTS_KEY, String.valueOf(numDoc))); Repositories.consumeNoTransaction(repository, conn -> { try (TupleQueryResult res = conn.prepareTupleQuery( "SELECT ?Resource {\n" From 5833e571454370813c37b8139f46dd6e83e2a70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Wed, 20 Nov 2024 16:02:51 +0100 Subject: [PATCH 4/4] GH-5149 code simplification and cleanup --- .../java/org/eclipse/rdf4j/model/Model.java | 2 +- .../rdf4j/model/util/Configurations.java | 2 +- .../rdf4j/model/util/GraphComparisons.java | 2 +- .../evaluation/function/string/Contains.java | 2 +- .../rdf4j/rio/helpers/RDFParserHelper.java | 2 +- .../legacy/JSONLDInternalRDFParser.java | 2 +- .../elasticsearch/ElasticsearchIndex.java | 76 +++++++++---------- .../sail/lucene/AbstractSearchIndex.java | 44 ++++++++--- .../rdf4j/sail/lucene/impl/LuceneIndex.java | 37 +++++---- .../UniqueLangConstraintComponent.java | 2 +- .../ast/planNodes/NonUniqueTargetLang.java | 2 +- .../eclipse/rdf4j/sail/solr/SolrIndex.java | 26 ++++--- .../sail/lucene/AbstractLuceneSailTest.java | 20 +++++ .../rdf4j/console/command/Convert.java | 4 +- .../rdf4j/console/command/QueryEvaluator.java | 4 +- 15 files changed, 141 insertions(+), 86 deletions(-) diff --git a/core/model-api/src/main/java/org/eclipse/rdf4j/model/Model.java b/core/model-api/src/main/java/org/eclipse/rdf4j/model/Model.java index c409dd4eeed..3c83fdfc658 100644 --- a/core/model-api/src/main/java/org/eclipse/rdf4j/model/Model.java +++ b/core/model-api/src/main/java/org/eclipse/rdf4j/model/Model.java @@ -45,7 +45,7 @@ public interface Model extends Set, Serializable, NamespaceAware { */ default Namespace setNamespace(String prefix, String name) { Optional result = getNamespace(prefix); - if (!result.isPresent() || !result.get().getName().equals(name)) { + if (result.isEmpty() || !result.get().getName().equals(name)) { result = Optional.of(new ModelNamespace(prefix, name)); setNamespace(result.get()); } diff --git a/core/model/src/main/java/org/eclipse/rdf4j/model/util/Configurations.java b/core/model/src/main/java/org/eclipse/rdf4j/model/util/Configurations.java index b74e2f84ec4..7c9bb003ea2 100644 --- a/core/model/src/main/java/org/eclipse/rdf4j/model/util/Configurations.java +++ b/core/model/src/main/java/org/eclipse/rdf4j/model/util/Configurations.java @@ -240,7 +240,7 @@ public static Optional getSubjectByType(Model model, IRI type, IRI leg private static void logDiscrepancyWarning(Optional preferred, Optional fallback) { - if (!fallback.isEmpty() && !preferred.equals(fallback)) { + if (fallback.isPresent() && !preferred.equals(fallback)) { var msg = "Discrepancy between use of the old and new config vocabulary."; // depending on whether preferred is set, we log on warn or debug if (preferred.isEmpty()) { diff --git a/core/model/src/main/java/org/eclipse/rdf4j/model/util/GraphComparisons.java b/core/model/src/main/java/org/eclipse/rdf4j/model/util/GraphComparisons.java index 0c5911ee00f..1f3ceb59c80 100644 --- a/core/model/src/main/java/org/eclipse/rdf4j/model/util/GraphComparisons.java +++ b/core/model/src/main/java/org/eclipse/rdf4j/model/util/GraphComparisons.java @@ -169,7 +169,7 @@ private static boolean isomorphicSingleContext(Model model1, Model model2) { // Because we have previously already checked that the models are the same size, we don't have to check both // ways to establish model equality. - return !missingInModel2.isPresent(); + return missingInModel2.isEmpty(); } private static boolean mappingsIncompatible(Map mapping1, Map mapping2) { diff --git a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/function/string/Contains.java b/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/function/string/Contains.java index df4b84dd32a..8e5c5191cdf 100644 --- a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/function/string/Contains.java +++ b/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/function/string/Contains.java @@ -45,7 +45,7 @@ public Literal evaluate(ValueFactory valueFactory, Value... args) throws ValueEx Literal rightLit = (Literal) rightVal; if (leftLit.getLanguage().isPresent()) { - if (!rightLit.getLanguage().isPresent() || rightLit.getLanguage().equals(leftLit.getLanguage())) { + if (rightLit.getLanguage().isEmpty() || rightLit.getLanguage().equals(leftLit.getLanguage())) { String leftLexVal = leftLit.getLabel(); String rightLexVal = rightLit.getLabel(); diff --git a/core/rio/api/src/main/java/org/eclipse/rdf4j/rio/helpers/RDFParserHelper.java b/core/rio/api/src/main/java/org/eclipse/rdf4j/rio/helpers/RDFParserHelper.java index 3d9d0f5414e..fbb27d608ab 100644 --- a/core/rio/api/src/main/java/org/eclipse/rdf4j/rio/helpers/RDFParserHelper.java +++ b/core/rio/api/src/main/java/org/eclipse/rdf4j/rio/helpers/RDFParserHelper.java @@ -190,7 +190,7 @@ public static Literal createLiteral(String label, String lang, IRI datatype, Par try { // Removes datatype for langString datatype with no language tag when VERIFY_DATATYPE_VALUES is False. if ((workingDatatype == null || RDF.LANGSTRING.equals(workingDatatype)) - && (!workingLang.isPresent() || workingLang.get().isEmpty()) + && (workingLang.isEmpty() || workingLang.get().isEmpty()) && !parserConfig.get(BasicParserSettings.VERIFY_DATATYPE_VALUES)) { workingLang = Optional.ofNullable(null); workingDatatype = null; diff --git a/core/rio/jsonld-legacy/src/main/java/org/eclipse/rdf4j/rio/jsonld/legacy/JSONLDInternalRDFParser.java b/core/rio/jsonld-legacy/src/main/java/org/eclipse/rdf4j/rio/jsonld/legacy/JSONLDInternalRDFParser.java index f61f310ef3a..2bb9232b2e2 100644 --- a/core/rio/jsonld-legacy/src/main/java/org/eclipse/rdf4j/rio/jsonld/legacy/JSONLDInternalRDFParser.java +++ b/core/rio/jsonld-legacy/src/main/java/org/eclipse/rdf4j/rio/jsonld/legacy/JSONLDInternalRDFParser.java @@ -64,7 +64,7 @@ public void handleStatement(RDFDataset result, Statement nextStatement) { // In RDF-1.1, RDF-1.0 Plain Literals are now Typed Literals with // type xsd:String - if (!literal.getLanguage().isPresent() && datatype == null) { + if (literal.getLanguage().isEmpty() && datatype == null) { datatype = XSD.STRING.stringValue(); } diff --git a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java index 19846bfc827..9373162dd79 100644 --- a/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java +++ b/core/sail/elasticsearch/src/main/java/org/eclipse/rdf4j/sail/elasticsearch/ElasticsearchIndex.java @@ -18,7 +18,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Objects; import java.util.Properties; import java.util.Set; @@ -88,14 +87,13 @@ /** * Requires an Elasticsearch cluster with the DeleteByQuery plugin. - * + *

* Note that, while RDF4J is licensed under the EDL, several ElasticSearch dependencies are licensed under the Elastic * license or the SSPL, which may have implications for some projects. - * + *

* Please consult the ElasticSearch website and license FAQ for more information. * * @see Elastic License FAQ - * * @see LuceneSail */ public class ElasticsearchIndex extends AbstractSearchIndex { @@ -410,13 +408,8 @@ protected SearchDocument getDocument(String id) throws IOException { @Override protected Iterable getDocuments(String resourceId) throws IOException { SearchHits hits = getDocuments(QueryBuilders.termQuery(SearchFields.URI_FIELD_NAME, resourceId)); - return Iterables.transform(hits, new Function<>() { - - @Override - public SearchDocument apply(SearchHit hit) { - return new ElasticsearchDocument(hit, geoContextMapper); - } - }); + return Iterables.transform(hits, + (Function) hit -> new ElasticsearchDocument(hit, geoContextMapper)); } @Override @@ -578,19 +571,26 @@ protected Iterable query(Resource subject, QuerySpec sp } SearchHits hits; - int numDocs = Objects.requireNonNullElse(spec.getNumDocs(), -1); + + int numDocs; + + Integer specNumDocs = spec.getNumDocs(); + if (specNumDocs != null) { + if (specNumDocs < 0) { + throw new IllegalArgumentException("numDocs must be >= 0"); + } + numDocs = specNumDocs; + } else { + numDocs = -1; + } + if (subject != null) { hits = search(subject, request, qb, numDocs); } else { hits = search(request, qb, numDocs); } - return Iterables.transform(hits, new Function<>() { - - @Override - public DocumentScore apply(SearchHit hit) { - return new ElasticsearchDocumentScore(hit, geoContextMapper); - } - }); + return Iterables.transform(hits, + (Function) hit -> new ElasticsearchDocumentScore(hit, geoContextMapper)); } /** @@ -701,13 +701,8 @@ protected Iterable geoRelationQuery(String relation, I SearchRequestBuilder request = client.prepareSearch(); SearchHits hits = search(request, QueryBuilders.boolQuery().must(qb).filter(fb)); - return Iterables.transform(hits, new Function<>() { - - @Override - public DocumentResult apply(SearchHit hit) { - return new ElasticsearchDocumentResult(hit, geoContextMapper); - } - }); + return Iterables.transform(hits, + (Function) hit -> new ElasticsearchDocumentResult(hit, geoContextMapper)); } private ShapeRelation toSpatialOp(String relation) { @@ -735,30 +730,35 @@ public SearchHits search(SearchRequestBuilder request, QueryBuilder query) { */ public SearchHits search(SearchRequestBuilder request, QueryBuilder query, int numDocs) { String[] types = getTypes(); - int nDocs; - if (numDocs > 0) { - if (maxDocs > 0 && maxDocs < numDocs) { - nDocs = maxDocs; - } else { - nDocs = numDocs; - } - } else if (defaultNumDocs > 0) { - nDocs = defaultNumDocs; - } else { + + if (numDocs < -1) { + throw new IllegalArgumentException("numDocs should be 0 or greater if defined by the user"); + } + + int size = defaultNumDocs; + if (numDocs >= 0) { + // If the user has set numDocs we will use that. If it is 0 then the implementation may end up throwing an + // exception. + size = Math.min(maxDocs, numDocs); + } + + if (size < 0) { + // defaultNumDocs is not set long docCount = client.prepareSearch(indexName) .setTypes(types) .setSource(new SearchSourceBuilder().size(0).query(query)) .get() .getHits() .getTotalHits().value; - nDocs = Math.max((int) Math.min(docCount, Integer.MAX_VALUE), 1); + size = Math.max((int) Math.min(docCount, maxDocs), 1); } + SearchResponse response = request.setIndices(indexName) .setTypes(types) .setVersion(false) .seqNoAndPrimaryTerm(true) .setQuery(query) - .setSize(nDocs) + .setSize(size) .execute() .actionGet(); return response.getHits(); diff --git a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java index e52fc63ab81..ba3a7f3d35c 100644 --- a/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java +++ b/core/sail/lucene-api/src/main/java/org/eclipse/rdf4j/sail/lucene/AbstractSearchIndex.java @@ -41,6 +41,7 @@ import org.eclipse.rdf4j.query.MalformedQueryException; import org.eclipse.rdf4j.query.algebra.Var; import org.eclipse.rdf4j.query.algebra.evaluation.QueryBindingSet; +import org.eclipse.rdf4j.sail.Sail; import org.eclipse.rdf4j.sail.SailException; import org.eclipse.rdf4j.sail.lucene.util.MapOfListMaps; import org.locationtech.spatial4j.context.SpatialContext; @@ -65,8 +66,8 @@ public abstract class AbstractSearchIndex implements SearchIndex { REJECTED_DATATYPES.add("http://www.w3.org/2001/XMLSchema#float"); } - protected int defaultNumDocs; - protected int maxDocs; + protected int defaultNumDocs = -1; + protected int maxDocs = Integer.MAX_VALUE; protected Set wktFields = Collections.singleton(SearchFields.getPropertyField(GEO.AS_WKT)); @@ -77,9 +78,28 @@ public abstract class AbstractSearchIndex implements SearchIndex { @Override public void initialize(Properties parameters) throws Exception { String maxDocumentsParam = parameters.getProperty(LuceneSail.MAX_DOCUMENTS_KEY); - maxDocs = (maxDocumentsParam != null) ? Integer.parseInt(maxDocumentsParam) : -1; String defaultNumDocsParam = parameters.getProperty(LuceneSail.DEFAULT_NUM_DOCS_KEY); - defaultNumDocs = (defaultNumDocsParam != null) ? Integer.parseInt(defaultNumDocsParam) : defaultNumDocs; + + if ((maxDocumentsParam != null)) { + maxDocs = Integer.parseInt(maxDocumentsParam); + + // if maxDocs is set then defaultNumDocs is set to maxDocs if it is not set, because we now have a known + // upper limit + defaultNumDocs = (defaultNumDocsParam != null) ? Math.min(maxDocs, Integer.parseInt(defaultNumDocsParam)) + : maxDocs; + } else { + // we can never return more than Integer.MAX_VALUE documents + maxDocs = Integer.MAX_VALUE; + + // legacy behaviour is to return the number of documents that the query would return if there was no limit, + // so if the defaultNumDocs is not set, we set it to -1 to signal that there is no limit + defaultNumDocs = (defaultNumDocsParam != null) ? Integer.parseInt(defaultNumDocsParam) : -1; + } + + if (defaultNumDocs > maxDocs) { + throw new IllegalArgumentException(LuceneSail.DEFAULT_NUM_DOCS_KEY + " must be less than or equal to " + + LuceneSail.MAX_DOCUMENTS_KEY + " (" + defaultNumDocs + " > " + maxDocs + ")"); + } String wktFieldParam = parameters.getProperty(LuceneSail.WKT_FIELDS); if (wktFieldParam != null) { @@ -149,7 +169,7 @@ public boolean accept(Literal literal) { // we reject literals that aren't in the list of the indexed lang if (indexedLangs != null - && (!literal.getLanguage().isPresent() + && (literal.getLanguage().isEmpty() || !indexedLangs.contains(literal.getLanguage().get().toLowerCase() ))) { return false; @@ -356,11 +376,8 @@ public final synchronized void addRemoveStatements(Collection added, // remove value from both property field and the // corresponding text field String field = SearchFields.getPropertyField(r.getPredicate()); - Set removedValues = removedOfResource.get(field); - if (removedValues == null) { - removedValues = new HashSet<>(); - removedOfResource.put(field, removedValues); - } + Set removedValues = removedOfResource.computeIfAbsent(field, + k -> new HashSet<>()); removedValues.add(val); } } @@ -548,7 +565,8 @@ private Iterable evaluateQuery(QuerySpec query) { hits = query(query.getSubject(), query); } } catch (Exception e) { - logger.error("There was a problem evaluating query '" + query.getCatQuery() + "'!", e); + logger.error("There was a problem evaluating query '{}'!", query.getCatQuery(), e); + assert false : "There was a problem evaluating query '" + query.getCatQuery() + "'!"; } return hits; @@ -720,6 +738,8 @@ private Iterable evaluateQuery(DistanceQuerySpec que } catch (Exception e) { logger.error("There was a problem evaluating distance query 'within " + distance + getUnitSymbol(units) + " of " + from.getLabel() + "'!", e); + assert false : "There was a problem evaluating distance query 'within " + distance + getUnitSymbol(units) + + " of " + from.getLabel() + "'!"; } return hits; @@ -828,6 +848,8 @@ private Iterable evaluateQuery(GeoRelationQuerySpec qu } catch (Exception e) { logger.error("There was a problem evaluating spatial relation query '" + query.getRelation() + " " + qgeom.getLabel() + "'!", e); + assert false : "There was a problem evaluating spatial relation query '" + query.getRelation() + " " + + qgeom.getLabel() + "'!"; } return hits; diff --git a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java index 5e17fa2b36d..5999a91cbe8 100644 --- a/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java +++ b/core/sail/lucene/src/main/java/org/eclipse/rdf4j/sail/lucene/impl/LuceneIndex.java @@ -752,7 +752,17 @@ protected Iterable query(Resource subject, QuerySpec sp highlighter = null; } - int numDocs = Objects.requireNonNullElse(spec.getNumDocs(), -1); + int numDocs; + + Integer specNumDocs = spec.getNumDocs(); + if (specNumDocs != null) { + if (specNumDocs < 0) { + throw new IllegalArgumentException("numDocs must be >= 0"); + } + numDocs = specNumDocs; + } else { + numDocs = -1; + } TopDocs docs; if (subject != null) { @@ -1004,19 +1014,20 @@ public synchronized TopDocs search(Query query) throws IOException { * @throws IOException */ public synchronized TopDocs search(Query query, int numDocs) throws IOException { - int nDocs; - if (numDocs > 0) { - if (maxDocs > 0 && maxDocs < numDocs) { - nDocs = maxDocs; - } else { - nDocs = numDocs; - } - } else if (defaultNumDocs > 0) { - nDocs = defaultNumDocs; - } else { - nDocs = Math.max(getIndexReader().numDocs(), 1); + if (numDocs < -1) { + throw new IllegalArgumentException("numDocs should be 0 or greater if defined by the user"); + } + + int size = defaultNumDocs; + if (numDocs >= 0) { + // If the user has set numDocs we will use that. If it is 0 then the implementation may end up throwing an + // exception. + size = Math.min(maxDocs, numDocs); + } + if (size < 0) { + size = Math.max(getIndexReader().numDocs(), 1); } - return getIndexSearcher().search(query, nDocs); + return getIndexSearcher().search(query, size); } private QueryParser getQueryParser(IRI propertyURI) { diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/UniqueLangConstraintComponent.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/UniqueLangConstraintComponent.java index 453ff412372..e84386170d8 100644 --- a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/UniqueLangConstraintComponent.java +++ b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/constraintcomponents/UniqueLangConstraintComponent.java @@ -120,7 +120,7 @@ public PlanNode generateTransactionalValidationPlan(ConnectionsGroup connections connectionsGroup.getRdfsSubClassOfReasoner(), stableRandomVariableProvider); Optional path = getTargetChain().getPath(); - if (!path.isPresent() || scope != Scope.propertyShape) { + if (path.isEmpty() || scope != Scope.propertyShape) { throw new IllegalStateException("UniqueLang only operates on paths"); } diff --git a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/NonUniqueTargetLang.java b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/NonUniqueTargetLang.java index eab7562ecac..83a4424c156 100644 --- a/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/NonUniqueTargetLang.java +++ b/core/sail/shacl/src/main/java/org/eclipse/rdf4j/sail/shacl/ast/planNodes/NonUniqueTargetLang.java @@ -149,7 +149,7 @@ private void calculateNext() { if (value.isLiteral()) { Optional lang = ((Literal) value).getLanguage(); - if (!lang.isPresent()) { + if (lang.isEmpty()) { next = null; } else if (!seenLanguages.contains(lang.get())) { seenLanguages.add(lang.get()); diff --git a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java index a7c54396cf2..f04e6cc9cb6 100644 --- a/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java +++ b/core/sail/solr/src/main/java/org/eclipse/rdf4j/sail/solr/SolrIndex.java @@ -583,20 +583,22 @@ public QueryResponse search(SolrQuery query) throws SolrServerException, IOExcep * @throws IOException */ public QueryResponse search(SolrQuery query, int numDocs) throws SolrServerException, IOException { - int nDocs; - if (numDocs > 0) { - if (maxDocs > 0 && maxDocs < numDocs) { - nDocs = maxDocs; - } else { - nDocs = numDocs; - } - } else if (defaultNumDocs > 0) { - nDocs = defaultNumDocs; - } else { + if (numDocs < -1) { + throw new IllegalArgumentException("numDocs should be 0 or greater if defined by the user"); + } + + int size = defaultNumDocs; + if (numDocs >= 0) { + // If the user has set numDocs we will use that. If it is 0 then the implementation may end up throwing an + // exception. + size = Math.min(maxDocs, numDocs); + } + + if (size < 0) { long docCount = client.query(query.setRows(0)).getResults().getNumFound(); - nDocs = Math.max((int) Math.min(docCount, Integer.MAX_VALUE), 1); + size = Math.max((int) Math.min(docCount, maxDocs), 1); } - return client.query(query.setRows(nDocs)); + return client.query(query.setRows(size)); } private SolrQuery prepareQuery(IRI propertyURI, SolrQuery query) { diff --git a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java index fa01c118a38..53aa2e13261 100644 --- a/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java +++ b/testsuites/lucene/src/main/java/org/eclipse/testsuite/rdf4j/sail/lucene/AbstractLuceneSailTest.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -1179,6 +1180,25 @@ public void testNumDocsResult(int numDoc) { }); } + @ParameterizedTest + @ValueSource(ints = { -1, -2, -3 }) + public void testNumDocsResultNegative(int numDocs) { + // assert that negative values cause an assertion error + assertThrows(AssertionError.class, () -> { + Repositories.consumeNoTransaction(repository, conn -> { + try (TupleQueryResult res = conn.prepareTupleQuery( + "SELECT ?Resource {\n" + + " ?Resource <" + MATCHES + "> [\n " + + " <" + QUERY + "> \"one\";\n " + + " <" + NUM_DOCS + "> " + numDocs + ";\n " + + " ]. } " + ).evaluate()) { + assertFalse(res.hasNext()); + } + }); + }); + } + protected void assertQueryResult(String literal, IRI predicate, Resource resultUri) { try (RepositoryConnection connection = repository.getConnection()) { // fire a query for all subjects with a given term diff --git a/tools/console/src/main/java/org/eclipse/rdf4j/console/command/Convert.java b/tools/console/src/main/java/org/eclipse/rdf4j/console/command/Convert.java index 70af757cb4e..602cc55f132 100644 --- a/tools/console/src/main/java/org/eclipse/rdf4j/console/command/Convert.java +++ b/tools/console/src/main/java/org/eclipse/rdf4j/console/command/Convert.java @@ -104,7 +104,7 @@ private void convert(String fileFrom, String fileTo) { return; } Optional fmtFrom = Rio.getParserFormatForFileName(fileFrom); - if (!fmtFrom.isPresent()) { + if (fmtFrom.isEmpty()) { writeError("No RDF parser for " + fileFrom); return; } @@ -116,7 +116,7 @@ private void convert(String fileFrom, String fileTo) { return; } Optional fmtTo = Rio.getWriterFormatForFileName(fileTo); - if (!fmtTo.isPresent()) { + if (fmtTo.isEmpty()) { writeError("No RDF writer for " + fileTo); return; } diff --git a/tools/console/src/main/java/org/eclipse/rdf4j/console/command/QueryEvaluator.java b/tools/console/src/main/java/org/eclipse/rdf4j/console/command/QueryEvaluator.java index 06cfde2e29a..af11e6bbd98 100644 --- a/tools/console/src/main/java/org/eclipse/rdf4j/console/command/QueryEvaluator.java +++ b/tools/console/src/main/java/org/eclipse/rdf4j/console/command/QueryEvaluator.java @@ -316,7 +316,7 @@ private QueryResultWriter getQueryResultWriter(Path path, OutputStream out) thro w = new ConsoleQueryResultWriter(consoleIO, getConsoleWidth()); } else { Optional fmt = QueryResultIO.getWriterFormatForFileName(path.toFile().toString()); - if (!fmt.isPresent()) { + if (fmt.isEmpty()) { throw new IllegalArgumentException("No suitable result writer found"); } w = QueryResultIO.createWriter(fmt.get(), out); @@ -342,7 +342,7 @@ private RDFWriter getRDFWriter(Path path, OutputStream out) throws IllegalArgume w = new ConsoleRDFWriter(consoleIO, getConsoleWidth()); } else { Optional fmt = Rio.getWriterFormatForFileName(path.toFile().toString()); - if (!fmt.isPresent()) { + if (fmt.isEmpty()) { throw new IllegalArgumentException("No suitable result writer found"); } w = Rio.createWriter(fmt.get(), out);