From b8b1698864563044fa0779eb8a5b5e04cf1b7b48 Mon Sep 17 00:00:00 2001
From: Nicolas Pope <nwpope@utu.fi>
Date: Fri, 12 Jun 2020 13:31:34 +0300
Subject: [PATCH] Tidy frame template code

---
 .../structures/include/ftl/data/new_frame.hpp | 74 +++++--------------
 components/structures/src/new_frame.cpp       | 40 ++++++++++
 components/structures/test/CMakeLists.txt     |  6 +-
 3 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/components/structures/include/ftl/data/new_frame.hpp b/components/structures/include/ftl/data/new_frame.hpp
index 5743e9552..5a45662de 100644
--- a/components/structures/include/ftl/data/new_frame.hpp
+++ b/components/structures/include/ftl/data/new_frame.hpp
@@ -92,7 +92,7 @@ class Frame {
 
 	inline size_t size() const { return data_.size(); }
 
-	inline bool has(ftl::codecs::Channel c) const;
+	bool has(ftl::codecs::Channel c) const;
 
 	inline bool hasOwn(ftl::codecs::Channel c) const;
 
@@ -157,8 +157,6 @@ class Frame {
 	template <typename T>
 	T &createChange(ftl::codecs::Channel c, ftl::data::ChangeType t, std::vector<uint8_t> &data);
 
-	std::any &createAnyChange(ftl::codecs::Channel c, ftl::data::ChangeType t);
-
 	const std::vector<uint8_t> &getEncoded(ftl::codecs::Channel c) const;
 
 	template <typename T, typename ...ARGS>
@@ -237,6 +235,13 @@ class Frame {
 
 	inline MUTEX &mutex();
 
+	protected:
+	std::any &createAnyChange(ftl::codecs::Channel c, ftl::data::ChangeType t);
+
+	std::any &createAnyChange(ftl::codecs::Channel c, ftl::data::ChangeType t, std::vector<uint8_t> &data);
+
+	std::any &createAny(ftl::codecs::Channel c);
+
 	private:
 	struct ChannelData {
 		ChannelStatus status=ChannelStatus::INVALID;
@@ -301,15 +306,6 @@ class Session : public Frame {
 
 MUTEX &ftl::data::Frame::mutex() { return parent_->mutex(); }
 
-bool ftl::data::Frame::has(ftl::codecs::Channel c) const {
-	const auto &i = data_.find(c);
-	if (i != data_.end() && i->second.status != ftl::data::ChannelStatus::INVALID) {
-		return true;
-	} else {
-		return (parent_ && parent_->has(c)); 
-	}
-}
-
 bool ftl::data::Frame::hasOwn(ftl::codecs::Channel c) const {
 	const auto &i = data_.find(c);
 	return (i != data_.end() && i->second.status != ftl::data::ChannelStatus::INVALID);
@@ -367,7 +363,6 @@ template <typename T>
 const T &ftl::data::Frame::get(ftl::codecs::Channel c) const {
 	const auto &d = _getData(c);
 	if (d.status != ftl::data::ChannelStatus::INVALID) {
-		if (!d.data.has_value()) throw FTL_Error("Missing value in channel 'any' structure");
 		auto *p = std::any_cast<T>(&d.data);
 		if (!p) throw FTL_Error("'get' wrong type for channel (" << static_cast<unsigned int>(c) << ")");
 		return *p;
@@ -377,63 +372,34 @@ const T &ftl::data::Frame::get(ftl::codecs::Channel c) const {
 // Non-list version
 template <typename T, std::enable_if_t<!is_list<T>::value,int> = 0>
 T &ftl::data::Frame::create(ftl::codecs::Channel c) {
-	if (status_ != FrameStatus::STORED) throw FTL_Error("Cannot modify before store");
 	if (isAggregate(c)) throw FTL_Error("Aggregate channels must be of list type");
 
-	size_t t = getChannelType(c);
-	if (t > 0 && t != typeid(T).hash_code()) throw FTL_Error("Incorrect type for channel " << static_cast<unsigned int>(c));
+	ftl::data::verifyChannelType<T>(c);
 
-	auto &d = data_[c];
-	if (d.status != ftl::data::ChannelStatus::FLUSHED) {
-		d.status = ftl::data::ChannelStatus::VALID;
-		d.encoded.clear();
-		touch(c);
-		if (!isType<T>(c)) return d.data.emplace<T>();
-		else return *std::any_cast<T>(&d.data);
-	} else {
-		throw FTL_Error("Channel is flushed and read-only: " << static_cast<unsigned int>(c));
-	}
+	std::any &a = createAny(c);
+	if (!isType<T>(c)) return a.emplace<T>();
+	else return *std::any_cast<T>(&a);
 }
 
 // List version
 template <typename T, std::enable_if_t<is_list<T>::value,int> = 0>
 ftl::data::Aggregator<T> ftl::data::Frame::create(ftl::codecs::Channel c) {
-	if (status_ != FrameStatus::STORED) throw FTL_Error("Cannot modify before store");
-
-	size_t t = getChannelType(c);
-	if (t > 0 && t != typeid(T).hash_code()) throw FTL_Error("Incorrect type for channel " << static_cast<unsigned int>(c));
+	ftl::data::verifyChannelType<T>(c);
 
-	auto &d = data_[c];
-	if (d.status != ftl::data::ChannelStatus::FLUSHED) {
-		d.status = ftl::data::ChannelStatus::VALID;
-		d.encoded.clear();
-		touch(c);
-		if (!isType<T>(c)) {
-			d.data.emplace<T>();
-			return ftl::data::Aggregator<T>{*std::any_cast<T>(&d.data), isAggregate(c)};
-		} else return ftl::data::Aggregator<T>{*std::any_cast<T>(&d.data), isAggregate(c)};
-	} else {
-		throw FTL_Error("Channel is flushed and read-only: " << static_cast<unsigned int>(c));
-	}
+	std::any &a = createAny(c);
+	if (!isType<T>(c)) a.emplace<T>();
+	return ftl::data::Aggregator<T>{*std::any_cast<T>(&a), isAggregate(c)};
 }
 
 template <typename T>
 T &ftl::data::Frame::createChange(ftl::codecs::Channel c, ftl::data::ChangeType type, std::vector<uint8_t> &data) {
 	if (!bool(is_list<T>{}) && isAggregate(c)) throw FTL_Error("Aggregate channels must be of list type");
 
-	size_t t = getChannelType(c);
-	if (t > 0 && t != typeid(T).hash_code()) throw FTL_Error("Incorrect type for channel " << static_cast<unsigned int>(c));
+	ftl::data::verifyChannelType<T>(c);
 
-	auto &d = data_[c];
-	if (d.status != ftl::data::ChannelStatus::FLUSHED) {
-		d.status = ftl::data::ChannelStatus::DISPATCHED;
-		d.encoded = std::move(data);
-		touch(c, type);
-		if (!isType<T>(c)) return d.data.emplace<T>();
-		else return *std::any_cast<T>(&d.data);
-	} else {
-		throw FTL_Error("Channel is flushed and read-only: " << static_cast<unsigned int>(c));
-	}
+	std::any &a = createAnyChange(c, type, data);
+	if (!isType<T>(c)) return a.emplace<T>();
+	else return *std::any_cast<T>(&a);
 }
 
 // Non-list version
diff --git a/components/structures/src/new_frame.cpp b/components/structures/src/new_frame.cpp
index d90c2c162..7ed685d0c 100644
--- a/components/structures/src/new_frame.cpp
+++ b/components/structures/src/new_frame.cpp
@@ -55,7 +55,17 @@ Frame::~Frame() {
 	if (status_ != FrameStatus::RELEASED && pool_) pool_->release(*this);
 };
 
+bool ftl::data::Frame::has(ftl::codecs::Channel c) const {
+	const auto &i = data_.find(c);
+	if (i != data_.end() && i->second.status != ftl::data::ChannelStatus::INVALID) {
+		return true;
+	} else {
+		return (parent_ && parent_->has(c)); 
+	}
+}
+
 Frame::ChannelData &Frame::_getData(ftl::codecs::Channel c) {
+	if (status_ == FrameStatus::RELEASED) throw FTL_Error("Reading a released frame");
 	const auto &i = data_.find(c);
 	if (i != data_.end()) {
 		return i->second;
@@ -65,6 +75,7 @@ Frame::ChannelData &Frame::_getData(ftl::codecs::Channel c) {
 }
 
 const Frame::ChannelData &Frame::_getData(ftl::codecs::Channel c) const {
+	if (status_ == FrameStatus::RELEASED) throw FTL_Error("Reading a released frame");
 	const auto &i = data_.find(c);
 	if (i != data_.end()) {
 		return i->second;
@@ -79,6 +90,7 @@ std::any &Frame::createAnyChange(ftl::codecs::Channel c, ftl::data::ChangeType t
 	auto &d = data_[c];
 	if (d.status != ftl::data::ChannelStatus::FLUSHED) {
 		d.status = ftl::data::ChannelStatus::DISPATCHED;
+		d.encoded.clear();
 		touch(c, t);
 		return d.data;
 	} else {
@@ -86,6 +98,34 @@ std::any &Frame::createAnyChange(ftl::codecs::Channel c, ftl::data::ChangeType t
 	}
 }
 
+std::any &Frame::createAnyChange(ftl::codecs::Channel c, ftl::data::ChangeType t, std::vector<uint8_t> &data) {
+	if (status_ != FrameStatus::CREATED) throw FTL_Error("Cannot apply change after store " << static_cast<int>(status_));
+
+	auto &d = data_[c];
+	if (d.status != ftl::data::ChannelStatus::FLUSHED) {
+		d.status = ftl::data::ChannelStatus::DISPATCHED;
+		d.encoded = std::move(data);
+		touch(c, t);
+		return d.data;
+	} else {
+		throw FTL_Error("Channel is flushed and read-only: " << static_cast<unsigned int>(c));
+	}
+}
+
+std::any &Frame::createAny(ftl::codecs::Channel c) {
+	if (status_ != FrameStatus::STORED) throw FTL_Error("Cannot create before store or after flush");
+
+	auto &d = data_[c];
+	if (d.status != ftl::data::ChannelStatus::FLUSHED) {
+		d.status = ftl::data::ChannelStatus::VALID;
+		d.encoded.clear();
+		touch(c);
+		return d.data;
+	} else {
+		throw FTL_Error("Channel is flushed and read-only: " << static_cast<unsigned int>(c));
+	}
+}
+
 std::any &Frame::getAnyMutable(ftl::codecs::Channel c) {
 	auto &d = _getData(c);
 	return d.data;
diff --git a/components/structures/test/CMakeLists.txt b/components/structures/test/CMakeLists.txt
index 5ed458c7e..653cfd3c4 100644
--- a/components/structures/test/CMakeLists.txt
+++ b/components/structures/test/CMakeLists.txt
@@ -9,6 +9,8 @@ target_include_directories(nframe_unit PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../in
 target_link_libraries(nframe_unit
 	ftlcommon ftlcodecs)
 
+add_test(NFrameUnitTest nframe_unit)
+
 ### Pool Unit ##################################################################
 add_executable(pool_unit
 	$<TARGET_OBJECTS:CatchTest>
@@ -18,4 +20,6 @@ add_executable(pool_unit
 )
 target_include_directories(pool_unit PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../include")
 target_link_libraries(pool_unit
-	ftlcommon ftlcodecs)
\ No newline at end of file
+	ftlcommon ftlcodecs)
+
+add_test(MemPoolUnitTest pool_unit)
\ No newline at end of file
-- 
GitLab