code review planning

[Rod Widdowson]

> It seems that this paragraph is embodied in the Matcher component; it
> consolidates both concerns in a single component. Could you explain a
> bit further why it's valuable or necessary to consolidate these two
> orthogonal (and admittedly related) concerns?

It's driven by the language.  All comparisons are done by things the schema
calls MatchFunctors (which are represented as the Matcher interface).

In the schema a PolicyRequirmentRule is defined by a MatchFunctor but so is
a PermitValueRule, and so anything which can fit into one has to be
shoe-horned into the other.

This does make for quite a natural filtering language, but it makes
implementation a bit painful by times since things like (for instance)

<AttributeFilterPolicy id="targettedValueInEPA">
	<PolicyRequirementRule xsi:type="basic:ANY" /> 
          	<AttributeRule attributeID="eduPersonAffiliation">
		<PermitValueRule xsi:type="basic:AttributeValueString"
value="jsmith" attributeId="uid" ignoreCase="true"/>
          	</AttributeRule>
<AttributeFilterPolicy/>

Is an acceptable equivalent to the more natural  (to my mind)

<AttributeFilterPolicy id="targettedValueInEPA">
	<PolicyRequirementRule xsi:type="basic:AttributeValueString"
value="jsmith" attributeId="uid" ignoreCase="true"/>
          	<AttributeRule attributeID="eduPersonAffiliation">
		<PermitValueRule xsi:type="basic:ANY"/>
          	</AttributeRule>
<AttributeFilterPolicy/>

As an aside if you look in
idp-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/
impl/complex/, there are explicit tests for the odd usage patterns.

And they get even odder, what do you suppose:

	<PolicyRequirementRule xsi:type="basic:AttributeValueString"
value="jsmith" ignoreCase="true"/>

Would be useful for?  It's a valid construction...
 
> I believe part of my befuddlement may be due to naming. Matcher seems to
> be a strategy pattern for application of policy, and the mixed language
> "policy" and "rule" is further confusing.

Guilty as charged.  Choice of the correct nouns in vital in a situation like
this and it is not necessarily one of my strengths.  As for befuddlement I
don't think anyone has ever looked at this code and grocked it straight off.
It took me several weeks to understand the subtleties and Scott was still
correcting me at that point.

.Rod