Don't derive v7 serialization from a prior versioned protocol

Make `ProtocolSerialization` a proper interface and introduce an
intermediate `LegacyProtocolSerialization` class for the legacy
versioned protocol implementations.

7 files changed, 54 insertions, 42 deletions

diff --git a/storageapi/src/vespa/storageapi/mbusprot/legacyprotocolserialization.h b/storageapi/src/vespa/storageapi/mbusprot/legacyprotocolserialization.h
new file mode 100644
index 00000000000..0c5f786589c
--- /dev/null
+++ b/storageapi/src/vespa/storageapi/mbusprot/legacyprotocolserialization.h
@@ -0,0 +1,26 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include "protocolserialization.h"
+
+namespace storage::mbusprot {
+
+class LegacyProtocolSerialization : public ProtocolSerialization {
+    const std::shared_ptr<const document::DocumentTypeRepo> _repo;
+public:
+    explicit LegacyProtocolSerialization(const std::shared_ptr<const document::DocumentTypeRepo>& repo)
+        : _repo(repo)
+    {}
+
+    const document::DocumentTypeRepo& getTypeRepo() const { return *_repo; }
+    const std::shared_ptr<const document::DocumentTypeRepo> getTypeRepoSp() const { return _repo; }
+
+    virtual document::Bucket getBucket(document::ByteBuffer& buf) const = 0;
+    virtual void putBucket(const document::Bucket& bucket, vespalib::GrowableByteBuffer& buf) const = 0;
+    virtual document::BucketSpace getBucketSpace(document::ByteBuffer& buf) const = 0;
+    virtual void putBucketSpace(document::BucketSpace bucketSpace, vespalib::GrowableByteBuffer& buf) const = 0;
+    virtual api::BucketInfo getBucketInfo(document::ByteBuffer& buf) const = 0;
+    virtual void putBucketInfo(const api::BucketInfo& info, vespalib::GrowableByteBuffer& buf) const = 0;
+};
+
+} // storage::mbusprot
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp
index 172cd6c8de5..917b60c50c3 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp
@@ -17,11 +17,6 @@ LOG_SETUP(".storage.api.mbusprot.serialization.base");
 
 namespace storage::mbusprot {
 
-ProtocolSerialization::ProtocolSerialization(const std::shared_ptr<const document::DocumentTypeRepo>& repo)
-    : _repo(repo)
-{
-}
-
 mbus::Blob
 ProtocolSerialization::encode(const api::StorageMessage& msg) const
 {
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h
index 9c3ddb88bdf..6fe152302ee 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h
@@ -59,21 +59,14 @@ class StorageCommand;
 class StorageReply;
 
 class ProtocolSerialization {
-    const std::shared_ptr<const document::DocumentTypeRepo> _repo;
-
 public:
     virtual mbus::Blob encode(const api::StorageMessage&) const;
     virtual std::unique_ptr<StorageCommand> decodeCommand(mbus::BlobRef) const;
     virtual std::unique_ptr<StorageReply> decodeReply(
                             mbus::BlobRef, const api::StorageCommand&) const;
-
 protected:
-    const document::DocumentTypeRepo& getTypeRepo() const { return *_repo; }
-    const std::shared_ptr<const document::DocumentTypeRepo> getTypeRepoSp() const
-    { return _repo; }
-
-    ProtocolSerialization(const std::shared_ptr<const document::DocumentTypeRepo> &repo);
-    virtual ~ProtocolSerialization() {}
+    ProtocolSerialization() = default;
+    virtual ~ProtocolSerialization() = default;
 
     typedef api::StorageCommand SCmd;
     typedef api::StorageReply SRep;
@@ -160,14 +153,6 @@ protected:
     virtual SRep::UP onDecodeDestroyVisitorReply(const SCmd&, BBuf&) const = 0;
     virtual SCmd::UP onDecodeRemoveLocationCommand(BBuf&) const = 0;
     virtual SRep::UP onDecodeRemoveLocationReply(const SCmd&, BBuf&) const = 0;
-
-    virtual document::Bucket getBucket(document::ByteBuffer& buf) const = 0;
-    virtual void putBucket(const document::Bucket& bucket, vespalib::GrowableByteBuffer& buf) const = 0;
-    virtual document::BucketSpace getBucketSpace(document::ByteBuffer& buf) const = 0;
-    virtual void putBucketSpace(document::BucketSpace bucketSpace, vespalib::GrowableByteBuffer& buf) const = 0;
-    virtual api::BucketInfo getBucketInfo(document::ByteBuffer& buf) const = 0;
-    virtual void putBucketInfo(const api::BucketInfo& info, vespalib::GrowableByteBuffer& buf) const = 0;
-
 };
 
 }
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp
index 74a0c964d19..466ff85f398 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp
@@ -20,7 +20,7 @@ namespace storage::mbusprot {
 
 ProtocolSerialization4_2::ProtocolSerialization4_2(
         const std::shared_ptr<const document::DocumentTypeRepo>& repo)
-    : ProtocolSerialization(repo)
+    : LegacyProtocolSerialization(repo)
 {
 }
 
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h
index 56aa3d4ed30..e4ab36dc989 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h
@@ -1,13 +1,13 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include "protocolserialization.h"
+#include "legacyprotocolserialization.h"
 
 namespace storage::mbusprot {
 
-class ProtocolSerialization4_2 : public ProtocolSerialization {
+class ProtocolSerialization4_2 : public LegacyProtocolSerialization {
 public:
-    ProtocolSerialization4_2(const std::shared_ptr<const document::DocumentTypeRepo>&);
+    explicit ProtocolSerialization4_2(const std::shared_ptr<const document::DocumentTypeRepo>&);
 
 protected:
     void onEncode(GBBuf&, const api::GetCommand&) const override;
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp
index 2e0ac075853..b5b560a3df0 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp
@@ -8,19 +8,23 @@
 #include "protocolserialization7.h"
 #include "serializationhelper.h"
 #include "storageapi.pb.h"
+
+#pragma GCC diagnostic pop
+
 #include <vespa/document/update/documentupdate.h>
 #include <vespa/document/util/bufferexceptions.h>
 #include <vespa/storageapi/message/bucketsplitting.h>
+#include <vespa/storageapi/message/persistence.h>
 #include <vespa/storageapi/message/removelocation.h>
 #include <vespa/storageapi/message/visitor.h>
 
-#pragma GCC diagnostic pop
-
 namespace storage::mbusprot {
 
-ProtocolSerialization7::ProtocolSerialization7(const std::shared_ptr<const document::DocumentTypeRepo>& repo,
-                                                   const documentapi::LoadTypeSet& loadTypes)
-    : ProtocolSerialization6_0(repo, loadTypes)
+ProtocolSerialization7::ProtocolSerialization7(std::shared_ptr<const document::DocumentTypeRepo> repo,
+                                               const documentapi::LoadTypeSet& load_types)
+    : ProtocolSerialization(),
+      _repo(std::move(repo)),
+      _load_types(load_types)
 {
 }
 
@@ -278,7 +282,7 @@ void encode_response(vespalib::GrowableByteBuffer& out_buf, const api::StorageRe
 template <typename ProtobufType, typename Func>
 std::unique_ptr<api::StorageCommand>
 ProtocolSerialization7::decode_request(document::ByteBuffer& in_buf, Func&& f) const {
-    RequestDecoder<ProtobufType> dec(in_buf, loadTypes());
+    RequestDecoder<ProtobufType> dec(in_buf, _load_types);
     const auto& req = dec.request();
     auto cmd = f(req);
     dec.transfer_meta_information_to(*cmd);
@@ -331,7 +335,7 @@ void encode_bucket_info_response(vespalib::GrowableByteBuffer& out_buf, const ap
 template <typename ProtobufType, typename Func>
 std::unique_ptr<api::StorageCommand>
 ProtocolSerialization7::decode_bucket_request(document::ByteBuffer& in_buf, Func&& f) const {
-    RequestDecoder<ProtobufType> dec(in_buf, loadTypes());
+    RequestDecoder<ProtobufType> dec(in_buf, _load_types);
     const auto& req = dec.request();
     if (!req.has_bucket()) {
         throw vespalib::IllegalArgumentException("Malformed protocol buffer request; no bucket"); // TODO proto type name?
@@ -404,7 +408,7 @@ void ProtocolSerialization7::onEncode(GBBuf& buf, const api::PutReply& msg) cons
 
 api::StorageCommand::UP ProtocolSerialization7::onDecodePutCommand(BBuf& buf) const {
     return decode_bucket_request<protobuf::PutRequest>(buf, [&](auto& req, auto& bucket) {
-        auto document = get_document(req.document(), getTypeRepo());
+        auto document = get_document(req.document(), type_repo());
         auto cmd = std::make_unique<api::PutCommand>(bucket, std::move(document), req.new_timestamp());
         cmd->setUpdateTimestamp(req.expected_old_timestamp());
         if (req.has_condition()) {
@@ -450,7 +454,7 @@ api::StorageCommand::UP ProtocolSerialization7::onDecodeUpdateCommand(BBuf& buf)
         // TODO move out
         std::shared_ptr<document::DocumentUpdate> update;
         if (req.has_update() && !req.update().payload().empty()) {
-            update = document::DocumentUpdate::createHEAD(getTypeRepo(), vespalib::nbostream(
+            update = document::DocumentUpdate::createHEAD(type_repo(), vespalib::nbostream(
                     req.update().payload().data(), req.update().payload().size()));
         }
         auto cmd = std::make_unique<api::UpdateCommand>(bucket, std::move(update), req.new_timestamp());
@@ -538,7 +542,7 @@ api::StorageCommand::UP ProtocolSerialization7::onDecodeGetCommand(BBuf& buf) co
 api::StorageReply::UP ProtocolSerialization7::onDecodeGetReply(const SCmd& cmd, BBuf& buf) const {
     return decode_bucket_info_response<protobuf::GetResponse>(buf, [&](auto& res) {
         try {
-            auto document = get_document(res.document(), getTypeRepo());
+            auto document = get_document(res.document(), type_repo());
             return std::make_unique<api::GetReply>(static_cast<const api::GetCommand&>(cmd),
                                                    std::move(document), res.last_modified_timestamp());
         } catch (std::exception& e) {
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.h b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.h
index 17c4c71ef8e..f3499150278 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.h
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.h
@@ -2,7 +2,7 @@
 
 #pragma once
 
-#include "protocolserialization6_0.h"
+#include "protocolserialization.h"
 #include <vespa/documentapi/loadtypes/loadtypeset.h>
 
 namespace storage {
@@ -11,13 +11,15 @@ namespace mbusprot {
 /**
  * Protocol serialization version that uses Protocol Buffers for all its binary
  * encoding and decoding.
- *
- * TODO stop inheriting from _versioned_ protocol impl once all methods are implemented here.
  */
-class ProtocolSerialization7 : public ProtocolSerialization6_0 {
+class ProtocolSerialization7 : public ProtocolSerialization {
+    const std::shared_ptr<const document::DocumentTypeRepo> _repo;
+    const documentapi::LoadTypeSet& _load_types;
 public:
-    ProtocolSerialization7(const std::shared_ptr<const document::DocumentTypeRepo> &repo,
-                             const documentapi::LoadTypeSet &loadTypes);
+    ProtocolSerialization7(std::shared_ptr<const document::DocumentTypeRepo> repo,
+                           const documentapi::LoadTypeSet& load_types);
+
+    const document::DocumentTypeRepo& type_repo() const { return *_repo; }
 
     // Put
     void onEncode(GBBuf&, const api::PutCommand&) const override;