Introduce ReadOnlyMetadataStorage

And use it for:
- MetadataStorageFromPlainObjectStorage
- MetadataStorageFromStaticFilesWebServer

This will allow to reduce ~100-200 lines of duplicated code, and plus
make the code less error prone.

Note, for now I tried to make this without behaviour changes.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.cpp

@@ -12,7 +12,6 @@ namespace DB
 
 namespace ErrorCodes
 {
-    extern const int NOT_IMPLEMENTED;
     extern const int LOGICAL_ERROR;
 }
 
@@ -63,23 +62,6 @@ bool MetadataStorageFromPlainObjectStorage::isDirectory(const std::string & path
     return !children.empty();
 }
 
-Poco::Timestamp MetadataStorageFromPlainObjectStorage::getLastModified(const std::string &) const
-{
-    /// NOTE: This is required for MergeTree
-    return {};
-}
-
-struct stat MetadataStorageFromPlainObjectStorage::stat(const std::string &) const
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "stat is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-time_t MetadataStorageFromPlainObjectStorage::getLastChanged(const std::string &) const
-{
-    /// NOTE: by analogy with MetadataStorageFromStaticFilesWebServer::getLastChanged()
-    return {};
-}
-
 uint64_t MetadataStorageFromPlainObjectStorage::getFileSize(const String & path) const
 {
     RelativePathsWithSize children;
@@ -113,16 +95,6 @@ DirectoryIteratorPtr MetadataStorageFromPlainObjectStorage::iterateDirectory(con
     return std::make_unique<StaticDirectoryIterator>(std::move(fs_paths));
 }
 
-std::string MetadataStorageFromPlainObjectStorage::readFileToString(const std::string &) const
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "readFileToString is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-std::unordered_map<String, String> MetadataStorageFromPlainObjectStorage::getSerializedMetadata(const std::vector<String> &) const
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "getSerializedMetadata is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
 StoredObjects MetadataStorageFromPlainObjectStorage::getStorageObjects(const std::string & path) const
 {
     std::string blob_name = object_storage->generateBlobNameForPath(path);
@@ -131,99 +103,30 @@ StoredObjects MetadataStorageFromPlainObjectStorage::getStorageObjects(const std
     return {std::move(object)};
 }
 
-uint32_t MetadataStorageFromPlainObjectStorage::getHardlinkCount(const std::string &) const
-{
-    return 1;
-}
-
 const IMetadataStorage & MetadataStorageFromPlainObjectStorageTransaction::getStorageForNonTransactionalReads() const
 {
     return metadata_storage;
 }
 
-void MetadataStorageFromPlainObjectStorageTransaction::writeStringToFile(const std::string &, const std::string & /* data */)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "writeStringToFile is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::setLastModified(const std::string &, const Poco::Timestamp &)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "setLastModified is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
 void MetadataStorageFromPlainObjectStorageTransaction::unlinkFile(const std::string & path)
 {
     auto object = StoredObject::create(*metadata_storage.object_storage, metadata_storage.getAbsolutePath(path));
     metadata_storage.object_storage->removeObject(object);
 }
 
-void MetadataStorageFromPlainObjectStorageTransaction::removeRecursive(const std::string &)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "removeRecursive is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
 void MetadataStorageFromPlainObjectStorageTransaction::createDirectory(const std::string &)
 {
     /// Noop. It is an Object Storage not a filesystem.
 }
-
 void MetadataStorageFromPlainObjectStorageTransaction::createDirectoryRecursive(const std::string &)
 {
     /// Noop. It is an Object Storage not a filesystem.
 }
-
-void MetadataStorageFromPlainObjectStorageTransaction::removeDirectory(const std::string &)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "removeDirectory is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::moveFile(const std::string & /* path_from */, const std::string & /* path_to */)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "moveFile is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::moveDirectory(const std::string & /* path_from */, const std::string & /* path_to */)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "moveDirectory is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::replaceFile(const std::string & /* path_from */, const std::string & /* path_to */)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "replaceFile is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::chmod(const String &, mode_t)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "chmod is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::setReadOnly(const std::string &)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "setReadOnly is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::createHardLink(const std::string & /* path_from */, const std::string & /* path_to */)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "createHardLink is not implemented for MetadataStorageFromPlainObjectStorage");
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::createEmptyMetadataFile(const std::string &)
-{
-    /// Noop, no separate metadata.
-}
-
-void MetadataStorageFromPlainObjectStorageTransaction::createMetadataFile(
-    const std::string &, const std::string & /* blob_name */, uint64_t /* size_in_bytes */)
-{
-    /// Noop, no separate metadata.
-}
-
 void MetadataStorageFromPlainObjectStorageTransaction::addBlobToMetadata(
     const std::string &, const std::string & /* blob_name */, uint64_t /* size_in_bytes */)
 {
     /// Noop, local metadata files is only one file, it is the metadata file itself.
 }
-
 void MetadataStorageFromPlainObjectStorageTransaction::unlinkMetadata(const std::string &)
 {
     /// Noop, no separate metadata.

src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.h

@@ -2,6 +2,7 @@
 
 #include <Disks/IDisk.h>
 #include <Disks/ObjectStorages/IMetadataStorage.h>
+#include <Disks/ObjectStorages/ReadOnlyMetadataStorage.h>
 #include <Disks/ObjectStorages/MetadataFromDiskTransactionState.h>
 #include <Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.h>
 
@@ -20,7 +21,10 @@ namespace DB
 /// It is used to allow BACKUP/RESTORE to ObjectStorage (S3/...) with the same
 /// structure as on disk MergeTree, and does not requires metadata from local
 /// disk to restore.
-class MetadataStorageFromPlainObjectStorage final : public IMetadataStorage
+///
+/// NOTE: Inheritance from ReadOnlyMetadataStorage is used here to throw
+/// NOT_IMPLEMENTED error for lost of unsupported methods (mtime/move/stat/...)
+class MetadataStorageFromPlainObjectStorage final : public ReadOnlyMetadataStorage
 {
 private:
     friend class MetadataStorageFromPlainObjectStorageTransaction;
@@ -45,26 +49,10 @@ public:
 
     uint64_t getFileSize(const String & path) const override;
 
-    Poco::Timestamp getLastModified(const std::string & path) const override;
-
-    time_t getLastChanged(const std::string & path) const override;
-
-    bool supportsChmod() const override { return false; }
-
-    bool supportsStat() const override { return false; }
-
-    struct stat stat(const String & path) const override;
-
     std::vector<std::string> listDirectory(const std::string & path) const override;
 
     DirectoryIteratorPtr iterateDirectory(const std::string & path) const override;
 
-    std::string readFileToString(const std::string & path) const override;
-
-    std::unordered_map<String, String> getSerializedMetadata(const std::vector<String> & file_paths) const override;
-
-    uint32_t getHardlinkCount(const std::string & path) const override;
-
     DiskPtr getDisk() const { return {}; }
 
     StoredObjects getStorageObjects(const std::string & path) const override;
@@ -75,7 +63,7 @@ private:
     std::filesystem::path getAbsolutePath(const std::string & path) const;
 };
 
-class MetadataStorageFromPlainObjectStorageTransaction final : public IMetadataTransaction
+class MetadataStorageFromPlainObjectStorageTransaction final : public ReadOnlyMetadataTransaction
 {
 private:
     const MetadataStorageFromPlainObjectStorage & metadata_storage;
@@ -90,41 +78,13 @@ public:
 
     const IMetadataStorage & getStorageForNonTransactionalReads() const final;
 
-    void commit() final {}
-
-    void writeStringToFile(const std::string & path, const std::string & data) override;
-
-    void createEmptyMetadataFile(const std::string & path) override;
-
-    void createMetadataFile(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) override;
-
     void addBlobToMetadata(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) override;
 
-    void setLastModified(const std::string & path, const Poco::Timestamp & timestamp) override;
-
-    bool supportsChmod() const override { return false; }
-
-    void chmod(const String & path, mode_t mode) override;
-
-    void setReadOnly(const std::string & path) override;
-
-    void unlinkFile(const std::string & path) override;
-
     void createDirectory(const std::string & path) override;
 
     void createDirectoryRecursive(const std::string & path) override;
 
-    void removeDirectory(const std::string & path) override;
+    void unlinkFile(const std::string & path) override;
-
-    void removeRecursive(const std::string & path) override;
-
-    void createHardLink(const std::string & path_from, const std::string & path_to) override;
-
-    void moveFile(const std::string & path_from, const std::string & path_to) override;
-
-    void moveDirectory(const std::string & path_from, const std::string & path_to) override;
-
-    void replaceFile(const std::string & path_from, const std::string & path_to) override;
 
     void unlinkMetadata(const std::string & path) override;
 };

src/Disks/ObjectStorages/ReadOnlyMetadataStorage.h

@@ -0,0 +1,165 @@
+#pragma once
+
+#include <Disks/ObjectStorages/IMetadataStorage.h>
+#include <Common/ErrorCodes.h>
+
+namespace DB
+{
+
+namespace ErrorCodes
+{
+    extern const int NOT_IMPLEMENTED;
+}
+
+class ReadOnlyMetadataStorage;
+
+/// Readonly, throws NOT_IMPLEMENTED error.
+/// Can be used to add limited read-only support of MergeTree.
+class ReadOnlyMetadataTransaction : public IMetadataTransaction
+{
+public:
+    ///
+    /// Noop
+    ///
+    void commit() override
+    {
+        /// Noop, nothing to commit.
+    }
+    void createEmptyMetadataFile(const std::string & /* path */) override
+    {
+        /// No metadata, no need to create anything.
+    }
+    void createMetadataFile(const std::string & /* path */, const std::string & /* blob_name */, uint64_t /* size_in_bytes */) override
+    {
+        /// Noop
+    }
+
+    ///
+    /// Throws
+    ///
+    void writeStringToFile(const std::string & /* path */, const std::string & /* data */) override
+    {
+        throwNotAllowed();
+    }
+    void setLastModified(const std::string & /* path */, const Poco::Timestamp & /* timestamp */) override
+    {
+        throwNotAllowed();
+    }
+    void chmod(const String & /* path */, mode_t /* mode */) override
+    {
+        throwNotAllowed();
+    }
+    void setReadOnly(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+    void unlinkFile(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+    void removeDirectory(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+    void removeRecursive(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+    void createHardLink(const std::string & /* path_from */, const std::string & /* path_to */) override
+    {
+        throwNotAllowed();
+    }
+    void moveFile(const std::string & /* path_from */, const std::string & /* path_to */) override
+    {
+        throwNotAllowed();
+    }
+    void moveDirectory(const std::string & /* path_from */, const std::string & /* path_to */) override
+    {
+        throwNotAllowed();
+    }
+    void replaceFile(const std::string & /* path_from */, const std::string & /* path_to */) override
+    {
+        throwNotAllowed();
+    }
+    void createDirectory(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+    void createDirectoryRecursive(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+    void addBlobToMetadata(const std::string & /* path */, const std::string & /* blob_name */, uint64_t /* size_in_bytes */) override
+    {
+        throwNotAllowed();
+    }
+    void unlinkMetadata(const std::string & /* path */) override
+    {
+        throwNotAllowed();
+    }
+
+
+    ///
+    /// Others
+    ///
+    bool supportsChmod() const override { return false; }
+
+private:
+    [[noreturn]] static void throwNotAllowed()
+    {
+        throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Only read-only transaction operations are supported");
+    }
+};
+
+/// Readonly, throws NOT_IMPLEMENTED error.
+/// Can be used to add limited read-only support of MergeTree.
+class ReadOnlyMetadataStorage : public IMetadataStorage
+{
+public:
+    ///
+    /// Noop
+    ///
+    Poco::Timestamp getLastModified(const std::string & /* path */) const override
+    {
+        /// Required by MergeTree
+        return {};
+    }
+    time_t getLastChanged(const std::string & /* path */) const override
+    {
+        return {};
+    }
+
+    struct stat stat(const String & /* path */) const override
+    {
+        throwNotAllowed();
+    }
+
+    uint32_t getHardlinkCount(const std::string & /* path */) const override
+    {
+        return 1;
+    }
+
+    std::string readFileToString(const std::string & /* path */) const override
+    {
+        throwNotAllowed();
+    }
+
+    std::unordered_map<std::string, std::string> getSerializedMetadata(const std::vector<String> & /* file_paths */) const override
+    {
+        throwNotAllowed();
+    }
+
+    ///
+    /// Others
+    ///
+    bool supportsChmod() const override { return false; }
+    bool supportsStat() const override { return false; }
+
+private:
+    [[noreturn]] static void throwNotAllowed()
+    {
+        throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Only read-only metadata operations are supported");
+    }
+};
+
+}

src/Disks/ObjectStorages/Web/MetadataStorageFromStaticFilesWebServer.cpp

@@ -12,7 +12,6 @@ namespace DB
 
 namespace ErrorCodes
 {
-    extern const int NOT_IMPLEMENTED;
     extern const int FILE_DOESNT_EXIST;
     extern const int NETWORK_ERROR;
 }
@@ -168,91 +167,11 @@ DirectoryIteratorPtr MetadataStorageFromStaticFilesWebServer::iterateDirectory(c
     return std::make_unique<StaticDirectoryIterator>(std::move(dir_file_paths));
 }
 
-std::string MetadataStorageFromStaticFilesWebServer::readFileToString(const std::string &) const
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-Poco::Timestamp MetadataStorageFromStaticFilesWebServer::getLastModified(const std::string &) const
-{
-    return {};
-}
-
-time_t MetadataStorageFromStaticFilesWebServer::getLastChanged(const std::string &) const
-{
-    return {};
-}
-
-uint32_t MetadataStorageFromStaticFilesWebServer::getHardlinkCount(const std::string &) const
-{
-    return 1;
-}
-
 const IMetadataStorage & MetadataStorageFromStaticFilesWebServerTransaction::getStorageForNonTransactionalReads() const
 {
     return metadata_storage;
 }
 
-void MetadataStorageFromStaticFilesWebServerTransaction::writeStringToFile(const std::string &, const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::setLastModified(const std::string &, const Poco::Timestamp &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::unlinkFile(const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::removeRecursive(const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::removeDirectory(const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::moveFile(const std::string &, const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::moveDirectory(const std::string &, const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::replaceFile(const std::string &, const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::setReadOnly(const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::createHardLink(const std::string &, const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::addBlobToMetadata(const std::string &, const std::string &, uint64_t)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::unlinkMetadata(const std::string &)
-{
-    WebObjectStorage::throwNotAllowed();
-}
-
 void MetadataStorageFromStaticFilesWebServerTransaction::createDirectory(const std::string &)
 {
     /// Noop.
@@ -263,30 +182,4 @@ void MetadataStorageFromStaticFilesWebServerTransaction::createDirectoryRecursiv
     /// Noop.
 }
 
-void MetadataStorageFromStaticFilesWebServerTransaction::createEmptyMetadataFile(const std::string & /* path */)
-{
-    /// Noop.
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::createMetadataFile(
-    const std::string & /* path */, const std::string & /* blob_name */, uint64_t /* size_in_bytes */)
-{
-    /// Noop.
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::commit()
-{
-    /// Noop.
-}
-
-std::unordered_map<String, String> MetadataStorageFromStaticFilesWebServer::getSerializedMetadata(const std::vector<String> &) const
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "getSerializedMetadata is not implemented for MetadataStorageFromStaticFilesWebServer");
-}
-
-void MetadataStorageFromStaticFilesWebServerTransaction::chmod(const String &, mode_t)
-{
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "chmod is not implemented for MetadataStorageFromStaticFilesWebServer");
-}
-
 }

src/Disks/ObjectStorages/Web/MetadataStorageFromStaticFilesWebServer.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <Disks/ObjectStorages/IMetadataStorage.h>
+#include <Disks/ObjectStorages/ReadOnlyMetadataStorage.h>
 #include <Disks/ObjectStorages/MetadataFromDiskTransactionState.h>
 #include <Disks/ObjectStorages/Web/WebObjectStorage.h>
 #include <Disks/IDisk.h>
@@ -9,7 +10,7 @@
 namespace DB
 {
 
-class MetadataStorageFromStaticFilesWebServer final : public IMetadataStorage
+class MetadataStorageFromStaticFilesWebServer final : public ReadOnlyMetadataStorage
 {
 private:
     friend class MetadataStorageFromStaticFilesWebServerTransaction;
@@ -36,32 +37,18 @@ public:
 
     uint64_t getFileSize(const String & path) const override;
 
-    Poco::Timestamp getLastModified(const std::string & path) const override;
-
-    time_t getLastChanged(const std::string & path) const override;
-
     std::vector<std::string> listDirectory(const std::string & path) const override;
 
     DirectoryIteratorPtr iterateDirectory(const std::string & path) const override;
 
-    std::string readFileToString(const std::string & path) const override;
-
-    std::unordered_map<String, String> getSerializedMetadata(const std::vector<String> & file_paths) const override;
-
-    uint32_t getHardlinkCount(const std::string & path) const override;
-
     StoredObjects getStorageObjects(const std::string & path) const override;
 
     std::string getObjectStorageRootPath() const override { return ""; }
 
-    bool supportsChmod() const override { return false; }
+    struct stat stat(const String & /* path */) const override { return {}; }
-
-    bool supportsStat() const override { return false; }
-
-    struct stat stat(const String &) const override { return {}; }
 };
 
-class MetadataStorageFromStaticFilesWebServerTransaction final : public IMetadataTransaction
+class MetadataStorageFromStaticFilesWebServerTransaction final : public ReadOnlyMetadataTransaction
 {
 private:
     DiskPtr disk;
@@ -77,43 +64,9 @@ public:
 
     const IMetadataStorage & getStorageForNonTransactionalReads() const override;
 
-    void commit() override;
-
-    void writeStringToFile(const std::string & path, const std::string & data) override;
-
-    void createEmptyMetadataFile(const std::string & path) override;
-
-    void createMetadataFile(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) override;
-
-    void addBlobToMetadata(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) override;
-
-    void setLastModified(const std::string & path, const Poco::Timestamp & timestamp) override;
-
-    void setReadOnly(const std::string & path) override;
-
-    void unlinkFile(const std::string & path) override;
-
     void createDirectory(const std::string & path) override;
 
     void createDirectoryRecursive(const std::string & path) override;
-
-    void removeDirectory(const std::string & path) override;
-
-    void removeRecursive(const std::string & path) override;
-
-    void createHardLink(const std::string & path_from, const std::string & path_to) override;
-
-    void moveFile(const std::string & path_from, const std::string & path_to) override;
-
-    void moveDirectory(const std::string & path_from, const std::string & path_to) override;
-
-    void replaceFile(const std::string & path_from, const std::string & path_to) override;
-
-    void unlinkMetadata(const std::string & path) override;
-
-    bool supportsChmod() const override { return false; }
-
-    void chmod(const String &, mode_t) override;
 };
 
 }