groovy
  1. groovy
  2. GROOVY-3946

Allow overriding the default Groovy class loader

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.6.7
    • Fix Version/s: 1.6.8, 1.7.1, 1.8-beta-1
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      2

      Description

      I am using Groovy with JSR-223 and I need to override the default Groovy class loader so I can provide a custom CompilationUnit with an additional PrimaryClassNodeOperation in the SEMANTIC_ANALYSIS phase.

      I don't know too much about Groovy internals, but as GroovyScriptEngineImpl's loader field is private and never gets reassigned, the only way I found for making it work is using reflection:

      private void overrideDefaultGroovyClassLoader(final ScriptEngine engine) throws Exception {
        final Field classLoader = engine.getClass().getDeclaredField("loader");
        classLoader.setAccessible(true);
        classLoader.set(engine, new CustomGroovyClassLoader());
      }
      

      If there is not a better way for doing this today, maybe GroovyScriptEngineImpl should provide a setter or a post construct-like callback method for allowing overriding such field.

      1. GroovyScriptEngineImpl.java
        18 kB
        Tiago Fernandez
      2. JSR223SecurityTest.java
        5 kB
        Tiago Fernandez

        Activity

        Hide
        Guillaume Laforge added a comment -

        You can provide a little patch for opening up the implementation (ie. providing getter/setter for the loader), if you wish.
        Ideally, even a sample test case showing how you're doing this class shuttering may be interesting, and would cover that part of the code and show people how they can realize that as well.

        Show
        Guillaume Laforge added a comment - You can provide a little patch for opening up the implementation (ie. providing getter/setter for the loader), if you wish. Ideally, even a sample test case showing how you're doing this class shuttering may be interesting, and would cover that part of the code and show people how they can realize that as well.
        Hide
        Tiago Fernandez added a comment -

        Thanks for your support Guillaume. I am attaching the JUnit test case using both reflection and injection for overriding the default class loader. The patch is pretty simple, I have just added getter/setter for the GroovyClassLoader member variable. Cheers.

        Show
        Tiago Fernandez added a comment - Thanks for your support Guillaume. I am attaching the JUnit test case using both reflection and injection for overriding the default class loader. The patch is pretty simple, I have just added getter/setter for the GroovyClassLoader member variable. Cheers.
        Hide
        Guillaume Laforge added a comment -

        A proper "patch" file would be better the next time
        Do you mind if instead I provide a getter only, but an additional constructor that would let you specify the classloader?
        The reasoning behind that would be to avoid people from change the classloader in the middle of the usage of the engine, that could lead to weird behaviour.
        So if one sets the load at construction time, you've got the same ability to customize it, but at least, we prefernt any weird behaviour in case someone inadvertantly change the classloader at a later stage.
        What do you think?

        Show
        Guillaume Laforge added a comment - A proper "patch" file would be better the next time Do you mind if instead I provide a getter only, but an additional constructor that would let you specify the classloader? The reasoning behind that would be to avoid people from change the classloader in the middle of the usage of the engine, that could lead to weird behaviour. So if one sets the load at construction time, you've got the same ability to customize it, but at least, we prefernt any weird behaviour in case someone inadvertantly change the classloader at a later stage. What do you think?
        Hide
        Tiago Fernandez added a comment - - edited

        Good point, passing the class loader in the engine's construction would avoid the weird behavior you mentioned. The only thing is, by the time I get the concrete implementation from the ScriptEngineManager the GroovyScriptEngineImpl class is already instantiated. In this case what would be the solution, how would I tell to the GroovyScriptEngineFactory it should instantiate the GroovyScriptEngineImpl with my custom GroovyClassLoader?

        PS: Sorry about the proper patch, I never submitted anything to open-source projects rather than logging bugs. Let me know what you expect next time (tiago182 at gmail dot com).

        Show
        Tiago Fernandez added a comment - - edited Good point, passing the class loader in the engine's construction would avoid the weird behavior you mentioned. The only thing is, by the time I get the concrete implementation from the ScriptEngineManager the GroovyScriptEngineImpl class is already instantiated. In this case what would be the solution, how would I tell to the GroovyScriptEngineFactory it should instantiate the GroovyScriptEngineImpl with my custom GroovyClassLoader? PS: Sorry about the proper patch, I never submitted anything to open-source projects rather than logging bugs. Let me know what you expect next time (tiago182 at gmail dot com).
        Hide
        Guillaume Laforge added a comment -

        Ah true, I had forgotten that that's the GSEFactory does instanciate the engine, bad luck.
        On the other hand, as you're doing already something specific with the engine, you could decide to bypass the factory
        But well, we'll go with the getter/setter, even if it's not ideal.
        Anyway, someone using those methods means that he's using the concrete implementation directly (ie. through a cast), and that means he should be aware of the risks!
        So I'll go with getter/setter.

        As for contributing code, you can use diff / patch to provide patches, or also IDEs these days provide these facilities.
        The change being trivial (getter/setter) + a new test case (that I'll have to adapt since we're not using JUnit annotations) are not complicated to apply anyway, so it's not critical.
        I think we've got some guidelines on the wiki, I'll have to see if they properly explain how users can contribute.

        Thanks again for your help on this!

        Show
        Guillaume Laforge added a comment - Ah true, I had forgotten that that's the GSEFactory does instanciate the engine, bad luck. On the other hand, as you're doing already something specific with the engine, you could decide to bypass the factory But well, we'll go with the getter/setter, even if it's not ideal. Anyway, someone using those methods means that he's using the concrete implementation directly (ie. through a cast), and that means he should be aware of the risks! So I'll go with getter/setter. As for contributing code, you can use diff / patch to provide patches, or also IDEs these days provide these facilities. The change being trivial (getter/setter) + a new test case (that I'll have to adapt since we're not using JUnit annotations) are not complicated to apply anyway, so it's not critical. I think we've got some guidelines on the wiki, I'll have to see if they properly explain how users can contribute. Thanks again for your help on this!

          People

          • Assignee:
            Guillaume Laforge
            Reporter:
            Tiago Fernandez
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: