Move the transaction implementation from StructuredFieldValue to Document

10 files changed, 153 insertions, 126 deletions

diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp
index 9990c00f57e..956f2d07e85 100644
--- a/document/src/tests/documenttestcase.cpp
+++ b/document/src/tests/documenttestcase.cpp
@@ -36,9 +36,9 @@ TEST(DocumentTest, testSizeOf)
     EXPECT_EQ(32u, sizeof(vespalib::GrowableByteBuffer));
     EXPECT_EQ(88ul, sizeof(IdString));
     EXPECT_EQ(104ul, sizeof(DocumentId));
-    EXPECT_EQ(256ul, sizeof(Document));
-    EXPECT_EQ(120ul, sizeof(StructFieldValue));
-    EXPECT_EQ(24ul, sizeof(StructuredFieldValue));
+    EXPECT_EQ(248ul, sizeof(Document));
+    EXPECT_EQ(112ul, sizeof(StructFieldValue));
+    EXPECT_EQ(16ul, sizeof(StructuredFieldValue));
     EXPECT_EQ(72ul, sizeof(SerializableArray));
 }
 
diff --git a/document/src/vespa/document/base/documentid.cpp b/document/src/vespa/document/base/documentid.cpp
index 58d9a83fc5b..182a96985cd 100644
--- a/document/src/vespa/document/base/documentid.cpp
+++ b/document/src/vespa/document/base/documentid.cpp
@@ -29,7 +29,7 @@ DocumentId::DocumentId(vespalib::nbostream & is)
 
 DocumentId::DocumentId(const DocumentId & rhs) = default;
 DocumentId & DocumentId::operator = (const DocumentId & rhs) = default;
-DocumentId::~DocumentId() = default;
+DocumentId::~DocumentId() noexcept = default;
 
 vespalib::string
 DocumentId::toString() const {
diff --git a/document/src/vespa/document/base/documentid.h b/document/src/vespa/document/base/documentid.h
index 2e4f54b43d3..5dcda838623 100644
--- a/document/src/vespa/document/base/documentid.h
+++ b/document/src/vespa/document/base/documentid.h
@@ -35,11 +35,11 @@ public:
 
     DocumentId();
     DocumentId(vespalib::nbostream & os);
-    DocumentId(DocumentId && rhs) = default;
-    DocumentId & operator = (DocumentId && rhs) = default;
+    DocumentId(DocumentId && rhs) noexcept = default;
+    DocumentId & operator = (DocumentId && rhs) noexcept = default;
     DocumentId(const DocumentId & rhs);
     DocumentId & operator = (const DocumentId & rhs);
-    ~DocumentId();
+    ~DocumentId() noexcept ;
     /**
      * Parse the given document identifier given as string, and create an
      * identifier object from it.
diff --git a/document/src/vespa/document/fieldvalue/document.cpp b/document/src/vespa/document/fieldvalue/document.cpp
index 744409ac206..cc60234a093 100644
--- a/document/src/vespa/document/fieldvalue/document.cpp
+++ b/document/src/vespa/document/fieldvalue/document.cpp
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "document.h"
+#include "structuredcache.h"
 #include <vespa/document/datatype/documenttype.h>
 #include <vespa/vespalib/util/crc.h>
 #include <vespa/document/repo/documenttyperepo.h>
@@ -11,6 +12,8 @@
 #include <vespa/document/fieldset/fieldsets.h>
 #include <vespa/document/util/bytebuffer.h>
 #include <vespa/vespalib/util/xmlstream.h>
+#include <vespa/vespalib/stllike/hash_map.hpp>
+#include <cassert>
 #include <sstream>
 
 using vespalib::nbostream;
@@ -73,7 +76,12 @@ Document::Document()
     _fields.setDocumentType(getType());
 }
 
-Document::Document(const Document& other) = default;
+Document::Document(const Document& rhs)
+    : StructuredFieldValue(rhs),
+      _id(rhs._id),
+      _fields(rhs._fields),
+      _lastModified(rhs._lastModified)
+{}
 
 Document::Document(const DataType &type, DocumentId documentId)
     : StructuredFieldValue(verifyDocumentType(&type)),
@@ -101,15 +109,35 @@ Document::Document(const DocumentTypeRepo& repo, vespalib::nbostream & is)
     deserialize(repo, is);
 }
 
-Document::~Document() = default;
+Document::Document(Document &&) noexcept = default;
+Document::~Document() noexcept = default;
+
+Document &
+Document::operator =(Document &&rhs) noexcept {
+    assert( ! _cache && ! rhs._cache);
+    _id = std::move(rhs._id);
+    _fields = std::move(rhs._fields);
+    _lastModified = rhs._lastModified;
+    StructuredFieldValue::operator=(std::move(rhs));
+    return *this;
+}
+
+Document &
+Document::operator =(const Document &rhs) {
+    assert( ! _cache && ! rhs._cache);
+    _id = rhs._id;
+    _fields = rhs._fields;
+    _lastModified = rhs._lastModified;
+    StructuredFieldValue::operator=(rhs);
+    return *this;
+}
+
 
 const DocumentType&
 Document::getType() const {
     return static_cast<const DocumentType &>(StructuredFieldValue::getType());
 }
 
-Document& Document::operator=(const Document& doc) = default;
-
 void
 Document::clear()
 {
@@ -134,7 +162,8 @@ Document::assign(const FieldValue& value)
     /// \todo TODO (was warning):  This type checking doesnt work with the way assign is used.
 //    if (*value.getDataType() == *_type) {
     auto & other(dynamic_cast<const Document&>(value));
-    return operator=(other);
+    *this = Document(other);
+    return *this;
 //    }
 //    return FieldValue::assign(value); // Generates exception
 }
@@ -243,4 +272,20 @@ Document::getIterator(const Field* first) const
     return _fields.getIterator(first);
 }
 
+void
+Document::beginTransaction() {
+    _cache = std::make_unique<StructuredCache>();
+}
+void
+Document::commitTransaction() {
+    for (auto & e : *_cache) {
+        if (e.second.status == fieldvalue::ModificationStatus::REMOVED) {
+            removeFieldValue(e.first);
+        } else if (e.second.status == fieldvalue::ModificationStatus::MODIFIED) {
+            setFieldValue(e.first, std::move(e.second.value));
+        }
+    }
+    _cache.reset();
+}
+
 } // document
diff --git a/document/src/vespa/document/fieldvalue/document.h b/document/src/vespa/document/fieldvalue/document.h
index f20ef969e0c..b487aa067da 100644
--- a/document/src/vespa/document/fieldvalue/document.h
+++ b/document/src/vespa/document/fieldvalue/document.h
@@ -21,11 +21,14 @@
 
 namespace document {
 
+class TransactionGuard;
+
 class Document : public StructuredFieldValue
 {
 private:
     DocumentId _id;
     StructFieldValue _fields;
+    std::unique_ptr<StructuredCache> _cache;
 
     // To avoid having to return another container object out of docblocks
     // the meta data has been added to document. This will not be serialized
@@ -42,15 +45,16 @@ public:
 
     Document();
     Document(const Document&);
+    Document(Document &&) noexcept;
+    Document & operator =(const Document &);
+    Document & operator =(Document &&) noexcept;
     Document(const DataType &, DocumentId id);
     Document(const DocumentTypeRepo& repo, vespalib::nbostream& stream);
-    ~Document() override;
+    ~Document() noexcept override;
 
     void setRepo(const DocumentTypeRepo & repo);
     const DocumentTypeRepo * getRepo() const { return _fields.getRepo(); }
 
-    Document& operator=(const Document&);
-
     void accept(FieldValueVisitor &visitor) override { visitor.visit(*this); }
     void accept(ConstFieldValueVisitor &visitor) const override { visitor.visit(*this); }
 
@@ -107,6 +111,9 @@ public:
 
     void setFieldValue(const Field& field, FieldValue::UP data) override;
 private:
+    friend TransactionGuard;
+    void beginTransaction();
+    void commitTransaction();
     void deserializeHeader(const DocumentTypeRepo& repo, vespalib::nbostream & header);
     void deserializeBody(const DocumentTypeRepo& repo, vespalib::nbostream & body);
     bool hasFieldValue(const Field& field) const override { return _fields.hasValue(field); }
@@ -115,6 +122,19 @@ private:
     bool getFieldValue(const Field& field, FieldValue& value) const override { return _fields.getValue(field, value); }
 
     StructuredIterator::UP getIterator(const Field* first) const override;
+    StructuredCache * getCache() const override { return _cache.get(); }
+};
+
+class TransactionGuard {
+public:
+    TransactionGuard(Document & value)
+        : _value(value)
+    {
+        _value.beginTransaction();
+    }
+    ~TransactionGuard() { _value.commitTransaction(); }
+private:
+    Document & _value;
 };
 
 }  // document
diff --git a/document/src/vespa/document/fieldvalue/fieldvalue.h b/document/src/vespa/document/fieldvalue/fieldvalue.h
index 9ed8d522b17..66d5eaa8c58 100644
--- a/document/src/vespa/document/fieldvalue/fieldvalue.h
+++ b/document/src/vespa/document/fieldvalue/fieldvalue.h
@@ -33,6 +33,8 @@ class FieldValue : public vespalib::Identifiable
 protected:
     FieldValue(const FieldValue&) = default;
     FieldValue& operator=(const FieldValue&) = default;
+    FieldValue(FieldValue &&) = default;
+    FieldValue& operator=(FieldValue &&) = default;
     static std::unique_ptr<vespalib::IArrayBase> createArray(const DataType & baseType);
 
 public:
diff --git a/document/src/vespa/document/fieldvalue/structfieldvalue.h b/document/src/vespa/document/fieldvalue/structfieldvalue.h
index e2784189e04..fc930951df3 100644
--- a/document/src/vespa/document/fieldvalue/structfieldvalue.h
+++ b/document/src/vespa/document/fieldvalue/structfieldvalue.h
@@ -41,7 +41,7 @@ public:
     StructFieldValue & operator = (const StructFieldValue & rhs);
     StructFieldValue(StructFieldValue && rhs) noexcept = default;
     StructFieldValue & operator = (StructFieldValue && rhs) noexcept = default;
-    ~StructFieldValue() override;
+    ~StructFieldValue() noexcept override;
 
     void setRepo(const DocumentTypeRepo & repo) { _repo = & repo; }
     const DocumentTypeRepo * getRepo() const { return _repo; }
diff --git a/document/src/vespa/document/fieldvalue/structuredcache.h b/document/src/vespa/document/fieldvalue/structuredcache.h
new file mode 100644
index 00000000000..24d1a3dbd78
--- /dev/null
+++ b/document/src/vespa/document/fieldvalue/structuredcache.h
@@ -0,0 +1,54 @@
+#pragma once
+
+#include "fieldvalue.h"
+#include <vespa/vespalib/stllike/hash_map.h>
+
+namespace document {
+
+class StructuredCache {
+public:
+    using ModificationStatus = fieldvalue::ModificationStatus;
+    struct ValuePair {
+        fieldvalue::ModificationStatus status;
+        FieldValue::UP value;
+
+        ValuePair() : status(ModificationStatus::NOT_MODIFIED), value() {}
+
+        ValuePair(ModificationStatus status_, FieldValue::UP value_)
+            : status(status_),
+              value(std::move(value_))
+        {}
+    };
+
+    using Cache = vespalib::hash_map<Field, ValuePair>;
+
+    void remove(const Field &field) {
+        ValuePair &entry = _cache[field];
+        entry.status = ModificationStatus::REMOVED;
+        entry.value.reset();
+    }
+
+    Cache::iterator find(const Field &field) {
+        return _cache.find(field);
+    }
+
+    void set(const Field &field, FieldValue::UP value, ModificationStatus status) {
+        ValuePair &entry = _cache[field];
+        // If the entry has previously been tagged modified, the value we're now reinserting
+        // is likely based on those changes. We cannot lose that modification status.
+        entry.status = ((status == ModificationStatus::NOT_MODIFIED) &&
+                        (entry.status == ModificationStatus::MODIFIED))
+                       ? ModificationStatus::MODIFIED
+                       : status;
+        entry.value = std::move(value);
+    }
+
+    Cache::iterator begin() { return _cache.begin(); }
+
+    Cache::iterator end() { return _cache.end(); }
+
+private:
+    Cache _cache;
+};
+
+}
diff --git a/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp b/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp
index 39af319bc8a..755186edd4c 100644
--- a/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp
+++ b/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp
@@ -4,6 +4,7 @@
 #include "iteratorhandler.h"
 #include "weightedsetfieldvalue.h"
 #include "arrayfieldvalue.h"
+#include "structuredcache.h"
 #include <vespa/vespalib/stllike/hash_map.hpp>
 
 #include <vespa/log/log.h>
@@ -29,34 +30,12 @@ StructuredFieldValue::Iterator::Iterator(const StructuredFieldValue& owner, cons
 {
 }
 
-StructuredFieldValue::StructuredFieldValue(const StructuredFieldValue& other)
-    : FieldValue(other),
-      _type(other._type)
-{
-}
-
 StructuredFieldValue::StructuredFieldValue(const DataType &type)
     : FieldValue(),
       _type(&type)
 {
 }
 
-StructuredFieldValue::~StructuredFieldValue() = default;
-
-
-StructuredFieldValue&
-StructuredFieldValue::operator=(const StructuredFieldValue& other)
-{
-    if (this == &other) {
-        return *this;
-    }
-
-    FieldValue::operator=(other);
-    _type = other._type;
-
-    return *this;
-}
-
 void
 StructuredFieldValue::setFieldValue(const Field & field, const FieldValue & value)
 {
@@ -85,53 +64,11 @@ StructuredFieldValue::onGetNestedFieldValue(PathRange nested) const
     return fv;
 }
 
-namespace {
-    struct ValuePair {
-        fieldvalue::ModificationStatus status;
-        FieldValue::UP value;
-
-        ValuePair() : status(fieldvalue::ModificationStatus::NOT_MODIFIED), value() {}
-        ValuePair(fieldvalue::ModificationStatus status_,
-                  FieldValue::UP value_)
-            : status(status_),
-              value(std::move(value_))
-        {}
-    };
-
-    using Cache = vespalib::hash_map<Field, ValuePair>;
-}
-
-class StructuredCache {
-public:
-    void remove(const Field & field) {
-        ValuePair & entry = _cache[field];
-        entry.status = ModificationStatus::REMOVED;
-        entry.value.reset();
-    }
-    Cache::iterator find(const Field & field) {
-        return _cache.find(field);
-    }
-    void set(const Field & field, FieldValue::UP value, ModificationStatus status) {
-        ValuePair & entry = _cache[field];
-        // If the entry has previously been tagged modified, the value we're now reinserting
-        // is likely based on those changes. We cannot lose that modification status.
-        entry.status = ((status == ModificationStatus::NOT_MODIFIED) &&
-                        (entry.status == ModificationStatus::MODIFIED))
-                ? ModificationStatus::MODIFIED
-                : status;
-        entry.value = std::move(value);
-    }
-    Cache::iterator begin() { return _cache.begin(); }
-    Cache::iterator end() { return _cache.end(); }
-private:
-    Cache _cache;
-};
-
-
 void
 StructuredFieldValue::remove(const Field& field) {
-    if (_cache) {
-        _cache->remove(field);
+    StructuredCache * cache = getCache();
+    if (cache) {
+        cache->remove(field);
     } else {
         removeFieldValue(field);
     }
@@ -139,8 +76,9 @@ StructuredFieldValue::remove(const Field& field) {
 
 void
 StructuredFieldValue::updateValue(const Field & field, FieldValue::UP value) const {
-    if (_cache) {
-        _cache->set(field, std::move(value), ModificationStatus::MODIFIED);
+    StructuredCache * cache = getCache();
+    if (cache) {
+        cache->set(field, std::move(value), ModificationStatus::MODIFIED);
     } else {
         const_cast<StructuredFieldValue&>(*this).setFieldValue(field, std::move(value));
     }
@@ -148,18 +86,20 @@ StructuredFieldValue::updateValue(const Field & field, FieldValue::UP value) con
 
 void
 StructuredFieldValue::returnValue(const Field & field, FieldValue::UP value) const {
-    if (_cache) {
-        _cache->set(field, std::move(value), ModificationStatus::NOT_MODIFIED);
+    StructuredCache * cache = getCache();
+    if (cache) {
+        cache->set(field, std::move(value), ModificationStatus::NOT_MODIFIED);
     }
 }
 
 FieldValue::UP
 StructuredFieldValue::getValue(const Field& field, FieldValue::UP container) const {
-    if (_cache) {
-        auto found = _cache->find(field);
-        if (found == _cache->end()) {
+    StructuredCache * cache = getCache();
+    if (cache) {
+        auto found = cache->find(field);
+        if (found == cache->end()) {
             container = getFieldValue(field);
-            _cache->set(field, FieldValue::UP(), ModificationStatus::NOT_MODIFIED);
+            cache->set(field, FieldValue::UP(), ModificationStatus::NOT_MODIFIED);
         } else {
             container = std::move(found->second.value);
         }
@@ -238,22 +178,6 @@ StructuredFieldValue::onIterateNested(PathRange nested, IteratorHandler & handle
     }
 }
 
-void
-StructuredFieldValue::beginTransaction() {
-    _cache = std::make_unique<StructuredCache>();
-}
-void
-StructuredFieldValue::commitTransaction() {
-    for (auto & e : *_cache) {
-        if (e.second.status == ModificationStatus::REMOVED) {
-            removeFieldValue(e.first);
-        } else if (e.second.status == ModificationStatus::MODIFIED) {
-            setFieldValue(e.first, std::move(e.second.value));
-        }
-    }
-    _cache.reset();
-}
-
 using ConstCharP = const char *;
 template void StructuredFieldValue::set(vespalib::stringref field, int32_t value);
 template void StructuredFieldValue::set(vespalib::stringref field, int64_t value);
diff --git a/document/src/vespa/document/fieldvalue/structuredfieldvalue.h b/document/src/vespa/document/fieldvalue/structuredfieldvalue.h
index a96e9a95af1..55b964147be 100644
--- a/document/src/vespa/document/fieldvalue/structuredfieldvalue.h
+++ b/document/src/vespa/document/fieldvalue/structuredfieldvalue.h
@@ -33,7 +33,6 @@ class StructuredCache;
 class StructuredFieldValue : public FieldValue
 {
     const DataType *_type;
-    std::unique_ptr<StructuredCache> _cache;
 
     UP onGetNestedFieldValue(PathRange nested) const override;
     /** @return Retrieve value of given field. Null pointer if not set.
@@ -42,12 +41,10 @@ class StructuredFieldValue : public FieldValue
     VESPA_DLL_LOCAL FieldValue::UP getValue(const Field& field, FieldValue::UP container) const;
     VESPA_DLL_LOCAL void updateValue(const Field & field, FieldValue::UP value) const;
     VESPA_DLL_LOCAL void returnValue(const Field & field, FieldValue::UP value) const;
+    virtual StructuredCache * getCache() const { return nullptr; }
 
 protected:
     VESPA_DLL_LOCAL StructuredFieldValue(const DataType &type);
-    StructuredFieldValue(const StructuredFieldValue&);
-    StructuredFieldValue& operator=(const StructuredFieldValue&);
-    ~StructuredFieldValue() override;
 
     /** Called from Document when deserializing alters type. */
     virtual void setType(const DataType& type) { _type = &type; }
@@ -117,9 +114,6 @@ public:
      */
     virtual const Field& getField(vespalib::stringref name) const = 0;
 
-    void beginTransaction();
-    void commitTransaction();
-
     /**
      * Retrieve value of given field and assign it to given field.
      *
@@ -197,17 +191,5 @@ public:
     std::unique_ptr<T> getAs(const Field &field) const;
 };
 
-class TransactionGuard {
-public:
-    TransactionGuard(StructuredFieldValue & value)
-        : _value(value)
-    {
-        _value.beginTransaction();
-    }
-    ~TransactionGuard() { _value.commitTransaction(); }
-private:
-    StructuredFieldValue & _value;
-};
-
 } // document