Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ffi: Add class for key-value pair log events. #507

Merged
merged 29 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
0da3f44
Implement CLP string
LinZhihao-723 Jul 26, 2024
141f954
Remove unused line
LinZhihao-723 Jul 26, 2024
f3525aa
Add value template file
LinZhihao-723 Jul 27, 2024
bbd3a56
Implement more helper templates...
LinZhihao-723 Jul 27, 2024
b1fa61c
Implement the super type
LinZhihao-723 Jul 28, 2024
e148af7
Merge from main
LinZhihao-723 Jul 30, 2024
f41e5d6
Add unit tests
LinZhihao-723 Jul 31, 2024
d29a17a
Fix cmake
LinZhihao-723 Jul 31, 2024
12679f7
... class
LinZhihao-723 Aug 3, 2024
534abf0
Implement kv pair class
LinZhihao-723 Aug 3, 2024
b7bed71
linting...
LinZhihao-723 Aug 3, 2024
88aa36a
Update KeyValuePair implementation
LinZhihao-723 Aug 7, 2024
92c3388
Merge
LinZhihao-723 Aug 8, 2024
698b406
Add missing comments
LinZhihao-723 Aug 8, 2024
bc565f5
Update doc string
LinZhihao-723 Aug 8, 2024
2393038
Update comments
LinZhihao-723 Aug 9, 2024
285aab3
Apply suggestions from code review
LinZhihao-723 Aug 14, 2024
7623d76
Apply code review comments and add unit tests
LinZhihao-723 Aug 14, 2024
ef0dd80
Reorder a little bit...
LinZhihao-723 Aug 14, 2024
54ec36a
Fix macos build
LinZhihao-723 Aug 14, 2024
941fcbc
Use const shared ptr and rely on type conversion
LinZhihao-723 Aug 14, 2024
6623b5a
Apply suggestions from code review
LinZhihao-723 Aug 15, 2024
3ae389a
Apply review comments to the src file
LinZhihao-723 Aug 15, 2024
a066c67
Apply comments on the unit tests
LinZhihao-723 Aug 15, 2024
353dccd
Apply suggestions from code review
LinZhihao-723 Aug 15, 2024
09dd01a
Apply code review comments
LinZhihao-723 Aug 15, 2024
83ae4e0
Remove default declaration since they are not implicitly declared alr…
LinZhihao-723 Aug 15, 2024
7cd1ea7
Rename
LinZhihao-723 Aug 15, 2024
6dd95a6
Fix the error
LinZhihao-723 Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ set(SOURCE_FILES_unitTest
src/clp/ffi/ir_stream/Serializer.hpp
src/clp/ffi/ir_stream/utils.cpp
src/clp/ffi/ir_stream/utils.hpp
src/clp/ffi/KeyValuePairLogEvent.cpp
src/clp/ffi/KeyValuePairLogEvent.hpp
src/clp/ffi/SchemaTree.cpp
src/clp/ffi/SchemaTree.hpp
src/clp/ffi/SchemaTreeNode.hpp
Expand Down
91 changes: 91 additions & 0 deletions components/core/src/clp/ffi/KeyValuePairLogEvent.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#include "KeyValuePairLogEvent.hpp"

#include <memory>
#include <string>
#include <system_error>
#include <unordered_map>
#include <unordered_set>
#include <utility>

#include <outcome/single-header/outcome.hpp>

#include "../ir/EncodedTextAst.hpp"
#include "../time_types.hpp"
#include "SchemaTree.hpp"
#include "SchemaTreeNode.hpp"
#include "Value.hpp"

using clp::ir::EightByteEncodedTextAst;
using clp::ir::FourByteEncodedTextAst;
using std::string;

namespace clp::ffi {
namespace {
/**
* @param type
* @param value
* @return Whether the given schema tree node type matches the given value's type.
*/
[[nodiscard]] auto is_valid_value_type(SchemaTreeNode::Type type, Value const& value) -> bool;
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved

auto is_valid_value_type(SchemaTreeNode::Type type, Value const& value) -> bool {
switch (type) {
case SchemaTreeNode::Type::Obj:
return value.is_null();
case SchemaTreeNode::Type::Int:
return value.is<value_int_t>();
case SchemaTreeNode::Type::Float:
return value.is<value_float_t>();
case SchemaTreeNode::Type::Bool:
return value.is<value_bool_t>();
case SchemaTreeNode::Type::UnstructuredArray:
return value.is<FourByteEncodedTextAst>() || value.is<EightByteEncodedTextAst>();
case SchemaTreeNode::Type::Str:
return value.is<string>() || value.is<FourByteEncodedTextAst>()
|| value.is<EightByteEncodedTextAst>();
default:
return false;
}
}
} // namespace

auto KeyValuePairLogEvent::create(
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
std::shared_ptr<SchemaTree> schema_tree,
KeyValuePairs kv_pairs,
UtcOffset utc_offset
) -> OUTCOME_V2_NAMESPACE::std_result<KeyValuePairLogEvent> {
std::unordered_map<SchemaTreeNode::id_t, std::unordered_set<string>> key_sets;
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
try {
for (auto const& [key_id, value] : kv_pairs) {
if (SchemaTree::cRootId == key_id) {
return std::errc::protocol_error;
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
}

auto const& node{schema_tree->get_node(key_id)};
auto const type{node.get_type()};
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
if (false == value.has_value()) {
// Empty value
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
if (SchemaTreeNode::Type::Obj != type) {
return std::errc::protocol_error;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda feel like this can be moved into the value type validation method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to leave it outside for two reasons:

  1. empty isn't a Value type
  2. This simplifies the helper function so it doesn't have to check has_value() and we don't need to symbols inside the helper (one for the std::optional<Value> and the over for the reference of the underlying Value)

} else if (false == is_valid_value_type(type, value.value())) {
return std::errc::protocol_error;
}

auto const parent_id{node.get_parent_id()};
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
auto const key_name{node.get_key_name()};
if (key_sets.contains(parent_id)) {
if (key_sets.at(parent_id).contains({key_name.begin(), key_name.end()})) {
// The key is duplicated
return std::errc::protocol_not_supported;
}
} else {
key_sets.emplace(parent_id, std::unordered_set{string{key_name}});
}
}
} catch (SchemaTree::OperationFailed const& ex) {
return std::errc::operation_not_permitted;
}
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
return KeyValuePairLogEvent{std::move(schema_tree), std::move(kv_pairs), utc_offset};
}
} // namespace clp::ffi
81 changes: 81 additions & 0 deletions components/core/src/clp/ffi/KeyValuePairLogEvent.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#ifndef CLP_FFI_KEYVALUEPAIRLOGEVENT_HPP
#define CLP_FFI_KEYVALUEPAIRLOGEVENT_HPP

#include <memory>
#include <optional>
#include <unordered_map>
#include <utility>

#include <outcome/single-header/outcome.hpp>

#include "../time_types.hpp"
#include "SchemaTree.hpp"
#include "SchemaTreeNode.hpp"
#include "Value.hpp"

namespace clp::ffi {
/**
* Class for key-value pair log events. Each key-value pair log event contains the following items:
* - A list of key-value pairs
* - A reference to the schema tree
* - The UTC offset of the current log event
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
*/
class KeyValuePairLogEvent {
public:
// Types
using KeyValuePairs = std::unordered_map<SchemaTreeNode::id_t, std::optional<Value>>;
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved

// Factory functions
/**
* Creates a key-value pair log event from valid given inputs.
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
* @param schema_tree
* @param kv_pairs
* @param utc_offset
* @return A result containing the key-value pair log event or an error code indicating the
* failure:
* - std::errc::operation_not_permitted if the key ID doesn't represent a valid node in the
* schema tree.
LinZhihao-723 marked this conversation as resolved.
Show resolved Hide resolved
* - std::errc::protocol_error if the schema tree node type doesn't match the value's type.
* - std::errc::protocol_not_supported if the same key appears more than once under a parent
* node.
*/
[[nodiscard]] static auto
create(std::shared_ptr<SchemaTree> schema_tree, KeyValuePairs kv_pairs, UtcOffset utc_offset
) -> OUTCOME_V2_NAMESPACE::std_result<KeyValuePairLogEvent>;

// Disable copy constructor and assignment operator
KeyValuePairLogEvent(KeyValuePairLogEvent const&) = delete;
auto operator=(KeyValuePairLogEvent const&) -> KeyValuePairLogEvent& = delete;

// Default move constructor and assignment operator
KeyValuePairLogEvent(KeyValuePairLogEvent&&) = default;
auto operator=(KeyValuePairLogEvent&&) -> KeyValuePairLogEvent& = default;

// Destructor
~KeyValuePairLogEvent() = default;
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved

// Methods
[[nodiscard]] auto get_schema_tree() const -> SchemaTree const& { return *m_schema_tree; }

[[nodiscard]] auto get_key_value_pairs() const -> KeyValuePairs const& { return m_kv_pairs; }

[[nodiscard]] auto get_utc_offset() const -> UtcOffset { return m_utc_offset; }

private:
// Constructor
KeyValuePairLogEvent(
haiqi96 marked this conversation as resolved.
Show resolved Hide resolved
std::shared_ptr<SchemaTree> schema_tree,
KeyValuePairs key_value_pairs,
UtcOffset utc_offset
)
: m_schema_tree{std::move(schema_tree)},
m_kv_pairs{std::move(key_value_pairs)},
m_utc_offset{utc_offset} {}

std::shared_ptr<SchemaTree> m_schema_tree;
Copy link
Contributor

@haiqi96 haiqi96 Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Techinically it is possible to modify the original schema tree using this pointer in the class, right?
Should we consider using std::shared_ptr<SchemaTree const> ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense

KeyValuePairs m_kv_pairs;
UtcOffset m_utc_offset{0};
};
} // namespace clp::ffi

#endif // CLP_FFI_KEYVALUEPAIRLOGEVENT_HPP
Loading