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.

2 files changed, 15 insertions, 11 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..b93553c3aa3 100644
--- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
+++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
@@ -49,6 +49,7 @@ using storage::spi::IterateResult;
 using storage::spi::Selection;
 using storage::spi::Timestamp;
 using storage::spi::test::makeSpiBucket;
+using storage::spi::test::equal;
 
 using namespace proton;
 
@@ -390,7 +391,7 @@ void checkEntry(const IterateResult &res, size_t idx, const Timestamp &timestamp
 {
     ASSERT_LESS(idx, res.getEntries().size());
     DocEntry expect(timestamp, flags);
-    EXPECT_EQUAL(expect, *res.getEntries()[idx]);
+    EXPECT_TRUE(equal(expect, *res.getEntries()[idx]));
     EXPECT_EQUAL(getSize(), res.getEntries()[idx]->getSize());
 }
 
@@ -398,7 +399,7 @@ void checkEntry(const IterateResult &res, size_t idx, const DocumentId &id, cons
 {
     ASSERT_LESS(idx, res.getEntries().size());
     DocEntry expect(timestamp, storage::spi::REMOVE_ENTRY, id);
-    EXPECT_EQUAL(expect, *res.getEntries()[idx]);
+    EXPECT_TRUE(equal(expect, *res.getEntries()[idx]));
     EXPECT_EQUAL(getSize(id), res.getEntries()[idx]->getSize());
     EXPECT_GREATER(getSize(id), 0u);
 }
@@ -407,7 +408,7 @@ void checkEntry(const IterateResult &res, size_t idx, const Document &doc, const
 {
     ASSERT_LESS(idx, res.getEntries().size());
     DocEntry expect(timestamp, storage::spi::NONE, Document::UP(doc.clone()));
-    EXPECT_EQUAL(expect, *res.getEntries()[idx]);
+    EXPECT_TRUE(equal(expect, *res.getEntries()[idx]));
     EXPECT_EQUAL(getSize(doc), res.getEntries()[idx]->getSize());
     EXPECT_GREATER(getSize(doc), 0u);
 }
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
index b39b5dc5734..f7b38fa6560 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");
@@ -23,18 +23,20 @@ namespace proton {
 
 namespace {
 
-DocEntry *createDocEntry(Timestamp timestamp, bool removed) {
+std::unique_ptr<DocEntry>
+createDocEntry(Timestamp timestamp, bool removed) {
     int flags = removed ? storage::spi::REMOVE_ENTRY : storage::spi::NONE;
-    return new DocEntry(timestamp, flags);
+    return std::make_unique<DocEntry>(timestamp, flags);
 }
 
-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 std::make_unique<DocEntry>(timestamp, storage::spi::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 std::make_unique<DocEntry>(timestamp, storage::spi::NONE, 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);