AttributeFilter Review 21Jun

Object lifecycle

This is still not understood.  A question which illustrates this is “Do objects initialize/destroy their injected children or do they come preinitialized?”.  The question is not important (since init and destroy can be called multiple times), but it illustrates that this design is not bottomed out.  ACTION RDW ensure that there is a case in JIRA to capture this. STATUS: IDP-288 created.

The language

Significant discussion:

  • Can we fix it? no.
  • Can we think of a representation of a policy which makes this work easier and which the current language can be parsed into, leaving us the option of creating a better new language - May, possible but…
  • When it comes to it the language works for the task and implementation difficulties are always trumped by ease of deployment.

FilterContext

Check the annotations.  It appears that getRequestedAttribute is missing @Unmodifiable, further it might need to be made so (if feasible). ACTION RDW Jira or fix. STATUS: Work will be done under case idp-287.

AttributeFilter

Questions about tristating and the use of MatcherException.  Can we make tristate handling configurable? Answer: it is be better to configure the tristate away by effective PolicyRequirementRules and so the answer is what policy requirement rules are missing?

Why the odd initialization of attributeId? Answer: it predated @NonnullAfterInit. ACTION RDW: Fix. STATUS: fixed r4539

AttributeFilterPolicy

Why export isApplicable?  Why not just check applicability in apply()

Lines 179..183 achieve nothing and should probably be deleted.

ACTIONS RDW make these things so. STATUS: complete r4543 r4543

AttributeRule

Have we make this an interface and have Permit and Deny variants?  There are three options

  1. What we currently have.
  2. Use a boolean field to decide whether this is permit or deny.
  3. Use different classes.

ACTION RDW: look at each of these in the code (both implementation and parsing) and circulate the results. STATUS complete.

Matcher

Scott still unhappy with the shoe-horning of “once class two jobs”. STATUS see the updated design notes

We need to ensure that unmodifiable returns are  (and the static ones are not).  ACTION RDW do so or JIRA STATUS: complete r4544

AbstractComparisonMatcher

Does this become easier once we separate the two roles?  Then we can get rid of the injected inline predicates. Predicate is looking like an increasingly uncomfortable match for the comparison. Answer: RDW thinks so.  He needs to complete my other actions and then play with this.  STATUS new design thrashed out and is now in The design notes