groovy

AST Browser doesn't use same classloader as groovyConsole. loaded JARs aren't found in AST Browser

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7-beta-2
  • Fix Version/s: 1.7-rc-1
  • Component/s: Groovy Console
  • Labels:
    None
  • Environment:
    all
  • Number of attachments :
    1

Description

The GroovyConsole AST browser doesn't use the same classloader for the AST BRowser.

1. Open groovyConsole and import a JAR using the Script menu item.
2. Write a script that uses something out of that JAR
3. Open AST Browser
4. AST browser will show a ClassNodeFOund exception for anything out of the JAR

I suspect that the classes are being loaded into the classloader GroovyConsole uses to execute the script but not the classloader used to execute AST Browser.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Roshan Dawrani added a comment -

Hi Hamlet, attaching a patch here for review. Let me know if it seems ok. I have tested from UI with the steps you provided in the JIRA.

Show
Roshan Dawrani added a comment - Hi Hamlet, attaching a patch here for review. Let me know if it seems ok. I have tested from UI with the steps you provided in the JIRA.
Roshan Dawrani made changes -
Field Original Value New Value
Attachment 3833_v17x.txt [ 45484 ]
Hide
Permalink
Hamlet D'Arcy added a comment - - edited

This patch looks good... I just have one change I would make:

The classLoader parameter/field in ScriptToTreeNodeAdapter is optional... and then there is a block 

        if(!classLoader) {
            // AstBrowser has been launched independently and not from groovy console 
            classLoader = new GroovyClassLoader()
        }

I would prefer classLoader to be a final field and a required parameter, then it is up to the calling clients to specify the correct classloader. This way there is not a null value floating around the codebase. It is best to drive null out of our programs.

Show
Hamlet D'Arcy added a comment - - edited This patch looks good... I just have one change I would make: The classLoader parameter/field in ScriptToTreeNodeAdapter is optional... and then there is a block if (!classLoader) { // AstBrowser has been launched independently and not from groovy console classLoader = new GroovyClassLoader() } I would prefer classLoader to be a final field and a required parameter, then it is up to the calling clients to specify the correct classloader. This way there is not a null value floating around the codebase. It is best to drive null out of our programs.
Hide
Permalink
Roshan Dawrani added a comment -

Sure. I will make that change and commit then.

Thanks.

Show
Roshan Dawrani added a comment - Sure. I will make that change and commit then. Thanks.
Roshan Dawrani made changes -
Assignee Roshan Dawrani [ roshandawrani ]
Hide
Permalink
Roshan Dawrani added a comment -

Done.

Show
Roshan Dawrani added a comment - Done.
Roshan Dawrani made changes -
Resolution Fixed [ 1 ]
Fix Version/s 1.7-rc-1 [ 14666 ]
Status Open [ 1 ] Resolved [ 5 ]
Paul King made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

  • Assignee:
    Roshan Dawrani
    Reporter:
    Hamlet D'Arcy
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: