#989: Trim script class and import script options

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",

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 <iostream>
 
 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);
     }
 }
 

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 <algorithm>
 #include <iostream>
@@ -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));

exaudfclient/base/javacontainer/script_options/string_ops.cc

@@ -0,0 +1 @@
+#include "base/javacontainer/script_options/string_ops.h"

exaudfclient/base/javacontainer/script_options/string_ops.h

@@ -0,0 +1,41 @@
+#ifndef SCRIPTOPTIONLINEPARSERSTRINGOPS_H
+#define SCRIPTOPTIONLINEPARSERSTRINGOPS_H 1
+
+#include <string>
+#include <algorithm>
+
+
+
+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

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",

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");
+}
+

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 <string.h>
 
+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<std::string> 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<SwigFactoryTestImpl>();
+
+    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<std::string> 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<SwigFactoryTestImpl>();
+
+    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<std::string> 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"

exaudfclient/base/javacontainer/test/cpp/javavm_test.cc

@@ -18,8 +18,7 @@ JavaVMTest::JavaVMTest(std::string scriptCode, std::unique_ptr<SwigFactoryTestIm
 }
 
 void JavaVMTest::run(std::string scriptCode, std::unique_ptr<SwigFactoryTestImpl> 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<SWIGVMContainers::JavaScriptOptions::ScriptOptionLinesParserLegacy> parser =
          std::make_unique<SWIGVMContainers::JavaScriptOptions::ScriptOptionLinesParserLegacy>(std::move(swigFactory));
@@ -35,6 +34,5 @@ void JavaVMTest::run(std::string scriptCode, std::unique_ptr<SwigFactoryTestImpl
     javaVMInternalStatus.m_classpath = javaVMImpl.m_classpath;
     javaVMInternalStatus.m_jvmOptions = javaVMImpl.m_jvmOptions;
     javaVMInternalStatus.m_needsCompilation = javaVMImpl.m_needsCompilation;
-    ::free(script_code);
 }