From c303f644f73ff9922293d6d9652d930844850616 Mon Sep 17 00:00:00 2001 From: Arthur Passos Date: Tue, 13 Dec 2022 12:35:40 -0300 Subject: [PATCH] formatting challenge --- .../src/KeyValuePairEscapingProcessor.h | 2 +- .../keyvaluepair/src/KeyValuePairExtractor.h | 2 +- .../src/KeyValuePairExtractorBuilder.h | 4 +- .../impl/LazyEscapingKeyValuePairExtractor.h | 27 ++-- .../SimpleKeyValuePairEscapingProcessor.cpp | 18 ++- .../src/impl/state/KeyStateHandler.h | 7 +- .../src/impl/state/StateHandler.cpp | 8 +- .../src/impl/state/StateHandler.h | 3 +- .../src/impl/state/ValueStateHandler.cpp | 117 ++++++++-------- .../src/impl/state/ValueStateHandler.h | 13 +- .../tests/gtest_key_value_pair_extractor.cpp | 132 +++++++----------- 11 files changed, 152 insertions(+), 181 deletions(-) diff --git a/src/Functions/keyvaluepair/src/KeyValuePairEscapingProcessor.h b/src/Functions/keyvaluepair/src/KeyValuePairEscapingProcessor.h index 439d666b8f6..b2a569cd30c 100644 --- a/src/Functions/keyvaluepair/src/KeyValuePairEscapingProcessor.h +++ b/src/Functions/keyvaluepair/src/KeyValuePairEscapingProcessor.h @@ -12,7 +12,7 @@ struct KeyValuePairEscapingProcessor KeyValuePairEscapingProcessor() = default; virtual ~KeyValuePairEscapingProcessor() = default; - virtual Response process(const ResponseViews&) const = 0; + virtual Response process(const ResponseViews &) const = 0; }; } diff --git a/src/Functions/keyvaluepair/src/KeyValuePairExtractor.h b/src/Functions/keyvaluepair/src/KeyValuePairExtractor.h index 8bd54012ddf..761dc66d031 100644 --- a/src/Functions/keyvaluepair/src/KeyValuePairExtractor.h +++ b/src/Functions/keyvaluepair/src/KeyValuePairExtractor.h @@ -1,7 +1,7 @@ #pragma once -#include #include +#include namespace DB { diff --git a/src/Functions/keyvaluepair/src/KeyValuePairExtractorBuilder.h b/src/Functions/keyvaluepair/src/KeyValuePairExtractorBuilder.h index c8543824c95..7b190003cf4 100644 --- a/src/Functions/keyvaluepair/src/KeyValuePairExtractorBuilder.h +++ b/src/Functions/keyvaluepair/src/KeyValuePairExtractorBuilder.h @@ -3,11 +3,11 @@ #include #include #include -#include "KeyValuePairExtractor.h" #include #include #include #include +#include "KeyValuePairExtractor.h" namespace DB { @@ -60,7 +60,7 @@ public: if (!escaping_processor) { - throw std::runtime_error {"Escaping processor must be set, cannot build KeyValuePairExtractor without one"}; + throw std::runtime_error{"Escaping processor must be set, cannot build KeyValuePairExtractor without one"}; } return std::make_shared>(key_state_handler, value_state_handler, escaping_processor); diff --git a/src/Functions/keyvaluepair/src/impl/LazyEscapingKeyValuePairExtractor.h b/src/Functions/keyvaluepair/src/impl/LazyEscapingKeyValuePairExtractor.h index f94c5a3e0d8..734af5eeb44 100644 --- a/src/Functions/keyvaluepair/src/impl/LazyEscapingKeyValuePairExtractor.h +++ b/src/Functions/keyvaluepair/src/impl/LazyEscapingKeyValuePairExtractor.h @@ -24,9 +24,13 @@ template class LazyEscapingKeyValuePairExtractor : public KeyValuePairExtractor { public: - LazyEscapingKeyValuePairExtractor(KeyStateHandler key_state_handler_, ValueStateHandler value_state_handler_, - std::shared_ptr> escaping_processor_) - : key_state_handler(key_state_handler_), value_state_handler(value_state_handler_), escaping_processor(escaping_processor_){} + LazyEscapingKeyValuePairExtractor( + KeyStateHandler key_state_handler_, + ValueStateHandler value_state_handler_, + std::shared_ptr> escaping_processor_) + : key_state_handler(key_state_handler_), value_state_handler(value_state_handler_), escaping_processor(escaping_processor_) + { + } [[nodiscard]] Response extract(const std::string & file) override { @@ -34,7 +38,8 @@ public: std::size_t pos = 0; - while (state != State::END) { + while (state != State::END) + { auto nextState = processState(file, pos, state); pos = nextState.pos; @@ -47,7 +52,8 @@ public: private: NextState processState(const std::string & file, std::size_t pos, State state) { - switch (state) { + switch (state) + { case State::WAITING_KEY: return key_state_handler.wait(file, pos); case State::READING_KEY: @@ -67,10 +73,7 @@ private: case State::FLUSH_PAIR: return flushPair(file, pos); case END: - return { - pos, - state - }; + return {pos, state}; } } @@ -78,10 +81,7 @@ private: { response_views[key_state_handler.get()] = value_state_handler.get(); - return { - pos, - pos == file.size() ? State::END : State::WAITING_KEY - }; + return {pos, pos == file.size() ? State::END : State::WAITING_KEY}; } KeyStateHandler key_state_handler; @@ -92,4 +92,3 @@ private: }; } - diff --git a/src/Functions/keyvaluepair/src/impl/SimpleKeyValuePairEscapingProcessor.cpp b/src/Functions/keyvaluepair/src/impl/SimpleKeyValuePairEscapingProcessor.cpp index 5cb22baaffa..2c06f4efafe 100644 --- a/src/Functions/keyvaluepair/src/impl/SimpleKeyValuePairEscapingProcessor.cpp +++ b/src/Functions/keyvaluepair/src/impl/SimpleKeyValuePairEscapingProcessor.cpp @@ -4,8 +4,9 @@ namespace DB { SimpleKeyValuePairEscapingProcessor::SimpleKeyValuePairEscapingProcessor(char escape_character_) -: KeyValuePairEscapingProcessor>(), escape_character(escape_character_) -{} + : KeyValuePairEscapingProcessor>(), escape_character(escape_character_) +{ +} SimpleKeyValuePairEscapingProcessor::Response SimpleKeyValuePairEscapingProcessor::process(const ResponseViews & response_views) const { @@ -13,7 +14,8 @@ SimpleKeyValuePairEscapingProcessor::Response SimpleKeyValuePairEscapingProcesso response.reserve(response_views.size()); - for (auto [key_view, value_view] : response_views) { + for (auto [key_view, value_view] : response_views) + { response[escape(key_view)] = escape(value_view); } @@ -27,10 +29,14 @@ std::string SimpleKeyValuePairEscapingProcessor::escape(std::string_view element element.reserve(element_view.size()); - for (char character : element_view) { - if (escape) { + for (char character : element_view) + { + if (escape) + { escape = false; - } else if (character == escape_character) { + } + else if (character == escape_character) + { escape = true; continue; } diff --git a/src/Functions/keyvaluepair/src/impl/state/KeyStateHandler.h b/src/Functions/keyvaluepair/src/impl/state/KeyStateHandler.h index dcc44d7cb9a..aef1b8bfccb 100644 --- a/src/Functions/keyvaluepair/src/impl/state/KeyStateHandler.h +++ b/src/Functions/keyvaluepair/src/impl/state/KeyStateHandler.h @@ -1,7 +1,7 @@ #pragma once -#include #include +#include #include "State.h" #include "StateHandler.h" @@ -9,13 +9,14 @@ namespace DB { -class KeyStateHandler : StateHandler { +class KeyStateHandler : StateHandler +{ public: KeyStateHandler(char key_value_delimiter, char escape_character, std::optional enclosing_character); [[nodiscard]] NextState wait(const std::string & file, size_t pos) const; [[nodiscard]] NextState read(const std::string & file, size_t pos); - [[nodiscard]] NextState readEnclosed(const std::string &file, size_t pos); + [[nodiscard]] NextState readEnclosed(const std::string & file, size_t pos); [[nodiscard]] NextState readKeyValueDelimiter(const std::string & file, size_t pos) const; [[nodiscard]] std::string_view get() const override; diff --git a/src/Functions/keyvaluepair/src/impl/state/StateHandler.cpp b/src/Functions/keyvaluepair/src/impl/state/StateHandler.cpp index 366c0d776be..07f8d4cd5fa 100644 --- a/src/Functions/keyvaluepair/src/impl/state/StateHandler.cpp +++ b/src/Functions/keyvaluepair/src/impl/state/StateHandler.cpp @@ -1,5 +1,5 @@ -#include #include "StateHandler.h" +#include namespace DB { @@ -7,11 +7,11 @@ namespace DB StateHandler::StateHandler(char escape_character_, std::optional enclosing_character_) : escape_character(escape_character_), enclosing_character(enclosing_character_) { - } -std::string_view StateHandler::createElement(const std::string & file, std::size_t begin, std::size_t end) const { - return std::string_view {file.begin() + begin, file.begin() + end}; +std::string_view StateHandler::createElement(const std::string & file, std::size_t begin, std::size_t end) const +{ + return std::string_view{file.begin() + begin, file.begin() + end}; } } diff --git a/src/Functions/keyvaluepair/src/impl/state/StateHandler.h b/src/Functions/keyvaluepair/src/impl/state/StateHandler.h index 31aa3c45714..bb8dc4632e0 100644 --- a/src/Functions/keyvaluepair/src/impl/state/StateHandler.h +++ b/src/Functions/keyvaluepair/src/impl/state/StateHandler.h @@ -6,7 +6,8 @@ namespace DB { -struct StateHandler { +struct StateHandler +{ StateHandler(char escape_character, std::optional enclosing_character); StateHandler(const StateHandler &) = default; diff --git a/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.cpp b/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.cpp index ee8ff26576e..3a56573e13c 100644 --- a/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.cpp +++ b/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.cpp @@ -3,109 +3,108 @@ namespace DB { -ValueStateHandler::ValueStateHandler(char escape_character_, char item_delimiter_, - std::optional enclosing_character_, - std::unordered_set special_character_allowlist_) - : StateHandler(escape_character_, enclosing_character_), item_delimiter(item_delimiter_), - special_character_allowlist(std::move(special_character_allowlist_)) -{} +ValueStateHandler::ValueStateHandler( + char escape_character_, + char item_delimiter_, + std::optional enclosing_character_, + std::unordered_set special_character_allowlist_) + : StateHandler(escape_character_, enclosing_character_) + , item_delimiter(item_delimiter_) + , special_character_allowlist(std::move(special_character_allowlist_)) +{ +} -NextState ValueStateHandler::wait(const std::string &file, size_t pos) const { - while (pos < file.size()) { +NextState ValueStateHandler::wait(const std::string & file, size_t pos) const +{ + while (pos < file.size()) + { const auto current_character = file[pos]; - if (current_character == enclosing_character) { - return { - pos + 1u, - State::READING_ENCLOSED_VALUE - }; - } else if (current_character == item_delimiter) { - return { - pos, - State::READING_EMPTY_VALUE - }; - } else if (isValidCharacter(current_character)) { - return { - pos, - State::READING_VALUE - }; - } else { + if (current_character == enclosing_character) + { + return {pos + 1u, State::READING_ENCLOSED_VALUE}; + } + else if (current_character == item_delimiter) + { + return {pos, State::READING_EMPTY_VALUE}; + } + else if (isValidCharacter(current_character)) + { + return {pos, State::READING_VALUE}; + } + else + { pos++; } } - return { - pos, - State::READING_EMPTY_VALUE - }; + return {pos, State::READING_EMPTY_VALUE}; } -NextState ValueStateHandler::read(const std::string &file, size_t pos) { +NextState ValueStateHandler::read(const std::string & file, size_t pos) +{ bool escape = false; auto start_index = pos; value = {}; - while (pos < file.size()) { + while (pos < file.size()) + { const auto current_character = file[pos++]; - if (escape) { + if (escape) + { escape = false; - } else if (escape_character == current_character) { + } + else if (escape_character == current_character) + { escape = true; - } else if (current_character == item_delimiter || !isValidCharacter(current_character)) { + } + else if (current_character == item_delimiter || !isValidCharacter(current_character)) + { value = createElement(file, start_index, pos - 1); - return { - pos, - State::FLUSH_PAIR - }; + return {pos, State::FLUSH_PAIR}; } } // TODO: do I really need the below logic? // this allows empty values at the end value = createElement(file, start_index, pos); - return { - pos, - State::FLUSH_PAIR - }; + return {pos, State::FLUSH_PAIR}; } -NextState ValueStateHandler::readEnclosed(const std::string &file, size_t pos) { +NextState ValueStateHandler::readEnclosed(const std::string & file, size_t pos) +{ auto start_index = pos; - while (pos < file.size()) { + while (pos < file.size()) + { const auto current_character = file[pos++]; - if (enclosing_character == current_character) { + if (enclosing_character == current_character) + { // not checking for empty value because with current waitValue implementation // there is no way this piece of code will be reached for the very first value character value = createElement(file, start_index, pos - 1); - return { - pos, - State::FLUSH_PAIR - }; + return {pos, State::FLUSH_PAIR}; } } - return { - pos, - State::END - }; + return {pos, State::END}; } -NextState ValueStateHandler::readEmpty(const std::string &, size_t pos) { +NextState ValueStateHandler::readEmpty(const std::string &, size_t pos) +{ value = {}; - return { - pos + 1, - State::FLUSH_PAIR - }; + return {pos + 1, State::FLUSH_PAIR}; } -bool ValueStateHandler::isValidCharacter(char character) const { +bool ValueStateHandler::isValidCharacter(char character) const +{ return special_character_allowlist.contains(character) || std::isalnum(character) || character == '_'; } -std::string_view ValueStateHandler::get() const { +std::string_view ValueStateHandler::get() const +{ return value; } diff --git a/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.h b/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.h index c038aa03bd0..71184f70a7d 100644 --- a/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.h +++ b/src/Functions/keyvaluepair/src/impl/state/ValueStateHandler.h @@ -1,7 +1,7 @@ #pragma once -#include #include +#include #include #include "State.h" #include "StateHandler.h" @@ -9,11 +9,14 @@ namespace DB { -class ValueStateHandler : StateHandler { - +class ValueStateHandler : StateHandler +{ public: - ValueStateHandler(char escape_character, char item_delimiter, - std::optional enclosing_character, std::unordered_set special_character_allowlist_); + ValueStateHandler( + char escape_character, + char item_delimiter, + std::optional enclosing_character, + std::unordered_set special_character_allowlist_); [[nodiscard]] NextState wait(const std::string & file, size_t pos) const; [[nodiscard]] NextState read(const std::string & file, size_t pos); diff --git a/src/Functions/keyvaluepair/tests/gtest_key_value_pair_extractor.cpp b/src/Functions/keyvaluepair/tests/gtest_key_value_pair_extractor.cpp index 52f93e59097..fc2b03c75ef 100644 --- a/src/Functions/keyvaluepair/tests/gtest_key_value_pair_extractor.cpp +++ b/src/Functions/keyvaluepair/tests/gtest_key_value_pair_extractor.cpp @@ -4,7 +4,8 @@ namespace DB { -struct LazyKeyValuePairExtractorTestCase { +struct LazyKeyValuePairExtractorTestCase +{ std::string input; std::unordered_map expected_output; std::shared_ptr> extractor; @@ -15,11 +16,12 @@ std::ostream & operator<<(std::ostream & ostr, const LazyKeyValuePairExtractorTe return ostr << test_case.input; } -struct KeyValuePairExtractorTest : public ::testing::TestWithParam { - +struct KeyValuePairExtractorTest : public ::testing::TestWithParam +{ }; -TEST_P(KeyValuePairExtractorTest, KeyValuePairExtractorTests) { +TEST_P(KeyValuePairExtractorTest, KeyValuePairExtractorTests) +{ const auto & [input, expected_output, extractor] = GetParam(); auto result = extractor->extract(input); @@ -30,98 +32,58 @@ TEST_P(KeyValuePairExtractorTest, KeyValuePairExtractorTests) { INSTANTIATE_TEST_SUITE_P( ValuesCanBeEmptyString, KeyValuePairExtractorTest, - ::testing::ValuesIn(std::initializer_list { - { - "age:", - { - {"age", ""} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().build() - }, - { - "name: neymar, favorite_movie:,favorite_song:", - { - {"name", "neymar"}, - {"favorite_movie", ""}, - {"favorite_song", ""}, - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().build() - } - }) -); + ::testing::ValuesIn(std::initializer_list{ + {"age:", {{"age", ""}}, KeyValuePairExtractorBuilder().withEscapingProcessor().build()}, + {"name: neymar, favorite_movie:,favorite_song:", + { + {"name", "neymar"}, + {"favorite_movie", ""}, + {"favorite_song", ""}, + }, + KeyValuePairExtractorBuilder().withEscapingProcessor().build()}})); INSTANTIATE_TEST_SUITE_P( MixString, KeyValuePairExtractorTest, - ::testing::ValuesIn(std::initializer_list { - { - R"(9 ads =nm, no\:me: neymar, age: 30, daojmskdpoa and a height: 1.75, school: lupe\ picasso, team: psg,)", - { - {R"(no:me)", "neymar"}, - {"age", "30"}, - {"height", "1.75"}, - {"school", "lupe picasso"}, - {"team", "psg"} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().withValueSpecialCharacterAllowList({'.'}).build() - }, - { - "XNFHGSSF_RHRUZHVBS_KWBT: F,", - { - {"XNFHGSSF_RHRUZHVBS_KWBT", "F"} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().build() - }, - } - ) -); + ::testing::ValuesIn(std::initializer_list{ + {R"(9 ads =nm, no\:me: neymar, age: 30, daojmskdpoa and a height: 1.75, school: lupe\ picasso, team: psg,)", + {{R"(no:me)", "neymar"}, {"age", "30"}, {"height", "1.75"}, {"school", "lupe picasso"}, {"team", "psg"}}, + KeyValuePairExtractorBuilder() + .withEscapingProcessor() + .withValueSpecialCharacterAllowList({'.'}) + .build()}, + {"XNFHGSSF_RHRUZHVBS_KWBT: F,", + {{"XNFHGSSF_RHRUZHVBS_KWBT", "F"}}, + KeyValuePairExtractorBuilder().withEscapingProcessor().build()}, + })); INSTANTIATE_TEST_SUITE_P( Escaping, KeyValuePairExtractorTest, - ::testing::ValuesIn(std::initializer_list { - { - "na,me,: neymar, age:30", - { - {"age", "30"} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().build() - }, - { - "na$me,: neymar, age:30", - { - {"age", "30"} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().build() - }, - { - R"(name: neymar, favorite_quote: Premature\ optimization\ is\ the\ r\$\$t\ of\ all\ evil, age:30)", - { - {"name", "neymar"}, - {"favorite_quote", R"(Premature optimization is the r$$t of all evil)"}, - {"age", "30"} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().withEnclosingCharacter('"').build() - } - }) -); + ::testing::ValuesIn(std::initializer_list{ + {"na,me,: neymar, age:30", + {{"age", "30"}}, + KeyValuePairExtractorBuilder().withEscapingProcessor().build()}, + {"na$me,: neymar, age:30", + {{"age", "30"}}, + KeyValuePairExtractorBuilder().withEscapingProcessor().build()}, + {R"(name: neymar, favorite_quote: Premature\ optimization\ is\ the\ r\$\$t\ of\ all\ evil, age:30)", + {{"name", "neymar"}, {"favorite_quote", R"(Premature optimization is the r$$t of all evil)"}, {"age", "30"}}, + KeyValuePairExtractorBuilder() + .withEscapingProcessor() + .withEnclosingCharacter('"') + .build()}})); INSTANTIATE_TEST_SUITE_P( EnclosedElements, KeyValuePairExtractorTest, - ::testing::ValuesIn(std::initializer_list { - { - R"("name": "Neymar", "age": 30, team: "psg", "favorite_movie": "", height: 1.75)", - { - {"name", "Neymar"}, - {"age", "30"}, - {"team", "psg"}, - {"favorite_movie", ""}, - {"height", "1.75"} - }, - KeyValuePairExtractorBuilder().withEscapingProcessor().withValueSpecialCharacterAllowList({'.'}).withEnclosingCharacter('"').build() - } - }) -); + ::testing::ValuesIn(std::initializer_list{ + {R"("name": "Neymar", "age": 30, team: "psg", "favorite_movie": "", height: 1.75)", + {{"name", "Neymar"}, {"age", "30"}, {"team", "psg"}, {"favorite_movie", ""}, {"height", "1.75"}}, + KeyValuePairExtractorBuilder() + .withEscapingProcessor() + .withValueSpecialCharacterAllowList({'.'}) + .withEnclosingCharacter('"') + .build()}})); }