diff options
33 files changed, 527 insertions, 308 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java index beba1cdb358..563c59630d1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java @@ -189,8 +189,12 @@ public class TenantController { } private void requireNonExistent(TenantName name) { + var tenant = get(name, true); + if (tenant.isPresent() && tenant.get().type().equals(Tenant.Type.deleted)) { + throw new IllegalArgumentException("Tenant '" + name + "' cannot be created, try a different name"); + } if (SystemApplication.TENANT.equals(name) - || get(name, true).isPresent() + || tenant.isPresent() // Underscores are allowed in existing tenant names, but tenants with - and _ cannot co-exist. E.g. // my-tenant cannot be created if my_tenant exists. || get(name.value().replace('-', '_')).isPresent()) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index e3292700432..fee97b4e157 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -925,7 +925,8 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1", POST).userIdentity(USER_ID) .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}") .oAuthCredentials(OKTA_CREDENTIALS), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Tenant 'tenant1' already exists\"}", 400); + """ + {"error-code":"BAD_REQUEST","message":"Tenant 'tenant1' cannot be created, try a different name"}""", 400); // Forget a deleted tenant diff --git a/dist/vespa.spec b/dist/vespa.spec index 70105aaa4fc..097be19689a 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -32,6 +32,9 @@ License: Commercial URL: http://vespa.ai Source0: vespa-%{version}.tar.gz +# Avoid auto requirement for perl +%global __requires_exclude_from vespa-fbench-result-filter.pl|%{_prefix}/lib/perl5 + %if 0%{?centos} || 0%{?rocky} BuildRequires: epel-release %endif @@ -225,7 +228,7 @@ BuildRequires: perl-Net-INET6Glue %endif BuildRequires: perl-Pod-Usage BuildRequires: perl-URI -Requires: valgrind +BuildRequires: valgrind Requires: xxhash Requires: xxhash-libs >= 0.8.0 Requires: gdb @@ -460,6 +463,18 @@ Requires: %{name}-base-libs = %{version}-%{release} Vespa - The open big data serving engine - tools +%package systemtest-tools + +Summary: Vespa - The open big data serving engine - tools for system tests + +Requires: %{name} = %{version}-%{release} +Requires: %{name}-base-libs = %{version}-%{release} +Requires: valgrind + +%description systemtest-tools + +Vespa - The open big data serving engine - tools for system tests + %package ann-benchmark Summary: Vespa - The open big data serving engine - ann-benchmark @@ -635,6 +650,7 @@ fi %exclude %{_prefix}/bin/vespa-destination %exclude %{_prefix}/bin/vespa-document-statistics %exclude %{_prefix}/bin/vespa-fbench +%exclude %{_prefix}/bin/vespa-fbench-result-filter.pl %exclude %{_prefix}/bin/vespa-feed-client %exclude %{_prefix}/bin/vespa-feeder %exclude %{_prefix}/bin/vespa-get @@ -644,6 +660,8 @@ fi %exclude %{_prefix}/bin/vespa-stat %exclude %{_prefix}/bin/vespa-security-env %exclude %{_prefix}/bin/vespa-summary-benchmark +%exclude %{_prefix}/bin/vespa-tensor-conformance +%exclude %{_prefix}/bin/vespa-tensor-instructions-benchmark %exclude %{_prefix}/bin/vespa-visit %exclude %{_prefix}/bin/vespa-visit-target %dir %{_prefix}/conf @@ -902,6 +920,16 @@ fi %dir %{_prefix}/lib/jars %{_prefix}/lib/jars/vespaclient-java-jar-with-dependencies.jar +%files systemtest-tools +%if %{_defattr_is_vespa_vespa} +%defattr(-,%{_vespa_user},%{_vespa_group},-) +%endif +%dir %{_prefix} +%dir %{_prefix}/bin +%{_prefix}/bin/vespa-fbench-result-filter.pl +%{_prefix}/bin/vespa-tensor-conformance +%{_prefix}/bin/vespa-tensor-instructions-benchmark + %files ann-benchmark %if %{_defattr_is_vespa_vespa} %defattr(-,%{_vespa_user},%{_vespa_group},-) diff --git a/searchsummary/CMakeLists.txt b/searchsummary/CMakeLists.txt index 6e970bdfc00..a5dc62da5c0 100644 --- a/searchsummary/CMakeLists.txt +++ b/searchsummary/CMakeLists.txt @@ -16,6 +16,7 @@ vespa_define_module( TESTS src/tests/docsummary + src/tests/docsummary/annotation_converter src/tests/docsummary/attribute_combiner src/tests/docsummary/attributedfw src/tests/docsummary/document_id_dfw diff --git a/searchsummary/src/tests/docsummary/CMakeLists.txt b/searchsummary/src/tests/docsummary/CMakeLists.txt index 26a2963809a..4cd12eb4db6 100644 --- a/searchsummary/src/tests/docsummary/CMakeLists.txt +++ b/searchsummary/src/tests/docsummary/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchsummary_positionsdfw_test_app TEST positionsdfw_test.cpp DEPENDS searchsummary + GTest::GTest ) vespa_add_test(NAME searchsummary_positionsdfw_test_app COMMAND searchsummary_positionsdfw_test_app) diff --git a/searchsummary/src/tests/docsummary/annotation_converter/CMakeLists.txt b/searchsummary/src/tests/docsummary/annotation_converter/CMakeLists.txt new file mode 100644 index 00000000000..22e0d3e6477 --- /dev/null +++ b/searchsummary/src/tests/docsummary/annotation_converter/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchsummary_annotation_converter_test_app TEST + SOURCES + annotation_converter_test.cpp + DEPENDS + searchsummary + GTest::GTest +) +vespa_add_test(NAME searchsummary_annotation_converter_test_app COMMAND searchsummary_annotation_converter_test_app) diff --git a/searchsummary/src/tests/docsummary/annotation_converter/annotation_converter_test.cpp b/searchsummary/src/tests/docsummary/annotation_converter/annotation_converter_test.cpp new file mode 100644 index 00000000000..753ae8d9044 --- /dev/null +++ b/searchsummary/src/tests/docsummary/annotation_converter/annotation_converter_test.cpp @@ -0,0 +1,176 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/document/annotation/annotation.h> +#include <vespa/document/annotation/span.h> +#include <vespa/document/annotation/spanlist.h> +#include <vespa/document/annotation/spantree.h> +#include <vespa/document/fieldvalue/stringfieldvalue.h> +#include <vespa/document/repo/configbuilder.h> +#include <vespa/document/repo/fixedtyperepo.h> +#include <vespa/juniper/juniper_separators.h> +#include <vespa/searchsummary/docsummary/annotation_converter.h> +#include <vespa/searchsummary/docsummary/i_juniper_converter.h> +#include <vespa/searchsummary/docsummary/linguisticsannotation.h> +#include <vespa/vespalib/data/slime/slime.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/stllike/asciistream.h> + +using document::Annotation; +using document::DocumentType; +using document::DocumentTypeRepo; +using document::Span; +using document::SpanList; +using document::SpanTree; +using document::StringFieldValue; +using search::docsummary::AnnotationConverter; +using search::docsummary::IJuniperConverter; +using search::linguistics::SPANTREE_NAME; +using search::linguistics::TERM; +using vespalib::Slime; +using vespalib::slime::SlimeInserter; + +namespace { + +DocumenttypesConfig +get_document_types_config() +{ + using namespace document::config_builder; + DocumenttypesConfigBuilderHelper builder; + builder.document(42, "indexingdocument", + Struct("indexingdocument.header"), + Struct("indexingdocument.body")); + return builder.config(); +} + +class MockJuniperConverter : public IJuniperConverter +{ + vespalib::string _result; +public: + void convert(vespalib::stringref input, vespalib::slime::Inserter&) override { + _result = input; + } + const vespalib::string& get_result() const noexcept { return _result; } +}; + +} + +class AnnotationConverterTest : public testing::Test +{ +protected: + std::shared_ptr<const DocumentTypeRepo> _repo; + const DocumentType* _document_type; + document::FixedTypeRepo _fixed_repo; + + AnnotationConverterTest(); + ~AnnotationConverterTest() override; + void set_span_tree(StringFieldValue& value, std::unique_ptr<SpanTree> tree); + StringFieldValue make_annotated_string(); + StringFieldValue make_annotated_chinese_string(); + vespalib::string make_exp_il_annotated_string(); + vespalib::string make_exp_il_annotated_chinese_string(); + void expect_annotated(const vespalib::string& exp, const StringFieldValue& fv); +}; + +AnnotationConverterTest::AnnotationConverterTest() + : testing::Test(), + _repo(std::make_unique<DocumentTypeRepo>(get_document_types_config())), + _document_type(_repo->getDocumentType("indexingdocument")), + _fixed_repo(*_repo, *_document_type) +{ +} + +AnnotationConverterTest::~AnnotationConverterTest() = default; + +void +AnnotationConverterTest::set_span_tree(StringFieldValue & value, std::unique_ptr<SpanTree> tree) +{ + StringFieldValue::SpanTrees trees; + trees.push_back(std::move(tree)); + value.setSpanTrees(trees, _fixed_repo); +} + +StringFieldValue +AnnotationConverterTest::make_annotated_string() +{ + auto span_list_up = std::make_unique<SpanList>(); + auto span_list = span_list_up.get(); + auto tree = std::make_unique<SpanTree>(SPANTREE_NAME, std::move(span_list_up)); + tree->annotate(span_list->add(std::make_unique<Span>(0, 3)), *TERM); + tree->annotate(span_list->add(std::make_unique<Span>(4, 3)), + Annotation(*TERM, std::make_unique<StringFieldValue>("baz"))); + StringFieldValue value("foo bar"); + set_span_tree(value, std::move(tree)); + return value; +} + +StringFieldValue +AnnotationConverterTest::make_annotated_chinese_string() +{ + auto span_list_up = std::make_unique<SpanList>(); + auto span_list = span_list_up.get(); + auto tree = std::make_unique<SpanTree>(SPANTREE_NAME, std::move(span_list_up)); + // These chinese characters each use 3 bytes in their UTF8 encoding. + tree->annotate(span_list->add(std::make_unique<Span>(0, 15)), *TERM); + tree->annotate(span_list->add(std::make_unique<Span>(15, 9)), *TERM); + StringFieldValue value("我就是那个大灰狼"); + set_span_tree(value, std::move(tree)); + return value; +} + +vespalib::string +AnnotationConverterTest::make_exp_il_annotated_string() +{ + using namespace juniper::separators; + vespalib::asciistream exp; + exp << "foo" << unit_separator_string << + " " << unit_separator_string << interlinear_annotation_anchor_string << + "bar" << interlinear_annotation_separator_string << + "baz" << interlinear_annotation_terminator_string << unit_separator_string; + return exp.str(); +} + +vespalib::string +AnnotationConverterTest::make_exp_il_annotated_chinese_string() +{ + using namespace juniper::separators; + vespalib::asciistream exp; + exp << "我就是那个" << unit_separator_string << + "大灰狼" << unit_separator_string; + return exp.str(); +} + +void +AnnotationConverterTest::expect_annotated(const vespalib::string& exp, const StringFieldValue& fv) +{ + MockJuniperConverter juniper_converter; + AnnotationConverter annotation_converter(juniper_converter); + Slime slime; + SlimeInserter inserter(slime); + annotation_converter.convert(fv, inserter); + EXPECT_EQ(exp, juniper_converter.get_result()); +} + + +TEST_F(AnnotationConverterTest, convert_plain_string) +{ + using namespace juniper::separators; + vespalib::string exp("Foo Bar Baz"); + StringFieldValue plain_string("Foo Bar Baz"); + expect_annotated(exp + unit_separator_string, plain_string); +} + +TEST_F(AnnotationConverterTest, convert_annotated_string) +{ + auto exp = make_exp_il_annotated_string(); + auto annotated_string = make_annotated_string(); + expect_annotated(exp, annotated_string); +} + +TEST_F(AnnotationConverterTest, convert_annotated_chinese_string) +{ + auto exp = make_exp_il_annotated_chinese_string(); + auto annotated_chinese_string = make_annotated_chinese_string(); + expect_annotated(exp, annotated_chinese_string); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchsummary/src/tests/docsummary/positionsdfw_test.cpp b/searchsummary/src/tests/docsummary/positionsdfw_test.cpp index b4e82ed2aa8..f23bd2f0437 100644 --- a/searchsummary/src/tests/docsummary/positionsdfw_test.cpp +++ b/searchsummary/src/tests/docsummary/positionsdfw_test.cpp @@ -1,17 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -// Unit tests for positionsdfw. +#include <vespa/juniper/rpinterface.h> #include <vespa/searchlib/attribute/extendableattributes.h> #include <vespa/searchlib/attribute/iattributemanager.h> #include <vespa/searchlib/common/matching_elements.h> #include <vespa/searchsummary/docsummary/docsum_field_writer.h> -#include <vespa/searchsummary/docsummary/positionsdfw.h> -#include <vespa/searchsummary/docsummary/idocsumenvironment.h> #include <vespa/searchsummary/docsummary/docsumstate.h> +#include <vespa/searchsummary/docsummary/idocsumenvironment.h> +#include <vespa/searchsummary/docsummary/positionsdfw.h> #include <vespa/searchsummary/test/slime_value.h> -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/data/slime/slime.h> -#include <vespa/juniper/rpinterface.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> LOG_SETUP("positionsdfw_test"); @@ -29,33 +28,6 @@ namespace search::docsummary { namespace { -class Test : public vespalib::TestApp { - void requireThat2DPositionFieldIsWritten(); - -public: - int Main() override; -}; - -int -Test::Main() -{ - TEST_INIT("positionsdfw_test"); - - TEST_DO(requireThat2DPositionFieldIsWritten()); - - TEST_DONE(); -} - -struct MyEnvironment : IDocsumEnvironment { - IAttributeManager *attribute_man; - - MyEnvironment() : attribute_man(0) {} - - const IAttributeManager *getAttributeManager() const override { return attribute_man; } - string lookupIndex(const string &s) const override { return s; } - const juniper::Juniper *getJuniper() const override { return nullptr; } -}; - class MyAttributeContext : public IAttributeContext { const IAttributeVector &_attr; public: @@ -141,10 +113,13 @@ void checkWritePositionField(AttrType &attr, writer->insertField(doc_id, state, inserter); test::SlimeValue expected(expect_json); - EXPECT_EQUAL(expected.slime, target); + EXPECT_EQ(expected.slime, target); } -void Test::requireThat2DPositionFieldIsWritten() { +} // namespace + +TEST(PositionsDFWTest, require_that_2D_position_field_is_written) +{ SingleInt64ExtAttribute attr("foo"); checkWritePositionField(attr, 0x3e, "{x:6,y:7,latlong:'N0.000007;E0.000006'}"); checkWritePositionField(attr, 007, "{x:-1,y:-1,latlong:'S0.000001;W0.000001'}"); @@ -153,7 +128,6 @@ void Test::requireThat2DPositionFieldIsWritten() { checkWritePositionField(attr, 42, "null"); } -} // namespace } -TEST_APPHOOK(search::docsummary::Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp b/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp index 4ebdfb10cb7..505386f5b91 100644 --- a/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp +++ b/searchsummary/src/tests/docsummary/slime_filler/slime_filler_test.cpp @@ -1,12 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/document/annotation/annotation.h> -#include <vespa/document/annotation/span.h> -#include <vespa/document/annotation/spanlist.h> -#include <vespa/document/annotation/spantree.h> #include <vespa/document/base/documentid.h> #include <vespa/document/datatype/documenttype.h> -#include <vespa/document/datatype/urldatatype.h> #include <vespa/document/datatype/referencedatatype.h> #include <vespa/document/datatype/tensor_data_type.h> #include <vespa/document/fieldvalue/arrayfieldvalue.h> @@ -31,25 +26,18 @@ #include <vespa/eval/eval/tensor_spec.h> #include <vespa/eval/eval/value.h> #include <vespa/eval/eval/value_codec.h> -#include <vespa/juniper/juniper_separators.h> -#include <vespa/searchsummary/docsummary/annotation_converter.h> -#include <vespa/searchsummary/docsummary/docsum_field_writer.h> -#include <vespa/searchsummary/docsummary/i_docsum_field_writer_factory.h> -#include <vespa/searchsummary/docsummary/i_juniper_converter.h> #include <vespa/searchsummary/docsummary/i_string_field_converter.h> #include <vespa/searchsummary/docsummary/linguisticsannotation.h> #include <vespa/searchsummary/docsummary/resultconfig.h> #include <vespa/searchsummary/docsummary/slime_filler.h> +#include <vespa/searchsummary/docsummary/slime_filler_filter.h> #include <vespa/vespalib/data/slime/binary_format.h> #include <vespa/vespalib/data/slime/json_format.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/simple_buffer.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/stllike/asciistream.h> -#include <vespa/vespalib/util/size_literals.h> -#include <vespa/config-summary.h> -using document::Annotation; using document::ArrayFieldValue; using document::BoolFieldValue; using document::ByteFieldValue; @@ -71,23 +59,16 @@ using document::RawFieldValue; using document::ReferenceDataType; using document::ReferenceFieldValue; using document::ShortFieldValue; -using document::Span; -using document::SpanList; -using document::SpanTree; using document::StringFieldValue; using document::StructDataType; using document::StructFieldValue; using document::TensorDataType; using document::TensorFieldValue; -using document::UrlDataType; using document::WeightedSetFieldValue; -using search::docsummary::AnnotationConverter; -using search::docsummary::IDocsumFieldWriterFactory; -using search::docsummary::IJuniperConverter; using search::docsummary::IStringFieldConverter; -using search::docsummary::DocsumFieldWriter; using search::docsummary::ResultConfig; using search::docsummary::SlimeFiller; +using search::docsummary::SlimeFillerFilter; using search::linguistics::SPANTREE_NAME; using search::linguistics::TERM; using vespalib::SimpleBuffer; @@ -99,7 +80,6 @@ using vespalib::eval::ValueType; using vespalib::slime::Cursor; using vespalib::slime::JsonFormat; using vespalib::slime::SlimeInserter; -using vespa::config::search::SummaryConfigBuilder; namespace { @@ -118,15 +98,6 @@ slime_to_string(const Slime& slime) } vespalib::string -make_slime_string(vespalib::stringref value) -{ - Slime slime; - SlimeInserter inserter(slime); - inserter.insertString({value}); - return slime_to_string(slime); -} - -vespalib::string make_slime_data_string(vespalib::stringref data) { Slime slime; @@ -143,15 +114,6 @@ make_slime_tensor_string(const Value& value) return make_slime_data_string({s.peek(), s.size()}); } -class MockDocsumFieldWriterFactory : public IDocsumFieldWriterFactory -{ -public: - std::unique_ptr<DocsumFieldWriter> create_docsum_field_writer(const vespalib::string&, const vespalib::string&, const vespalib::string&, bool&) override { - return {}; - } - -}; - DocumenttypesConfig get_document_types_config() { @@ -176,35 +138,28 @@ get_document_types_config() .addField("d", nested_type_id) .addField("e", nested_type_id) .addField("f", nested_type_id)) + .addField("nested_array", Array(nested_type_id)) + .addField("nested_map", Map(DataType::T_STRING, nested_type_id)) .addField("ref", ref_type_id), Struct("indexingdocument.body")) .referenceType(ref_type_id, ref_target_doctype_id); return builder.config(); } -class MockJuniperConverter : public IJuniperConverter +class MockStringFieldConverter : public IStringFieldConverter { vespalib::string _result; public: - void convert(vespalib::stringref input, vespalib::slime::Inserter&) override { - _result = input; - } - const vespalib::string& get_result() const noexcept { return _result; } -}; - -class PassThroughStringFieldConverter : public IStringFieldConverter -{ - IJuniperConverter& _juniper_converter; -public: - PassThroughStringFieldConverter(IJuniperConverter& juniper_converter) + MockStringFieldConverter() : IStringFieldConverter(), - _juniper_converter(juniper_converter) + _result() { } - ~PassThroughStringFieldConverter() override = default; - void convert(const document::StringFieldValue& input, vespalib::slime::Inserter& inserter) override { - _juniper_converter.convert(input.getValueRef(), inserter); + ~MockStringFieldConverter() override = default; + void convert(const document::StringFieldValue& input, vespalib::slime::Inserter&) override { + _result = input.getValueRef(); } + const vespalib::string& get_result() const noexcept { return _result; } }; } @@ -214,31 +169,26 @@ class SlimeFillerTest : public testing::Test protected: std::shared_ptr<const DocumentTypeRepo> _repo; const DocumentType* _document_type; - document::FixedTypeRepo _fixed_repo; SlimeFillerTest(); ~SlimeFillerTest() override; const DataType& get_data_type(const vespalib::string& name) const; const ReferenceDataType& get_as_ref_type(const vespalib::string& name) const; - void set_span_tree(StringFieldValue& value, std::unique_ptr<SpanTree> tree); - StringFieldValue make_annotated_string(); - StringFieldValue make_annotated_chinese_string(); - vespalib::string make_exp_il_annotated_string(); - vespalib::string make_exp_il_annotated_chinese_string(); ArrayFieldValue make_array(); WeightedSetFieldValue make_weighted_set(); MapFieldValue make_map(); + StructFieldValue make_nested_value(int i); void expect_insert(const vespalib::string& exp, const FieldValue& fv, const std::vector<uint32_t>* matching_elems); void expect_insert(const vespalib::string& exp, const FieldValue& fv); void expect_insert_filtered(const vespalib::string& exp, const FieldValue& fv, const std::vector<uint32_t>& matching_elems); - void expect_insert_callback(const vespalib::string& exp, const FieldValue& fv, bool tokenize); + void expect_insert(const vespalib::string& exp, const FieldValue& fv, SlimeFillerFilter& filter); + void expect_insert_callback(const vespalib::string& exp, const FieldValue& fv); }; SlimeFillerTest::SlimeFillerTest() : testing::Test(), _repo(std::make_unique<DocumentTypeRepo>(get_document_types_config())), - _document_type(_repo->getDocumentType("indexingdocument")), - _fixed_repo(*_repo, *_document_type) + _document_type(_repo->getDocumentType("indexingdocument")) { } @@ -257,64 +207,6 @@ SlimeFillerTest::get_as_ref_type(const vespalib::string& name) const { return dynamic_cast<const ReferenceDataType&>(get_data_type(name)); } -void -SlimeFillerTest::set_span_tree(StringFieldValue & value, std::unique_ptr<SpanTree> tree) -{ - StringFieldValue::SpanTrees trees; - trees.push_back(std::move(tree)); - value.setSpanTrees(trees, _fixed_repo); -} - -StringFieldValue -SlimeFillerTest::make_annotated_string() -{ - auto span_list_up = std::make_unique<SpanList>(); - auto span_list = span_list_up.get(); - auto tree = std::make_unique<SpanTree>(SPANTREE_NAME, std::move(span_list_up)); - tree->annotate(span_list->add(std::make_unique<Span>(0, 3)), *TERM); - tree->annotate(span_list->add(std::make_unique<Span>(4, 3)), - Annotation(*TERM, std::make_unique<StringFieldValue>("baz"))); - StringFieldValue value("foo bar"); - set_span_tree(value, std::move(tree)); - return value; -} - -StringFieldValue -SlimeFillerTest::make_annotated_chinese_string() -{ - auto span_list_up = std::make_unique<SpanList>(); - auto span_list = span_list_up.get(); - auto tree = std::make_unique<SpanTree>(SPANTREE_NAME, std::move(span_list_up)); - // These chinese characters each use 3 bytes in their UTF8 encoding. - tree->annotate(span_list->add(std::make_unique<Span>(0, 15)), *TERM); - tree->annotate(span_list->add(std::make_unique<Span>(15, 9)), *TERM); - StringFieldValue value("我就是那个大灰狼"); - set_span_tree(value, std::move(tree)); - return value; -} - -vespalib::string -SlimeFillerTest::make_exp_il_annotated_string() -{ - using namespace juniper::separators; - vespalib::asciistream exp; - exp << "foo" << unit_separator_string << - " " << unit_separator_string << interlinear_annotation_anchor_string << - "bar" << interlinear_annotation_separator_string << - "baz" << interlinear_annotation_terminator_string << unit_separator_string; - return exp.str(); -} - -vespalib::string -SlimeFillerTest::make_exp_il_annotated_chinese_string() -{ - using namespace juniper::separators; - vespalib::asciistream exp; - exp << "我就是那个" << unit_separator_string << - "大灰狼" << unit_separator_string; - return exp.str(); -} - ArrayFieldValue SlimeFillerTest::make_array() { @@ -345,6 +237,21 @@ SlimeFillerTest::make_map() return map; } +StructFieldValue +SlimeFillerTest::make_nested_value(int i) +{ + StructFieldValue nested(get_data_type("nested")); + StructFieldValue nested2(get_data_type("nested")); + nested.setValue("a", IntFieldValue(42 + 100 * i)); + nested.setValue("b", IntFieldValue(44 + 100 * i)); + nested.setValue("c", IntFieldValue(46 + 100 * i)); + nested2.setValue("a", IntFieldValue(62 + 100 * i)); + nested2.setValue("c", IntFieldValue(66 + 100 * i)); + nested.setValue("d", nested2); + nested.setValue("f", nested2); + return nested; +} + void SlimeFillerTest::expect_insert(const vespalib::string& exp, const FieldValue& fv, const std::vector<uint32_t>* matching_elems) { @@ -352,8 +259,6 @@ SlimeFillerTest::expect_insert(const vespalib::string& exp, const FieldValue& fv SlimeInserter inserter(slime); SlimeFiller filler(inserter, matching_elems); fv.accept(filler); - SimpleBuffer buf; - JsonFormat::encode(slime, buf, true); auto act = slime_to_string(slime); EXPECT_EQ(exp, act); } @@ -371,14 +276,23 @@ SlimeFillerTest::expect_insert(const vespalib::string& exp, const FieldValue& fv } void -SlimeFillerTest::expect_insert_callback(const vespalib::string& exp, const FieldValue& fv, bool tokenize) +SlimeFillerTest::expect_insert(const vespalib::string& exp, const FieldValue& fv, SlimeFillerFilter& filter) { Slime slime; SlimeInserter inserter(slime); - MockJuniperConverter converter; - AnnotationConverter annotation_converter(converter); - PassThroughStringFieldConverter passthrough_converter(converter); - SlimeFiller filler(inserter, tokenize ? (IStringFieldConverter*) &annotation_converter : (IStringFieldConverter*) &passthrough_converter); + SlimeFiller filler(inserter, nullptr, &filter); + fv.accept(filler); + auto act = slime_to_string(slime); + EXPECT_EQ(exp, act); +} + +void +SlimeFillerTest::expect_insert_callback(const vespalib::string& exp, const FieldValue& fv) +{ + Slime slime; + SlimeInserter inserter(slime); + MockStringFieldConverter converter; + SlimeFiller filler(inserter, &converter, nullptr); fv.accept(filler); auto act_null = slime_to_string(slime); EXPECT_EQ("null", act_null); @@ -426,15 +340,8 @@ TEST_F(SlimeFillerTest, insert_string) expect_insert(R"("Foo Bar Baz")", StringFieldValue("Foo Bar Baz")); } { - SCOPED_TRACE("annotated string"); - auto exp = make_exp_il_annotated_string(); - expect_insert(R"("foo bar")", make_annotated_string()); - } - { - SCOPED_TRACE("annotated chinese string"); - auto annotated_chinese_string = make_annotated_chinese_string(); - auto exp = annotated_chinese_string.getValue(); - expect_insert(make_slime_string(exp), annotated_chinese_string); + SCOPED_TRACE("empty string"); + expect_insert(R"("")", StringFieldValue()); } } @@ -581,43 +488,45 @@ TEST_F(SlimeFillerTest, insert_map_filtered) TEST_F(SlimeFillerTest, insert_struct) { - StructFieldValue nested(get_data_type("nested")); - StructFieldValue nested2(get_data_type("nested")); - nested.setValue("a", IntFieldValue(42)); - nested.setValue("b", IntFieldValue(44)); - nested.setValue("c", IntFieldValue(46)); - nested2.setValue("a", IntFieldValue(62)); - nested2.setValue("c", IntFieldValue(66)); - nested.setValue("d", nested2); - nested.setValue("f", nested2); - // Field order depends on assigned field ids, cf. document::Field::calculateIdV7() + auto nested = make_nested_value(0); + // Field order depends on assigned field ids, cf. document::Field::calculateIdV7(), and symbol insertion order in slime expect_insert(R"({"f":{"c":66,"a":62},"c":46,"a":42,"b":44,"d":{"c":66,"a":62}})", nested); + SlimeFillerFilter filter; + filter.add("a").add("c").add("f.a").add("d"); + expect_insert(R"({"f":{"a":62},"a":42,"c":46,"d":{"a":62,"c":66}})", nested, filter); } -TEST_F(SlimeFillerTest, insert_string_with_callback) +TEST_F(SlimeFillerTest, insert_struct_array) { - { - SCOPED_TRACE("plain string"); - using namespace juniper::separators; - vespalib::string exp("Foo Bar Baz"); - StringFieldValue plain_string("Foo Bar Baz"); - expect_insert_callback(exp + unit_separator_string, plain_string, true); - expect_insert_callback(exp, plain_string, false); - } - { - SCOPED_TRACE("annotated string"); - auto exp = make_exp_il_annotated_string(); - auto annotated_string = make_annotated_string(); - expect_insert_callback(exp, annotated_string, true); - expect_insert_callback("foo bar", annotated_string, false); + ArrayFieldValue array(get_data_type("Array<nested>")); + for (int i = 0; i < 3; ++i) { + array.add(make_nested_value(i)); } - { - SCOPED_TRACE("annotated chinese string"); - auto exp = make_exp_il_annotated_chinese_string(); - auto annotated_chinese_string = make_annotated_chinese_string(); - expect_insert_callback(exp, annotated_chinese_string, true); - expect_insert_callback(annotated_chinese_string.getValueRef(), annotated_chinese_string, false); + expect_insert(R"([{"f":{"c":66,"a":62},"c":46,"a":42,"b":44,"d":{"c":66,"a":62}},{"f":{"c":166,"a":162},"c":146,"a":142,"b":144,"d":{"c":166,"a":162}},{"f":{"c":266,"a":262},"c":246,"a":242,"b":244,"d":{"c":266,"a":262}}])", array); + SlimeFillerFilter filter; + filter.add("a").add("c").add("f.a").add("d"); + expect_insert(R"([{"f":{"a":62},"a":42,"c":46,"d":{"a":62,"c":66}},{"f":{"a":162},"a":142,"c":146,"d":{"a":162,"c":166}},{"f":{"a":262},"a":242,"c":246,"d":{"a":262,"c":266}}])", array, filter); +} + +TEST_F(SlimeFillerTest, insert_struct_map) +{ + MapFieldValue map(get_data_type("Map<String,nested>")); + for (int i = 0; i < 3; ++i) { + vespalib::asciistream key; + key << "key" << (i + 1); + map.put(StringFieldValue(key.str()), make_nested_value(i)); } + expect_insert(R"([{"key":"key1","value":{"f":{"c":66,"a":62},"c":46,"a":42,"b":44,"d":{"c":66,"a":62}}},{"key":"key2","value":{"f":{"c":166,"a":162},"c":146,"a":142,"b":144,"d":{"c":166,"a":162}}},{"key":"key3","value":{"f":{"c":266,"a":262},"c":246,"a":242,"b":244,"d":{"c":266,"a":262}}}])", map); + SlimeFillerFilter filter; + filter.add("value.a").add("value.c").add("value.f.a").add("value.d"); + expect_insert(R"([{"key":"key1","value":{"f":{"a":62},"a":42,"c":46,"d":{"a":62,"c":66}}},{"key":"key2","value":{"f":{"a":162},"a":142,"c":146,"d":{"a":162,"c":166}}},{"key":"key3","value":{"f":{"a":262},"a":242,"c":246,"d":{"a":262,"c":266}}}])", map, filter); +} + +TEST_F(SlimeFillerTest, insert_string_with_callback) +{ + vespalib::string exp("Foo Bar Baz"); + StringFieldValue plain_string(exp); + expect_insert_callback(exp, plain_string); } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchsummary/src/tests/docsummary/slime_summary/CMakeLists.txt b/searchsummary/src/tests/docsummary/slime_summary/CMakeLists.txt index 344a33952d6..26456dae395 100644 --- a/searchsummary/src/tests/docsummary/slime_summary/CMakeLists.txt +++ b/searchsummary/src/tests/docsummary/slime_summary/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchsummary_slime_summary_test_app TEST slime_summary_test.cpp DEPENDS searchsummary + GTest::GTest ) vespa_add_test(NAME searchsummary_slime_summary_test_app COMMAND searchsummary_slime_summary_test_app) diff --git a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp index 5f2f1578f66..fa53cf202ff 100644 --- a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp +++ b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp @@ -1,8 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + #include <vespa/document/base/documentid.h> #include <vespa/document/datatype/documenttype.h> -#include <vespa/document/fieldvalue/document.h> #include <vespa/document/fieldvalue/bytefieldvalue.h> +#include <vespa/document/fieldvalue/document.h> #include <vespa/document/fieldvalue/doublefieldvalue.h> #include <vespa/document/fieldvalue/floatfieldvalue.h> #include <vespa/document/fieldvalue/intfieldvalue.h> @@ -10,14 +11,14 @@ #include <vespa/document/fieldvalue/rawfieldvalue.h> #include <vespa/document/fieldvalue/shortfieldvalue.h> #include <vespa/document/fieldvalue/stringfieldvalue.h> -#include <vespa/vespalib/testkit/test_kit.h> #include <vespa/searchlib/common/matching_elements.h> -#include <vespa/searchsummary/docsummary/docsumwriter.h> +#include <vespa/searchsummary/docsummary/docsum_store_document.h> #include <vespa/searchsummary/docsummary/docsumstate.h> +#include <vespa/searchsummary/docsummary/docsumwriter.h> #include <vespa/searchsummary/docsummary/keywordextractor.h> -#include <vespa/searchsummary/docsummary/docsum_store_document.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/smart_buffer.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/util/size_literals.h> using namespace vespalib::slime::convenience; @@ -42,15 +43,15 @@ using document::StructFieldValue; namespace { -struct DocsumFixture : IDocsumStore, GetDocsumsStateCallback { +struct SlimeSummaryTest : testing::Test, IDocsumStore, GetDocsumsStateCallback { std::unique_ptr<DynamicDocsumWriter> writer; StructDataType int_pair_type; DocumentType doc_type; GetDocsumsState state; bool fail_get_mapped_docsum; bool empty_get_mapped_docsum; - DocsumFixture(); - ~DocsumFixture() override; + SlimeSummaryTest(); + ~SlimeSummaryTest() override; void getDocsum(Slime &slime) { Slime slimeOut; SlimeInserter inserter(slimeOut); @@ -58,11 +59,11 @@ struct DocsumFixture : IDocsumStore, GetDocsumsStateCallback { writer->insertDocsum(rci, 1u, state, this, inserter); vespalib::SmartBuffer buf(4_Ki); BinaryFormat::encode(slimeOut, buf); - EXPECT_GREATER(BinaryFormat::decode(buf.obtain(), slime), 0u); + EXPECT_GT(BinaryFormat::decode(buf.obtain(), slime), 0u); } uint32_t getNumDocs() const override { return 2; } std::unique_ptr<const IDocsumStoreDocument> getMappedDocsum(uint32_t docid) override { - EXPECT_EQUAL(1u, docid); + EXPECT_EQ(1u, docid); if (fail_get_mapped_docsum) { return {}; } @@ -94,7 +95,7 @@ struct DocsumFixture : IDocsumStore, GetDocsumsStateCallback { }; -DocsumFixture::DocsumFixture() +SlimeSummaryTest::SlimeSummaryTest() : writer(), int_pair_type("int_pair"), doc_type("test"), @@ -132,49 +133,54 @@ DocsumFixture::DocsumFixture() doc_type.addField(Field("longdata_field", *DataType::RAW)); doc_type.addField(Field("int_pair_field", int_pair_type)); } -DocsumFixture::~DocsumFixture() = default; +SlimeSummaryTest::~SlimeSummaryTest() = default; } // namespace <unnamed> -TEST_FF("require that docsum can be written as slime", DocsumFixture(), Slime()) { - f1.getDocsum(f2); - EXPECT_EQUAL(f2.get()["int_field"].asLong(), 4u); - EXPECT_EQUAL(f2.get()["short_field"].asLong(), 2u); - EXPECT_EQUAL(f2.get()["byte_field"].asLong(), 1u); - EXPECT_EQUAL(f2.get()["float_field"].asDouble(), 4.5); - EXPECT_EQUAL(f2.get()["double_field"].asDouble(), 8.75); - EXPECT_EQUAL(f2.get()["int64_field"].asLong(), 8u); - EXPECT_EQUAL(f2.get()["string_field"].asString().make_string(), std::string("string")); - EXPECT_EQUAL(f2.get()["data_field"].asData().make_string(), std::string("data")); - EXPECT_EQUAL(f2.get()["longstring_field"].asString().make_string(), std::string("long_string")); - EXPECT_EQUAL(f2.get()["longdata_field"].asData().make_string(), std::string("long_data")); - EXPECT_EQUAL(f2.get()["int_pair_field"]["foo"].asLong(), 1u); - EXPECT_EQUAL(f2.get()["int_pair_field"]["bar"].asLong(), 2u); +TEST_F(SlimeSummaryTest, docsum_can_be_written_as_slime) +{ + Slime s; + getDocsum(s); + EXPECT_EQ(s.get()["int_field"].asLong(), 4u); + EXPECT_EQ(s.get()["short_field"].asLong(), 2u); + EXPECT_EQ(s.get()["byte_field"].asLong(), 1u); + EXPECT_EQ(s.get()["float_field"].asDouble(), 4.5); + EXPECT_EQ(s.get()["double_field"].asDouble(), 8.75); + EXPECT_EQ(s.get()["int64_field"].asLong(), 8u); + EXPECT_EQ(s.get()["string_field"].asString().make_string(), std::string("string")); + EXPECT_EQ(s.get()["data_field"].asData().make_string(), std::string("data")); + EXPECT_EQ(s.get()["longstring_field"].asString().make_string(), std::string("long_string")); + EXPECT_EQ(s.get()["longdata_field"].asData().make_string(), std::string("long_data")); + EXPECT_EQ(s.get()["int_pair_field"]["foo"].asLong(), 1u); + EXPECT_EQ(s.get()["int_pair_field"]["bar"].asLong(), 2u); } -TEST_FF("require that unknown summary class gives empty slime", DocsumFixture(), Slime()) +TEST_F(SlimeSummaryTest, unknown_summary_class_gives_empty_slime) { - f1.state._args.setResultClassName("unknown"); - f1.getDocsum(f2); - EXPECT_TRUE(f2.get().valid()); - EXPECT_EQUAL(vespalib::slime::NIX::ID, f2.get().type().getId()); + state._args.setResultClassName("unknown"); + Slime s; + getDocsum(s); + EXPECT_TRUE(s.get().valid()); + EXPECT_EQ(vespalib::slime::NIX::ID, s.get().type().getId()); } -TEST_FF("require that failure to retrieve docsum store document gives empty slime", DocsumFixture(), Slime()) +TEST_F(SlimeSummaryTest, failure_to_retrieve_docsum_store_document_gives_empty_slime) { - f1.fail_get_mapped_docsum = true; - f1.getDocsum(f2); - EXPECT_TRUE(f2.get().valid()); - EXPECT_EQUAL(vespalib::slime::NIX::ID, f2.get().type().getId()); + fail_get_mapped_docsum = true; + Slime s; + getDocsum(s); + EXPECT_TRUE(s.get().valid()); + EXPECT_EQ(vespalib::slime::NIX::ID, s.get().type().getId()); } -TEST_FF("require that empty docsum store document gives empty object", DocsumFixture(), Slime()) +TEST_F(SlimeSummaryTest, empty_docsum_store_document_gives_empty_object) { - f1.empty_get_mapped_docsum = true; - f1.getDocsum(f2); - EXPECT_TRUE(f2.get().valid()); - EXPECT_EQUAL(vespalib::slime::OBJECT::ID, f2.get().type().getId()); - EXPECT_EQUAL(0u, f2.get().fields()); + empty_get_mapped_docsum = true; + Slime s; + getDocsum(s); + EXPECT_TRUE(s.get().valid()); + EXPECT_EQ(vespalib::slime::OBJECT::ID, s.get().type().getId()); + EXPECT_EQ(0u, s.get().fields()); } -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt b/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt index 6aba9614e73..be435b49348 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt +++ b/searchsummary/src/vespa/searchsummary/docsummary/CMakeLists.txt @@ -34,6 +34,7 @@ vespa_add_library(searchsummary_docsummary OBJECT searchdatatype.cpp simple_dfw.cpp slime_filler.cpp + slime_filler_filter.cpp struct_fields_resolver.cpp struct_map_attribute_combiner_dfw.cpp summaryfeaturesdfw.cpp diff --git a/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp index ec7740b371d..b36a2f8383e 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/annotation_converter.cpp @@ -13,8 +13,8 @@ #include <vespa/vespalib/util/exceptions.h> #include <utility> -using document::Annotation; using document::AlternateSpanList; +using document::Annotation; using document::FieldValue; using document::SimpleSpanList; using document::Span; @@ -125,8 +125,8 @@ AnnotationConverter::handleIndexingTerms(const StringFieldValue& value) { StringFieldValue::SpanTrees trees = value.getSpanTrees(); const SpanTree *tree = StringFieldValue::findTree(trees, linguistics::SPANTREE_NAME); - typedef std::pair<Span, const FieldValue *> SpanTerm; - typedef std::vector<SpanTerm> SpanTermVector; + using SpanTerm = std::pair<Span, const FieldValue *>; + using SpanTermVector = std::vector<SpanTerm>; if (!tree) { // Treat a string without annotations as a single span. SpanTerm str(Span(0, _text.size()), @@ -137,7 +137,7 @@ AnnotationConverter::handleIndexingTerms(const StringFieldValue& value) SpanTermVector terms; for (const Annotation& annotation : *tree) { // For now, skip any composite spans. - const Span *span = dynamic_cast<const Span*>(annotation.getSpanNode()); + const auto *span = dynamic_cast<const Span*>(annotation.getSpanNode()); if ((span != nullptr) && annotation.valid() && (annotation.getType() == *linguistics::TERM)) { terms.push_back(std::make_pair(getSpan(*span), diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp index 1ec95a58c00..889169f8888 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/attribute_combiner_dfw.cpp @@ -41,7 +41,7 @@ AttributeCombinerDFW::create(const vespalib::string &fieldName, IAttributeContex { StructFieldsResolver structFields(fieldName, attrCtx, true); if (structFields.has_error()) { - return std::unique_ptr<DocsumFieldWriter>(); + return {}; } else if (structFields.is_map_of_struct()) { return std::make_unique<StructMapAttributeCombinerDFW>(fieldName, structFields, filter_elements, std::move(matching_elems_fields)); } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp index 1dff340666e..74d67aabe88 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp @@ -25,12 +25,12 @@ using search::attribute::IAttributeContext; using search::attribute::IAttributeVector; using search::attribute::IMultiValueAttribute; using search::attribute::IMultiValueReadView; +using vespalib::Issue; using vespalib::Memory; +using vespalib::eval::Value; using vespalib::slime::Cursor; using vespalib::slime::Inserter; using vespalib::slime::Symbol; -using vespalib::eval::Value; -using vespalib::Issue; namespace search::docsummary { @@ -347,7 +347,7 @@ AttributeDFWFactory::create(const IAttributeManager& attr_mgr, const auto* attr = ctx->getAttribute(attr_name); if (attr == nullptr) { Issue::report("No valid attribute vector found: '%s'", attr_name.c_str()); - return std::unique_ptr<DocsumFieldWriter>(); + return {}; } if (attr->hasMultiValue()) { return create_multi_writer(*attr, filter_elements, std::move(matching_elems_fields)); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer.h b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer.h index e9e34175c65..77dc5d5d2d6 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsum_field_writer.h @@ -2,7 +2,6 @@ #pragma once -#include "res_type_utils.h" #include <vespa/vespalib/stllike/string.h> namespace vespalib::slime { struct Inserter; } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstore.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumstore.h index b112b7ab0bf..7f3a88b05eb 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstore.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstore.h @@ -15,10 +15,7 @@ class IDocsumStoreDocument; class IDocsumStore { public: - /** - * Convenience typedef. - */ - typedef std::unique_ptr<IDocsumStore> UP; + using UP = std::unique_ptr<IDocsumStore>; /** * Destructor. No cleanup needed for base class. diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp index bf35d6131e5..b4b663718bd 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumwriter.cpp @@ -14,8 +14,8 @@ LOG_SETUP(".searchlib.docsummary.docsumwriter"); using vespalib::Issue; -using vespalib::slime::ObjectInserter; using vespalib::Memory; +using vespalib::slime::ObjectInserter; namespace search::docsummary { @@ -77,7 +77,9 @@ DynamicDocsumWriter::insertDocsum(const ResolveClassInfo & rci, uint32_t docid, vespalib::slime::Cursor & docsum = topInserter.insertObject(); for (uint32_t i = 0; i < rci.outputClass->GetNumEntries(); ++i) { const ResConfigEntry *outCfg = rci.outputClass->GetEntry(i); - if ( ! state._args.needField(outCfg->_name)) continue; + if (!state._args.needField(outCfg->_name)) { + continue; + } const DocsumFieldWriter *writer = outCfg->_docsum_field_writer.get(); const Memory field_name(outCfg->_name.data(), outCfg->_name.size()); ObjectInserter inserter(docsum, field_name); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp index 4cd79bd8351..6d668561651 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp @@ -104,21 +104,20 @@ GeoPositionDFW::create(const char *attribute_name, const IAttributeManager *attribute_manager, bool useV8geoPositions) { - GeoPositionDFW::UP ret; if (attribute_manager != nullptr) { if (!attribute_name) { LOG(warning, "create: missing attribute name '%p'", attribute_name); - return ret; + return {}; } IAttributeContext::UP context = attribute_manager->createContext(); if (!context.get()) { LOG(warning, "create: could not create context from attribute manager"); - return ret; + return {}; } const IAttributeVector *attribute = context->getAttribute(attribute_name); if (!attribute) { Issue::report("GeoPositionDFW::create: could not get attribute '%s' from context", attribute_name); - return ret; + return {}; } } return std::make_unique<GeoPositionDFW>(attribute_name, useV8geoPositions); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.h b/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.h index 31dfc9ceef4..6e470d479ff 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.h @@ -14,7 +14,7 @@ class GeoPositionDFW : public AttrDFW private: bool _useV8geoPositions; public: - typedef std::unique_ptr<GeoPositionDFW> UP; + using UP = std::unique_ptr<GeoPositionDFW>; GeoPositionDFW(const vespalib::string & attrName, bool useV8geoPositions); void insertField(uint32_t docid, GetDocsumsState& state, vespalib::slime::Inserter &target) const override; static UP create(const char *attribute_name, const IAttributeManager *attribute_manager, bool useV8geoPositions); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/juniper_query_adapter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/juniper_query_adapter.cpp index 814fe0aafe4..a13f65db5ce 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/juniper_query_adapter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/juniper_query_adapter.cpp @@ -27,8 +27,9 @@ JuniperQueryAdapter::SkipItem(search::SimpleQueryStackDumpIterator *iterator) co uint32_t skipCount = iterator->getArity(); while (skipCount > 0) { - if (!iterator->next()) + if (!iterator->next()) { return false; // stack too small + } skipCount = skipCount - 1 + iterator->getArity(); } return true; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp index 0256965e7f4..e8ff3068a4c 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.cpp @@ -100,8 +100,9 @@ KeywordExtractor::GetLegalIndexSpec() } for (const auto & index : _legalIndexes) { - if (!spec.empty()) + if (!spec.empty()) { spec.append(';'); + } spec.append(index); } return spec; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h index 5f87de762f9..9d46f0c8d89 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/keywordextractor.h @@ -23,7 +23,7 @@ public: }; private: - typedef vespalib::hash_set<vespalib::string> Set; + using Set = vespalib::hash_set<vespalib::string>; const IDocsumEnvironment *_env; std::vector<IndexPrefix> _legalPrefixes; Set _legalIndexes; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp index 4d381f940ae..1a029cfd16f 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/matched_elements_filter_dfw.cpp @@ -54,7 +54,7 @@ MatchedElementsFilterDFW::create(const std::string& input_field_name, { StructFieldsResolver resolver(input_field_name, attr_ctx, false); if (resolver.has_error()) { - return std::unique_ptr<DocsumFieldWriter>(); + return {}; } resolver.apply_to(*matching_elems_fields); return std::make_unique<MatchedElementsFilterDFW>(input_field_name, std::move(matching_elems_fields)); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp index 02696c7b27a..5aba321b540 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp @@ -26,12 +26,12 @@ double to_degrees(int32_t microDegrees) { } +using search::attribute::BasicType; using search::attribute::IAttributeContext; using search::attribute::IAttributeVector; -using search::attribute::BasicType; using search::attribute::IntegerContent; -using search::common::Location; using search::common::GeoGcd; +using search::common::Location; LocationAttrDFW::AllLocations LocationAttrDFW::getAllLocations(GetDocsumsState& state) const @@ -232,21 +232,20 @@ PositionsDFW::insertField(uint32_t docid, GetDocsumsState& dsState, vespalib::sl //-------------------------------------------------------------------------- PositionsDFW::UP PositionsDFW::create(const char *attribute_name, const IAttributeManager *attribute_manager, bool useV8geoPositions) { - PositionsDFW::UP ret; if (attribute_manager != nullptr) { if (!attribute_name) { LOG(debug, "createPositionsDFW: missing attribute name '%p'", attribute_name); - return ret; + return {}; } IAttributeContext::UP context = attribute_manager->createContext(); if (!context.get()) { LOG(debug, "createPositionsDFW: could not create context from attribute manager"); - return ret; + return {}; } const IAttributeVector *attribute = context->getAttribute(attribute_name); if (!attribute) { LOG(debug, "createPositionsDFW: could not get attribute '%s' from context", attribute_name); - return ret; + return {}; } } return std::make_unique<PositionsDFW>(attribute_name, useV8geoPositions); @@ -254,21 +253,20 @@ PositionsDFW::UP PositionsDFW::create(const char *attribute_name, const IAttribu std::unique_ptr<DocsumFieldWriter> AbsDistanceDFW::create(const char *attribute_name, const IAttributeManager *attribute_manager) { - std::unique_ptr<DocsumFieldWriter> ret; if (attribute_manager != nullptr) { if (!attribute_name) { LOG(debug, "createAbsDistanceDFW: missing attribute name '%p'", attribute_name); - return ret; + return {}; } IAttributeContext::UP context = attribute_manager->createContext(); if (!context.get()) { LOG(debug, "createAbsDistanceDFW: could not create context from attribute manager"); - return ret; + return {}; } const IAttributeVector *attribute = context->getAttribute(attribute_name); if (!attribute) { LOG(debug, "createAbsDistanceDFW: could not get attribute '%s' from context", attribute_name); - return ret; + return {}; } } return std::make_unique<AbsDistanceDFW>(attribute_name); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h index d388b4d545f..5ac5f0fe051 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h @@ -57,7 +57,7 @@ class PositionsDFW : public AttrDFW private: bool _useV8geoPositions; public: - typedef std::unique_ptr<PositionsDFW> UP; + using UP = std::unique_ptr<PositionsDFW>; PositionsDFW(const vespalib::string & attrName, bool useV8geoPositions); bool IsGenerated() const override { return true; } void insertField(uint32_t docid, GetDocsumsState& state, vespalib::slime::Inserter &target) const override; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp index 781cd62a818..d19a111080f 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/resultclass.cpp @@ -23,15 +23,16 @@ ResultClass::~ResultClass() = default; int ResultClass::GetIndexFromName(const char* name) const { - NameIdMap::const_iterator found(_nameMap.find(name)); + auto found = _nameMap.find(name); return (found != _nameMap.end()) ? found->second : -1; } bool ResultClass::AddConfigEntry(const char *name, ResType type, std::unique_ptr<DocsumFieldWriter> docsum_field_writer) { - if (_nameMap.find(name) != _nameMap.end()) + if (_nameMap.find(name) != _nameMap.end()) { return false; + } _nameMap[name] = _entries.size(); ResConfigEntry e; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp b/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp index 77714ddd98f..65b11a2b66f 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/resultconfig.cpp @@ -70,14 +70,14 @@ ResultConfig::set_default_result_class_id(uint32_t id) const ResultClass* ResultConfig::LookupResultClass(uint32_t id) const { - IdMap::const_iterator it(_classLookup.find(id)); + auto it = _classLookup.find(id); return (it != _classLookup.end()) ? it->second.get() : nullptr; } uint32_t ResultConfig::LookupResultClassId(const vespalib::string &name) const { - NameMap::const_iterator found(_nameLookup.find(name)); + auto found = _nameLookup.find(name); return (found != _nameLookup.end()) ? found->second : ((name.empty() || (name == "default")) ? _defaultSummaryId : NoClassID()); } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp index 230c03d6644..94774c1bee4 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.cpp @@ -5,6 +5,7 @@ #include "i_string_field_converter.h" #include "resultconfig.h" #include "searchdatatype.h" +#include "slime_filler_filter.h" #include <vespa/document/datatype/positiondatatype.h> #include <vespa/document/fieldvalue/arrayfieldvalue.h> #include <vespa/document/fieldvalue/boolfieldvalue.h> @@ -65,23 +66,27 @@ private: Cursor& _array; Symbol _key_sym; Symbol _val_sym; + std::optional<const SlimeFillerFilter*> _filter; public: - MapFieldValueInserter(Inserter& parent_inserter) + MapFieldValueInserter(Inserter& parent_inserter, std::optional<const SlimeFillerFilter*> filter) : _array(parent_inserter.insertArray()), _key_sym(_array.resolve("key")), - _val_sym(_array.resolve("value")) + _val_sym(_array.resolve("value")), + _filter(std::move(filter)) { } void insert_entry(const FieldValue& key, const FieldValue& value) { Cursor& c = _array.addObject(); ObjectSymbolInserter ki(c, _key_sym); - ObjectSymbolInserter vi(c, _val_sym); SlimeFiller key_conv(ki); - SlimeFiller val_conv(vi); key.accept(key_conv); - value.accept(val_conv); + if (_filter.has_value()) { + ObjectSymbolInserter vi(c, _val_sym); + SlimeFiller val_conv(vi, nullptr, _filter.value()); + value.accept(val_conv); + } } }; @@ -90,21 +95,24 @@ public: SlimeFiller::SlimeFiller(Inserter& inserter) : _inserter(inserter), _matching_elems(nullptr), - _string_converter(nullptr) + _string_converter(nullptr), + _filter(nullptr) { } SlimeFiller::SlimeFiller(Inserter& inserter, const std::vector<uint32_t>* matching_elems) : _inserter(inserter), _matching_elems(matching_elems), - _string_converter(nullptr) + _string_converter(nullptr), + _filter(nullptr) { } -SlimeFiller::SlimeFiller(Inserter& inserter, IStringFieldConverter* string_converter) +SlimeFiller::SlimeFiller(Inserter& inserter, IStringFieldConverter* string_converter, const SlimeFillerFilter* filter) : _inserter(inserter), _matching_elems(nullptr), - _string_converter(string_converter) + _string_converter(string_converter), + _filter(filter) { } @@ -136,7 +144,7 @@ SlimeFiller::visit(const MapFieldValue& v) if (empty_or_empty_after_filtering(v)) { return; } - MapFieldValueInserter map_inserter(_inserter); + MapFieldValueInserter map_inserter(_inserter, SlimeFillerFilter::get_filter(_filter, "value")); if (filter_matching_elements()) { assert(v.has_no_erased_keys()); for (uint32_t id_to_keep : (*_matching_elems)) { @@ -158,7 +166,7 @@ SlimeFiller::visit(const ArrayFieldValue& value) } Cursor& a = _inserter.insertArray(); ArrayInserter ai(a); - SlimeFiller conv(ai, _string_converter); + SlimeFiller conv(ai, _string_converter, _filter); if (filter_matching_elements()) { for (uint32_t id_to_keep : (*_matching_elems)) { value[id_to_keep].accept(conv); @@ -266,11 +274,15 @@ SlimeFiller::visit(const StructFieldValue& value) } Cursor& c = _inserter.insertObject(); for (StructFieldValue::const_iterator itr = value.begin(); itr != value.end(); ++itr) { - Memory keymem(itr.field().getName()); - ObjectInserter vi(c, keymem); - SlimeFiller conv(vi); - FieldValue::UP nextValue(value.getValue(itr.field())); - (*nextValue).accept(conv); + auto& name = itr.field().getName(); + auto sub_filter = SlimeFillerFilter::get_filter(_filter, name); + if (sub_filter.has_value()) { + Memory keymem(name); + ObjectInserter vi(c, keymem); + SlimeFiller conv(vi, nullptr, sub_filter.value()); + FieldValue::UP nextValue(value.getValue(itr.field())); + (*nextValue).accept(conv); + } } } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h index 329dd3c6bb2..a81a20814c4 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler.h @@ -11,6 +11,7 @@ namespace vespalib::slime { struct Inserter; } namespace search::docsummary { class IStringFieldConverter; +class SlimeFillerFilter; /* * Class inserting a field value into a slime object. @@ -20,6 +21,7 @@ class SlimeFiller : public document::ConstFieldValueVisitor { vespalib::slime::Inserter& _inserter; const std::vector<uint32_t>* _matching_elems; IStringFieldConverter* _string_converter; + const SlimeFillerFilter* _filter; bool filter_matching_elements() const { return _matching_elems != nullptr; @@ -51,7 +53,7 @@ class SlimeFiller : public document::ConstFieldValueVisitor { public: SlimeFiller(vespalib::slime::Inserter& inserter); SlimeFiller(vespalib::slime::Inserter& inserter, const std::vector<uint32_t>* matching_elems); - SlimeFiller(vespalib::slime::Inserter& inserter, IStringFieldConverter* string_converter); + SlimeFiller(vespalib::slime::Inserter& inserter, IStringFieldConverter* string_converter, const SlimeFillerFilter* filter); ~SlimeFiller() override; }; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp new file mode 100644 index 00000000000..db28a1ae5cf --- /dev/null +++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.cpp @@ -0,0 +1,67 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "slime_filler_filter.h" +#include <vespa/vespalib/stllike/hash_map.hpp> +#include <cassert> + +namespace search::docsummary { + +SlimeFillerFilter::SlimeFillerFilter() + : _filter() +{ +} + +SlimeFillerFilter::~SlimeFillerFilter() = default; + +std::optional<const SlimeFillerFilter*> +SlimeFillerFilter::get_filter(vespalib::stringref field_name) const +{ + auto itr = _filter.find(field_name); + if (itr == _filter.end()) { + return std::nullopt; + } + return itr->second.get(); +} + +std::optional<const SlimeFillerFilter*> +SlimeFillerFilter::get_filter(const SlimeFillerFilter* filter, vespalib::stringref field_name) +{ + return (filter != nullptr) ? filter->get_filter(field_name) : nullptr; +} + +bool +SlimeFillerFilter::empty() const { return _filter.empty(); } + +SlimeFillerFilter& +SlimeFillerFilter::add(vespalib::stringref field_path) +{ + vespalib::stringref field_name; + vespalib::stringref remaining_path; + auto dot_pos = field_path.find('.'); + if (dot_pos != vespalib::string::npos) { + field_name = field_path.substr(0, dot_pos); + remaining_path = field_path.substr(dot_pos + 1); + } else { + field_name = field_path; + } + auto itr = _filter.find(field_name); + if (itr != _filter.end()) { + if (itr->second) { + if (remaining_path.empty()) { + itr->second.reset(); + } else { + itr->second->add(remaining_path); + } + } + } else { + auto insres = _filter.insert(std::make_pair(field_name, std::unique_ptr<SlimeFillerFilter>())); + assert(insres.second); + if (!remaining_path.empty()) { + insres.first->second = std::make_unique<SlimeFillerFilter>(); + insres.first->second->add(remaining_path); + } + } + return *this; +} + +} diff --git a/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h new file mode 100644 index 00000000000..ba7ba6fe159 --- /dev/null +++ b/searchsummary/src/vespa/searchsummary/docsummary/slime_filler_filter.h @@ -0,0 +1,29 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/vespalib/stllike/string.h> +#include <vespa/vespalib/stllike/hash_map.h> +#include <optional> + +namespace search::docsummary { + +/* + * Class filtering which fields to render in a struct field. + */ +class SlimeFillerFilter { + vespalib::hash_map<vespalib::string, std::unique_ptr<SlimeFillerFilter>> _filter; + std::optional<const SlimeFillerFilter*> get_filter(vespalib::stringref field_name) const; +public: + SlimeFillerFilter(); + ~SlimeFillerFilter(); + /* + * If field is blocked by the filter then the return value is not set, + * otherwise it is set to the filter for the next level. + */ + static std::optional<const SlimeFillerFilter*> get_filter(const SlimeFillerFilter* filter, vespalib::stringref field_name); + bool empty() const; + SlimeFillerFilter& add(vespalib::stringref field_path); +}; + +} diff --git a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp index 5c6a87664e5..dd5a59e46af 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp @@ -37,7 +37,7 @@ SummaryFieldConverter::insert_juniper_field(const document::FieldValue& value, v CheckUndefinedValueVisitor check_undefined; value.accept(check_undefined); if (!check_undefined.is_undefined()) { - SlimeFiller visitor(inserter, &converter); + SlimeFiller visitor(inserter, &converter, nullptr); value.accept(visitor); } } |