Skip to content

Add support for Kotlin JSR223 scripts #2898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 19, 2019

Conversation

artembilan
Copy link
Member

  • Add required Kotlin dependencies into the spring-integration-scripting
    module
  • Introduce KotlinScriptExecutor to interact with the
    KotlinJsr223JvmLocalScriptEngineFactory directly since there is no
    META-INF/services/javax.script.ScriptEngineFactory file in the Kotlin
  • Also set an idea.use.native.fs.for.win system property to false in
    this class to disable check for native support on Windows.
    (Might be removed in future Kotlin versions)
  • Move ScriptParser.getLanguageFromFileExtension() logic into the
    ScriptExecutorFactory.deriveLanguageFromFileExtension() since the same
    one must be applied in the DslScriptExecutingMessageProcessor, too.
  • Modify tests to reflect Kotlin support
  • Fix some test scripts to their official extensions

* Add required Kotlin dependencies into the `spring-integration-scripting`
module
* Introduce `KotlinScriptExecutor` to interact with the
`KotlinJsr223JvmLocalScriptEngineFactory` directly since there is no
`META-INF/services/javax.script.ScriptEngineFactory` file in the Kotlin
* Also set an `idea.use.native.fs.for.win` system property to `false` in
this class to disable check for native support on Windows.
(Might be removed in future Kotlin versions)
* Move `ScriptParser.getLanguageFromFileExtension()` logic into the
`ScriptExecutorFactory.deriveLanguageFromFileExtension()` since the same
one must be applied in the `DslScriptExecutingMessageProcessor`, too.
* Modify tests to reflect Kotlin support
* Fix some test scripts to their official extensions
Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small issue.

* @since 2.1
*/
public abstract class ScriptExecutorFactory {
public final class ScriptExecutorFactory {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a private CTOR.

return engine.getFactory().getLanguageName();
}

private ScriptExecutorFactory() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a private CTOR

Doesn't this work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry; missed that 😦

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it needs super() to prevent Sonar from complaining.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Will add. Can we tweak Sonar do not complain on the matter?
There is definitely a super() in the target byte code, so it's complaining is pointless.

Or do I miss something else ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding super is a no-op - the compiler generates one anyway.

I don't think we want to disable empty block detection.

* Polishing according Sonar objections
@artembilan
Copy link
Member Author

garyrussell approved these changes 34 minutes ago

Are you going to merge this? Or have you let it to me to squash and rebase?

Thanks

@garyrussell
Copy link
Contributor

I was waiting for our friend Travis - merging now.

@garyrussell garyrussell merged commit 7dff1d5 into spring-projects:master Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants