Anotating Constructor Parameters (IDP-1047)

[Rod Widdowson]

I've been playing with this a bit;  I have the ParameterNameDiscoverer coded, I have worked out how to wire it into our stuff (not
checked in but it’s a some minor changes around the ApplicationContext classes we define), and I have also annotated up one class,
but I'd like to pause before the next stage and get some feedback.

Starting with the annotations:

https://git.shibboleth.net/view/?p=spring-extensions.git;a=blob;f=src/main/java/net/shibboleth/ext/spring/resource/FileBackedHTTPRes
ource.java

Has the annotations added.  I'd like feedback as to whether what is there looks reasonable.  This is a pretty busy class and so the
constructor declaration is getting pretty noisy;  I don't think that we have a choice but before I make further changes I want to
check what other think.

FWIW I chose this class in particular by arranging to fail any tests which exercises c:parameter="foo" and finds a class lacking the
annotation.  I am intending doing the same across all our tests (and in idp-testbed) as the driver for the next stage.

Next up is the implementation itself:

https://git.shibboleth.net/view/?p=spring-extensions.git;a=blob;f=src/main/java/net/shibboleth/ext/spring/util/AnnotationParameterNa
meDiscoverer.java

My question is how helpful/noisy do we want this class to be.

The code is currently set up to only care about "net.shibboleth.*" and "org.opensaml.*" classes, and it warns if it sees a
constructor used for one of them with no annotation.  

But is this too much?  Or too little?

Too much:
- It will warn on implementation classes as well as api classes (yes we could add a filter on the name but that’s getting icky)

- It will warn when called with numbering parameters (c:_0) (I don't think that this is fixable)

Too little:

- We could add extra fields to the annotation to mention where (in idp-conf) we use this constructor; then if we chose to rename a
parameter in an impl class we could be directed to the configuration changes needed.  I don't like this idea much since the
information will rot.

- We could hijack the use of this class to detect if we summon up a bean using a deprecated constructor - this might be a useful
service.  It would only work for constructors with parameters, which means that the coverage would be minimal.  It is also an abuse
of the API which makes me feel vaguely nauseous.  If we wanted to do this (and it is an interesting idea) I would imagine that there
is a better way of hooking into Spring (maybe the InstantationStrategy?)

Thoughts?

R