From 235772a7d6a0e68065627941b02249c4fa5f3ea2 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Mon, 21 Oct 2024 16:01:18 -0300 Subject: [PATCH] #989: Trim script class and import script options --- .../base/javacontainer/script_options/BUILD | 2 +- .../javacontainer/script_options/converter.cc | 5 +- .../parser_ctpg_script_importer.cc | 5 +- .../script_options/string_ops.cc | 1 + .../javacontainer/script_options/string_ops.h | 40 +++++++++ .../test/cpp/javacontainer_test.cc | 84 +++++++++++++++++++ 6 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 exaudfclient/base/javacontainer/script_options/string_ops.cc create mode 100644 exaudfclient/base/javacontainer/script_options/string_ops.h diff --git a/exaudfclient/base/javacontainer/script_options/BUILD b/exaudfclient/base/javacontainer/script_options/BUILD index 6874aa4a..1359a4fe 100644 --- a/exaudfclient/base/javacontainer/script_options/BUILD +++ b/exaudfclient/base/javacontainer/script_options/BUILD @@ -6,7 +6,7 @@ cc_library( hdrs = [":extractor.h", ":parser_legacy.h", ":parser_ctpg.h"], srcs = [":parser.h", ":converter.h", ":converter.cc", ":parser_legacy.cc", ":extractor.cc", ":keywords.h", ":keywords.cc", ":checksum.h", ":checksum.cc", ":parser_ctpg.cc", - ":parser_ctpg_script_importer.cc", ":parser_ctpg_script_importer.h"], + ":parser_ctpg_script_importer.cc", ":parser_ctpg_script_importer.h", ":string_ops.h", ":string_ops.cc"], deps = ["@ssl//:ssl", "//base/script_options_parser/legacy:script_option_lines_parser_legacy", "//base/script_options_parser/ctpg:script_option_lines_parser_ctpg", "//base/utils:utils", "//base/exaudflib:header", "//base/exaudflib:exaudflib-deps", "//base/swig_factory:swig_factory_if", diff --git a/exaudfclient/base/javacontainer/script_options/converter.cc b/exaudfclient/base/javacontainer/script_options/converter.cc index e73cada5..1c194e4a 100644 --- a/exaudfclient/base/javacontainer/script_options/converter.cc +++ b/exaudfclient/base/javacontainer/script_options/converter.cc @@ -1,4 +1,5 @@ #include "base/javacontainer/script_options/converter.h" +#include "base/javacontainer/script_options/string_ops.h" #include namespace SWIGVMContainers { @@ -28,8 +29,10 @@ void Converter::convertExternalJar(const std::string & value) { } void Converter::convertScriptClassName(const std::string & value) { + std::string trimmedValue(value); + StringOps::trim(trimmedValue); if (value != "") { - m_jvmOptions.push_back("-Dexasol.scriptclass=" + value); + m_jvmOptions.push_back("-Dexasol.scriptclass=" + trimmedValue); } } diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index 226e2359..d08a662f 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -3,6 +3,7 @@ #include "base/utils/debug_message.h" #include "base/utils/exceptions.h" #include "base/script_options_parser/exception.h" +#include "base/javacontainer/script_options/string_ops.h" #include #include @@ -80,12 +81,14 @@ void ScriptImporter::importScript(std::string & scriptCode, } const char* ScriptImporter::findImportScript(const std::string & scriptKey) { + std::string trimmedScriptKey(scriptKey); + StringOps::trim(trimmedScriptKey); if (!m_metaData) { m_metaData.reset(m_swigFactory.makeSwigMetadata()); if (!m_metaData) throw std::runtime_error("F-UDF-CL-SL-JAVA-1631: Failure while importing scripts"); } - const char *importScriptCode = m_metaData->moduleContent(scriptKey.c_str()); + const char *importScriptCode = m_metaData->moduleContent(trimmedScriptKey.c_str()); const char *exception = m_metaData->checkException(); if (exception) throw std::runtime_error("F-UDF-CL-SL-JAVA-1632: " + std::string(exception)); diff --git a/exaudfclient/base/javacontainer/script_options/string_ops.cc b/exaudfclient/base/javacontainer/script_options/string_ops.cc new file mode 100644 index 00000000..df1b6081 --- /dev/null +++ b/exaudfclient/base/javacontainer/script_options/string_ops.cc @@ -0,0 +1 @@ +#include "base/javacontainer/script_options/string_ops.h" diff --git a/exaudfclient/base/javacontainer/script_options/string_ops.h b/exaudfclient/base/javacontainer/script_options/string_ops.h new file mode 100644 index 00000000..65f9b0bd --- /dev/null +++ b/exaudfclient/base/javacontainer/script_options/string_ops.h @@ -0,0 +1,40 @@ +#ifndef SCRIPTOPTIONLINEPARSERSTRINGOPS_H +#define SCRIPTOPTIONLINEPARSERSTRINGOPS_H 1 + +#include +#include + + + +namespace SWIGVMContainers { + +namespace JavaScriptOptions { + +namespace StringOps { + +inline void ltrim(std::string &s) { + s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](unsigned char ch) { + return !std::isspace(ch); + })); +} + + +inline void rtrim(std::string &s) { + s.erase(std::find_if(s.rbegin(), s.rend(), [](unsigned char ch) { + return !std::isspace(ch); + }).base(), s.end()); +} + +inline void trim(std::string &s) { + ltrim(s); + rtrim(s); +} + +} //namespace StringOps + + +} //namespace JavaScriptOptions + +} //namespace SWIGVMContainers + +#endif //SCRIPTOPTIONLINEPARSERSTRINGOPS_H \ No newline at end of file diff --git a/exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc b/exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc index 0c754206..91f62b18 100644 --- a/exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc +++ b/exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc @@ -2,9 +2,12 @@ #include "include/gtest/gtest.h" #include "gmock/gmock.h" #include "base/javacontainer/test/cpp/javavm_test.h" +#include "base/javacontainer/javacontainer.h" #include "base/javacontainer/test/cpp/swig_factory_test.h" #include +using ::testing::MatchesRegex; + TEST(JavaContainer, basic_jar) { const std::string script_code = "%scriptclass com.exasol.udf_profiling.UdfProfiler;\n" @@ -24,6 +27,46 @@ TEST(JavaContainer, basic_jar) { EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions); } +TEST(JavaContainer, basic_jar_script_class_with_white_spaces) { + const std::string script_code = "%scriptclass com.exasol.udf_profiling.UdfProfiler\t ;\n" + "%jar base/javacontainer/test/test.jar;"; + JavaVMTest vm(script_code); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJavaPath, "/exaudf/base/javacontainer"); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_localClasspath, "/tmp"); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_scriptCode, "\n"); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJarPath, "/exaudf/base/javacontainer/exaudf_deploy.jar"); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_classpath, "/exaudf/base/javacontainer/exaudf_deploy.jar:base/javacontainer/test/test.jar"); + EXPECT_FALSE(vm.getJavaVMInternalStatus().m_needsCompilation); + const std::vector expectedJVMOptions = { "-Dexasol.scriptclass=com.exasol.udf_profiling.UdfProfiler", + "-Xms128m", "-Xmx128m", "-Xss512k", + "-XX:ErrorFile=/tmp/hs_err_pid%p.log", + "-Djava.class.path=/exaudf/base/javacontainer/exaudf_deploy.jar:base/javacontainer/test/test.jar", + "-XX:+UseSerialGC" }; + EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions); +} + + +TEST(JavaContainer, basic_jar_with_white_spaces) { + const std::string script_code = "%jar base/javacontainer/test/test.jar \t ;"; + +#ifndef USE_CTPG_PARSER //The parsers behave differently: The legacy parser removes trailing white spaces. + JavaVMTest vm(script_code); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_classpath, "/exaudf/base/javacontainer/exaudf_deploy.jar:base/javacontainer/test/test.jar"); +#else + EXPECT_THROW({ + try + { + JavaVMTest vm(script_code); + } + catch( const SWIGVMContainers::JavaVMach::exception& e ) + { + EXPECT_THAT( e.what(), MatchesRegex("^.*Java VM cannot find 'base/javacontainer/test/test\\.jar \t ': No such file or directory$")); + throw; + } + }, SWIGVMContainers::JavaVMach::exception ); +#endif +} + TEST(JavaContainer, basic_inline) { const std::string script_code = "import java.time.LocalDateTime;" @@ -156,6 +199,47 @@ TEST(JavaContainer, simple_import_script) { EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions); } +TEST(JavaContainer, simple_import_script_with_white_space) { + const std::string script_code = + "%import other_script\t ;\n\n" + "%jvmoption -Dhttp.agent=\"ABC\";\n\n" + "class JVMOPTION_TEST {\n" + "static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {\n\n" + " ctx.emit(\"Success!\");\n" + " }\n" + "}\n"; + auto swigFactory = std::make_unique(); + + const std::string other_script_code = + "class OtherClass {\n" + "static void doSomething() {\n\n" + " }\n" + "}\n"; + swigFactory->addModule("other_script", other_script_code); + JavaVMTest vm(script_code, std::move(swigFactory)); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJavaPath, "/exaudf/base/javacontainer"); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_localClasspath, "/tmp"); + const std::string expected_script_code = + "package com.exasol;\r\n" + "class OtherClass {\n" + "static void doSomething() {\n\n" + " }\n" + "}\n\n\n\n\n" + "class JVMOPTION_TEST {\n" + "static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {\n\n" + "\tctx.emit(\"Success!\");\n" + " }\n}\n"; + EXPECT_EQ(vm.getJavaVMInternalStatus().m_scriptCode, expected_script_code); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJarPath, "/exaudf/base/javacontainer/exaudf_deploy.jar"); + EXPECT_EQ(vm.getJavaVMInternalStatus().m_classpath, "/tmp:/exaudf/base/javacontainer/exaudf_deploy.jar"); + EXPECT_TRUE(vm.getJavaVMInternalStatus().m_needsCompilation); + const std::vector expectedJVMOptions = { "-Dhttp.agent=\"ABC\"", "-Xms128m", "-Xmx128m", "-Xss512k", + "-XX:ErrorFile=/tmp/hs_err_pid%p.log", + "-Djava.class.path=/tmp:/exaudf/base/javacontainer/exaudf_deploy.jar", + "-XX:+UseSerialGC" }; + EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions); +} + TEST(JavaContainer, import_script_with_recursion) { const std::string script_code = "%import other_script;\n\n"