OpenSAML dependency on Velocity breaks application

[Tom van den Berge]

On Wed, May 2, 2012 at 12:07 AM, Brent Putman <putmanb at georgetown.edu>wrote:

>
>
> On 5/1/12 3:21 PM, Tom van den Berge wrote:
> > Hi,
> >
> > After doing some research, I found that there is a way to use multiple
> > instances of Velocity. It's actually pretty simple. It is explained
> > here:
> http://velocity.apache.org/engine/releases/velocity-1.7/developer-guide.html#separate
> >
> >
>
>
>
> You know, we actually already use injected instances of VelocityEngine
> on the things that use Velocity templates.  We actually don't ever use
> what they call there the "singleton model".  So to be honest, I'm not
> actually sure why we are doing the global Velocity static init like
> that.


You are using the singleton model in DefaultBootstrap: "Velocity.init();".
Maybe you are using the separate instances model when you are using
velocity templates, but not when initializing.


> It might be that newly created VelocityEngine instances just
> inherit the properties from the global Velocity singleton if they are
> otherwise unspecified, so maybe that is the (not very good) reason.  If
> that is the case, we can probably just get rid of the Velocity singleton
> init entirely, and just make sure to move those properties into the
> Velocity Engine instance that is used in the encoders by, for example,
> the Shibboleth IdP.
>

I'm not sure what you mean with you last phrase, but that's probably
because I'm not very familiar with OpenSAML. I would say if OpenSAML needs
Velocity, it needs to take care of its own initialization, and obviously
not break any application or library that is using OpenSAML.

>
> I'll do some more research to confirm.  If accurate, I believe we might
> be able to do that in a patch release, as it involves no interface
> additions or changes.
>

That would be great!

>
> Of course, not to point out the obvious, but I believe your app could
> also do the same thing, and avoid being stepped on by the current
> behavior of OpenSAML, by using the "separate instance model".  As a
> short-term workaround.
>

That would definitely be an option. However, I already chose to overwrite
DefaultBoostrap, and leave out the velocity initialization, since I dont'
need it.

>
>
>
> >
> > Unfortunately overriding initializeVelocity doesn't work since it is a
> > static method, so the only option seems to be to override bootstrap()
> > and leave out the initializeVelocity() method. Sadly, this also means
> > that the static String[] xmlToolingsConfigs must be copied.
>
>
> Yeah, you're right.  Java static methods can't be overridden.  I
> probably knew this at one point and just forgot.  ;-)  I thought we were
> doing this in a test bootstrap for our unit tests, but turns out it's
> not really inheritance we were doing.  I was misremembering.
>
>
> --
> To unsubscribe from this list send an email to
> dev-unsubscribe at shibboleth.net
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://shibboleth.net/pipermail/dev/attachments/20120502/531eb27a/attachment.html