BeanLookupBindings for simpler script access to beans

[travisfw]

The new class BeanLookupBindings allows scripts to skip getBean calls.
Lines like 'beanname = appCtx.getBean("beanname")' can be left out.
Past scripts remain compatible.
This change effects action directory scripts and rest console scripts.

[travisfw]

Just scratching an itch.

[nlevitt]

What happens if you have a script with a line like
beanname = appCtx.getBean("beanname")
You say "Past scripts remain compatible", but in the code you commented, "Caveat: access is read only" and it does look like that put() method will throw an exception.

[gojomo]

Very interesting! I also wonder if the collision (assigning into a bean name) or surprise could cause problems for scripts/habits already developed... but even if it did, it might be worth the convenience, with fair warning/documentation.

For a particularly hard case, consider: what if a context had a bean already named 'rawOut' or 'appCtx'?

Might this be something a script could opt into with an early line... like the way in Beanshell the setAccessibility(true) invocation can make all protected/private members accessible from that point on?

[travisfw]

Good point Noah,
beanname = appCtx.getBean("beanname")
would have thrown an exception, breaking a lot of scripts.

Gordon, I took your idea of enabling bean lookup. A line like
beanBindings = true
should do the trick.

This should even work with a config that has a bean named 'beanBindings', though once beanBindings is set to true in the script it would be impossible to unset.

[nlevitt]

One other thought, just because requiring "beanBindings=true" makes the whole idea a little less appealing, since it conflicts with the goal of typing less hard to remember stuff. What if you didn't override put() at all but let it set a variable in the namespace even if it conflicted with a bean name. And then get() would first look in the explicit bindings (call super.get() first) and only if nothing is found, look for a bean by that name. Then
beanname = appCtx.getBean("beanname")
would work the same way it always has, but omitting it would work the way you have in mind.

In other words looking up variables as bean names would be a read-only fallback.

[travisfw]

Noah, that's what I originally considered doing, but if you set beanname to something other than appCtx.getBean("beanname") like maybe appCtx.getBean("beanname").getSomeProperty() then the script would still be broken. What's worse: silent enigmatic failure in a small number of cases or obvious documentented failure in a large number of cases?

An argument for having beanBindings enabled by default is that there isn't much systemic use of the script console that I'm aware of yet. Netarchive, anyway, is on H1. Our own 'useful scripts' page is probably the main corpus to support. Now's the time to make the change. One can always disable it too, but it's probably better for disabling the feature to be the obscure useful tip to be discovered on the mailing list vs enabling the feature to be rarely used.

[nlevitt]

On Jan 11, 2013 4:48 PM, "Travis Wellman" notifications@github.com wrote:

Noah, that's what I originally considered doing, but if you set beanname
to something other than appCtx.getBean("beanname") like maybe
appCtx.getBean("beanname").getSomeProperty() then the script would still be
broken.

How so?

[travisfw]

When you try to use beanname it would not be appCtx.getBean("beanname"). It would be something else...

[nlevitt]

If you set it explicitly to something else then yeah it will be that something else. Seems to me that's exactly what we want to maintain backward compatibility.

[travisfw]

Ok so you're suggesting do the bindings lookup first and if unset then do the bean name lookup? That makes sense...

[nlevitt]

Yeah

[travisfw]

I like that it's a shorter class now. Should we merge?