Merge pull request #13997 from vespa-engine/balder/cleanup-test-code-before-api-change

Cleanup code by following clion advise an move some classes to its pr…

18 files changed, 184 insertions, 191 deletions

diff --git a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
index b76d433337b..f52ca606900 100644
--- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
+++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
@@ -10,7 +10,7 @@
 #include <vespa/persistence/spi/test.h>
 #include <vespa/searchcore/proton/common/attribute_updater.h>
 #include <vespa/searchcore/proton/persistenceengine/document_iterator.h>
-#include <vespa/searchcore/proton/server/commit_and_wait_document_retriever.h>
+#include <vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.h>
 #include <vespa/searchlib/attribute/attributecontext.h>
 #include <vespa/searchlib/attribute/attributefactory.h>
 #include <vespa/searchlib/test/mock_attribute_manager.h>
@@ -116,7 +116,7 @@ struct UnitDR : DocumentRetrieverBaseForTest {
     UnitDR();
     UnitDR(document::Document::UP d, Timestamp t, Bucket b, bool r);
     UnitDR(const document::DocumentType &dt, document::Document::UP d, Timestamp t, Bucket b, bool r);
-    ~UnitDR();
+    ~UnitDR() override;
 
     const document::DocumentTypeRepo &getDocumentTypeRepo() const override {
         return repo;
@@ -134,7 +134,7 @@ struct UnitDR : DocumentRetrieverBaseForTest {
         return DocumentMetaData();
     }
     document::Document::UP getDocument(DocumentIdT lid) const override {
-        return Document::UP((lid == docid) ? document->clone() : 0);
+        return Document::UP((lid == docid) ? document->clone() : nullptr);
     }
 
     uint32_t getDocIdLimit() const override {
@@ -165,7 +165,7 @@ UnitDR::UnitDR(const document::DocumentType &dt, document::Document::UP d, Times
     : repo(dt), document(std::move(d)), timestamp(t), bucket(b), removed(r), docid(++_docidCnt),
       docIdLimit(std::numeric_limits<uint32_t>::max())
 {}
-UnitDR::~UnitDR() {}
+UnitDR::~UnitDR() = default;
 
 
 struct VisitRecordingUnitDR : UnitDR {
@@ -250,7 +250,7 @@ struct PairDR : DocumentRetrieverBaseForTest {
     IDocumentRetriever::SP first;
     IDocumentRetriever::SP second;
     PairDR(IDocumentRetriever::SP f, IDocumentRetriever::SP s)
-        : first(f), second(s) {}
+        : first(std::move(f)), second(std::move(s)) {}
     const document::DocumentTypeRepo &getDocumentTypeRepo() const override {
         return first->getDocumentTypeRepo();
     }
@@ -264,7 +264,7 @@ struct PairDR : DocumentRetrieverBaseForTest {
     }
     document::Document::UP getDocument(DocumentIdT lid) const override {
         Document::UP ret = first->getDocument(lid);
-        return (ret.get() != 0) ? std::move(ret) : second->getDocument(lid);
+        return ret ? std::move(ret) : second->getDocument(lid);
     }
 
     CachedSelect::SP parseSelect(const vespalib::string &selection) const override {
@@ -309,13 +309,13 @@ IDocumentRetriever::SP rem(const std::string &id, Timestamp t, Bucket b) {
 }
 
 IDocumentRetriever::SP cat(IDocumentRetriever::SP first, IDocumentRetriever::SP second) {
-    return IDocumentRetriever::SP(new PairDR(first, second));
+    return std::make_unique<PairDR>(std::move(first), std::move(second));
 }
 
 const DocumentType &getDocType() {
     static DocumentType::UP doc_type;
-    if (!doc_type.get()) {
-        doc_type.reset(new DocumentType("foo", 42));
+    if (!doc_type) {
+        doc_type = std::make_unique<DocumentType>("foo", 42);
         doc_type->addField(Field("header", 43, *DataType::STRING, true));
         doc_type->addField(Field("body", 44, *DataType::STRING, false));
     }
@@ -324,8 +324,8 @@ const DocumentType &getDocType() {
 
 const DocumentType &getAttrDocType() {
     static DocumentType::UP doc_type;
-    if (!doc_type.get()) {
-        doc_type.reset(new DocumentType("foo", 42));
+    if (!doc_type) {
+        doc_type = std::make_unique<DocumentType>("foo", 42);
         doc_type->addField(Field("header", 43, *DataType::STRING, true));
         doc_type->addField(Field("body", 44, *DataType::STRING, false));
         doc_type->addField(Field("aa", 45, *DataType::INT, false));
@@ -384,7 +384,7 @@ void checkDoc(const IDocumentRetriever &dr, const std::string &id,
     EXPECT_EQUAL(DocumentId(id).getGlobalId(), dmd.gid);
     EXPECT_EQUAL(removed, dmd.removed);
     Document::UP doc = dr.getDocument(dmd.lid);
-    ASSERT_TRUE(doc.get() != 0);
+    ASSERT_TRUE(doc);
     EXPECT_TRUE(DocumentId(id) == doc->getId());
 }
 
@@ -421,9 +421,9 @@ TEST("require that custom retrievers work as expected") {
             cat(doc("id:ns:document::3", Timestamp(7), bucket(6)),
                 nil()));
     EXPECT_FALSE(dr->getDocumentMetaData(DocumentId("id:ns:document::bogus")).valid());
-    EXPECT_TRUE(dr->getDocument(1).get() == 0);
-    EXPECT_TRUE(dr->getDocument(2).get() == 0);
-    EXPECT_TRUE(dr->getDocument(3).get() != 0);
+    EXPECT_FALSE(dr->getDocument(1));
+    EXPECT_FALSE(dr->getDocument(2));
+    EXPECT_TRUE(dr->getDocument(3));
     TEST_DO(checkDoc(*dr, "id:ns:document::1", 2, 5, false));
     TEST_DO(checkDoc(*dr, "id:ns:document::2", 3, 5, true));
     TEST_DO(checkDoc(*dr, "id:ns:document::3", 7, 6, false));
@@ -489,7 +489,7 @@ TEST("require that iterator ignoring maxbytes stops at the end, and does not aut
 
 void verifyReadConsistency(DocumentIterator & itr, Committer & committer) {
     IDocumentRetriever::SP retriever = doc("id:ns:document::1", Timestamp(2), bucket(5));
-    IDocumentRetriever::SP commitAndWaitRetriever(new CommitAndWaitDocumentRetriever(retriever, committer));
+    auto commitAndWaitRetriever = std::make_shared<CommitAndWaitDocumentRetriever>(retriever, committer);
     itr.add(commitAndWaitRetriever);
 
     IterateResult res = itr.iterate(largeNum);
@@ -517,7 +517,7 @@ TEST("require that readconsistency::strong does commit") {
 
 TEST("require that docid limit is honoured") {
     IDocumentRetriever::SP retriever = doc("id:ns:document::1", Timestamp(2), bucket(5));
-    UnitDR & udr = dynamic_cast<UnitDR &>(*retriever);
+    auto & udr = dynamic_cast<UnitDR &>(*retriever);
     udr.docid = 7;
     DocumentIterator itr(bucket(5), document::AllFields(), selectAll(), newestV(), -1, false);
     itr.add(retriever);
@@ -642,7 +642,7 @@ TEST("require that at least one document is returned by visit") {
     itr.add(cat(rem("id:ns:document::2", Timestamp(3), bucket(5)),
                 doc("id:ns:document::3", Timestamp(4), bucket(5))));
     IterateResult res1 = itr.iterate(0);
-    EXPECT_TRUE(1u <= res1.getEntries().size());
+    EXPECT_TRUE( ! res1.getEntries().empty());
     TEST_DO(checkEntry(res1, 0, Document(*DataType::DOCUMENT,DocumentId("id:ns:document::1")), Timestamp(2)));
 }
 
diff --git a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp
index 10b1c829674..5fac9ef9c94 100644
--- a/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/documentbucketmover/documentbucketmover_test.cpp
@@ -56,7 +56,7 @@ const uint32_t SECOND_SCAN_PASS = 2;
 struct MyMoveOperationLimiter : public IMoveOperationLimiter {
     uint32_t beginOpCount;
     MyMoveOperationLimiter() : beginOpCount(0) {}
-    virtual IDestructorCallback::SP beginOperation() override {
+    IDestructorCallback::SP beginOperation() override {
         ++beginOpCount;
         return IDestructorCallback::SP();
     }
@@ -76,7 +76,7 @@ struct MyMoveHandler : public IDocumentMoveHandler
           _storeMoveDoneContexts(storeMoveDoneContext),
           _moveDoneContexts()
     {}
-    virtual void handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) override {
+    void handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) override {
         _moves.push_back(op);
         if (_bucketDb.takeGuard()->isCachedBucket(op.getBucketId())) {
             ++_numCachedBuckets;
@@ -98,20 +98,17 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest
 {
     std::shared_ptr<const DocumentTypeRepo> _repo;
     DocumentVector       _docs;
-    MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo) : _repo(repo), _docs() {
+    MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo) : _repo(std::move(repo)), _docs() {
         _docs.push_back(Document::SP()); // lid 0 invalid
     }
-    virtual const document::DocumentTypeRepo &getDocumentTypeRepo() const override { return *_repo; }
-    virtual void getBucketMetaData(const storage::spi::Bucket &,
-                                   DocumentMetaData::Vector &) const override {}
-    virtual DocumentMetaData getDocumentMetaData(const DocumentId &) const override { return DocumentMetaData(); }
-    virtual Document::UP getDocument(DocumentIdT lid) const override {
+    const document::DocumentTypeRepo &getDocumentTypeRepo() const override { return *_repo; }
+    void getBucketMetaData(const storage::spi::Bucket &, DocumentMetaData::Vector &) const override {}
+    DocumentMetaData getDocumentMetaData(const DocumentId &) const override { return DocumentMetaData(); }
+    Document::UP getDocument(DocumentIdT lid) const override {
         return Document::UP(_docs[lid]->clone());
     }
 
-    virtual CachedSelect::SP
-    parseSelect(const vespalib::string &) const override
-    {
+    CachedSelect::SP parseSelect(const vespalib::string &) const override {
         return CachedSelect::SP();
     }
 };
@@ -119,7 +116,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest
 struct MyBucketModifiedHandler : public IBucketModifiedHandler
 {
     BucketIdVector _modified;
-    virtual void notifyBucketModified(const BucketId &bucket) override {
+    void notifyBucketModified(const BucketId &bucket) override {
         BucketIdVector::const_iterator itr = std::find(_modified.begin(), _modified.end(), bucket);
         ASSERT_TRUE(itr == _modified.end());
         _modified.push_back(bucket);
@@ -186,7 +183,7 @@ MySubDb::MySubDb(const std::shared_ptr<const DocumentTypeRepo> &repo, std::share
     : _metaStoreSP(std::make_shared<DocumentMetaStore>(bucketDB,
                                                        DocumentMetaStore::getFixedName(),
                                                        search::GrowStrategy(),
-                                                       documentmetastore::IGidCompare::SP(new documentmetastore::DefaultGidCompare),
+                                                       std::make_shared<documentmetastore::DefaultGidCompare>(),
                                                        subDbType)),
       _metaStore(*_metaStoreSP),
       _realRetriever(std::make_shared<MyDocumentRetriever>(repo)),
@@ -507,8 +504,7 @@ struct MyFrozenBucketHandler : public IFrozenBucketHandler
     {
     }
 
-    virtual ~MyFrozenBucketHandler()
-    {
+    ~MyFrozenBucketHandler() override {
         assert(_listeners.empty());
     }
 
@@ -523,14 +519,14 @@ struct MyFrozenBucketHandler : public IFrozenBucketHandler
         }
         return *this;
     }
-    virtual void addListener(IBucketFreezeListener *listener) override {
+    void addListener(IBucketFreezeListener *listener) override {
         _listeners.insert(listener);
     }
-    virtual void removeListener(IBucketFreezeListener *listener) override {
+    void removeListener(IBucketFreezeListener *listener) override {
         _listeners.erase(listener);
     }
 
-    virtual ExclusiveBucketGuard::UP acquireExclusiveBucket(BucketId bucket) override {
+    ExclusiveBucketGuard::UP acquireExclusiveBucket(BucketId bucket) override {
         return (_frozen.count(bucket) != 0)
             ? ExclusiveBucketGuard::UP()
             : std::make_unique<ExclusiveBucketGuard>(bucket);
@@ -539,10 +535,10 @@ struct MyFrozenBucketHandler : public IFrozenBucketHandler
 
 struct MyCountJobRunner : public IMaintenanceJobRunner {
     uint32_t runCount;
-    MyCountJobRunner(IMaintenanceJob &job) : runCount(0) {
+    explicit MyCountJobRunner(IMaintenanceJob &job) : runCount(0) {
         job.registerRunner(this);
     }
-    virtual void run() override { ++runCount; }
+    void run() override { ++runCount; }
 };
 
 struct ControllerFixtureBase
@@ -618,7 +614,7 @@ struct ControllerFixtureBase
 
 ControllerFixtureBase::ControllerFixtureBase(const BlockableMaintenanceJobConfig &blockableConfig, bool storeMoveDoneContexts)
     : _builder(),
-      _calc(new test::BucketStateCalculator),
+      _calc(std::make_shared<test::BucketStateCalculator>()),
       _bucketHandler(),
       _modifiedHandler(),
       _bucketDB(std::make_shared<BucketDBOwner>()),
diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp
index e679c028d1b..7303d4ef6d0 100644
--- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp
@@ -1,6 +1,5 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <vespa/searchcore/proton/server/i_disk_mem_usage_notifier.h>
 #include <vespa/searchcore/proton/server/i_lid_space_compaction_handler.h>
 #include <vespa/searchcore/proton/server/ifrozenbuckethandler.h>
 #include <vespa/searchcore/proton/server/imaintenancejobrunner.h>
@@ -49,13 +48,11 @@ struct MyScanIterator : public IDocumentScanIterator {
     LidVector _lids;
     LidVector::const_iterator _itr;
     bool _validItr;
-    MyScanIterator(const LidVector &lids) : _lids(lids), _itr(_lids.begin()), _validItr(true) {}
-    virtual bool valid() const override {
+    explicit MyScanIterator(const LidVector &lids) : _lids(lids), _itr(_lids.begin()), _validItr(true) {}
+    bool valid() const override {
         return _validItr;
     }
-    virtual search::DocumentMetaData next(uint32_t compactLidLimit,
-                                          uint32_t maxDocsToScan,
-                                          bool retry) override {
+    search::DocumentMetaData next(uint32_t compactLidLimit, uint32_t maxDocsToScan, bool retry) override {
         if (!retry && _itr != _lids.begin()) {
             ++_itr;
         }
@@ -86,8 +83,8 @@ struct MyHandler : public ILidSpaceCompactionHandler {
     documentmetastore::OperationListener::SP _op_listener;
     RemoveOperationsRateTracker* _rm_listener;
 
-    MyHandler(bool storeMoveDoneContexts = false);
-    ~MyHandler();
+    explicit MyHandler(bool storeMoveDoneContexts = false);
+    ~MyHandler() override;
     void clearMoveDoneContexts() { _moveDoneContexts.clear(); }
     void run_remove_ops(bool remove_batch) {
         // This ensures to max out the threshold time in the operation rate tracker.
@@ -101,45 +98,45 @@ struct MyHandler : public ILidSpaceCompactionHandler {
             _op_listener->notify_remove();
         }
     }
-    void stop_remove_ops(bool remove_batch) {
+    void stop_remove_ops(bool remove_batch) const {
         if (remove_batch) {
             _rm_listener->get_remove_batch_tracker().reset(vespalib::steady_clock::now());
         } else {
             _rm_listener->get_remove_tracker().reset(vespalib::steady_clock::now());
         }
     }
-    virtual vespalib::string getName() const override {
+    vespalib::string getName() const override {
         return "myhandler";
     }
-    virtual void set_operation_listener(documentmetastore::OperationListener::SP op_listener) override {
+    void set_operation_listener(documentmetastore::OperationListener::SP op_listener) override {
         auto* rm_listener = dynamic_cast<RemoveOperationsRateTracker*>(op_listener.get());
         assert(rm_listener != nullptr);
         _op_listener = std::move(op_listener);
         _rm_listener = rm_listener;
     }
-    virtual uint32_t getSubDbId() const override { return 2; }
-    virtual LidUsageStats getLidStatus() const override {
+    uint32_t getSubDbId() const override { return 2; }
+    LidUsageStats getLidStatus() const override {
         assert(_handleMoveCnt < _stats.size());
         return _stats[_handleMoveCnt];
     }
-    virtual IDocumentScanIterator::UP getIterator() const override {
+    IDocumentScanIterator::UP getIterator() const override {
         assert(_iteratorCnt < _lids.size());
-        return IDocumentScanIterator::UP(new MyScanIterator(_lids[_iteratorCnt++]));
+        return std::make_unique<MyScanIterator>(_lids[_iteratorCnt++]);
     }
-    virtual MoveOperation::UP createMoveOperation(const search::DocumentMetaData &document,
-                                                  uint32_t moveToLid) const override {
+    MoveOperation::UP createMoveOperation(const search::DocumentMetaData &document,
+                                          uint32_t moveToLid) const override {
         assert(document.lid > moveToLid);
         _moveFromLid = document.lid;
         _moveToLid = moveToLid;
-        return MoveOperation::UP(new MoveOperation());
+        return std::make_unique<MoveOperation>();
     }
-    virtual void handleMove(const MoveOperation &, IDestructorCallback::SP moveDoneCtx) override {
+    void handleMove(const MoveOperation &, IDestructorCallback::SP moveDoneCtx) override {
         ++_handleMoveCnt;
         if (_storeMoveDoneContexts) {
             _moveDoneContexts.push_back(std::move(moveDoneCtx));
         }
     }
-    virtual void handleCompactLidSpace(const CompactLidSpaceOperation &op) override {
+    void handleCompactLidSpace(const CompactLidSpaceOperation &op) override {
         _wantedSubDbId = op.getSubDbId();
         _wantedLidLimit = op.getLidLimit();
     }
@@ -159,7 +156,7 @@ MyHandler::MyHandler(bool storeMoveDoneContexts)
       _rm_listener()
 {}
 
-MyHandler::~MyHandler() {}
+MyHandler::~MyHandler() = default;
 
 struct MyStorer : public IOperationStorer {
     uint32_t _moveCnt;
@@ -180,17 +177,17 @@ struct MyStorer : public IOperationStorer {
 struct MyFrozenBucketHandler : public IFrozenBucketHandler {
     BucketId _bucket;
     MyFrozenBucketHandler() : _bucket() {}
-    virtual ExclusiveBucketGuard::UP acquireExclusiveBucket(BucketId bucket) override {
+    ExclusiveBucketGuard::UP acquireExclusiveBucket(BucketId bucket) override {
         return (_bucket == bucket)
                ? ExclusiveBucketGuard::UP()
                : std::make_unique<ExclusiveBucketGuard>(bucket);
     }
-    virtual void addListener(IBucketFreezeListener *) override { }
-    virtual void removeListener(IBucketFreezeListener *) override { }
+    void addListener(IBucketFreezeListener *) override { }
+    void removeListener(IBucketFreezeListener *) override { }
 };
 
 struct MyFeedView : public test::DummyFeedView {
-    MyFeedView(std::shared_ptr<const DocumentTypeRepo> repo)
+    explicit MyFeedView(std::shared_ptr<const DocumentTypeRepo> repo)
         : test::DummyFeedView(std::move(repo))
     {
     }
@@ -200,7 +197,7 @@ struct MyDocumentStore : public test::DummyDocumentStore {
     Document::SP _readDoc;
     mutable uint32_t _readLid;
     MyDocumentStore() : _readDoc(), _readLid(0) {}
-    ~MyDocumentStore();
+    ~MyDocumentStore() override;
     document::Document::UP read(search::DocumentIdT lid, const document::DocumentTypeRepo &) const override {
         _readLid = lid;
         return Document::UP(_readDoc->clone());
@@ -213,7 +210,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest {
     std::shared_ptr<const DocumentTypeRepo> repo;
     const MyDocumentStore& store;
     MyDocumentRetriever(std::shared_ptr<const DocumentTypeRepo> repo_in, const MyDocumentStore& store_in)
-        : repo(repo_in),
+        : repo(std::move(repo_in)),
           store(store_in)
     {
     }
@@ -229,11 +226,11 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest {
 struct MySubDb {
     test::DummyDocumentSubDb sub_db;
     MaintenanceDocumentSubDB maintenance_sub_db;
-    MySubDb(std::shared_ptr<BucketDBOwner> bucket_db, const MyDocumentStore& store, std::shared_ptr<const DocumentTypeRepo> repo);
+    MySubDb(std::shared_ptr<BucketDBOwner> bucket_db, const MyDocumentStore& store, const std::shared_ptr<const DocumentTypeRepo> & repo);
     ~MySubDb();
 };
 
-MySubDb::MySubDb(std::shared_ptr<BucketDBOwner> bucket_db, const MyDocumentStore& store, std::shared_ptr<const DocumentTypeRepo> repo)
+MySubDb::MySubDb(std::shared_ptr<BucketDBOwner> bucket_db, const MyDocumentStore& store, const std::shared_ptr<const DocumentTypeRepo> & repo)
     : sub_db(std::move(bucket_db), SUBDB_ID),
       maintenance_sub_db(sub_db.getName(), sub_db.getSubDbId(), sub_db.getDocumentMetaStoreContext().getSP(),
                          std::make_shared<MyDocumentRetriever>(repo, store),
@@ -245,20 +242,20 @@ MySubDb::~MySubDb() = default;
 
 struct MyDirectJobRunner : public IMaintenanceJobRunner {
     IMaintenanceJob &_job;
-    MyDirectJobRunner(IMaintenanceJob &job)
+    explicit MyDirectJobRunner(IMaintenanceJob &job)
         : _job(job)
     {
         _job.registerRunner(this);
     }
-    virtual void run() override { _job.run(); }
+    void run() override { _job.run(); }
 };
 
 struct MyCountJobRunner : public IMaintenanceJobRunner {
     uint32_t runCnt;
-    MyCountJobRunner(IMaintenanceJob &job) : runCnt(0) {
+    explicit MyCountJobRunner(IMaintenanceJob &job) : runCnt(0) {
         job.registerRunner(this);
     }
-    virtual void run() override { ++runCnt; }
+    void run() override { ++runCnt; }
 };
 
 struct JobTestBase : public ::testing::Test {
@@ -296,7 +293,7 @@ struct JobTestBase : public ::testing::Test {
                                                        BlockableMaintenanceJobConfig(resourceLimitFactor, maxOutstandingMoveOps),
                                                        _clusterStateHandler, nodeRetired);
     }
-    ~JobTestBase();
+    ~JobTestBase() override;
     JobTestBase &addStats(uint32_t docIdLimit,
                           const LidVector &usedLids,
                           const LidPairVector &usedFreePairs) {
@@ -309,8 +306,7 @@ struct JobTestBase : public ::testing::Test {
         for (auto pair : usedFreePairs) {
             uint32_t highestUsedLid = pair.first;
             uint32_t lowestFreeLid = pair.second;
-            _handler->_stats.push_back(LidUsageStats
-                    (docIdLimit, usedLids, lowestFreeLid, highestUsedLid));
+            _handler->_stats.emplace_back(docIdLimit, usedLids, lowestFreeLid, highestUsedLid);
         }
         _handler->_lids = usedLidsVector;
         return *this;
@@ -319,11 +315,10 @@ struct JobTestBase : public ::testing::Test {
                           uint32_t numDocs,
                           uint32_t lowestFreeLid,
                           uint32_t highestUsedLid) {
-        _handler->_stats.push_back(LidUsageStats
-                (docIdLimit, numDocs, lowestFreeLid, highestUsedLid));
+        _handler->_stats.emplace_back(docIdLimit, numDocs, lowestFreeLid, highestUsedLid);
         return *this;
     }
-    bool run() {
+    bool run() const {
         return _job->run();
     }
     JobTestBase &endScan() {
@@ -343,7 +338,7 @@ struct JobTestBase : public ::testing::Test {
                           uint32_t moveFromLid,
                           uint32_t handleMoveCnt,
                           uint32_t wantedLidLimit,
-                          uint32_t compactStoreCnt)
+                          uint32_t compactStoreCnt) const
     {
         EXPECT_EQ(moveToLid, _handler->_moveToLid);
         EXPECT_EQ(moveFromLid, _handler->_moveFromLid);
@@ -352,7 +347,7 @@ struct JobTestBase : public ::testing::Test {
         EXPECT_EQ(wantedLidLimit, _handler->_wantedLidLimit);
         EXPECT_EQ(compactStoreCnt, _storer._compactCnt);
     }
-    void assertNoWorkDone() {
+    void assertNoWorkDone() const {
         assertJobContext(0, 0, 0, 0, 0);
     }
     JobTestBase &setupOneDocumentToCompact() {
diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp
index 6eaa1bd373a..7e13f99d7b3 100644
--- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp
@@ -104,7 +104,7 @@ public:
     Document::UP
     getDocument(DocumentIdT lid) const
     {
-        DocMap::const_iterator it(_docs.find(lid));
+        auto it(_docs.find(lid));
         if (it != _docs.end()) {
             return Document::UP(it->second->clone());
         } else {
@@ -133,7 +133,7 @@ MyDocumentSubDB::MyDocumentSubDB(uint32_t subDBId, SubDbType subDbType, const st
     : _docs(),
       _subDBId(subDBId),
       _metaStoreSP(std::make_shared<DocumentMetaStore>(
-              bucketDB, DocumentMetaStore::getFixedName(), search::GrowStrategy(),
+              std::move(bucketDB), DocumentMetaStore::getFixedName(), search::GrowStrategy(),
               DocumentMetaStore::IGidCompare::SP(new DocumentMetaStore::DefaultGidCompare), subDbType)),
       _metaStore(*_metaStoreSP),
       _repo(repo),
@@ -141,42 +141,42 @@ MyDocumentSubDB::MyDocumentSubDB(uint32_t subDBId, SubDbType subDbType, const st
 {
     _metaStore.constructFreeList();
 }
-MyDocumentSubDB::~MyDocumentSubDB() {}
+MyDocumentSubDB::~MyDocumentSubDB() = default;
 
 struct MyDocumentRetriever : public DocumentRetrieverBaseForTest
 {
     MyDocumentSubDB &_subDB;
 
-    MyDocumentRetriever(MyDocumentSubDB &subDB)
+    explicit MyDocumentRetriever(MyDocumentSubDB &subDB)
         : _subDB(subDB)
     {
     }
 
-    virtual const document::DocumentTypeRepo &
+    const document::DocumentTypeRepo &
     getDocumentTypeRepo() const override
     {
         LOG_ABORT("should not be reached");
     }
 
-    virtual void
+    void
     getBucketMetaData(const storage::spi::Bucket &,
                       DocumentMetaData::Vector &) const override
     {
         LOG_ABORT("should not be reached");
     }
-    virtual DocumentMetaData
+    DocumentMetaData
     getDocumentMetaData(const DocumentId &) const override
     {
         return DocumentMetaData();
     }
 
-    virtual Document::UP
+    Document::UP
     getDocument(DocumentIdT lid) const override
     {
         return _subDB.getDocument(lid);
     }
 
-    virtual CachedSelect::SP
+    CachedSelect::SP
     parseSelect(const vespalib::string &) const override
     {
         return CachedSelect::SP();
@@ -187,7 +187,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest
 struct MyBucketModifiedHandler : public IBucketModifiedHandler
 {
     BucketIdVector _modified;
-    virtual void notifyBucketModified(const BucketId &bucket) override {
+    void notifyBucketModified(const BucketId &bucket) override {
         BucketIdVector::const_iterator itr = std::find(_modified.begin(), _modified.end(), bucket);
         if (itr == _modified.end()) {
             _modified.push_back(bucket);
@@ -218,17 +218,14 @@ class MyFeedHandler : public IDocumentMoveHandler,
     SerialNum                      _serialNum;
     uint32_t                       _heartBeats;
 public:
-    MyFeedHandler(FastOS_ThreadId &executorThreadId);
-
-    virtual~MyFeedHandler();
-
-    bool isExecutorThread();
+    explicit MyFeedHandler(FastOS_ThreadId &executorThreadId);
 
-    virtual void handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) override;
+    ~MyFeedHandler() override;
 
-    virtual void performPruneRemovedDocuments(PruneRemovedDocumentsOperation &op) override;
-
-    virtual void heartBeat() override;
+    bool isExecutorThread() const;
+    void handleMove(MoveOperation &op, IDestructorCallback::SP moveDoneCtx) override;
+    void performPruneRemovedDocuments(PruneRemovedDocumentsOperation &op) override;
+    void heartBeat() override;
 
     void setSubDBs(const std::vector<MyDocumentSubDB *> &subDBs);
 
@@ -237,9 +234,9 @@ public:
     }
 
     // Implements IOperationStorer
-    virtual void storeOperation(const FeedOperation &op, DoneCallback) override;
+    void storeOperation(const FeedOperation &op, DoneCallback) override;
 
-    uint32_t getHeartBeats() {
+    uint32_t getHeartBeats() const {
         return _heartBeats;
     }
 };
@@ -252,7 +249,7 @@ public:
 
     MyExecutor();
 
-    ~MyExecutor();
+    ~MyExecutor() override;
 
     bool isIdle();
     bool waitIdle(vespalib::duration timeout);
@@ -294,7 +291,7 @@ struct MySimpleJob : public BlockableMaintenanceJob
     {
     }
     void block() { setBlocked(BlockedReason::FROZEN_BUCKET); }
-    virtual bool run() override {
+    bool run() override {
         LOG(info, "MySimpleJob::run()");
         _latch.countDown();
         ++_runCnt;
@@ -310,7 +307,7 @@ struct MySplitJob : public MySimpleJob
         : MySimpleJob(delay, interval, finishCount)
     {
     }
-    virtual bool run() override {
+    bool run() override {
         LOG(info, "MySplitJob::run()");
         _latch.countDown();
         ++_runCnt;
@@ -329,7 +326,7 @@ struct MyLongRunningJob : public BlockableMaintenanceJob
     {
     }
     void block() { setBlocked(BlockedReason::FROZEN_BUCKET); }
-    virtual bool run() override {
+    bool run() override {
         _firstRun.countDown();
         usleep(10000);
         return false;
@@ -342,15 +339,15 @@ struct MockLidSpaceCompactionHandler : public ILidSpaceCompactionHandler
 {
     vespalib::string name;
 
-    MockLidSpaceCompactionHandler(const vespalib::string &name_) : name(name_) {}
-    virtual vespalib::string getName() const override { return name; }
-    virtual void set_operation_listener(documentmetastore::OperationListener::SP) override {}
-    virtual uint32_t getSubDbId() const override { return 0; }
-    virtual search::LidUsageStats getLidStatus() const override { return search::LidUsageStats(); }
-    virtual IDocumentScanIterator::UP getIterator() const override { return IDocumentScanIterator::UP(); }
-    virtual MoveOperation::UP createMoveOperation(const search::DocumentMetaData &, uint32_t) const override { return MoveOperation::UP(); }
-    virtual void handleMove(const MoveOperation &, IDestructorCallback::SP) override {}
-    virtual void handleCompactLidSpace(const CompactLidSpaceOperation &) override {}
+    explicit MockLidSpaceCompactionHandler(const vespalib::string &name_) : name(name_) {}
+    vespalib::string getName() const override { return name; }
+    void set_operation_listener(documentmetastore::OperationListener::SP) override {}
+    uint32_t getSubDbId() const override { return 0; }
+    search::LidUsageStats getLidStatus() const override { return search::LidUsageStats(); }
+    IDocumentScanIterator::UP getIterator() const override { return IDocumentScanIterator::UP(); }
+    MoveOperation::UP createMoveOperation(const search::DocumentMetaData &, uint32_t) const override { return MoveOperation::UP(); }
+    void handleMove(const MoveOperation &, IDestructorCallback::SP) override {}
+    void handleCompactLidSpace(const CompactLidSpaceOperation &) override {}
 };
 
 
@@ -385,7 +382,7 @@ public:
 
     MaintenanceControllerFixture();
 
-    virtual ~MaintenanceControllerFixture();
+    ~MaintenanceControllerFixture() override;
 
     void syncSubDBs();
     void commit() override { }
@@ -507,8 +504,7 @@ MyDocumentSubDB::getSubDB()
 
 
 void
-MyDocumentSubDB::handlePruneRemovedDocuments(
-        const PruneRemovedDocumentsOperation &op)
+MyDocumentSubDB::handlePruneRemovedDocuments(const PruneRemovedDocumentsOperation &op)
 {
     assert(_subDBId == 1u);
     typedef LidVectorContext::LidVector LidVector;
@@ -518,10 +514,7 @@ MyDocumentSubDB::handlePruneRemovedDocuments(
     _metaStore.removeBatch(lidsToRemove, lidCtx.getDocIdLimit());
     _metaStore.removeBatchComplete(lidsToRemove);
     _metaStore.commit(serialNum);
-    for (LidVector::const_iterator it = lidsToRemove.begin(),
-                                  ite = lidsToRemove.end();
-         it != ite; ++it) {
-        search::DocumentIdT lid(*it);
+    for (auto lid : lidsToRemove) {
         _docs.erase(lid);
     }
 }
@@ -689,13 +682,11 @@ MyFeedHandler::MyFeedHandler(FastOS_ThreadId &executorThreadId)
 }
 
 
-MyFeedHandler::~MyFeedHandler()
-{
-}
+MyFeedHandler::~MyFeedHandler() = default;
 
 
 bool
-MyFeedHandler::isExecutorThread()
+MyFeedHandler::isExecutorThread() const
 {
     FastOS_ThreadId threadId(FastOS_Thread::GetCurrentThreadId());
     return FastOS_Thread::CompareThreadIds(_executorThreadId, threadId);
@@ -934,10 +925,9 @@ void
 MaintenanceControllerFixture::insertDocs(const test::UserDocuments &docs, MyDocumentSubDB &subDb)
 {
 
-    for (auto itr = docs.begin(); itr != docs.end(); ++itr) {
-        const test::BucketDocuments &bucketDocs = itr->second;
-        for (size_t i = 0; i < bucketDocs.getDocs().size(); ++i) {
-            const test::Document &testDoc = bucketDocs.getDocs()[i];
+    for (const auto & entry : docs) {
+        const test::BucketDocuments &bucketDocs = entry.second;
+        for (const test::Document &testDoc : bucketDocs.getDocs()) {
             PutOperation op(testDoc.getBucket(), testDoc.getTimestamp(), testDoc.getDoc());
             op.setDbDocumentId(DbDocumentId(subDb.getSubDBId(), testDoc.getLid()));
             _fh.storeOperation(op, std::make_shared<search::IgnoreCallback>());
@@ -948,14 +938,12 @@ MaintenanceControllerFixture::insertDocs(const test::UserDocuments &docs, MyDocu
 
 
 void
-MaintenanceControllerFixture::removeDocs(const test::UserDocuments &docs,
-        Timestamp timestamp)
+MaintenanceControllerFixture::removeDocs(const test::UserDocuments &docs, Timestamp timestamp)
 {
 
-    for (auto itr = docs.begin(); itr != docs.end(); ++itr) {
-        const test::BucketDocuments &bucketDocs = itr->second;
-        for (size_t i = 0; i < bucketDocs.getDocs().size(); ++i) {
-            const test::Document &testDoc = bucketDocs.getDocs()[i];
+    for (const auto & entry : docs) {
+        const test::BucketDocuments &bucketDocs = entry.second;
+        for (const test::Document &testDoc : bucketDocs.getDocs()) {
             RemoveOperationWithDocId op(testDoc.getBucket(), timestamp, testDoc.getDoc()->getId());
             op.setDbDocumentId(DbDocumentId(_removed.getSubDBId(), testDoc.getLid()));
             _fh.storeOperation(op, std::make_shared<search::IgnoreCallback>());
@@ -1020,7 +1008,7 @@ TEST_F("require that document pruner is active",
        MaintenanceControllerFixture)
 {
     uint64_t tshz = 1000000;
-    uint64_t now = static_cast<uint64_t>(time(0)) * tshz;
+    uint64_t now = static_cast<uint64_t>(time(nullptr)) * tshz;
     Timestamp remTime(static_cast<Timestamp::Type>(now - 3600 * tshz));
     Timestamp keepTime(static_cast<Timestamp::Type>(now + 3600 * tshz));
     f._builder.createDocs(1, 1, 4); // 3 docs
@@ -1162,8 +1150,8 @@ TEST_F("require that active bucket is not moved until de-activated", Maintenance
 
 TEST_F("require that a simple maintenance job is executed", MaintenanceControllerFixture)
 {
-    IMaintenanceJob::UP job(new MySimpleJob(200ms, 200ms, 3));
-    MySimpleJob &myJob = static_cast<MySimpleJob &>(*job);
+    auto job = std::make_unique<MySimpleJob>(200ms, 200ms, 3);
+    MySimpleJob &myJob = *job;
     f._mc.registerJobInMasterThread(std::move(job));
     f._injectDefaultJobs = false;
     f.startMaintenance();
@@ -1174,8 +1162,8 @@ TEST_F("require that a simple maintenance job is executed", MaintenanceControlle
 
 TEST_F("require that a split maintenance job is executed", MaintenanceControllerFixture)
 {
-    IMaintenanceJob::UP job(new MySplitJob(200ms, TIMEOUT_SEC * 2, 3));
-    MySplitJob &myJob = static_cast<MySplitJob &>(*job);
+    auto job = std::make_unique<MySplitJob>(200ms, TIMEOUT_SEC * 2, 3);
+    MySplitJob &myJob = *job;
     f._mc.registerJobInMasterThread(std::move(job));
     f._injectDefaultJobs = false;
     f.startMaintenance();
@@ -1187,10 +1175,10 @@ TEST_F("require that a split maintenance job is executed", MaintenanceController
 TEST_F("require that a blocked job is unblocked and executed after thaw bucket",
         MaintenanceControllerFixture)
 {
-    IMaintenanceJob::UP job1(new MySimpleJob(TIMEOUT_SEC * 2, TIMEOUT_SEC * 2, 1));
-    MySimpleJob &myJob1 = static_cast<MySimpleJob &>(*job1);
-    IMaintenanceJob::UP job2(new MySimpleJob(TIMEOUT_SEC * 2, TIMEOUT_SEC * 2, 0));
-    MySimpleJob &myJob2 = static_cast<MySimpleJob &>(*job2);
+    auto job1 = std::make_unique<MySimpleJob>(TIMEOUT_SEC * 2, TIMEOUT_SEC * 2, 1);
+    MySimpleJob &myJob1 = *job1;
+    auto job2 = std::make_unique< MySimpleJob>(TIMEOUT_SEC * 2, TIMEOUT_SEC * 2, 0);
+    MySimpleJob &myJob2 = *job2;
     f._mc.registerJobInMasterThread(std::move(job1));
     f._mc.registerJobInMasterThread(std::move(job2));
     f._injectDefaultJobs = false;
@@ -1219,8 +1207,8 @@ TEST_F("require that a blocked job is unblocked and executed after thaw bucket",
 
 TEST_F("require that blocked jobs are not executed", MaintenanceControllerFixture)
 {
-    IMaintenanceJob::UP job(new MySimpleJob(200ms, 200ms, 0));
-    MySimpleJob &myJob = static_cast<MySimpleJob &>(*job);
+    auto job = std::make_unique<MySimpleJob>(200ms, 200ms, 0);
+    MySimpleJob &myJob = *job;
     myJob.block();
     f._mc.registerJobInMasterThread(std::move(job));
     f._injectDefaultJobs = false;
@@ -1234,7 +1222,7 @@ TEST_F("require that maintenance controller state list jobs", MaintenanceControl
     {
         IMaintenanceJob::UP job1(new MySimpleJob(TIMEOUT_SEC * 2, TIMEOUT_SEC * 2, 0));
         IMaintenanceJob::UP job2(new MyLongRunningJob(200ms, 200ms));
-        MyLongRunningJob &longRunningJob = static_cast<MyLongRunningJob &>(*job2);
+        auto &longRunningJob = dynamic_cast<MyLongRunningJob &>(*job2);
         f._mc.registerJobInMasterThread(std::move(job1));
         f._mc.registerJobInMasterThread(std::move(job2));
         f._injectDefaultJobs = false;
diff --git a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
index d6a3fea4735..f5346213a7a 100644
--- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
+++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
@@ -59,7 +59,7 @@ createDocType(const vespalib::string &name, int32_t id)
 document::Document::SP
 createDoc(const DocumentType &docType, const DocumentId &docId)
 {
-    return document::Document::SP(new document::Document(docType, docId));
+    return std::make_shared<document::Document>(docType, docId);
 }
 
 
@@ -116,19 +116,19 @@ struct MyDocumentRetriever : DocumentRetrieverBaseForTest {
         return repo;
     }
     void getBucketMetaData(const storage::spi::Bucket &, search::DocumentMetaData::Vector &v) const override {
-        if (document != 0) {
+        if (document != nullptr) {
             v.push_back(getDocumentMetaData(document->getId()));
         }
     }
     DocumentMetaData getDocumentMetaData(const DocumentId &id) const override {
         last_doc_id = id;
-        if (document != 0) {
+        if (document != nullptr) {
             return DocumentMetaData(1, timestamp, document::BucketId(1), document->getId().getGlobalId());
         }
         return DocumentMetaData();
     }
     document::Document::UP getDocument(search::DocumentIdT) const override {
-        if (document != 0) {
+        if (document != nullptr) {
             return Document::UP(document->clone());
         }
         return Document::UP();
@@ -155,9 +155,9 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
     Result                       _splitResult;
     Result                       _joinResult;
     Result                       _createBucketResult;
-    const Document *document;
-    std::multiset<uint64_t> frozen;
-    std::multiset<uint64_t> was_frozen;
+    const Document              *document;
+    std::multiset<uint64_t>      frozen;
+    std::multiset<uint64_t>      was_frozen;
 
     MyHandler()
         : initialized(false),
@@ -165,7 +165,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
           lastTimestamp(),
           lastDocId(),
           existingTimestamp(),
-          lastCalc(NULL),
+          lastCalc(nullptr),
           lastBucketState(),
           bucketList(),
           bucketStateResult(),
@@ -175,7 +175,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
           _splitResult(),
           _joinResult(),
           _createBucketResult(),
-          document(0),
+          document(nullptr),
           frozen(),
           was_frozen()
     {
@@ -188,7 +188,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
         document = &doc;
         setExistingTimestamp(ts);
     }
-    void handle(FeedToken token, const Bucket &bucket, Timestamp timestamp, const DocumentId &docId) {
+    void handle(const FeedToken & token, const Bucket &bucket, Timestamp timestamp, const DocumentId &docId) {
         (void) token;
         lastBucket = bucket;
         lastTimestamp = timestamp;
@@ -199,20 +199,20 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
 
     void handlePut(FeedToken token, const Bucket& bucket,
                    Timestamp timestamp, DocumentSP doc) override {
-        token->setResult(ResultUP(new storage::spi::Result()), false);
+        token->setResult(std::make_unique<Result>(), false);
         handle(token, bucket, timestamp, doc->getId());
     }
 
     void handleUpdate(FeedToken token, const Bucket& bucket,
                       Timestamp timestamp, const document::DocumentUpdate::SP& upd) override {
-        token->setResult(ResultUP(new storage::spi::UpdateResult(existingTimestamp)), existingTimestamp > 0);
+        token->setResult(std::make_unique<UpdateResult>(existingTimestamp), existingTimestamp > 0);
         handle(token, bucket, timestamp, upd->getId());
     }
 
     void handleRemove(FeedToken token, const Bucket& bucket,
                       Timestamp timestamp, const DocumentId& id) override {
         bool wasFound = existingTimestamp > 0;
-        token->setResult(ResultUP(new storage::spi::RemoveResult(wasFound)), wasFound);
+        token->setResult(std::make_unique<RemoveResult>(wasFound), wasFound);
         handle(token, bucket, timestamp, id);
     }
 
@@ -237,11 +237,11 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
     }
 
     void handleCreateBucket(FeedToken token, const storage::spi::Bucket &) override {
-        token->setResult(ResultUP(new Result(_createBucketResult)), true);
+        token->setResult(std::make_unique<Result>(_createBucketResult), true);
     }
 
     void handleDeleteBucket(FeedToken token, const storage::spi::Bucket &) override {
-        token->setResult(ResultUP(new Result(deleteBucketResult)), true);
+        token->setResult(std::make_unique<Result>(deleteBucketResult), true);
     }
 
     void handleGetModifiedBuckets(IBucketIdListResultHandler &resultHandler) override {
@@ -251,24 +251,24 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
     void handleSplit(FeedToken token, const storage::spi::Bucket &, const storage::spi::Bucket &,
                      const storage::spi::Bucket &) override
     {
-        token->setResult(ResultUP(new Result(_splitResult)), true);
+        token->setResult(std::make_unique<Result>(_splitResult), true);
     }
 
     void handleJoin(FeedToken token, const storage::spi::Bucket &, const storage::spi::Bucket &,
                const storage::spi::Bucket &) override
     {
-        token->setResult(ResultUP(new Result(_joinResult)), true);
+        token->setResult(std::make_unique<Result>(_joinResult), true);
     }
 
     RetrieversSP getDocumentRetrievers(storage::spi::ReadConsistency) override {
         RetrieversSP ret(new std::vector<IDocumentRetriever::SP>);
-        ret->push_back(IDocumentRetriever::SP(new MyDocumentRetriever(0, Timestamp(), lastDocId)));
+        ret->push_back(IDocumentRetriever::SP(new MyDocumentRetriever(nullptr, Timestamp(), lastDocId)));
         ret->push_back(IDocumentRetriever::SP(new MyDocumentRetriever(document, existingTimestamp, lastDocId)));
         return ret;
     }
 
     BucketGuard::UP lockBucket(const storage::spi::Bucket &b) override {
-        return BucketGuard::UP(new BucketGuard(b.getBucketId(), *this));
+        return std::make_unique<BucketGuard>(b.getBucketId(), *this);
     }
 
     void handleListActiveBuckets(IBucketIdListResultHandler &resultHandler) override {
@@ -286,7 +286,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
         was_frozen.insert(bucket.getId());
     }
     void thawBucket(BucketId bucket) override {
-        std::multiset<uint64_t>::iterator it = frozen.find(bucket.getId());
+        auto it = frozen.find(bucket.getId());
         ASSERT_TRUE(it != frozen.end());
         frozen.erase(it);
     }
@@ -311,10 +311,10 @@ struct HandlerSet {
 };
 
 HandlerSet::HandlerSet()
-    : phandler1(new MyHandler()),
-      phandler2(new MyHandler()),
-      handler1(static_cast<MyHandler &>(*phandler1.get())),
-      handler2(static_cast<MyHandler &>(*phandler2.get()))
+    : phandler1(std::make_shared<MyHandler>()),
+      phandler2(std::make_shared<MyHandler>()),
+      handler1(dynamic_cast<MyHandler &>(*phandler1.get())),
+      handler2(dynamic_cast<MyHandler &>(*phandler2.get()))
 {}
 HandlerSet::~HandlerSet() = default;
 
@@ -398,7 +398,7 @@ struct SimpleFixture {
     SimpleResourceWriteFilter _writeFilter;
     PersistenceEngine engine;
     HandlerSet hset;
-    SimpleFixture(BucketSpace bucketSpace2)
+    explicit SimpleFixture(BucketSpace bucketSpace2)
         : _owner(),
           engine(_owner, _writeFilter, -1, false),
           hset()
diff --git a/searchcore/src/vespa/searchcore/proton/server/icommitable.h b/searchcore/src/vespa/searchcore/proton/common/icommitable.h
index d1c2a1277cc..a9d7f5e85ab 100644
--- a/searchcore/src/vespa/searchcore/proton/server/icommitable.h
+++ b/searchcore/src/vespa/searchcore/proton/common/icommitable.h
@@ -12,7 +12,7 @@ public:
     virtual void commit() = 0;
     virtual void commitAndWait() = 0;
 protected:
-    virtual ~ICommitable() { }
+    virtual ~ICommitable() = default;
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/persistenceengine/CMakeLists.txt
index 7b0727d5496..989f6f16b1b 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/CMakeLists.txt
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/CMakeLists.txt
@@ -2,6 +2,7 @@
 vespa_add_library(searchcore_persistenceengine STATIC
     SOURCES
     document_iterator.cpp
+    commit_and_wait_document_retriever.cpp
     i_document_retriever.cpp
     persistenceengine.cpp
     persistence_handler_map.cpp
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp
new file mode 100644
index 00000000000..f3c8df31aa8
--- /dev/null
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.cpp
@@ -0,0 +1,14 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "commit_and_wait_document_retriever.h"
+
+namespace proton {
+
+CommitAndWaitDocumentRetriever::CommitAndWaitDocumentRetriever(IDocumentRetriever::SP retriever, ICommitable &commit)
+    : _retriever(std::move(retriever)),
+      _commit(commit)
+{ }
+
+CommitAndWaitDocumentRetriever::~CommitAndWaitDocumentRetriever() = default;
+
+} // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/commit_and_wait_document_retriever.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.h
index ec1f2cbcdc3..55e5ecdcdcc 100644
--- a/searchcore/src/vespa/searchcore/proton/server/commit_and_wait_document_retriever.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.h
@@ -2,8 +2,8 @@
 
 #pragma once
 
-#include <vespa/searchcore/proton/persistenceengine/i_document_retriever.h>
-#include <vespa/searchcore/proton/server/icommitable.h>
+#include "i_document_retriever.h"
+#include <vespa/searchcore/proton/common/icommitable.h>
 #include <vespa/document/fieldvalue/document.h>
 
 namespace proton {
@@ -19,12 +19,8 @@ class CommitAndWaitDocumentRetriever : public IDocumentRetriever
     ICommitable           &_commit;
     using Bucket = storage::spi::Bucket;
 public:
-    CommitAndWaitDocumentRetriever(const IDocumentRetriever::SP &retriever, ICommitable &commit)
-        : _retriever(retriever),
-          _commit(commit)
-    { }
-
-    ~CommitAndWaitDocumentRetriever() {}
+    CommitAndWaitDocumentRetriever(IDocumentRetriever::SP retriever, ICommitable &commit);
+    ~CommitAndWaitDocumentRetriever() override;
 
     const document::DocumentTypeRepo &getDocumentTypeRepo() const override {
         return _retriever->getDocumentTypeRepo();
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/i_document_retriever.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/i_document_retriever.h
index 3f4bd0a406d..1ec03426d20 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/i_document_retriever.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/i_document_retriever.h
@@ -30,7 +30,7 @@ public:
     using LidVector = search::IDocumentStore::LidVector;
     using DocumentUP = std::unique_ptr<document::Document>;
 
-    virtual ~IDocumentRetriever() {}
+    virtual ~IDocumentRetriever() = default;
 
     virtual const document::DocumentTypeRepo &getDocumentTypeRepo() const = 0;
     virtual void getBucketMetaData(const storage::spi::Bucket &bucket, search::DocumentMetaData::Vector &result) const = 0;
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
index 7567aa9c322..35c72db0c6f 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
@@ -2,7 +2,6 @@
 
 #include "bootstrapconfig.h"
 #include "combiningfeedview.h"
-#include "commit_and_wait_document_retriever.h"
 #include "document_meta_store_read_guards.h"
 #include "document_subdb_collection_explorer.h"
 #include "documentdb.h"
@@ -12,6 +11,7 @@
 #include "lid_space_compaction_handler.h"
 #include "maintenance_jobs_injector.h"
 #include "reconfig_params.h"
+#include <vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.h>
 #include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/searchcore/proton/attribute/attribute_config_inspector.h>
 #include <vespa/searchcore/proton/attribute/attribute_writer.h>
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb_commit_job.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb_commit_job.cpp
index 81979bbfb94..80ef7a74f6d 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb_commit_job.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb_commit_job.cpp
@@ -1,7 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "documentdb_commit_job.h"
-#include "icommitable.h"
+#include <vespa/searchcore/proton/common/icommitable.h>
 
 namespace proton {
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp
index 1e01203b431..db69704663f 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp
@@ -1,12 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "combiningfeedview.h"
-#include "commit_and_wait_document_retriever.h"
 #include "document_subdb_collection_initializer.h"
 #include "documentsubdbcollection.h"
 #include "i_document_subdb_owner.h"
 #include "maintenancecontroller.h"
 #include "searchabledocsubdb.h"
+#include <vespa/searchcore/proton/persistenceengine/commit_and_wait_document_retriever.h>
 #include <vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h>
 #include <vespa/vespalib/util/lambdatask.h>
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h
index f0859335919..a60cad1cb2b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h
+++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h
@@ -6,7 +6,7 @@
 #include "i_lid_space_compaction_handler.h"
 #include "i_operation_storer.h"
 #include "iheartbeathandler.h"
-#include "icommitable.h"
+#include <vespa/searchcore/proton/common/icommitable.h>
 #include <vespa/searchcore/proton/matching/isessioncachepruner.h>
 #include <vespa/searchcore/proton/metrics/documentdb_job_trackers.h>
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp
index 1281785e898..2865d8c0536 100644
--- a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp
@@ -21,6 +21,8 @@ VisibilityHandler::VisibilityHandler(const IGetSerialNum & serial,
 {
 }
 
+VisibilityHandler::~VisibilityHandler() = default;
+
 void VisibilityHandler::commit()
 {
     if (hasVisibilityDelay()) {
diff --git a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h
index 3bcdca5fc40..250ef03f846 100644
--- a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h
+++ b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h
@@ -3,8 +3,8 @@
 #pragma once
 
 #include "ifeedview.h"
-#include "icommitable.h"
 #include "igetserialnum.h"
+#include <vespa/searchcore/proton/common/icommitable.h>
 #include <vespa/searchcorespi/index/ithreadingservice.h>
 #include <vespa/vespalib/util/varholder.h>
 #include <mutex>
@@ -25,6 +25,7 @@ public:
     VisibilityHandler(const IGetSerialNum &serial,
                       IThreadingService &threadingService,
                       const FeedViewHolder &feedView);
+    ~VisibilityHandler() override;
     void setVisibilityDelay(vespalib::duration visibilityDelay) { _visibilityDelay = visibilityDelay; }
     vespalib::duration getVisibilityDelay() const { return _visibilityDelay; }
     bool hasVisibilityDelay() const { return _visibilityDelay != vespalib::duration::zero(); }
diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.cpp
index df23d79e08f..53ca9e68676 100644
--- a/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.cpp
+++ b/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.cpp
@@ -9,8 +9,8 @@ DummyFeedView::DummyFeedView()
 {
 }
 
-DummyFeedView::DummyFeedView(const std::shared_ptr<const document::DocumentTypeRepo> &docTypeRepo)
-    : _docTypeRepo(docTypeRepo)
+DummyFeedView::DummyFeedView(std::shared_ptr<const document::DocumentTypeRepo> docTypeRepo)
+    : _docTypeRepo(std::move(docTypeRepo))
 {
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h b/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h
index 36079da73c7..556f4188afa 100644
--- a/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h
+++ b/searchcore/src/vespa/searchcore/proton/test/dummy_feed_view.h
@@ -10,8 +10,8 @@ struct DummyFeedView : public IFeedView
     std::shared_ptr<const document::DocumentTypeRepo> _docTypeRepo;
 
     DummyFeedView();
-    DummyFeedView(const std::shared_ptr<const document::DocumentTypeRepo> &docTypeRepo);
-    virtual ~DummyFeedView() override;
+    explicit DummyFeedView(std::shared_ptr<const document::DocumentTypeRepo> docTypeRepo);
+    ~DummyFeedView() override;
     const std::shared_ptr<const document::DocumentTypeRepo> &getDocumentTypeRepo() const override {
         return _docTypeRepo;
     }