Simplify DocEntry to get a clean interface with multiple implementations, instead of an mutant.

Also add tests for the different variations a DocEntry can have.

24 files changed, 47 insertions, 30 deletions

diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp
index 146dcab2ba7..56e20309c17 100644
--- a/storage/src/tests/persistence/testandsettest.cpp
+++ b/storage/src/tests/persistence/testandsettest.cpp
@@ -10,6 +10,7 @@
 #include <vespa/document/fieldset/fieldsets.h>
 #include <vespa/persistence/spi/test.h>
 #include <vespa/persistence/spi/persistenceprovider.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <functional>
 
 using std::unique_ptr;
diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp
index 494af8a0eff..1b5f077aaf0 100644
--- a/storage/src/tests/visiting/visitortest.cpp
+++ b/storage/src/tests/visiting/visitortest.cpp
@@ -16,6 +16,7 @@
 #include <tests/common/teststorageapp.h>
 #include <tests/common/dummystoragelink.h>
 #include <tests/storageserver/testvisitormessagesession.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/vespalib/io/fileutil.h>
 #include <vespa/vespalib/gtest/gtest.h>
 #include <thread>
diff --git a/storage/src/vespa/storage/persistence/asynchandler.cpp b/storage/src/vespa/storage/persistence/asynchandler.cpp
index b5161673af3..3d24ee87879 100644
--- a/storage/src/vespa/storage/persistence/asynchandler.cpp
+++ b/storage/src/vespa/storage/persistence/asynchandler.cpp
@@ -6,6 +6,7 @@
 #include "bucketownershipnotifier.h"
 #include "bucketprocessor.h"
 #include <vespa/persistence/spi/persistenceprovider.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/persistence/spi/catchresult.h>
 #include <vespa/storageapi/message/bucket.h>
 #include <vespa/document/update/documentupdate.h>
diff --git a/storage/src/vespa/storage/persistence/bucketprocessor.h b/storage/src/vespa/storage/persistence/bucketprocessor.h
index c605a9338e3..002cba6c15c 100644
--- a/storage/src/vespa/storage/persistence/bucketprocessor.h
+++ b/storage/src/vespa/storage/persistence/bucketprocessor.h
@@ -7,11 +7,14 @@
 #pragma once
 
 #include <vespa/persistence/spi/bucket.h>
-#include <vespa/persistence/spi/docentry.h>
 #include <vespa/persistence/spi/context.h>
+#include <persistence/spi/types.h>
 
 namespace document { class FieldSet; }
-namespace storage::spi { struct PersistenceProvider; }
+namespace storage::spi {
+    struct PersistenceProvider;
+    class DocEntry;
+}
 
 namespace storage {
 
diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp
index 0c9cecdb6a1..aff6332439b 100644
--- a/storage/src/vespa/storage/persistence/mergehandler.cpp
+++ b/storage/src/vespa/storage/persistence/mergehandler.cpp
@@ -6,7 +6,7 @@
 #include "apply_bucket_diff_state.h"
 #include <vespa/storage/persistence/filestorage/mergestatus.h>
 #include <vespa/persistence/spi/persistenceprovider.h>
-#include <vespa/persistence/spi/catchresult.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/vdslib/distribution/distribution.h>
 #include <vespa/document/fieldset/fieldsets.h>
 #include <vespa/vespalib/objects/nbostream.h>
diff --git a/storage/src/vespa/storage/persistence/mergehandler.h b/storage/src/vespa/storage/persistence/mergehandler.h
index 5e66b364242..0f427c11eec 100644
--- a/storage/src/vespa/storage/persistence/mergehandler.h
+++ b/storage/src/vespa/storage/persistence/mergehandler.h
@@ -16,7 +16,6 @@
 #include "types.h"
 #include "merge_bucket_info_syncer.h"
 #include <vespa/persistence/spi/bucket.h>
-#include <vespa/persistence/spi/docentry.h>
 #include <vespa/storageapi/message/bucket.h>
 #include <vespa/storage/common/cluster_context.h>
 #include <vespa/storage/common/messagesender.h>
@@ -30,9 +29,9 @@ namespace storage {
 namespace spi {
     struct PersistenceProvider;
     class Context;
+    class DocEntry;
 }
 class PersistenceUtil;
-class ApplyBucketDiffEntryResult;
 class ApplyBucketDiffState;
 class MergeStatus;
 
@@ -117,7 +116,7 @@ private:
      */
     void populateMetaData(const spi::Bucket&,
                           Timestamp maxTimestamp,
-                          std::vector<spi::DocEntry::UP>& entries,
+                          std::vector<std::unique_ptr<spi::DocEntry>> & entries,
                           spi::Context& context) const;
 
     Document::UP deserializeDiffDocument(
diff --git a/storage/src/vespa/storage/persistence/messages.cpp b/storage/src/vespa/storage/persistence/messages.cpp
index c6f7442cfdc..cf05ccd65b8 100644
--- a/storage/src/vespa/storage/persistence/messages.cpp
+++ b/storage/src/vespa/storage/persistence/messages.cpp
@@ -1,8 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "messages.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <ostream>
-#include <cassert>
 
 using document::BucketSpace;
 
diff --git a/storage/src/vespa/storage/persistence/messages.h b/storage/src/vespa/storage/persistence/messages.h
index 594506f53ba..86cf1ac3b06 100644
--- a/storage/src/vespa/storage/persistence/messages.h
+++ b/storage/src/vespa/storage/persistence/messages.h
@@ -2,7 +2,6 @@
 #pragma once
 
 #include <vespa/storageapi/message/internal.h>
-#include <vespa/persistence/spi/docentry.h>
 #include <vespa/persistence/spi/bucket.h>
 #include <vespa/persistence/spi/selection.h>
 #include <vespa/persistence/spi/read_consistency.h>
@@ -10,6 +9,8 @@
 
 namespace storage {
 
+namespace spi { class DocEntry; }
+
 class GetIterCommand : public api::InternalCommand {
 private:
     document::Bucket _bucket;
@@ -44,9 +45,10 @@ private:
 
 class GetIterReply : public api::InternalReply {
 private:
+    using List = std::vector<std::unique_ptr<spi::DocEntry>>;
     document::Bucket _bucket;
-    std::vector<spi::DocEntry::UP> _entries;
-    bool _completed;
+    List             _entries;
+    bool             _completed;
 
 public:
     typedef std::unique_ptr<GetIterReply> UP;
@@ -58,13 +60,9 @@ public:
 
     document::Bucket getBucket() const override { return _bucket; }
 
-    const std::vector<spi::DocEntry::UP>& getEntries() const {
-        return _entries;
-    }
+    const List & getEntries() const { return _entries; }
 
-    std::vector<spi::DocEntry::UP>& getEntries() {
-        return _entries;
-    }
+    List & getEntries() { return _entries; }
 
     void setCompleted(bool completed = true) { _completed = completed; }
     bool isCompleted() const { return _completed; }
diff --git a/storage/src/vespa/storage/persistence/processallhandler.cpp b/storage/src/vespa/storage/persistence/processallhandler.cpp
index 6a2b5e79450..3da6372cda8 100644
--- a/storage/src/vespa/storage/persistence/processallhandler.cpp
+++ b/storage/src/vespa/storage/persistence/processallhandler.cpp
@@ -5,6 +5,7 @@
 #include "persistenceutil.h"
 #include <vespa/document/fieldset/fieldsets.h>
 #include <vespa/persistence/spi/persistenceprovider.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/vespalib/util/stringfmt.h>
 
 #include <vespa/log/log.h>
diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp
index 752c1175f22..b7344098698 100644
--- a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp
+++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp
@@ -2,6 +2,7 @@
 
 #include "provider_error_wrapper.h"
 #include "persistenceutil.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/vespalib/util/idestructorcallback.h>
 
 namespace storage {
diff --git a/storage/src/vespa/storage/persistence/simplemessagehandler.cpp b/storage/src/vespa/storage/persistence/simplemessagehandler.cpp
index 9a7a451b906..74813e2e891 100644
--- a/storage/src/vespa/storage/persistence/simplemessagehandler.cpp
+++ b/storage/src/vespa/storage/persistence/simplemessagehandler.cpp
@@ -3,6 +3,7 @@
 #include "simplemessagehandler.h"
 #include "persistenceutil.h"
 #include <vespa/persistence/spi/persistenceprovider.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/storageapi/message/bucket.h>
 #include <vespa/document/base/exceptions.h>
 #include <vespa/document/fieldset/fieldsetrepo.h>
diff --git a/storage/src/vespa/storage/persistence/splitbitdetector.cpp b/storage/src/vespa/storage/persistence/splitbitdetector.cpp
index bd472868e06..2a9ac635cff 100644
--- a/storage/src/vespa/storage/persistence/splitbitdetector.cpp
+++ b/storage/src/vespa/storage/persistence/splitbitdetector.cpp
@@ -3,6 +3,7 @@
 #include "splitbitdetector.h"
 #include "bucketprocessor.h"
 #include <vespa/persistence/spi/persistenceprovider.h>
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/document/bucket/bucketidfactory.h>
 #include <vespa/document/base/documentid.h>
 #include <vespa/document/fieldset/fieldsets.h>
diff --git a/storage/src/vespa/storage/visiting/countvisitor.cpp b/storage/src/vespa/storage/visiting/countvisitor.cpp
index e20699aa799..3971544a9a0 100644
--- a/storage/src/vespa/storage/visiting/countvisitor.cpp
+++ b/storage/src/vespa/storage/visiting/countvisitor.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "countvisitor.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/documentapi/messagebus/messages/visitor.h>
 #include <vespa/vespalib/util/stringfmt.h>
@@ -22,7 +23,7 @@ CountVisitor::CountVisitor(StorageComponent& component,
 
 void
 CountVisitor::handleDocuments(const document::BucketId& /*bucketId*/,
-                              std::vector<spi::DocEntry::UP>& entries,
+                              DocEntryList& entries,
                               HitCounter& hitCounter)
 {
     for (size_t i = 0; i < entries.size(); ++i) {
diff --git a/storage/src/vespa/storage/visiting/countvisitor.h b/storage/src/vespa/storage/visiting/countvisitor.h
index 4c436b3d07c..4120a96225f 100644
--- a/storage/src/vespa/storage/visiting/countvisitor.h
+++ b/storage/src/vespa/storage/visiting/countvisitor.h
@@ -18,11 +18,11 @@ public:
     CountVisitor(StorageComponent&,
                  const vdslib::Parameters& params);
 
-    virtual void completedVisiting(HitCounter&) override;
+    void completedVisiting(HitCounter&) override;
 
 private:
     void handleDocuments(const document::BucketId& bucketId,
-                         std::vector<spi::DocEntry::UP>& entries,
+                         DocEntryList & entries,
                          HitCounter& hitCounter) override;
 
     bool _doScheme;
diff --git a/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp b/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp
index 704aaa219cb..92c080169ac 100644
--- a/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp
+++ b/storage/src/vespa/storage/visiting/dumpvisitorsingle.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "dumpvisitorsingle.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/document/update/documentupdate.h>
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/documentapi/messagebus/messages/putdocumentmessage.h>
@@ -16,8 +17,8 @@ DumpVisitorSingle::DumpVisitorSingle(StorageComponent& component, const vdslib::
 {
 }
 
-void DumpVisitorSingle::handleDocuments(const document::BucketId& /*bucketId*/,
-                                        std::vector<spi::DocEntry::UP>& entries,
+void DumpVisitorSingle::handleDocuments(const document::BucketId&,
+                                        DocEntryList& entries,
                                         HitCounter& hitCounter)
 {
     LOG(debug, "Visitor %s handling block of %zu documents.",
diff --git a/storage/src/vespa/storage/visiting/dumpvisitorsingle.h b/storage/src/vespa/storage/visiting/dumpvisitorsingle.h
index 81f4ab7e989..c98bad17e84 100644
--- a/storage/src/vespa/storage/visiting/dumpvisitorsingle.h
+++ b/storage/src/vespa/storage/visiting/dumpvisitorsingle.h
@@ -19,7 +19,7 @@ public:
                       const vdslib::Parameters& params);
 
 private:
-    void handleDocuments(const document::BucketId&, std::vector<spi::DocEntry::UP>&, HitCounter&) override;
+    void handleDocuments(const document::BucketId&, DocEntryList&, HitCounter&) override;
 };
 
 struct DumpVisitorSingleFactory : public VisitorFactory {
diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp
index df965f93ae6..1c3c97d7dcb 100644
--- a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp
+++ b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp
@@ -2,6 +2,7 @@
 
 
 #include "recoveryvisitor.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/vespalib/objects/nbostream.h>
 #include <vespa/documentapi/messagebus/messages/visitor.h>
 #include <vespa/vespalib/text/stringtokenizer.h>
@@ -31,7 +32,7 @@ RecoveryVisitor::RecoveryVisitor(StorageComponent& component,
 
 void
 RecoveryVisitor::handleDocuments(const document::BucketId& bid,
-                                 std::vector<spi::DocEntry::UP>& entries,
+                                 DocEntryList & entries,
                                  HitCounter& hitCounter)
 {
     std::lock_guard guard(_mutex);
diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.h b/storage/src/vespa/storage/visiting/recoveryvisitor.h
index 045c8aeb484..e850eca3f37 100644
--- a/storage/src/vespa/storage/visiting/recoveryvisitor.h
+++ b/storage/src/vespa/storage/visiting/recoveryvisitor.h
@@ -22,7 +22,7 @@ public:
 
 private:
     void handleDocuments(const document::BucketId& bucketId,
-                         std::vector<spi::DocEntry::UP>& entries,
+                         DocEntryList & entries,
                          HitCounter& hitCounter) override;
 
     void completedBucket(const document::BucketId&, HitCounter&) override;
diff --git a/storage/src/vespa/storage/visiting/reindexing_visitor.cpp b/storage/src/vespa/storage/visiting/reindexing_visitor.cpp
index 528f83e29cc..91ed2810420 100644
--- a/storage/src/vespa/storage/visiting/reindexing_visitor.cpp
+++ b/storage/src/vespa/storage/visiting/reindexing_visitor.cpp
@@ -3,6 +3,7 @@
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/documentapi/messagebus/messages/putdocumentmessage.h>
 #include <vespa/storage/common/reindexing_constants.h>
+#include <vespa/persistence/spi/docentry.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".visitor.instance.reindexing_visitor");
@@ -14,7 +15,7 @@ ReindexingVisitor::ReindexingVisitor(StorageComponent& component)
 {
 }
 
-void ReindexingVisitor::handleDocuments(const document::BucketId& /*bucketId*/,
+void ReindexingVisitor::handleDocuments(const document::BucketId& ,
                                         std::vector<spi::DocEntry::UP>& entries,
                                         HitCounter& hitCounter)
 {
diff --git a/storage/src/vespa/storage/visiting/reindexing_visitor.h b/storage/src/vespa/storage/visiting/reindexing_visitor.h
index 9bde9903617..d9e18542818 100644
--- a/storage/src/vespa/storage/visiting/reindexing_visitor.h
+++ b/storage/src/vespa/storage/visiting/reindexing_visitor.h
@@ -20,7 +20,7 @@ public:
     ~ReindexingVisitor() override = default;
 
 private:
-    void handleDocuments(const document::BucketId&, std::vector<spi::DocEntry::UP>&, HitCounter&) override;
+    void handleDocuments(const document::BucketId&, DocEntryList&, HitCounter&) override;
     bool remap_docapi_message_error_code(api::ReturnCode& in_out_code) override;
     vespalib::string make_lock_access_token() const;
 };
diff --git a/storage/src/vespa/storage/visiting/testvisitor.cpp b/storage/src/vespa/storage/visiting/testvisitor.cpp
index b5ace673e48..10c79ea6f18 100644
--- a/storage/src/vespa/storage/visiting/testvisitor.cpp
+++ b/storage/src/vespa/storage/visiting/testvisitor.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "testvisitor.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/documentapi/messagebus/messages/visitor.h>
 #include <sstream>
 
@@ -39,7 +40,7 @@ TestVisitor::startingVisitor(const std::vector<document::BucketId>& buckets)
 
 void
 TestVisitor::handleDocuments(const document::BucketId& /*bucketId*/,
-                             std::vector<spi::DocEntry::UP>& entries,
+                             DocEntryList & entries,
                              HitCounter& /*hitCounter*/)
 {
     std::ostringstream ost;
diff --git a/storage/src/vespa/storage/visiting/testvisitor.h b/storage/src/vespa/storage/visiting/testvisitor.h
index 482b1347faa..989581ac121 100644
--- a/storage/src/vespa/storage/visiting/testvisitor.h
+++ b/storage/src/vespa/storage/visiting/testvisitor.h
@@ -20,7 +20,7 @@ private:
     void startingVisitor(const std::vector<document::BucketId>& buckets) override;
 
     void handleDocuments(const document::BucketId& bucketId,
-                         std::vector<spi::DocEntry::UP>& entries,
+                         DocEntryList & entries,
                          HitCounter& hitCounter) override;
 
     void completedBucket(const document::BucketId& bucket, HitCounter& hitCounter) override;
diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp
index dfb78122e07..7da397b670a 100644
--- a/storage/src/vespa/storage/visiting/visitor.cpp
+++ b/storage/src/vespa/storage/visiting/visitor.cpp
@@ -2,6 +2,7 @@
 
 #include "visitor.h"
 #include "visitormetrics.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/storageframework/generic/clock/timer.h>
 #include <vespa/storageapi/message/datagram.h>
 #include <vespa/storage/persistence/messages.h>
diff --git a/storage/src/vespa/storage/visiting/visitor.h b/storage/src/vespa/storage/visiting/visitor.h
index 52b2d586c78..8857a54e8df 100644
--- a/storage/src/vespa/storage/visiting/visitor.h
+++ b/storage/src/vespa/storage/visiting/visitor.h
@@ -18,7 +18,6 @@
 #include <vespa/storage/common/storagecomponent.h>
 #include <vespa/storage/common/visitorfactory.h>
 #include <vespa/documentapi/messagebus/messages/documentmessage.h>
-#include <vespa/persistence/spi/docentry.h>
 #include <vespa/persistence/spi/selection.h>
 #include <vespa/persistence/spi/read_consistency.h>
 #include <list>
@@ -39,6 +38,10 @@ namespace documentapi {
 
 namespace storage {
 
+namespace spi {
+    class DocEntry;
+}
+
 namespace api {
     class ReturnCode;
     class StorageCommand;
@@ -357,6 +360,7 @@ protected:
     // error code, false if the DocumentAPI message should be retried later.
     [[nodiscard]] virtual bool remap_docapi_message_error_code(api::ReturnCode& in_out_code);
 public:
+    using DocEntryList = std::vector<std::unique_ptr<spi::DocEntry>>;
     Visitor(StorageComponent& component);
     virtual ~Visitor();
 
@@ -398,7 +402,7 @@ public:
      * vector of documents arrive from the persistence layer.
      */
     virtual void handleDocuments(const document::BucketId&,
-                                 std::vector<spi::DocEntry::UP>& entries,
+                                 DocEntryList & entries,
                                  HitCounter& hitCounter) = 0;
 
     /**