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 84dfb708..e7df481f 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 @@ -87,13 +88,15 @@ 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..8e0ff72c --- /dev/null +++ b/exaudfclient/base/javacontainer/script_options/string_ops.h @@ -0,0 +1,41 @@ +#ifndef SCRIPTOPTIONLINEPARSERSTRINGOPS_H +#define SCRIPTOPTIONLINEPARSERSTRINGOPS_H 1 + +#include +#include + + + +namespace SWIGVMContainers { + +namespace JavaScriptOptions { + +namespace StringOps { + +//Following code is based on https://stackoverflow.com/a/217605 + +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/script_options/test/BUILD b/exaudfclient/base/javacontainer/script_options/test/BUILD index 5c7f0cf5..c76203ab 100644 --- a/exaudfclient/base/javacontainer/script_options/test/BUILD +++ b/exaudfclient/base/javacontainer/script_options/test/BUILD @@ -1,7 +1,7 @@ cc_test( name = "java-script-options-tests", - srcs = ["ctpg_script_importer_test.cc", "swig_factory_test.cc", "swig_factory_test.h"], + srcs = ["ctpg_script_importer_test.cc", "swig_factory_test.cc", "swig_factory_test.h", "string_ops_tests.cc"], deps = [ "//base/javacontainer/script_options:java_script_option_lines", "@googletest//:gtest_main", diff --git a/exaudfclient/base/javacontainer/script_options/test/string_ops_tests.cc b/exaudfclient/base/javacontainer/script_options/test/string_ops_tests.cc new file mode 100644 index 00000000..8eaf722b --- /dev/null +++ b/exaudfclient/base/javacontainer/script_options/test/string_ops_tests.cc @@ -0,0 +1,27 @@ + +#include "include/gtest/gtest.h" +#include "gmock/gmock.h" +#include "base/javacontainer/script_options/string_ops.h" + +using namespace SWIGVMContainers::JavaScriptOptions; + + + + +TEST(StringOpsTest, trim) { + std::string sample = " \tHello World \t"; + StringOps::trim(sample); + EXPECT_EQ(sample, "Hello World"); +} + +TEST(StringOpsTest, trimWithNoneASCII) { + /* + Test that trim works correctly with None-ASCII characters + \xa0's bit sequence is '1010 0000', while space bit sequence '0010 0000'. + If StringOps::trim() would not work correctly with characters where MSB is set, it would interpret \xa0 as space. + */ + std::string sample = " \t\xa0Hello World\xa0 \t"; + StringOps::trim(sample); + EXPECT_EQ(sample, "\xa0Hello World\xa0"); +} + diff --git a/exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc b/exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc index 0c754206..3ea3a28c 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,88 @@ 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, simple_import_script_with_quotes) { + 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\t \"", 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" diff --git a/exaudfclient/base/javacontainer/test/cpp/javavm_test.cc b/exaudfclient/base/javacontainer/test/cpp/javavm_test.cc index 6a26dc35..79dfdad7 100644 --- a/exaudfclient/base/javacontainer/test/cpp/javavm_test.cc +++ b/exaudfclient/base/javacontainer/test/cpp/javavm_test.cc @@ -18,8 +18,7 @@ JavaVMTest::JavaVMTest(std::string scriptCode, std::unique_ptr swigFactory) { - char* script_code = ::strdup(scriptCode.c_str()); - SWIGVMContainers::SWIGVM_params->script_code = script_code; + SWIGVMContainers::SWIGVM_params->script_code = scriptCode.data(); #ifndef USE_CTPG_PARSER std::unique_ptr parser = std::make_unique(std::move(swigFactory)); @@ -35,6 +34,5 @@ void JavaVMTest::run(std::string scriptCode, std::unique_ptr