Clean up a very dangerous interface to prevent potential misuse.

10 files changed, 65 insertions, 199 deletions

diff --git a/vsm/src/tests/docsum/docsum.cpp b/vsm/src/tests/docsum/docsum.cpp
index c084bd5593f..4ebd7061b69 100644
--- a/vsm/src/tests/docsum/docsum.cpp
+++ b/vsm/src/tests/docsum/docsum.cpp
@@ -245,35 +245,6 @@ DocsumTest::requireThatJSONDocsumWriterHandlesMap()
     }
 }
 
-void
-DocsumTest::testDocSumCache()
-{
-    Document::SP d1(new TestDocument(0, 1));
-    d1->setField(0, FieldValue::UP(new StringFieldValue("aa")));
-    Document::SP d2(new TestDocument(1, 2));
-    d2->setField(0, FieldValue::UP(new StringFieldValue("bbb")));
-    d2->setField(1, FieldValue::UP(new StringFieldValue("cccc")));
-    DocSumCache cac1;
-    cac1.push_back(d1);
-    cac1.push_back(d2);
-    EXPECT_EQUAL(cac1.cache().size(), 2u);
-
-    Document::SP d3(new TestDocument(2, 1));
-    d3->setField(0, FieldValue::UP(new StringFieldValue("ddddd")));
-    DocSumCache cac2;
-    cac2.push_back(d3);
-    cac1.insert(cac2);
-    EXPECT_EQUAL(cac1.cache().size(), 3u);
-
-    Document::SP d4(new TestDocument(2, 1));
-    d4->setField(0, FieldValue::UP(new StringFieldValue("eeeeee")));
-    DocSumCache cac3;
-    cac3.push_back(d4);
-    cac1.insert(cac3);
-    EXPECT_EQUAL(cac1.cache().size(), 3u);
-    EXPECT_EQUAL(2u, cac1.getDocSum(2).getDocId());
-}
-
 int
 DocsumTest::Main()
 {
@@ -282,7 +253,6 @@ DocsumTest::Main()
     testFlattenDocsumWriter();
     testJSONDocsumWriter();
     requireThatJSONDocsumWriterHandlesMap();
-    testDocSumCache();
 
     TEST_DONE();
 }
diff --git a/vsm/src/tests/document/document.cpp b/vsm/src/tests/document/document.cpp
index a824d59a788..f13d4020054 100644
--- a/vsm/src/tests/document/document.cpp
+++ b/vsm/src/tests/document/document.cpp
@@ -41,11 +41,8 @@ DocumentTest::testStorageDocument()
     ASSERT_TRUE((*fpmap)[1].size() == 1);
     ASSERT_TRUE((*fpmap)[2].size() == 0);
 
-    StorageDocument sdoc(std::move(doc));
+    StorageDocument sdoc(std::move(doc), fpmap, 3);
     ASSERT_TRUE(sdoc.valid());
-    sdoc.setFieldCount(3);
-    sdoc.fieldPathMap(fpmap);
-    sdoc.init();
 
     EXPECT_EQUAL(std::string("foo"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("bar"), sdoc.getField(1)->getAsString());
@@ -69,8 +66,6 @@ DocumentTest::testStorageDocument()
     EXPECT_EQUAL(std::string("qux"), sdoc.getField(1)->getAsString());
     EXPECT_EQUAL(std::string("quux"), sdoc.getField(2)->getAsString());
 
-    // reset cached field values
-    sdoc.init();
     EXPECT_EQUAL(std::string("foo"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("bar"), sdoc.getField(1)->getAsString());
     EXPECT_TRUE(sdoc.getField(2) == NULL);
@@ -78,7 +73,7 @@ DocumentTest::testStorageDocument()
     EXPECT_TRUE(!sdoc.setField(3, FieldValue::UP(new StringFieldValue("thud"))));
 
     SharedFieldPathMap fim;
-    StorageDocument s2(fim);
+    StorageDocument s2(std::make_unique<document::Document>(), fim, 0);
     EXPECT_EQUAL(vespalib::string("null::"), s2.docDoc().getId().toString());
 }
 
diff --git a/vsm/src/tests/searcher/searcher.cpp b/vsm/src/tests/searcher/searcher.cpp
index 28e97f5e726..5bb47b7d2ae 100644
--- a/vsm/src/tests/searcher/searcher.cpp
+++ b/vsm/src/tests/searcher/searcher.cpp
@@ -300,9 +300,7 @@ performSearch(FieldSearcher & fs, const StringList & query, const FieldValue & f
     // setup document
     SharedFieldPathMap sfim(new FieldPathMapT());
     sfim->push_back(FieldPath());
-    StorageDocument doc(sfim);
-    doc.setFieldCount(1);
-    doc.init();
+    StorageDocument doc(std::make_unique<document::Document>(), sfim, 1);
     doc.setField(0, document::FieldValue::UP(fv.clone()));
 
     fs.search(doc);
diff --git a/vsm/src/vespa/vsm/common/CMakeLists.txt b/vsm/src/vespa/vsm/common/CMakeLists.txt
index 7f3618ac12c..9be6703eba9 100644
--- a/vsm/src/vespa/vsm/common/CMakeLists.txt
+++ b/vsm/src/vespa/vsm/common/CMakeLists.txt
@@ -2,7 +2,6 @@
 vespa_add_library(vsm_vsmcommon OBJECT
     SOURCES
     charbuffer.cpp
-    docsum.cpp
     document.cpp
     documenttypemapping.cpp
     fieldmodifier.cpp
diff --git a/vsm/src/vespa/vsm/common/docsum.cpp b/vsm/src/vespa/vsm/common/docsum.cpp
deleted file mode 100644
index 3d32f30aa87..00000000000
--- a/vsm/src/vespa/vsm/common/docsum.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include "docsum.h"
-#include <vespa/document/fieldvalue/fieldvalue.h>
-#include <vespa/document/fieldvalue/intfieldvalue.h>
-#include <vespa/document/fieldvalue/longfieldvalue.h>
-#include <vespa/document/fieldvalue/bytefieldvalue.h>
-#include <vespa/vespalib/stllike/hash_map.hpp>
-
-#define DEBUGMASK 0x00
-
-using search::DocumentIdT;
-
-namespace vsm {
-
-using document::FieldValue;
-using document::StringFieldValue;
-
-IMPLEMENT_DUPLICATE(DocSumCache);
-
-DocSumCache::DocSumCache() :
-  _list()
-{ }
-
-DocSumCache::~DocSumCache() { }
-
-const Document & DocSumCache::getDocSum(const DocumentIdT & docId) const
-{
-    DocSumCacheT::const_iterator found = _list.find(docId);
-    return *found->second;
-}
-
-void DocSumCache::push_back(const Document::SP & docSum)
-{
-  _list[docSum->getDocId()] = docSum;
-}
-
-void DocSumCache::insert(const DocSumCache & dc)
-{
-  for (DocSumCacheT::const_iterator itr = dc._list.begin(); itr != dc._list.end(); ++itr) {
-    if (_list.find(itr->first) == _list.end()) {
-      _list[itr->first] = itr->second;
-    }
-  }
-}
-
-}
diff --git a/vsm/src/vespa/vsm/common/docsum.h b/vsm/src/vespa/vsm/common/docsum.h
index bde2625f7a8..9bfaf7eac05 100644
--- a/vsm/src/vespa/vsm/common/docsum.h
+++ b/vsm/src/vespa/vsm/common/docsum.h
@@ -1,11 +1,9 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include <vespa/vsm/common/document.h>
-#include <vespa/document/fieldvalue/stringfieldvalue.h>
+#include "document.h"
 
-namespace vsm
-{
+namespace vsm {
 
 /**
   Will represent a cache of the document summaries. -> Actual docsums will be
@@ -20,20 +18,5 @@ public:
   virtual ~IDocSumCache() { }
 };
 
-class DocSumCache : public search::Object, public IDocSumCache
-{
- public:
-  typedef vespalib::hash_map<search::DocumentIdT, Document::SP> DocSumCacheT;
-  DUPLICATE(DocSumCache);
-  DocSumCache();
-  virtual ~DocSumCache();
-  virtual const Document & getDocSum(const search::DocumentIdT & docId) const;
-  void push_back(const Document::SP & doc);
-  void insert(const DocSumCache & dc);
-  const DocSumCacheT & cache()  const { return _list; }
- private:
-  DocSumCacheT _list;
-};
-
 }
 
diff --git a/vsm/src/vespa/vsm/common/document.cpp b/vsm/src/vespa/vsm/common/document.cpp
index cc7414b8107..7939391a503 100644
--- a/vsm/src/vespa/vsm/common/document.cpp
+++ b/vsm/src/vespa/vsm/common/document.cpp
@@ -57,24 +57,14 @@ FieldIdT StringFieldIdTMap::fieldNo(const vespalib::string & fName) const
 size_t StringFieldIdTMap::highestFieldNo() const
 {
   size_t maxFNo(0);
-  for (StringFieldIdTMapT::const_iterator it = _map.begin(), mt = _map.end(); it != mt; it++) {
-    if (it->second >= maxFNo) {
-      maxFNo = it->second + 1;
+  for (const auto & field : _map) {
+    if (field.second > maxFNo) {
+      maxFNo = field.second;
     }
   }
-  return maxFNo;
+  return maxFNo+1;
 }
 
-Document::Document(const DocumentIdT & doc, size_t maxField) :
-  _docId(doc),
-  _fieldCount(maxField)
-{ }
-
-Document::Document() :
-  _docId(0),
-  _fieldCount(0)
-{ }
-
 Document::~Document() { }
 
 }
diff --git a/vsm/src/vespa/vsm/common/document.h b/vsm/src/vespa/vsm/common/document.h
index b4fc7c86d8e..a0341aaf7c0 100644
--- a/vsm/src/vespa/vsm/common/document.h
+++ b/vsm/src/vespa/vsm/common/document.h
@@ -47,14 +47,12 @@ typedef vespalib::stringref FieldRef;
 class Document
 {
  public:
-  typedef std::shared_ptr<Document> SP;
-  Document();
-  Document(const search::DocumentIdT & doc, size_t maxFieldCount);
+  Document(size_t maxFieldCount) : _docId(0), _fieldCount(maxFieldCount) { }
+  Document(search::DocumentIdT doc, size_t maxFieldCount) : _docId(doc), _fieldCount(maxFieldCount) { }
   virtual ~Document();
   const search::DocumentIdT & getDocId()        const { return _docId; }
   size_t getFieldCount()                        const { return _fieldCount; }
   void setDocId(const search::DocumentIdT & v)        { _docId = v; }
-  void setFieldCount(size_t v)                        { _fieldCount = v; }
   virtual const document::FieldValue * getField(FieldIdT fId) const = 0;
   /**
    Returns true, if not possible to set.
@@ -62,7 +60,7 @@ class Document
   virtual bool setField(FieldIdT fId, document::FieldValue::UP fv) = 0;
  private:
   search::DocumentIdT _docId;
-  size_t              _fieldCount;
+  const size_t        _fieldCount;
 };
 
 }
diff --git a/vsm/src/vespa/vsm/common/storagedocument.cpp b/vsm/src/vespa/vsm/common/storagedocument.cpp
index 4963e1661f0..1cb636a80c7 100644
--- a/vsm/src/vespa/vsm/common/storagedocument.cpp
+++ b/vsm/src/vespa/vsm/common/storagedocument.cpp
@@ -2,51 +2,29 @@
 #include "storagedocument.h"
 #include <vespa/document/fieldvalue/arrayfieldvalue.h>
 #include <vespa/document/fieldvalue/weightedsetfieldvalue.h>
-#include <vespa/document/datatype/datatype.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".vsm.storagedocument");
 
 namespace vsm {
 
-StorageDocument::StorageDocument(const SharedFieldPathMap & fim) :
-    Document(),
-    _doc(new document::Document()),
+StorageDocument::StorageDocument(document::Document::UP doc, const SharedFieldPathMap & fim, size_t fieldNoLimit) :
+    Document(fieldNoLimit),
+    _doc(std::move(doc)),
     _fieldMap(fim),
-    _cachedFields(),
-    _backedFields()
-{ }
-
-StorageDocument::StorageDocument(document::Document::UP doc) :
-    Document(),
-    _doc(doc.release()),
-    _fieldMap(),
-    _cachedFields(),
+    _cachedFields(getFieldCount()),
     _backedFields()
 { }
 
 StorageDocument::~StorageDocument() { }
 
-void StorageDocument::init()
-{
-    _cachedFields.clear();
-    _cachedFields.resize(getFieldCount());
-}
-
 namespace {
     FieldPath _emptyFieldPath;
     StorageDocument::SubDocument _empySubDocument(NULL, _emptyFieldPath.begin(), _emptyFieldPath.end());
 }
 
-void StorageDocument::SubDocument::swap(SubDocument & rhs)
-{
-    std::swap(_fieldValue, rhs._fieldValue);
-    std::swap(_it, rhs._it);
-    std::swap(_mt, rhs._mt);
-}
-
-
-const StorageDocument::SubDocument & StorageDocument::getComplexField(FieldIdT fId) const
+const StorageDocument::SubDocument &
+StorageDocument::getComplexField(FieldIdT fId) const
 {
     if (_cachedFields[fId].getFieldValue() == NULL) {
         const FieldPath & fp = (*_fieldMap)[fId];
@@ -68,7 +46,7 @@ const StorageDocument::SubDocument & StorageDocument::getComplexField(FieldIdT f
     return _cachedFields[fId];
 }
 
-void StorageDocument::saveCachedFields()
+void StorageDocument::saveCachedFields() const
 {
     size_t m(_cachedFields.size());
     _backedFields.reserve(m);
@@ -80,7 +58,8 @@ void StorageDocument::saveCachedFields()
     }
 }
 
-const document::FieldValue * StorageDocument::getField(FieldIdT fId) const
+const document::FieldValue *
+StorageDocument::getField(FieldIdT fId) const
 {
     return getComplexField(fId).getFieldValue();
 }
diff --git a/vsm/src/vespa/vsm/common/storagedocument.h b/vsm/src/vespa/vsm/common/storagedocument.h
index cf0638cb3c3..3c1f2ddc375 100644
--- a/vsm/src/vespa/vsm/common/storagedocument.h
+++ b/vsm/src/vespa/vsm/common/storagedocument.h
@@ -11,49 +11,50 @@ typedef document::FieldPath FieldPath; // field path to navigate a field value
 typedef std::vector<FieldPath> FieldPathMapT; // map from field id to field path
 typedef std::shared_ptr<FieldPathMapT> SharedFieldPathMap;
 
-class StorageDocument : public Document
-{
- public:
-  typedef vespalib::LinkedPtr<StorageDocument> SP;
-  class SubDocument {
-  public:
-      SubDocument() :
-          _fieldValue(NULL)
-      { }
-      SubDocument(document::FieldValue * fv, FieldPath::const_iterator it, FieldPath::const_iterator mt) :
-          _fieldValue(fv),
-          _it(it),
-          _mt(mt)
-      { }
-      const document::FieldValue * getFieldValue() const { return _fieldValue; }
-      void setFieldValue(document::FieldValue * fv) { _fieldValue = fv; }
-      FieldPath::const_iterator begin() const { return _it; }
-      FieldPath::const_iterator end()   const { return _mt; }
-      void swap(SubDocument & rhs);
-  private:
-      document::FieldValue    * _fieldValue;
-      FieldPath::const_iterator _it;
-      FieldPath::const_iterator _mt;
-  };
-  StorageDocument(const SharedFieldPathMap & fim);
-  StorageDocument(const document::Document & doc);
-  StorageDocument(document::Document::UP doc);
-  virtual ~StorageDocument();
-  void init();
-  const document::Document & docDoc()         const { return *_doc; }
-  void fieldPathMap(const SharedFieldPathMap & fim) { _fieldMap = fim; }
-  const SharedFieldPathMap & fieldPathMap()   const { return _fieldMap; }
-  bool valid()                                const { return _doc.get() != NULL; }
-  const SubDocument & getComplexField(FieldIdT fId) const;
-  virtual const document::FieldValue * getField(FieldIdT fId) const;
-  virtual bool setField(FieldIdT fId, document::FieldValue::UP fv);
-  void saveCachedFields();
- private:
-  typedef vespalib::CloneablePtr<document::Document> DocumentContainer;
-  DocumentContainer      _doc;
-  SharedFieldPathMap     _fieldMap;
-  mutable std::vector<SubDocument> _cachedFields;
-  mutable std::vector<document::FieldValue::UP> _backedFields;
+class StorageDocument : public Document {
+public:
+    typedef vespalib::LinkedPtr<StorageDocument> LP;
+
+    class SubDocument {
+    public:
+        SubDocument() : _fieldValue(NULL) {}
+        SubDocument(document::FieldValue *fv, FieldPath::const_iterator it, FieldPath::const_iterator mt) :
+                _fieldValue(fv),
+                _it(it),
+                _mt(mt)
+        { }
+
+        const document::FieldValue *getFieldValue() const { return _fieldValue; }
+        void setFieldValue(document::FieldValue *fv) { _fieldValue = fv; }
+        FieldPath::const_iterator begin() const { return _it; }
+        FieldPath::const_iterator end() const { return _mt; }
+        void swap(SubDocument &rhs) {
+            std::swap(_fieldValue, rhs._fieldValue);
+            std::swap(_it, rhs._it);
+            std::swap(_mt, rhs._mt);
+        }
+    private:
+        document::FieldValue *_fieldValue;
+        FieldPath::const_iterator _it;
+        FieldPath::const_iterator _mt;
+    };
+public:
+    StorageDocument(document::Document::UP doc, const SharedFieldPathMap &fim, size_t fieldNoLimit);
+    StorageDocument(const StorageDocument &) = delete;
+    StorageDocument & operator = (const StorageDocument &) = delete;
+    ~StorageDocument();
+
+    const document::Document &docDoc() const { return *_doc; }
+    bool valid() const { return _doc.get() != NULL; }
+    const SubDocument &getComplexField(FieldIdT fId) const;
+    const document::FieldValue *getField(FieldIdT fId) const override;
+    bool setField(FieldIdT fId, document::FieldValue::UP fv) override ;
+    void saveCachedFields() const;
+private:
+    document::Document::UP _doc;
+    SharedFieldPathMap     _fieldMap;
+    mutable std::vector<SubDocument> _cachedFields;
+    mutable std::vector<document::FieldValue::UP> _backedFields;
 };
 
 }