Skip to content

Commit

Permalink
Fixed findings from review
Browse files Browse the repository at this point in the history
  • Loading branch information
tomuben committed Oct 25, 2024
1 parent a882098 commit add6861
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 171 deletions.
7 changes: 5 additions & 2 deletions exaudfclient/base/javacontainer/script_options/converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ namespace SWIGVMContainers {

namespace JavaScriptOptions {

/**
* This class retrieves the raw Java script option values (scriptclass, jvmoption, jar)
* and converts them to the proper format expected by the JvmContainerImpl class.
* Besides the converter functions it provides methods to access the converted values.
*/
class Converter {

public:
Expand All @@ -26,10 +31,8 @@ class Converter {

virtual void convertExternalJar(const std::string & value) = 0;


virtual void iterateJarPaths(tJarIteratorCallback callback) const = 0;


private:

std::vector<std::string> m_jvmOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ namespace SWIGVMContainers {

namespace JavaScriptOptions {

/**
* This class is a specialization for the generic converter class.
* It implements conversion of the jar option according to the requirements in the old
* parser implementation.
*/
class ConverterLegacy : public Converter {

public:
Expand Down
5 changes: 5 additions & 0 deletions exaudfclient/base/javacontainer/script_options/converter_v2.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ namespace SWIGVMContainers {

namespace JavaScriptOptions {

/**
* This class is a specialization for the generic converter class.
* It implements conversion of the jar option according to the requirements in the new
* parser implementation.
*/
class ConverterV2 : public Converter {

public:
Expand Down
23 changes: 23 additions & 0 deletions exaudfclient/base/javacontainer/script_options/extractor.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,39 @@ namespace SWIGVMContainers {

namespace JavaScriptOptions {

/**
* Abstract interface for the Extractor class.
* Defines methods to extract the Java options from the script code.
* The extraction process searches for the known Java Options and handles them appropriatly.
* The script code is then modified, where the found options are removed.
* The interface defines methods to access the found Jar- and JvmOption options.
* The scriptclass and import options are processed internally by the respective extractor implementation.
*/
struct Extractor {

typedef std::function<void(const std::string &option)> tJarIteratorCallback;

virtual ~Extractor() {}

/**
* Access the found Jar paths. For each found jar path the given callback function is called with the jar option as argument.
*/
virtual void iterateJarPaths(tJarIteratorCallback callback) const = 0;

/**
* Access the Jvm option options. The extractor implementations must store the found Jvm Options in a std::vector.
* The vector is returned as rvalue.
*/
virtual std::vector<std::string>&& moveJvmOptions() = 0;

/**
* Run the extraction. This will:
* 1. Add the first `scriptclass` option to the list of Jvm options.
* 2. Replace and (nested) reference of an `import` script
* 3. Find and store all Jar options
* 4. Find and store all `jvmoption` options
* 5. Remove `scriptclass`, `jar`, `import` and `jvmoption` from the script code. The behavior is implementation specific.
*/
virtual void extract(std::string & scriptCode) = 0;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ namespace SWIGVMContainers {

namespace JavaScriptOptions {

/**
* Concrete implementation for the Extractor class.
* Given template parameter TParser and TConverter define concrete behavior.
*/
template<typename TParser, typename TConverter>
class ExtractorImpl : public Extractor {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ TEST(JavaContainer, basic_jars_ordering) {
* This test validates correct behavior of collecting the %jar options in Extractor V1/V2.
* Both jar files referenced in `script_code` do not exist.
* The JavaVM will throw an exception with the first JAR file it gets from Extractor.
* For extractor V21 Our assumption is that the exception will be for `base/javacontainer/test/abc.jar`
* For extractor V1 Our assumption is that the exception will be for `base/javacontainer/test/abc.jar`
* as this is alphabetically the first JAR file. (re-ordering)
* For extractor V2: Our assumption is that the exception will be for `base/javacontainer/test/test1.jar`
* as this is the first JAR file. (no re-ordering)
Expand Down
168 changes: 0 additions & 168 deletions exaudfclient/docs/java_script_options_parser.drawio

This file was deleted.

Binary file modified exaudfclient/docs/java_script_options_parser.drawio.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit add6861

Please sign in to comment.