diff options
58 files changed, 1449 insertions, 177 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/test/ConfigModelTestUtil.java b/config-model/src/main/java/com/yahoo/config/model/test/TestUtil.java index b75147774ab..f2256845750 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/ConfigModelTestUtil.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/TestUtil.java @@ -13,9 +13,9 @@ import java.util.List; /** * @author lulf - * @since 5.1 + * @author gjoranv */ -public class ConfigModelTestUtil { +public class TestUtil { /** * @param xmlLines XML with " replaced with ' */ @@ -33,8 +33,11 @@ public class ConfigModelTestUtil { } } + public static String joinLines(CharSequence... lines) { + return String.join("\n", lines); + } + private static InputSource inputSource(String str) { return new InputSource(new StringReader(str)); } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index 993798789a4..6015aaaaa20 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -511,6 +511,11 @@ public final class ContainerCluster return (Collection<Handler<?>>)(Collection)componentGroup.getComponents(Handler.class); } + // Returns all servlets, including rest-api/jersey servlets. + public Collection<Servlet> getAllServlets() { + return allServlets().collect(Collectors.toCollection(ArrayList::new)); + } + public Map<ComponentId, Component<?, ?>> getComponentsMap() { return componentGroup.getComponentMap(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/Servlet.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/Servlet.java index 68ba3436209..c321ce84669 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/Servlet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/Servlet.java @@ -9,7 +9,7 @@ import com.yahoo.osgi.provider.model.ComponentModel; * @author stiankri */ public class Servlet extends SimpleComponent { - private final String bindingPath; + public final String bindingPath; public Servlet(ComponentModel componentModel, String bindingPath) { super(componentModel); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java index 76540f14563..a2cc89092a8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java @@ -7,13 +7,18 @@ import com.yahoo.component.ComponentSpecification; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent; import com.yahoo.vespa.model.container.component.Handler; +import com.yahoo.vespa.model.container.component.Servlet; import com.yahoo.vespa.model.container.http.Http.Binding; +import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Helper class for http access control. @@ -39,6 +44,8 @@ public final class AccessControl { private boolean readEnabled = false; private boolean writeEnabled = true; private final Set<String> excludeBindings = new LinkedHashSet<>(); + private Collection<Handler<?>> handlers = Collections.emptyList(); + private Collection<Servlet> servlets = Collections.emptyList(); public Builder(String domain, String applicationId) { this.domain = domain; @@ -65,8 +72,19 @@ public final class AccessControl { return this; } + public Builder setHandlers(Collection<Handler<?>> handlers) { + this.handlers = handlers; + return this; + } + + public Builder setServlets(Collection<Servlet> servlets) { + this.servlets = servlets; + return this; + } + public AccessControl build() { - return new AccessControl(domain, applicationId, writeEnabled, readEnabled, excludeBindings, vespaDomain); + return new AccessControl(domain, applicationId, writeEnabled, readEnabled, + excludeBindings, vespaDomain, servlets, handlers); } } @@ -74,30 +92,62 @@ public final class AccessControl { public final String applicationId; public final boolean readEnabled; public final boolean writeEnabled; - public final Set<String> excludedBindings; public final Optional<String> vespaDomain; + private final Set<String> excludedBindings; + private final Collection<Handler<?>> handlers; + private final Collection<Servlet> servlets; private AccessControl(String domain, String applicationId, boolean writeEnabled, boolean readEnabled, Set<String> excludedBindings, - Optional<String> vespaDomain) { + Optional<String> vespaDomain, + Collection<Servlet> servlets, + Collection<Handler<?>> handlers) { this.domain = domain; this.applicationId = applicationId; this.readEnabled = readEnabled; this.writeEnabled = writeEnabled; this.excludedBindings = Collections.unmodifiableSet(excludedBindings); this.vespaDomain = vespaDomain; + this.handlers = handlers; + this.servlets = servlets; + } + + public List<Binding> getBindings() { + return Stream.concat(getHandlerBindings(), getServletBindings()) + .collect(Collectors.toCollection(ArrayList::new)); } - public boolean shouldHandlerBeProtected(Handler<?> handler) { + private Stream<Binding> getHandlerBindings() { + return handlers.stream() + .filter(this::shouldHandlerBeProtected) + .flatMap(handler -> handler.getServerBindings().stream()) + .map(AccessControl::accessControlBinding); + } + + private Stream<Binding> getServletBindings() { + return servlets.stream() + .filter(this::shouldServletBeProtected) + .flatMap(AccessControl::servletBindings) + .map(AccessControl::accessControlBinding); + } + + private boolean shouldHandlerBeProtected(Handler<?> handler) { return ! UNPROTECTED_HANDLERS.contains(handler.getClassId().getName()) && handler.getServerBindings().stream().noneMatch(excludedBindings::contains); } - public static Binding accessControlBinding(String binding) { + private boolean shouldServletBeProtected(Servlet servlet) { + return servletBindings(servlet).noneMatch(excludedBindings::contains); + } + + private static Binding accessControlBinding(String binding) { return new Binding(new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()), binding); } + private static Stream<String> servletBindings(Servlet servlet) { + return Stream.of("http://*/", "https://*/").map(protocol -> protocol + servlet.bindingPath); + } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java index bbf1628223d..65b739f06c6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java @@ -22,7 +22,6 @@ import org.w3c.dom.Element; import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import static com.yahoo.vespa.model.container.http.AccessControl.ACCESS_CONTROL_CHAIN_ID; @@ -46,7 +45,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http> Element accessControlElem = XML.getChild(filteringElem, "access-control"); if (accessControlElem != null) { accessControl = buildAccessControl(ancestor, accessControlElem); - bindings.addAll(getAccessControlBindings(ancestor, accessControl)); + bindings.addAll(accessControl.getBindings()); filterChains.add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID))); } } else { @@ -67,6 +66,11 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http> AccessControl.Builder builder = new AccessControl.Builder(accessControlElem.getAttribute("domain"), application); + getContainerCluster(ancestor).ifPresent(cluster -> { + builder.setHandlers(cluster.getHandlers()); + builder.setServlets(cluster.getAllServlets()); + }); + XmlHelper.getOptionalAttribute(accessControlElem, "read").ifPresent( readAttr -> builder.readEnabled(Boolean.valueOf(readAttr))); XmlHelper.getOptionalAttribute(accessControlElem, "write").ifPresent( @@ -91,16 +95,6 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http> .orElse(ApplicationId.defaultId().application()); } - private static List<Binding> getAccessControlBindings(AbstractConfigProducer ancestor, AccessControl accessControl) { - return getContainerCluster(ancestor) - .map(cluster -> cluster.getHandlers().stream() - .filter(accessControl::shouldHandlerBeProtected) - .flatMap(handler -> handler.getServerBindings().stream()) - .map(AccessControl::accessControlBinding) - .collect(Collectors.toList())) - .orElse(new ArrayList<>()); - } - private static Optional<ContainerCluster> getContainerCluster(AbstractConfigProducer configProducer) { AbstractConfigProducer currentProducer = configProducer; while (currentProducer.getClass() != ContainerCluster.class) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/jersey/Jersey2Servlet.java b/config-model/src/main/java/com/yahoo/vespa/model/container/jersey/Jersey2Servlet.java index d7c9482cab4..8c01943484c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/jersey/Jersey2Servlet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/jersey/Jersey2Servlet.java @@ -15,6 +15,7 @@ public class Jersey2Servlet extends Servlet { public static final String BUNDLE = "container-jersey2"; public static final String CLASS = "com.yahoo.container.servlet.jersey.JerseyServletProvider"; + public static final String BINDING_SUFFIX = "/*"; private static final ComponentId REST_API_NAMESPACE = ComponentId.fromString("rest-api"); @@ -23,7 +24,7 @@ public class Jersey2Servlet extends Servlet { new BundleInstantiationSpecification(idSpecFromPath(bindingPath), ComponentSpecification.fromString(CLASS), ComponentSpecification.fromString(BUNDLE))), - bindingPath + "/*"); + bindingPath + BINDING_SUFFIX); } private static ComponentSpecification idSpecFromPath(String path) { diff --git a/config-model/src/test/java/com/yahoo/config/model/builder/xml/test/DomBuilderTest.java b/config-model/src/test/java/com/yahoo/config/model/builder/xml/test/DomBuilderTest.java index ba5e3e4d816..36c32b2b7e9 100644 --- a/config-model/src/test/java/com/yahoo/config/model/builder/xml/test/DomBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/builder/xml/test/DomBuilderTest.java @@ -1,7 +1,7 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.builder.xml.test; -import com.yahoo.config.model.test.ConfigModelTestUtil; +import com.yahoo.config.model.test.TestUtil; import com.yahoo.config.model.test.MockRoot; import org.junit.Before; import org.w3c.dom.Element; @@ -17,7 +17,7 @@ import org.w3c.dom.Element; abstract public class DomBuilderTest { public static Element parse(String... xmlLines) { - return ConfigModelTestUtil.parse(xmlLines); + return TestUtil.parse(xmlLines); } protected MockRoot root; diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java index aa65cca1b6a..a19fe9629fd 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java @@ -8,7 +8,7 @@ import java.util.Iterator; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static com.yahoo.searchdefinition.TestUtils.joinLines; +import static com.yahoo.config.model.test.TestUtil.joinLines; /** * @author gjoranv diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/TestUtils.java b/config-model/src/test/java/com/yahoo/searchdefinition/TestUtils.java deleted file mode 100644 index d2c963d814d..00000000000 --- a/config-model/src/test/java/com/yahoo/searchdefinition/TestUtils.java +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.searchdefinition; - -/** - * @author geirst - */ -public class TestUtils { - - public static String joinLines(String... lines) { - return String.join("\n", lines); - } -} diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/SummaryTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/SummaryTestCase.java index cdc4cbce77f..6220ab9f7d5 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/SummaryTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/SummaryTestCase.java @@ -11,7 +11,7 @@ import org.junit.Test; import java.io.IOException; import java.util.Iterator; -import static com.yahoo.searchdefinition.TestUtils.joinLines; +import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsTestCase.java index 4222a4ef66a..00ea49a9025 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsTestCase.java @@ -10,7 +10,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import static org.junit.Assert.assertEquals; -import static com.yahoo.searchdefinition.TestUtils.joinLines; +import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.Assert.assertNotNull; /** diff --git a/config-model/src/test/java/com/yahoo/vespa/documentmodel/DocumentModelBuilderReferenceTypeTestCase.java b/config-model/src/test/java/com/yahoo/vespa/documentmodel/DocumentModelBuilderReferenceTypeTestCase.java index bb115ff43e2..f9b15343f8e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/documentmodel/DocumentModelBuilderReferenceTypeTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/documentmodel/DocumentModelBuilderReferenceTypeTestCase.java @@ -13,7 +13,7 @@ import org.junit.Test; import java.io.IOException; -import static com.yahoo.searchdefinition.TestUtils.joinLines; +import static com.yahoo.config.model.test.TestUtil.joinLines; import static junit.framework.TestCase.assertSame; /** diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java index f6254f9b96d..6672a418af2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java @@ -4,12 +4,14 @@ package com.yahoo.vespa.model.container.xml; import com.google.common.collect.ImmutableSet; import com.yahoo.collections.CollectionUtil; import com.yahoo.config.model.builder.xml.test.DomBuilderTest; +import com.yahoo.config.model.test.TestUtil; import com.yahoo.container.jdisc.state.StateHandler; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.http.AccessControl; import com.yahoo.vespa.model.container.http.Http; import com.yahoo.vespa.model.container.http.Http.Binding; import com.yahoo.vespa.model.container.http.xml.HttpBuilder; +import com.yahoo.vespa.model.container.jersey.Jersey2Servlet; import org.junit.Test; import org.w3c.dom.Element; import org.xml.sax.SAXException; @@ -20,6 +22,7 @@ import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; +import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -30,7 +33,7 @@ import static org.junit.Assert.assertTrue; */ public class AccessControlTest extends ContainerModelBuilderTestBase { - private static final Set<String> REQUIRED_BINDINGS = ImmutableSet.of( + private static final Set<String> REQUIRED_HANDLER_BINDINGS = ImmutableSet.of( "/custom-handler/", "/search/", "/feed/", @@ -42,7 +45,7 @@ public class AccessControlTest extends ContainerModelBuilderTestBase { "/feedstatus/", ContainerCluster.RESERVED_URI_PREFIX); - private static final Set<String> FORBIDDEN_BINDINGS = ImmutableSet.of( + private static final Set<String> FORBIDDEN_HANDLER_BINDINGS = ImmutableSet.of( "/ApplicationStatus", "/status.html", "/statistics/", @@ -135,15 +138,15 @@ public class AccessControlTest extends ContainerModelBuilderTestBase { Http http = getHttp(clusterElem); - Set<String> foundRequiredBindings = REQUIRED_BINDINGS.stream() + Set<String> foundRequiredBindings = REQUIRED_HANDLER_BINDINGS.stream() .filter(requiredBinding -> containsBinding(http.getBindings(), requiredBinding)) .collect(Collectors.toSet()); - Set<String> missingRequiredBindings = new HashSet<>(REQUIRED_BINDINGS); + Set<String> missingRequiredBindings = new HashSet<>(REQUIRED_HANDLER_BINDINGS); missingRequiredBindings.removeAll(foundRequiredBindings); assertTrue("Access control chain was not bound to: " + CollectionUtil.mkString(missingRequiredBindings, ", "), missingRequiredBindings.isEmpty()); - FORBIDDEN_BINDINGS.forEach(forbiddenBinding -> http.getBindings().forEach( + FORBIDDEN_HANDLER_BINDINGS.forEach(forbiddenBinding -> http.getBindings().forEach( binding -> assertFalse("Access control chain was bound to: " + binding.binding, binding.binding.contains(forbiddenBinding)))); } @@ -154,29 +157,102 @@ public class AccessControlTest extends ContainerModelBuilderTestBase { final String excludedBinding = "http://*/excluded/*"; Element clusterElem = DomBuilderTest.parse( "<jdisc version='1.0'>", + httpWithExcludedBinding(excludedBinding), " <handler id='custom.Handler'>", " <binding>" + notExcludedBinding + "</binding>", " <binding>" + excludedBinding + "</binding>", " </handler>", + "</jdisc>"); + + Http http = getHttp(clusterElem); + assertFalse("Excluded binding was not removed.", + containsBinding(http.getBindings(), excludedBinding)); + assertFalse("Not all bindings of an excluded handler were removed.", + containsBinding(http.getBindings(), notExcludedBinding)); + + } + + @Test + public void access_control_filter_chain_has_all_servlet_bindings() throws Exception { + final String servletPath = "servlet/path"; + final String restApiPath = "api/v0"; + final Set<String> requiredBindings = ImmutableSet.of(servletPath, restApiPath); + Element clusterElem = DomBuilderTest.parse( + "<jdisc version='1.0'>", + " <servlet id='foo' class='bar' bundle='baz'>", + " <path>" + servletPath + "</path>", + " </servlet>", + " <rest-api jersey2='true' path='" + restApiPath + "' />", " <http>", " <filtering>", - " <access-control domain='foo'>", - " <exclude>", - " <binding>" + excludedBinding + "</binding>", - " </exclude>", - " </access-control>", + " <access-control domain='foo' />", " </filtering>", " </http>", "</jdisc>"); Http http = getHttp(clusterElem); + + Set<String> missingRequiredBindings = requiredBindings.stream() + .filter(requiredBinding -> ! containsBinding(http.getBindings(), requiredBinding)) + .collect(Collectors.toSet()); + + assertTrue("Access control chain was not bound to: " + CollectionUtil.mkString(missingRequiredBindings, ", "), + missingRequiredBindings.isEmpty()); + } + + @Test + public void servlet_can_be_excluded_by_excluding_one_of_its_bindings() throws Exception { + final String servletPath = "servlet/path"; + final String notExcludedBinding = "https://*/" + servletPath; + final String excludedBinding = "http://*/" + servletPath; + Element clusterElem = DomBuilderTest.parse( + "<jdisc version='1.0'>", + httpWithExcludedBinding(excludedBinding), + " <servlet id='foo' class='bar' bundle='baz'>", + " <path>" + servletPath + "</path>", + " </servlet>", + "</jdisc>"); + + Http http = getHttp(clusterElem); assertFalse("Excluded binding was not removed.", containsBinding(http.getBindings(), excludedBinding)); - assertFalse("Not all bindings of an excluded handler was removed.", + assertFalse("Not all bindings of an excluded servlet were removed.", containsBinding(http.getBindings(), notExcludedBinding)); } + @Test + public void rest_api_can_be_excluded_by_excluding_one_of_its_bindings() throws Exception { + final String restApiPath = "api/v0"; + final String notExcludedBinding = "http://*/" + restApiPath + Jersey2Servlet.BINDING_SUFFIX;; + final String excludedBinding = "https://*/" + restApiPath + Jersey2Servlet.BINDING_SUFFIX;; + Element clusterElem = DomBuilderTest.parse( + "<jdisc version='1.0'>", + httpWithExcludedBinding(excludedBinding), + " <rest-api jersey2='true' path='" + restApiPath + "' />", + "</jdisc>"); + + Http http = getHttp(clusterElem); + assertFalse("Excluded binding was not removed.", + containsBinding(http.getBindings(), excludedBinding)); + assertFalse("Not all bindings of an excluded rest-api were removed.", + containsBinding(http.getBindings(), notExcludedBinding)); + + } + + private String httpWithExcludedBinding(String excludedBinding) { + return joinLines( + " <http>", + " <filtering>", + " <access-control domain='foo'>", + " <exclude>", + " <binding>" + excludedBinding + "</binding>", + " </exclude>", + " </access-control>", + " </filtering>", + " </http>"); + } + private Http getHttp(Element clusterElem) throws SAXException, IOException { createModel(root, clusterElem); ContainerCluster cluster = (ContainerCluster) root.getChildren().get("jdisc"); @@ -192,5 +268,4 @@ public class AccessControlTest extends ContainerModelBuilderTestBase { } return false; } - } diff --git a/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h b/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h index 28f7f7df061..8e17e697d0e 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h +++ b/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h @@ -45,14 +45,15 @@ public: class IAttributeVector { public: - typedef uint32_t DocId; - typedef uint32_t EnumHandle; - typedef int64_t largeint_t; - typedef WeightedType<double> WeightedFloat; - typedef WeightedType<largeint_t> WeightedInt; - typedef WeightedType<EnumHandle> WeightedEnum; - typedef WeightedType<const char *> WeightedConstChar; - typedef WeightedType<vespalib::string> WeightedString; + using SP = std::shared_ptr<IAttributeVector>; + using DocId = uint32_t; + using EnumHandle = uint32_t; + using largeint_t = int64_t; + using WeightedFloat = WeightedType<double>; + using WeightedInt = WeightedType<largeint_t>; + using WeightedEnum = WeightedType<EnumHandle>; + using WeightedConstChar = WeightedType<const char *>; + using WeightedString = WeightedType<vespalib::string>; /** * Returns the name of this attribute vector. diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index 882062baa64..b45c4b38058 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -67,6 +67,7 @@ vespa_define_module( src/tests/proton/attribute/attributes_state_explorer src/tests/proton/attribute/document_field_populator src/tests/proton/attribute/exclusive_attribute_read_accessor + src/tests/proton/attribute/imported_attributes_repo src/tests/proton/bucketdb/bucketdb src/tests/proton/common src/tests/proton/common/document_type_inspector @@ -117,7 +118,9 @@ vespa_define_module( src/tests/proton/persistenceconformance src/tests/proton/persistenceengine src/tests/proton/proton + src/tests/proton/reference/gid_to_lid_change_handler src/tests/proton/reference/gid_to_lid_change_listener + src/tests/proton/reference/gid_to_lid_change_registrator src/tests/proton/reference/gid_to_lid_mapper src/tests/proton/reference/document_db_reference_resolver src/tests/proton/reference/document_db_referent_registry diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_repo/CMakeLists.txt b/searchcore/src/tests/proton/attribute/imported_attributes_repo/CMakeLists.txt new file mode 100644 index 00000000000..cac0574a8f7 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/imported_attributes_repo/CMakeLists.txt @@ -0,0 +1,8 @@ +# Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchcore_imported_attributes_repo_test_app TEST + SOURCES + imported_attributes_repo_test.cpp + DEPENDS + searchcore_attribute +) +vespa_add_test(NAME searchcore_imported_attributes_repo_test_app COMMAND searchcore_imported_attributes_repo_test_app) diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_repo/DESC b/searchcore/src/tests/proton/attribute/imported_attributes_repo/DESC new file mode 100644 index 00000000000..7d312b1df98 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/imported_attributes_repo/DESC @@ -0,0 +1 @@ +imported_attributes_repo test. Take a look at imported_attributes_repo_test.cpp for details. diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_repo/FILES b/searchcore/src/tests/proton/attribute/imported_attributes_repo/FILES new file mode 100644 index 00000000000..0e4f940c76c --- /dev/null +++ b/searchcore/src/tests/proton/attribute/imported_attributes_repo/FILES @@ -0,0 +1 @@ +imported_attributes_repo_test.cpp diff --git a/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp b/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp new file mode 100644 index 00000000000..f44004a3a2e --- /dev/null +++ b/searchcore/src/tests/proton/attribute/imported_attributes_repo/imported_attributes_repo_test.cpp @@ -0,0 +1,73 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/fastos/fastos.h> +#include <vespa/log/log.h> +LOG_SETUP("imported_attributes_repo_test"); +#include <vespa/vespalib/testkit/testapp.h> + +#include <vespa/searchcommon/attribute/iattributevector.h> +#include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> +#include <vespa/searchlib/attribute/not_implemented_attribute.h> +#include <vespa/searchcommon/attribute/basictype.h> + +using proton::ImportedAttributesRepo; +using search::attribute::BasicType; +using search::attribute::Config; +using search::attribute::IAttributeVector; +using search::NotImplementedAttribute; + +struct MyAttributeVector : public NotImplementedAttribute { +private: + virtual void onCommit() override {} + virtual void onUpdateStat() override {} + +public: + MyAttributeVector(const vespalib::string &name) + : NotImplementedAttribute(name, Config(BasicType::NONE)) + {} + static IAttributeVector::SP create(const vespalib::string &name) { + return std::make_shared<MyAttributeVector>(name); + } +}; + +struct Fixture { + ImportedAttributesRepo repo; + Fixture() : repo() {} + void add(IAttributeVector::SP attr) { + repo.add(attr->getName(), attr); + } + IAttributeVector::SP get(const vespalib::string &name) const { + return repo.get(name); + } +}; + +TEST_F("require that attributes can be added and retrieved", Fixture) +{ + IAttributeVector::SP fooAttr = MyAttributeVector::create("foo"); + IAttributeVector::SP barAttr = MyAttributeVector::create("bar"); + f.add(fooAttr); + f.add(barAttr); + EXPECT_EQUAL(2u, f.repo.size()); + EXPECT_EQUAL(f.get("foo").get(), fooAttr.get()); + EXPECT_EQUAL(f.get("bar").get(), barAttr.get()); +} + +TEST_F("require that attribute can be replaced", Fixture) +{ + IAttributeVector::SP attr1 = MyAttributeVector::create("foo"); + IAttributeVector::SP attr2 = MyAttributeVector::create("foo"); + f.add(attr1); + f.add(attr2); + EXPECT_EQUAL(1u, f.repo.size()); + EXPECT_EQUAL(f.get("foo").get(), attr2.get()); +} + +TEST_F("require that not-found attribute returns nullptr", Fixture) +{ + IAttributeVector *notFound = nullptr; + EXPECT_EQUAL(f.get("not_found").get(), notFound); +} + +TEST_MAIN() +{ + TEST_RUN_ALL(); +} diff --git a/searchcore/src/tests/proton/reference/document_db_reference_resolver/CMakeLists.txt b/searchcore/src/tests/proton/reference/document_db_reference_resolver/CMakeLists.txt index 1a2aa573ad1..144c028c545 100644 --- a/searchcore/src/tests/proton/reference/document_db_reference_resolver/CMakeLists.txt +++ b/searchcore/src/tests/proton/reference/document_db_reference_resolver/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchcore_document_db_reference_resolver_test_app TEST document_db_reference_resolver_test.cpp DEPENDS searchcore_reference + searchcore_attribute ) vespa_add_test(NAME searchcore_document_db_reference_resolver_test_app COMMAND searchcore_document_db_reference_resolver_test_app) diff --git a/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp b/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp index c8158d7d8c5..ee7151f2075 100644 --- a/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp +++ b/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp @@ -1,12 +1,15 @@ // Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/testapp.h> +#include <vespa/config-imported-fields.h> #include <vespa/document/datatype/documenttype.h> #include <vespa/document/datatype/referencedatatype.h> #include <vespa/log/log.h> +#include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> #include <vespa/searchcore/proton/reference/document_db_reference_resolver.h> #include <vespa/searchcore/proton/reference/i_document_db_referent.h> #include <vespa/searchcore/proton/reference/i_document_db_referent_registry.h> +#include <vespa/searchcore/proton/test/mock_document_db_referent.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/attribute/reference_attribute.h> #include <vespa/searchlib/common/i_gid_to_lid_mapper.h> @@ -19,6 +22,10 @@ using namespace document; using namespace proton; using namespace search::attribute; using namespace search; +using proton::test::MockDocumentDBReferent; +using search::attribute::test::MockAttributeManager; +using vespa::config::search::ImportedFieldsConfig; +using vespa::config::search::ImportedFieldsConfigBuilder; struct MyGidToLidMapperFactory : public IGidToLidMapperFactory { using SP = std::shared_ptr<MyGidToLidMapperFactory>; @@ -27,15 +34,24 @@ struct MyGidToLidMapperFactory : public IGidToLidMapperFactory { } }; -struct MyDocumentDBReferent : public IDocumentDBReferent { +struct MyDocumentDBReferent : public MockDocumentDBReferent { + using SP = std::shared_ptr<MyDocumentDBReferent>; + using AttributesMap = std::map<vespalib::string, AttributeVector::SP>; MyGidToLidMapperFactory::SP factory; + AttributesMap attributes; + MyDocumentDBReferent(MyGidToLidMapperFactory::SP factory_) : factory(factory_) {} - virtual AttributeVector::SP getAttribute(vespalib::stringref) override { - return AttributeVector::SP(); - } virtual IGidToLidMapperFactory::SP getGidToLidMapperFactory() override { return factory; } + virtual AttributeVector::SP getAttribute(vespalib::stringref name) override { + auto itr = attributes.find(name); + ASSERT_TRUE(itr != attributes.end()); + return itr->second; + } + void addIntAttribute(vespalib::stringref name) { + attributes[name] = AttributeFactory::createAttribute(name, Config(BasicType::INT32)); + } }; struct MyReferentRegistry : public IDocumentDBReferentRegistry { @@ -43,10 +59,8 @@ struct MyReferentRegistry : public IDocumentDBReferentRegistry { ReferentMap map; virtual IDocumentDBReferent::SP get(vespalib::stringref name) const override { auto itr = map.find(name); - if (itr != map.end()) { - return itr->second; - } - return IDocumentDBReferent::SP(); + ASSERT_TRUE(itr != map.end()); + return itr->second; } virtual void add(vespalib::stringref name, IDocumentDBReferent::SP referent) override { map[name] = referent; @@ -54,7 +68,7 @@ struct MyReferentRegistry : public IDocumentDBReferentRegistry { virtual void remove(vespalib::stringref) override {} }; -struct MyAttributeManager : public test::MockAttributeManager { +struct MyAttributeManager : public MockAttributeManager { void addIntAttribute(const vespalib::string &name) { addAttribute(name, AttributeFactory::createAttribute(name, Config(BasicType::INT32))); } @@ -86,12 +100,42 @@ struct DocumentModel { } }; +void +set(const vespalib::string &name, + const vespalib::string &referenceField, + const vespalib::string &targetField, + ImportedFieldsConfigBuilder::Attribute &attr) +{ + attr.name = name; + attr.referencefield = referenceField; + attr.targetfield = targetField; +} + +ImportedFieldsConfig +createImportedFieldsConfig() +{ + ImportedFieldsConfigBuilder builder; + builder.attribute.resize(2); + set("imported_a", "ref", "target_a", builder.attribute[0]); + set("imported_b", "other_ref", "target_b", builder.attribute[1]); + return builder; +} + +const ImportedAttributeVector & +asImportedAttribute(const IAttributeVector &attr) +{ + const ImportedAttributeVector *result = dynamic_cast<const ImportedAttributeVector *>(&attr); + ASSERT_TRUE(result != nullptr); + return *result; +} + struct Fixture { MyGidToLidMapperFactory::SP factory; MyDocumentDBReferent::SP parentReferent; MyReferentRegistry registry; MyAttributeManager attrMgr; DocumentModel docModel; + ImportedFieldsConfig importedFieldsCfg; DocumentDBReferenceResolver resolver; Fixture() : factory(std::make_shared<MyGidToLidMapperFactory>()), @@ -99,22 +143,38 @@ struct Fixture { registry(), attrMgr(), docModel(), - resolver(registry, attrMgr, docModel.childDocType) + importedFieldsCfg(createImportedFieldsConfig()), + resolver(registry, docModel.childDocType, importedFieldsCfg) { registry.add("parent", parentReferent); + populateTargetAttributes(); populateAttributeManager(); } + void populateTargetAttributes() { + parentReferent->addIntAttribute("target_a"); + parentReferent->addIntAttribute("target_b"); + } void populateAttributeManager() { attrMgr.addReferenceAttribute("ref"); attrMgr.addReferenceAttribute("other_ref"); attrMgr.addIntAttribute("int_attr"); } - void resolve() { - resolver.resolve(); + ImportedAttributesRepo::UP resolve() { + return resolver.resolve(attrMgr); } const IGidToLidMapperFactory *getMapperFactoryPtr(const vespalib::string &attrName) { return attrMgr.getReferenceAttribute(attrName)->getGidToLidMapperFactory().get(); } + void assertImportedAttribute(const vespalib::string &name, + const vespalib::string &referenceField, + const vespalib::string &targetField, + IAttributeVector::SP attr) { + ASSERT_TRUE(attr.get()); + EXPECT_EQUAL(name, attr->getName()); + const ImportedAttributeVector &importedAttr = asImportedAttribute(*attr); + EXPECT_EQUAL(attrMgr.getReferenceAttribute(referenceField), importedAttr.getReferenceAttribute().get()); + EXPECT_EQUAL(parentReferent->getAttribute(targetField).get(), importedAttr.getTargetAttribute().get()); + } }; TEST_F("require that reference attributes are connected to gid mapper", Fixture) @@ -124,6 +184,14 @@ TEST_F("require that reference attributes are connected to gid mapper", Fixture) EXPECT_EQUAL(f.factory.get(), f.getMapperFactoryPtr("other_ref")); } +TEST_F("require that imported attributes are instantiated", Fixture) +{ + auto repo = f.resolve(); + EXPECT_EQUAL(2u, repo->size()); + f.assertImportedAttribute("imported_a", "ref", "target_a", repo->get("imported_a")); + f.assertImportedAttribute("imported_b", "other_ref", "target_b", repo->get("imported_b")); +} + TEST_MAIN() { TEST_RUN_ALL(); diff --git a/searchcore/src/tests/proton/reference/document_db_referent_registry/document_db_referent_registry_test.cpp b/searchcore/src/tests/proton/reference/document_db_referent_registry/document_db_referent_registry_test.cpp index 2df82a9cd4e..b8f3675b20f 100644 --- a/searchcore/src/tests/proton/reference/document_db_referent_registry/document_db_referent_registry_test.cpp +++ b/searchcore/src/tests/proton/reference/document_db_referent_registry/document_db_referent_registry_test.cpp @@ -2,7 +2,7 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/stllike/string.h> #include <vespa/searchcore/proton/reference/document_db_referent_registry.h> -#include <vespa/searchcore/proton/reference/i_document_db_referent.h> +#include <vespa/searchcore/proton/test/mock_document_db_referent.h> #include <thread> #include <vespa/log/log.h> LOG_SETUP("document_db_reference_registry_test"); @@ -32,21 +32,6 @@ std::shared_ptr<IDocumentDBReferent> checkFooResult() } -struct MyDocumentDBReferent : public IDocumentDBReferent -{ - MyDocumentDBReferent() - { - } - virtual ~MyDocumentDBReferent() { } - virtual std::shared_ptr<search::AttributeVector> getAttribute(vespalib::stringref name) override { - (void) name; - return std::shared_ptr<search::AttributeVector>(); - } - virtual std::shared_ptr<search::IGidToLidMapperFactory> getGidToLidMapperFactory() override { - return std::shared_ptr<search::IGidToLidMapperFactory>(); - } -}; - struct Fixture { @@ -57,9 +42,9 @@ struct Fixture { } - std::shared_ptr<MyDocumentDBReferent> + test::MockDocumentDBReferent::SP add(vespalib::string name) { - auto referent = std::make_shared<MyDocumentDBReferent>(); + auto referent = std::make_shared<test::MockDocumentDBReferent>(); _registry.add(name, referent); return referent; } diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/CMakeLists.txt b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/CMakeLists.txt new file mode 100644 index 00000000000..eda9b7dde40 --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchcore_gid_to_lid_change_handler_test_app TEST + SOURCES + gid_to_lid_change_handler_test.cpp + DEPENDS + searchcore_reference + searchcore_server +) +vespa_add_test(NAME searchcore_gid_to_lid_change_handler_test_app COMMAND searchcore_gid_to_lid_change_handler_test_app) diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/DESC b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/DESC new file mode 100644 index 00000000000..362de49669b --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/DESC @@ -0,0 +1 @@ +gid_to_lid_change_handler test. Take a look at gid_to_lid_change_handler_test.cpp for details. diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/FILES b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/FILES new file mode 100644 index 00000000000..451157507fd --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/FILES @@ -0,0 +1 @@ +gid_to_lid_change_handler_test.cpp diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp new file mode 100644 index 00000000000..4d5bbd52a14 --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp @@ -0,0 +1,219 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/stllike/string.h> +#include <vespa/document/base/documentid.h> +#include <vespa/vespalib/util/threadstackexecutor.h> +#include <vespa/searchcore/proton/server/executor_thread_service.h> +#include <vespa/searchlib/common/lambdatask.h> +#include <vespa/searchcore/proton/reference/i_gid_to_lid_change_listener.h> +#include <vespa/searchcore/proton/reference/gid_to_lid_change_handler.h> +#include <map> +#include <vespa/log/log.h> +LOG_SETUP("gid_to_lid_change_handler_test"); + +using document::GlobalId; +using document::DocumentId; +using search::makeLambdaTask; + +namespace proton { + +namespace { + +GlobalId toGid(vespalib::stringref docId) { + return DocumentId(docId).getGlobalId(); +} + +vespalib::string doc1("id:test:music::1"); + +} + +class ListenerStats { + using lock_guard = std::lock_guard<std::mutex>; + std::mutex _lock; + uint32_t _changes; + uint32_t _createdListeners; + uint32_t _registeredListeners; + uint32_t _destroyedListeners; + +public: + ListenerStats() + : _lock(), + _changes(0u), + _createdListeners(0u), + _registeredListeners(0u), + _destroyedListeners(0u) + { + } + + ~ListenerStats() + { + EXPECT_EQUAL(_createdListeners, _destroyedListeners); + } + + void notifyGidToLidChange() { + lock_guard guard(_lock); + ++_changes; + } + void markCreatedListener() { lock_guard guard(_lock); ++_createdListeners; } + void markRegisteredListener() { lock_guard guard(_lock); ++_registeredListeners; } + void markDestroyedListener() { lock_guard guard(_lock); ++_destroyedListeners; } + + uint32_t getCreatedListeners() const { return _createdListeners; } + uint32_t getRegisteredListeners() const { return _registeredListeners; } + uint32_t getDestroyedListeners() const { return _destroyedListeners; } + + void assertCounts(uint32_t expCreatedListeners, + uint32_t expRegisteredListeners, + uint32_t expDestroyedListeners, + uint32_t expChanges) + { + EXPECT_EQUAL(expCreatedListeners, getCreatedListeners()); + EXPECT_EQUAL(expRegisteredListeners, getRegisteredListeners()); + EXPECT_EQUAL(expDestroyedListeners, getDestroyedListeners()); + EXPECT_EQUAL(expChanges, _changes); + } +}; + +class MyListener : public IGidToLidChangeListener +{ + ListenerStats &_stats; + vespalib::string _name; + vespalib::string _docTypeName; +public: + MyListener(ListenerStats &stats, + const vespalib::string &name, + const vespalib::string &docTypeName) + : IGidToLidChangeListener(), + _stats(stats), + _name(name), + _docTypeName(docTypeName) + { + _stats.markCreatedListener(); + } + virtual ~MyListener() { _stats.markDestroyedListener(); } + virtual void notifyGidToLidChange(GlobalId, uint32_t) override { _stats.notifyGidToLidChange(); } + virtual void notifyRegistered() override { _stats.markRegisteredListener(); } + virtual const vespalib::string &getName() const override { return _name; } + virtual const vespalib::string &getDocTypeName() const override { return _docTypeName; } +}; + +struct Fixture +{ + vespalib::ThreadStackExecutor _masterExecutor; + ExecutorThreadService _master; + std::vector<std::shared_ptr<ListenerStats>> _statss; + std::shared_ptr<GidToLidChangeHandler> _handler; + + Fixture() + : _masterExecutor(1, 128 * 1024), + _master(_masterExecutor), + _statss(), + _handler(std::make_shared<GidToLidChangeHandler>(&_master)) + { + } + + ~Fixture() + { + close(); + } + + void close() + { + _master.execute(makeLambdaTask([this]() { _handler->close(); })); + _master.sync(); + } + + ListenerStats &addStats() { + _statss.push_back(std::make_shared<ListenerStats>()); + return *_statss.back(); + } + + void addListener(std::unique_ptr<IGidToLidChangeListener> listener) { + _handler->addListener(std::move(listener)); + _master.sync(); + } + + void notifyGidToLidChange(GlobalId gid, uint32_t lid) { + _master.execute(makeLambdaTask([this, gid, lid]() { _handler->notifyGidToLidChange(gid, lid); })); + _master.sync(); + } + + void removeListeners(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) { + _handler->removeListeners(docTypeName, keepNames); + _master.sync(); + } + +}; + +TEST_F("Test that we can register a listener", Fixture) +{ + auto &stats = f.addStats(); + auto listener = std::make_unique<MyListener>(stats, "test", "testdoc"); + TEST_DO(stats.assertCounts(1, 0, 0, 0)); + f.addListener(std::move(listener)); + TEST_DO(stats.assertCounts(1, 1, 0, 0)); + f.notifyGidToLidChange(toGid(doc1), 10); + TEST_DO(stats.assertCounts(1, 1, 0, 1)); + f.removeListeners("testdoc", {}); + TEST_DO(stats.assertCounts(1, 1, 1, 1)); +} + +TEST_F("Test that we can register multiple listeners", Fixture) +{ + auto &stats1 = f.addStats(); + auto &stats2 = f.addStats(); + auto &stats3 = f.addStats(); + auto listener1 = std::make_unique<MyListener>(stats1, "test1", "testdoc"); + auto listener2 = std::make_unique<MyListener>(stats2, "test2", "testdoc"); + auto listener3 = std::make_unique<MyListener>(stats3, "test3", "testdoc2"); + TEST_DO(stats1.assertCounts(1, 0, 0, 0)); + TEST_DO(stats2.assertCounts(1, 0, 0, 0)); + TEST_DO(stats3.assertCounts(1, 0, 0, 0)); + f.addListener(std::move(listener1)); + f.addListener(std::move(listener2)); + f.addListener(std::move(listener3)); + TEST_DO(stats1.assertCounts(1, 1, 0, 0)); + TEST_DO(stats2.assertCounts(1, 1, 0, 0)); + TEST_DO(stats3.assertCounts(1, 1, 0, 0)); + f.notifyGidToLidChange(toGid(doc1), 10); + TEST_DO(stats1.assertCounts(1, 1, 0, 1)); + TEST_DO(stats2.assertCounts(1, 1, 0, 1)); + TEST_DO(stats3.assertCounts(1, 1, 0, 1)); + f.removeListeners("testdoc", {"test1"}); + TEST_DO(stats1.assertCounts(1, 1, 0, 1)); + TEST_DO(stats2.assertCounts(1, 1, 1, 1)); + TEST_DO(stats3.assertCounts(1, 1, 0, 1)); + f.removeListeners("testdoc", {}); + TEST_DO(stats1.assertCounts(1, 1, 1, 1)); + TEST_DO(stats2.assertCounts(1, 1, 1, 1)); + TEST_DO(stats3.assertCounts(1, 1, 0, 1)); + f.removeListeners("testdoc2", {"test3"}); + TEST_DO(stats1.assertCounts(1, 1, 1, 1)); + TEST_DO(stats2.assertCounts(1, 1, 1, 1)); + TEST_DO(stats3.assertCounts(1, 1, 0, 1)); + f.removeListeners("testdoc2", {"foo"}); + TEST_DO(stats1.assertCounts(1, 1, 1, 1)); + TEST_DO(stats2.assertCounts(1, 1, 1, 1)); + TEST_DO(stats3.assertCounts(1, 1, 1, 1)); +} + +TEST_F("Test that we keep old listener when registering duplicate", Fixture) +{ + auto &stats = f.addStats(); + auto listener = std::make_unique<MyListener>(stats, "test1", "testdoc"); + TEST_DO(stats.assertCounts(1, 0, 0, 0)); + f.addListener(std::move(listener)); + TEST_DO(stats.assertCounts(1, 1, 0, 0)); + listener = std::make_unique<MyListener>(stats, "test1", "testdoc"); + TEST_DO(stats.assertCounts(2, 1, 0, 0)); + f.addListener(std::move(listener)); + TEST_DO(stats.assertCounts(2, 1, 1, 0)); +} + +} + +TEST_MAIN() +{ + TEST_RUN_ALL(); +} diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp index fb762bd0d44..651cd30ce68 100644 --- a/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_listener/gid_to_lid_change_listener_test.cpp @@ -5,6 +5,9 @@ #include <vespa/searchlib/common/sequencedtaskexecutor.h> #include <vespa/searchcore/proton/common/monitored_refcount.h> #include <vespa/searchcore/proton/reference/gid_to_lid_change_listener.h> +#include <vespa/searchlib/common/i_gid_to_lid_mapper_factory.h> +#include <vespa/searchlib/common/i_gid_to_lid_mapper.h> +#include <map> #include <vespa/log/log.h> LOG_SETUP("gid_to_lid_change_listener_test"); @@ -28,6 +31,41 @@ vespalib::string doc1("id:test:music::1"); vespalib::string doc2("id:test:music::2"); vespalib::string doc3("id:test:music::3"); +using MockGidToLidMap = std::map<GlobalId, uint32_t>; + +struct MyGidToLidMapper : public search::IGidToLidMapper +{ + const MockGidToLidMap &_map; + MyGidToLidMapper(const MockGidToLidMap &map) + : _map(map) + { + } + virtual uint32_t mapGidToLid(const document::GlobalId &gid) const override { + auto itr = _map.find(gid); + if (itr != _map.end()) { + return itr->second; + } else { + return 0u; + } + } +}; + +struct MyGidToLidMapperFactory : public search::IGidToLidMapperFactory +{ + MockGidToLidMap _map; + + MyGidToLidMapperFactory() + : _map() + { + _map.insert({toGid(doc1), 10}); + _map.insert({toGid(doc2), 17}); + } + + virtual std::unique_ptr<search::IGidToLidMapper> getMapper() const { + return std::make_unique<MyGidToLidMapper>(_map); + } +}; + } struct Fixture @@ -69,13 +107,22 @@ struct Fixture EXPECT_EQUAL(expLid, ref->lid()); } + void assertNoRefLid(uint32_t doc) { + auto ref = getRef(doc); + EXPECT_TRUE(ref == nullptr); + } + void allocListener() { - _listener = std::make_unique<GidToLidChangeListener>(_writer, _attr, _refCount); + _listener = std::make_unique<GidToLidChangeListener>(_writer, _attr, _refCount, "test", "testdoc"); } void notifyGidToLidChange(const GlobalId &gid, uint32_t referencedDoc) { _listener->notifyGidToLidChange(gid, referencedDoc); } + + void notifyListenerRegistered() { + _listener->notifyRegistered(); + } }; TEST_F("Test that we can use gid to lid change listener", Fixture) @@ -97,6 +144,31 @@ TEST_F("Test that we can use gid to lid change listener", Fixture) TEST_DO(f.assertRefLid(10, 3)); } +TEST_F("Test that referenced lids are populated when listener is registered", Fixture) +{ + f.ensureDocIdLimit(6); + f.set(1, toGid(doc1)); + f.set(2, toGid(doc2)); + f.set(3, toGid(doc1)); + f.set(4, toGid(doc3)); + f.commit(); + TEST_DO(f.assertRefLid(0, 1)); + TEST_DO(f.assertRefLid(0, 2)); + TEST_DO(f.assertRefLid(0, 3)); + TEST_DO(f.assertRefLid(0, 4)); + TEST_DO(f.assertNoRefLid(5)); + std::shared_ptr<search::IGidToLidMapperFactory> factory = + std::make_shared<MyGidToLidMapperFactory>(); + f._attr->setGidToLidMapperFactory(factory); + f.allocListener(); + f.notifyListenerRegistered(); + TEST_DO(f.assertRefLid(10, 1)); + TEST_DO(f.assertRefLid(17, 2)); + TEST_DO(f.assertRefLid(10, 3)); + TEST_DO(f.assertRefLid(0, 4)); + TEST_DO(f.assertNoRefLid(5)); +} + } TEST_MAIN() diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/CMakeLists.txt b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/CMakeLists.txt new file mode 100644 index 00000000000..0d80606c22f --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchcore_gid_to_lid_change_registrator_test_app TEST + SOURCES + gid_to_lid_change_registrator_test.cpp + DEPENDS + searchcore_reference + searchcore_server +) +vespa_add_test(NAME searchcore_gid_to_lid_change_registrator_test_app COMMAND searchcore_gid_to_lid_change_registrator_test_app) diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/DESC b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/DESC new file mode 100644 index 00000000000..a5f481dd092 --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/DESC @@ -0,0 +1 @@ +gid_to_lid_change_registrator test. Take a look at gid_to_lid_change_registrator_test.cpp for details. diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/FILES b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/FILES new file mode 100644 index 00000000000..60e12f3f836 --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/FILES @@ -0,0 +1 @@ +gid_to_lid_change_registrator_test.cpp diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/gid_to_lid_change_registrator_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/gid_to_lid_change_registrator_test.cpp new file mode 100644 index 00000000000..a8ffc30c6c7 --- /dev/null +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_registrator/gid_to_lid_change_registrator_test.cpp @@ -0,0 +1,127 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/stllike/string.h> +#include <vespa/document/base/globalid.h> +#include <vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h> +#include <vespa/searchcore/proton/reference/i_gid_to_lid_change_listener.h> +#include <vespa/searchcore/proton/reference/gid_to_lid_change_registrator.h> +#include <vespa/vespalib/test/insertion_operators.h> +#include <map> +#include <vespa/log/log.h> +LOG_SETUP("gid_to_lid_change_registrator_test"); + +namespace proton { + +class MyListener : public IGidToLidChangeListener +{ + vespalib::string _docTypeName; + vespalib::string _name; +public: + MyListener(const vespalib::string &docTypeName, const vespalib::string &name) + : _docTypeName(docTypeName), + _name(name) + { + } + virtual ~MyListener() { } + virtual void notifyGidToLidChange(document::GlobalId, uint32_t) override { } + virtual void notifyRegistered() override { } + virtual const vespalib::string &getName() const override { return _name; } + virtual const vespalib::string &getDocTypeName() const override { return _docTypeName; } +}; + +using AddEntry = std::pair<vespalib::string, vespalib::string>; +using RemoveEntry = std::pair<vespalib::string, std::set<vespalib::string>>; + +class MyHandler : public IGidToLidChangeHandler { + std::vector<AddEntry> _adds; + std::vector<RemoveEntry> _removes; +public: + MyHandler() + : IGidToLidChangeHandler(), + _adds(), + _removes() + { + } + + ~MyHandler() { } + + virtual void addListener(std::unique_ptr<IGidToLidChangeListener> listener) override { + _adds.emplace_back(listener->getDocTypeName(), listener->getName()); + } + + virtual void removeListeners(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) override { + _removes.emplace_back(docTypeName, keepNames); + } + + void assertAdds(const std::vector<AddEntry> &expAdds) + { + EXPECT_EQUAL(expAdds, _adds); + } + + void assertRemoves(const std::vector<RemoveEntry> &expRemoves) + { + EXPECT_EQUAL(expRemoves, _removes); + } +}; + +struct Fixture +{ + std::shared_ptr<MyHandler> _handler; + + Fixture() + : _handler(std::make_shared<MyHandler>()) + { + } + + ~Fixture() + { + } + + std::unique_ptr<GidToLidChangeRegistrator> + getRegistrator(const vespalib::string &docTypeName) { + return std::make_unique<GidToLidChangeRegistrator>(_handler, docTypeName); + } + + void assertAdds(const std::vector<AddEntry> &expAdds) { + _handler->assertAdds(expAdds); + } + + void assertRemoves(const std::vector<RemoveEntry> &expRemoves) { + _handler->assertRemoves(expRemoves); + } +}; + +TEST_F("Test that we can register a listener", Fixture) +{ + auto registrator = f.getRegistrator("testdoc"); + TEST_DO(f.assertAdds({})); + TEST_DO(f.assertRemoves({})); + registrator->addListener(std::make_unique<MyListener>("testdoc", "f1")); + TEST_DO(f.assertAdds({{"testdoc","f1"}})); + TEST_DO(f.assertRemoves({})); + registrator.reset(); + TEST_DO(f.assertAdds({{"testdoc","f1"}})); + TEST_DO(f.assertRemoves({{"testdoc",{"f1"}}})); +} + +TEST_F("Test that we can register multiple listeners", Fixture) +{ + auto registrator = f.getRegistrator("testdoc"); + TEST_DO(f.assertAdds({})); + TEST_DO(f.assertRemoves({})); + registrator->addListener(std::make_unique<MyListener>("testdoc", "f1")); + registrator->addListener(std::make_unique<MyListener>("testdoc", "f2")); + TEST_DO(f.assertAdds({{"testdoc","f1"},{"testdoc","f2"}})); + TEST_DO(f.assertRemoves({})); + registrator.reset(); + TEST_DO(f.assertAdds({{"testdoc","f1"},{"testdoc","f2"}})); + TEST_DO(f.assertRemoves({{"testdoc",{"f1","f2"}}})); +} + +} + +TEST_MAIN() +{ + TEST_RUN_ALL(); +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt index 57a76366f78..dc6b9c5e8d1 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt @@ -23,6 +23,7 @@ vespa_add_library(searchcore_attribute STATIC exclusive_attribute_read_accessor.cpp filter_attribute_manager.cpp flushableattribute.cpp + imported_attributes_repo.cpp initialized_attributes_result.cpp sequential_attributes_initializer.cpp DEPENDS diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp new file mode 100644 index 00000000000..2e36a64069c --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.cpp @@ -0,0 +1,36 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/fastos/fastos.h> +#include "imported_attributes_repo.h" +#include <vespa/searchcommon/attribute/iattributevector.h> + +namespace proton { + +using search::attribute::IAttributeVector; + +ImportedAttributesRepo::ImportedAttributesRepo() + : _repo() +{ +} + +ImportedAttributesRepo::~ImportedAttributesRepo() +{ +} + +void +ImportedAttributesRepo::add(const vespalib::string &name, + IAttributeVector::SP attr) +{ + _repo[name] = std::move(attr); +} + +IAttributeVector::SP +ImportedAttributesRepo::get(const vespalib::string &name) const +{ + auto itr = _repo.find(name); + if (itr != _repo.end()) { + return itr->second; + } + return IAttributeVector::SP(); +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h new file mode 100644 index 00000000000..5a776dfb057 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/imported_attributes_repo.h @@ -0,0 +1,31 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include <map> +#include <vespa/vespalib/stllike/string.h> + +namespace search { +namespace attribute { class IAttributeVector; } } + +namespace proton { + +/** + * A repository of imported attribute vector instances. + */ +class ImportedAttributesRepo { +private: + using IAttributeVector = search::attribute::IAttributeVector; + using Repo = std::map<vespalib::string, std::shared_ptr<IAttributeVector>>; + + Repo _repo; + +public: + using UP = std::unique_ptr<ImportedAttributesRepo>; + ImportedAttributesRepo(); + ~ImportedAttributesRepo(); + void add(const vespalib::string &name, std::shared_ptr<IAttributeVector> attr); + std::shared_ptr<IAttributeVector> get(const vespalib::string &name) const; + size_t size() const { return _repo.size(); } +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/reference/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/reference/CMakeLists.txt index 94d7cec68ea..11f7bc1b81f 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/reference/CMakeLists.txt @@ -4,10 +4,13 @@ vespa_add_library(searchcore_reference STATIC document_db_reference_resolver.cpp document_db_referent.cpp document_db_referent_registry.cpp + gid_to_lid_change_handler.cpp gid_to_lid_change_listener.cpp + gid_to_lid_change_registrator.cpp gid_to_lid_mapper.cpp gid_to_lid_mapper_factory.cpp DEPENDS + searchcore_attribute searchcore_documentmetastore searchcore_pcommon ) diff --git a/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp b/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp index 244bfcc6218..388f5dd6336 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp +++ b/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp @@ -4,22 +4,28 @@ #include "document_db_reference_resolver.h" #include "i_document_db_referent_registry.h" #include "i_document_db_referent.h" +#include <vespa/config-imported-fields.h> #include <vespa/document/datatype/documenttype.h> +#include <vespa/document/datatype/referencedatatype.h> #include <vespa/document/repo/documenttyperepo.h> -#include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> #include <vespa/searchlib/attribute/iattributemanager.h> #include <vespa/searchlib/attribute/reference_attribute.h> -#include <vespa/document/datatype/referencedatatype.h> +#include <vespa/searchcommon/attribute/iattributevector.h> using document::DataType; using document::DocumentType; using document::DocumentTypeRepo; using document::ReferenceDataType; using search::attribute::BasicType; +using search::attribute::Config; +using search::attribute::IAttributeVector; using search::attribute::ReferenceAttribute; using search::AttributeGuard; using search::AttributeVector; using search::IAttributeManager; +using search::NotImplementedAttribute; +using vespa::config::search::ImportedFieldsConfig; namespace proton { @@ -43,36 +49,73 @@ asReferenceAttribute(AttributeVector &attr) return *result; } +ReferenceAttribute::SP +getReferenceAttribute(const vespalib::string &name, const IAttributeManager &attrMgr) +{ + AttributeGuard::UP guard = attrMgr.getAttribute(name); + assert(guard.get()); + assert(guard->get().getBasicType() == BasicType::REFERENCE); + return std::dynamic_pointer_cast<ReferenceAttribute>(guard->getSP()); +} + +} + +ImportedAttributeVector::ImportedAttributeVector(vespalib::stringref name, + ReferenceAttribute::SP refAttr, + IAttributeVector::SP targetAttr) + : NotImplementedAttribute(name, Config(targetAttr->getBasicType(), targetAttr->getCollectionType())), + _refAttr(std::move(refAttr)), + _targetAttr(std::move(targetAttr)) +{ +} + +IDocumentDBReferent::SP +DocumentDBReferenceResolver::getTargetDocumentDB(const vespalib::string &refAttrName) const +{ + return _registry.get(getTargetDocTypeName(refAttrName, _thisDocType)); } void -DocumentDBReferenceResolver::connectReferenceAttributesToGidMapper() +DocumentDBReferenceResolver::connectReferenceAttributesToGidMapper(const IAttributeManager &attrMgr) { std::vector<AttributeGuard> attributeList; - _attrMgr.getAttributeList(attributeList); + attrMgr.getAttributeList(attributeList); for (auto &guard : attributeList) { AttributeVector &attr = guard.get(); if (attr.getBasicType() == BasicType::REFERENCE) { - IDocumentDBReferent::SP targetDB = _registry.get(getTargetDocTypeName(attr.getName(), - _thisDocType)); + IDocumentDBReferent::SP targetDB = getTargetDocumentDB(attr.getName()); asReferenceAttribute(attr).setGidToLidMapperFactory(targetDB->getGidToLidMapperFactory()); } } } +ImportedAttributesRepo::UP +DocumentDBReferenceResolver::createImportedAttributesRepo(const IAttributeManager &attrMgr) +{ + auto result = std::make_unique<ImportedAttributesRepo>(); + for (const auto &attr : _importedFieldsCfg.attribute) { + ReferenceAttribute::SP refAttr = getReferenceAttribute(attr.referencefield, attrMgr); + IAttributeVector::SP targetAttr = getTargetDocumentDB(refAttr->getName())->getAttribute(attr.targetfield); + IAttributeVector::SP importedAttr = std::make_shared<ImportedAttributeVector>(attr.name, refAttr, targetAttr); + result->add(importedAttr->getName(), importedAttr); + } + return result; +} + DocumentDBReferenceResolver::DocumentDBReferenceResolver(const IDocumentDBReferentRegistry ®istry, - const IAttributeManager &attrMgr, - const DocumentType &thisDocType) + const DocumentType &thisDocType, + const ImportedFieldsConfig &importedFieldsCfg) : _registry(registry), - _attrMgr(attrMgr), - _thisDocType(thisDocType) + _thisDocType(thisDocType), + _importedFieldsCfg(importedFieldsCfg) { } -void -DocumentDBReferenceResolver::resolve() +ImportedAttributesRepo::UP +DocumentDBReferenceResolver::resolve(const IAttributeManager &attrMgr) { - connectReferenceAttributesToGidMapper(); + connectReferenceAttributesToGidMapper(attrMgr); + return createImportedAttributesRepo(attrMgr); } } diff --git a/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.h b/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.h index efb6ef6a76c..bc18cfb8cdb 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.h +++ b/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.h @@ -1,34 +1,63 @@ // Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -namespace document { -class DocumentType; -class DocumentTypeRepo; -} -namespace search { class IAttributeManager; } +#include "i_document_db_reference_resolver.h" +#include <vespa/vespalib/stllike/string.h> +#include <vespa/searchlib/attribute/not_implemented_attribute.h> + +namespace document { class DocumentType; class DocumentTypeRepo; } +namespace search { class IAttributeManager; namespace attribute { class IAttributeVector; class ReferenceAttribute; } } +namespace vespa { namespace config { namespace search { namespace internal { class InternalImportedFieldsType; } } } } namespace proton { +class IDocumentDBReferent; class IDocumentDBReferentRegistry; +class ImportedAttributesRepo; /** * Class that for a given document db resolves all references to parent document dbs: * 1) Connects all reference attributes to gid mappers of parent document dbs. */ -class DocumentDBReferenceResolver { +class DocumentDBReferenceResolver : public IDocumentDBReferenceResolver { private: + using ImportedFieldsConfig = const vespa::config::search::internal::InternalImportedFieldsType; const IDocumentDBReferentRegistry &_registry; - const search::IAttributeManager &_attrMgr; const document::DocumentType &_thisDocType; + const ImportedFieldsConfig &_importedFieldsCfg; - void connectReferenceAttributesToGidMapper(); + std::shared_ptr<IDocumentDBReferent> getTargetDocumentDB(const vespalib::string &refAttrName) const; + void connectReferenceAttributesToGidMapper(const search::IAttributeManager &attrMgr); + std::unique_ptr<ImportedAttributesRepo> createImportedAttributesRepo(const search::IAttributeManager &attrMgr); public: DocumentDBReferenceResolver(const IDocumentDBReferentRegistry ®istry, - const search::IAttributeManager &attrMgr, - const document::DocumentType &thisDocType); + const document::DocumentType &thisDocType, + const ImportedFieldsConfig &importedFieldsCfg); + + virtual std::unique_ptr<ImportedAttributesRepo> resolve(const search::IAttributeManager &attrMgr) override; +}; + +/** + * Temporary placeholder for an imported attribute vector. + * TODO: Remove when search::attribute::ImportedAttributeVector is finished + */ +class ImportedAttributeVector : public search::NotImplementedAttribute { +private: + vespalib::string _name; + std::shared_ptr<search::attribute::ReferenceAttribute> _refAttr; + std::shared_ptr<search::attribute::IAttributeVector> _targetAttr; + + virtual void onCommit() override {} + virtual void onUpdateStat() override {} + +public: + ImportedAttributeVector(vespalib::stringref name, + std::shared_ptr<search::attribute::ReferenceAttribute> refAttr, + std::shared_ptr<search::attribute::IAttributeVector> targetAttr); - void resolve(); + std::shared_ptr<search::attribute::ReferenceAttribute> getReferenceAttribute() const { return _refAttr; } + std::shared_ptr<search::attribute::IAttributeVector> getTargetAttribute() const { return _targetAttr; } }; } diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp new file mode 100644 index 00000000000..0ae8642cf9e --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.cpp @@ -0,0 +1,124 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "gid_to_lid_change_handler.h" +#include "i_gid_to_lid_change_listener.h" +#include <vespa/searchlib/common/lambdatask.h> +#include <vespa/searchcorespi/index/i_thread_service.h> +#include <vespa/document/base/globalid.h> +#include <assert.h> + +using search::makeLambdaTask; + + +namespace proton { + +GidToLidChangeHandler::GidToLidChangeHandler(searchcorespi::index::IThreadService *master) + : _lock(), + _listeners(), + _master(master) +{ +} + + +GidToLidChangeHandler::~GidToLidChangeHandler() +{ + assert(_master == nullptr); + assert(_listeners.empty()); +} + +void +GidToLidChangeHandler::notifyGidToLidChange(document::GlobalId gid, uint32_t lid) +{ + for (const auto &listener : _listeners) { + listener->notifyGidToLidChange(gid, lid); + } +} + +void +GidToLidChangeHandler::close() +{ + lock_guard guard(_lock); + if (_master != nullptr) { + assert(_master->isCurrentThread()); + _master = nullptr; + _listeners.clear(); + } +} + +void +GidToLidChangeHandler::addListener(std::unique_ptr<IGidToLidChangeListener> listener) +{ + lock_guard guard(_lock); + if (_master) { + auto self(shared_from_this()); + _master->execute(makeLambdaTask([self,listener(std::move(listener))]() mutable { self->performAddListener(std::move(listener)); })); + } else { + assert(_listeners.empty()); + } +} + + +void +GidToLidChangeHandler::performAddListener(std::unique_ptr<IGidToLidChangeListener> listener) +{ + lock_guard guard(_lock); + if (_master) { + const vespalib::string &docTypeName = listener->getDocTypeName(); + const vespalib::string &name = listener->getName(); + for (const auto &oldlistener : _listeners) { + if (oldlistener->getDocTypeName() == docTypeName && oldlistener->getName() == name) { + return; + } + } + _listeners.emplace_back(std::move(listener)); + _listeners.back()->notifyRegistered(); + } else { + assert(_listeners.empty()); + } +} + +void +GidToLidChangeHandler::removeListeners(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) +{ + lock_guard guard(_lock); + if (_master) { + auto self(shared_from_this()); + _master->execute(makeLambdaTask([self,docTypeName,keepNames]() mutable { self->performRemoveListener(docTypeName, keepNames); })); + } else { + assert(_listeners.empty()); + } +} + +namespace { + +bool shouldRemoveListener(const IGidToLidChangeListener &listener, + const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) +{ + return ((listener.getDocTypeName() == docTypeName) && + (keepNames.find(listener.getName()) == keepNames.end())); +} + +} + +void +GidToLidChangeHandler::performRemoveListener(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) +{ + lock_guard guard(_lock); + if (_master) { + auto itr = _listeners.begin(); + while (itr != _listeners.end()) { + if (shouldRemoveListener(**itr, docTypeName, keepNames)) { + itr = _listeners.erase(itr); + } else { + ++itr; + } + } + } else { + assert(_listeners.empty()); + } +} + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h new file mode 100644 index 00000000000..01a13beaaff --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_handler.h @@ -0,0 +1,59 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "i_gid_to_lid_change_handler.h" +#include <vector> +#include <mutex> + +namespace document { class GlobalId; } +namespace searchcorespi { namespace index { class IThreadService; } } + +namespace proton { + +/* + * Class for registering listeners that get notification when + * gid to lid mapping changes. Also handles notifications which have + * to be executed by master thread service. + */ +class GidToLidChangeHandler : public std::enable_shared_from_this<GidToLidChangeHandler>, + public IGidToLidChangeHandler +{ + using lock_guard = std::lock_guard<std::mutex>; + std::mutex _lock; + std::vector<std::unique_ptr<IGidToLidChangeListener>> _listeners; + searchcorespi::index::IThreadService *_master; + + void performAddListener(std::unique_ptr<IGidToLidChangeListener> listener); + void performRemoveListener(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames); +public: + GidToLidChangeHandler(searchcorespi::index::IThreadService *master); + virtual ~GidToLidChangeHandler(); + + /** + * Notify gid to lid mapping change. Called by master executor. + */ + void notifyGidToLidChange(document::GlobalId gid, uint32_t lid); + + /** + * Close handler, further notifications are blocked. Called by master + * executor. + */ + void close(); + + /* + * Add listener unless a listener with matching docTypeName and + * name already exists. + */ + virtual void addListener(std::unique_ptr<IGidToLidChangeListener> listener) override; + + /** + * Remove listeners with matching docTypeName unless name is present in + * keepNames. + */ + virtual void removeListeners(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) override; +}; + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp index 32b84172c45..271399fbdb5 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.cpp @@ -8,10 +8,14 @@ namespace proton { GidToLidChangeListener::GidToLidChangeListener(search::ISequencedTaskExecutor &attributeFieldWriter, std::shared_ptr<search::attribute::ReferenceAttribute> attr, - MonitoredRefCount &refCount) + MonitoredRefCount &refCount, + const vespalib::string &name, + const vespalib::string &docTypeName) : _attributeFieldWriter(attributeFieldWriter), _attr(std::move(attr)), - _refCount(refCount) + _refCount(refCount), + _name(name), + _docTypeName(docTypeName) { _refCount.retain(); } @@ -30,4 +34,26 @@ GidToLidChangeListener::notifyGidToLidChange(document::GlobalId gid, uint32_t li (void) future.get(); } +void +GidToLidChangeListener::notifyRegistered() +{ + std::promise<bool> promise; + std::future<bool> future = promise.get_future(); + _attributeFieldWriter.execute(_attr->getName(), + [this, &promise]() { _attr->populateReferencedLids(); promise.set_value(true); }); + (void) future.get(); +} + +const vespalib::string & +GidToLidChangeListener::getName() const +{ + return _name; +} + +const vespalib::string & +GidToLidChangeListener::getDocTypeName() const +{ + return _docTypeName; +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.h b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.h index 94db5b4be6c..5b3edcb4ffb 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.h +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_listener.h @@ -2,7 +2,7 @@ #pragma once -#include <vespa/vespalib/stllike/string.h> +#include "i_gid_to_lid_change_listener.h" #include <vespa/searchlib/attribute/reference_attribute.h> #include <vespa/searchlib/common/sequencedtaskexecutor.h> #include <vespa/searchcore/proton/common/monitored_refcount.h> @@ -14,18 +14,25 @@ namespace proton { * Class for listening to changes in mapping from gid to lid and updating * reference attribute appropriately. */ -class GidToLidChangeListener +class GidToLidChangeListener : public IGidToLidChangeListener { search::ISequencedTaskExecutor &_attributeFieldWriter; std::shared_ptr<search::attribute::ReferenceAttribute> _attr; MonitoredRefCount &_refCount; + vespalib::string _name; + vespalib::string _docTypeName; public: GidToLidChangeListener(search::ISequencedTaskExecutor &attributeFieldWriter, std::shared_ptr<search::attribute::ReferenceAttribute> attr, - MonitoredRefCount &refCount); + MonitoredRefCount &refCount, + const vespalib::string &name, + const vespalib::string &docTypeName); virtual ~GidToLidChangeListener(); - void notifyGidToLidChange(document::GlobalId gid, uint32_t lid); + virtual void notifyGidToLidChange(document::GlobalId gid, uint32_t lid) override; + virtual void notifyRegistered() override; + virtual const vespalib::string &getName() const override; + virtual const vespalib::string &getDocTypeName() const override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_registrator.cpp b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_registrator.cpp new file mode 100644 index 00000000000..99f03d19532 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_registrator.cpp @@ -0,0 +1,30 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "gid_to_lid_change_registrator.h" +#include "i_gid_to_lid_change_listener.h" +#include <assert.h> + +namespace proton { + +GidToLidChangeRegistrator::GidToLidChangeRegistrator(std::shared_ptr<IGidToLidChangeHandler> handler, + const vespalib::string &docTypeName) + : _handler(std::move(handler)), + _docTypeName(docTypeName), + _keepNames() +{ +} + +GidToLidChangeRegistrator::~GidToLidChangeRegistrator() +{ + _handler->removeListeners(_docTypeName, _keepNames); +} + +void +GidToLidChangeRegistrator::addListener(std::unique_ptr<IGidToLidChangeListener> listener) +{ + assert(listener->getDocTypeName() == _docTypeName); + _keepNames.insert(listener->getName()); + _handler->addListener(std::move(listener)); +} + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_registrator.h b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_registrator.h new file mode 100644 index 00000000000..0f026dc287d --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/gid_to_lid_change_registrator.h @@ -0,0 +1,27 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "i_gid_to_lid_change_handler.h" + +namespace proton { + +/* + * Helper class for registering listeners that get notification when + * gid to lid mapping changes. Will also unregister stale listeners for + * same doctype. + */ +class GidToLidChangeRegistrator +{ + std::shared_ptr<IGidToLidChangeHandler> _handler; + vespalib::string _docTypeName; + std::set<vespalib::string> _keepNames; + +public: + GidToLidChangeRegistrator(std::shared_ptr<IGidToLidChangeHandler> handler, + const vespalib::string &docTypeName); + ~GidToLidChangeRegistrator(); + void addListener(std::unique_ptr<IGidToLidChangeListener> listener); +}; + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/i_document_db_reference_resolver.h b/searchcore/src/vespa/searchcore/proton/reference/i_document_db_reference_resolver.h new file mode 100644 index 00000000000..c9f0ccdb317 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/i_document_db_reference_resolver.h @@ -0,0 +1,18 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +namespace search { class IAttributeManager; } + +namespace proton { + +class ImportedAttributesRepo; + +/** + * Interface used by a given document db to resolve all references to parent document dbs. + */ +struct IDocumentDBReferenceResolver { + virtual ~IDocumentDBReferenceResolver() {} + virtual std::unique_ptr<ImportedAttributesRepo> resolve(const search::IAttributeManager &attrMgr) = 0; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h b/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h new file mode 100644 index 00000000000..b2b682b1d42 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h @@ -0,0 +1,28 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <set> +#include <memory> +#include <vespa/vespalib/stllike/string.h> + +namespace proton { + +class IGidToLidChangeListener; + +/* + * Interface class for registering listeners that get notification when + * gid to lid mapping changes. + */ +class IGidToLidChangeHandler +{ +public: + virtual ~IGidToLidChangeHandler() { } + + virtual void addListener(std::unique_ptr<IGidToLidChangeListener> listener) = 0; + + virtual void removeListeners(const vespalib::string &docTypeName, + const std::set<vespalib::string> &keepNames) = 0; +}; + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_listener.h b/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_listener.h new file mode 100644 index 00000000000..0aef4240aae --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/reference/i_gid_to_lid_change_listener.h @@ -0,0 +1,26 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <stdint.h> +#include <vespa/vespalib/stllike/string.h> + +namespace document { class GlobalId; } + +namespace proton { + +/* + * Interface class for listening to changes in mapping from gid to lid + * and updating reference attribute appropriately. + */ +class IGidToLidChangeListener +{ +public: + virtual ~IGidToLidChangeListener() { } + virtual void notifyGidToLidChange(document::GlobalId gid, uint32_t lid) = 0; + virtual void notifyRegistered() = 0; + virtual const vespalib::string &getName() const = 0; + virtual const vespalib::string &getDocTypeName() const = 0; +}; + +} // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_document_db_referent.h b/searchcore/src/vespa/searchcore/proton/test/mock_document_db_referent.h new file mode 100644 index 00000000000..a6644f96912 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/test/mock_document_db_referent.h @@ -0,0 +1,28 @@ +// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include <vespa/searchcore/proton/reference/i_document_db_referent.h> + +namespace search { +class AttributeVector; +class IGidToLidMapperFactory; +} + +namespace proton { +namespace test { + +/** + * Mock of the IDocumentDBReferent interface used for unit testing. + */ +struct MockDocumentDBReferent : public IDocumentDBReferent { + using SP = std::shared_ptr<MockDocumentDBReferent>; + virtual std::shared_ptr<search::AttributeVector> getAttribute(vespalib::stringref) override { + return std::shared_ptr<search::AttributeVector>(); + } + virtual std::shared_ptr<search::IGidToLidMapperFactory> getGidToLidMapperFactory() override { + return std::shared_ptr<search::IGidToLidMapperFactory>(); + } +}; + +} +} diff --git a/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp b/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp index 61ac12e634f..eb035189b31 100644 --- a/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp +++ b/searchlib/src/tests/attribute/reference_attribute/reference_attribute_test.cpp @@ -143,6 +143,11 @@ struct Fixture EXPECT_EQUAL(expLid, ref->lid()); } + void assertNoRefLid(uint32_t doc) { + auto ref = getRef(doc); + EXPECT_TRUE(ref == nullptr); + } + void save() { attr().save(); } @@ -179,6 +184,9 @@ struct Fixture void notifyGidToLidChange(const GlobalId &gid, uint32_t referencedDoc) { _attr->notifyGidToLidChange(gid, referencedDoc); } + void populateReferencedLids() { + _attr->populateReferencedLids(); + } }; TEST_F("require that we can instantiate reference attribute", Fixture) @@ -309,4 +317,28 @@ TEST_F("require that notifyGidToLidChange works", Fixture) TEST_DO(f.assertRefLid(10, 3)); } +TEST_F("require that populateReferencedLids works", Fixture) +{ + f.ensureDocIdLimit(6); + f.set(1, toGid(doc1)); + f.set(2, toGid(doc2)); + f.set(3, toGid(doc1)); + f.set(4, toGid(doc3)); + f.commit(); + TEST_DO(f.assertRefLid(0, 1)); + TEST_DO(f.assertRefLid(0, 2)); + TEST_DO(f.assertRefLid(0, 3)); + TEST_DO(f.assertRefLid(0, 4)); + TEST_DO(f.assertNoRefLid(5)); + std::shared_ptr<search::IGidToLidMapperFactory> factory = + std::make_shared<MyGidToLidMapperFactory>(); + f._attr->setGidToLidMapperFactory(factory); + f.populateReferencedLids(); + TEST_DO(f.assertRefLid(10, 1)); + TEST_DO(f.assertRefLid(17, 2)); + TEST_DO(f.assertRefLid(10, 3)); + TEST_DO(f.assertRefLid(0, 4)); + TEST_DO(f.assertNoRefLid(5)); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/attribute/attributeiterators.cpp b/searchlib/src/vespa/searchlib/attribute/attributeiterators.cpp index 4b8c1ac911d..0fac4d2461e 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributeiterators.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributeiterators.cpp @@ -10,12 +10,8 @@ using fef::TermFieldMatchData; AttributeIteratorBase::AttributeIteratorBase(TermFieldMatchData * matchData) : _matchData(matchData), - _matchPosition(NULL) -{ - fef::TermFieldMatchDataPosition pos; - _matchData->appendPosition(pos); - _matchPosition = _matchData->getPositions(); -} + _matchPosition(_matchData->populate_fixed()) +{ } void AttributeIteratorBase::visitMembers(vespalib::ObjectVisitor &visitor) const @@ -54,9 +50,7 @@ FlagAttributeIterator::doUnpack(uint32_t docId) _matchData->resetOnlyDocId(docId); } -AttributePostingListIterator:: - AttributePostingListIterator(bool hasWeight, - TermFieldMatchData *matchData) +AttributePostingListIterator:: AttributePostingListIterator(bool hasWeight, TermFieldMatchData *matchData) : AttributeIteratorBase(matchData), _hasWeight(hasWeight) // _hasWeight(_searchContext.attribute().hasWeightedSetType()) diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp index b3cfdbde054..3ff9a674abd 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp @@ -259,6 +259,20 @@ ReferenceAttribute::notifyGidToLidChange(const GlobalId &gid, DocId referencedLi } } +void +ReferenceAttribute::populateReferencedLids() +{ + if (_gidToLidMapperFactory) { + std::unique_ptr<IGidToLidMapper> mapperUP = _gidToLidMapperFactory->getMapper(); + const IGidToLidMapper &mapper = *mapperUP; + const auto &store = _store; + const auto saver = _store.getSaver(); + saver.foreach_key([&store,&mapper](EntryRef ref) + { const Reference &entry = store.get(ref); + entry.setLid(mapper.mapGidToLid(entry.gid())); }); + } +} + IMPLEMENT_IDENTIFIABLE_ABSTRACT(ReferenceAttribute, AttributeVector); } diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h index ebff6c2bff6..e5f74232420 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h @@ -65,6 +65,7 @@ private: IndicesCopyVector getIndicesCopy(uint32_t size) const; public: + using SP = std::shared_ptr<ReferenceAttribute>; DECLARE_IDENTIFIABLE_ABSTRACT(ReferenceAttribute); ReferenceAttribute(const vespalib::stringref baseFileName, const Config & cfg); @@ -77,6 +78,7 @@ public: std::shared_ptr<IGidToLidMapperFactory> getGidToLidMapperFactory() const { return _gidToLidMapperFactory; } DocId getReferencedLid(DocId doc) const; void notifyGidToLidChange(const GlobalId &gid, DocId referencedLid); + void populateReferencedLids(); }; } diff --git a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp index 2ba9cf90870..e4c7d92dce4 100644 --- a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp +++ b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp @@ -84,21 +84,21 @@ constexpr size_t MAX_ELEMS = std::numeric_limits<uint16_t>::max(); void TermFieldMatchData::resizePositionVector(size_t sz) { - size_t newSize(std::min(MAX_ELEMS, std::max(1ul, sz*2))); + size_t newSize(std::min(MAX_ELEMS, std::max(1ul, sz))); TermFieldMatchDataPosition * n = new TermFieldMatchDataPosition[newSize]; - if (sz > 0) { + if (_sz > 0) { if (isMultiPos()) { for (size_t i(0); i < _data._positions._allocated; i++) { n[i] = _data._positions._positions[i]; } delete [] _data._positions._positions; } else { - assert(sz == 1); - _fieldId = _fieldId | 0x4000; + assert(_sz == 1); n[0] = *getFixed(); _data._positions._maxElementLength = getFixed()->getElementLen(); } } + _fieldId = _fieldId | 0x4000; _data._positions._allocated = newSize; _data._positions._positions = n; } @@ -107,7 +107,7 @@ void TermFieldMatchData::appendPositionToAllocatedVector(const TermFieldMatchDataPosition &pos) { if (__builtin_expect(_sz >= _data._positions._allocated, false)) { - resizePositionVector(_sz); + resizePositionVector(_sz*2); } if (__builtin_expect(pos.getElementLen() > _data._positions._maxElementLength, false)) { _data._positions._maxElementLength = pos.getElementLen(); diff --git a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h index 61286ba861f..58dfe24df23 100644 --- a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h +++ b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h @@ -70,6 +70,11 @@ public: PositionsIterator end() const { return allocated() ? getMultiple() + _sz : empty() ? getFixed() : getFixed()+1; } size_t size() const { return _sz; } size_t capacity() const { return allocated() ? _data._positions._allocated : 1; } + void reservePositions(size_t sz) { + if (sz > capacity()) { + resizePositionVector(sz); + } + } /** * Create empty object. To complete object setup, field id must be @@ -90,6 +95,15 @@ public: **/ void swap(TermFieldMatchData &rhs); + MutablePositionsIterator populate_fixed() { + assert(!allocated()); + if (_sz == 0) { + new (_data._position) TermFieldMatchDataPosition(); + _sz = 1; + } + return getFixed(); + } + /** * Set which field this object has match information for. * diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp index b7c35384bc2..097cb483f26 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp @@ -73,6 +73,7 @@ public: } _data_begin = &_data_space[0]; _data_end = _data_begin + _data_space.size(); + _tmd.reservePositions(_children.size()); } void doSeek(uint32_t docId) override { diff --git a/vespalib/src/tests/objects/nbostream/nbostream_test.cpp b/vespalib/src/tests/objects/nbostream/nbostream_test.cpp index 38c843e170b..3fedc48563d 100644 --- a/vespalib/src/tests/objects/nbostream/nbostream_test.cpp +++ b/vespalib/src/tests/objects/nbostream/nbostream_test.cpp @@ -24,14 +24,6 @@ std::ostream &operator<<(std::ostream &out, const std::vector<uint8_t> &rhs) return out; } -template <typename T, typename U> -std::ostream &operator<<(std::ostream &out, const std::pair<T, U> &rhs) -{ - out << "{ " << rhs.first << ", " << rhs.second << " }"; - return out; -} - - template <typename T> std::ostream & operator<<(std::ostream &os, const vespalib::Array<T> &set) diff --git a/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp b/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp index a624c057943..127553ffc91 100644 --- a/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp +++ b/vespalib/src/vespa/vespalib/hwaccelrated/generic.cpp @@ -32,6 +32,32 @@ multiplyAdd(const T * a, const T * b, size_t sz) return sum; } +template<size_t UNROLL, typename Operation> +void +bitOperation(Operation operation, void * aOrg, const void * bOrg, size_t bytes) { + + const size_t sz(bytes/sizeof(uint64_t)); + { + uint64_t *a(static_cast<uint64_t *>(aOrg)); + const uint64_t *b(static_cast<const uint64_t *>(bOrg)); + size_t i(0); + for (; i + UNROLL <= sz; i += UNROLL) { + for (size_t j(0); j < UNROLL; j++) { + a[i + j] = operation(a[i + j], b[i + j]); + } + } + for (; i < sz; i++) { + a[i] = operation(a[i], b[i]); + } + } + + uint8_t *a(static_cast<uint8_t *>(aOrg)); + const uint8_t *b(static_cast<const uint8_t *>(bOrg)); + for (size_t i(sz*sizeof(uint64_t)); i < bytes; i++) { + a[i] = operation(a[i], b[i]); + } +} + } float @@ -61,48 +87,18 @@ GenericAccelrator::dotProduct(const int64_t * a, const int64_t * b, size_t sz) c void GenericAccelrator::orBit(void * aOrg, const void * bOrg, size_t bytes) const { - uint64_t *a(static_cast<uint64_t *>(aOrg)); - const uint64_t *b(static_cast<const uint64_t *>(bOrg)); - const size_t sz(bytes/sizeof(uint64_t)); - for (size_t i(0); i < sz; i++) { - a[i] |= b[i]; - } - uint8_t *ac(static_cast<uint8_t *>(aOrg)); - const uint8_t *bc(static_cast<const uint8_t *>(bOrg)); - for (size_t i(sz*sizeof(uint64_t)); i < bytes; i++) { - ac[i] |= bc[i]; - } + bitOperation<8>([](uint64_t a, uint64_t b) { return a | b; }, aOrg, bOrg, bytes); } void GenericAccelrator::andBit(void * aOrg, const void * bOrg, size_t bytes) const { - uint64_t *a(static_cast<uint64_t *>(aOrg)); - const uint64_t *b(static_cast<const uint64_t *>(bOrg)); - const size_t sz(bytes/sizeof(uint64_t)); - for (size_t i(0); i < sz; i++) { - a[i] &= b[i]; - } - uint8_t *ac(static_cast<uint8_t *>(aOrg)); - const uint8_t *bc(static_cast<const uint8_t *>(bOrg)); - for (size_t i(sz*sizeof(uint64_t)); i < bytes; i++) { - ac[i] &= bc[i]; - } + bitOperation<8>([](uint64_t a, uint64_t b) { return a & b; }, aOrg, bOrg, bytes); } void GenericAccelrator::andNotBit(void * aOrg, const void * bOrg, size_t bytes) const { - uint64_t *a(static_cast<uint64_t *>(aOrg)); - const uint64_t *b(static_cast<const uint64_t *>(bOrg)); - const size_t sz(bytes/sizeof(uint64_t)); - for (size_t i(0); i < sz; i++) { - a[i] &= ~b[i]; - } - uint8_t *ac(static_cast<uint8_t *>(aOrg)); - const uint8_t *bc(static_cast<const uint8_t *>(bOrg)); - for (size_t i(sz*sizeof(uint64_t)); i < bytes; i++) { - ac[i] &= ~bc[i]; - } + bitOperation<8>([](uint64_t a, uint64_t b) { return a & ~b; }, aOrg, bOrg, bytes); } void diff --git a/vespalib/src/vespa/vespalib/test/insertion_operators.h b/vespalib/src/vespa/vespalib/test/insertion_operators.h index ac4fa3541e3..6baeaacb2b8 100644 --- a/vespalib/src/vespa/vespalib/test/insertion_operators.h +++ b/vespalib/src/vespa/vespalib/test/insertion_operators.h @@ -59,5 +59,17 @@ operator<<(std::ostream &os, const std::map<K, V> &map) return os; } +template <typename T, typename U> +std::ostream & +operator<<(std::ostream &os, const std::pair<T,U> &pair) +{ + os << "{"; + os << pair.first; + os << ","; + os << pair.second; + os << "}"; + return os; +} + } // namespace std diff --git a/vsm/src/vespa/vsm/vsm/slimefieldwriter.cpp b/vsm/src/vespa/vsm/vsm/slimefieldwriter.cpp index 2d53e4d3fa4..eb860315bf1 100644 --- a/vsm/src/vespa/vsm/vsm/slimefieldwriter.cpp +++ b/vsm/src/vespa/vsm/vsm/slimefieldwriter.cpp @@ -112,8 +112,10 @@ SlimeFieldWriter::traverseRecursive(const document::FieldValue & fv, case document::DataType::T_LONG: inserter.insertLong(fv.getAsLong()); break; - case document::DataType::T_FLOAT: case document::DataType::T_DOUBLE: + inserter.insertDouble(fv.getAsDouble()); + break; + case document::DataType::T_FLOAT: inserter.insertDouble(fv.getAsFloat()); break; default: |