diff options
63 files changed, 793 insertions, 689 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java index 55e640d4ec7..0c8c8946cbc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionActiveResponse.java @@ -18,6 +18,7 @@ public class SessionActiveResponse extends SlimeJsonResponse { Cursor root = metaData.get(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(sessionId)); root.setString("message", message); root.setString("url", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java index e436675fb59..617df3868ab 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareAndActivateResponse.java @@ -25,6 +25,7 @@ public class SessionPrepareAndActivateResponse extends SlimeJsonResponse { Cursor root = slime.get(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(result.sessionId())); root.setString("url", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName + "/application/" + applicationId.application().value() + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java index 563f50d0012..ac745f833f9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/response/SessionPrepareResponse.java @@ -31,6 +31,7 @@ public class SessionPrepareResponse extends SlimeJsonResponse { Cursor root = deployLog.get().type() != Type.NIX ? deployLog.get() : deployLog.setObject(); root.setString("tenant", tenantName.value()); + root.setString("session-id", Long.toString(sessionId)); root.setString("activate", "http://" + request.getHost() + ":" + request.getPort() + "/application/v2/tenant/" + tenantName.value() + "/session/" + sessionId + "/active"); root.setString("message", "Session " + sessionId + " for tenant '" + tenantName.value() + "' prepared."); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java index 8638a29cf75..1c71ef0b7fb 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -155,7 +155,10 @@ public class SessionActiveHandlerTest { private void assertActivationMessageOK(ActivateRequest activateRequest, String message) throws IOException { ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); new JsonFormat(true).encode(byteArrayOutputStream, activateRequest.getMetaData().getSlime()); - assertTrue(message.contains("\"tenant\":\"" + tenantName + "\",\"message\":\"Session " + activateRequest.getSessionId() + activatedMessage)); + long sessionId = activateRequest.getSessionId(); + assertTrue(message.contains("\"tenant\":\"" + tenantName)); + assertTrue(message.contains("\"session-id\":\"" + sessionId)); + assertTrue(message.contains("\"message\":\"Session " + sessionId + activatedMessage)); assertTrue(message.contains("/application/v2/tenant/" + tenantName + "/application/" + appName + "/environment/" + "prod" + diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index 66da009946e..2b07cffffce 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -61,7 +61,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { private ConfigserverConfig configserverConfig; private String preparedMessage = " prepared.\"}"; - private String tenantMessage = ""; private TenantRepository tenantRepository; @Rule @@ -90,7 +89,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { .build(); pathPrefix = "/application/v2/tenant/" + tenant + "/session/"; preparedMessage = " for tenant '" + tenant + "' prepared.\""; - tenantMessage = ",\"tenant\":\"" + tenant + "\""; } @Test @@ -123,13 +121,16 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void require_that_activate_url_is_returned_on_success() throws Exception { + public void require_that_response_has_all_fields() throws Exception { long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertNotNull(response); assertEquals(OK, response.getStatus()); - assertResponseContains(response, "\"activate\":\"http://foo:1337" + pathPrefix + sessionId + - "/active\",\"message\":\"Session " + sessionId + preparedMessage); + assertResponseContains(response, "\"activate\":\"http://foo:1337" + pathPrefix + sessionId + "/active\""); + assertResponseContains(response, "\"message\":\"Session " + sessionId + preparedMessage); + assertResponseContains(response, "\"tenant\":\"" + tenant + "\""); + assertResponseContains(response, "\"session-id\":\"" + sessionId + "\""); + assertResponseContains(response, "\"log\":[]"); } @Test @@ -186,15 +187,6 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } @Test - public void require_that_tenant_is_in_response() throws Exception { - long sessionId = createSession(applicationId()); - HttpResponse response = request(HttpRequest.Method.PUT, sessionId); - assertNotNull(response); - assertEquals(OK, response.getStatus()); - assertResponseContains(response, tenantMessage); - } - - @Test public void require_that_preparing_with_multiple_tenants_work() throws Exception { SessionHandler handler = createHandler(); @@ -307,7 +299,8 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { } private static void assertResponseContains(HttpResponse response, String string) throws IOException { - assertTrue(SessionHandlerTest.getRenderedString(response).contains(string)); + String s = SessionHandlerTest.getRenderedString(response); + assertTrue(s, s.contains(string)); } private static void assertResponseNotContains(HttpResponse response, String string) throws IOException { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index 9f57512f657..2d76db889b8 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -5,12 +5,19 @@ import com.yahoo.language.Language; import com.yahoo.language.process.Segmenter; import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; -import com.yahoo.prelude.query.*; +import com.yahoo.prelude.query.AndSegmentItem; +import com.yahoo.prelude.query.CompositeItem; +import com.yahoo.prelude.query.Item; +import com.yahoo.prelude.query.NullItem; +import com.yahoo.prelude.query.PhraseItem; +import com.yahoo.prelude.query.PhraseSegmentItem; +import com.yahoo.prelude.query.WordItem; import com.yahoo.search.query.QueryTree; import com.yahoo.search.query.parser.Parsable; import com.yahoo.search.query.parser.ParserEnvironment; -import java.util.*; +import java.util.List; +import java.util.ListIterator; /** * The Vespa query parser. @@ -20,6 +27,7 @@ import java.util.*; */ public abstract class AbstractParser implements CustomParser { + /** The current submodes of this parser */ protected Submodes submodes = new Submodes(); @@ -32,6 +40,8 @@ public abstract class AbstractParser implements CustomParser { /** The IndexFacts.Session of this query */ protected IndexFacts.Session indexFacts; + protected String defaultIndex; + /** * The counter for braces in URLs, braces in URLs are accepted so long as * they are balanced. @@ -125,41 +135,38 @@ public abstract class AbstractParser implements CustomParser { @Override public final Item parse(String queryToParse, String filterToParse, Language parsingLanguage, - IndexFacts.Session indexFacts, String defaultIndexName) { - return parse(queryToParse, filterToParse, parsingLanguage, indexFacts, defaultIndexName, null); + IndexFacts.Session indexFacts, String defaultIndex) { + return parse(queryToParse, filterToParse, parsingLanguage, indexFacts, defaultIndex, null); } private Item parse(String queryToParse, String filterToParse, Language parsingLanguage, - IndexFacts.Session indexFacts, String defaultIndexName, Parsable parsable) { + IndexFacts.Session indexFacts, String defaultIndex, Parsable parsable) { if (queryToParse == null) return null; - if (defaultIndexName != null) - defaultIndexName = indexFacts.getCanonicName(defaultIndexName); + if (defaultIndex != null) + defaultIndex = indexFacts.getCanonicName(defaultIndex); - tokenize(queryToParse, defaultIndexName, indexFacts, parsingLanguage); + tokenize(queryToParse, defaultIndex, indexFacts, parsingLanguage); if (parsingLanguage == null && parsable != null) { - String detectionText = generateLanguageDetectionTextFrom(tokens, indexFacts, defaultIndexName); + String detectionText = generateLanguageDetectionTextFrom(tokens, indexFacts, defaultIndex); if (detectionText.isEmpty()) // heuristic detection text extraction is fallible detectionText = queryToParse; parsingLanguage = parsable.getOrDetectLanguage(detectionText); } - setState(parsingLanguage, indexFacts); - Item root = parseItems(defaultIndexName); + setState(parsingLanguage, indexFacts, defaultIndex); + Item root = parseItems(); + if (filterToParse != null) { AnyParser filterParser = new AnyParser(environment); if (root == null) { - root = filterParser.parseFilter(filterToParse, parsingLanguage, indexFacts); + root = filterParser.parseFilter(filterToParse, parsingLanguage, indexFacts, defaultIndex); } else { - root = filterParser.applyFilter(root, filterToParse, parsingLanguage, indexFacts); + root = filterParser.applyFilter(root, filterToParse, parsingLanguage, indexFacts, defaultIndex); } } - root = simplifyPhrases(root); - if (defaultIndexName != null) { - assignDefaultIndex(indexFacts.getCanonicName(defaultIndexName), root); - } - return root; + return simplifyPhrases(root); } /** @@ -226,30 +233,7 @@ public abstract class AbstractParser implements CustomParser { return kind.equals(tokenOrNull.kind); } - protected abstract Item parseItems(String defaultIndexName); - - /** - * Assigns the default index to query terms having no default index. The - * parser _should_ have done this, for some reason it doesn't. - * - * @param defaultIndex the default index to assign - * @param item the item to check - */ - private static void assignDefaultIndex(String defaultIndex, Item item) { - if (defaultIndex == null || item == null) return; - - if (item instanceof IndexedItem) { - IndexedItem indexName = (IndexedItem) item; - - if ("".equals(indexName.getIndexName())) - indexName.setIndexName(defaultIndex); - } - else if (item instanceof CompositeItem) { - Iterator<Item> items = ((CompositeItem)item).getItemIterator(); - while (items.hasNext()) - assignDefaultIndex(defaultIndex, items.next()); - } - } + protected abstract Item parseItems(); /** * Unicode normalizes some piece of natural language text. The chosen form @@ -261,10 +245,11 @@ public abstract class AbstractParser implements CustomParser { return environment.getLinguistics().getNormalizer().normalize(input); } - protected void setState(Language queryLanguage, IndexFacts.Session indexFacts) { + protected void setState(Language queryLanguage, IndexFacts.Session indexFacts, String defaultIndex) { this.indexFacts = indexFacts; - language = queryLanguage; - submodes.reset(); + this.defaultIndex = defaultIndex; + this.language = queryLanguage; + this.submodes.reset(); } /** @@ -293,8 +278,7 @@ public abstract class AbstractParser implements CustomParser { return unwashed; } else if (unwashed instanceof PhraseItem) { return collapsePhrase((PhraseItem) unwashed); - } else if (unwashed instanceof CompositeItem) { - CompositeItem composite = (CompositeItem) unwashed; + } else if (unwashed instanceof CompositeItem composite) { ListIterator<Item> i = composite.getItemIterator(); while (i.hasNext()) { @@ -312,9 +296,8 @@ public abstract class AbstractParser implements CustomParser { } private static Item collapsePhrase(PhraseItem phrase) { - if (phrase.getItemCount() == 1 && phrase.getItem(0) instanceof WordItem) { + if (phrase.getItemCount() == 1 && phrase.getItem(0) instanceof WordItem word) { // TODO: Other stuff which needs propagation? - WordItem word = (WordItem) phrase.getItem(0); word.setWeight(phrase.getWeight()); return word; } else { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java index 3358075d670..8f98763a838 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java @@ -1,14 +1,25 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query.parser; -import com.yahoo.prelude.query.*; +import com.yahoo.prelude.query.AndItem; +import com.yahoo.prelude.query.CompositeItem; +import com.yahoo.prelude.query.EquivItem; +import com.yahoo.prelude.query.Item; +import com.yahoo.prelude.query.NearItem; +import com.yahoo.prelude.query.NotItem; +import com.yahoo.prelude.query.ONearItem; +import com.yahoo.prelude.query.OrItem; +import com.yahoo.prelude.query.RankItem; +import com.yahoo.prelude.query.SegmentItem; +import com.yahoo.prelude.query.WeakAndItem; +import com.yahoo.prelude.query.WordItem; import com.yahoo.search.query.parser.ParserEnvironment; import static com.yahoo.prelude.query.parser.Token.Kind.LBRACE; import static com.yahoo.prelude.query.parser.Token.Kind.NUMBER; /** - * Parser for queries of type advanced. + * Parser for queries of type 'advanced'. * * @author Steinar Knutsen * @deprecated YQL should be used for formal queries @@ -20,7 +31,8 @@ public class AdvancedParser extends StructuredParser { super(environment); } - protected Item parseItems(String defaultIndexName) { + @Override + protected Item parseItems() { return advancedItems(true); } @@ -53,46 +65,40 @@ public class AdvancedParser extends StructuredParser { boolean expectingOperator = false; do { - item = null; - + item = indexableItem().getFirst(); if (item == null) { - item = indexableItem(); - if (item == null) { - item = compositeItem(); - itemIsComposite = true; - } else { - itemIsComposite = false; - } - if (item != null) { - Item newTop = null; + item = compositeItem(); + itemIsComposite = true; + } else { + itemIsComposite = false; + } + if (item != null) { + Item newTop = null; - if (expectingOperator) { - newTop = handleAdvancedOperator(topLevelItem, item, - topLevelIsClosed); - } - if (newTop != null) { // Operator found - topLevelIsClosed = false; - expectingOperator = false; - topLevelItem = newTop; - } else if (topLevelItem == null) { - topLevelItem = item; - if (itemIsComposite) { - topLevelIsClosed = true; - } - expectingOperator = true; - } else if (topLevelItem instanceof CompositeItem - && !(topLevelItem instanceof SegmentItem)) { - ((CompositeItem) topLevelItem).addItem(item); - expectingOperator = true; - } else { - AndItem and = new AndItem(); - - and.addItem(topLevelItem); - and.addItem(item); - topLevelItem = and; - topLevelIsClosed = false; - expectingOperator = true; + if (expectingOperator) { + newTop = handleAdvancedOperator(topLevelItem, item, topLevelIsClosed); + } + if (newTop != null) { // Operator found + topLevelIsClosed = false; + expectingOperator = false; + topLevelItem = newTop; + } else if (topLevelItem == null) { + topLevelItem = item; + if (itemIsComposite) { + topLevelIsClosed = true; } + expectingOperator = true; + } else if (topLevelItem instanceof CompositeItem && !(topLevelItem instanceof SegmentItem)) { + ((CompositeItem) topLevelItem).addItem(item); + expectingOperator = true; + } else { + AndItem and = new AndItem(); + + and.addItem(topLevelItem); + and.addItem(item); + topLevelItem = and; + topLevelIsClosed = false; + expectingOperator = true; } } @@ -178,7 +184,7 @@ public class AdvancedParser extends StructuredParser { int distance = consumeNumericArgument(); if (distance==0) distance=NearItem.defaultDistance; - if (topLevelIsClosed || !(topLevelItem instanceof NearItem) || distance!=((NearItem)topLevelItem).getDistance()) { + if (topLevelIsClosed || !(topLevelItem instanceof NearItem) || distance != ((NearItem)topLevelItem).getDistance()) { NearItem near = new NearItem(distance); near.addItem(topLevelItem); @@ -188,7 +194,7 @@ public class AdvancedParser extends StructuredParser { } else if (isTheWord("onear", item)) { int distance = consumeNumericArgument(); if (distance==0) - distance=ONearItem.defaultDistance; + distance= ONearItem.defaultDistance; if (topLevelIsClosed || !(topLevelItem instanceof ONearItem) || distance!=((ONearItem)topLevelItem).getDistance()) { ONearItem oNear = new ONearItem(distance); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java index 09caa72ca59..9a60eaef76b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java @@ -41,16 +41,16 @@ public class AllParser extends SimpleParser { } @Override - protected Item parseItems(String defaultIndexName) { + protected Item parseItems() { int position = tokens.getPosition(); try { - return parseItemsBody(defaultIndexName); + return parseItemsBody(); } finally { tokens.setPosition(position); } } - protected Item parseItemsBody(String defaultIndexName) { + protected Item parseItemsBody() { // Algorithm: Collect positive, negative, and and'ed items, then combine. CompositeItem and = null; NotItem not = null; // Store negatives here as we go @@ -65,7 +65,7 @@ public class AllParser extends SimpleParser { current = positiveItem(); if (current == null) - current = indexableItem(defaultIndexName); + current = indexableItem().getFirst(); if (current == null) current = compositeItem(); @@ -129,8 +129,9 @@ public class AllParser extends SimpleParser { try { if ( ! tokens.skip(MINUS)) return null; if (tokens.currentIsNoIgnore(SPACE)) return null; - - item = indexableItem(); + var itemAndExplicitIndex = indexableItem(); + item = itemAndExplicitIndex.getFirst(); + boolean explicitIndex = itemAndExplicitIndex.getSecond(); if (item == null) { item = compositeItem(); @@ -155,11 +156,11 @@ public class AllParser extends SimpleParser { // but interpret -(N) as a negative item matching a positive number // but interpret --N as a negative item matching a negative number if (item instanceof IntItem && - ((IntItem)item).getIndexName().isEmpty() && + ! explicitIndex && ! isComposited && - ! ((IntItem)item).getNumber().startsWith(("-"))) + ! ((IntItem)item).getNumber().startsWith(("-"))) { item = null; - + } return item; } finally { if (item == null) { @@ -204,8 +205,7 @@ public class AllParser extends SimpleParser { rank.addItem(topLevelItem); } return rank; - } else if ((item instanceof RankItem) && (((RankItem)item).getItem(0) instanceof OrItem)) { - RankItem itemAsRank = (RankItem) item; + } else if ((item instanceof RankItem itemAsRank) && (((RankItem)item).getItem(0) instanceof OrItem)) { OrItem or = (OrItem) itemAsRank.getItem(0); ((RankItem) topLevelItem).addItem(0, or); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java index f4ff769ad05..bf778409364 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java @@ -14,9 +14,7 @@ import com.yahoo.prelude.query.RankItem; import com.yahoo.prelude.query.TermItem; import com.yahoo.search.query.parser.ParserEnvironment; -import java.util.Collections; import java.util.Iterator; -import java.util.Set; import static com.yahoo.prelude.query.parser.Token.Kind.*; @@ -31,12 +29,13 @@ public class AnyParser extends SimpleParser { super(environment); } - protected Item parseItems(String defaultIndexName) { - return anyItems(true, defaultIndexName); + @Override + protected Item parseItems() { + return anyItems(true); } - Item parseFilter(String filter, Language queryLanguage, IndexFacts.Session indexFacts) { - setState(queryLanguage, indexFacts); + Item parseFilter(String filter, Language queryLanguage, IndexFacts.Session indexFacts, String defaultIndex) { + setState(queryLanguage, indexFacts, defaultIndex); tokenize(filter, null, indexFacts, queryLanguage); Item filterRoot = anyItems(true); @@ -55,7 +54,7 @@ public class AnyParser extends SimpleParser { if ( ! tokens.skipMultiple(MINUS)) return null; if (tokens.currentIsNoIgnore(SPACE)) return null; - item = indexableItem(); + item = indexableItem().getFirst(); if (item == null) { item = compositeItem(); @@ -123,8 +122,8 @@ public class AnyParser extends SimpleParser { } } - Item applyFilter(Item root, String filter, Language queryLanguage, IndexFacts.Session indexFacts) { - setState(queryLanguage, indexFacts); + Item applyFilter(Item root, String filter, Language queryLanguage, IndexFacts.Session indexFacts, String defaultIndex) { + setState(queryLanguage, indexFacts, defaultIndex); tokenize(filter, null, indexFacts, queryLanguage); return filterItems(root); } @@ -148,16 +147,14 @@ public class AnyParser extends SimpleParser { private Item filterItems(Item root) { while (tokens.hasNext()) { - Item item = null; - - item = positiveItem(); + Item item = positiveItem(); root = addAndFilter(root, item); if (item == null) { item = negativeItem(); root = addNotFilter(root, item); } if (item == null) { - item = indexableItem(); + item = indexableItem().getFirst(); root = addRankFilter(root, item); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/CustomParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/CustomParser.java index e867def5903..e3b2278475b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/CustomParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/CustomParser.java @@ -7,7 +7,6 @@ import com.yahoo.prelude.query.Item; import com.yahoo.search.query.parser.Parser; import java.util.Collections; -import java.util.Objects; import java.util.Set; /** diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/ParseException.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/ParseException.java index bef2ca9ffe9..82515c51c05 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/ParseException.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/ParseException.java @@ -1,13 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query.parser; - /** * Parser exceptions. JavaCC legacy, never thrown. * - * @author bratseth + * @author bratseth */ -@SuppressWarnings("serial") public class ParseException extends RuntimeException { public ParseException(String message) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java index 72eb56dd0fb..01b5b943829 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java @@ -16,7 +16,8 @@ public class PhraseParser extends AbstractParser { super(environment); } - protected Item parseItems(String defaultIndex) { + @Override + protected Item parseItems() { return forcedPhrase(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/ProgrammaticParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/ProgrammaticParser.java index 6a005bc0ec9..209753a596c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/ProgrammaticParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/ProgrammaticParser.java @@ -9,8 +9,6 @@ import com.yahoo.search.query.QueryTree; import com.yahoo.search.query.parser.Parsable; import com.yahoo.search.query.textserialize.TextSerialize; -import java.util.Set; - /** * @author Simon Thoresen Hult */ @@ -32,4 +30,5 @@ public final class ProgrammaticParser implements CustomParser { if (queryToParse == null) return null; return TextSerialize.parse(queryToParse); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java index fafbf55a522..b7355c43f81 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java @@ -1,7 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query.parser; -import com.yahoo.prelude.query.*; +import com.yahoo.prelude.query.AndItem; +import com.yahoo.prelude.query.BlockItem; +import com.yahoo.prelude.query.CompositeItem; +import com.yahoo.prelude.query.Item; +import com.yahoo.prelude.query.NotItem; +import com.yahoo.prelude.query.OrItem; +import com.yahoo.prelude.query.PhraseItem; +import com.yahoo.prelude.query.RankItem; +import com.yahoo.prelude.query.TermItem; +import com.yahoo.prelude.query.TrueItem; import com.yahoo.search.query.parser.ParserEnvironment; import java.util.Iterator; @@ -33,12 +42,12 @@ abstract class SimpleParser extends StructuredParser { * If there's a explicit composite and some other terms, * a rank terms combines them */ - protected Item anyItems(boolean topLevel, String defaultIndexName) { + protected Item anyItems(boolean topLevel) { int position = tokens.getPosition(); Item item = null; try { - item = anyItemsBody(topLevel, defaultIndexName); + item = anyItemsBody(topLevel); return item; } finally { if (item == null) { @@ -47,14 +56,10 @@ abstract class SimpleParser extends StructuredParser { } } - protected Item anyItems(boolean topLevel) { - return anyItems(topLevel, null); - } - - private Item anyItemsBody(boolean topLevel, String defaultIndexName) { + private Item anyItemsBody(boolean topLevel) { Item topLevelItem = null; NotItem not = null; - Item item = null; + Item item; do { item = positiveItem(); if (item != null) { @@ -92,7 +97,7 @@ abstract class SimpleParser extends StructuredParser { } if (item == null) { - item = indexableItem(defaultIndexName); + item = indexableItem().getFirst(); if (item != null) { if (topLevelItem == null) { topLevelItem = item; @@ -177,9 +182,7 @@ abstract class SimpleParser extends StructuredParser { return null; } - if (item == null) { - item = indexableItem(); - } + item = indexableItem().getFirst(); if (item == null) { item = compositeItem(); @@ -200,12 +203,10 @@ abstract class SimpleParser extends StructuredParser { * (+ items) are not found, but negatives are. */ private Item getItemAsPositiveItem(Item item, NotItem not) { - if (!(item instanceof RankItem)) { + if (!(item instanceof RankItem rank)) { return item; } - RankItem rank = (RankItem) item; - // Remove the not from the rank item, the rank should generally // be the first, but this is not always the case int limit = rank.getItemCount(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java index c668cf66447..88490237fc7 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java @@ -1,8 +1,26 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query.parser; +import com.yahoo.collections.Pair; import com.yahoo.prelude.IndexFacts; -import com.yahoo.prelude.query.*; +import com.yahoo.prelude.query.AndItem; +import com.yahoo.prelude.query.AndSegmentItem; +import com.yahoo.prelude.query.BlockItem; +import com.yahoo.prelude.query.CompositeItem; +import com.yahoo.prelude.query.IntItem; +import com.yahoo.prelude.query.Item; +import com.yahoo.prelude.query.MarkerWordItem; +import com.yahoo.prelude.query.PhraseItem; +import com.yahoo.prelude.query.PhraseSegmentItem; +import com.yahoo.prelude.query.PrefixItem; +import com.yahoo.prelude.query.SegmentItem; +import com.yahoo.prelude.query.Substring; +import com.yahoo.prelude.query.SubstringItem; +import com.yahoo.prelude.query.SuffixItem; +import com.yahoo.prelude.query.TaggableItem; +import com.yahoo.prelude.query.TermItem; +import com.yahoo.prelude.query.UriItem; +import com.yahoo.prelude.query.WordItem; import com.yahoo.search.query.parser.ParserEnvironment; import java.util.ArrayList; @@ -52,19 +70,22 @@ abstract class StructuredParser extends AbstractParser { submodes.setFromIndex(indexName, indexFacts); } - protected Item indexableItem() { - return indexableItem(null); - } - - protected Item indexableItem(String defaultIndexName) { + /** + * Returns an item and whether it had an explicit index ('indexname:' prefix). + * + * @return an item and whether it has an explicit index, or a Pair with the first element null if none + */ + protected Pair<Item, Boolean> indexableItem() { int position = tokens.getPosition(); Item item = null; try { + boolean explicitIndex = false; String indexName = indexPrefix(); - if (Objects.isNull(indexName)) { - indexName = defaultIndexName; - } + if (indexName != null) + explicitIndex = true; + else + indexName = this.defaultIndex; setSubmodeFromIndex(indexName, indexFacts); item = number(); @@ -86,7 +107,6 @@ abstract class StructuredParser extends AbstractParser { if (item != null) { weight = weightSuffix(); } - if (indexName != null && item != null) { item.setIndexName(indexName); } @@ -95,7 +115,7 @@ abstract class StructuredParser extends AbstractParser { item.setWeight(weight); } - return item; + return new Pair<>(item, explicitIndex); } finally { if (item == null) { tokens.setPosition(position); @@ -109,8 +129,7 @@ abstract class StructuredParser extends AbstractParser { if (tokens.currentIsNoIgnore(SPACE)) { return false; } - if (tokens.currentIsNoIgnore(NUMBER) - || tokens.currentIsNoIgnore(WORD)) { + if (tokens.currentIsNoIgnore(NUMBER) || tokens.currentIsNoIgnore(WORD)) { return true; } tokens.skipNoIgnore(); @@ -286,7 +305,6 @@ abstract class StructuredParser extends AbstractParser { tokens.skip(LSQUAREBRACKET); if (item == null) tokens.skipNoIgnore(SPACE); - // TODO: Better definition of start and end of numeric items if (item == null && tokens.currentIsNoIgnore(MINUS) && (tokens.currentNoIgnore(1).kind == NUMBER)) { tokens.skipNoIgnore(); @@ -592,7 +610,7 @@ abstract class StructuredParser extends AbstractParser { if (firstWord instanceof IntItem) { IntItem asInt = (IntItem) firstWord; firstWord = new WordItem(asInt.stringValue(), asInt.getIndexName(), - true, asInt.getOrigin()); + true, asInt.getOrigin()); } composite.addItem(firstWord); composite.addItem(word); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/Token.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/Token.java index b668df9208c..3bf4d9dcf01 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/Token.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/Token.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query.parser; - import com.yahoo.prelude.query.Substring; /** @@ -11,7 +10,7 @@ import com.yahoo.prelude.query.Substring; */ public class Token { - public static enum Kind { + public enum Kind { EOF("<EOF>"), NUMBER("<NUMBER>"), WORD("<WORD>"), @@ -77,31 +76,6 @@ public class Token { /** Returns whether this is a <i>special token</i> */ public boolean isSpecial() { return special; } - public String toString() { return image; } - - public boolean equals(Object object) { - if (this == object) { - return true; - } - if (object == null) { - return false; - } - if (object.getClass() != this.getClass()) { - return false; - } - - Token other = (Token) object; - - if (this.kind != other.kind) { - return false; - } - if (!(this.image.equals(other.image))) { - return false; - } - - return true; - } - /** * Returns the substring containing the image ins original form (including casing), * as well as all the text surrounding the token @@ -110,6 +84,22 @@ public class Token { */ public Substring getSubstring() { return substring; } + @Override + public String toString() { return image; } + + @Override + public boolean equals(Object object) { + if (this == object) return true; + if (object == null) return false; + if (object.getClass() != this.getClass()) return false; + + Token other = (Token) object; + if (this.kind != other.kind) return false; + if (!(this.image.equals(other.image))) return false; + return true; + } + + @Override public int hashCode() { return image.hashCode() ^ kind.hashCode(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenPosition.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenPosition.java index 9c60abab637..5ead962e430 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenPosition.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenPosition.java @@ -37,9 +37,7 @@ final class TokenPosition { * Returns null (no exception) if there are no more tokens. */ public Token current() { - Token token = current(0); - - return token; + return current(0); } /** diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java index eefbe5fa0d0..dbbc321d057 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/TokenizeParser.java @@ -22,7 +22,7 @@ public final class TokenizeParser extends AbstractParser { } @Override - protected Item parseItems(String defaultIndex) { + protected Item parseItems() { WeakAndItem weakAnd = new WeakAndItem(); Token token; while (null != (token = tokens.next())) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java index 93b8cf1ed83..c1d415b8e27 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java @@ -84,7 +84,6 @@ public final class Tokenizer { * @param indexFacts information about the indexes we will search * @return a read-only list of tokens. This list can only be used by this thread */ - @SuppressWarnings({"deprecation"}) // To avoid this we need to pass an IndexFacts.session down instead - easily done but not without breaking API's public List<Token> tokenize(String string, String defaultIndexName, IndexFacts.Session indexFacts) { this.source = string; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/UnicodePropertyDump.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/UnicodePropertyDump.java index b01b1295f45..8d2adfe0d78 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/UnicodePropertyDump.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/UnicodePropertyDump.java @@ -26,13 +26,13 @@ class UnicodePropertyDump { boolean debug = false; if (arg.length > 0) { - start = Integer.valueOf(arg[0]).intValue(); + start = Integer.parseInt(arg[0]); } if (arg.length > 1) { - end = Integer.valueOf(arg[1]).intValue(); + end = Integer.parseInt(arg[1]); } if (arg.length > 2) { - debug = Boolean.valueOf(arg[2]).booleanValue(); + debug = Boolean.parseBoolean(arg[2]); } dumpProperties(start, end, debug, System.out); } @@ -109,4 +109,5 @@ class UnicodePropertyDump { out.println(); } } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java index 40497d94a6d..aff28179050 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/WebParser.java @@ -28,7 +28,7 @@ public class WebParser extends AllParser { } @Override - protected Item parseItemsBody(String defaultIndexName) { + protected Item parseItemsBody() { // Algorithm: Collect positive, negative, and'ed and or'ed elements, then combine. CompositeItem and = null; OrItem or = null; @@ -45,7 +45,7 @@ public class WebParser extends AllParser { current = positiveItem(); if (current == null) - current = indexableItem(defaultIndexName); + current = indexableItem().getFirst(); if (current != null) { if (and != null && (current instanceof WordItem) && "OR".equals(((WordItem)current).getRawWord())) { diff --git a/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java index 95ecf3c2dba..06b6eca5f84 100644 --- a/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java @@ -195,7 +195,7 @@ public class QueryTestCase { assertTrue(p.hashCode() != q.hashCode()); } - /** Test using the defauultindex feature */ + /** Test using the defaultindex feature */ @Test void testDefaultIndex() { Query q = newQuery("?query=hi hello keyword:kanoo " + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index ac7c6319c1b..a340982bec0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore; +import com.yahoo.vespa.hosted.controller.api.role.SimplePrincipal; import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; @@ -154,8 +155,11 @@ public abstract class LockedTenant { public Cloud withDeveloperKey(PublicKey key, Principal principal) { BiMap<PublicKey, Principal> keys = HashBiMap.create(developerKeys); + principal = new SimplePrincipal(principal.getName()); if (keys.containsKey(key)) throw new IllegalArgumentException("Key " + KeyUtils.toPem(key) + " is already owned by " + keys.get(key)); + if (keys.inverse().containsKey(principal)) + throw new IllegalArgumentException(principal + " is already associated with key " + KeyUtils.toPem(keys.inverse().get(principal))); keys.put(key, principal); return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java index 324c9706df9..a927439de1c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java @@ -80,7 +80,7 @@ public class ControllerContainerCloudTest extends ControllerContainerTest { } public RequestBuilder data(byte[] data) { this.data = data; return this; } public RequestBuilder data(String data) { this.data = data.getBytes(StandardCharsets.UTF_8); return this; } - public RequestBuilder principal(String principal) { this.principal = new SimplePrincipal(principal); return this; } + public RequestBuilder principal(String principal) { this.principal = new SimplePrincipal(principal){ }; return this; } public RequestBuilder user(User user) { this.user = user; return this; } public RequestBuilder roles(Set<Role> roles) { this.roles = roles; return this; } public RequestBuilder roles(Role... roles) { return roles(Set.of(roles)); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index 1344b106bbe..f34dd3fe629 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -136,6 +136,14 @@ public class UserApiTest extends ControllerContainerCloudTest { "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Key " + quotedPemPublicKey + " is already owned by joe@dev\"}", 400); + // POST a different developer key for an existing user is forbidden + tester.assertResponse(request("/application/v4/tenant/my-tenant/key", POST) + .principal("joe@dev") + .roles(Set.of(Role.developer(id.tenant()))) + .data("{\"key\":\"" + otherPemPublicKey + "\"}"), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"joe@dev is already associated with key " + quotedPemPublicKey + "\"}", + 400); + // POST in a different pem developer key tester.assertResponse(request("/application/v4/tenant/my-tenant/key", POST) .principal("developer@tenant") diff --git a/default_build_settings.cmake b/default_build_settings.cmake index 009cd6d615e..979b71f96b3 100644 --- a/default_build_settings.cmake +++ b/default_build_settings.cmake @@ -64,7 +64,7 @@ endfunction() function(setup_vespa_default_build_settings_darwin) message("-- Setting up default build settings for darwin") - set(DEFAULT_VESPA_LLVM_VERSION "13" PARENT_SCOPE) + set(DEFAULT_VESPA_LLVM_VERSION "14" PARENT_SCOPE) set(DEFAULT_CMAKE_PREFIX_PATH "${VESPA_DEPS}" "/usr/local/opt/bison" "/usr/local/opt/flex" "/usr/local/opt/openssl@1.1" "/usr/local/opt/openblas" "/usr/local/opt/icu4c" PARENT_SCOPE) set(DEFAULT_EXTRA_LINK_DIRECTORY "${VESPA_DEPS}/lib" "/usr/local/opt/bison/lib" "/usr/local/opt/flex/lib" "/usr/local/opt/icu4c/lib" "/usr/local/opt/openssl@1.1/lib" "/usr/local/opt/openblas/lib") list(APPEND DEFAULT_EXTRA_LINK_DIRECTORY "/usr/local/lib") @@ -351,8 +351,8 @@ function(vespa_use_default_cxx_compiler) unset(DEFAULT_CMAKE_CXX_COMPILER) if(NOT DEFINED VESPA_COMPILER_VARIANT OR VESPA_COMPILER_VARIANT STREQUAL "gcc") if(APPLE) - set(DEFAULT_CMAKE_C_COMPILER "/usr/local/bin/gcc-11") - set(DEFAULT_CMAKE_CXX_COMPILER "/usr/local/bin/g++-11") + set(DEFAULT_CMAKE_C_COMPILER "/usr/local/bin/gcc-12") + set(DEFAULT_CMAKE_CXX_COMPILER "/usr/local/bin/g++-12") elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "amzn 2") set(DEFAULT_CMAKE_C_COMPILER "/usr/bin/gcc10-gcc") set(DEFAULT_CMAKE_CXX_COMPILER "/usr/bin/gcc10-g++") @@ -403,12 +403,12 @@ function(vespa_use_default_java_home) if (DEFINED JAVA_HOME) return() endif() - set(DEFAULT_JAVA_HOME "/usr/lib/jvm/java-11-openjdk") + set(DEFAULT_JAVA_HOME "/usr/lib/jvm/java-17-openjdk") if(APPLE) execute_process(COMMAND "/usr/libexec/java_home" OUTPUT_VARIABLE DEFAULT_JAVA_HOME) string(STRIP "${DEFAULT_JAVA_HOME}" DEFAULT_JAVA_HOME) - elseif(VESPA_OS_DISTRO STREQUAL "ubuntu") - set(DEFAULT_JAVA_HOME "/usr/lib/jvm/java-11-openjdk-amd64" PARENT_SCOPE) + elseif(VESPA_OS_DISTRO STREQUAL "ubuntu" OR VESPA_OS_DISTRO STREQUAL "debian") + set(DEFAULT_JAVA_HOME "/usr/lib/jvm/java-17-openjdk-amd64" PARENT_SCOPE) endif() if(COMMAND vespa_use_specific_java_home) vespa_use_specific_java_home() diff --git a/dist/vespa.spec b/dist/vespa.spec index b94a77491b8..b05a8853da1 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -111,7 +111,7 @@ BuildRequires: vespa-gtest = 1.11.0 %define _use_vespa_gtest 1 BuildRequires: vespa-icu-devel >= 65.1.0-1 BuildRequires: vespa-lz4-devel >= 1.9.2-2 -BuildRequires: vespa-onnxruntime-devel = 1.11.0 +BuildRequires: vespa-onnxruntime-devel = 1.12.1 BuildRequires: vespa-openssl-devel >= 1.1.1o-1 %define _use_vespa_openssl 1 BuildRequires: vespa-protobuf-devel = 3.19.1 @@ -139,7 +139,7 @@ BuildRequires: vespa-openssl-devel >= 1.1.1o-1 BuildRequires: vespa-gtest = 1.11.0 %define _use_vespa_gtest 1 BuildRequires: vespa-lz4-devel >= 1.9.2-2 -BuildRequires: vespa-onnxruntime-devel = 1.11.0 +BuildRequires: vespa-onnxruntime-devel = 1.12.1 BuildRequires: vespa-protobuf-devel = 3.19.1 BuildRequires: vespa-libzstd-devel >= 1.4.5-2 %endif @@ -149,7 +149,7 @@ BuildRequires: maven BuildRequires: maven-openjdk17 BuildRequires: openssl-devel BuildRequires: vespa-lz4-devel >= 1.9.2-2 -BuildRequires: vespa-onnxruntime-devel = 1.11.0 +BuildRequires: vespa-onnxruntime-devel = 1.12.1 BuildRequires: vespa-libzstd-devel >= 1.4.5-2 BuildRequires: protobuf-devel %if 0%{?_centos_stream} @@ -169,7 +169,7 @@ BuildRequires: maven-openjdk17 %endif BuildRequires: openssl-devel BuildRequires: vespa-lz4-devel >= 1.9.2-2 -BuildRequires: vespa-onnxruntime-devel = 1.11.0 +BuildRequires: vespa-onnxruntime-devel = 1.12.1 BuildRequires: vespa-libzstd-devel >= 1.4.5-2 %if 0%{?fc34} BuildRequires: protobuf-devel @@ -476,7 +476,7 @@ Requires: llvm-libs >= 14.0.0 Requires: llvm-libs >= 14.0.0 %endif %endif -Requires: vespa-onnxruntime = 1.11.0 +Requires: vespa-onnxruntime = 1.12.1 %description libs diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsLogic.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsLogic.java index 3f6801eebe7..1ff76fd45ac 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsLogic.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsLogic.java @@ -41,6 +41,6 @@ class CorsLogic { } private static boolean requestOriginMatchesAnyAllowed(String requestOrigin, Set<String> allowedUrls) { - return allowedUrls.stream().anyMatch(requestOrigin::startsWith) || allowedUrls.contains("*"); + return allowedUrls.stream().anyMatch(requestOrigin::equals) || allowedUrls.contains("*"); } } diff --git a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsPreflightRequestFilterTest.java b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsPreflightRequestFilterTest.java index b5b94d5a2c2..7ba050b7cc0 100644 --- a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsPreflightRequestFilterTest.java +++ b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cors/CorsPreflightRequestFilterTest.java @@ -43,6 +43,14 @@ public class CorsPreflightRequestFilterTest { } @Test + void extended_request_origin_does_not_yield_allow_origin_header_in_response() { + final String ALLOWED_ORIGIN = "https://allowed.origin"; + final String EXTENDED_ORIGIN = "https://allowed.origin.as.subdomain.com"; + HeaderFields headers = doFilterRequest(newRequestFilter(ALLOWED_ORIGIN), EXTENDED_ORIGIN); + assertNull(headers.getFirst(ALLOW_ORIGIN_HEADER)); + } + + @Test void allowed_wildcard_origin_yields_origin_header_in_response() { final String ALLOWED_ORIGIN = "http://allowed.origin"; HeaderFields headers = doFilterRequest(newRequestFilter("*"), ALLOWED_ORIGIN); diff --git a/screwdriver.yaml b/screwdriver.yaml index 0ffc3316f95..21ab6625eb4 100644 --- a/screwdriver.yaml +++ b/screwdriver.yaml @@ -283,6 +283,17 @@ jobs: exit 1 fi + verify-opensource-rpm-installable: + image: quay.io/centos/centos:stream8 + annotations: + screwdriver.cd/buildPeriodically: H 0 * * * + steps: + - install: | + dnf config-manager --add-repo https://raw.githubusercontent.com/vespa-engine/vespa/master/dist/vespa-engine.repo + dnf config-manager --enable powertools + dnf install -y epel-release + dnf install -y vespa + mirror-copr-rpms-to-artifactory: annotations: screwdriver.cd/cpu: LOW diff --git a/screwdriver/release-container-image.sh b/screwdriver/release-container-image.sh index 3ae2a7feb1e..def2f037b63 100755 --- a/screwdriver/release-container-image.sh +++ b/screwdriver/release-container-image.sh @@ -26,27 +26,35 @@ cd $BUILD_DIR git clone --depth 1 https://github.com/vespa-engine/docker-image cd docker-image -docker build --build-arg VESPA_VERSION=$VESPA_VERSION --file Dockerfile \ - --tag docker.io/vespaengine/vespa:$VESPA_VERSION --tag docker.io/vespaengine/vespa:latest \ - --tag ghcr.io/vespa-engine/vespa:$VESPA_VERSION --tag ghcr.io/vespa-engine/vespa:latest . +docker info +docker version +docker buildx version +docker buildx install + +unset DOCKER_HOST +docker context create vespa-context --docker "host=tcp://localhost:2376,ca=/certs/client/ca.pem,cert=/certs/client/cert.pem,key=/certs/client/key.pem" +docker context use vespa-context + +docker buildx create --name vespa-builder --driver docker-container --use +docker buildx inspect --bootstrap # Push to Docker Hub -docker login --username aressem --password "$DOCKER_HUB_DEPLOY_KEY" if curl -fsSL https://index.docker.io/v1/repositories/vespaengine/vespa/tags/$VESPA_VERSION &> /dev/null; then echo "Container image docker.io/vespaengine/vespa:$VESPA_VERSION aldready exists." else - docker push docker.io/vespaengine/vespa:$VESPA_VERSION - docker push docker.io/vespaengine/vespa:latest + docker login --username aressem --password "$DOCKER_HUB_DEPLOY_KEY" + docker buildx build --progress plain --push --platform linux/amd64,linux/arm64 --build-arg VESPA_VERSION=$VESPA_VERSION \ + --tag docker.io/vespaengine/vespa:$VESPA_VERSION --tag docker.io/vespaengine/vespa:latest . fi # Push to GitHub Container Registry -docker login --username aressem --password "$GHCR_DEPLOY_KEY" ghcr.io JWT=$(curl -sSL -u aressem:$GHCR_DEPLOY_KEY "https://ghcr.io/token?service=ghcr.io&scope=repository:vespa-engine/vespa:pull" | jq -re '.token') IMAGE_TAGS=$(curl -sSL -H "Authorization: Bearer $JWT" https://ghcr.io/v2/vespa-engine/vespa/tags/list | jq -re '.tags[]') if grep $VESPA_VERSION <<< "$IMAGE_TAGS" &> /dev/null; then echo "Container image ghcr.io/vespa-engine/vespa:$VESPA_VERSION aldready exists." else - docker push ghcr.io/vespa-engine/vespa:$VESPA_VERSION - docker push ghcr.io/vespa-engine/vespa:latest + docker login --username aressem --password "$GHCR_DEPLOY_KEY" ghcr.io + docker buildx build --progress plain --push --platform linux/amd64,linux/arm64 --build-arg VESPA_VERSION=$VESPA_VERSION \ + --tag ghcr.io/vespa-engine/vespa:$VESPA_VERSION --tag ghcr.io/vespa-engine/vespa:latest . fi diff --git a/searchcore/src/apps/verify_ranksetup/CMakeLists.txt b/searchcore/src/apps/verify_ranksetup/CMakeLists.txt index cc8434ef46f..536392739d9 100644 --- a/searchcore/src/apps/verify_ranksetup/CMakeLists.txt +++ b/searchcore/src/apps/verify_ranksetup/CMakeLists.txt @@ -1,12 +1,20 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_verify_ranksetup_app +vespa_add_library(searchcore_verify_ranksetup SOURCES verify_ranksetup.cpp - OUTPUT_NAME vespa-verify-ranksetup-bin - INSTALL bin + INSTALL lib64 DEPENDS searchcore_fconfig searchcore_matching searchcore_documentmetastore ) -vespa_generate_config(searchcore_verify_ranksetup_app verify-ranksetup.def) +vespa_generate_config(searchcore_verify_ranksetup verify-ranksetup.def) + +vespa_add_executable(searchcore_verify_ranksetup_app + SOURCES + verify_ranksetup_app.cpp + OUTPUT_NAME vespa-verify-ranksetup-bin + INSTALL bin + DEPENDS + searchcore_verify_ranksetup +) diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp index e72771f5aa6..dcfbc34653d 100644 --- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp @@ -1,5 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include "verify_ranksetup.h" #include "config-verify-ranksetup.h" #include <vespa/config-attributes.h> #include <vespa/config-indexschema.h> @@ -21,12 +22,10 @@ #include <vespa/searchlib/fef/fef.h> #include <vespa/searchlib/fef/test/plugin/setup.h> #include <vespa/config/subscription/configsubscriber.hpp> -#include <vespa/vespalib/util/signalhandler.h> +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/stllike/asciistream.h> #include <optional> -#include <vespa/log/log.h> -LOG_SETUP("vespa-verify-ranksetup"); - using config::ConfigContext; using config::ConfigHandle; using config::ConfigRuntimeException; @@ -50,8 +49,10 @@ using vespalib::eval::SimpleConstantValue; using vespalib::eval::TensorSpec; using vespalib::eval::Value; using vespalib::eval::ValueType; +using vespalib::make_string_short::fmt; -std::optional<vespalib::string> get_file(const vespalib::string &ref, const VerifyRanksetupConfig &myCfg) { +std::optional<vespalib::string> +get_file(const vespalib::string &ref, const VerifyRanksetupConfig &myCfg) { for (const auto &entry: myCfg.file) { if (ref == entry.ref) { return entry.path; @@ -60,37 +61,43 @@ std::optional<vespalib::string> get_file(const vespalib::string &ref, const Veri return std::nullopt; } -RankingExpressions make_expressions(const RankingExpressionsConfig &expressionsCfg, const VerifyRanksetupConfig &myCfg) { +RankingExpressions +make_expressions(const RankingExpressionsConfig &expressionsCfg, const VerifyRanksetupConfig &myCfg, + std::vector<search::fef::Message> & messages) { RankingExpressions expressions; for (const auto &entry: expressionsCfg.expression) { if (auto file = get_file(entry.fileref, myCfg)) { - LOG(debug, "Add expression %s with ref=%s and path=%s", entry.name.c_str(), entry.fileref.c_str(), file.value().c_str()); expressions.add(entry.name, file.value()); } else { - LOG(warning, "could not find file name for ranking expression '%s' (ref:'%s')", - entry.name.c_str(), entry.fileref.c_str()); + messages.emplace_back(search::fef::Level::WARNING, + fmt("could not find file name for ranking expression '%s' (ref:'%s')", + entry.name.c_str(), entry.fileref.c_str())); } } return expressions; } -OnnxModels make_models(const OnnxModelsConfig &modelsCfg, const VerifyRanksetupConfig &myCfg) { +OnnxModels +make_models(const OnnxModelsConfig &modelsCfg, const VerifyRanksetupConfig &myCfg, + std::vector<search::fef::Message> & messages) { OnnxModels::Vector model_list; for (const auto &entry: modelsCfg.model) { if (auto file = get_file(entry.fileref, myCfg)) { model_list.emplace_back(entry.name, file.value()); OnnxModels::configure(entry, model_list.back()); } else { - LOG(warning, "could not find file name for onnx model '%s' (ref:'%s')", - entry.name.c_str(), entry.fileref.c_str()); + messages.emplace_back(search::fef::Level::WARNING, + fmt("could not find file name for onnx model '%s' (ref:'%s')", + entry.name.c_str(), entry.fileref.c_str())); } } return OnnxModels(model_list); } -class App +class VerifyRankSetup { -public: +private: + std::vector<search::fef::Message> _messages; bool verify(const search::index::Schema &schema, const search::fef::Properties &props, const IConstantValueRepo &repo, @@ -105,30 +112,42 @@ public: const RankingExpressionsConfig &expressionsCfg, const OnnxModelsConfig &modelsCfg); - int usage(); - int main(int argc, char **argv); +public: + VerifyRankSetup(); + ~VerifyRankSetup(); + const std::vector<search::fef::Message> & getMessages() const { return _messages; } + bool verify(const std::string & configId); }; struct DummyConstantValueRepo : IConstantValueRepo { const RankingConstantsConfig &cfg; DummyConstantValueRepo(const RankingConstantsConfig &cfg_in) : cfg(cfg_in) {} - virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const override { - for (const auto &entry: cfg.constant) { - if (entry.name == name) { - try { - auto tensor = vespalib::eval::value_from_spec(TensorSpec(entry.type), FastValueBuilderFactory::get()); - return std::make_unique<SimpleConstantValue>(std::move(tensor)); - } catch (std::exception &) { - return std::make_unique<BadConstantValue>(); - } + vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const override; +}; + +vespalib::eval::ConstantValue::UP +DummyConstantValueRepo::getConstant(const vespalib::string &name) const { + for (const auto &entry: cfg.constant) { + if (entry.name == name) { + try { + auto tensor = vespalib::eval::value_from_spec(TensorSpec(entry.type), FastValueBuilderFactory::get()); + return std::make_unique<SimpleConstantValue>(std::move(tensor)); + } catch (std::exception &) { + return std::make_unique<BadConstantValue>(); } } - return vespalib::eval::ConstantValue::UP(nullptr); } -}; + return vespalib::eval::ConstantValue::UP(nullptr); +} + +VerifyRankSetup::VerifyRankSetup() + : _messages() +{ } + +VerifyRankSetup::~VerifyRankSetup() = default; bool -App::verify(const search::index::Schema &schema, +VerifyRankSetup::verify(const search::index::Schema &schema, const search::fef::Properties &props, const IConstantValueRepo &repo, const RankingExpressions &expressions, @@ -143,25 +162,25 @@ App::verify(const search::index::Schema &schema, rankSetup.configure(); // reads config values from the property map bool ok = true; if (!rankSetup.getFirstPhaseRank().empty()) { - ok = verifyFeature(factory, indexEnv, rankSetup.getFirstPhaseRank(), "first phase ranking") && ok; + ok = verifyFeature(factory, indexEnv, rankSetup.getFirstPhaseRank(), "first phase ranking", _messages) && ok; } if (!rankSetup.getSecondPhaseRank().empty()) { - ok = verifyFeature(factory, indexEnv, rankSetup.getSecondPhaseRank(), "second phase ranking") && ok; + ok = verifyFeature(factory, indexEnv, rankSetup.getSecondPhaseRank(), "second phase ranking", _messages) && ok; } for (size_t i = 0; i < rankSetup.getSummaryFeatures().size(); ++i) { - ok = verifyFeature(factory, indexEnv, rankSetup.getSummaryFeatures()[i], "summary features") && ok; + ok = verifyFeature(factory, indexEnv, rankSetup.getSummaryFeatures()[i], "summary features", _messages) && ok; } for (const auto & feature : rankSetup.get_match_features()) { - ok = verifyFeature(factory, indexEnv, feature, "match features") && ok; + ok = verifyFeature(factory, indexEnv, feature, "match features", _messages) && ok; } for (size_t i = 0; i < rankSetup.getDumpFeatures().size(); ++i) { - ok = verifyFeature(factory, indexEnv, rankSetup.getDumpFeatures()[i], "dump features") && ok; + ok = verifyFeature(factory, indexEnv, rankSetup.getDumpFeatures()[i], "dump features", _messages) && ok; } return ok; } bool -App::verifyConfig(const VerifyRanksetupConfig &myCfg, +VerifyRankSetup::verifyConfig(const VerifyRanksetupConfig &myCfg, const RankProfilesConfig &rankCfg, const IndexschemaConfig &schemaCfg, const AttributesConfig &attributeCfg, @@ -174,8 +193,8 @@ App::verifyConfig(const VerifyRanksetupConfig &myCfg, search::index::SchemaBuilder::build(schemaCfg, schema); search::index::SchemaBuilder::build(attributeCfg, schema); DummyConstantValueRepo repo(constantsCfg); - auto expressions = make_expressions(expressionsCfg, myCfg); - auto models = make_models(modelsCfg, myCfg); + auto expressions = make_expressions(expressionsCfg, myCfg, _messages); + auto models = make_models(modelsCfg, myCfg, _messages); for(size_t i = 0; i < rankCfg.rankprofile.size(); i++) { search::fef::Properties properties; const RankProfilesConfig::Rankprofile &profile = rankCfg.rankprofile[i]; @@ -184,33 +203,20 @@ App::verifyConfig(const VerifyRanksetupConfig &myCfg, profile.fef.property[j].value); } if (verify(schema, properties, repo, expressions, models)) { - LOG(info, "rank profile '%s': pass", profile.name.c_str()); + _messages.emplace_back(search::fef::Level::INFO, + fmt("rank profile '%s': pass", profile.name.c_str())); } else { - LOG(error, "rank profile '%s': FAIL", profile.name.c_str()); + _messages.emplace_back(search::fef::Level::ERROR, + fmt("rank profile '%s': FAIL", profile.name.c_str())); ok = false; } } return ok; } -int -App::usage() -{ - fprintf(stderr, "Usage: vespa-verify-ranksetup <config-id>\n"); - return 1; -} - -int -App::main(int argc, char **argv) +bool +VerifyRankSetup::verify(const std::string & configid) { - if (argc != 2) { - return usage(); - } - - const std::string configid = argv[1]; - LOG(debug, "verifying rank setup for config id '%s' ...", - configid.c_str()); - bool ok = false; try { auto ctx = std::make_shared<ConfigContext>(*config::legacyConfigId2Spec(configid)); @@ -233,18 +239,19 @@ App::main(int argc, char **argv) *expressionsHandle->getConfig(), *modelsHandle->getConfig()); } catch (ConfigRuntimeException & e) { - LOG(error, "Unable to subscribe to config: %s", e.getMessage().c_str()); + _messages.emplace_back(search::fef::Level::ERROR, + fmt("Unable to subscribe to config: %s", e.getMessage().c_str())); } catch (InvalidConfigException & e) { - LOG(error, "Error getting config: %s", e.getMessage().c_str()); - } - if (!ok) { - return 1; + _messages.emplace_back(search::fef::Level::ERROR, + fmt("Error getting config: %s", e.getMessage().c_str())); } - return 0; + return ok; } -int main(int argc, char **argv) { - vespalib::SignalHandler::PIPE.ignore(); - App app; - return app.main(argc, argv); +std::pair<bool, std::vector<search::fef::Message>> +verifyRankSetup(const char * configId) { + VerifyRankSetup verifier; + bool ok = verifier.verify(configId); + + return {ok, verifier.getMessages()}; } diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.h b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.h new file mode 100644 index 00000000000..d65dede8696 --- /dev/null +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.h @@ -0,0 +1,7 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/searchlib/fef/verify_feature.h> + +std::pair<bool, std::vector<search::fef::Message>> verifyRankSetup(const char * configId); diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup_app.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup_app.cpp new file mode 100644 index 00000000000..4f1bf1acbed --- /dev/null +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup_app.cpp @@ -0,0 +1,56 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "verify_ranksetup.h" +#include <vespa/vespalib/util/signalhandler.h> + +#include <vespa/log/log.h> +LOG_SETUP("vespa-verify-ranksetup"); + +class App +{ +public: + int usage(); + int main(int argc, char **argv); +}; + +int +App::usage() +{ + fprintf(stderr, "Usage: vespa-verify-ranksetup <config-id>\n"); + return 1; +} + +namespace { +ns_log::Logger::LogLevel +toLogLevel(search::fef::Level level) { + switch (level) { + case search::fef::Level::INFO: + return ns_log::Logger::LogLevel::info; + case search::fef::Level::WARNING: + return ns_log::Logger::LogLevel::warning; + case search::fef::Level::ERROR: + return ns_log::Logger::LogLevel::error; + } + abort(); +} +} +int +App::main(int argc, char **argv) +{ + if (argc != 2) { + return usage(); + } + + auto [ok, messages] = verifyRankSetup(argv[1]); + + for (const auto & msg : messages) { + VLOG(toLogLevel(msg.first), "%s", msg.second.c_str()); + } + return ok ? 0 : 1; +} + +int main(int argc, char **argv) { + vespalib::SignalHandler::PIPE.ignore(); + App app; + return app.main(argc, argv); +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 826860b743e..108397e6e5a 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -32,10 +32,12 @@ using search::MatchingElements; using search::attribute::IAttributeContext; using search::fef::MatchDataLayout; using search::fef::MatchData; +using search::fef::RankSetup; using search::fef::indexproperties::hitcollector::HeapSize; using search::queryeval::Blueprint; using search::queryeval::SearchIterator; using vespalib::Doom; +using vespalib::make_string_short::fmt; namespace proton::matching { @@ -117,7 +119,8 @@ Matcher::Matcher(const search::index::Schema &schema, const Properties &props, c _rankSetup = std::make_shared<search::fef::RankSetup>(_blueprintFactory, _indexEnv); _rankSetup->configure(); // reads config values from the property map if (!_rankSetup->compile()) { - throw vespalib::IllegalArgumentException("failed to compile rank setup", VESPA_STRLOC); + throw vespalib::IllegalArgumentException(fmt("failed to compile rank setup :\n%s", + _rankSetup->getJoinedWarnings().c_str()), VESPA_STRLOC); } } diff --git a/searchlib/src/tests/fef/object_passing/object_passing_test.cpp b/searchlib/src/tests/fef/object_passing/object_passing_test.cpp index 3639da05b9e..d7e5fd2600e 100644 --- a/searchlib/src/tests/fef/object_passing/object_passing_test.cpp +++ b/searchlib/src/tests/fef/object_passing/object_passing_test.cpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/vespalib/stllike/string.h> -#include <vespa/vespalib/util/stringfmt.h> #include <vespa/searchlib/features/valuefeature.h> #include <vespa/searchlib/fef/blueprintfactory.h> #include <vespa/searchlib/fef/test/indexenvironment.h> @@ -102,7 +101,8 @@ struct Fixture { } bool verify(const vespalib::string &feature) { - return verifyFeature(factory, indexEnv, feature, "unit test"); + std::vector<search::fef::Message> errors; + return verifyFeature(factory, indexEnv, feature, "unit test", errors); } }; diff --git a/searchlib/src/tests/ranksetup/verify_feature/verify_feature_test.cpp b/searchlib/src/tests/ranksetup/verify_feature/verify_feature_test.cpp index 24358db8f3a..6d49704e7c1 100644 --- a/searchlib/src/tests/ranksetup/verify_feature/verify_feature_test.cpp +++ b/searchlib/src/tests/ranksetup/verify_feature/verify_feature_test.cpp @@ -4,10 +4,37 @@ #include <vespa/searchlib/fef/test/indexenvironment.h> #include <vespa/searchlib/fef/test/plugin/setup.h> #include <vespa/searchlib/features/valuefeature.h> +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/stllike/asciistream.h> +#include <regex> using namespace search::features; using namespace search::fef::test; using namespace search::fef; +using vespalib::make_string_short::fmt; + +typedef bool (*cmp)(const vespalib::string & a, const vespalib::string &b); + +namespace search::fef { +std::ostream &operator<<(std::ostream &os, Level level) { + if (level == Level::INFO) { + return os << "info"; + } + if (level == Level::WARNING) { + return os << "warning"; + } + return os << "error"; +} +} + +bool equal(const vespalib::string & a, const vespalib::string &b) { + return a == b; +} + +bool regex(const vespalib::string & a, const vespalib::string &b) { + std::regex exp(b.c_str()); + return std::regex_match(a.c_str(), exp); +} struct RankFixture { BlueprintFactory factory; @@ -15,43 +42,89 @@ struct RankFixture { RankFixture() : factory(), indexEnv() { setup_fef_test_plugin(factory); - factory.addPrototype(Blueprint::SP(new ValueBlueprint())); + factory.addPrototype(std::make_shared<ValueBlueprint>()); } - bool verify(const std::string &feature) const { - return verifyFeature(factory, indexEnv, feature, "feature verification test"); + bool verify(const std::string &feature, std::vector<std::pair<cmp, Message>> expected) const { + std::vector<Message> errors; + bool ok = verifyFeature(factory, indexEnv, feature, "feature verification test", errors); + EXPECT_EQUAL(errors.size(), expected.size()); + for (size_t i(0); i < std::min(errors.size(), expected.size()); i++) { + EXPECT_EQUAL(errors[i].first, expected[i].second.first); + EXPECT_TRUE(expected[i].first(errors[i].second, expected[i].second.second)); + } + return ok; } }; TEST_F("verify valid rank feature", RankFixture) { - EXPECT_TRUE(f1.verify("value(1, 2, 3).0")); - EXPECT_TRUE(f1.verify("value(1, 2, 3).1")); - EXPECT_TRUE(f1.verify("value(1, 2, 3).2")); + EXPECT_TRUE(f1.verify("value(1, 2, 3).0", {})); + EXPECT_TRUE(f1.verify("value(1, 2, 3).1", {})); + EXPECT_TRUE(f1.verify("value(1, 2, 3).2", {})); } TEST_F("verify unknown feature", RankFixture) { - EXPECT_FALSE(f1.verify("unknown")); + EXPECT_FALSE(f1.verify("unknown", + {{equal, {Level::WARNING, "invalid rank feature 'unknown': unknown basename: 'unknown'"}}, + {equal, {Level::ERROR, "verification failed: rank feature 'unknown' (feature verification test)"}}})); } TEST_F("verify unknown output", RankFixture) { - EXPECT_FALSE(f1.verify("value(1, 2, 3).3")); + EXPECT_FALSE(f1.verify("value(1, 2, 3).3", + {{equal, {Level::WARNING, "invalid rank feature 'value(1,2,3).3': unknown output: '3'"}}, + {equal, {Level::ERROR, "verification failed: rank feature 'value(1, 2, 3).3' (feature verification test)"}}})); } TEST_F("verify illegal input parameters", RankFixture) { - EXPECT_FALSE(f1.verify("value.0")); + EXPECT_FALSE(f1.verify("value.0", + {{equal, {Level::WARNING, "invalid rank feature 'value.0':" + " The parameter list used for setting up rank feature value is not valid:" + " Expected 1+1x parameter(s), but got 0"}}, + {equal, {Level::ERROR, "verification failed: rank feature 'value.0' (feature verification test)"}}})); } TEST_F("verify illegal feature name", RankFixture) { - EXPECT_FALSE(f1.verify("value(2).")); + EXPECT_FALSE(f1.verify("value(2).", + {{equal, {Level::WARNING, "invalid rank feature 'value(2).': malformed name"}}, + {equal, {Level::ERROR, "verification failed: rank feature 'value(2).' (feature verification test)"}}})); } TEST_F("verify too deep dependency graph", RankFixture) { - EXPECT_TRUE(f1.verify("chain(basic, 255, 4)")); - EXPECT_FALSE(f1.verify("chain(basic, 256, 4)")); + EXPECT_TRUE(f1.verify("chain(basic, 255, 4)", {})); + EXPECT_FALSE(f1.verify("chain(basic, 256, 4)", + {{equal, + {Level::WARNING, + "invalid rank feature 'value(4)': dependency graph too deep\n" + " ... needed by rank feature 'chain(basic,1,4)'\n" + " ... needed by rank feature 'chain(basic,2,4)'\n" + " ... needed by rank feature 'chain(basic,3,4)'\n" + " ... needed by rank feature 'chain(basic,4,4)'\n" + " ... needed by rank feature 'chain(basic,5,4)'\n" + " ... needed by rank feature 'chain(basic,6,4)'\n" + " ... needed by rank feature 'chain(basic,7,4)'\n" + " ... needed by rank feature 'chain(basic,8,4)'\n" + " ... needed by rank feature 'chain(basic,9,4)'\n" + " ... needed by rank feature 'chain(basic,10,4)'\n" + " (skipped 241 entries)\n" + " ... needed by rank feature 'chain(basic,252,4)'\n" + " ... needed by rank feature 'chain(basic,253,4)'\n" + " ... needed by rank feature 'chain(basic,254,4)'\n" + " ... needed by rank feature 'chain(basic,255,4)'\n" + " ... needed by rank feature 'chain(basic,256,4)'\n"}}, + {regex, {Level::WARNING, "high stack usage: [0-9]+ bytes"}}, + {equal, {Level::ERROR, "verification failed: rank feature 'chain(basic, 256, 4)' (feature verification test)"}}})); } TEST_F("verify dependency cycle", RankFixture) { - EXPECT_FALSE(f1.verify("chain(cycle, 4, 2)")); + EXPECT_FALSE(f1.verify("chain(cycle, 4, 2)", + {{equal, + {Level::WARNING, + "invalid rank feature 'chain(cycle,2,2)': dependency cycle detected\n" + " ... needed by rank feature 'chain(cycle,1,2)'\n" + " ... needed by rank feature 'chain(cycle,2,2)'\n" + " ... needed by rank feature 'chain(cycle,3,2)'\n" + " ... needed by rank feature 'chain(cycle,4,2)'\n"}}, + {equal, {Level::ERROR, "verification failed: rank feature 'chain(cycle, 4, 2)' (feature verification test)"}}})); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.cpp b/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.cpp index 90386dffd51..4b2d67c933d 100644 --- a/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.cpp +++ b/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.cpp @@ -79,13 +79,13 @@ resolve_attribute_for_field(const fef::IQueryEnvironment& env, } -DistanceCalculatorBundle::Element::Element(fef::TermFieldHandle handle_in) +DistanceCalculatorBundle::Element::Element(fef::TermFieldHandle handle_in) noexcept : handle(handle_in), calc() { } -DistanceCalculatorBundle::Element::Element(fef::TermFieldHandle handle_in, std::unique_ptr<search::tensor::DistanceCalculator> calc_in) +DistanceCalculatorBundle::Element::Element(fef::TermFieldHandle handle_in, std::unique_ptr<search::tensor::DistanceCalculator> calc_in) noexcept : handle(handle_in), calc(std::move(calc_in)) { diff --git a/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.h b/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.h index dd3fc521d96..35295c771a6 100644 --- a/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.h +++ b/searchlib/src/vespa/searchlib/features/distance_calculator_bundle.h @@ -27,8 +27,8 @@ public: fef::TermFieldHandle handle; std::unique_ptr<search::tensor::DistanceCalculator> calc; Element(Element&& rhs) noexcept = default; // Needed as std::vector::reserve() is used. - Element(fef::TermFieldHandle handle_in); - Element(fef::TermFieldHandle handle_in, std::unique_ptr<search::tensor::DistanceCalculator> calc_in); + Element(fef::TermFieldHandle handle_in) noexcept; + Element(fef::TermFieldHandle handle_in, std::unique_ptr<search::tensor::DistanceCalculator> calc_in) noexcept; ~Element(); }; private: diff --git a/searchlib/src/vespa/searchlib/fef/blueprint.cpp b/searchlib/src/vespa/searchlib/fef/blueprint.cpp index 58094121e24..72e4021986a 100644 --- a/searchlib/src/vespa/searchlib/fef/blueprint.cpp +++ b/searchlib/src/vespa/searchlib/fef/blueprint.cpp @@ -5,9 +5,6 @@ #include <cassert> #include <vespa/vespalib/util/stringfmt.h> -#include <vespa/log/log.h> -LOG_SETUP(".fef.blueprint"); - namespace search::fef { std::optional<FeatureType> diff --git a/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp b/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp index 731306d1bea..b672d754b72 100644 --- a/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp +++ b/searchlib/src/vespa/searchlib/fef/blueprintresolver.cpp @@ -7,14 +7,10 @@ #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/util/threadstackexecutor.h> -#include <stack> #include <cassert> #include <set> #include <thread> -#include <vespa/log/log.h> -LOG_SETUP(".fef.blueprintresolver"); - using vespalib::make_string_short::fmt; using vespalib::ThreadStackExecutor; using vespalib::makeLambdaTask; @@ -63,6 +59,7 @@ struct Compiler : public Blueprint::DependencyHandler { : spec(std::move(blueprint)), parser(parser_in) {} }; using Stack = std::vector<Frame>; + using Errors = std::vector<vespalib::string>; struct FrameGuard { Stack &stack; @@ -73,15 +70,16 @@ struct Compiler : public Blueprint::DependencyHandler { } }; - const BlueprintFactory &factory; - const IIndexEnvironment &index_env; - Stack resolve_stack; - ExecutorSpecList &spec_list; - FeatureMap &feature_map; - std::set<vespalib::string> setup_set; - std::set<vespalib::string> failed_set; - const char *min_stack; - const char *max_stack; + const BlueprintFactory &factory; + const IIndexEnvironment &index_env; + Stack resolve_stack; + Errors errors; + ExecutorSpecList &spec_list; + FeatureMap &feature_map; + std::set<vespalib::string> setup_set; + std::set<vespalib::string> failed_set; + const char *min_stack; + const char *max_stack; Compiler(const BlueprintFactory &factory_in, const IIndexEnvironment &index_env_in, @@ -90,6 +88,7 @@ struct Compiler : public Blueprint::DependencyHandler { : factory(factory_in), index_env(index_env_in), resolve_stack(), + errors(), spec_list(spec_list_out), feature_map(feature_map_out), setup_set(), @@ -138,11 +137,13 @@ struct Compiler : public Blueprint::DependencyHandler { if (failed_set.count(feature_name) == 0) { failed_set.insert(feature_name); auto trace = make_trace(skip_self); + vespalib::string msg; if (trace.empty()) { - LOG(warning, "invalid %s: %s", describe(feature_name).c_str(), reason.c_str()); + msg = fmt("invalid %s: %s", describe(feature_name).c_str(), reason.c_str()); } else { - LOG(warning, "invalid %s: %s\n%s", describe(feature_name).c_str(), reason.c_str(), trace.c_str()); + msg = fmt("invalid %s: %s\n%s", describe(feature_name).c_str(), reason.c_str(), trace.c_str()); } + errors.emplace_back(msg); } probe_stack(); return FeatureRef(); @@ -264,7 +265,8 @@ BlueprintResolver::BlueprintResolver(const BlueprintFactory &factory, _seeds(), _executorSpecs(), _featureMap(), - _seedMap() + _seedMap(), + _warnings() { } @@ -300,6 +302,7 @@ BlueprintResolver::compile() for (const auto &seed: _seeds) { auto ref = compiler.resolve_feature(seed, Blueprint::AcceptInput::ANY); if (compiler.failed()) { + _warnings = std::move(compiler.errors); return; } _seedMap.emplace(FeatureNameParser(seed).featureName(), ref); @@ -311,27 +314,9 @@ BlueprintResolver::compile() executor.shutdown(); size_t stack_usage = compiler.stack_usage(); if (stack_usage > (128_Ki)) { - LOG(warning, "high stack usage: %zu bytes", stack_usage); + _warnings.emplace_back(fmt("high stack usage: %zu bytes", stack_usage)); } return !compiler.failed(); } -const BlueprintResolver::ExecutorSpecList & -BlueprintResolver::getExecutorSpecs() const -{ - return _executorSpecs; -} - -const BlueprintResolver::FeatureMap & -BlueprintResolver::getFeatureMap() const -{ - return _featureMap; -} - -const BlueprintResolver::FeatureMap & -BlueprintResolver::getSeedMap() const -{ - return _seedMap; -} - } diff --git a/searchlib/src/vespa/searchlib/fef/blueprintresolver.h b/searchlib/src/vespa/searchlib/fef/blueprintresolver.h index 3e3b5879518..459082b4534 100644 --- a/searchlib/src/vespa/searchlib/fef/blueprintresolver.h +++ b/searchlib/src/vespa/searchlib/fef/blueprintresolver.h @@ -24,7 +24,8 @@ class FeatureNameParser; class BlueprintResolver { public: - typedef std::shared_ptr<BlueprintResolver> SP; + using SP = std::shared_ptr<BlueprintResolver>; + using Warnings = std::vector<vespalib::string>; /** * Low-level reference to a single output from a feature @@ -44,7 +45,7 @@ public: : executor(executor_in), output(output_in) {} bool valid() { return (executor != undef); } }; - typedef std::map<vespalib::string, FeatureRef> FeatureMap; + using FeatureMap = std::map<vespalib::string, FeatureRef>; /** * Thin blueprint wrapper with additional information about how @@ -59,7 +60,7 @@ public: ExecutorSpec(Blueprint::SP blueprint_in); ~ExecutorSpec(); }; - typedef std::vector<ExecutorSpec> ExecutorSpecList; + using ExecutorSpecList = std::vector<ExecutorSpec>; /** * The maximum dependency depth. This value is defined to protect @@ -84,6 +85,7 @@ private: ExecutorSpecList _executorSpecs; FeatureMap _featureMap; FeatureMap _seedMap; + Warnings _warnings; public: BlueprintResolver(const BlueprintResolver &) = delete; @@ -134,7 +136,7 @@ public: * * @return feature executor assembly directions **/ - const ExecutorSpecList &getExecutorSpecs() const; + const ExecutorSpecList &getExecutorSpecs() const { return _executorSpecs; } /** * Obtain the location of all named features known to this @@ -145,7 +147,7 @@ public: * * @return feature locations **/ - const FeatureMap &getFeatureMap() const; + const FeatureMap &getFeatureMap() const { return _featureMap; } /** * Obtain the location of all seeds used by this resolver. This @@ -156,7 +158,13 @@ public: * * @return seed locations **/ - const FeatureMap &getSeedMap() const; + const FeatureMap &getSeedMap() const { return _seedMap; } + + /** + * Will return any accumulated warnings during compile + * @return list of warnings + **/ + const Warnings & getWarnings() const { return _warnings; } }; } diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp index ed841ae24b3..30e220b0a34 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp @@ -3,9 +3,10 @@ #include "ranksetup.h" #include "indexproperties.h" #include "featurenameparser.h" +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/log/log.h> -LOG_SETUP(".fef.ranksetup"); +using vespalib::make_string_short::fmt; namespace { class VisitorAdapter : public search::fef::IDumpFeatureVisitor @@ -53,6 +54,7 @@ RankSetup::RankSetup(const BlueprintFactory &factory, const IIndexEnvironment &i _match_features(), _summaryFeatures(), _dumpFeatures(), + _warnings(), _feature_rename_map(), _ignoreDefaultRankFeatures(false), _compiled(false), @@ -134,50 +136,59 @@ RankSetup::configure() void RankSetup::setFirstPhaseRank(const vespalib::string &featureName) { - LOG_ASSERT(!_compiled); + assert(!_compiled); _firstPhaseRankFeature = featureName; } void RankSetup::setSecondPhaseRank(const vespalib::string &featureName) { - LOG_ASSERT(!_compiled); + assert(!_compiled); _secondPhaseRankFeature = featureName; } void RankSetup::add_match_feature(const vespalib::string &match_feature) { - LOG_ASSERT(!_compiled); + assert(!_compiled); _match_features.push_back(match_feature); } void RankSetup::addSummaryFeature(const vespalib::string &summaryFeature) { - LOG_ASSERT(!_compiled); + assert(!_compiled); _summaryFeatures.push_back(summaryFeature); } void RankSetup::addDumpFeature(const vespalib::string &dumpFeature) { - LOG_ASSERT(!_compiled); + assert(!_compiled); _dumpFeatures.push_back(dumpFeature); } +void +RankSetup::compileAndCheckForErrors(BlueprintResolver &bpr) { + bool ok = bpr.compile(); + if ( ! ok ) { + _compileError = true; + const auto & warnings = bpr.getWarnings(); + _warnings.insert(_warnings.end(), warnings.begin(), warnings.end()); + } +} bool RankSetup::compile() { - LOG_ASSERT(!_compiled); + assert(!_compiled); if (!_firstPhaseRankFeature.empty()) { FeatureNameParser parser(_firstPhaseRankFeature); if (parser.valid()) { _firstPhaseRankFeature = parser.featureName(); _first_phase_resolver->addSeed(_firstPhaseRankFeature); } else { - LOG(warning, "invalid feature name for initial rank: '%s'", - _firstPhaseRankFeature.c_str()); + vespalib::string e = fmt("invalid feature name for initial rank: '%s'", _firstPhaseRankFeature.c_str()); + _warnings.emplace_back(e); _compileError = true; } } @@ -187,8 +198,8 @@ RankSetup::compile() _secondPhaseRankFeature = parser.featureName(); _second_phase_resolver->addSeed(_secondPhaseRankFeature); } else { - LOG(warning, "invalid feature name for final rank: '%s'", - _secondPhaseRankFeature.c_str()); + vespalib::string e = fmt("invalid feature name for final rank: '%s'", _secondPhaseRankFeature.c_str()); + _warnings.emplace_back(e); _compileError = true; } } @@ -206,12 +217,12 @@ RankSetup::compile() _dumpResolver->addSeed(feature); } _indexEnv.hintFeatureMotivation(IIndexEnvironment::RANK); - _compileError |= !_first_phase_resolver->compile(); - _compileError |= !_second_phase_resolver->compile(); - _compileError |= !_match_resolver->compile(); - _compileError |= !_summary_resolver->compile(); + compileAndCheckForErrors(*_first_phase_resolver); + compileAndCheckForErrors(*_second_phase_resolver); + compileAndCheckForErrors(*_match_resolver); + compileAndCheckForErrors(*_summary_resolver); _indexEnv.hintFeatureMotivation(IIndexEnvironment::DUMP); - _compileError |= !_dumpResolver->compile(); + compileAndCheckForErrors(*_dumpResolver); _compiled = true; return !_compileError; } @@ -219,7 +230,7 @@ RankSetup::compile() void RankSetup::prepareSharedState(const IQueryEnvironment &queryEnv, IObjectStore &objectStore) const { - LOG_ASSERT(_compiled && !_compileError); + assert(_compiled && !_compileError); for (const auto &spec : _first_phase_resolver->getExecutorSpecs()) { spec.blueprint->prepareSharedState(queryEnv, objectStore); } @@ -234,4 +245,13 @@ RankSetup::prepareSharedState(const IQueryEnvironment &queryEnv, IObjectStore &o } } +vespalib::string +RankSetup::getJoinedWarnings() const { + vespalib::asciistream os; + for (const auto & m : _warnings) { + os << m << "\n"; + } + return os.str(); +} + } diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.h b/searchlib/src/vespa/searchlib/fef/ranksetup.h index 6a2871827ab..1ad2fe0a77a 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.h +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.h @@ -23,6 +23,7 @@ namespace search::fef { class RankSetup { public: + using Warnings = BlueprintResolver::Warnings; struct MutateOperation { public: MutateOperation() : MutateOperation("", "") {} @@ -62,6 +63,7 @@ private: std::vector<vespalib::string> _match_features; std::vector<vespalib::string> _summaryFeatures; std::vector<vespalib::string> _dumpFeatures; + Warnings _warnings; StringStringMap _feature_rename_map; bool _ignoreDefaultRankFeatures; bool _compiled; @@ -80,6 +82,8 @@ private: MutateOperation _mutateOnFirstPhase; MutateOperation _mutateOnSecondPhase; MutateOperation _mutateOnSummary; + + void compileAndCheckForErrors(BlueprintResolver &bp); public: RankSetup(const RankSetup &) = delete; RankSetup &operator=(const RankSetup &) = delete; @@ -438,6 +442,12 @@ public: **/ bool compile(); + /** + * Will return any accumulated warnings during compile + * @return joined string of warnings separated by newline + */ + vespalib::string getJoinedWarnings() const; + // These functions create rank programs for different tasks. Note // that the setup function must be called on rank programs for // them to be ready to use. Also keep in mind that creating a rank diff --git a/searchlib/src/vespa/searchlib/fef/verify_feature.cpp b/searchlib/src/vespa/searchlib/fef/verify_feature.cpp index 85d1daffd01..b012e95c8fc 100644 --- a/searchlib/src/vespa/searchlib/fef/verify_feature.cpp +++ b/searchlib/src/vespa/searchlib/fef/verify_feature.cpp @@ -2,28 +2,31 @@ #include "verify_feature.h" #include "blueprintresolver.h" +#include <vespa/vespalib/util/stringfmt.h> -#include <vespa/log/log.h> -LOG_SETUP(".fef.verify_feature"); +using vespalib::make_string_short::fmt; -namespace search { -namespace fef { +namespace search::fef { bool verifyFeature(const BlueprintFactory &factory, const IIndexEnvironment &indexEnv, const std::string &featureName, - const std::string &desc) + const std::string &desc, + std::vector<Message> & errors) { indexEnv.hintFeatureMotivation(IIndexEnvironment::VERIFY_SETUP); BlueprintResolver resolver(factory, indexEnv); resolver.addSeed(featureName); bool result = resolver.compile(); if (!result) { - LOG(error, "verification failed: %s (%s)", - BlueprintResolver::describe_feature(featureName).c_str(), desc.c_str()); + const BlueprintResolver::Warnings & warnings(resolver.getWarnings()); + for (const auto & msg : warnings) { + errors.emplace_back(Level::WARNING, msg); + } + vespalib::string msg = fmt("verification failed: %s (%s)",BlueprintResolver::describe_feature(featureName).c_str(), desc.c_str()); + errors.emplace_back(Level::ERROR, msg); } return result; } -} // namespace fef -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/fef/verify_feature.h b/searchlib/src/vespa/searchlib/fef/verify_feature.h index 5c84403fc8a..f6f3924616a 100644 --- a/searchlib/src/vespa/searchlib/fef/verify_feature.h +++ b/searchlib/src/vespa/searchlib/fef/verify_feature.h @@ -4,10 +4,11 @@ #include "blueprintfactory.h" #include "iindexenvironment.h" -#include <string> -namespace search { -namespace fef { +namespace search::fef { + +enum class Level {INFO, WARNING, ERROR}; +using Message = std::pair<Level, vespalib::string>; /** * Verify whether a specific feature can be computed. If the feature @@ -23,8 +24,7 @@ namespace fef { bool verifyFeature(const BlueprintFactory &factory, const IIndexEnvironment &indexEnv, const std::string &featureName, - const std::string &desc); - -} // namespace fef -} // namespace search + const std::string &desc, + std::vector<Message> & errors); +} diff --git a/searchsummary/src/tests/juniper/auxTest.cpp b/searchsummary/src/tests/juniper/auxTest.cpp index 15f5ad1749e..c42cf4ccd83 100644 --- a/searchsummary/src/tests/juniper/auxTest.cpp +++ b/searchsummary/src/tests/juniper/auxTest.cpp @@ -136,15 +136,14 @@ AuxTest::TestDoubleWidth() juniper::QueryParser q("\xef\xbd\x93\xef\xbd\x8f\xef\xbd\x8e\xef\xbd\x99"); juniper::QueryHandle qh(q, nullptr, juniper.getModifier()); - juniper::Result* res = juniper::Analyse(&myConfig, &qh, - input, 17, 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(myConfig, qh, + input, 17, 0, 0, 0); + _test(static_cast<bool>(res)); - juniper::Summary* sum = juniper::GetTeaser(res, nullptr); + juniper::Summary* sum = juniper::GetTeaser(*res, nullptr); (void) sum; // this should work // _test(sum->Length() != 0); - juniper::ReleaseResult(res); } @@ -175,17 +174,15 @@ AuxTest::TestPartialUTF8() juniper::QueryParser q("ipod"); juniper::QueryHandle qh(q, nullptr, juniper.getModifier()); - juniper::Result* res = juniper::Analyse(&myConfig, &qh, - input, inputSize, 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(myConfig, qh, + input, inputSize, 0, 0, 0); + _test(static_cast<bool>(res)); - juniper::Summary* sum = juniper::GetTeaser(res, nullptr); + juniper::Summary* sum = juniper::GetTeaser(*res, nullptr); _test(sum->Length() != 0); // check for partial/broken utf-8 _test(countBrokenUTF8(sum->Text(), sum->Length()) == 0); - - juniper::ReleaseResult(res); } void AuxTest::TestLargeBlockChinese() @@ -215,11 +212,11 @@ void AuxTest::TestLargeBlockChinese() juniper::QueryParser q("希望"); juniper::QueryHandle qh(q, nullptr, juniper.getModifier()); - juniper::Result* res = juniper::Analyse(&myConfig, &qh, - input, inputSize, 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(myConfig, qh, + input, inputSize, 0, 0, 0); + _test(static_cast<bool>(res)); - juniper::Summary* sum = juniper::GetTeaser(res, nullptr); + juniper::Summary* sum = juniper::GetTeaser(*res, nullptr); _test(sum->Length() != 0); // check that the entire block of chinese data is not returned in the summary @@ -227,8 +224,6 @@ void AuxTest::TestLargeBlockChinese() // check for partial/broken utf-8 _test(countBrokenUTF8(sum->Text(), sum->Length()) == 0); - - juniper::ReleaseResult(res); } void AuxTest::TestExample() @@ -241,17 +236,14 @@ void AuxTest::TestExample() "&%#%&! cries the sleepy monkey and jumps down from the tree." "the last token here is split across lines consumed"; int content_len = strlen(content); - juniper::Result* res = - juniper::Analyse(juniper::TestConfig, - &qh, - content, content_len, - 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(*juniper::TestConfig, qh, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); res->Scan(); Matcher& m = *res->_matcher; _test(m.TotalMatchCnt(0) == 2 && m.ExactMatchCnt(0) == 0); - juniper::ReleaseResult(res); } @@ -410,8 +402,8 @@ void AuxTest::TestUTF8context() s.append(char_from_u8(u8" beste forekomst av s\u00f8ket med s\u00f8kemotor til brukeren blir det enda bedre. ")); s.append(char_from_u8(u8"Hvis bare UTF8-kodingen virker som den skal for tegn som tar mer enn \u00e9n byte.")); - juniper::Result* res = juniper::Analyse(juniper::TestConfig, &qh, s.c_str(), s.size(), 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(*juniper::TestConfig, qh, s.c_str(), s.size(), 0, 0, 0); + _test(static_cast<bool>(res)); size_t charsize; Matcher& m = *res->_matcher; @@ -454,7 +446,6 @@ void AuxTest::TestUTF8context() test_dump(s.c_str(), s.size()); m.dump_statistics(); } - juniper::ReleaseResult(res); } @@ -491,10 +482,10 @@ void AuxTest::TestJapanese() const char* content = testjap[i].text; int content_len = strlen(content); - juniper::Result* res = juniper::Analyse(juniper::TestConfig, &qh, - content, content_len, - 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(*juniper::TestConfig, qh, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); size_t charsize; Matcher& m = *res->_matcher; @@ -545,7 +536,6 @@ void AuxTest::TestJapanese() default: break; } - juniper::ReleaseResult(res); DeleteSummaryDesc(sumdesc); DeleteSummaryConfig(_sumconf); } @@ -582,15 +572,14 @@ void AuxTest::TestStartHits() " In fact it must be much longer. And then som more text at the end. But this text at " "the end must be much longer than this to trigger the case"; int content_len = strlen(content); - juniper::Result* res = juniper::Analyse(juniper::TestConfig, &qh, - content, content_len, - 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(*juniper::TestConfig, qh, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); - juniper::Summary* sum = juniper::GetTeaser(res, nullptr); + juniper::Summary* sum = juniper::GetTeaser(*res, nullptr); (void) sum; // TODO: ReEnable _test(sum->Length() != 0); - juniper::ReleaseResult(res); } @@ -607,14 +596,13 @@ void AuxTest::TestEndHit() "surround_len bytes closer than good towardstheend�����������������������������������"; size_t content_len = strlen(content) - 55; - juniper::Result* res = juniper::Analyse(juniper::TestConfig, &qh, - content, content_len, - 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(*juniper::TestConfig, qh, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); - juniper::Summary* sum = juniper::GetTeaser(res, nullptr); + juniper::Summary* sum = juniper::GetTeaser(*res, nullptr); _test(sum->Length() != 0); - juniper::ReleaseResult(res); } void AuxTest::TestJuniperStack() @@ -880,14 +868,13 @@ AuxTest::TestWhiteSpacePreserved() juniper::QueryParser q("best"); juniper::QueryHandle qh(q, nullptr, juniper.getModifier()); - juniper::Result* res = juniper::Analyse(&myConfig, &qh, input.c_str(), input.size(), 0, 0, 0); - _test(res != nullptr); + auto res = juniper::Analyse(myConfig, qh, input.c_str(), input.size(), 0, 0, 0); + _test(static_cast<bool>(res)); - juniper::Summary* sum = juniper::GetTeaser(res, nullptr); + juniper::Summary* sum = juniper::GetTeaser(*res, nullptr); vespalib::string expected = "<hi>best</hi> of \nmetallica"; vespalib::string actual(sum->Text(), sum->Length()); _test(actual == expected); - juniper::ReleaseResult(res); } void AuxTest::Run(MethodContainer::iterator &itr) { diff --git a/searchsummary/src/tests/juniper/matchobjectTest.cpp b/searchsummary/src/tests/juniper/matchobjectTest.cpp index 07e3cf84767..d8b7724d8a5 100644 --- a/searchsummary/src/tests/juniper/matchobjectTest.cpp +++ b/searchsummary/src/tests/juniper/matchobjectTest.cpp @@ -28,10 +28,10 @@ void MatchObjectTest::testTerm() { size_t content_len = strlen(content); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Do the scanning manually. This calls accept several times res->Scan(); @@ -42,51 +42,39 @@ void MatchObjectTest::testTerm() { _test(ms.size() == 2); - delete res; // printf("%d %d\n", m.TotalHits(),ms.size()); TestQuery q1("t*t"); TestQuery q2("*ea*"); TestQuery q3("*d"); TestQuery q4("*word"); - Result* r1 = juniper::Analyse(juniper::TestConfig, &q1._qhandle, content, content_len, 0, 0, 0); - Result* r2 = juniper::Analyse(juniper::TestConfig, &q2._qhandle, content, content_len, 0, 0, 0); - Result* r3 = juniper::Analyse(juniper::TestConfig, &q3._qhandle, content, content_len, 0, 0, 0); - Result* r4 = juniper::Analyse(juniper::TestConfig, &q4._qhandle, content, content_len, 0, 0, 0); - if (r1 != 0) - { + auto r1 = juniper::Analyse(*juniper::TestConfig, q1._qhandle, content, content_len, 0, 0, 0); + auto r2 = juniper::Analyse(*juniper::TestConfig, q2._qhandle, content, content_len, 0, 0, 0); + auto r3 = juniper::Analyse(*juniper::TestConfig, q3._qhandle, content, content_len, 0, 0, 0); + auto r4 = juniper::Analyse(*juniper::TestConfig, q4._qhandle, content, content_len, 0, 0, 0); + _test(static_cast<bool>(r1)); + if (r1) { r1->Scan(); _test(r1->_matcher->TotalHits() == 1); - delete r1; } - else - _test(r1 != 0); - - if (r2 != 0) - { + _test(static_cast<bool>(r2)); + if (r2) { r2->Scan(); _test(r2->_matcher->TotalHits() == 2); - delete r2; } - else - _test(r2 != 0); - if (r3 != 0) - { + if (r3) { r3->Scan(); _test(r3->_matcher->TotalHits() == 2); - delete r3; + } else { + _test(static_cast<bool>(r3)); } - else - _test(r3 != 0); - if (r4 != 0) - { + if (r4) { r4->Scan(); _test_equal(r4->_matcher->TotalHits(), 2); - delete r4; + } else { + _test(static_cast<bool>(r4)); } - else - _test(r4 != 0); } /** @@ -98,7 +86,7 @@ void MatchObjectTest::testMatch() { juniper::QueryHandle qh(p, NULL, juniper::_Juniper->getModifier()); MatchObject* mo = qh.MatchObj(0); - juniper::Result res(juniper::TestConfig, &qh, "", 0, 0); + juniper::Result res(*juniper::TestConfig, qh, "", 0, 0); unsigned opts = 0; match_iterator mi(mo, &res); ucs4_t ucs4_str[10]; @@ -140,7 +128,7 @@ void MatchObjectTest::testMatch() { "extremelylongwordhit,extremelylongwordhits,extremelylongwordhit," "extremelylongwordhit))"); QueryHandle& qh1(q._qhandle); - juniper::Result res1(juniper::TestConfig, &qh1, + juniper::Result res1(*juniper::TestConfig, qh1, doc.c_str(), doc.size(), 0); juniper::Summary* sum = res1.GetTeaser(NULL); std::string s(sum->Text()); @@ -165,7 +153,7 @@ void MatchObjectTest::testMatchAnnotated() { " stuff"; TestQuery q("AND(big,buy)"); QueryHandle &qh1(q._qhandle); - juniper::Result res1(juniper::TestConfig, &qh1, + juniper::Result res1(*juniper::TestConfig, qh1, doc, strlen(doc), 0); juniper::Summary *sum = res1.GetTeaser(NULL); std::string s(sum->Text()); @@ -205,7 +193,7 @@ void MatchObjectTest::testLangid() std::string doc("see if we can match b or c somewhere in this a3 doc. " "Note that we should not match b1 or c1 or a somewhere.."); - juniper::Result res(juniper::TestConfig, &qh, doc.c_str(), doc.size(),0); + juniper::Result res(*juniper::TestConfig, qh, doc.c_str(), doc.size(),0); juniper::Summary* sum = res.GetTeaser(NULL); std::string s(sum->Text()); @@ -218,7 +206,7 @@ void MatchObjectTest::testLangid() // Do another test with the same query handle (testing reuse of qh with rewriters) std::string doc("Try to run this on another doc just to see if b or c still can be" " matched with the same query handle"); - juniper::Result res(juniper::TestConfig, &qh, + juniper::Result res(*juniper::TestConfig, qh, doc.c_str(), doc.size(), 0); juniper::Summary* sum = res.GetTeaser(NULL); @@ -247,7 +235,7 @@ void MatchObjectTest::testCombined() { std::string doc("see if we can match a3 or c somewhere in this b doc. " "Note that we should not match b1 or c1 or a somewhere.."); - juniper::Result res(juniper::TestConfig, &qh, doc.c_str(), doc.size(), 0); + juniper::Result res(*juniper::TestConfig, qh, doc.c_str(), doc.size(), 0); juniper::Summary* sum = res.GetTeaser(NULL); std::string s(sum->Text()); diff --git a/searchsummary/src/tests/juniper/mcandTest.cpp b/searchsummary/src/tests/juniper/mcandTest.cpp index 5a465275a80..46bd4a5196f 100644 --- a/searchsummary/src/tests/juniper/mcandTest.cpp +++ b/searchsummary/src/tests/juniper/mcandTest.cpp @@ -41,24 +41,23 @@ void MatchCandidateTest::testLog() { TestQuery q(""); std::string content("Here we go hepp and then some words away hoi some silly text here"); - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content.c_str(), content.size(), - 0, 0, 0); - _test(res); // We get a result handle + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content.c_str(), content.size(), + 0, 0, 0); + _test(static_cast<bool>(res)); // We get a result handle _test(!res->_mo); // but it is empty - juniper::Summary* sum = juniper::GetTeaser(res); + juniper::Summary* sum = juniper::GetTeaser(*res); std::string s(sum->Text()); _test_equal(s, std::string("")); - long relevance = juniper::GetRelevancy(res); + long relevance = juniper::GetRelevancy(*res); _test_equal(relevance, PROXIMITYBOOST_NOCONSTRAINT_OFFSET); - sum = juniper::GetLog(res); + sum = juniper::GetLog(*res); s = sum->Text(); _test_equal(s, std::string("")); - juniper::ReleaseResult(res); } @@ -70,56 +69,52 @@ void MatchCandidateTest::testDump() { { TestQuery q("NEAR/1(hepp,hoi)"); - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content.c_str(), content.size(), - 0, 0, 0); - _test(res != NULL); - long relevance = juniper::GetRelevancy(res); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content.c_str(), content.size(), + 0, 0, 0); + _test(static_cast<bool>(res)); + long relevance = juniper::GetRelevancy(*res); // zero value since there are no hits and constraints are enabled.. _test_equal(relevance, 0); - juniper::ReleaseResult(res); } { TestQuery q("OR(NEAR/1(hepp,hoi),bananas)"); - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content.c_str(), content.size(), - 0, 0, 0); - _test(res != NULL); - long relevance = juniper::GetRelevancy(res); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content.c_str(), content.size(), + 0, 0, 0); + _test(static_cast<bool>(res)); + long relevance = juniper::GetRelevancy(*res); // Check that X_CONSTR propagates as intended _test_equal(relevance, 0); - juniper::ReleaseResult(res); } { TestQuery q("PHRASE(hepp,hoi)"); - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content.c_str(), content.size(), - 0, 0, 0); - _test(res != NULL); - long relevance = juniper::GetRelevancy(res); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content.c_str(), content.size(), + 0, 0, 0); + _test(static_cast<bool>(res)); + long relevance = juniper::GetRelevancy(*res); // constant value since there are no hits but this is // also not a constrained search.. _test_equal(relevance, PROXIMITYBOOST_NOCONSTRAINT_OFFSET); - juniper::ReleaseResult(res); } { TestQuery q("AND(hepp,hoi)"); - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content.c_str(), content.size(), - 0, 0, 0); - _test(res != NULL); - long relevance = juniper::GetRelevancy(res); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content.c_str(), content.size(), + 0, 0, 0); + _test(static_cast<bool>(res)); + long relevance = juniper::GetRelevancy(*res); // Relevance may change, but nice to discover such changes.. // The important is that we get a nonzero value here as a hit _test_equal(relevance, 4470); - juniper::ReleaseResult(res); } } @@ -135,11 +130,11 @@ void MatchCandidateTest::testorder() { size_t content_len = strlen(content); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Do the scanning manually. Scan calls accept several times res->Scan(); @@ -150,7 +145,6 @@ void MatchCandidateTest::testorder() { match_candidate_set& ms = m.OrderedMatchSet(); _test(ms.size() == 1); - juniper::ReleaseResult(res); } @@ -165,11 +159,11 @@ void MatchCandidateTest::testMatches_limit() { size_t content_len = strlen(content); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Do the scanning manually. This calls accept several times res->Scan(); @@ -182,11 +176,10 @@ void MatchCandidateTest::testMatches_limit() { _test(ms.size() == 2); // The first (complete) match and the second starting at "test" // Check if we get the correct teaser as well.. - juniper::Summary* sum = juniper::GetTeaser(res); + juniper::Summary* sum = juniper::GetTeaser(*res); _test(strcmp(sum->Text(), "This is a simple text where a <b>phrase</b> <b>match</b> can be found not" " quite adjacent to a <b>test</b> <b>word</b>") == 0); - juniper::ReleaseResult(res); } @@ -200,11 +193,11 @@ void MatchCandidateTest::testAccept() { size_t content_len = strlen(content); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Do the scanning manually. This calls accept several times res->Scan(); @@ -218,7 +211,6 @@ void MatchCandidateTest::testAccept() { _test(ms.size() > 0); if (!ms.size()) { - juniper::ReleaseResult(res); return; // No point in continuing.. } @@ -234,10 +226,9 @@ void MatchCandidateTest::testAccept() { _test(mc._klist.size() == 2); // Two occurrence elements in list // Just for the sake of it, verify that we get a proper teaser out of this also.. - juniper::Summary* sum = juniper::GetTeaser(res); + juniper::Summary* sum = juniper::GetTeaser(*res); _test(strcmp(sum->Text(), "This is a <b>simple</b> <b>test</b> where we should get a perfect match") == 0); - juniper::ReleaseResult(res); } @@ -260,11 +251,11 @@ void MatchCandidateTest::testMake_keylist() { size_t content_len = strlen(content); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Do the scanning manually. This calls accept several times res->Scan(); @@ -275,8 +266,6 @@ void MatchCandidateTest::testMake_keylist() { match_candidate_set& ms = m.OrderedMatchSet(); _test_equal(static_cast<size_t>(ms.size()), 6u); - - juniper::ReleaseResult(res); } @@ -291,11 +280,11 @@ void MatchCandidateTest::testAdd_to_keylist() { size_t content_len = strlen(content); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Do the scanning manually. This calls accept several times res->Scan(); @@ -308,13 +297,12 @@ void MatchCandidateTest::testAdd_to_keylist() { _test_equal(static_cast<size_t>(ms.size()), 1u); // Single result // Bug triggered when result is fetched.. - juniper::Summary* sum = juniper::GetTeaser(res); + juniper::Summary* sum = juniper::GetTeaser(*res); std::string s(sum->Text()); _test_equal(s, "connect truende. <b>phr1</b> <b>phr2</b> www www www <b>phr3</b>" " <b>phr4</b> acuicola 8844"); - juniper::ReleaseResult(res); } @@ -331,11 +319,11 @@ void MatchCandidateTest::testLength() { TestQuery q("NEAR/4(pattern,NEAR/1(simple,with),NEAR/2(simple,adjacent))"); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, &q._qhandle, - content, content_len, - 0, 0, 0); + auto res = juniper::Analyse(*juniper::TestConfig, q._qhandle, + content, content_len, + 0, 0, 0); - juniper::Summary* sum = juniper::GetTeaser(res); + juniper::Summary* sum = juniper::GetTeaser(*res); Matcher& m = *res->_matcher; match_candidate_set& ms = m.OrderedMatchSet(); _test_equal(static_cast<size_t>(ms.size()), 1u); @@ -345,7 +333,6 @@ void MatchCandidateTest::testLength() { "this <b>simple</b> text <b>with</b> <b>adjacent</b> words of " "a certain <b>pattern</b> must be matched according to specific" " rules to be detailed in this test."); - juniper::ReleaseResult(res); } { @@ -353,17 +340,16 @@ void MatchCandidateTest::testLength() { TestQuery q("ONEAR/4(pattern,NEAR/1(simple,with),NEAR/2(simple,adjacent))"); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, - &q._qhandle - ,content, content_len, - 0, 0, 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle + ,content, content_len, + 0, 0, 0); res->Scan(); Matcher& m = *res->_matcher; match_candidate_set& ms = m.OrderedMatchSet(); _test_equal(static_cast<size_t>(ms.size()), 0u); - juniper::ReleaseResult(res); } { @@ -372,16 +358,14 @@ void MatchCandidateTest::testLength() { TestQuery q("NEAR/4(pattern,NEAR/1(simple,with),NEAR/1(simple,adjacent))"); // Fetch a result descriptor: - Result* res = juniper::Analyse(juniper::TestConfig, &q._qhandle, - content, content_len, - 0, 0, 0); + auto res = juniper::Analyse(*juniper::TestConfig, q._qhandle, + content, content_len, + 0, 0, 0); res->Scan(); Matcher& m = *res->_matcher; match_candidate_set& ms = m.OrderedMatchSet(); _test_equal(static_cast<size_t>(ms.size()), 0u); - - juniper::ReleaseResult(res); } } @@ -416,11 +400,11 @@ void MatchCandidateTest::requireThatMaxNumberOfMatchCandidatesCanBeControlled() const char *content = "re re re re foo re re re re bar re re re re foo re re re re bar"; size_t content_len = strlen(content); - Result *res = juniper::Analyse(juniper::TestConfig, - &q._qhandle, - content, content_len, - 0, 0, 0); - _test(res != 0); + auto res = juniper::Analyse(*juniper::TestConfig, + q._qhandle, + content, content_len, + 0, 0, 0); + _test(static_cast<bool>(res)); // Deflect tokens to my processor Matcher &m = *res->_matcher; @@ -435,8 +419,6 @@ void MatchCandidateTest::requireThatMaxNumberOfMatchCandidatesCanBeControlled() _test_equal(m.TotalHits(), 20); match_candidate_set& mcs = m.OrderedMatchSet(); _test_equal(static_cast<size_t>(mcs.size()), 2u); - - juniper::ReleaseResult(res); } diff --git a/searchsummary/src/vespa/juniper/Matcher.cpp b/searchsummary/src/vespa/juniper/Matcher.cpp index e286068038b..d580faae927 100644 --- a/searchsummary/src/vespa/juniper/Matcher.cpp +++ b/searchsummary/src/vespa/juniper/Matcher.cpp @@ -38,7 +38,7 @@ Matcher::Matcher(Result* result) : _log_text("") { _occ.reserve(KEY_OCC_RESERVED); - DocsumParams& dsp = _result->_config->_docsumparams; + const DocsumParams& dsp = _result->_config->_docsumparams; _winsize = _result->WinSize(); _winsizeFallback = static_cast<size_t>(_result->WinSizeFallbackMultiplier() * _winsize); _max_match_candidates = _result->MaxMatchCandidates(); diff --git a/searchsummary/src/vespa/juniper/juniperparams.cpp b/searchsummary/src/vespa/juniper/juniperparams.cpp index 4f25b2446ad..2ee0f3c31f6 100644 --- a/searchsummary/src/vespa/juniper/juniperparams.cpp +++ b/searchsummary/src/vespa/juniper/juniperparams.cpp @@ -108,18 +108,12 @@ MatcherParams& MatcherParams::SetWordFolder(Fast_WordFolder* wordfolder) return *this; } -Fast_WordFolder* MatcherParams::WordFolder() { return _wordfolder; } - - MatcherParams& MatcherParams::SetProximityFactor(double factor) { _proximity_factor = factor; return *this; } -double MatcherParams::ProximityFactor() { return _proximity_factor; } - - bool operator==(MatcherParams& mp1, MatcherParams& mp2) { return memcmp(&mp1, &mp2, sizeof(MatcherParams)) == 0; diff --git a/searchsummary/src/vespa/juniper/juniperparams.h b/searchsummary/src/vespa/juniper/juniperparams.h index f4f17779f2d..415c254b3f0 100644 --- a/searchsummary/src/vespa/juniper/juniperparams.h +++ b/searchsummary/src/vespa/juniper/juniperparams.h @@ -48,8 +48,10 @@ class MatcherParams { public: MatcherParams(); - MatcherParams(MatcherParams &) = delete; - MatcherParams &operator=(MatcherParams &) = delete; + MatcherParams(const MatcherParams&) = delete; + MatcherParams(MatcherParams&&) = delete; + MatcherParams &operator=(const MatcherParams&) = delete; + MatcherParams &operator=(MatcherParams&&) = delete; MatcherParams& SetMatchWindowSize(size_t winsize); size_t MatchWindowSize() const; @@ -66,10 +68,10 @@ public: size_t StemMaxExtend() const; MatcherParams& SetWordFolder(Fast_WordFolder* wordfolder); - Fast_WordFolder* WordFolder(); + const Fast_WordFolder* WordFolder() const noexcept { return _wordfolder; } MatcherParams& SetProximityFactor(double factor); - double ProximityFactor(); + double ProximityFactor() const noexcept { return _proximity_factor; }; private: size_t _match_winsize; diff --git a/searchsummary/src/vespa/juniper/result.cpp b/searchsummary/src/vespa/juniper/result.cpp index 15ad9aa2a98..18d91fcae8e 100644 --- a/searchsummary/src/vespa/juniper/result.cpp +++ b/searchsummary/src/vespa/juniper/result.cpp @@ -27,14 +27,14 @@ public: }; -Result::Result(Config* config, QueryHandle* qhandle, +Result::Result(const Config& config, QueryHandle& qhandle, const char* docsum, size_t docsum_len, uint32_t langid) : - _qhandle(qhandle), - _mo(qhandle->MatchObj(langid)), + _qhandle(&qhandle), + _mo(qhandle.MatchObj(langid)), _docsum(docsum), _docsum_len(docsum_len), _langid(langid), - _config(config), + _config(&config), _matcher(), _tokenizer(), _summaries(), @@ -50,8 +50,8 @@ Result::Result(Config* config, QueryHandle* qhandle, { if (!_mo) return; // The empty result.. - MatcherParams& mp = _config->_matcherparams; - Fast_WordFolder* wordfolder = mp.WordFolder(); + const MatcherParams& mp = _config->_matcherparams; + const Fast_WordFolder* wordfolder = mp.WordFolder(); if (_qhandle->_stem_min < 0) _stem_min = mp.StemMinLength(); @@ -87,8 +87,8 @@ Result::Result(Config* config, QueryHandle* qhandle, _registry = std::make_unique<SpecialTokenRegistry>(_matcher->getQuery()); - if (qhandle->_log_mask) - _matcher->set_log(qhandle->_log_mask); + if (qhandle._log_mask) + _matcher->set_log(qhandle._log_mask); _tokenizer->SetSuccessor(_matcher.get()); if (!_registry->getSpecialTokens().empty()) { @@ -157,7 +157,7 @@ Summary* Result::GetTeaser(const Config* alt_config) const char *src_end = _docsum + _docsum_len; ucs4_t *dst = buf; ucs4_t *dst_end = dst + TOKEN_DSTLEN; - Fast_WordFolder *folder = _config->_matcherparams.WordFolder(); + const Fast_WordFolder *folder = _config->_matcherparams.WordFolder(); text.reserve(_dynsum_len*2); if (src_end - src <= _dynsum_len) { diff --git a/searchsummary/src/vespa/juniper/result.h b/searchsummary/src/vespa/juniper/result.h index f0dcf3d4335..015291cb4ef 100644 --- a/searchsummary/src/vespa/juniper/result.h +++ b/searchsummary/src/vespa/juniper/result.h @@ -14,7 +14,7 @@ namespace juniper class Result { public: - Result(Config* config, QueryHandle* qhandle, + Result(const Config& config, QueryHandle& qhandle, const char* docsum, size_t docsum_len, uint32_t langid); ~Result(); @@ -42,7 +42,7 @@ public: const char* _docsum; size_t _docsum_len; uint32_t _langid; - Config* _config; + const Config* _config; std::unique_ptr<Matcher> _matcher; std::unique_ptr<SpecialTokenRegistry> _registry; std::unique_ptr<JuniperTokenizer> _tokenizer; diff --git a/searchsummary/src/vespa/juniper/rpinterface.cpp b/searchsummary/src/vespa/juniper/rpinterface.cpp index f9e91073a9b..32ea7759170 100644 --- a/searchsummary/src/vespa/juniper/rpinterface.cpp +++ b/searchsummary/src/vespa/juniper/rpinterface.cpp @@ -79,9 +79,9 @@ std::unique_ptr<Config> Juniper::CreateConfig(const char* config_name) return std::unique_ptr<Config>(new Config(config_name, *this)); } -QueryHandle* Juniper::CreateQueryHandle(const IQuery& fquery, const char* juniperoptions) +std::unique_ptr<QueryHandle> Juniper::CreateQueryHandle(const IQuery& fquery, const char* juniperoptions) { - return new QueryHandle(fquery, juniperoptions, *_modifier); + return std::make_unique<QueryHandle>(fquery, juniperoptions, *_modifier); } void Juniper::AddRewriter(const char* index_name, IRewriter* rewriter, bool for_query, bool for_document) @@ -95,44 +95,29 @@ void Juniper::FlushRewriters() } -void ReleaseQueryHandle(QueryHandle*& handle) -{ - delete handle; - handle = NULL; -} - - -Result* Analyse(const Config* config, QueryHandle* qhandle, +std::unique_ptr<Result> Analyse(const Config& config, QueryHandle& qhandle, const char* docsum, size_t docsum_len, uint32_t docid, uint32_t /* inputfield_id */, uint32_t langid) { LOG(debug, "juniper::Analyse(): docId(%u), docsumLen(%zu), docsum(%s), langId(%u)", docid, docsum_len, docsum, langid); - Result* res = new Result(const_cast<Config*>(config), qhandle, docsum, docsum_len, langid); - return res; -} - -long GetRelevancy(Result* result_handle) -{ - return result_handle->GetRelevancy(); + return std::make_unique<Result>(config, qhandle, docsum, docsum_len, langid); } -Summary* GetTeaser(Result* result_handle, const Config* alt_config) +long GetRelevancy(Result& result_handle) { - return result_handle->GetTeaser(alt_config); + return result_handle.GetRelevancy(); } -Summary* GetLog(Result* result_handle) +Summary* GetTeaser(Result& result_handle, const Config* alt_config) { - return result_handle->GetLog(); + return result_handle.GetTeaser(alt_config); } -void ReleaseResult(Result*& result_handle) +Summary* GetLog(Result& result_handle) { - LOG(debug, "juniper::ReleaseResult"); - delete result_handle; - result_handle = NULL; + return result_handle.GetLog(); } } // end namespace juniper diff --git a/searchsummary/src/vespa/juniper/rpinterface.h b/searchsummary/src/vespa/juniper/rpinterface.h index 6cda324ae5c..d4ef5c5ebed 100644 --- a/searchsummary/src/vespa/juniper/rpinterface.h +++ b/searchsummary/src/vespa/juniper/rpinterface.h @@ -120,9 +120,9 @@ public: * behaviour such as user customization of teaser parameters, selectively * enabling of Juniper debugging/tracing features and to support Juniper extensions * to the query language. - * @return An allocated handle to be subsequently released by ReleaseQueryHandle() + * @return A unique pointer to a QueryHandle. */ - QueryHandle* CreateQueryHandle(const IQuery& query, const char* juniperoptions); + std::unique_ptr<QueryHandle> CreateQueryHandle(const IQuery& query, const char* juniperoptions); /** Add an rewriter for all terms that are prefixed with the given index. * When Juniper encounter a term in the query tagged with this index, @@ -150,11 +150,6 @@ private: */ bool AnalyseCompatible(Config* conf1, Config* conf2); -/** Release a QueryHandle as previously allocated by CreateQueryHandle. - * @param handle The QueryHandle object to release - */ -void ReleaseQueryHandle(QueryHandle*& handle); - /** Perform initial content analysis on a query/content pair. * Note that the content may either be a simple UTF-8 encoded string or a * more advanced representation including document structure elements, as provided @@ -170,10 +165,9 @@ void ReleaseQueryHandle(QueryHandle*& handle); within the document that contains the provided document summary. * @param langid A unique 32 bit id representing the language which this document summary is to be analysed in context of. - * @return A pointer to an allocated handle to be used in subsequent specific result - * requests (must later be released with ReleaseResult()) + * @return A unique pointer to a Result */ -Result* Analyse(const Config* config, QueryHandle* query, +std::unique_ptr<Result> Analyse(const Config& config, QueryHandle& query, const char* docsum, size_t docsum_len, uint32_t docid, uint32_t inputfield_id, uint32_t langid); @@ -182,17 +176,16 @@ Result* Analyse(const Config* config, QueryHandle* query, * @param result_handle The result to retrieve from * @return The relevancy (proximitymetric) of the processed content. */ -long GetRelevancy(Result* result_handle); +long GetRelevancy(Result& result_handle); /** Generate a teaser based on the provided analysis result * @param result_handle a handle obtained by a previous call to Analyse * @param alt_config An optional alternate config to use for this teaser generation * The purpose of alt_config is to allow generation of multiple teasers * based on the same content and analysis. - * @return The generated Teaser object. This object is valid until ReleaseResult - * is called for result_handle + * @return The generated Teaser object. This object is valid until result_handle is deleted. */ -Summary* GetTeaser(Result* result_handle, const Config* alt_config = NULL); +Summary* GetTeaser(Result& result_handle, const Config* alt_config = NULL); /** Retrieve log information based on the previous calls to this result handle. * Note that for the log to be complete, the juniper log override entry in @@ -200,15 +193,9 @@ Summary* GetTeaser(Result* result_handle, const Config* alt_config = NULL); * @param result_handle a handle obtained by a previous call to Analyse. * @return value: a summary description containing the Juniper log as a text field * if any log information is available, or else an empty summary. - * This object is valid until ReleaseResult is called for result_handle - */ -Summary* GetLog(Result* result_handle); - -/** Release all resources associated with the handle given including the - * summaries created by this result handle. - * @param result_handle The handle to release + * This object is valid until result_handle is deleted. */ -void ReleaseResult(Result*& result_handle); +Summary* GetLog(Result& result_handle); } // end namespace juniper diff --git a/searchsummary/src/vespa/juniper/tokenizer.cpp b/searchsummary/src/vespa/juniper/tokenizer.cpp index db6e1ecfccd..9253f81cf25 100644 --- a/searchsummary/src/vespa/juniper/tokenizer.cpp +++ b/searchsummary/src/vespa/juniper/tokenizer.cpp @@ -7,7 +7,7 @@ #include <vespa/log/log.h> LOG_SETUP(".juniper.tokenizer"); -JuniperTokenizer::JuniperTokenizer(Fast_WordFolder* wordfolder, +JuniperTokenizer::JuniperTokenizer(const Fast_WordFolder* wordfolder, const char* text, size_t len, ITokenProcessor* successor, const juniper::SpecialTokenRegistry * registry) : _wordfolder(wordfolder), _text(text), _len(len), _successor(successor), _registry(registry), diff --git a/searchsummary/src/vespa/juniper/tokenizer.h b/searchsummary/src/vespa/juniper/tokenizer.h index 34ed1dba5bb..bf0c9452665 100644 --- a/searchsummary/src/vespa/juniper/tokenizer.h +++ b/searchsummary/src/vespa/juniper/tokenizer.h @@ -11,7 +11,7 @@ class Fast_WordFolder; class JuniperTokenizer { public: - JuniperTokenizer(Fast_WordFolder* wordfolder, + JuniperTokenizer(const Fast_WordFolder* wordfolder, const char* text, size_t len, ITokenProcessor* = NULL, const juniper::SpecialTokenRegistry * registry = NULL); inline void SetSuccessor(ITokenProcessor* successor) { _successor = successor; } @@ -22,7 +22,7 @@ public: // Scan the input and dispatch to the successor void scan(); private: - Fast_WordFolder* _wordfolder; + const Fast_WordFolder* _wordfolder; const char* _text; // The current input text size_t _len; // Length of the text input ITokenProcessor* _successor; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp index 16575a2e9dc..9c810386703 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp @@ -4,6 +4,7 @@ #include "docsum_field_writer_state.h" #include <vespa/juniper/rpinterface.h> #include <vespa/document/datatype/positiondatatype.h> +#include <vespa/juniper/queryhandle.h> #include <vespa/searchcommon/attribute/iattributecontext.h> #include <vespa/searchlib/common/geo_location.h> #include <vespa/searchlib/common/geo_location_parser.h> @@ -36,24 +37,10 @@ GetDocsumsState::GetDocsumsState(GetDocsumsStateCallback &callback) _rankFeatures(nullptr), _matching_elements() { - _dynteaser._docid = static_cast<uint32_t>(-1); - _dynteaser._input = static_cast<uint32_t>(-1); - _dynteaser._lang = static_cast<uint32_t>(-1); - _dynteaser._config = nullptr; - _dynteaser._query = nullptr; - _dynteaser._result = nullptr; } -GetDocsumsState::~GetDocsumsState() -{ - if (_dynteaser._result != nullptr) { - juniper::ReleaseResult(_dynteaser._result); - } - if (_dynteaser._query != nullptr) { - juniper::ReleaseQueryHandle(_dynteaser._query); - } -} +GetDocsumsState::~GetDocsumsState() = default; const MatchingElements & GetDocsumsState::get_matching_elements(const MatchingElementsFields &matching_elems_fields) diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h index 438a8a6d847..adcd47c098c 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h @@ -57,12 +57,7 @@ public: GetDocsumsStateCallback &_callback; struct DynTeaserState { - uint32_t _docid; // document id ('cache key') - uint32_t _input; // input field ('cache key') - uint32_t _lang; // lang field ('cache key') - juniper::Config *_config; // juniper config ('cache key') - juniper::QueryHandle *_query; // juniper query representation - juniper::Result *_result; // juniper analyze result + std::unique_ptr<juniper::QueryHandle> _query; // juniper query representation } _dynteaser; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp index d9878cd2057..58dde39c336 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/dynamicteaserdfw.cpp @@ -9,6 +9,8 @@ #include <vespa/searchlib/queryeval/split_float.h> #include <vespa/vespalib/objects/hexdump.h> #include <vespa/juniper/config.h> +#include <vespa/juniper/queryhandle.h> +#include <vespa/juniper/result.h> #include <sstream> #include <vespa/log/log.h> @@ -368,27 +370,17 @@ DynamicTeaserDFW::getJuniperInput(GeneralResult *gres) { vespalib::string DynamicTeaserDFW::makeDynamicTeaser(uint32_t docid, vespalib::stringref input, GetDocsumsState *state) { - if (state->_dynteaser._query == nullptr) { + if (!state->_dynteaser._query) { JuniperQueryAdapter iq(state->_kwExtractor, state->_args.getStackDump(), &state->_args.highlightTerms()); state->_dynteaser._query = _juniper->CreateQueryHandle(iq, nullptr); } - LOG(debug, "makeDynamicTeaser: docid (%d,%d), fieldenum (%d,%d), lang (%d,%d) analyse %s", - docid, state->_dynteaser._docid, - _inputFieldEnumValue, state->_dynteaser._input, - _langFieldEnumValue, state->_dynteaser._lang, - (juniper::AnalyseCompatible(_juniperConfig.get(), state->_dynteaser._config) ? "no" : "yes")); + LOG(debug, "makeDynamicTeaser: docid (%d), fieldenum (%d), lang (%d)", + docid, _inputFieldEnumValue, _langFieldEnumValue); - if (state->_dynteaser._result != nullptr) - juniper::ReleaseResult(state->_dynteaser._result); - - state->_dynteaser._docid = docid; - state->_dynteaser._input = _inputFieldEnumValue; - state->_dynteaser._lang = _langFieldEnumValue; - state->_dynteaser._config = _juniperConfig.get(); - state->_dynteaser._result = nullptr; + std::unique_ptr<juniper::Result> result; if (state->_dynteaser._query != nullptr) { @@ -401,13 +393,12 @@ DynamicTeaserDFW::makeDynamicTeaser(uint32_t docid, vespalib::stringref input, G auto langid = static_cast<uint32_t>(-1); - state->_dynteaser._result = - juniper::Analyse(_juniperConfig.get(), state->_dynteaser._query, - input.data(), input.length(), docid, _inputFieldEnumValue, langid); + result = juniper::Analyse(*_juniperConfig, *state->_dynteaser._query, + input.data(), input.length(), docid, _inputFieldEnumValue, langid); } - juniper::Summary *teaser = (state->_dynteaser._result != nullptr) - ? juniper::GetTeaser(state->_dynteaser._result, _juniperConfig.get()) + juniper::Summary *teaser = result + ? juniper::GetTeaser(*result, _juniperConfig.get()) : nullptr; if (LOG_WOULD_LOG(debug)) { diff --git a/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp b/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp index 13e5ad1c84b..706325a0f7a 100644 --- a/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/rankmanager.cpp @@ -21,6 +21,7 @@ using search::fef::RankSetup; using vsm::VsmfieldsHandle; using vsm::VSMAdapter; using vsm::FieldIdTList; +using vespalib::make_string_short::fmt; namespace streaming { @@ -115,10 +116,10 @@ RankManager::Snapshot::initRankSetup(const BlueprintFactory & factory) for (uint32_t i = 0; i < _indexEnv.size(); ++i) { IndexEnvironment & ie = _indexEnv[i]; - RankSetup::SP rs(new RankSetup(factory, ie)); + auto rs = std::make_shared<RankSetup>(factory, ie); rs->configure(); // reads config values from the property map if (!rs->compile()) { - LOG(warning, "Could not compile rank setup for rank profile '%u'.", i); + LOG(warning, "Could not compile rank setup for rank profile '%u'. Errors = %s", i, rs->getJoinedWarnings().c_str()); return false; } _rankSetup.push_back(rs); @@ -127,7 +128,7 @@ RankManager::Snapshot::initRankSetup(const BlueprintFactory & factory) LOG(debug, "Number of index environments and rank setups: %u", (uint32_t)_indexEnv.size()); LOG_ASSERT(_properties.size() == _rankSetup.size()); for (uint32_t i = 0; i < _properties.size(); ++i) { - vespalib::string number = vespalib::make_string("%u", i); + vespalib::string number = fmt("%u", i); _rpmap[number] = i; } for (uint32_t i = 0; i < _properties.size(); ++i) { |