Merge pull request #11998 from vespa-engine/vekterli/add-readable-attribute-vector-accessor-to-iattribute-manager

Add ReadableAttributeVector accessor to IAttributeManager

9 files changed, 69 insertions, 8 deletions

diff --git a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp
index 62cde4d2c9c..7d09b2aa0b8 100644
--- a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp
+++ b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp
@@ -34,6 +34,7 @@ private:
     void testGuards();
     void testConfigConvert();
     void testContext();
+    void can_get_readable_attribute_vector_by_name();
 
     bool
     assertDataType(BT::Type exp,
@@ -377,6 +378,21 @@ AttributeManagerTest::testContext()
     }
 }
 
+void
+AttributeManagerTest::can_get_readable_attribute_vector_by_name()
+{
+    auto attr = AttributeFactory::createAttribute("cool_attr", Config(BT::INT32, CT::SINGLE));
+    // Ensure there's something to actually load, or fetching the attribute will throw.
+    attr->addDocs(64);
+    attr->commit();
+    AttributeManager manager;
+    manager.add(attr);
+    auto av = manager.readable_attribute_vector("cool_attr");
+    EXPECT_EQUAL(av.get(), static_cast<ReadableAttributeVector*>(attr.get()));
+    av = manager.readable_attribute_vector("uncool_attr");
+    EXPECT_TRUE(av.get() == nullptr);
+}
+
 int AttributeManagerTest::Main()
 {
     TEST_INIT("attributemanager_test");
@@ -385,6 +401,7 @@ int AttributeManagerTest::Main()
     testGuards();
     testConfigConvert();
     testContext();
+    can_get_readable_attribute_vector_by_name();
 
     TEST_DONE();
 }
diff --git a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp
index 4818287b429..2eafeab20bd 100644
--- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp
+++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp
@@ -123,6 +123,15 @@ public:
         return IAttributeContext::UP();
     }
 
+    std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override {
+        if (name == field) {
+            return _attribute_vector;
+        } else if (name == other) {
+            return _other;
+        }
+        return {};
+    }
+
     void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const override;
 };
 
diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
index e9addae07b9..24be21f65ec 100644
--- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
+++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
@@ -100,6 +100,10 @@ public:
     void asyncForAttribute(const vespalib::string &, std::unique_ptr<IAttributeFunctor>) const override {
         assert(!"Not implemented");
     }
+
+    std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string&) const override {
+        return _attribute_vector;
+    }
 };
 
 constexpr uint32_t DOCID_LIMIT = 3;
diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp
index da280aa7bda..5fa32e8ad75 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp
@@ -131,7 +131,7 @@ bool AttributeManager::hasReaders() const
 const AttributeManager::VectorHolder *
 AttributeManager::findAndLoadAttribute(const string & name) const
 {
-    const VectorHolder * loadedVector(NULL);
+    const VectorHolder * loadedVector(nullptr);
     AttributeMap::const_iterator found = _attributes.find(name);
     if (found != _attributes.end()) {
         AttributeVector & vec = *found->second;
@@ -263,5 +263,14 @@ AttributeManager::asyncForAttribute(const vespalib::string &, std::unique_ptr<at
     throw std::runtime_error("search::AttributeManager::asyncForAttribute should never be called.");
 }
 
+std::shared_ptr<attribute::ReadableAttributeVector>
+AttributeManager::readable_attribute_vector(const string& name) const
+{
+    const auto* attr = findAndLoadAttribute(name);
+    if (attr) {
+        return *attr;
+    }
+    return {};
+}
 
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.h b/searchlib/src/vespa/searchlib/attribute/attributemanager.h
index 1c60ab00585..9f30ff87a70 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributemanager.h
+++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.h
@@ -49,6 +49,8 @@ public:
     void getAttributeList(AttributeList & list) const override;
     attribute::IAttributeContext::UP createContext() const override;
 
+    std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override;
+
     const Snapshot & getSnapshot()         const { return _snapShot; }
     const string & getBaseDir()       const { return _baseDir; }
     void setBaseDir(const string & base);
diff --git a/searchlib/src/vespa/searchlib/attribute/iattributemanager.h b/searchlib/src/vespa/searchlib/attribute/iattributemanager.h
index ef9403e1c4b..51e432f2035 100644
--- a/searchlib/src/vespa/searchlib/attribute/iattributemanager.h
+++ b/searchlib/src/vespa/searchlib/attribute/iattributemanager.h
@@ -8,7 +8,10 @@
 
 namespace search {
 
-namespace attribute { class AttributeReadGuard; }
+namespace attribute {
+class AttributeReadGuard;
+class ReadableAttributeVector;
+}
 
 /**
  * This is an interface used to access all registered attribute vectors.
@@ -17,12 +20,17 @@ class IAttributeManager : public attribute::IAttributeExecutor {
 public:
     IAttributeManager(const IAttributeManager &) = delete;
     IAttributeManager & operator = (const IAttributeManager &) = delete;
-    typedef std::shared_ptr<IAttributeManager> SP;
-    typedef vespalib::string string;
+    using SP = std::shared_ptr<IAttributeManager>;
+    using string = vespalib::string;
 
     /**
      * Returns a view of the attribute vector with the given name.
      *
+     * NOTE: this method is deprecated! Prefer using readable_attribute_vector(name) instead,
+     * as that enforces appropriate guards to be taken before accessing the underlying vector.
+     *
+     * TODO remove this when all usages are gone.
+     *
      * @param name name of the attribute vector.
      * @return view of the attribute vector or empty view if the attribute vector does not exists.
      **/
@@ -52,9 +60,15 @@ public:
     virtual attribute::IAttributeContext::UP createContext() const = 0;
 
     /**
-     * Virtual destructor to allow safe subclassing.
-     **/
-    virtual ~IAttributeManager() {}
+     * Looks up and returns a readable attribute vector shared_ptr with the provided name.
+     * This transparently supports imported attribute vectors.
+     *
+     * @param name name of the attribute vector.
+     * @return The attribute vector, or an empty shared_ptr if no vector was found with the given name.
+     */
+    virtual std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const = 0;
+
+    ~IAttributeManager() override = default;
 protected:
     IAttributeManager() = default;
 };
diff --git a/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h b/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h
index 03058362cb1..0e3954bdcb5 100644
--- a/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h
+++ b/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h
@@ -13,7 +13,7 @@ class AttributeReadGuard;
  */
 class ReadableAttributeVector {
 public:
-    virtual ~ReadableAttributeVector() {}
+    virtual ~ReadableAttributeVector() = default;
     virtual std::unique_ptr<AttributeReadGuard> makeReadGuard(bool stableEnumGuard) const = 0;
 };
 
diff --git a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp
index 5ef9b6cb2d4..4c80c6f869c 100644
--- a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp
+++ b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp
@@ -61,4 +61,9 @@ MockAttributeManager::addAttribute(const AttributeVector::SP &attr) {
     addAttribute(attr->getName(), attr);
 }
 
+std::shared_ptr<attribute::ReadableAttributeVector>
+MockAttributeManager::readable_attribute_vector(const string& name) const {
+    return findAttribute(name);
+}
+
 }
diff --git a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h
index dbf84a40e84..a8ec686433a 100644
--- a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h
+++ b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h
@@ -28,6 +28,7 @@ public:
     IAttributeContext::UP createContext() const override;
     void addAttribute(const vespalib::string &name, const AttributeVector::SP &attr);
     void addAttribute(const AttributeVector::SP &attr);
+    std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override;
 };
 
 }