Change signatures of PrePostInvocationAttributeFactory to use String arguments rather than annotations

Details

Description

Hi,

For the next release of Cibet control framework (www.logitags.com/cibet) I have integrated Spring Security authorization. It was possible but I had to do some 'dirty' hacks which could be avoided if Spring Security provides some very small changes:

Again the MetadataSource: My ConfigAttributes come from another source. In my implementation of MethodSecurityMetadataSource I have to instantiate PreInvocationExpressionAttribute and PostInvocationExpressionAttribute.

Problem: PreInvocationExpressionAttribute and PostInvocationExpressionAttribute classes are not public and have no public constructors.
Workaround: Create own Attribute classes in package org.springframework.security.access.expression.method which inherit from the two Pre and Post classes.
Solution: Make classes and constructor of PreInvocationExpressionAttribute and PostInvocationExpressionAttribute public

Activity

An alternative here might be to alter the signature of PrePostInvocationAttributeFactory, making it non-annotation specific, thus allowing the metadata to be sourced from elsewhere. Alternatively, extra methods could be added to ExpressionBasedAnnotationAttributeFactory. I don't want to expose all the internal implementations though.

Luke Taylor
added a comment - 08/Nov/10 10:06 AM An alternative here might be to alter the signature of PrePostInvocationAttributeFactory, making it non-annotation specific, thus allowing the metadata to be sourced from elsewhere. Alternatively, extra methods could be added to ExpressionBasedAnnotationAttributeFactory. I don't want to expose all the internal implementations though.

That surely would mean bigger code changes for you especially changing method signatures is a big thing. On the other hand, the other implementations of ConfigAttribute are public, only the ExpressionBased classes are not. But I can understand when you want prevent a too tight coupling to your code.

Wolfgang Winter
added a comment - 08/Nov/10 11:40 AM That surely would mean bigger code changes for you especially changing method signatures is a big thing. On the other hand, the other implementations of ConfigAttribute are public, only the ExpressionBased classes are not. But I can understand when you want prevent a too tight coupling to your code.

I've decided to go with modifying the signature. There's no real reason why it needs to take annotations as the arguments. It should be a simple change for implementers, since the attributes only have a String value anyway. It will be more flexible to just use Strings directly.

Luke Taylor
added a comment - 05/Jan/11 8:35 AM I've decided to go with modifying the signature. There's no real reason why it needs to take annotations as the arguments. It should be a simple change for implementers, since the attributes only have a String value anyway. It will be more flexible to just use Strings directly.

Luke Taylor
added a comment - 05/Jan/11 9:41 AM Done. This will have a minor impact on anyone with a custom PrePostInvocationAttributeFactory implementation, but I suspect there are very few of those out there.