Spring Annotations That Look More Like the Disease Than the Cure #Java

First off, I will preface this post with the caveat that I have a lot of respect for the Spring framework and the talented engineers who brought it to the community.  Given a blind choice between a Spring framework and a competing framework from another vendor, I think I would be much inclined to select Spring.  I understand the pressures that public companies are under and that they have to cater to silly forces and marketing campaigns to stay relevant and profitable.

I was watching a webinar today about changes in Spring Security 3.0/3.1, and I have to say that it provided me with one of the most readily explainable examples of what has been giving me uneasy feelings for some time now, namely the proliferation of Java annotations for imprudent uses.

Annotations are a great mechanism for providing cross-cutting functionality in a more readable and less invasive mechanism.  In many cases they enable such object-oriented principles as “favor composition over inheritance”.  Behaviors perpendicular to business logic such as logging, auditing, and transaction management can be indicated in a simple way which points to functionality defined and running elsewhere.  Some functionality is, in my humble opinion, clearly inappropriate in such a container.  For example, I find it a rather poor way to “externalize” configuration.  Your username, password, ports, and protocols for a 3rd party integration don’t belong in annotations.  Nor is there any value in defining algorithms or strategies in 20 lines of bizarre declarative annotations.  Code has its place as do annotations.

The specific example I saw in Spring Security that I am highlighting was the new annotations that support an expression language.  It is, in all actuality, the classic anti-pattern of a magic string.  Scripting code to support permission evaluations is written in yet-another-expression-language and placed directly alongside the code it influences.  The code is untyped, completely framework-proprietary, and brittle.  It doesn’t take much imagination or foresight to see the things that can go wrong, like a very simple refactoring not being reflected in annotation code.

So far I’ve been very abstract, so here’s what I’m talking about:

  @PreAuthorize("hasRole('ROLE_ADMIN') or hasRole('ROLE_EDITOR')")
  public void doPrivilegedStuff() {
     ...
  }

Users of dynamic languages who have fully bought into Test-Driven Development might not give this any thought. However, the reality of the Java applications I have worked with professionally is that unit testing is considered a luxury and 20-30% line test coverage is pretty remarkable. I do know this type of code (and it is CODE) hidden in a inline DSL is likely to suffer from typos and errors that are only caught at runtime. If you are especially unlucky, a typo might even make security silently allow access inappropriately.

I think a better approach would be something like:

  @PreAuthorize(AdminPermissionEvaluator.class)
  public void doPrivilegedStuff() {
     ...
  }

  public class AdminPermissionEvaluator implements PermissionEvaluator {

      boolean hasPermission(Authentication authentication, 
                            Object targetDomainObject, 
                            Object permission) {
          return isUserInRole(authentication, Roles.ADMIN) 
                  || isUserInRole(authentication, ROLES.EDITOR)
   }

  boolean hasPermission(Authentication authentication, 
                        Serializable targetId, 
                        String targetType, 
                        Object permission) {
      return isUserInRole(Roles.ADMIN) 
              || isUserInRole(Roles.EDITOR);
  }

  boolean isUserInRole(Authentication authentication, GrantedAuthority authority) {
      for(final GrantedAuthority authority : authentication.getAuthorities()) {
          if(authority.equals(authority)) {
              return true;
          }
      }
     return false;
  }


(This is example blog pseudo-code that has never been in an IDE or compiled, but you get the idea).

So, what did we give up and what did we gain? We gave up some brevity and completely inlining the permission evaluator. We gained: 1.) strongly-typed code, 2.) a reusable and testable permission evaluator, 3.) a guarantee that refactoring won’t trash our security, 4.) the ability to unit test and validate security at build time, 5.) death to magic strings, 6.) A separate class in which we could inject DAOs, external services, or integration code necessary to determine the specifics of fine-grained permissions.

My usage of Spring Security until now has basically consisted of HTTP basic authentication for web apps and REST services as well as basic role-based security where the roles are known and can be enumerated when the code is written. So, I am almost sure there is a way in Spring Security to do exactly what I am suggesting. What I take issue with is a framework purporting to be a better alternative to JavaEE providing functionality that will almost NEVER improve development and will probably actively harm a novice developer who uses it, especially in the context of long-term MAINTENANCE development. A framework should actively steer users toward smart solutions while still allowing them low-level access to interfaces, base classes, and configurations which can be harmful if used incorrectly. It shouldn’t just hand them the dangerous functionality as a heralded and celebrated feature.

It’s not just Spring Security going down this road either. Spring’s core functionality is starting to provide silly ways to do things with annotations also. Using Spring but without tainting every class in your application used to be something that the Spring community really preached. Now, everyone seems to be screaming “death to XML configuration” while taking 3 steps backwards in code maintainability. Just because you can make it fit into an annotation doesn’t mean you should. Hell, you could just mix metaphors and put a whole XML config file into the a String value of an annotation if you really want to make a screwed-up application.

I can’t help but think of flawed Consult-Use-Retire-Consult cycle that I have observed in organizations with insufficient critical thinking skills. A Consultant produces some software using some new “cutting edge” framework or technology, a client Uses it for a time and eventually finds it doesn’t meet their needs, a developer or support team is unable to make it work causing it to be Retired, and finally a Consultant is paid to create the same base functionality but with a larger feature set and a completely different design that will soon lead to another consultant starting from scratch. In other words, the only people that ever seem to profit from poorly maintainable code are hourly consultants and the companies that employ them!

In conclusion, I like Spring. It’s a cool set of frameworks that have done a lot of great things for Java developers. But, I can’t help but feel the organization may be starting to lose its way dumbing down development to the point of framework-endorsed worst practices. At the end of the day, good software usually isn’t all that sexy or easy and trying too hard to make it so usually points toward lack of maturity on the part of a development team.