#990: refactor script options parser

exaudfclient/base/javacontainer/javacontainer.cc

@@ -1,13 +1,13 @@
 #include "base/javacontainer/javacontainer.h"
 #include "base/javacontainer/javacontainer_impl.h"
-#include "base/javacontainer/script_options/parser.h"
+#include "base/javacontainer/script_options/extractor.h"
 
 using namespace SWIGVMContainers;
 using namespace std;
 
-JavaVMach::JavaVMach(bool checkOnly, std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> scriptOptionsParser) {
+JavaVMach::JavaVMach(bool checkOnly,std::unique_ptr<JavaScriptOptions::Extractor> extractor) {
     try {
-        m_impl = new JavaVMImpl(checkOnly, false, std::move(scriptOptionsParser));
+        m_impl = new JavaVMImpl(checkOnly, false, std::move(extractor));
     } catch (std::exception& err) {
         lock_guard<mutex> lock(exception_msg_mtx);
         exception_msg = "F-UDF-CL-SL-JAVA-1000: "+std::string(err.what());

exaudfclient/base/javacontainer/javacontainer.h

@@ -14,7 +14,7 @@ class JavaVMImpl;
 
 namespace JavaScriptOptions {
 
-struct ScriptOptionsParser;
+struct Extractor;
 
 }
 
@@ -23,7 +23,7 @@ class JavaVMach: public SWIGVM {
         /*
          * scriptOptionsParser: JavaVMach takes ownership of ScriptOptionsParser pointer.
          */
-        JavaVMach(bool checkOnly, std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> scriptOptionsParser);
+        JavaVMach(bool checkOnly, std::unique_ptr<JavaScriptOptions::Extractor> extractor);
         virtual ~JavaVMach() {}
         virtual void shutdown();
         virtual bool run();

exaudfclient/base/javacontainer/javacontainer_builder.cc

@@ -1,6 +1,5 @@
 #include "base/javacontainer/javacontainer_builder.h"
-#include "base/javacontainer/script_options/parser_ctpg.h"
-#include "base/javacontainer/script_options/parser_legacy.h"
+#include "base/javacontainer/script_options/extractor_impl.h"
 #include "base/swig_factory/swig_factory_impl.h"
 
 #ifdef ENABLE_JAVA_VM
@@ -16,13 +15,13 @@ JavaContainerBuilder& JavaContainerBuilder::useCtpgParser() {
 }
 
 JavaVMach* JavaContainerBuilder::build() {
-    std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> parser;
+    std::unique_ptr<JavaScriptOptions::Extractor> extractor;
     if (m_useCtpgParser) {
-        parser = std::make_unique<JavaScriptOptions::ScriptOptionLinesParserCTPG>(std::make_unique<SwigFactoryImpl>());
+        extractor = std::make_unique<JavaScriptOptions::tExtractorV2>(std::make_unique<SwigFactoryImpl>());
     } else {
-        parser = std::make_unique<JavaScriptOptions::ScriptOptionLinesParserLegacy>(std::make_unique<SwigFactoryImpl>());
+        extractor = std::make_unique<JavaScriptOptions::tExtractorLegacy>(std::make_unique<SwigFactoryImpl>());
     }
-    return new JavaVMach(false, std::move(parser));
+    return new JavaVMach(false, std::move(extractor));
 }
 
 

exaudfclient/base/javacontainer/javacontainer_impl.cc

@@ -12,14 +12,13 @@
 #include "base/javacontainer/javacontainer.h"
 #include "base/javacontainer/javacontainer_impl.h"
 #include "base/javacontainer/script_options/extractor.h"
-#include "base/javacontainer/script_options/parser.h"
 
 
 using namespace SWIGVMContainers;
 using namespace std;
 
 JavaVMImpl::JavaVMImpl(bool checkOnly, bool noJNI,
-                        std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> scriptOptionsParser)
+                        std::unique_ptr<JavaScriptOptions::Extractor> extractor)
 : m_checkOnly(checkOnly)
 , m_exaJavaPath("")
 , m_localClasspath("/tmp") // **IMPORTANT**: /tmp needs to be in the classpath, otherwise ExaCompiler crashe with com.exasol.ExaCompilationException: /DATE_STRING.java:3: error: error while writing DATE_STRING: could not create parent directories
@@ -32,7 +31,7 @@ JavaVMImpl::JavaVMImpl(bool checkOnly, bool noJNI,
     stringstream ss;
     m_exaJavaPath = "/exaudf/base/javacontainer"; // TODO hardcoded path
 
-    parseScriptOptions(std::move(scriptOptionsParser));
+    parseScriptOptions(std::move(extractor));
 
     m_needsCompilation = checkNeedsCompilation();
     if (m_needsCompilation) {
@@ -49,18 +48,16 @@ JavaVMImpl::JavaVMImpl(bool checkOnly, bool noJNI,
     }
 }
 
-void JavaVMImpl::parseScriptOptions(std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> scriptOptionsParser) {
-    JavaScriptOptions::Extractor extractor(*scriptOptionsParser);
+void JavaVMImpl::parseScriptOptions(std::unique_ptr<JavaScriptOptions::Extractor> extractor) {
 
-    DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode));
+    DBG_FUNC_CALL(cerr,extractor->extract(m_scriptCode));
 
     DBG_FUNC_CALL(cerr,setClasspath());
 
-    m_jvmOptions = std::move(extractor.moveJvmOptions());
+    m_jvmOptions = std::move(extractor->moveJvmOptions());
 
-    for (set<string>::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end();
-         ++it) {
-        addJarToClasspath(*it);
+    for (const auto & jar :  extractor->getJarPaths()) {
+        addJarToClasspath(jar);
     }
 }
 

exaudfclient/base/javacontainer/javacontainer_impl.h

@@ -16,7 +16,7 @@ class JavaVMTest;
 namespace SWIGVMContainers {
 
 namespace JavaScriptOptions {
-    struct ScriptOptionsParser;
+    struct Extractor;
 }
 
 class JavaVMImpl {
@@ -25,8 +25,7 @@ class JavaVMImpl {
         /*
          * scriptOptionsParser: JavaVMImpl takes ownership of ScriptOptionsParser pointer.
          */
-        JavaVMImpl(bool checkOnly, bool noJNI,
-                    std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> scriptOptionsParser);
+        JavaVMImpl(bool checkOnly, bool noJNI, std::unique_ptr<JavaScriptOptions::Extractor> extractor);
         ~JavaVMImpl() {}
         void shutdown();
         bool run();
@@ -43,7 +42,7 @@ class JavaVMImpl {
         void setClasspath();
         void setJvmOptions();
         void addJarToClasspath(const std::string& path);
-        void parseScriptOptions(std::unique_ptr<JavaScriptOptions::ScriptOptionsParser> scriptOptionsParser);
+        void parseScriptOptions(std::unique_ptr<JavaScriptOptions::Extractor> extractor);
         bool m_checkOnly;
         std::string m_exaJavaPath;
         std::string m_localClasspath;

exaudfclient/base/javacontainer/script_options/BUILD

@@ -3,8 +3,11 @@ package(default_visibility = ["//visibility:public"])
 
 cc_library(
     name = "java_script_option_lines",
-    hdrs = [":extractor.h", ":parser_legacy.h", ":parser_ctpg.h"],
-    srcs = [":parser.h", ":converter.h", ":converter.cc", ":parser_legacy.cc", ":extractor.cc",
+    hdrs = [":extractor.h", ":extractor_impl.h", ":parser_legacy.h", ":parser_ctpg.h",
+            ":converter_v2.h", ":converter_legacy.h"],
+    srcs = [":parser.h", ":converter.h", ":converter.cc", ":parser_legacy.cc", ":extractor_impl.h",
+            ":extractor_impl.cc", ":converter_legacy.cc", ":converter_legacy.h",
+            ":converter_v2.cc", ":converter_v2.h",
             ":keywords.h", ":keywords.cc", ":checksum.h", ":checksum.cc", ":parser_ctpg.cc",
             ":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",

exaudfclient/base/javacontainer/script_options/converter.cc

@@ -8,26 +8,8 @@ namespace JavaScriptOptions {
 
 Converter::Converter()
 : m_jvmOptions()
-, m_jarPaths()
 , m_whitespace(" \t\f\v") {}
 
-void Converter::convertExternalJar(const std::string & value) {
-    for (size_t start = 0, delim = 0; ; start = delim + 1) {
-        delim = value.find(":", start);
-        if (delim != std::string::npos) {
-            std::string jar = value.substr(start, delim - start);
-            if (m_jarPaths.find(jar) == m_jarPaths.end())
-                m_jarPaths.insert(jar);
-        }
-        else {
-            std::string jar = value.substr(start);
-            if (m_jarPaths.find(jar) == m_jarPaths.end())
-                m_jarPaths.insert(jar);
-            break;
-        }
-    }
-}
-
 void Converter::convertScriptClassName(const std::string & value) {
     std::string trimmedValue(value);
     StringOps::trim(trimmedValue);

exaudfclient/base/javacontainer/script_options/converter.h

@@ -4,11 +4,6 @@
 #include <string>
 #include <vector>
 #include <set>
-#include <memory>
-
-#include "base/javacontainer/script_options/parser.h"
-
-
 
 namespace SWIGVMContainers {
 
@@ -19,32 +14,28 @@ class Converter {
     public:
         Converter();
 
-        void convertExternalJar(const std::string & value);
-
         void convertScriptClassName(const std::string & value);
 
         void convertJvmOption(const std::string & value);
 
-        const std::set<std::string> & getJarPaths() const {
-            return m_jarPaths;
-        }
-
         std::vector<std::string>&& moveJvmOptions() {
             return std::move(m_jvmOptions);
         }
 
+        virtual void convertExternalJar(const std::string & value) = 0;
+
+        virtual const std::set<std::string> & getJarPaths() const = 0;
+
+
     private:
 
         std::vector<std::string> m_jvmOptions;
-
-        std::set<std::string> m_jarPaths;
 
         const std::string m_whitespace;
 };
 
 
 
-
 } //namespace JavaScriptOptions
 
 } //namespace SWIGVMContainers

exaudfclient/base/javacontainer/script_options/converter_legacy.cc

@@ -0,0 +1,26 @@
+#include "base/javacontainer/script_options/converter_legacy.h"
+#include "base/javacontainer/script_options/string_ops.h"
+#include <iostream>
+#include <sstream>
+
+namespace SWIGVMContainers {
+
+namespace JavaScriptOptions {
+
+ConverterLegacy::ConverterLegacy()
+: Converter()
+, m_jarPaths() {}
+
+void ConverterLegacy::convertExternalJar(const std::string& value) {
+    std::istringstream stream(value);
+    std::string jar;
+
+    while (std::getline(stream, jar, ':')) {
+        m_jarPaths.insert(jar);
+    }
+}
+
+
+} //namespace JavaScriptOptions
+
+} //namespace SWIGVMContainers

exaudfclient/base/javacontainer/script_options/converter_legacy.h

@@ -0,0 +1,41 @@
+#ifndef SCRIPTOPTIONLINEPARSERCONVERTERLEGACY_H
+#define SCRIPTOPTIONLINEPARSERCONVERTERLEGACY_H 1
+
+#include <string>
+#include <vector>
+#include <set>
+#include <memory>
+
+#include "base/javacontainer/script_options/converter.h"
+
+
+
+namespace SWIGVMContainers {
+
+namespace JavaScriptOptions {
+
+class ConverterLegacy : public Converter {
+
+    public:
+        ConverterLegacy();
+
+        void convertExternalJar(const std::string & value);
+
+        const std::set<std::string> & getJarPaths() const {
+            return m_jarPaths;
+        }
+
+    private:
+
+        std::set<std::string> m_jarPaths;
+
+};
+
+
+
+
+} //namespace JavaScriptOptions
+
+} //namespace SWIGVMContainers
+
+#endif //SCRIPTOPTIONLINEPARSERCONVERTER_H

exaudfclient/base/javacontainer/script_options/converter_v2.cc

@@ -0,0 +1,25 @@
+#include "base/javacontainer/script_options/converter_v2.h"
+#include "base/javacontainer/script_options/string_ops.h"
+#include <iostream>
+#include <sstream>
+
+namespace SWIGVMContainers {
+
+namespace JavaScriptOptions {
+
+ConverterV2::ConverterV2()
+: Converter()
+, m_jarPaths() {}
+
+void ConverterV2::convertExternalJar(const std::string & value) {
+    std::istringstream stream(value);
+    std::string jar;
+
+    while (std::getline(stream, jar, ':')) {
+        m_jarPaths.insert(jar);
+    }
+}
+
+} //namespace JavaScriptOptions
+
+} //namespace SWIGVMContainers

exaudfclient/base/javacontainer/script_options/converter_v2.h

@@ -0,0 +1,41 @@
+#ifndef SCRIPTOPTIONLINEPARSERCONVERTERV2_H
+#define SCRIPTOPTIONLINEPARSERCONVERTERV2_H 1
+
+#include <string>
+#include <vector>
+#include <set>
+#include <memory>
+
+#include "base/javacontainer/script_options/converter.h"
+
+
+
+namespace SWIGVMContainers {
+
+namespace JavaScriptOptions {
+
+class ConverterV2 : public Converter {
+
+    public:
+        ConverterV2();
+
+        void convertExternalJar(const std::string & value);
+
+        const std::set<std::string> & getJarPaths() const {
+            return m_jarPaths;
+        }
+
+    private:
+
+        std::set<std::string> m_jarPaths;
+
+};
+
+
+
+
+} //namespace JavaScriptOptions
+
+} //namespace SWIGVMContainers
+
+#endif //SCRIPTOPTIONLINEPARSERCONVERTER_H