net.shibboleth.utilities.java.support.component.*

[Ian Young]

On 25 Feb 2014, at 20:53, Tom Zeller <tzeller at dragonacea.biz> wrote:

> Is there a compelling reason to provide a _protected_ setId() in
> AbstractIdentifiedInitializableComponent ?

I assume (I wasn't there, so I don't know) that it's protected so that concrete subclasses can choose whether to expose a public setId or not. In most cases, of course, they are choosing to have a public setId, but making setId public in that class would remove the choice.

One effect of that would be that all beans derived from that class would have writeable id properties, whether or not they are marked as IdentifiableComponent.

> It seems simpler to me if we make setId() public.

Removing choice always simplifies things, it's true. The question is whether that's a desirable simplification. It's not something we have discussed.

> I guess the idea was
> to "protect" the id, but we already do that somewhat by throwing an
> exception if the id is set after initialization.

That's an independent concern, I think. If you want a component that invents its own name, then you probably don't want it to be possible to override that either after *or before* initialization.

> Looking through our projects for use of the protected setId() yields a
> couple of cases in metadata.pipeline where it is overriden as public
> anyway,

Most of the aggregator components are intended to be user-configured as Spring beans, so they need settable ID properties. I think Rod has migrated that code to use the new abstract base class to tidy this up now.

> and the rest of the handful of calls are in constructors or
> via BeanNameAware.

It would be the ones in constructors that I'd be wary of. If a component's ID is being set in a constructor, I would think it would be likely that the intention would be to not allow it to be overridden later.

> My suggestion is that we make setId() public in
> AbstractIdentifiedInitializableComponent, and then
> AbstractIdentifiableInitializeableComponent will be unnecessary. That
> would mean a rename of AbstractIdentifiedInitializableComponent to
> AbstractIdentifiedIdentifiableInitializableComponent, for which I
> would be happy to entertain any other shorter name.
> 
> I hope this makes sense.

Going down this route is certainly going further than we discussed in the last couple of weeks. You're very close to proposing that we abolish the distinction between Identifiable and Identified entirely, which I think would be a mistake in principle even if it simplified things for the current code.

	-- Ian



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5943 bytes
Desc: not available
Url : http://shibboleth.net/pipermail/dev/attachments/20140227/d71bfbe7/attachment.bin