Revert "Revert "Balder/refactor docentry""

2 files changed, 31 insertions, 30 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 fab46e61494..82eac88a53e 100644
--- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
+++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
@@ -48,7 +48,9 @@ using storage::spi::IncludedVersions;
 using storage::spi::IterateResult;
 using storage::spi::Selection;
 using storage::spi::Timestamp;
+using storage::spi::DocumentMetaEnum;
 using storage::spi::test::makeSpiBucket;
+using storage::spi::test::equal;
 
 using namespace proton;
 
@@ -274,18 +276,14 @@ struct PairDR : DocumentRetrieverBaseForTest {
     }
 };
 
-size_t getSize() {
-    return sizeof(DocEntry);
-}
-
 size_t getSize(const document::Document &doc) {
     vespalib::nbostream tmp;
     doc.serialize(tmp);
-    return tmp.size() + getSize();
+    return tmp.size();
 }
 
 size_t getSize(const document::DocumentId &id) {
-    return id.getSerializedSize() + getSize();
+    return id.getSerializedSize();
 }
 
 IDocumentRetriever::SP nil() { return std::make_unique<UnitDR>(); }
@@ -386,19 +384,19 @@ void checkDoc(const IDocumentRetriever &dr, const std::string &id,
     EXPECT_TRUE(DocumentId(id) == doc->getId());
 }
 
-void checkEntry(const IterateResult &res, size_t idx, const Timestamp &timestamp, int flags)
+void checkEntry(const IterateResult &res, size_t idx, const Timestamp &timestamp, DocumentMetaEnum flags)
 {
     ASSERT_LESS(idx, res.getEntries().size());
-    DocEntry expect(timestamp, flags);
-    EXPECT_EQUAL(expect, *res.getEntries()[idx]);
-    EXPECT_EQUAL(getSize(), res.getEntries()[idx]->getSize());
+    auto expect = DocEntry::create(timestamp, flags);
+    EXPECT_TRUE(equal(*expect, *res.getEntries()[idx]));
+    EXPECT_EQUAL(sizeof(DocEntry), res.getEntries()[idx]->getSize());
 }
 
 void checkEntry(const IterateResult &res, size_t idx, const DocumentId &id, const Timestamp &timestamp)
 {
     ASSERT_LESS(idx, res.getEntries().size());
-    DocEntry expect(timestamp, storage::spi::REMOVE_ENTRY, id);
-    EXPECT_EQUAL(expect, *res.getEntries()[idx]);
+    auto expect = DocEntry::create(timestamp, DocumentMetaEnum::REMOVE_ENTRY, id);
+    EXPECT_TRUE(equal(*expect, *res.getEntries()[idx]));
     EXPECT_EQUAL(getSize(id), res.getEntries()[idx]->getSize());
     EXPECT_GREATER(getSize(id), 0u);
 }
@@ -406,8 +404,8 @@ void checkEntry(const IterateResult &res, size_t idx, const DocumentId &id, cons
 void checkEntry(const IterateResult &res, size_t idx, const Document &doc, const Timestamp &timestamp)
 {
     ASSERT_LESS(idx, res.getEntries().size());
-    DocEntry expect(timestamp, storage::spi::NONE, Document::UP(doc.clone()));
-    EXPECT_EQUAL(expect, *res.getEntries()[idx]);
+    auto expect = DocEntry::create(timestamp, Document::UP(doc.clone()));
+    EXPECT_TRUE(equal(*expect, *res.getEntries()[idx]));
     EXPECT_EQUAL(getSize(doc), res.getEntries()[idx]->getSize());
     EXPECT_GREATER(getSize(doc), 0u);
 }
@@ -608,9 +606,9 @@ TEST("require that using an empty field set returns meta-data only") {
     IterateResult res = itr.iterate(largeNum);
     EXPECT_TRUE(res.isCompleted());
     EXPECT_EQUAL(3u, res.getEntries().size());
-    TEST_DO(checkEntry(res, 0, Timestamp(2), storage::spi::NONE));
-    TEST_DO(checkEntry(res, 1, Timestamp(3), storage::spi::NONE));
-    TEST_DO(checkEntry(res, 2, Timestamp(4), storage::spi::REMOVE_ENTRY));
+    TEST_DO(checkEntry(res, 0, Timestamp(2), DocumentMetaEnum::NONE));
+    TEST_DO(checkEntry(res, 1, Timestamp(3), DocumentMetaEnum::NONE));
+    TEST_DO(checkEntry(res, 2, Timestamp(4), DocumentMetaEnum::REMOVE_ENTRY));
 }
 
 TEST("require that entries in other buckets are skipped") {
@@ -650,15 +648,15 @@ TEST("require that maxBytes splits iteration results for meta-data only iteratio
     itr.add(doc("id:ns:document::1", Timestamp(2), bucket(5)));
     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(getSize() + getSize());
+    IterateResult res1 = itr.iterate(2 * sizeof(DocEntry));
     EXPECT_TRUE(!res1.isCompleted());
     EXPECT_EQUAL(2u, res1.getEntries().size());
-    TEST_DO(checkEntry(res1, 0, Timestamp(2), storage::spi::NONE));
-    TEST_DO(checkEntry(res1, 1, Timestamp(3), storage::spi::REMOVE_ENTRY));
+    TEST_DO(checkEntry(res1, 0, Timestamp(2), DocumentMetaEnum::NONE));
+    TEST_DO(checkEntry(res1, 1, Timestamp(3), DocumentMetaEnum::REMOVE_ENTRY));
 
     IterateResult res2 = itr.iterate(largeNum);
     EXPECT_TRUE(res2.isCompleted());
-    TEST_DO(checkEntry(res2, 0, Timestamp(4), storage::spi::NONE));
+    TEST_DO(checkEntry(res2, 0, Timestamp(4), DocumentMetaEnum::NONE));
 
     IterateResult res3 = itr.iterate(largeNum);
     EXPECT_TRUE(res3.isCompleted());
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
index b39b5dc5734..278b0c68dab 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "document_iterator.h"
+#include <vespa/persistence/spi/docentry.h>
 #include <vespa/searchcore/proton/common/cachedselect.h>
 #include <vespa/searchcore/proton/common/selectcontext.h>
 #include <vespa/document/select/gid_filter.h>
@@ -8,7 +9,6 @@
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/vespalib/objects/nbostream.h>
 #include <vespa/vespalib/stllike/hash_map.h>
-#include <algorithm>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".proton.persistenceengine.document_iterator");
@@ -18,23 +18,25 @@ using storage::spi::DocEntry;
 using storage::spi::Timestamp;
 using document::Document;
 using document::DocumentId;
+using storage::spi::DocumentMetaEnum;
 
 namespace proton {
 
 namespace {
 
-DocEntry *createDocEntry(Timestamp timestamp, bool removed) {
-    int flags = removed ? storage::spi::REMOVE_ENTRY : storage::spi::NONE;
-    return new DocEntry(timestamp, flags);
+std::unique_ptr<DocEntry>
+createDocEntry(Timestamp timestamp, bool removed) {
+    return DocEntry::create(timestamp, removed ? DocumentMetaEnum::REMOVE_ENTRY : DocumentMetaEnum::NONE);
 }
 
-DocEntry *createDocEntry(Timestamp timestamp, bool removed, Document::UP doc, ssize_t defaultSerializedSize) {
+std::unique_ptr<DocEntry>
+createDocEntry(Timestamp timestamp, bool removed, Document::UP doc, ssize_t defaultSerializedSize) {
     if (doc) {
         if (removed) {
-            return new DocEntry(timestamp, storage::spi::REMOVE_ENTRY, doc->getId());
+            return DocEntry::create(timestamp, DocumentMetaEnum::REMOVE_ENTRY, doc->getId());
         } else {
             ssize_t serializedSize = defaultSerializedSize >= 0 ? defaultSerializedSize : doc->serialize().size();
-            return new DocEntry(timestamp, storage::spi::NONE, std::move(doc), serializedSize);
+            return DocEntry::create(timestamp, std::move(doc), serializedSize);
         }
     } else {
         return createDocEntry(timestamp, removed);
@@ -212,7 +214,7 @@ public:
             if (doc && _fields) {
                 document::FieldSet::stripFields(*doc, *_fields);
             }
-            _list.emplace_back(createDocEntry(meta.timestamp, meta.removed, std::move(doc), _defaultSerializedSize));
+            _list.push_back(createDocEntry(meta.timestamp, meta.removed, std::move(doc), _defaultSerializedSize));
         }
     }
 
@@ -262,11 +264,12 @@ DocumentIterator::fetchCompleteSource(const IDocumentRetriever & source, Iterate
     }
     LOG(debug, "metadata count after filtering: %zu", lidsToFetch.size());
 
+    list.reserve(lidsToFetch.size());
     if ( _metaOnly ) {
         for (uint32_t lid : lidsToFetch) {
             const search::DocumentMetaData & meta = metaData[lidIndexMap[lid]];
             assert(lid == meta.lid);
-            list.emplace_back(createDocEntry(meta.timestamp, meta.removed));
+            list.push_back(createDocEntry(meta.timestamp, meta.removed));
         }
     } else {
         MatchVisitor visitor(matcher, metaData, lidIndexMap, _fields.get(), list, _defaultSerializedSize);