The context is in API and really, life is too short.

I made the obvious fix, tidied up logging and also remove a spurious fatal error that my own review spotted.

Less is always more and this is now much less than I originally had.

All our impl classes inherently depend on each other's contracts, but if it really bugs you, add a flag to the context class to track whether the node processor has to do work.

I had thought of that but I thought it might be a bit nauseating to others to do work based on knoweldge of how an impl class elsewhere has behaved...

I suppose one can rationalize it away (Generated metadata won't have a parent and metadata from a source will).

If you () don't mind this change I'll add it - and if hates it too much we can take it to the dev list

I bet you could fix that, just do a quick check of whether the ACS object has a parent (in which case it's from metadata).

Changes made as described above. IMO the code is much cleaner.

There is a small extra cost on each processing when the AssertionConsumerService does come from the Metadata, and had RequestedAttribute statements but they do not map onto any IdP attributes (because we cannot detect that the mapping has already happened)

In this case we will duplicate the attempt to map the attributes - this will be the cost of iterating across all the mappers (i.e attribute encoders) looking for a mapping for the RequestedAttribute in the AssertionConsumerService.

I don't think that the condition is common and the cost should be minimal, but it is a worrying O(n * m) complexity.

The "fix" would be to revert to the old code but that it architecturally sucky (IMO)