Implement commit
The commit operation will serialize the binarystore protobuf and write
it to the designated sysfile location, with its size stored followed by
actual data.
Signed-off-by: Kun Yi <kunyi@google.com>
Change-Id: Idc16f410d3a1585daaddda58a3665d92a898f5c7
diff --git a/binarystore.cpp b/binarystore.cpp
index 98a6264..22e2e48 100644
--- a/binarystore.cpp
+++ b/binarystore.cpp
@@ -20,6 +20,42 @@
namespace binstore
{
+namespace internal
+{
+
+/* Helper methods to interconvert an uint64_t and bytes, LSB first.
+ Convert bytes to uint64. Input should be exactly 8 bytes. */
+uint64_t bytesToUint64(const std::string& bytes)
+{
+ if (bytes.size() != sizeof(uint64_t))
+ {
+ return 0;
+ }
+
+ return static_cast<uint64_t>(bytes[7]) << 56 |
+ static_cast<uint64_t>(bytes[6]) << 48 |
+ static_cast<uint64_t>(bytes[5]) << 40 |
+ static_cast<uint64_t>(bytes[4]) << 32 |
+ static_cast<uint64_t>(bytes[3]) << 24 |
+ static_cast<uint64_t>(bytes[2]) << 16 |
+ static_cast<uint64_t>(bytes[1]) << 8 |
+ static_cast<uint64_t>(bytes[0]);
+}
+
+/* Convert uint64 to bytes, LSB first. */
+std::string uint64ToBytes(uint64_t num)
+{
+ std::string result;
+ for (size_t i = 0; i < sizeof(uint64_t); ++i)
+ {
+ result += static_cast<char>(num & 0xff);
+ num >>= 8;
+ }
+ return result;
+}
+
+} // namespace internal
+
std::unique_ptr<BinaryStoreInterface>
BinaryStore::createFromConfig(const std::string& baseBlobId,
std::unique_ptr<SysFile> file,
@@ -71,6 +107,31 @@
writable_ = flags & blobs::OpenFlags::write;
+ /* Load blob from sysfile if we know it might not match what we have.
+ * Note it will overwrite existing unsaved data per design. */
+ if (commitState_ != CommitState::Clean)
+ {
+ try
+ {
+ /* Parse length-prefixed format to protobuf */
+ auto size =
+ internal::bytesToUint64(file_->readAsStr(0, sizeof(uint64_t)));
+ if (!blob_.ParseFromString(
+ file_->readAsStr(sizeof(uint64_t), size)))
+ {
+ /* Fail to parse the data, which might mean no preexsiting data
+ * and is a valid case to handle */
+ // TODO: logging
+ }
+ }
+ catch (const std::exception& e)
+ {
+ /* Read causes unexpected system-level failure */
+ // TODO: logging
+ return false;
+ }
+ }
+
/* Iterate and find if there is an existing blob with this id.
* blobsPtr points to a BinaryBlob container with STL-like semantics*/
auto blobsPtr = blob_.mutable_blobs();
@@ -152,13 +213,31 @@
bool BinaryStore::commit()
{
- return false;
+
+ auto blobData = blob_.SerializeAsString();
+ std::string commitData(internal::uint64ToBytes((uint64_t)blobData.size()));
+ commitData += blobData;
+
+ try
+ {
+ file_->writeStr(commitData, 0);
+ }
+ catch (const std::exception& e)
+ {
+ // TODO: logging
+ commitState_ = CommitState::CommitError;
+ return false;
+ };
+
+ commitState_ = CommitState::Clean;
+ return true;
}
bool BinaryStore::close()
{
currentBlob_ = nullptr;
writable_ = false;
+ commitState_ = CommitState::Dirty;
return true;
}
diff --git a/binarystore.hpp b/binarystore.hpp
index 2e5f562..551d22d 100644
--- a/binarystore.hpp
+++ b/binarystore.hpp
@@ -74,12 +74,15 @@
virtual bool write(uint32_t offset, const std::vector<uint8_t>& data) = 0;
/**
- * TODO
+ * Commits data to the persistent storage specified during blob init.
+ * @returns True if able to write data to sysfile successfully
*/
virtual bool commit() = 0;
/**
- * TODO
+ * Closes blob, which prevents further modifications. Uncommitted data will
+ * be lost.
+ * @returns True if able to close the blob successfully
*/
virtual bool close() = 0;
@@ -136,12 +139,20 @@
std::unique_ptr<SysFile> file, uint32_t maxSize);
private:
+ enum class CommitState
+ {
+ Dirty, // In-memory data might not match persisted data
+ Clean, // In-memory data matches persisted data
+ CommitError // Error happened during committing
+ };
+
std::string baseBlobId_;
binaryblobproto::BinaryBlobBase blob_;
binaryblobproto::BinaryBlob* currentBlob_ = nullptr;
bool writable_ = false;
std::unique_ptr<SysFile> file_ = nullptr;
uint32_t maxSize_;
+ CommitState commitState_ = CommitState::Dirty;
};
} // namespace binstore
diff --git a/binarystore_mock.hpp b/binarystore_mock.hpp
index bf8c811..3c7e6c8 100644
--- a/binarystore_mock.hpp
+++ b/binarystore_mock.hpp
@@ -31,6 +31,8 @@
.WillByDefault(Invoke(&real_store_, &BinaryStore::read));
ON_CALL(*this, write)
.WillByDefault(Invoke(&real_store_, &BinaryStore::write));
+ ON_CALL(*this, commit)
+ .WillByDefault(Invoke(&real_store_, &BinaryStore::commit));
}
MOCK_CONST_METHOD0(getBaseBlobId, std::string());
MOCK_CONST_METHOD0(getBlobIds, std::vector<std::string>());
diff --git a/handler.cpp b/handler.cpp
index e0a343a..fbfaf47 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -151,8 +151,13 @@
bool BinaryStoreBlobHandler::commit(uint16_t session,
const std::vector<uint8_t>& data)
{
- // TODO: implement
- return false;
+ auto it = sessions_.find(session);
+ if (it == sessions_.end())
+ {
+ return false;
+ }
+
+ return it->second->commit();
}
bool BinaryStoreBlobHandler::close(uint16_t session)
diff --git a/test/Makefile.am b/test/Makefile.am
index f5fe63b..cff1416 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -12,6 +12,7 @@
# Run all 'check' test programs
check_PROGRAMS = \
sys_file_unittest \
+ handler_commit_unittest \
handler_open_unittest \
handler_readwrite_unittest \
handler_unittest
@@ -29,6 +30,15 @@
-lprotobuf
handler_unittest_CXXFLAGS = $(PHOSPHOR_LOGGING_CFLAGS)
+handler_commit_unittest_SOURCES = handler_commit_unittest.cpp
+handler_commit_unittest_LDADD = $(PHOSPHOR_LOGGING_LIBS) \
+ $(top_builddir)/handler.o \
+ $(top_builddir)/binarystore.o \
+ $(top_builddir)/sys_file.o \
+ $(top_builddir)/libbinarystore_la-binaryblob.pb.o \
+ -lprotobuf
+handler_commit_unittest_CXXFLAGS = $(PHOSPHOR_LOGGING_CFLAGS)
+
handler_open_unittest_SOURCES = handler_open_unittest.cpp
handler_open_unittest_LDADD = $(PHOSPHOR_LOGGING_LIBS) \
$(top_builddir)/handler.o \
diff --git a/test/handler_commit_unittest.cpp b/test/handler_commit_unittest.cpp
new file mode 100644
index 0000000..9a79c36
--- /dev/null
+++ b/test/handler_commit_unittest.cpp
@@ -0,0 +1,101 @@
+#include "handler_unittest.hpp"
+
+using ::testing::_;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::Return;
+using ::testing::StartsWith;
+
+using namespace std::string_literals;
+
+namespace blobs
+{
+
+class BinaryStoreBlobHandlerCommitTest : public BinaryStoreBlobHandlerTest
+{
+ protected:
+ BinaryStoreBlobHandlerCommitTest()
+ {
+ addDefaultStore(commitTestBaseId);
+ }
+
+ static inline std::string commitTestBaseId = "/test/"s;
+ static inline std::string commitTestBlobId = "/test/blob0"s;
+ static inline std::vector<uint8_t> commitTestData = {0, 1, 2, 3, 4,
+ 5, 6, 7, 8, 9};
+ static inline std::vector<uint8_t> commitTestDataToOverwrite = {10, 11, 12,
+ 13, 14};
+ static inline std::vector<uint8_t> commitMetaUnused;
+
+ static inline uint16_t commitTestSessionId = 0;
+ static inline uint16_t commitTestNewSessionId = 1;
+ static inline uint32_t commitTestOffset = 0;
+
+ void openAndWriteTestData()
+ {
+ uint16_t flags = OpenFlags::read | OpenFlags::write;
+ EXPECT_TRUE(handler.open(commitTestSessionId, flags, commitTestBlobId));
+ EXPECT_TRUE(handler.write(commitTestSessionId, commitTestOffset,
+ commitTestData));
+ }
+
+ void openWriteThenCommitData()
+ {
+ openAndWriteTestData();
+ EXPECT_TRUE(handler.commit(commitTestSessionId, commitMetaUnused));
+ }
+};
+
+TEST_F(BinaryStoreBlobHandlerCommitTest, CommittedDataIsReadable)
+{
+ openWriteThenCommitData();
+
+ EXPECT_EQ(commitTestData,
+ handler.read(commitTestSessionId, commitTestOffset,
+ commitTestData.size()));
+}
+
+TEST_F(BinaryStoreBlobHandlerCommitTest, CommittedDataCanBeReopened)
+{
+ openWriteThenCommitData();
+
+ EXPECT_TRUE(handler.close(commitTestSessionId));
+ EXPECT_TRUE(handler.open(commitTestNewSessionId, OpenFlags::read,
+ commitTestBlobId));
+ EXPECT_EQ(commitTestData,
+ handler.read(commitTestNewSessionId, commitTestOffset,
+ commitTestData.size()));
+}
+
+TEST_F(BinaryStoreBlobHandlerCommitTest, OverwrittenDataCanBeCommitted)
+{
+ openWriteThenCommitData();
+
+ EXPECT_TRUE(handler.write(commitTestSessionId, commitTestOffset,
+ commitTestDataToOverwrite));
+ EXPECT_TRUE(handler.commit(commitTestSessionId, commitMetaUnused));
+ EXPECT_TRUE(handler.close(commitTestSessionId));
+
+ EXPECT_TRUE(handler.open(commitTestNewSessionId, OpenFlags::read,
+ commitTestBlobId));
+ EXPECT_EQ(commitTestDataToOverwrite,
+ handler.read(commitTestNewSessionId, commitTestOffset,
+ commitTestDataToOverwrite.size()));
+}
+
+TEST_F(BinaryStoreBlobHandlerCommitTest, UncommittedDataIsLostUponClose)
+{
+ openWriteThenCommitData();
+
+ EXPECT_TRUE(handler.write(commitTestSessionId, commitTestOffset,
+ commitTestDataToOverwrite));
+ EXPECT_TRUE(handler.close(commitTestSessionId));
+
+ EXPECT_TRUE(handler.open(commitTestNewSessionId, OpenFlags::read,
+ commitTestBlobId));
+ EXPECT_EQ(commitTestData,
+ handler.read(commitTestNewSessionId, commitTestOffset,
+ commitTestData.size()));
+}
+
+} // namespace blobs