Merge pull request #14007 from vespa-engine/balder/improve-code-healt-for-tests

Improve code health of test code.

6 files changed, 108 insertions, 138 deletions

diff --git a/searchcore/src/apps/tests/persistenceconformance_test.cpp b/searchcore/src/apps/tests/persistenceconformance_test.cpp
index 4056e49e37c..217d1edcd57 100644
--- a/searchcore/src/apps/tests/persistenceconformance_test.cpp
+++ b/searchcore/src/apps/tests/persistenceconformance_test.cpp
@@ -75,19 +75,19 @@ storeDocType(DocTypeVector *types, const DocumentType &type)
 struct SchemaConfigFactory {
     typedef DocumentDBConfig CS;
     typedef std::shared_ptr<SchemaConfigFactory> SP;
-    virtual ~SchemaConfigFactory() {}
-    static SchemaConfigFactory::SP get() { return SchemaConfigFactory::SP(new SchemaConfigFactory()); }
+    virtual ~SchemaConfigFactory() = default;
+    static SchemaConfigFactory::SP get() { return std::make_shared<SchemaConfigFactory>(); }
     virtual CS::IndexschemaConfigSP createIndexSchema(const DocumentType &docType) {
         (void) docType;
-        return CS::IndexschemaConfigSP(new IndexschemaConfig());
+        return std::make_shared<IndexschemaConfig>();
     }
     virtual CS::AttributesConfigSP createAttributes(const DocumentType &docType) {
         (void) docType;
-        return CS::AttributesConfigSP(new AttributesConfig());
+        return std::make_shared<AttributesConfig>();
     }
     virtual CS::SummaryConfigSP createSummary(const DocumentType &docType) {
         (void) docType;
-        return CS::SummaryConfigSP(new SummaryConfig());
+        return std::make_shared<SummaryConfig>();
     }
 };
 
@@ -98,12 +98,12 @@ private:
     SchemaConfigFactory::SP _schemaFactory;
 
 public:
-    ConfigFactory(const std::shared_ptr<const DocumentTypeRepo> &repo,
-                  const DocumenttypesConfigSP &typeCfg,
-                  const SchemaConfigFactory::SP &schemaFactory);
+    ConfigFactory(std::shared_ptr<const DocumentTypeRepo> repo,
+                  DocumenttypesConfigSP typeCfg,
+                  SchemaConfigFactory::SP schemaFactory);
     ~ConfigFactory();
-    const std::shared_ptr<const DocumentTypeRepo> getTypeRepo() const { return _repo; }
-    const DocumenttypesConfigSP getTypeCfg() const { return _typeCfg; }
+    std::shared_ptr<const DocumentTypeRepo> getTypeRepo() const { return _repo; }
+    DocumenttypesConfigSP getTypeCfg() const { return _typeCfg; }
     DocTypeVector getDocTypes() const {
         DocTypeVector types;
         _repo->forEachDocumentType(*makeClosure(storeDocType, &types));
@@ -112,7 +112,7 @@ public:
     DocumentDBConfig::SP create(const DocTypeName &docTypeName) const {
         const DocumentType *docType =
             _repo->getDocumentType(docTypeName.getName());
-        if (docType == NULL) {
+        if (docType == nullptr) {
             return DocumentDBConfig::SP();
         }
         typedef DocumentDBConfig CS;
@@ -123,7 +123,7 @@ public:
         SchemaBuilder::build(*indexschema, *schema);
         SchemaBuilder::build(*attributes, *schema);
         SchemaBuilder::build(*summary, *schema);
-        return DocumentDBConfig::SP(new DocumentDBConfig(
+        return std::make_shared<DocumentDBConfig>(
                         1,
                         std::make_shared<RankProfilesConfig>(),
                         std::make_shared<matching::RankingConstants>(),
@@ -140,18 +140,18 @@ public:
                         std::make_shared<DocumentDBMaintenanceConfig>(),
                         search::LogDocumentStore::Config(),
                         "client",
-                        docTypeName.getName()));
+                        docTypeName.getName());
     }
 };
 
 
-ConfigFactory::ConfigFactory(const std::shared_ptr<const DocumentTypeRepo> &repo, const DocumenttypesConfigSP &typeCfg,
-                             const SchemaConfigFactory::SP &schemaFactory)
-    : _repo(repo),
-      _typeCfg(typeCfg),
-      _schemaFactory(schemaFactory)
+ConfigFactory::ConfigFactory(std::shared_ptr<const DocumentTypeRepo> repo, DocumenttypesConfigSP typeCfg,
+                             SchemaConfigFactory::SP schemaFactory)
+    : _repo(std::move(repo)),
+      _typeCfg(std::move(typeCfg)),
+      _schemaFactory(std::move(schemaFactory))
 {}
-ConfigFactory::~ConfigFactory() {}
+ConfigFactory::~ConfigFactory() = default;
 
 class DocumentDBFactory : public DummyDBOwner {
 private:
@@ -167,7 +167,7 @@ private:
 
 public:
     DocumentDBFactory(const vespalib::string &baseDir, int tlsListenPort);
-    ~DocumentDBFactory();
+    ~DocumentDBFactory() override;
     DocumentDB::SP create(BucketSpace bucketSpace,
                           const DocTypeName &docType,
                           const ConfigFactory &factory) {
@@ -191,8 +191,7 @@ public:
                                                   tuneFileDocDB, HwInfo()));
         mgr.forwardConfig(b);
         mgr.nextGeneration(0ms);
-        return DocumentDB::SP(
-                new DocumentDB(_baseDir,
+        return std::make_shared<DocumentDB>(_baseDir,
                                mgr.getConfig(),
                                _tlsSpec,
                                _queryLimiter,
@@ -209,7 +208,7 @@ public:
                                _config_stores.getConfigStore(docType.toString()),
                                std::make_shared<vespalib::ThreadStackExecutor>
                                (16, 128 * 1024),
-                               HwInfo()));
+                               HwInfo());
     }
 };
 
@@ -224,40 +223,33 @@ DocumentDBFactory::DocumentDBFactory(const vespalib::string &baseDir, int tlsLis
       _metricsWireService(),
       _summaryExecutor(8, 128 * 1024)
 {}
-DocumentDBFactory::~DocumentDBFactory() {}
+DocumentDBFactory::~DocumentDBFactory()  = default;
 
 class DocumentDBRepo {
 private:
     DocumentDBMap _docDbs;
 public:
     typedef std::unique_ptr<DocumentDBRepo> UP;
-    DocumentDBRepo(const ConfigFactory &cfgFactory,
-                   DocumentDBFactory &docDbFactory) :
-        _docDbs()
+    DocumentDBRepo(const ConfigFactory &cfgFactory, DocumentDBFactory &docDbFactory)
+        : _docDbs()
     {
         DocTypeVector types = cfgFactory.getDocTypes();
-        for (size_t i = 0; i < types.size(); ++i) {
-            BucketSpace bucketSpace(makeBucketSpace(types[i].getName()));
-            DocumentDB::SP docDb = docDbFactory.create(bucketSpace,
-                                                       types[i],
-                                                       cfgFactory);
+        for (const auto & type : types) {
+            BucketSpace bucketSpace(makeBucketSpace(type.getName()));
+            DocumentDB::SP docDb = docDbFactory.create(bucketSpace, type, cfgFactory);
             docDb->start();
             docDb->waitForOnlineState();
-            _docDbs[types[i]] = docDb;
+            _docDbs[type] = docDb;
         }
     }
 
-    void
-    close()
-    {
-        for (DocumentDBMap::iterator itr = _docDbs.begin();
-             itr != _docDbs.end(); ++itr) {
-            itr->second->close();
+    void close() {
+        for (auto & dbEntry : _docDbs) {
+            dbEntry.second->close();
         }
     }
 
-    ~DocumentDBRepo()
-    {
+    ~DocumentDBRepo() {
         close();
     }
     const DocumentDBMap &getDocDbs() const { return _docDbs; }
@@ -269,20 +261,15 @@ class DocDBRepoHolder
 protected:
     DocumentDBRepo::UP      _docDbRepo;
 
-    DocDBRepoHolder(DocumentDBRepo::UP docDbRepo)
+    explicit DocDBRepoHolder(DocumentDBRepo::UP docDbRepo)
         : _docDbRepo(std::move(docDbRepo))
     {
     }
 
-    virtual
-    ~DocDBRepoHolder()
-    {
-    }
+    virtual ~DocDBRepoHolder() = default;
 
-    void
-    close()
-    {
-        if (_docDbRepo.get() != NULL)
+    void close() {
+        if (_docDbRepo)
             _docDbRepo->close();
     }
 };
@@ -290,17 +277,13 @@ protected:
 
 class MyPersistenceEngineOwner : public IPersistenceEngineOwner
 {
-    virtual void
-    setClusterState(BucketSpace, const storage::spi::ClusterState &calc) override
-    {
-        (void) calc;
-    }
+    void setClusterState(BucketSpace, const storage::spi::ClusterState &) override { }
 };
 
 struct MyResourceWriteFilter : public IResourceWriteFilter
 {
-    virtual bool acceptWriteOperation() const override { return true; }
-    virtual State getAcceptState() const override { return IResourceWriteFilter::State(); }
+    bool acceptWriteOperation() const override { return true; }
+    State getAcceptState() const override { return IResourceWriteFilter::State(); }
 };
 
 class MyPersistenceEngine : public DocDBRepoHolder,
@@ -320,35 +303,32 @@ public:
     void
     addHandlers(const vespalib::string &docType)
     {
-        if (_docDbRepo.get() == NULL)
+        if (!_docDbRepo)
             return;
         const DocumentDBMap &docDbs = _docDbRepo->getDocDbs();
-        for (DocumentDBMap::const_iterator itr = docDbs.begin();
-             itr != docDbs.end(); ++itr) {
-            if (!docType.empty() && docType != itr->first.getName()) {
+        for (const auto & dbEntry : docDbs) {
+            if (!docType.empty() && docType != dbEntry.first.getName()) {
                 continue;
             }
-            LOG(info, "putHandler(%s)", itr->first.toString().c_str());
-            IPersistenceHandler::SP proxy(
-                    new PersistenceHandlerProxy(itr->second));
-            putHandler(getWLock(), itr->second->getBucketSpace(), itr->first, proxy);
+            LOG(info, "putHandler(%s)", dbEntry.first.toString().c_str());
+            auto proxy = std::make_shared<PersistenceHandlerProxy>(dbEntry.second);
+            putHandler(getWLock(), dbEntry.second->getBucketSpace(), dbEntry.first, proxy);
         }
     }
 
     void
     removeHandlers()
     {
-        if (_docDbRepo.get() == NULL)
+        if ( ! _docDbRepo)
             return;
         const DocumentDBMap &docDbs = _docDbRepo->getDocDbs();
-        for (DocumentDBMap::const_iterator itr = docDbs.begin();
-             itr != docDbs.end(); ++itr) {
-            IPersistenceHandler::SP proxy(removeHandler(getWLock(), itr->second->getBucketSpace(), itr->first));
+        for (const auto & dbEntry : docDbs) {
+            IPersistenceHandler::SP proxy(removeHandler(getWLock(), dbEntry.second->getBucketSpace(), dbEntry.first));
+            (void) proxy;
         }
     }
 
-    virtual
-    ~MyPersistenceEngine()
+    ~MyPersistenceEngine() override
     {
         destroyIterators();
         removeHandlers(); // Block calls to document db from engine
@@ -367,11 +347,11 @@ private:
     MyResourceWriteFilter    _writeFilter;
 public:
     MyPersistenceFactory(const vespalib::string &baseDir, int tlsListenPort,
-                         const SchemaConfigFactory::SP &schemaFactory,
-                         const vespalib::string &docType = "")
+                         SchemaConfigFactory::SP schemaFactory,
+                         const vespalib::string & docType = "")
         : _baseDir(baseDir),
           _docDbFactory(baseDir, tlsListenPort),
-          _schemaFactory(schemaFactory),
+          _schemaFactory(std::move(schemaFactory)),
           _docDbRepo(),
           _docType(docType),
           _engineOwner(),
@@ -382,25 +362,25 @@ public:
     ~MyPersistenceFactory() override {
         clear();
     }
-    virtual PersistenceProvider::UP getPersistenceImplementation(const std::shared_ptr<const DocumentTypeRepo> &repo,
-                                                                 const DocumenttypesConfig &typesCfg) override {
-        ConfigFactory cfgFactory(repo, DocumenttypesConfigSP(new DocumenttypesConfig(typesCfg)), _schemaFactory);
-        _docDbRepo.reset(new DocumentDBRepo(cfgFactory, _docDbFactory));
+    PersistenceProvider::UP getPersistenceImplementation(const std::shared_ptr<const DocumentTypeRepo> &repo,
+                                                         const DocumenttypesConfig &typesCfg) override {
+        ConfigFactory cfgFactory(repo, std::make_shared<DocumenttypesConfig>(typesCfg), _schemaFactory);
+        _docDbRepo = std::make_unique<DocumentDBRepo>(cfgFactory, _docDbFactory);
         PersistenceEngine::UP engine(new MyPersistenceEngine(_engineOwner,
                                                              _writeFilter,
                                                              std::move(_docDbRepo),
                                                              _docType));
-        assert(_docDbRepo.get() == NULL); // Repo should be handed over
-        return PersistenceProvider::UP(engine.release());
+        assert( ! _docDbRepo); // Repo should be handed over
+        return engine;
     }
 
-    virtual void clear() override {
+    void clear() override {
         FastOS_FileInterface::EmptyAndRemoveDirectory(_baseDir.c_str());
     }
 
-    virtual bool hasPersistence() const override { return true; }
-    virtual bool supportsActiveState() const override { return true; }
-    virtual bool supportsBucketSpaces() const override { return true; }
+    bool hasPersistence() const override { return true; }
+    bool supportsActiveState() const override { return true; }
+    bool supportsBucketSpaces() const override { return true; }
 };
 
 
diff --git a/searchcore/src/tests/proton/server/documentretriever_test.cpp b/searchcore/src/tests/proton/server/documentretriever_test.cpp
index 30a87bd216e..40224902b8f 100644
--- a/searchcore/src/tests/proton/server/documentretriever_test.cpp
+++ b/searchcore/src/tests/proton/server/documentretriever_test.cpp
@@ -314,6 +314,17 @@ struct Fixture {
         attr->commit();
     }
 
+    Fixture &
+    addIndexField(const Schema::IndexField &field) {
+        schema.addIndexField(field);
+        return *this;
+    }
+
+    void
+    build() {
+        _retriever = std::make_unique<DocumentRetriever>(_dtName, repo, schema, meta_store, attr_manager, doc_store);
+    }
+
     Fixture()
         : repo(getRepoConfig()),
           meta_store(std::make_shared<BucketDBOwner>()),
@@ -335,46 +346,29 @@ struct Fixture {
         lid = putRes.getLid();
         ASSERT_TRUE(putRes.ok());
         schema::CollectionType ct = schema::CollectionType::SINGLE;
-        addAttribute<IntegerAttribute>(
-                dyn_field_i, dyn_value_i, DataType::INT32, ct);
-        addAttribute<FloatingPointAttribute>(
-                dyn_field_d, dyn_value_d, DataType::DOUBLE, ct);
-        addAttribute<StringAttribute>(
-                dyn_field_s, dyn_value_s, DataType::STRING, ct);
-        addAttribute<FloatingPointAttribute>(
-                dyn_field_n, DataType::FLOAT, ct);
-        addAttribute<IntegerAttribute>(
-                dyn_field_nai, DataType::INT32, ct);
-        addAttribute<StringAttribute>(
-                dyn_field_nas, DataType::STRING, ct);
-        addAttribute<IntegerAttribute>(
-                zcurve_field, dynamic_zcurve_value, DataType::INT64, ct);
+        addAttribute<IntegerAttribute>(dyn_field_i, dyn_value_i, DataType::INT32, ct);
+        addAttribute<FloatingPointAttribute>(dyn_field_d, dyn_value_d, DataType::DOUBLE, ct);
+        addAttribute<StringAttribute>(dyn_field_s, dyn_value_s, DataType::STRING, ct);
+        addAttribute<FloatingPointAttribute>(dyn_field_n, DataType::FLOAT, ct);
+        addAttribute<IntegerAttribute>(dyn_field_nai, DataType::INT32, ct);
+        addAttribute<StringAttribute>(dyn_field_nas, DataType::STRING, ct);
+        addAttribute<IntegerAttribute>(zcurve_field, dynamic_zcurve_value, DataType::INT64, ct);
         addTensorAttribute(dyn_field_tensor.c_str(), *dynamic_tensor);
-        PredicateAttribute *attr = addAttribute<PredicateAttribute>(
-                dyn_field_p, DataType::BOOLEANTREE, ct);
+        PredicateAttribute *attr = addAttribute<PredicateAttribute>(dyn_field_p, DataType::BOOLEANTREE, ct);
         attr->getIndex().indexEmptyDocument(lid);
         attr->commit();
         ct = schema::CollectionType::ARRAY;
-        addAttribute<IntegerAttribute>(
-                dyn_arr_field_i, dyn_value_i, DataType::INT32, ct);
-        addAttribute<FloatingPointAttribute>(
-                dyn_arr_field_d, dyn_value_d, DataType::DOUBLE, ct);
-        addAttribute<StringAttribute>(
-                dyn_arr_field_s, dyn_value_s, DataType::STRING, ct);
-        addAttribute<FloatingPointAttribute>(
-                dyn_arr_field_n, DataType::FLOAT, ct);
-        addAttribute<IntegerAttribute>(
-                zcurve_array_field, dynamic_zcurve_value, DataType::INT64, ct);
+        addAttribute<IntegerAttribute>(dyn_arr_field_i, dyn_value_i, DataType::INT32, ct);
+        addAttribute<FloatingPointAttribute>(dyn_arr_field_d, dyn_value_d, DataType::DOUBLE, ct);
+        addAttribute<StringAttribute>(dyn_arr_field_s, dyn_value_s, DataType::STRING, ct);
+        addAttribute<FloatingPointAttribute>(dyn_arr_field_n, DataType::FLOAT, ct);
+        addAttribute<IntegerAttribute>(zcurve_array_field, dynamic_zcurve_value, DataType::INT64, ct);
         ct = schema::CollectionType::WEIGHTEDSET;
-        addAttribute<IntegerAttribute>(
-                dyn_wset_field_i, dyn_value_i, DataType::INT32, ct);
-        addAttribute<FloatingPointAttribute>(
-                dyn_wset_field_d, dyn_value_d, DataType::DOUBLE, ct);
-        addAttribute<StringAttribute>(
-                dyn_wset_field_s, dyn_value_s, DataType::STRING, ct);
-        addAttribute<FloatingPointAttribute>(
-                dyn_wset_field_n, DataType::FLOAT, ct);
-        _retriever = std::make_unique<DocumentRetriever>(_dtName, repo, schema, meta_store, attr_manager, doc_store);
+        addAttribute<IntegerAttribute>(dyn_wset_field_i, dyn_value_i, DataType::INT32, ct);
+        addAttribute<FloatingPointAttribute>(dyn_wset_field_d, dyn_value_d, DataType::DOUBLE, ct);
+        addAttribute<StringAttribute>(dyn_wset_field_s, dyn_value_s, DataType::STRING, ct);
+        addAttribute<FloatingPointAttribute>(dyn_wset_field_n, DataType::FLOAT, ct);
+        build();
     }
 
     void clearAttributes(std::vector<vespalib::string> names) {
@@ -386,15 +380,13 @@ struct Fixture {
     }
 };
 
-TEST_F("require that document retriever can retrieve document meta data",
-       Fixture) {
+TEST_F("require that document retriever can retrieve document meta data", Fixture) {
     DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
     EXPECT_EQUAL(f.lid, meta_data.lid);
     EXPECT_EQUAL(f.timestamp, meta_data.timestamp);
 }
 
-TEST_F("require that document retriever can retrieve bucket meta data",
-       Fixture) {
+TEST_F("require that document retriever can retrieve bucket meta data", Fixture) {
     DocumentMetaData::Vector result;
     f._retriever->getBucketMetaData(makeSpiBucket(f.bucket_id, PartitionId(0)), result);
     ASSERT_EQUAL(1u, result.size());
@@ -476,7 +468,7 @@ TEST_F("require that attributes are patched into stored document", Fixture) {
 }
 
 TEST_F("require that attributes are patched into stored document unless also index field", Fixture) {
-    f.schema.addIndexField(Schema::IndexField(dyn_field_s, DataType::STRING));
+    f.addIndexField(Schema::IndexField(dyn_field_s, DataType::STRING)).build();
     DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
     Document::UP doc = f._retriever->getDocument(meta_data.lid);
     ASSERT_TRUE(doc.get());
@@ -574,9 +566,8 @@ TEST_F("require that tensor attribute can be retrieved", Fixture) {
     ASSERT_TRUE(doc.get());
 
     FieldValue::UP value = doc->getValue(dyn_field_tensor);
-    ASSERT_TRUE(value.get());
-    TensorFieldValue *tensor_value =
-        dynamic_cast<TensorFieldValue *>(value.get());
+    ASSERT_TRUE(value);
+    TensorFieldValue *tensor_value = dynamic_cast<TensorFieldValue *>(value.get());
     ASSERT_TRUE(tensor_value->getAsTensorPtr()->equals(*dynamic_tensor));
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
index 09550bd74dd..7466bcd99d5 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
@@ -143,7 +143,7 @@ public:
         }
     }
 
-    virtual bool allowVisitCaching() const override {
+    bool allowVisitCaching() const override {
         return _visitor.allowVisitCaching();
     }
 
@@ -154,7 +154,8 @@ private:
 
 }  // namespace
 
-Document::UP DocumentRetriever::getDocument(DocumentIdT lid) const
+Document::UP
+DocumentRetriever::getDocument(DocumentIdT lid) const
 {
     Document::UP doc = _doc_store.read(lid, getDocumentTypeRepo());
     if (doc) {
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp
index 65a4f7e7c4a..183e9eb0c0a 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp
@@ -26,7 +26,7 @@ DocumentRetrieverBase::DocumentRetrieverBase(
       _hasFields(hasFields)
 {
     const document::DocumentType * docType(_repo.getDocumentType(_docTypeName.getName()));
-    _emptyDoc.reset(new document::Document(*docType, DocumentId("id:empty:" + _docTypeName.getName() + "::empty")));
+    _emptyDoc = std::make_unique<document::Document>(*docType, DocumentId("id:empty:" + _docTypeName.getName() + "::empty"));
     _emptyDoc->setRepo(_repo);
 }
 
@@ -58,7 +58,7 @@ DocumentRetrieverBase::getDocumentMetaData(const DocumentId &id) const {
 const search::IAttributeManager *
 DocumentRetrieverBase::getAttrMgr() const
 {
-    return NULL;
+    return nullptr;
 }
 
     
@@ -71,7 +71,7 @@ DocumentRetrieverBase::parseSelect(const vespalib::string &selection) const
             return _selectCache[selection];
     }
     
-    CachedSelect::SP nselect(new CachedSelect());
+    auto nselect = std::make_shared<CachedSelect>();
     
     nselect->set(selection,
                  _docTypeName.getName(),
diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp
index af4af7f64fe..cd6df7c5816 100644
--- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "persistencehandlerproxy.h"
-#include "documentretriever.h"
 #include "documentdb.h"
 #include <vespa/searchcore/proton/feedoperation/createbucketoperation.h>
 #include <vespa/searchcore/proton/feedoperation/deletebucketoperation.h>
@@ -10,7 +9,6 @@
 #include <vespa/searchcore/proton/feedoperation/removeoperation.h>
 #include <vespa/searchcore/proton/feedoperation/splitbucketoperation.h>
 #include <vespa/searchcore/proton/feedoperation/updateoperation.h>
-#include <vespa/document/fieldvalue/document.h>
 #include <vespa/document/update/documentupdate.h>
 
 using storage::spi::Bucket;
@@ -18,8 +16,8 @@ using storage::spi::Timestamp;
 
 namespace proton {
 
-PersistenceHandlerProxy::PersistenceHandlerProxy(const DocumentDB::SP &documentDB)
-    : _documentDB(documentDB),
+PersistenceHandlerProxy::PersistenceHandlerProxy(DocumentDB::SP documentDB)
+    : _documentDB(std::move(documentDB)),
       _feedHandler(_documentDB->getFeedHandler()),
       _bucketHandler(_documentDB->getBucketHandler()),
       _clusterStateHandler(_documentDB->getClusterStateHandler())
diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h
index 5a82b46f91c..38ddb667213 100644
--- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h
+++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h
@@ -18,9 +18,9 @@ private:
     BucketHandler       &_bucketHandler;
     ClusterStateHandler &_clusterStateHandler;
 public:
-    PersistenceHandlerProxy(const std::shared_ptr<DocumentDB> &documentDB);
+    explicit PersistenceHandlerProxy(std::shared_ptr<DocumentDB> documentDB);
 
-    virtual ~PersistenceHandlerProxy();
+    ~PersistenceHandlerProxy() override;
 
 
     void initialize() override;