TINKERPOP-3040 Fixed limitation in multi-line parsing. #3162
Conversation
| class RemoteParser implements Parsing { | ||
|
|
||
| // Try to compile the script | ||
| def config = new CompilerConfiguration() |
There was a problem hiding this comment.
Nit: just for consistency
| def config = new CompilerConfiguration() | |
| private final CompilerConfiguration config = new CompilerConfiguration(); |
| def config = new CompilerConfiguration() | ||
|
|
||
| // Create a custom GroovyClassLoader that doesn't write class files | ||
| def gcl = new GroovyClassLoader(this.class.classLoader, config) { |
There was a problem hiding this comment.
Nit: just for consistency
| def gcl = new GroovyClassLoader(this.class.classLoader, config) { | |
| private final GroovyClassLoader gcl = new GroovyClassLoader(this.class.classLoader, config) { |
| * the Groovy compiler to try to compile the script and analyzes any compilation errors to determine if the script is | ||
| * incomplete. It does not evaluate the script, which prevents local execution and potential local failure. | ||
| */ | ||
| class RemoteParser implements Parsing { |
There was a problem hiding this comment.
Nit: RemoteParser name to me indicates that parsing is happening remotely but the reality is the opposite. How about something like LocalSafeParser?
| break | ||
| } | ||
| throw t | ||
| } catch (MissingPropertyException mpe) { |
There was a problem hiding this comment.
This MissingPropertyException handling was not ported into the new RemoteParser, was that intentional?
There was a problem hiding this comment.
you can't have a MissingPropertyException anymore because this change stops doing a local evaluation of the code in the interpreter. now it just parses for correctness in the script itself.
|
|
||
| switch (status.code) { | ||
| case ParseCode.COMPLETE: | ||
| if (!Boolean.valueOf(getPreference(INTERPRETER_MODE_PREFERENCE_KEY, 'false')) || isTypeOrMethodDeclaration(current)) { |
There was a problem hiding this comment.
Does interpretorMode no longer need to be specifically handled with the new RemoteParser?
There was a problem hiding this comment.
I sense this whole section of code i removed was all unnecessary. Note the interpreterMode tests are still passing so I think it's ok.
Added a custom parser to help better detect multi-line inputs that does not involve actual evaluation of the input string which can result in scenarios where the script is not sent to the server.
spmallette
force-pushed
the
TINKERPOP-3040
branch
from
e93f8bf to
4d20a24
Compare
https://issues.apache.org/jira/browse/TINKERPOP-3040
Added a custom parser to help better detect multi-line inputs that does not involve actual evaluation of the input string which can result in scenarios where the script is not sent to the server.
VOTE +1