From 0d6d81412ccdd5a4b12be7413a255da4337f426a Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:22:18 -0300 Subject: [PATCH 1/2] #972: Refactored exception handling for Script Options parser --- .../base/javacontainer/javacontainer_impl.cc | 19 ++-- .../javacontainer/script_options/extractor.cc | 14 ++- .../javacontainer/script_options/extractor.h | 4 +- .../javacontainer/script_options/parser.h | 12 +-- .../script_options/parser_legacy.cc | 99 ++++++++++--------- .../script_options/parser_legacy.h | 18 ++-- .../ctpg/script_option_lines_ctpg.cc | 8 +- .../ctpg/script_option_lines_ctpg.h | 4 +- .../ctpg/test/script_option_lines_test.cpp | 36 +++---- .../legacy/script_option_lines.cc | 10 +- .../legacy/script_option_lines.h | 4 +- .../legacy/test/script_option_lines_test.cpp | 38 +++---- 12 files changed, 120 insertions(+), 146 deletions(-) diff --git a/exaudfclient/base/javacontainer/javacontainer_impl.cc b/exaudfclient/base/javacontainer/javacontainer_impl.cc index 31d37d30..ec857723 100644 --- a/exaudfclient/base/javacontainer/javacontainer_impl.cc +++ b/exaudfclient/base/javacontainer/javacontainer_impl.cc @@ -35,18 +35,21 @@ JavaVMImpl::JavaVMImpl(bool checkOnly, bool noJNI, SwigFactory& swigFactory) m_exaJavaPath = "/exaudf/base/javacontainer"; // TODO hardcoded path JavaScriptOptions::ScriptOptionLinesParserLegacy scriptOptionsParser; - JavaScriptOptions::Extractor extractor(scriptOptionsParser, swigFactory, - [&](const std::string &msg){throwException(msg);}); + try { + JavaScriptOptions::Extractor extractor(scriptOptionsParser, swigFactory); - DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode)); // To be called before scripts are imported. Otherwise, the script classname from an imported script could be used + DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode)); // To be called before scripts are imported. Otherwise, the script classname from an imported script could be used - DBG_FUNC_CALL(cerr,setClasspath()); + DBG_FUNC_CALL(cerr,setClasspath()); - m_jvmOptions = std::move(extractor.moveJvmOptions()); + m_jvmOptions = std::move(extractor.moveJvmOptions()); - for (set::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end(); - ++it) { - addJarToClasspath(*it); + for (set::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end(); + ++it) { + addJarToClasspath(*it); + } + } catch(const std::runtime_error& ex) { + throwException(ex); } m_needsCompilation = checkNeedsCompilation(); diff --git a/exaudfclient/base/javacontainer/script_options/extractor.cc b/exaudfclient/base/javacontainer/script_options/extractor.cc index f14e1ef4..5f6bb0cf 100644 --- a/exaudfclient/base/javacontainer/script_options/extractor.cc +++ b/exaudfclient/base/javacontainer/script_options/extractor.cc @@ -11,24 +11,22 @@ namespace SWIGVMContainers { namespace JavaScriptOptions { Extractor::Extractor(ScriptOptionsParser & parser, - SwigFactory& swigFactory, - std::function throwException) -: m_throwException(throwException) -, m_parser(parser) + SwigFactory& swigFactory) +: m_parser(parser) , m_swigFactory(swigFactory) {} void Extractor::extract(std::string & scriptCode) { m_parser.prepareScriptCode(scriptCode); EXTR_DBG_FUNC_CALL(m_parser.parseForScriptClass( [&](const std::string& value){ EXTR_DBG_FUNC_CALL(m_converter.convertScriptClassName(value)); - }, m_throwException)); - EXTR_DBG_FUNC_CALL(m_parser.extractImportScripts(m_swigFactory, m_throwException)); + })); + EXTR_DBG_FUNC_CALL(m_parser.extractImportScripts(m_swigFactory)); EXTR_DBG_FUNC_CALL(m_parser.parseForJvmOptions( [&](const std::string& value){ EXTR_DBG_FUNC_CALL(m_converter.convertJvmOption(value)); - }, m_throwException)); + })); EXTR_DBG_FUNC_CALL(m_parser.parseForExternalJars( [&](const std::string& value){ EXTR_DBG_FUNC_CALL(m_converter.convertExternalJar(value)); - }, m_throwException)); + })); scriptCode = std::move(m_parser.getScriptCode()); } diff --git a/exaudfclient/base/javacontainer/script_options/extractor.h b/exaudfclient/base/javacontainer/script_options/extractor.h index 7b21bcda..ab4055ca 100644 --- a/exaudfclient/base/javacontainer/script_options/extractor.h +++ b/exaudfclient/base/javacontainer/script_options/extractor.h @@ -21,8 +21,7 @@ class Extractor { public: Extractor(ScriptOptionsParser & parser, - SwigFactory& swigFactory, - std::function throwException); + SwigFactory& swigFactory); const std::set & getJarPaths() const { return m_converter.getJarPaths(); @@ -35,7 +34,6 @@ class Extractor { void extract(std::string & scriptCode); private: - std::function m_throwException; Converter m_converter; ScriptOptionsParser & m_parser; diff --git a/exaudfclient/base/javacontainer/script_options/parser.h b/exaudfclient/base/javacontainer/script_options/parser.h index 974e80a0..68dd634f 100644 --- a/exaudfclient/base/javacontainer/script_options/parser.h +++ b/exaudfclient/base/javacontainer/script_options/parser.h @@ -23,30 +23,26 @@ struct ScriptOptionsParser { If the option is found, the function removes the option from "scriptCode" and calls "callback" with the option value and position within "scriptCode". */ - virtual void parseForScriptClass(std::function callback, - std::function throwException) = 0; + virtual void parseForScriptClass(std::function callback) = 0; /* Searches for JVM options. If an option is found, the function removes the option from "scriptCode" and calls "callback" with the option value and position within "scriptCode". */ - virtual void parseForJvmOptions(std::function callback, - std::function throwException) = 0; + virtual void parseForJvmOptions(std::function callback) = 0; /* Searches for External Jar. If an option is found, the function removes the option from "scriptCode" and calls "callback" with the option value and position within "scriptCode". */ - virtual void parseForExternalJars(std::function callback, - std::function throwException) = 0; + virtual void parseForExternalJars(std::function callback) = 0; /* Searches for the "%import" options and embeds the respective imported script code at the same location as the option in the script code. */ - virtual void extractImportScripts(SwigFactory & swigFactory, - std::function throwException) = 0; + virtual void extractImportScripts(SwigFactory & swigFactory) = 0; /* Returns the (eventually modified) script code. diff --git a/exaudfclient/base/javacontainer/script_options/parser_legacy.cc b/exaudfclient/base/javacontainer/script_options/parser_legacy.cc index 212fa455..c32dfec7 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_legacy.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_legacy.cc @@ -5,6 +5,9 @@ #include "base/swig_factory/swig_factory.h" #include +#include + + namespace SWIGVMContainers { @@ -20,8 +23,7 @@ void ScriptOptionLinesParserLegacy::prepareScriptCode(const std::string & script m_scriptCode = scriptCode; } -void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFactory, - std::function throwException) { +void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFactory) { std::unique_ptr metaData; // Attention: We must hash the parent script before modifying it (adding the // package definition). Otherwise we don't recognize if the script imports its self @@ -101,19 +103,22 @@ void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFacto while (true) { std::string newScript; size_t scriptPos; - parseForSingleOption(m_keywords.importKeyword(), - [&](const std::string& value, size_t pos){scriptPos = pos; newScript = value;}, - [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1614" + msg);}); + try { + parseForSingleOption(m_keywords.importKeyword(), + [&](const std::string& value, size_t pos){scriptPos = pos; newScript = value;}); + } catch (const std::runtime_error & ex) { + throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1614 ") + ex.what()); + } if (!newScript.empty()) { if (!metaData) { metaData.reset(swigFactory.makeSwigMetadata()); if (!metaData) - throwException("F-UDF-CL-SL-JAVA-1615: Failure while importing scripts"); + throw std::runtime_error("F-UDF-CL-SL-JAVA-1615: Failure while importing scripts"); } const char *importScriptCode = metaData->moduleContent(newScript.c_str()); const char *exception = metaData->checkException(); if (exception) - throwException("F-UDF-CL-SL-JAVA-1616: " + std::string(exception)); + throw std::runtime_error("F-UDF-CL-SL-JAVA-1616: " + std::string(exception)); if (importedScriptChecksums.addScript(importScriptCode)) { // Script has not been imported yet // If this imported script contains %import statements @@ -126,25 +131,31 @@ void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFacto } } -void ScriptOptionLinesParserLegacy::parseForScriptClass(std::function callback, - std::function throwException) { +void ScriptOptionLinesParserLegacy::parseForScriptClass(std::function callback) { + try { parseForSingleOption(m_keywords.scriptClassKeyword(), - [&](const std::string& value, size_t pos){callback(value);}, - [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1610" + msg);}); + [&](const std::string& value, size_t pos){callback(value);}); + } catch (const std::runtime_error& ex) { + throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1610") + ex.what()); + } } -void ScriptOptionLinesParserLegacy::parseForJvmOptions(std::function callback, - std::function throwException) { - parseForMultipleOptions(m_keywords.jvmOptionKeyword(), - [&](const std::string& value, size_t pos){callback(value);}, - [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1612" + msg);}); +void ScriptOptionLinesParserLegacy::parseForJvmOptions(std::function callback) { + try { + parseForMultipleOptions(m_keywords.jvmOptionKeyword(), + [&](const std::string& value, size_t pos){callback(value);}); + } catch(const std::runtime_error& ex) { + throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1612") + ex.what()); + } } -void ScriptOptionLinesParserLegacy::parseForExternalJars(std::function callback, - std::function throwException) { - parseForMultipleOptions(m_keywords.jarKeyword(), - [&](const std::string& value, size_t pos){callback(value);}, - [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1613" + msg);}); +void ScriptOptionLinesParserLegacy::parseForExternalJars(std::function callback) { + try { + parseForMultipleOptions(m_keywords.jarKeyword(), + [&](const std::string& value, size_t pos){callback(value);}); + } catch(const std::runtime_error& ex) { + throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1613") + ex.what()); + } } std::string && ScriptOptionLinesParserLegacy::getScriptCode() { @@ -152,40 +163,32 @@ std::string && ScriptOptionLinesParserLegacy::getScriptCode() { } void ScriptOptionLinesParserLegacy::parseForSingleOption(const std::string keyword, - std::function callback, - std::function throwException) { + std::function callback) { size_t pos; - const std::string option = - ExecutionGraph::extractOptionLine( - m_scriptCode, - keyword, - m_whitespace, - m_lineend, - pos, - [&](const char* msg){throwException(std::string("F-UDF-CL-SL-JAVA-1606: ") + msg);} - ); - if (option != "") { - callback(option, pos); + try { + const std::string option = + ExecutionGraph::extractOptionLine(m_scriptCode, keyword, m_whitespace, m_lineend, pos); + if (option != "") { + callback(option, pos); + } + } catch(const std::runtime_error& ex) { + throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1606") + ex.what()); } } void ScriptOptionLinesParserLegacy::parseForMultipleOptions(const std::string keyword, - std::function callback, - std::function throwException) { + std::function callback) { size_t pos; while (true) { - const std::string option = - ExecutionGraph::extractOptionLine( - m_scriptCode, - keyword, - m_whitespace, - m_lineend, - pos, - [&](const char* msg){throwException(std::string("F-UDF-CL-SL-JAVA-1607: ") + msg);} - ); - if (option == "") - break; - callback(option, pos); + try { + const std::string option = + ExecutionGraph::extractOptionLine(m_scriptCode, keyword, m_whitespace, m_lineend, pos); + if (option == "") + break; + callback(option, pos); + } catch(const std::runtime_error& ex) { + throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1607") + ex.what()); + } } } diff --git a/exaudfclient/base/javacontainer/script_options/parser_legacy.h b/exaudfclient/base/javacontainer/script_options/parser_legacy.h index 8928d21a..655f6e0f 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_legacy.h +++ b/exaudfclient/base/javacontainer/script_options/parser_legacy.h @@ -17,27 +17,21 @@ class ScriptOptionLinesParserLegacy : public ScriptOptionsParser { void prepareScriptCode(const std::string & scriptCode) override; - void parseForScriptClass(std::function callback, - std::function throwException) override; + void parseForScriptClass(std::function callback) override; - void parseForJvmOptions(std::function callback, - std::function throwException) override; + void parseForJvmOptions(std::function callback) override; - void parseForExternalJars(std::function callback, - std::function throwException) override; + void parseForExternalJars(std::function callback) override; - void extractImportScripts(SwigFactory & swigFactory, - std::function throwException) override; + void extractImportScripts(SwigFactory & swigFactory) override; std::string && getScriptCode() override; private: void parseForSingleOption(const std::string key, - std::function callback, - std::function throwException); + std::function callback); void parseForMultipleOptions(const std::string key, - std::function callback, - std::function throwException); + std::function callback); private: const std::string m_whitespace; diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc index f5584a33..a338be27 100644 --- a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc +++ b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc @@ -133,7 +133,7 @@ constexpr parser option_parser( ) ); -void parse(std::string&& code, options_type& result, std::function throwException) { +void parse(std::string&& code, options_type& result) { std::stringstream error_buffer; auto res = option_parser.parse( parse_options{}.set_skip_whitespace(false), @@ -147,13 +147,13 @@ void parse(std::string&& code, options_type& result, std::function throwException) { +void parseOptions(const std::string& code, options_map_t & result) { size_t current_pos = 0; @@ -163,7 +163,7 @@ void parseOptions(const std::string& code, options_map_t & result, std::function std::string line = code.substr(current_pos, new_pos); if (!line.empty() && !std::all_of(line.begin(),line.end(), [](const char c) {return std::isspace(c);})) { options_type parser_result; - ParserInternals::parse(std::move(line), parser_result, throwException); + ParserInternals::parse(std::move(line), parser_result); for (const auto & option: parser_result) { ScriptOption entry = { diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h index 92d164da..23699a6b 100644 --- a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h +++ b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h @@ -45,9 +45,9 @@ using options_map_t = std::map; * \param code Reference to string where the script code is stored. * \param result Result of all found options. * \param throwException Function to be called to throw exception. - * + * \throws std::runtime_error if parsing fails */ -void parseOptions(const std::string& code, options_map_t & result, std::function throwException); +void parseOptions(const std::string& code, options_map_t & result); } //namespace CTPG diff --git a/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp b/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp index 892cfb29..630778e5 100644 --- a/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp +++ b/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp @@ -5,16 +5,6 @@ #include - -class TestException : public std::runtime_error { - using std::runtime_error::runtime_error; -}; - -void throwException(const char* ex) { - throw TestException(std::string(ex)); -} - - using namespace ExecutionGraph::OptionsLineParser::CTPG; using ::testing::MatchesRegex; @@ -37,7 +27,7 @@ TEST_P(ScriptOptionLinesWhitespaceTest, WhitespaceExtractOptionLineTest) { const std::string payload = std::get<6>(GetParam()); const std::string code = prefix + '%' + option + delimeter + value + ';' + suffix + new_line + payload; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); ASSERT_EQ(result.size(), 1); const auto option_result = result.find(option); ASSERT_NE(option_result, result.end()); @@ -71,7 +61,7 @@ TEST(ScriptOptionLinesTest, ignore_anything_other_than_whitepsace) { "abc %option myoption;\n" "\nmycode"; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); EXPECT_TRUE(result.empty()); } @@ -83,15 +73,15 @@ TEST(ScriptOptionLinesTest, need_option_termination_character) { EXPECT_THROW({ try { - parseOptions(code, result, throwException); + parseOptions(code, result); } - catch( const TestException& e ) + catch( const std::runtime_error& e ) { // and this tests that it has the correct message EXPECT_STREQ( e.what(), "Error parsing script options: [1:17] PARSE: Syntax error: Unexpected ''\n"); throw; } - }, TestException ); + }, std::runtime_error ); } TEST(ScriptOptionLinesTest, finds_the_two_options_same_key) { @@ -99,7 +89,7 @@ TEST(ScriptOptionLinesTest, finds_the_two_options_same_key) { "%some_option myoption; %some_option mysecondoption;\n" "\nmycode"; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); ASSERT_EQ(result.size(), 1); const auto option_result = result.find("some_option"); @@ -114,7 +104,7 @@ TEST(ScriptOptionLinesTest, finds_the_two_options_different_keys) { "%some_option myoption; %otheroption mysecondoption;\n" "\nmycode"; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); ASSERT_EQ(result.size(), 2); const auto option_result = result.find("some_option"); @@ -139,14 +129,14 @@ TEST_P(ScriptOptionLinesInvalidOptionTest, value_is_mandatory) { EXPECT_THROW({ try { - parseOptions(code, result, throwException); + parseOptions(code, result); } - catch( const TestException& e ) + catch( const std::runtime_error& e ) { EXPECT_THAT( e.what(), MatchesRegex("^Error parsing script options.*PARSE: Syntax error: Unexpected.*$")); throw; } - }, TestException ); + }, std::runtime_error ); } const std::vector invalid_options = {"%some_option ;", "%some_option \n", "\n%some_option\n;", "%some_option\nvalue;"}; @@ -164,7 +154,7 @@ TEST(ScriptOptionLinesTest, test_when_two_options_plus_code_in_same_line_then_op */ const std::string code = "%jar /buckets/bucketfs1/jars/exajdbc.jar; %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;"; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); ASSERT_EQ(result.size(), 2); const auto jar_option_result = result.find("jar"); @@ -192,7 +182,7 @@ TEST(ScriptOptionLinesTest, test_values_can_contain_spaces) { " }\n" "}\n"; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); ASSERT_EQ(result.size(), 1); const auto jvm_option_result = result.find("jvmoption"); @@ -210,7 +200,7 @@ TEST(ScriptOptionLinesTest, test_multiple_lines_with_code) { "%jar /buckets/bucketfs1/jars/exajdbc.jar; class DEF{};\n"; options_map_t result; - parseOptions(code, result, throwException); + parseOptions(code, result); ASSERT_EQ(result.size(), 2); const auto jvm_option_result = result.find("jvmoption"); diff --git a/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc b/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc index 4b3752e9..180f896f 100644 --- a/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc +++ b/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc @@ -7,7 +7,7 @@ namespace ExecutionGraph { -std::string extractOptionLine(std::string& code, const std::string option, const std::string whitespace, const std::string lineEnd, size_t& pos, std::functionthrowException) +std::string extractOptionLine(std::string& code, const std::string option, const std::string whitespace, const std::string lineEnd, size_t& pos) { std::string result; size_t startPos = code.find(option); @@ -62,27 +62,27 @@ std::string extractOptionLine(std::string& code, const std::string option, const if (firstPos == std::string::npos) { std::stringstream ss; ss << "No values found for " << option << " statement"; - throwException(ss.str().c_str()); + throw std::runtime_error(ss.str().c_str()); } // Find the end of line. size_t lastPos = code.find_first_of(lineEnd + "\r\n", firstPos); if (lastPos == std::string::npos || code.compare(lastPos, lineEnd.length(), lineEnd) != 0) { std::stringstream ss; ss << "End of " << option << " statement not found"; - throwException(ss.str().c_str()); + throw std::runtime_error(ss.str().c_str()); } // If no values were found if (firstPos >= lastPos) { std::stringstream ss; ss << "No values found for " << option << " statement"; - throwException(ss.str().c_str()); + throw std::runtime_error(ss.str().c_str()); } // If no values were found size_t optionsEnd = code.find_last_not_of(whitespace, lastPos - 1); if (optionsEnd == std::string::npos || optionsEnd < firstPos) { std::stringstream ss; ss << "No values found for " << option << " statement"; - throwException(ss.str().c_str()); + throw std::runtime_error(ss.str().c_str()); } result = code.substr(firstPos, optionsEnd - firstPos + 1); code.erase(startPos, lastPos - startPos + 1); diff --git a/exaudfclient/base/script_options_parser/legacy/script_option_lines.h b/exaudfclient/base/script_options_parser/legacy/script_option_lines.h index e84fbdc5..b16efe54 100644 --- a/exaudfclient/base/script_options_parser/legacy/script_option_lines.h +++ b/exaudfclient/base/script_options_parser/legacy/script_option_lines.h @@ -15,11 +15,11 @@ namespace ExecutionGraph * \param whitespace String of characters that should be treated as white space characters. * \param lineEnd String of characters that should be treated as line end characters. * \param pos If option is found, contains the start position of the option. Otherwise, contains std::string::npos. - * \param throwException Function to be called to throw exception. + * \throws std::runtime_error if parser fails. * * \return String with the option line. */ -std::string extractOptionLine(std::string& code, const std::string option, const std::string whitespace, const std::string lineEnd, size_t& pos, std::function throwException); +std::string extractOptionLine(std::string& code, const std::string option, const std::string whitespace, const std::string lineEnd, size_t& pos); } // namespace ExecutionGraph diff --git a/exaudfclient/base/script_options_parser/legacy/test/script_option_lines_test.cpp b/exaudfclient/base/script_options_parser/legacy/test/script_option_lines_test.cpp index b6fdc6c3..99889da2 100644 --- a/exaudfclient/base/script_options_parser/legacy/test/script_option_lines_test.cpp +++ b/exaudfclient/base/script_options_parser/legacy/test/script_option_lines_test.cpp @@ -6,14 +6,6 @@ const std::string whitespace = " \t\f\v"; const std::string lineEnd = ";"; -class TestException : public std::runtime_error { - using std::runtime_error::runtime_error; -}; - -void throwException(const char* ex) { - throw TestException(std::string(ex)); -} - using namespace ExecutionGraph; @@ -27,7 +19,7 @@ TEST_P(ScriptOptionLinesWhitespaceTest, WhitespaceExtractOptionLineTest) { const std::string value = std::get<3>(GetParam()); const std::string payload = std::get<4>(GetParam()); std::string code = prefix + option + value + lineEnd + suffix + "\n" + payload; - const std::string res = extractOptionLine(code, option, whitespace, lineEnd, pos, throwException); + const std::string res = extractOptionLine(code, option, whitespace, lineEnd, pos); EXPECT_EQ(res, value); EXPECT_EQ(code, prefix + suffix + "\n" + payload); } @@ -53,7 +45,7 @@ TEST(ScriptOptionLinesTest, ignore_anything_other_than_whitepsace) { std::string code = "abc %option myoption;\n" "\nmycode"; - const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException); + const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos); EXPECT_TRUE(res.empty()); } @@ -63,8 +55,8 @@ TEST(ScriptOptionLinesTest, need_line_end_character) { "%option myoption\n" "\nmycode"; EXPECT_THROW({ - const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException); - }, TestException ); + const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos); + }, std::runtime_error ); } TEST(ScriptOptionLinesTest, only_finds_the_first_option_same_key) { @@ -72,7 +64,7 @@ TEST(ScriptOptionLinesTest, only_finds_the_first_option_same_key) { std::string code = "%option myoption; %option mysecondoption;\n" "\nmycode"; - const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException); + const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos); const std::string expected_resulting_code = " %option mysecondoption;\n" "\nmycode"; @@ -86,7 +78,7 @@ TEST(ScriptOptionLinesTest, only_finds_the_first_option_different_key) { std::string code = "%option myoption; %otheroption mysecondoption;\n" "\nmycode"; - const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException); + const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos); const std::string expected_resulting_code = " %otheroption mysecondoption;\n" "\nmycode"; @@ -103,8 +95,8 @@ TEST_P(ScriptOptionLinesInvalidOptionTest, value_is_mandatory) { const std::string invalid_option = GetParam(); std::string code = invalid_option + "\nsomething"; EXPECT_THROW({ - const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException); - }, TestException ); + const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos); + }, std::runtime_error ); } std::vector invalid_options = {"%option ;", "%option \n", "\n%option\n;", "%option\nvalue;"}; @@ -121,7 +113,7 @@ TEST(ScriptOptionLinesTest, ignores_any_other_option) { "%option myoption; %option mysecondoption;\n" "\nmycode"; std::string code = original_code; - const std::string res = extractOptionLine(code, "%mythirdoption", whitespace, lineEnd, pos, throwException); + const std::string res = extractOptionLine(code, "%mythirdoption", whitespace, lineEnd, pos); EXPECT_TRUE(res.empty()); EXPECT_EQ(code, original_code); } @@ -136,7 +128,7 @@ TEST(ScriptOptionLinesTest, test_all_in_one_line_does_second_option_does_not_wor size_t pos; const std::string original_code = "%jar /buckets/bucketfs1/jars/exajdbc.jar; %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;"; std::string code = original_code; - const std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException); + const std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos); EXPECT_TRUE(res.empty()); EXPECT_EQ(code, original_code); } @@ -150,10 +142,10 @@ TEST(ScriptOptionLinesTest, test_all_in_one_line_does_first_option_does_work) { size_t pos; const std::string original_code = "%jar /buckets/bucketfs1/jars/exajdbc.jar; %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;"; std::string code = original_code; - std::string res = extractOptionLine(code, "%jar", whitespace, lineEnd, pos, throwException); + std::string res = extractOptionLine(code, "%jar", whitespace, lineEnd, pos); EXPECT_EQ(res, "/buckets/bucketfs1/jars/exajdbc.jar"); EXPECT_EQ(code, " %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;"); - res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException); + res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos); EXPECT_EQ(code, " class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;"); } @@ -171,7 +163,7 @@ TEST(ScriptOptionLinesTest, test_values_must_not_contain_spaces) { " }\n" "}\n"; std::string code = original_code; - std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException); + std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos); const std::string expected_result_code = "\n\n" "class JVMOPTION_TEST_WITH_SPACE {\n" @@ -193,14 +185,14 @@ TEST(ScriptOptionLinesTest, test_multiple_lines_with_code) { "%jar /buckets/bucketfs1/jars/exajdbc.jar; class DEF{};\n"; std::string code = original_code; - std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException); + std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos); EXPECT_EQ(res, "-Dhttp.agent=\"ABC DEF\""); std::string expected_result_code = " class Abc{};\n\n" "%jar /buckets/bucketfs1/jars/exajdbc.jar; class DEF{};\n"; EXPECT_EQ(code, expected_result_code); - res = extractOptionLine(code, "%jar", whitespace, lineEnd, pos, throwException); + res = extractOptionLine(code, "%jar", whitespace, lineEnd, pos); EXPECT_EQ(res, "/buckets/bucketfs1/jars/exajdbc.jar"); expected_result_code = " class Abc{};\n\n" From 43ecf04e24f7066d26f3b0715b369cfd05d68b06 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Thu, 10 Oct 2024 07:31:35 -0300 Subject: [PATCH 2/2] Findings from review: 1. Added a utils pkg with a Utils::rethrow() method 2. Defined a custom exception class: ExecutionGraph::OptionParserException --- exaudfclient/BUILD | 4 +-- exaudfclient/base/BUILD | 8 +----- exaudfclient/base/benchmark_container/BUILD | 2 +- .../benchmark_container.cc | 2 +- exaudfclient/base/exaudflib/BUILD | 2 +- exaudfclient/base/exaudflib/impl/check.cc | 2 +- .../base/exaudflib/impl/exaudflib_main.cc | 2 +- .../base/exaudflib/impl/socket_high_level.cc | 2 +- .../base/exaudflib/impl/socket_low_level.cc | 2 +- exaudfclient/base/javacontainer/BUILD | 2 +- .../base/javacontainer/javacontainer_impl.cc | 21 ++++++-------- .../base/javacontainer/script_options/BUILD | 5 ++-- .../javacontainer/script_options/converter.h | 1 - .../javacontainer/script_options/extractor.cc | 2 +- .../javacontainer/script_options/extractor.h | 1 - .../script_options/parser_legacy.cc | 28 +++++++++---------- exaudfclient/base/python/python3/BUILD | 4 +-- .../python/python3/python_ext_dataframe.cc | 2 +- exaudfclient/base/python/pythoncontainer.cc | 2 +- exaudfclient/base/script_options_parser/BUILD | 5 ++++ .../base/script_options_parser/ctpg/BUILD | 1 + .../ctpg/script_option_lines_ctpg.cc | 3 +- .../ctpg/script_option_lines_ctpg.h | 1 - .../ctpg/test/script_option_lines_test.cpp | 10 ++++--- .../base/script_options_parser/exception.h | 17 +++++++++++ .../base/script_options_parser/legacy/BUILD | 1 + .../legacy/script_option_lines.cc | 9 +++--- .../legacy/script_option_lines.h | 2 +- .../legacy/test/script_option_lines_test.cpp | 5 ++-- exaudfclient/base/utils/BUILD | 10 +++++++ exaudfclient/base/{ => utils}/debug_message.h | 0 exaudfclient/base/utils/exceptions.h | 16 +++++++++++ exaudfclient/exaudfclient.cc | 2 +- 33 files changed, 110 insertions(+), 66 deletions(-) create mode 100644 exaudfclient/base/script_options_parser/exception.h create mode 100644 exaudfclient/base/utils/BUILD rename exaudfclient/base/{ => utils}/debug_message.h (100%) create mode 100644 exaudfclient/base/utils/exceptions.h diff --git a/exaudfclient/BUILD b/exaudfclient/BUILD index a0c3665c..656ed774 100644 --- a/exaudfclient/BUILD +++ b/exaudfclient/BUILD @@ -77,7 +77,7 @@ cc_binary( name = "exaudfclient_bin", srcs = ["exaudfclient.cc", "//base:load_dynamic"], linkopts = ["-ldl"], # needed for dynamicly loading libexaudflib_complete.so into another linker namespace - deps = ["//base/exaudflib:header", "//base:debug_message_h"]+VM_ENABLED_DEPS+VM_PYTHON3_DEPS+ + deps = ["//base/exaudflib:header", "//base/utils:utils"]+VM_ENABLED_DEPS+VM_PYTHON3_DEPS+ ["//base/exaudflib:exaudflib-deps", "//base/swig_factory:swig_factory"], defines = VM_ENABLED_DEFINES, data = ["//base:libexaudflib_complete.so"] @@ -100,7 +100,7 @@ cc_binary( name = "exaudfclient_static_bin", srcs = ["exaudfclient.cc", "//base:load_dynamic"], linkopts = ["-ldl"], # needed for dynamicly loading libexaudflib_complete.so into another linker namespace - deps = ["//base/exaudflib:header", "//base:debug_message_h"]+VM_ENABLED_DEPS+VM_PYTHON3_DEPS+ + deps = ["//base/exaudflib:header", "//base/utils:utils"]+VM_ENABLED_DEPS+VM_PYTHON3_DEPS+ ["//base/exaudflib:exaudflib-deps", "//base/swig_factory:swig_factory"] + [ "@zmq//:zmq", "@protobuf//:protobuf"], defines = VM_ENABLED_DEFINES, diff --git a/exaudfclient/base/BUILD b/exaudfclient/base/BUILD index 687ba0d2..357c34da 100644 --- a/exaudfclient/base/BUILD +++ b/exaudfclient/base/BUILD @@ -6,19 +6,13 @@ exports_files(["filter_swig_code.py", "build_integrated.py", load("//:variables.bzl", "VM_ENABLED_DEFINES") -cc_library( - name = "debug_message_h", - hdrs = [ - "debug_message.h" - ], -) cc_library( name = "load_dynamic", srcs = [ "load_dynamic.cc" ], - deps = ["//base/exaudflib:header", "//base:debug_message_h", "//base/exaudflib:exaudflib-deps"], + deps = ["//base/exaudflib:header", "//base/utils:utils", "//base/exaudflib:exaudflib-deps"], defines = VM_ENABLED_DEFINES, ) diff --git a/exaudfclient/base/benchmark_container/BUILD b/exaudfclient/base/benchmark_container/BUILD index 33b5be8c..9c7c0ba8 100644 --- a/exaudfclient/base/benchmark_container/BUILD +++ b/exaudfclient/base/benchmark_container/BUILD @@ -4,5 +4,5 @@ cc_library( name = "benchmark_container", srcs = ["benchmark_container.cc","benchmark_container.h"], hdrs = ["benchmark_container.h"], - deps = ["//base/exaudflib:exaudflib-deps","//base/exaudflib:header", "//base:debug_message_h"]+["//base/script_options_parser:scriptoptionlines"] + deps = ["//base/exaudflib:exaudflib-deps","//base/exaudflib:header", "//base/utils:utils"]+["//base/script_options_parser:scriptoptionlines"] ) \ No newline at end of file diff --git a/exaudfclient/base/benchmark_container/benchmark_container.cc b/exaudfclient/base/benchmark_container/benchmark_container.cc index 3efa97cd..057bc375 100644 --- a/exaudfclient/base/benchmark_container/benchmark_container.cc +++ b/exaudfclient/base/benchmark_container/benchmark_container.cc @@ -2,7 +2,7 @@ #define ENABLE_BENCHMARK_VM #endif -#include "base/debug_message.h" +#include "base/utils/debug_message.h" #include "benchmark_container.h" #include #include diff --git a/exaudfclient/base/exaudflib/BUILD b/exaudfclient/base/exaudflib/BUILD index ed07bfde..4cc20aac 100644 --- a/exaudfclient/base/exaudflib/BUILD +++ b/exaudfclient/base/exaudflib/BUILD @@ -94,5 +94,5 @@ cc_library( "impl/swig/swig_general_iterator.h", "impl/swig/swig_general_iterator.cc"], defines = VM_ENABLED_DEFINES, linkstatic = False, # Needs to be false, because otherwise we might get missing symbols when loading exaudflib_complete.so - deps = [":exaudflib-deps", ":zmqcontainer", "@zmq//:zmq", ":header", "//base:debug_message_h"], + deps = [":exaudflib-deps", ":zmqcontainer", "@zmq//:zmq", ":header", "//base/utils:utils"], ) diff --git a/exaudfclient/base/exaudflib/impl/check.cc b/exaudfclient/base/exaudflib/impl/check.cc index 44ca9345..b39e8510 100644 --- a/exaudfclient/base/exaudflib/impl/check.cc +++ b/exaudfclient/base/exaudflib/impl/check.cc @@ -6,7 +6,7 @@ #include -#include "base/debug_message.h" +#include "base/utils/debug_message.h" namespace exaudflib { namespace check { diff --git a/exaudfclient/base/exaudflib/impl/exaudflib_main.cc b/exaudfclient/base/exaudflib/impl/exaudflib_main.cc index cd74f240..2e0943a4 100644 --- a/exaudfclient/base/exaudflib/impl/exaudflib_main.cc +++ b/exaudfclient/base/exaudflib/impl/exaudflib_main.cc @@ -10,7 +10,7 @@ #include #include -#include "base/debug_message.h" +#include "base/utils/debug_message.h" // swig lib #include diff --git a/exaudfclient/base/exaudflib/impl/socket_high_level.cc b/exaudfclient/base/exaudflib/impl/socket_high_level.cc index 64721cef..3a4d4d8d 100644 --- a/exaudfclient/base/exaudflib/impl/socket_high_level.cc +++ b/exaudfclient/base/exaudflib/impl/socket_high_level.cc @@ -7,7 +7,7 @@ #include "base/exaudflib/swig/swig_common.h" #include "base/exaudflib/vm/swig_vm.h" #include -#include "base/debug_message.h" +#include "base/utils/debug_message.h" namespace exaudflib { namespace socket_high_level { diff --git a/exaudfclient/base/exaudflib/impl/socket_low_level.cc b/exaudfclient/base/exaudflib/impl/socket_low_level.cc index 5bd14a30..74e2de8b 100644 --- a/exaudfclient/base/exaudflib/impl/socket_low_level.cc +++ b/exaudfclient/base/exaudflib/impl/socket_low_level.cc @@ -1,6 +1,6 @@ #include "base/exaudflib/impl/socket_low_level.h" #include "base/exaudflib/impl/check.h" -#include "base/debug_message.h" +#include "base/utils/debug_message.h" #include #include #include diff --git a/exaudfclient/base/javacontainer/BUILD b/exaudfclient/base/javacontainer/BUILD index c42f4cff..fa42cd31 100644 --- a/exaudfclient/base/javacontainer/BUILD +++ b/exaudfclient/base/javacontainer/BUILD @@ -109,7 +109,7 @@ cc_library( srcs = [":javacontainer.cc", ":javacontainer.h", ":javacontainer_impl.cc", ":javacontainer_impl.h", ":dummy"], hdrs = [":filter_swig_code_exascript_java_h", "exascript_java_jni_decl.h"], deps = ["@ssl//:ssl","@java//:java", ":exascript_java", "//base/exaudflib:header", - "//base:debug_message_h","//base/javacontainer/script_options:java_script_option_lines", + "//base/utils:utils","//base/javacontainer/script_options:java_script_option_lines", "//base/swig_factory:swig_factory_if"], # copts= ["-O0","-fno-lto"], alwayslink=True, diff --git a/exaudfclient/base/javacontainer/javacontainer_impl.cc b/exaudfclient/base/javacontainer/javacontainer_impl.cc index ec857723..96aa0898 100644 --- a/exaudfclient/base/javacontainer/javacontainer_impl.cc +++ b/exaudfclient/base/javacontainer/javacontainer_impl.cc @@ -8,7 +8,7 @@ #include "exascript_java_jni_decl.h" -#include "base/debug_message.h" +#include "base/utils/debug_message.h" #include "base/javacontainer/javacontainer.h" #include "base/javacontainer/javacontainer_impl.h" #include "base/javacontainer/script_options/extractor.h" @@ -35,21 +35,18 @@ JavaVMImpl::JavaVMImpl(bool checkOnly, bool noJNI, SwigFactory& swigFactory) m_exaJavaPath = "/exaudf/base/javacontainer"; // TODO hardcoded path JavaScriptOptions::ScriptOptionLinesParserLegacy scriptOptionsParser; - try { - JavaScriptOptions::Extractor extractor(scriptOptionsParser, swigFactory); - DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode)); // To be called before scripts are imported. Otherwise, the script classname from an imported script could be used + JavaScriptOptions::Extractor extractor(scriptOptionsParser, swigFactory); - DBG_FUNC_CALL(cerr,setClasspath()); + DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode)); // To be called before scripts are imported. Otherwise, the script classname from an imported script could be used - m_jvmOptions = std::move(extractor.moveJvmOptions()); + DBG_FUNC_CALL(cerr,setClasspath()); - for (set::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end(); - ++it) { - addJarToClasspath(*it); - } - } catch(const std::runtime_error& ex) { - throwException(ex); + m_jvmOptions = std::move(extractor.moveJvmOptions()); + + for (set::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end(); + ++it) { + addJarToClasspath(*it); } m_needsCompilation = checkNeedsCompilation(); diff --git a/exaudfclient/base/javacontainer/script_options/BUILD b/exaudfclient/base/javacontainer/script_options/BUILD index e453ec0b..585f8a31 100644 --- a/exaudfclient/base/javacontainer/script_options/BUILD +++ b/exaudfclient/base/javacontainer/script_options/BUILD @@ -6,6 +6,7 @@ cc_library( hdrs = [":extractor.h", ":parser_legacy.h"], srcs = [":parser.h", ":converter.h", ":converter.cc", ":parser_legacy.cc", ":extractor.cc", ":keywords.h", ":checksum.h", ":checksum.cc"], - deps = ["//base/script_options_parser/legacy:script_option_lines_parser_legacy", "//base:debug_message_h", - "//base/exaudflib:header", "//base/exaudflib:exaudflib-deps", "//base/swig_factory:swig_factory_if"], + deps = ["//base/script_options_parser/legacy:script_option_lines_parser_legacy", "//base/utils:utils", + "//base/exaudflib:header", "//base/exaudflib:exaudflib-deps", "//base/swig_factory:swig_factory_if", + "//base/script_options_parser:exception"], ) diff --git a/exaudfclient/base/javacontainer/script_options/converter.h b/exaudfclient/base/javacontainer/script_options/converter.h index eda178db..53a07029 100644 --- a/exaudfclient/base/javacontainer/script_options/converter.h +++ b/exaudfclient/base/javacontainer/script_options/converter.h @@ -3,7 +3,6 @@ #include #include -#include #include #include diff --git a/exaudfclient/base/javacontainer/script_options/extractor.cc b/exaudfclient/base/javacontainer/script_options/extractor.cc index 5f6bb0cf..ed33388b 100644 --- a/exaudfclient/base/javacontainer/script_options/extractor.cc +++ b/exaudfclient/base/javacontainer/script_options/extractor.cc @@ -1,7 +1,7 @@ #include "base/javacontainer/script_options/extractor.h" #include "base/javacontainer/script_options/parser.h" -#include "base/debug_message.h" +#include "base/utils/debug_message.h" #include #define EXTR_DBG_FUNC_CALL(f) DBG_FUNC_CALL(std::cerr, f) diff --git a/exaudfclient/base/javacontainer/script_options/extractor.h b/exaudfclient/base/javacontainer/script_options/extractor.h index ab4055ca..d69ae354 100644 --- a/exaudfclient/base/javacontainer/script_options/extractor.h +++ b/exaudfclient/base/javacontainer/script_options/extractor.h @@ -3,7 +3,6 @@ #include #include -#include #include diff --git a/exaudfclient/base/javacontainer/script_options/parser_legacy.cc b/exaudfclient/base/javacontainer/script_options/parser_legacy.cc index c32dfec7..4462e73b 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_legacy.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_legacy.cc @@ -3,10 +3,10 @@ #include "base/script_options_parser/legacy/script_option_lines.h" #include "base/exaudflib/swig/swig_meta_data.h" #include "base/swig_factory/swig_factory.h" +#include "base/utils/exceptions.h" +#include "base/script_options_parser/exception.h" #include -#include - namespace SWIGVMContainers { @@ -106,8 +106,8 @@ void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFacto try { parseForSingleOption(m_keywords.importKeyword(), [&](const std::string& value, size_t pos){scriptPos = pos; newScript = value;}); - } catch (const std::runtime_error & ex) { - throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1614 ") + ex.what()); + } catch (const ExecutionGraph::OptionParserException & ex) { + Utils::rethrow(ex, "F-UDF-CL-SL-JAVA-1614"); } if (!newScript.empty()) { if (!metaData) { @@ -135,8 +135,8 @@ void ScriptOptionLinesParserLegacy::parseForScriptClass(std::function #include "base/exaudflib/swig/swig_common.h" -#include "base/debug_message.h" +#include "base/utils/debug_message.h" #include diff --git a/exaudfclient/base/python/pythoncontainer.cc b/exaudfclient/base/python/pythoncontainer.cc index cc258113..a52b29f5 100644 --- a/exaudfclient/base/python/pythoncontainer.cc +++ b/exaudfclient/base/python/pythoncontainer.cc @@ -9,7 +9,7 @@ #include #include "exascript_python_int.h" #include "exascript_python.h" -#include "base/debug_message.h" +#include "base/utils/debug_message.h" #include "base/exaudflib/swig/script_data_transfer_objects.h" diff --git a/exaudfclient/base/script_options_parser/BUILD b/exaudfclient/base/script_options_parser/BUILD index ffd0fb0c..03bb142f 100644 --- a/exaudfclient/base/script_options_parser/BUILD +++ b/exaudfclient/base/script_options_parser/BUILD @@ -1 +1,6 @@ package(default_visibility = ["//visibility:public"]) + +cc_library( + name = "exception", + hdrs = ["exception.h"] +) diff --git a/exaudfclient/base/script_options_parser/ctpg/BUILD b/exaudfclient/base/script_options_parser/ctpg/BUILD index c2da0d62..cdb9ee34 100644 --- a/exaudfclient/base/script_options_parser/ctpg/BUILD +++ b/exaudfclient/base/script_options_parser/ctpg/BUILD @@ -4,5 +4,6 @@ cc_library( name = "script_option_lines_parser_ctpg", hdrs = ["script_option_lines_ctpg.h"], srcs = ["script_option_lines_ctpg.cc","ctpg.hpp"], + deps = ["//base/script_options_parser:exception"], copts= ["-fno-lto"], ) diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc index a338be27..9baa6c67 100644 --- a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc +++ b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc @@ -3,6 +3,7 @@ #include #include #include +#include "base/script_options_parser/exception.h" using namespace exaudf_ctpg; using namespace exaudf_ctpg::ftors; @@ -147,7 +148,7 @@ void parse(std::string&& code, options_type& result) { { std::stringstream ss; ss << "Error parsing script options: " << error_buffer.str(); - throw std::runtime_error(ss.str()); + throw OptionParserException(ss.str()); } } diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h index 23699a6b..21c29034 100644 --- a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h +++ b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h @@ -2,7 +2,6 @@ #define SCRIPTOPTIONLINESCTPG_H #include -#include #include #include #include diff --git a/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp b/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp index 630778e5..11f04b6c 100644 --- a/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp +++ b/exaudfclient/base/script_options_parser/ctpg/test/script_option_lines_test.cpp @@ -3,8 +3,10 @@ #include #include #include +#include "base/script_options_parser/exception.h" +using namespace ExecutionGraph; using namespace ExecutionGraph::OptionsLineParser::CTPG; using ::testing::MatchesRegex; @@ -75,13 +77,13 @@ TEST(ScriptOptionLinesTest, need_option_termination_character) { { parseOptions(code, result); } - catch( const std::runtime_error& e ) + catch( const OptionParserException& e ) { // and this tests that it has the correct message EXPECT_STREQ( e.what(), "Error parsing script options: [1:17] PARSE: Syntax error: Unexpected ''\n"); throw; } - }, std::runtime_error ); + }, OptionParserException ); } TEST(ScriptOptionLinesTest, finds_the_two_options_same_key) { @@ -131,12 +133,12 @@ TEST_P(ScriptOptionLinesInvalidOptionTest, value_is_mandatory) { { parseOptions(code, result); } - catch( const std::runtime_error& e ) + catch( const OptionParserException& e ) { EXPECT_THAT( e.what(), MatchesRegex("^Error parsing script options.*PARSE: Syntax error: Unexpected.*$")); throw; } - }, std::runtime_error ); + }, OptionParserException ); } const std::vector invalid_options = {"%some_option ;", "%some_option \n", "\n%some_option\n;", "%some_option\nvalue;"}; diff --git a/exaudfclient/base/script_options_parser/exception.h b/exaudfclient/base/script_options_parser/exception.h new file mode 100644 index 00000000..ff535c7f --- /dev/null +++ b/exaudfclient/base/script_options_parser/exception.h @@ -0,0 +1,17 @@ +#ifndef SCRIPTOPTIONLINESEXCEPTION_H +#define SCRIPTOPTIONLINESEXCEPTION_H + +#include + + +namespace ExecutionGraph +{ + +class OptionParserException : public std::runtime_error { + using std::runtime_error::runtime_error; +}; + + +} // namespace ExecutionGraph + +#endif // SCRIPTOPTIONLINESEXCEPTION_H \ No newline at end of file diff --git a/exaudfclient/base/script_options_parser/legacy/BUILD b/exaudfclient/base/script_options_parser/legacy/BUILD index 30bdb8fb..d4aa0421 100644 --- a/exaudfclient/base/script_options_parser/legacy/BUILD +++ b/exaudfclient/base/script_options_parser/legacy/BUILD @@ -4,5 +4,6 @@ cc_library( name = "script_option_lines_parser_legacy", hdrs = ["script_option_lines.h"], srcs = ["script_option_lines.cc"], + deps = ["//base/script_options_parser:exception"], copts= ["-fno-lto"], ) diff --git a/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc b/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc index 180f896f..b0b7e487 100644 --- a/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc +++ b/exaudfclient/base/script_options_parser/legacy/script_option_lines.cc @@ -1,6 +1,7 @@ #include "script_option_lines.h" #include #include +#include "base/script_options_parser/exception.h" @@ -62,27 +63,27 @@ std::string extractOptionLine(std::string& code, const std::string option, const if (firstPos == std::string::npos) { std::stringstream ss; ss << "No values found for " << option << " statement"; - throw std::runtime_error(ss.str().c_str()); + throw OptionParserException(ss.str().c_str()); } // Find the end of line. size_t lastPos = code.find_first_of(lineEnd + "\r\n", firstPos); if (lastPos == std::string::npos || code.compare(lastPos, lineEnd.length(), lineEnd) != 0) { std::stringstream ss; ss << "End of " << option << " statement not found"; - throw std::runtime_error(ss.str().c_str()); + throw OptionParserException(ss.str().c_str()); } // If no values were found if (firstPos >= lastPos) { std::stringstream ss; ss << "No values found for " << option << " statement"; - throw std::runtime_error(ss.str().c_str()); + throw OptionParserException(ss.str().c_str()); } // If no values were found size_t optionsEnd = code.find_last_not_of(whitespace, lastPos - 1); if (optionsEnd == std::string::npos || optionsEnd < firstPos) { std::stringstream ss; ss << "No values found for " << option << " statement"; - throw std::runtime_error(ss.str().c_str()); + throw OptionParserException(ss.str().c_str()); } result = code.substr(firstPos, optionsEnd - firstPos + 1); code.erase(startPos, lastPos - startPos + 1); diff --git a/exaudfclient/base/script_options_parser/legacy/script_option_lines.h b/exaudfclient/base/script_options_parser/legacy/script_option_lines.h index b16efe54..00670a71 100644 --- a/exaudfclient/base/script_options_parser/legacy/script_option_lines.h +++ b/exaudfclient/base/script_options_parser/legacy/script_option_lines.h @@ -2,11 +2,11 @@ #define SCRIPTOPTIONLINES_H #include -#include namespace ExecutionGraph { + /*! * \brief extractOptionLine Extracts syntactically valid option lines of form %