lsc icon indicating copy to clipboard operation
lsc copied to clipboard

Fix: Added a function to improve jscript prefix identifier

Open abpai94 opened this issue 1 year ago • 7 comments

When using the following prefix to scripts such as rjs or others:

<dataset>
    <name>member</name>
    <policy>FORCE</policy>
    <forceValues>
        <string>
              <![CDATA[rjs:
              ...jscript_function...
              ]]>
        </string>
    </forceValues>
</dataset>

The following error is thrown:

java.lang.NullPointerException: Cannot invoke "org.lsc.utils.ScriptableEvaluator.evalToObjectList(org.lsc.Task, String, java.util.Map)" because "se" is null

This occurs due to the prefix containing the following:

\n                            
            rjs

The .contains method in Maps object is unable to match this pattern.

This functionality works well with the following script:

<mainIdentifier>rjs:...jscript_function...</mainIdentifier>

As the script is done in-line the .contains method is able to match and find the appropriate jscript evaluator.

I have added a new function that uses pattern matching with the help of a small regex function which should resolve the above described issue.

I have tested this in scenarios where no script engine has been defined without posing any issue. Also I have built on top of the work in #280 to seamlessly merge this minor fix.

abpai94 avatar Sep 04 '24 13:09 abpai94

@philhaworteks I have built this minor fix on top of your work in #280. We can merge when your changes are approved.

abpai94 avatar Sep 04 '24 13:09 abpai94

Even my intial request is not working with java 8 due to java.util.Optional.or that was provided only from java 9. java.lang.NoSuchMethodError: java.util.Optional.or(Ljava/util/function/Supplier;)Ljava/util/Optional;

artlog avatar Sep 04 '24 15:09 artlog

I used java 9 to get the Optional code to work. I changed the code to the following:

			defaultImplementation = Optional.ofNullable(Optional.ofNullable(instancesTypeCache.get("js"))
                    .orElse(instancesTypeCache.get("rjs")));

It doesn't look pretty but it works. It ensures that an optional is provided with rjs script evaluator.

abpai94 avatar Sep 05 '24 07:09 abpai94

I'm not sure this is worth it as this solution works fine:

<string><![CDATA[rjs:
              ...jscript_function...
              ]]></string>

rouazana avatar Sep 15 '24 15:09 rouazana

Its a minor change that added additional tolerance to detect inline javascript. Seems much cleaner to use regex to determine the JS engine from the prefix.

I made additional changes in the most recent commit that adds further tolerance to detect a variety of cases. Expected use case:

        <mainIdentifier><![CDATA[js:
           ...jscript...
          ]]></mainIdentifier>

Other use case 1:

        <mainIdentifier><![CDATA[
           js:
           ...jscript...
          ]]></mainIdentifier>

Other use case 2:

        <mainIdentifier>
          <![CDATA[js:
           ...jscript...
          ]]></mainIdentifier>

Other use case 3:

        <mainIdentifier>
          <![CDATA[
          js:
           ...jscript...
          ]]></mainIdentifier>

Other use case 4:

        <mainIdentifier><![CDATA[
           
           js:
           ...jscript...
          ]]></mainIdentifier>

Or any other variants that might be occur.

abpai94 avatar Sep 16 '24 08:09 abpai94

@rouazana If this seems unnecessary feel free to close off the issue. Found the lack of JS engine detection a minor inconvenience personally.

abpai94 avatar Sep 16 '24 14:09 abpai94

@rouazana I have implemented some tests and ameliorated the pattern compilation to reduce the overhead.

abpai94 avatar Sep 18 '24 15:09 abpai94

I close this PR as it has many conflicts and has been rewritten in #361

davidcoutadeur avatar Feb 25 '25 19:02 davidcoutadeur