Enhance Groovy classloader experience

[niloc132]

Introduces new features when using Groovy as a console language. This
should not remove or change existing features at all, but only serves
to backfill the removed source() and sourceOnce() functions with an
approach that encourages users to write real groovy scripts.

Makes it possible to load .groovy files from the classpath (or a custom
classloader) as classes. Standalone groovy scripts provide a static
main() method (and an instance run() method) to invoke one or more
times, and classes can be reimported to read fresh from disk. Includes
a feature where groovy files loaded from the classpath can use
Deephaven default imports, and can inherit imports from the console,
both disabled by default.

Replaces a list of import statements with the Groovy ImportCustomizer,
so that evaluated code doesn't have a variable number of prefix lines.
This has the advantage of also being applicable to scripts loaded from
the classpath.

Restored unit tests lost when migrating to the deephaven-core codebase,
which helped to validate some of these cases, and verify
console<->querylanguage classloader interactions.

[niloc132]

Note for reviewers (or future me, who forgot this fact again): SomeClass.class.getClassLoader() is only the correct answer when trying to say "load a thing from the same classloader that this class came from", usually for something like getting a resource known to be in the same jar as this class. The context classloader is almost always preferred for "load with the classloader currently being used", since this lets a caller replace the current classloader with a more specific one (that probably points to getClass().getClassLoader() as a parent or other ancestor, so the same class can still be loaded).

[niloc132]

As above, a custom classloader installed by the app before calling this wouldn't necessarily be called when a class needs to be loaded in this way. Between this and the above change, not only does groovy get a chance to compile if needed, but a custom classloader can also be used.

[niloc132]

Reviewer note: these could be made static again if the classloader was passed as an argument, it is needed in loadClass.

[niloc132]

Reviewer note: I made this a list of class literals to assist in future refactoring, but can revert to just putting a list of strings for each of these.

[nbauernfeind]

You could introduce a helper method to invoke getName automatically, but this doesn't look like it will be gross to maintain as is =P.

[niloc132]

I'm not sure how we got away with not closing the exec context before, are we possibly just opening other context on top of it and never checking that none were leaked?

[nbauernfeind]

Removing is helpful to ensure that we are setting it before every test that needs it. It is otherwise not an error to not "close" an open execution at this time. Thanks for finding that it was missing here!

[niloc132]

Here's an extremely hacky example classloader that produces groovy files on the fly, which could easily be a blocking call to read network/table data. Set this up before DHC has started, so that it will be used to load groovy files as requested by the DHE groovy IDE.

        final Path dir = Files.createTempDirectory("dhc");

        Thread.currentThread().setContextClassLoader(new ClassLoader(Thread.currentThread().getContextClassLoader()) {
            @Nullable
            @Override
            public URL getResource(String name) {
                // if this is a groovy file, pretend to load contents remotely, and return a file: URL
                // to it, otherwise groovy won't parse it.
                if (name.startsWith("notebook/")) {// be sure not to take over deephaven/groovy/java files
                    Path path = Path.of(name);
                    String packageString = StreamSupport.stream(path.getParent().spliterator(), false)
                            .map(Path::toString).collect(Collectors.joining("."));
                    Path fileName = path.getParent().relativize(path);
                    if (fileName.endsWith("Script.groovy")) {// this check doesn't make sense for a real use case, but
                                                             // we're just going to make a fake file
                        Path result = dir.resolve(path);
                        try {
                            if (!Files.exists(result)) {
                                Files.createDirectories(result.getParent());
                                System.out.println(result);
                                Files.writeString(result, "package " + packageString + "\nprintln 'hello, world'\n");
                            }
                            return result.toFile().toURL();
                        } catch (IOException e) {
                            // instead of failing, possibly log, then pass the buck to the next link in the chain
                        }
                    }
                }

                return super.getResource(name);
            }
        });

[nbauernfeind]

Removing is helpful to ensure that we are setting it before every test that needs it. It is otherwise not an error to not "close" an open execution at this time. Thanks for finding that it was missing here!

[nbauernfeind]

io.deephaven? we probably don't want to reference illumon in these tests

[niloc132]

Sure, but the point is that the package does not exist...

[nbauernfeind]

Technically, io.deephaven.foo.bar does not exist. I'm just considering all of the effort we went through to remove references to illumon and db, for example.

[nbauernfeind]

I don't understand why you're addParam on each test - in this particular case you're setting z but looks unused. Why not set all parameters once at the top of the method?

[niloc132]

Agreed, this seems a bit off, but is part of the original test from DHE. I tried to keep my changes here limited, ensuring that except where I explicitly changed something, the same things that pass for DHE passes in DHC.

[nbauernfeind]

I'd collapse this with the line above to reduce a level of indentation:

} else if (classExists(body)) {
// ...
} else {
// ...
}

[nbauernfeind]

I would collapse this if with else above.

[niloc132]

Or drop the else entirely and just return once? no opinions offered either way, so I'm going to apply that change and your suggestions...

[nbauernfeind]

You could introduce a helper method to invoke getName automatically, but this doesn't look like it will be gross to maintain as is =P.

[rbasralian]

do we still need this? thought it was deprecated

[niloc132]

Added for test classpaths only, as an example used in DHE tests for groovy parameter disambiguation hit one of these methods.

[rbasralian]

Does this PR close #230?

[niloc132]

No, just moves it to here rather than a config entry.

[devinrsmith]

LGTM, leaving approval for others.

[deephaven-internal]

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/3304
Reference: https://github.com/deephaven/deephaven.io/issues/3303