porting spring wiring to v3 : rewrite or copy-paste ?

[Tom Zeller]

In general, I am looking for agreement that we should follow a
copy-paste strategy for porting the spring wiring from v2 to v3. When
I say "copy-paste" I also imply "least number of changes".

Originally, I was leaning more towards rewrite because I wanted to
really reduce the number of spring wiring classes in v3 compared to
v2. Seemed like so much noise. However, I think the logical simplicity
of the v2 pattern is good. So, I hope we can continue the v2 pattern
and copy-paste with appropriate minor cleanup for v3.

With the v2 spring extensible xml authoring, custom xml configuration
elements have a corresponding BeanDefinitionParser and FactoryBean.
Most FactoryBeans should not need to be ported to v3, a real win.

A BeanDefinitionParser is responsible for populating bean properties
from xml elements. I think we can copy-paste these from v2 with
cleanup of support classes, for example, refactoring from
SpringSupport to ElementSupport, etc.

For every xml namespace, a NamespaceHandler is responsible for mapping
an xml element to a BeanDefinitionParser. The content of a
NamespaceHandler follows a simple pattern :

   registerBeanDefinitionParser("SomeElement", new SomeBeanDefinitionParser());

One option, now being un-suggested by me, is to create a
BaseBeanDefinitionParser to reduce the number of BeanDefinitionParser
classes ported to v3. If so, the NamespaceHandler contents would
follow a less simple pattern, and would have more logic, for example,
a simple case :

  registerBeanDefinitionParser("SomeElement", new
BaseBeanDefinitionParser(Some.class));

  BeanDefinitionParser attributeResolverParser =
    new ParseCustomChildElementsBeanDefinitionParser("DataConnector",
"AttributeDefinition");
  registerBeanDefinitionParser("AttributeResolver", attributeResolverParser);

The reason I do not suggest the above is because someone new to the
spring wiring might look for an AttributeResolverBeanDefinitionParser
class, but would never find it. Instead, they would need to find
java-identity-provider -name '*.java' -exec grep -Hn AttributeResolver
{} \;. Or just look in the AttributeResolverNamespaceHandler.

In general, here is the v2 spring wiring pattern :

1. Every custom xml configuration element maps to an appropriately
named BeanDefinitionParser, for example, <SomeElement /> and
SomeElementBeanDefinitionParser.
2. Custom xml configuration elements which use schema type should be
represented by a base BeanDefinitionParser.

So, given service.xml :

<srv:Services ...>
  <srv:Service id="fooService" xsi:type="ShibbolethAttributeResolver" >
    <srv:ConfigurationResource file="attribute-resolver.xml" />
  </srv:Service>
</srv:Services>

and attribute-resolver.xml

<res:AttributeResolver>
  <res:DataConnector id="..." xsi:type="SimpleDataConnector" />
  <res:AttributeDefinition id="..." xsi:type="SimpleAttributeDefinition "/>
</res:AttributeResolver>

the spring wiring would contain :

net.shibboleth.idp.spring
 ServicesBeanDefinitionParser
 BaseServiceBeanDefinitionParser
 BaseResourceBeanDefinitionParser

net.shibboleth.idp.attribute.resolver.spring
  AttributeResolverBeanDefinitionParser
  AttributeResolverServiceBeanDefinitionParser

net.shibboleth.idp.attribute.resolver.spring.ad
  BaseAttributeDefinitionBeanDefinitionParser
  SimpleAttributeDefinitionBeanDefinitionParser

net.shibboleth.idp.attribute.resolver.spring.dc
  BaseDataConnectorBeanDefinitionParser
  SimpleDataConnectorBeanDefinitionParser

Notice I typed BaseResourceBeanDefinitionParser and not
BaseConfigurationResourceBeanDefinitionParser. We might consider
changing <srv:ConfigurationResource /> to <srv:Resource /> one day,
with backwards compatibility. This is all because we have a Resource
class, but not a ConfigurationResource.

I also typed AttributeResolverServiceBeanDefinitionParser and not
ShibbolethAttributeResolverServiceBeanDefinitionParser, because there
is an AttributeResolverService class.

Anyone read this far and have opinions on refactoring
"BeanDefinitionParser" to "Parser" ?

Make sense ?

Feedback is welcome, here or on the call tomorrow so we can roll.