diff --git a/src/block_service/local/local_service.cpp b/src/block_service/local/local_service.cpp index ad71e2a613..9977fbe943 100644 --- a/src/block_service/local/local_service.cpp +++ b/src/block_service/local/local_service.cpp @@ -53,14 +53,38 @@ namespace block_service { DEFINE_TASK_CODE(LPC_LOCAL_SERVICE_CALL, TASK_PRIORITY_COMMON, THREAD_POOL_BLOCK_SERVICE) -bool file_metadata_from_json(const std::string &data, file_metadata &fmeta) noexcept +error_code file_metadata::dump_to_file(const std::string &file_path) const { + std::string data = nlohmann::json(*this).dump(); + auto s = + rocksdb::WriteStringToFile(dsn::utils::PegasusEnv(dsn::utils::FileDataType::kSensitive), + rocksdb::Slice(data), + file_path, + /* should_sync */ true); + if (!s.ok()) { + LOG_WARNING("write to metadata file '{}' failed, err = {}", file_path, s.ToString()); + return ERR_FS_INTERNAL; + } + + return ERR_OK; +} + +error_code file_metadata::load_from_file(const std::string &file_path) +{ + std::string data; + auto s = rocksdb::ReadFileToString( + dsn::utils::PegasusEnv(dsn::utils::FileDataType::kSensitive), file_path, &data); + if (!s.ok()) { + LOG_WARNING("load from metadata file '{}' failed, err = {}", file_path, s.ToString()); + return ERR_FS_INTERNAL; + } + try { - nlohmann::json::parse(data).get_to(fmeta); - return true; + nlohmann::json::parse(data).get_to(*this); + return ERR_OK; } catch (nlohmann::json::exception &exp) { - LOG_WARNING("decode metadata from json failed: {} [{}]", exp.what(), data); - return false; + LOG_WARNING("decode metadata from json failed: {}, data = [{}]", exp.what(), data); + return ERR_FS_INTERNAL; } } @@ -264,50 +288,23 @@ uint64_t local_file_object::get_size() { return _size; } error_code local_file_object::load_metadata() { - if (_has_meta_synced) + if (_has_meta_synced) { return ERR_OK; - - std::string metadata_path = local_service::get_metafile(file_name()); - std::string data; - auto s = rocksdb::ReadFileToString( - dsn::utils::PegasusEnv(dsn::utils::FileDataType::kSensitive), metadata_path, &data); - if (!s.ok()) { - LOG_WARNING("read file '{}' failed, err = {}", metadata_path, s.ToString()); - return ERR_FS_INTERNAL; } - file_metadata meta; - bool ans = file_metadata_from_json(data, meta); - if (!ans) { - LOG_WARNING("decode metadata '{}' file content failed", metadata_path); + file_metadata fmd; + std::string filepath = local_service::get_metafile(file_name()); + auto ec = fmd.load_from_file(filepath); + if (ec != ERR_OK) { + LOG_WARNING("load metadata file '{}' failed", filepath); return ERR_FS_INTERNAL; } - _size = meta.size; - _md5_value = meta.md5; + _size = fmd.size; + _md5_value = fmd.md5; _has_meta_synced = true; return ERR_OK; } -error_code local_file_object::store_metadata() -{ - file_metadata meta; - meta.md5 = _md5_value; - meta.size = _size; - std::string data = nlohmann::json(meta).dump(); - std::string metadata_path = local_service::get_metafile(file_name()); - auto s = - rocksdb::WriteStringToFile(dsn::utils::PegasusEnv(dsn::utils::FileDataType::kSensitive), - rocksdb::Slice(data), - metadata_path, - /* should_sync */ true); - if (!s.ok()) { - LOG_WARNING("store to metadata file {} failed, err={}", metadata_path, s.ToString()); - return ERR_FS_INTERNAL; - } - - return ERR_OK; -} - dsn::task_ptr local_file_object::write(const write_request &req, dsn::task_code code, const write_callback &cb, @@ -359,11 +356,10 @@ dsn::task_ptr local_file_object::write(const write_request &req, // a lot, but it is somewhat not correct. _size = resp.written_size; _md5_value = utils::string_md5(req.buffer.data(), req.buffer.length()); - // TODO(yingchun): make store_metadata as a local function, do not depend on the - // member variables (i.e. _size and _md5_value). - auto err = store_metadata(); + auto err = file_metadata(_size, _md5_value) + .dump_to_file(local_service::get_metafile(file_name())); if (err != ERR_OK) { - LOG_WARNING("store_metadata failed"); + LOG_ERROR("file_metadata write failed"); resp.err = ERR_FS_INTERNAL; break; } @@ -502,9 +498,10 @@ dsn::task_ptr local_file_object::upload(const upload_request &req, break; } - auto err = store_metadata(); + auto err = file_metadata(_size, _md5_value) + .dump_to_file(local_service::get_metafile(file_name())); if (err != ERR_OK) { - LOG_ERROR("store_metadata failed"); + LOG_ERROR("file_metadata write failed"); resp.err = ERR_FS_INTERNAL; break; } diff --git a/src/block_service/local/local_service.h b/src/block_service/local/local_service.h index 9c944e1d0e..0c1a5ee883 100644 --- a/src/block_service/local/local_service.h +++ b/src/block_service/local/local_service.h @@ -39,6 +39,14 @@ struct file_metadata { int64_t size = 0; std::string md5; + + file_metadata(int64_t s = 0, const std::string &m = "") : size(s), md5(m) {} + + // Dump the object to a file in JSON format. + error_code dump_to_file(const std::string &file_path) const; + + // Load the object from a file in JSON format. + error_code load_from_file(const std::string &file_path); }; NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(file_metadata, size, md5) @@ -102,7 +110,6 @@ class local_file_object : public block_file dsn::task_tracker *tracker = nullptr) override; error_code load_metadata(); - error_code store_metadata(); private: std::string compute_md5(); diff --git a/src/block_service/test/local_service_test.cpp b/src/block_service/test/local_service_test.cpp index 692a2d2c81..5eeb8dd0fe 100644 --- a/src/block_service/test/local_service_test.cpp +++ b/src/block_service/test/local_service_test.cpp @@ -24,11 +24,8 @@ #include #include #include -#include -#include -#include +#include #include -#include #include "block_service/local/local_service.h" #include "gtest/gtest.h" @@ -47,23 +44,18 @@ class local_service_test : public pegasus::encrypt_data_test_base INSTANTIATE_TEST_CASE_P(, local_service_test, ::testing::Values(false, true)); -TEST_P(local_service_test, store_metadata) +TEST_P(local_service_test, file_metadata) { - local_file_object file("a.txt"); - error_code ec = file.store_metadata(); - ASSERT_EQ(ERR_OK, ec); - - auto meta_file_path = local_service::get_metafile(file.file_name()); + const int64_t kSize = 12345; + const std::string kMD5 = "0123456789abcdef0123456789abcdef"; + auto meta_file_path = local_service::get_metafile("a.txt"); + ASSERT_EQ(ERR_OK, file_metadata(kSize, kMD5).dump_to_file(meta_file_path)); ASSERT_TRUE(boost::filesystem::exists(meta_file_path)); - std::string data; - auto s = rocksdb::ReadFileToString( - dsn::utils::PegasusEnv(dsn::utils::FileDataType::kSensitive), meta_file_path, &data); - ASSERT_TRUE(s.ok()) << s.ToString(); - - nlohmann::json j = nlohmann::json::parse(data); - ASSERT_EQ("", j["md5"]); - ASSERT_EQ(0, j["size"]); + file_metadata fm; + fm.load_from_file(meta_file_path); + ASSERT_EQ(kSize, fm.size); + ASSERT_EQ(kMD5, fm.md5); } TEST_P(local_service_test, load_metadata) @@ -72,15 +64,7 @@ TEST_P(local_service_test, load_metadata) auto meta_file_path = local_service::get_metafile(file.file_name()); { - nlohmann::json j({{"md5", "abcde"}, {"size", 5}}); - std::string data = j.dump(); - auto s = - rocksdb::WriteStringToFile(dsn::utils::PegasusEnv(dsn::utils::FileDataType::kSensitive), - rocksdb::Slice(data), - meta_file_path, - /* should_sync */ true); - ASSERT_TRUE(s.ok()) << s.ToString(); - + ASSERT_EQ(ERR_OK, file_metadata(5, "abcde").dump_to_file(meta_file_path)); ASSERT_EQ(ERR_OK, file.load_metadata()); ASSERT_EQ("abcde", file.get_md5sum()); ASSERT_EQ(5, file.get_size()); @@ -95,7 +79,7 @@ TEST_P(local_service_test, load_metadata) ASSERT_TRUE(s.ok()) << s.ToString(); local_file_object file2("a.txt"); - ASSERT_EQ(file2.load_metadata(), ERR_FS_INTERNAL); + ASSERT_EQ(ERR_FS_INTERNAL, file2.load_metadata()); } {