#989: Trim script class and import script options

exasol · Oct 21, 2024 · 235772a · 235772a
1 parent c289794
commit 235772a
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 3 deletions.
diff --git 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
@@ -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);
     }
 }
 

diff --git 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 <algorithm>
 #include <iostream>
@@ -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
@@ -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
@@ -0,0 +1,40 @@
+#ifndef SCRIPTOPTIONLINEPARSERSTRINGOPS_H
+#define SCRIPTOPTIONLINEPARSERSTRINGOPS_H 1
+
+#include <string>
+#include <algorithm>
+
+
+
+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
diff --git 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 <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,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<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, import_script_with_recursion) {
     const std::string script_code =
         "%import other_script;\n\n"
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		#include "base/javacontainer/script_options/string_ops.h"