Add ReadableAttributeVector accessor to IAttributeManager

Provides a unified interface for fetching both regular as well as
imported attributes. Exposing `ReadableAttributeVector` instead of
raw `AttributeVector` instances enforces that all access is done via
appropriate acquired read guards.

Refactor document selection processing code to use the new interface
in order to prepare for imported field support in selections.

2 files changed, 42 insertions, 10 deletions

diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
index c6d49c479c4..44ff8cb925e 100644
--- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
@@ -215,7 +215,7 @@ struct SequentialAttributeManager
     {
         mgr.addInitializedAttributes(initializer.getInitializedAttributes());
     }
-    ~SequentialAttributeManager() {}
+    ~SequentialAttributeManager() = default;
 };
 
 struct DummyInitializerTask : public InitializerTask
@@ -262,23 +262,33 @@ ParallelAttributeManager::ParallelAttributeManager(search::SerialNum configSeria
     initializer::TaskRunner taskRunner(executor);
     taskRunner.runTask(initializer);
 }
-ParallelAttributeManager::~ParallelAttributeManager() {}
+ParallelAttributeManager::~ParallelAttributeManager() = default;
 
 TEST_F("require that attributes are added", Fixture)
 {
-    EXPECT_TRUE(f.addAttribute("a1").get() != NULL);
-    EXPECT_TRUE(f.addAttribute("a2").get() != NULL);
+    EXPECT_TRUE(f.addAttribute("a1").get() != nullptr);
+    EXPECT_TRUE(f.addAttribute("a2").get() != nullptr);
     EXPECT_EQUAL("a1", (*f._m.getAttribute("a1"))->getName());
     EXPECT_EQUAL("a1", (*f._m.getAttributeReadGuard("a1", true))->getName());
     EXPECT_EQUAL("a2", (*f._m.getAttribute("a2"))->getName());
     EXPECT_EQUAL("a2", (*f._m.getAttributeReadGuard("a2", true))->getName());
     EXPECT_TRUE(!f._m.getAttribute("not")->valid());
+
+    auto rv = f._m.readable_attribute_vector("a1");
+    ASSERT_TRUE(rv.get() != nullptr);
+    EXPECT_EQUAL("a1", rv->makeReadGuard(true)->attribute()->getName());
+
+    rv = f._m.readable_attribute_vector("a2");
+    ASSERT_TRUE(rv.get() != nullptr);
+    EXPECT_EQUAL("a2", rv->makeReadGuard(true)->attribute()->getName());
+
+    EXPECT_TRUE(f._m.readable_attribute_vector("not_valid").get() == nullptr);
 }
 
 TEST_F("require that predicate attributes are added", Fixture)
 {
     EXPECT_TRUE(f._m.addAttribute({"p1", AttributeUtils::getPredicateConfig()},
-                                  createSerialNum).get() != NULL);
+                                  createSerialNum).get() != nullptr);
     EXPECT_EQUAL("p1", (*f._m.getAttribute("p1"))->getName());
     EXPECT_EQUAL("p1", (*f._m.getAttributeReadGuard("p1", true))->getName());
 }
@@ -376,7 +386,7 @@ TEST_F("require that predicate attributes are flushed and loaded", BaseFixture)
         AttributeVector::SP a1 = am.addAttribute({"a1", AttributeUtils::getPredicateConfig()}, createSerialNum);
         EXPECT_EQUAL(1u, a1->getNumDocs());
 
-        PredicateAttribute &pa = static_cast<PredicateAttribute &>(*a1);
+        auto &pa = static_cast<PredicateAttribute &>(*a1);
         PredicateIndex &index = pa.getIndex();
         uint32_t doc_id;
         a1->addDoc(doc_id);
@@ -396,7 +406,7 @@ TEST_F("require that predicate attributes are flushed and loaded", BaseFixture)
         AttributeVector::SP a1 = am.addAttribute({"a1", AttributeUtils::getPredicateConfig()}, createSerialNum); // loaded
         EXPECT_EQUAL(2u, a1->getNumDocs());
 
-        PredicateAttribute &pa = static_cast<PredicateAttribute &>(*a1);
+        auto &pa = static_cast<PredicateAttribute &>(*a1);
         PredicateIndex &index = pa.getIndex();
         uint32_t doc_id;
         a1->addDoc(doc_id);
@@ -746,7 +756,7 @@ TEST_F("require that we can acquire exclusive read access to attribute", Fixture
     EXPECT_TRUE(noneAccessor.get() == nullptr);
 }
 
-TEST_F("require that imported attributes are exposed via attribute context together vi regular attributes", Fixture)
+TEST_F("require that imported attributes are exposed via attribute context together with regular attributes", Fixture)
 {
     f.addAttribute("attr");
     f.addImportedAttribute("imported");
@@ -767,6 +777,17 @@ TEST_F("require that imported attributes are exposed via attribute context toget
     EXPECT_EQUAL("imported", all[1]->getName());
 }
 
+TEST_F("imported attributes are transparently returned from readable_attribute_vector", Fixture)
+{
+    f.addAttribute("attr");
+    f.addImportedAttribute("imported");
+    f.setImportedAttributes();
+    auto av = f._m.readable_attribute_vector("imported");
+    ASSERT_TRUE(av);
+    auto g = av->makeReadGuard(false);
+    EXPECT_EQUAL("imported", g->attribute()->getName());
+}
+
 TEST_F("require that attribute vector of wrong type is dropped", BaseFixture)
 {
     AVConfig generic_tensor(BasicType::TENSOR);
diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp
index 14b72c9d8f8..edb5a07b059 100644
--- a/searchcore/src/tests/proton/attribute/attribute_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp
@@ -20,6 +20,7 @@
 #include <vespa/searchcore/proton/test/attribute_utils.h>
 #include <vespa/searchcorespi/flush/iflushtarget.h>
 #include <vespa/searchlib/attribute/attributefactory.h>
+#include <vespa/searchlib/attribute/attribute_read_guard.h>
 #include <vespa/searchlib/attribute/bitvector_search_cache.h>
 #include <vespa/searchlib/attribute/imported_attribute_vector.h>
 #include <vespa/searchlib/attribute/imported_attribute_vector_factory.h>
@@ -588,8 +589,8 @@ struct FilterFixture
 
 TEST_F("require that filter attribute manager can filter attributes", FilterFixture)
 {
-    EXPECT_TRUE(f._filterMgr.getAttribute("a1").get() == NULL);
-    EXPECT_TRUE(f._filterMgr.getAttribute("a2").get() != NULL);
+    EXPECT_TRUE(f._filterMgr.getAttribute("a1").get() == nullptr);
+    EXPECT_TRUE(f._filterMgr.getAttribute("a2").get() != nullptr);
     std::vector<AttributeGuard> attrs;
     f._filterMgr.getAttributeList(attrs);
     EXPECT_EQUAL(1u, attrs.size());
@@ -607,6 +608,16 @@ TEST_F("require that filter attribute manager can return flushed serial number",
     EXPECT_EQUAL(100u, f._filterMgr.getFlushedSerialNum("a2"));
 }
 
+TEST_F("readable_attribute_vector filters attributes", FilterFixture)
+{
+    auto av = f._filterMgr.readable_attribute_vector("a2");
+    ASSERT_TRUE(av);
+    EXPECT_EQUAL("a2", av->makeReadGuard(false)->attribute()->getName());
+
+    av = f._filterMgr.readable_attribute_vector("a1");
+    EXPECT_FALSE(av);
+}
+
 namespace {
 
 Tensor::UP make_tensor(const TensorSpec &spec) {