[GROOVY-4529] GroovyClassLoader script caching not working properly if scripts not in root dir

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.7.1
• Fix Version/s: 1.9-beta-4, 1.8.4
• Component/s: class generator
• Environment: Windows Vista
• Assignee: Guillaume Sauthier
• Reporter: Michal Rembiszewski
• Created: 24/Nov/10 09:53
• Updated: 14/Oct/11 00:28
• Resolved: 12/Oct/11 10:04

Description

GroovyClassLoader caches class files compiled from scripts not in root dir under keys not matching original name.

GroovyClassLoader loader = new GroovyClassLoader();
loader.addURL(myScriptRoot);

Class class1, class2;

try {
  class1 = loader.loadClass("test/script1");
  class2 = loader.loadClass("test/script1");
  assertSame(class1, class2);  // this fails
} catch (ClassNotFoundException e) {
  fail("Class node found!");
}

If I write as above, class is cached by GroovyClassLoader under key "test.script1" so looking it up under "test/script1" fails. If I try with loader.loadClass("test.script1") it will be cached under "test.test" which seems even worse.

Is there any workaround other than creating my own cache for compiled classes?

As a fix in the groovy code, we could move the line

String className = name.replace('/', '.');

to the beginning of the loadClass(String, boolean, boolean, boolean) method and using it instead of name in the entire method. GroovyResourceLoader will handle dots fine and className will be proper cache key then.

This would fix looking up scripts with '/' as package separator. It is quite possible this is not recommended way of loading scripts anyway. To fix it for dot separated script paths the problem lies I believe here (GroovyClassLoader):

    protected Class recompile(URL source, String className, Class oldClass) throws CompilationFailedException, IOException {
        if (source != null) {
            // found a source, compile it if newer
            if ((oldClass != null && isSourceNewer(source, oldClass)) || (oldClass == null)) {
                sourceCache.remove(className);
                return parseClass(source.openStream(), className);
            }
        }
        return oldClass;
    }

parseClass second argument is fileName and we pass dot-separated className here. I would change it to
return parseClass(source.openStream(), className.replace('.','/'));