- Use modern enum class.

- Add hash method to FieldCollection.

11 files changed, 138 insertions, 118 deletions

diff --git a/document/src/tests/fieldsettest.cpp b/document/src/tests/fieldsettest.cpp
index b82cca86ce7..2642bbe23ce 100644
--- a/document/src/tests/fieldsettest.cpp
+++ b/document/src/tests/fieldsettest.cpp
@@ -2,9 +2,6 @@
 
 #include <vespa/document/base/testdocman.h>
 #include <vespa/document/fieldset/fieldsetrepo.h>
-#include <vespa/vespalib/io/fileutil.h>
-#include <vespa/document/datatype/annotationreferencedatatype.h>
-#include <vespa/document/repo/configbuilder.h>
 #include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/vespalib/objects/nbostream.h>
 #include <algorithm>
@@ -12,8 +9,6 @@
 
 using vespalib::nbostream;
 
-using namespace document::config_builder;
-
 namespace document {
 
 class FieldSetTest : public ::testing::Test {
@@ -290,4 +285,27 @@ TEST_F(FieldSetTest, testStripFields)
               doStripFields(*src, repo, "testdoctype1:hstringval,content"));
 }
 
+TEST(FieldCollectionTest, testHash ) {
+    TestDocMan testDocMan;
+    const DocumentTypeRepo& repo = testDocMan.getTypeRepo();
+    const DocumentType & type = *repo.getDocumentType("testdoctype1");
+    FieldCollection fc(type);
+    EXPECT_EQ(0ul, fc.hash());
+    fc.insert(type.getField("headerval"));
+    EXPECT_EQ(0x548599858c77ef83ul, fc.hash());
+    fc.insert(type.getField("hstringval"));
+    EXPECT_EQ(0x4a7ff2406d36a9b0ul, fc.hash());
+    fc.insert(type.getField("headerval"));
+    EXPECT_EQ(0x4a7ff2406d36a9b0ul, fc.hash());
+
+    FieldCollection fc2(type);
+    EXPECT_EQ(0ul, fc2.hash());
+    fc2.insert(type.getField("hstringval"));
+    EXPECT_EQ(0x1e0918531b19734ul, fc2.hash());
+    fc2.insert(type.getField("headerval"));
+    EXPECT_EQ(fc.hash(), fc2.hash());
+    fc2.insert(type.getField("headerval"));
+    EXPECT_EQ(fc.hash(), fc2.hash());
+}
+
 } // document
diff --git a/document/src/vespa/document/base/field.cpp b/document/src/vespa/document/base/field.cpp
index e85703d203c..316394c289f 100644
--- a/document/src/vespa/document/base/field.cpp
+++ b/document/src/vespa/document/base/field.cpp
@@ -3,6 +3,7 @@
 #include "field.h"
 #include <vespa/document/fieldvalue/fieldvalue.h>
 #include <vespa/document/datatype/datatype.h>
+#include <vespa/document/fieldset/fieldsets.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <vespa/vespalib/util/bobhash.h>
@@ -52,17 +53,16 @@ bool
 Field::contains(const FieldSet& fields) const
 {
     switch (fields.getType()) {
-    case FIELD:
-        return static_cast<const Field&>(fields).getId() == getId();
-    case SET:
-    {
-        // Go through each.
-        return false;
-    }
-    case NONE:
-    case DOCID:
+        case Type::FIELD:
+            return static_cast<const Field&>(fields).getId() == getId();
+        case Type::SET: {
+            const auto & set = static_cast<const FieldCollection &>(fields);
+            return (set.getFields().size() == 1) && ((*set.getFields().begin())->getId() == getId());
+        }
+        case Type::NONE:
+        case Type::DOCID:
         return true;
-    case ALL:
+        case Type::ALL:
         return false;
     }
 
@@ -92,7 +92,7 @@ Field::validateId(int newId) {
                     getName().data(), newId));
     }
 
-    if ((newId & 0x80000000) != 0) // Highest bit must not be set
+    if ((uint32_t(newId) & 0x80000000u) != 0) // Highest bit must not be set
     {
         throw vespalib::IllegalArgumentException(vespalib::make_string(
                     "Attempt to set the id of %s to %d"
diff --git a/document/src/vespa/document/base/field.h b/document/src/vespa/document/base/field.h
index f17a0a41703..a77034932cf 100644
--- a/document/src/vespa/document/base/field.h
+++ b/document/src/vespa/document/base/field.h
@@ -78,7 +78,7 @@ public:
 
     vespalib::string toString(bool verbose=false) const;
     bool contains(const FieldSet& fields) const override;
-    Type getType() const override { return FIELD; }
+    Type getType() const override { return Type::FIELD; }
     bool valid() const { return _fieldId != 0; }
     uint32_t hash() const { return getId(); }
 private:
diff --git a/document/src/vespa/document/fieldset/fieldset.h b/document/src/vespa/document/fieldset/fieldset.h
index 6c9acadcf5d..35c912c5e45 100644
--- a/document/src/vespa/document/fieldset/fieldset.h
+++ b/document/src/vespa/document/fieldset/fieldset.h
@@ -15,7 +15,7 @@ class Document;
 class FieldSet
 {
 public:
-    enum Type {
+    enum class Type {
         FIELD,
         SET,
         ALL,
diff --git a/document/src/vespa/document/fieldset/fieldsetrepo.cpp b/document/src/vespa/document/fieldset/fieldsetrepo.cpp
index e3212f3bd25..614a8740f90 100644
--- a/document/src/vespa/document/fieldset/fieldsetrepo.cpp
+++ b/document/src/vespa/document/fieldset/fieldsetrepo.cpp
@@ -17,13 +17,13 @@ parseSpecialValues(vespalib::stringref name)
 {
     FieldSet::UP fs;
     if ((name.size() == 4) && (name[1] == 'i') && (name[2] == 'd') && (name[3] == ']')) {
-        fs.reset(new DocIdOnly());
+        fs = std::make_unique<DocIdOnly>();
     } else if ((name.size() == 5) && (name[1] == 'a') && (name[2] == 'l') && (name[3] == 'l') && (name[4] == ']')) {
-        fs.reset(new AllFields());
+        fs = std::make_unique<AllFields>();
     } else if ((name.size() == 6) && (name[1] == 'n') && (name[2] == 'o') && (name[3] == 'n') && (name[4] == 'e') && (name[5] == ']')) {
-        fs.reset(new NoFields());
+        fs = std::make_unique<NoFields>();
     } else if ((name.size() == 7) && (name[1] == 'd') && (name[2] == 'o') && (name[3] == 'c') && (name[4] == 'i') && (name[5] == 'd') && (name[6] == ']')) {
-        fs.reset(new DocIdOnly());
+        fs = std::make_unique<DocIdOnly>();
     } else {
         throw vespalib::IllegalArgumentException(
                 "The only special names (enclosed in '[]') allowed are "
@@ -44,20 +44,20 @@ parseFieldCollection(const DocumentTypeRepo& repo,
     const DocumentType& type(*typePtr);
 
     StringTokenizer tokenizer(fieldNames, ",");
-    FieldCollection::UP collection(new FieldCollection(type));
+    auto collection = std::make_unique<FieldCollection>(type);
 
-    for (uint32_t i = 0; i < tokenizer.size(); ++i) {
-        const DocumentType::FieldSet * fs = type.getFieldSet(tokenizer[i]);
+    for (const auto & token : tokenizer) {
+        const DocumentType::FieldSet * fs = type.getFieldSet(token);
         if (fs) {
-            for (DocumentType::FieldSet::Fields::const_iterator it(fs->getFields().begin()), mt(fs->getFields().end()); it != mt; it++) {
-                collection->insert(type.getField(*it));
+            for (const auto & fieldName : fs->getFields()) {
+                collection->insert(type.getField(fieldName));
             }
         } else {
-            collection->insert(type.getField(tokenizer[i]));
+            collection->insert(type.getField(token));
         }
     }
 
-    return FieldSet::UP(collection.release());
+    return collection;
 }
 
 }
@@ -83,34 +83,33 @@ vespalib::string
 FieldSetRepo::serialize(const FieldSet& fieldSet)
 {
     switch (fieldSet.getType()) {
-    case FieldSet::FIELD:
-        return static_cast<const Field&>(fieldSet).getName();
-    case FieldSet::SET:
-    {
-        const FieldCollection& collection = static_cast<const FieldCollection&>(fieldSet);
-
-        vespalib::asciistream stream;
-        stream << collection.getDocumentType().getName() << ":";
-        for (Field::Set::const_iterator iter = collection.getFields().begin();
-             iter != collection.getFields().end();
-             ++iter) {
-            if (iter != collection.getFields().begin()) {
-                stream << ",";
+        case FieldSet::Type::FIELD:
+            return static_cast<const Field&>(fieldSet).getName();
+        case FieldSet::Type::SET: {
+            const auto & collection = static_cast<const FieldCollection&>(fieldSet);
+
+            vespalib::asciistream stream;
+            stream << collection.getDocumentType().getName() << ":";
+            bool first = true;
+            for (const Field * field : collection.getFields()) {
+                if (first) {
+                    first = false;
+                } else {
+                    stream << ",";
+                }
+                stream << field->getName();
             }
 
-            stream << (*iter)->getName();
+            return stream.str();
         }
-
-        return stream.str();
-    }
-    case FieldSet::ALL:
-        return AllFields::NAME;
-    case FieldSet::NONE:
-        return NoFields::NAME;
-    case FieldSet::DOCID:
-        return DocIdOnly::NAME;
-    default:
-        return "";
+        case FieldSet::Type::ALL:
+            return AllFields::NAME;
+        case FieldSet::Type::NONE:
+            return NoFields::NAME;
+        case FieldSet::Type::DOCID:
+            return DocIdOnly::NAME;
+        default:
+            return "";
     }
 }
 
diff --git a/document/src/vespa/document/fieldset/fieldsets.cpp b/document/src/vespa/document/fieldset/fieldsets.cpp
index 1a6cd5e6b84..d878d38be14 100644
--- a/document/src/vespa/document/fieldset/fieldsets.cpp
+++ b/document/src/vespa/document/fieldset/fieldsets.cpp
@@ -3,46 +3,63 @@
 #include "fieldsets.h"
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/document/datatype/documenttype.h>
+#include <vespa/vespalib/stllike/asciistream.h>
+#include <xxhash.h>
 
 namespace document {
 
-FieldCollection::FieldCollection(const DocumentType& type, const Field::Set& s)
-    : _set(s),
+namespace {
+
+uint64_t
+computeHash(const Field::Set & set) {
+    vespalib::asciistream os;
+    for (const Field * field : set) {
+        os << field->getName() << ':';
+    }
+    return XXH64(os.c_str(), os.size(), 0);
+}
+
+}
+
+FieldCollection::FieldCollection(const DocumentType& type)
+    : _set(),
+      _hash(0),
       _docType(type)
 {
 }
 
+FieldCollection::~FieldCollection() = default;
+
 bool
 FieldCollection::contains(const FieldSet& fields) const
 {
     switch (fields.getType()) {
-    case FIELD:
-        return _set.find(static_cast<const Field*>(&fields)) != _set.end();
-    case SET:
-    {
-        const FieldCollection& coll = static_cast<const FieldCollection&>(fields);
+        case Type::FIELD:
+            return _set.find(static_cast<const Field*>(&fields)) != _set.end();
+        case Type::SET: {
+            const auto & coll = static_cast<const FieldCollection&>(fields);
 
-        if (_set.size() < coll._set.size()) {
-            return false;
-        }
+            if (_set.size() < coll._set.size()) {
+                return false;
+            }
 
-        Field::Set::const_iterator iter = coll.getFields().begin();
+            auto iter = coll.getFields().begin();
 
-        while (iter != coll.getFields().end()) {
-            if (_set.find(*iter) == _set.end()) {
-                return false;
+            while (iter != coll.getFields().end()) {
+                if (_set.find(*iter) == _set.end()) {
+                    return false;
+                }
+
+                ++iter;
             }
 
-            ++iter;
+            return true;
         }
-
-        return true;
-    }
-    case NONE:
-    case DOCID:
-        return true;
-    case ALL:
-        return false;
+        case Type::NONE:
+        case Type::DOCID:
+            return true;
+        case Type::ALL:
+            return false;
     }
 
     return false;
@@ -52,18 +69,13 @@ void
 FieldCollection::insert(const Field& f)
 {
     _set.insert(&f);
-}
-
-void
-FieldCollection::insert(const Field::Set& c)
-{
-    _set.insert(c.begin(), c.end());
+    _hash = computeHash(_set);
 }
 
 void
 FieldSet::copyFields(Document& dest, const Document& src, const FieldSet& fields)
 {
-    if (fields.getType() == ALL) {
+    if (fields.getType() == Type::ALL) {
         dest.getFields() = src.getFields();
         return;
     }
@@ -89,10 +101,10 @@ FieldSet::createDocumentSubsetCopy(const Document& src, const FieldSet& fields)
 void
 FieldSet::stripFields(Document& doc, const FieldSet& fieldsToKeep)
 {
-    if (fieldsToKeep.getType() == ALL) {
+    if (fieldsToKeep.getType() == Type::ALL) {
         return;
-    } else if (fieldsToKeep.getType() == DOCID
-               || fieldsToKeep.getType() == NONE)
+    } else if (fieldsToKeep.getType() == Type::DOCID
+               || fieldsToKeep.getType() == Type::NONE)
     {
         doc.clear();
         return;
@@ -106,8 +118,8 @@ FieldSet::stripFields(Document& doc, const FieldSet& fieldsToKeep)
             fieldsToRemove.push_back(&f);
         }
     }
-    for (size_t i = 0; i < fieldsToRemove.size(); ++i) {
-        doc.remove(*fieldsToRemove[i]);
+    for (const Field * field : fieldsToRemove) {
+        doc.remove(*field);
     }
 }
 
diff --git a/document/src/vespa/document/fieldset/fieldsets.h b/document/src/vespa/document/fieldset/fieldsets.h
index 7b8ec6bbff0..eac35244012 100644
--- a/document/src/vespa/document/fieldset/fieldsets.h
+++ b/document/src/vespa/document/fieldset/fieldsets.h
@@ -12,7 +12,7 @@ class AllFields final : public FieldSet
 public:
     static constexpr const char * NAME = "[all]";
     bool contains(const FieldSet&) const override { return true; }
-    Type getType() const override { return ALL; }
+    Type getType() const override { return Type::ALL; }
     FieldSet* clone() const override { return new AllFields(); }
 };
 
@@ -20,8 +20,8 @@ class NoFields final : public FieldSet
 {
 public:
     static constexpr const char * NAME = "[none]";
-    bool contains(const FieldSet& f) const override { return f.getType() == NONE; }
-    Type getType() const override { return NONE; }
+    bool contains(const FieldSet& f) const override { return f.getType() == Type::NONE; }
+    Type getType() const override { return Type::NONE; }
     FieldSet* clone() const override { return new NoFields(); }
 };
 
@@ -30,9 +30,9 @@ class DocIdOnly final : public FieldSet
 public:
     static constexpr const char * NAME = "[id]";
     bool contains(const FieldSet& fields) const override {
-        return fields.getType() == DOCID || fields.getType() == NONE;
+        return fields.getType() == Type::DOCID || fields.getType() == Type::NONE;
     }
-    Type getType() const override { return DOCID; }
+    Type getType() const override { return Type::DOCID; }
     FieldSet* clone() const override { return new DocIdOnly(); }
 };
 
@@ -41,11 +41,11 @@ class FieldCollection : public FieldSet
 public:
     typedef std::unique_ptr<FieldCollection> UP;
 
-    FieldCollection(const DocumentType& docType) : _docType(docType) {};
-    FieldCollection(const DocumentType& docType, const Field::Set& set);
+    FieldCollection(const DocumentType& docType);
+    ~FieldCollection() override;
 
     bool contains(const FieldSet& fields) const override;
-    Type getType() const override { return SET; }
+    Type getType() const override { return Type::SET; }
 
     /**
      * @return Returns the document type the collection is associated with.
@@ -60,19 +60,16 @@ public:
     void insert(const Field& f);
 
     /**
-     * Inserts all the field in the given collection into this collection.
-     */
-    void insert(const Field::Set& f);
-
-    /**
      * Returns all the fields contained in this collection.
      */
     const Field::Set& getFields() const { return _set; }
 
     FieldSet* clone() const override { return new FieldCollection(*this); }
 
+    uint64_t hash() const { return _hash; }
 private:
     Field::Set _set;
+    uint64_t   _hash;
     const DocumentType& _docType;
 };
 
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index 92fea5ff6e4..73783413061 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -294,9 +294,7 @@ BucketContent::eraseEntry(Timestamp t)
     }
 }
 
-DummyPersistence::DummyPersistence(
-        const std::shared_ptr<const document::DocumentTypeRepo>& repo,
-        uint16_t partitionCount)
+DummyPersistence::DummyPersistence(const std::shared_ptr<const document::DocumentTypeRepo>& repo, uint16_t partitionCount)
     : _initialized(false),
       _repo(repo),
       _partitions(partitionCount),
@@ -314,8 +312,7 @@ DummyPersistence::parseDocumentSelection(const string& documentSelection, bool a
 {
     document::select::Node::UP ret;
     try {
-        document::select::Parser parser(
-                *_repo, document::BucketIdFactory());
+        document::select::Parser parser(*_repo, document::BucketIdFactory());
         ret = parser.parse(documentSelection);
     } catch (document::select::ParsingFailedException& e) {
         return document::select::Node::UP();
@@ -539,7 +536,7 @@ DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const
             return GetResult::make_for_tombstone(entry->getTimestamp());
         } else {
             Document::UP doc(entry->getDocument()->clone());
-            if (fieldSet.getType() != document::FieldSet::ALL) {
+            if (fieldSet.getType() != document::FieldSet::Type::ALL) {
                 document::FieldSet::stripFields(*doc, fieldSet);
             }
             return GetResult(std::move(doc), entry->getTimestamp());
@@ -562,10 +559,7 @@ DummyPersistence::createIterator(
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
     std::unique_ptr<document::select::Node> docSelection;
     if (!s.getDocumentSelection().getDocumentSelection().empty()) {
-        docSelection.reset(
-                parseDocumentSelection(
-                        s.getDocumentSelection().getDocumentSelection(),
-                        true).release());
+        docSelection = parseDocumentSelection(s.getDocumentSelection().getDocumentSelection(), true);
         if (!docSelection.get()) {
             return CreateIteratorResult(
                     Result::ErrorType::PERMANENT_ERROR,
@@ -678,7 +672,7 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con
             if (currentSize != 0 && currentSize + size > maxByteSize) break;
             currentSize += size;
             if (!entry->isRemove()
-                && it->_fieldSet->getType() != document::FieldSet::ALL)
+                && it->_fieldSet->getType() != document::FieldSet::Type::ALL)
             {
                 assert(entry->getDocument());
                 // Create new document with only wanted fields.
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
index 8175e612bd4..fcb9a672039 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
@@ -78,7 +78,7 @@ DocumentIterator::DocumentIterator(const storage::spi::Bucket &bucket,
       _fields(fields.clone()),
       _defaultSerializedSize((readConsistency == ReadConsistency::WEAK) ? defaultSerializedSize : -1),
       _readConsistency(readConsistency),
-      _metaOnly(fields.getType() == document::FieldSet::NONE),
+      _metaOnly(fields.getType() == document::FieldSet::Type::NONE),
       _ignoreMaxBytes((readConsistency == ReadConsistency::WEAK) && ignoreMaxBytes),
       _fetchedData(false),
       _sources(),
@@ -140,7 +140,7 @@ public:
                 _selectSession = _cachedSelect->createSession();
                 using document::select::GidFilter;
                 _gidFilter = GidFilter::for_selection_root_node(_selectSession->selectNode());
-                _selectCxt.reset(new SelectContext(*_cachedSelect));
+                _selectCxt = std::make_unique<SelectContext>(*_cachedSelect);
                 _selectCxt->getAttributeGuards();
             }
         } else {
@@ -216,7 +216,7 @@ public:
         }
     }
 
-    virtual bool allowVisitCaching() const override {
+    bool allowVisitCaching() const override {
         return _allowVisitCaching;
     }
 
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
index 2659952ae03..da1b90aa167 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
@@ -436,7 +436,7 @@ PersistenceEngine::get(const Bucket& b, const document::FieldSet& fields, const
                 if (meta.removed) {
                     return GetResult::make_for_tombstone(meta.timestamp);
                 }
-                if (fields.NONE == fields.getType()) {
+                if (document::FieldSet::Type::NONE == fields.getType()) {
                     return GetResult::make_for_metadata_only(meta.timestamp);
                 }
                 document::Document::UP doc = retriever.getDocument(meta.lid);
diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp
index 31858d2a875..c77b8a1b80d 100644
--- a/storage/src/vespa/storage/persistence/persistencethread.cpp
+++ b/storage/src/vespa/storage/persistence/persistencethread.cpp
@@ -292,7 +292,7 @@ PersistenceThread::handleGet(api::GetCommand& cmd, MessageTracker::UP tracker)
         _spi.get(getBucket(cmd.getDocumentId(), cmd.getBucket()), *fieldSet, cmd.getDocumentId(), tracker->context());
 
     if (tracker->checkForError(result)) {
-        if (!result.hasDocument() && (document::FieldSet::NONE != fieldSet->getType())) {
+        if (!result.hasDocument() && (document::FieldSet::Type::NONE != fieldSet->getType())) {
             _env._metrics.get[cmd.getLoadType()].notFound.inc();
         }
         tracker->setReply(std::make_shared<api::GetReply>(cmd, result.getDocumentPtr(), result.getTimestamp(),