At our company we have well defined rules for annotating domain model. Checking if validation annotations match column definition is very laborious and boring task and besides that it's easy to overlook something. Such task is great candidate for automation. In this post I'll show you how to create FindBugs plugin that can do the task for you.
We will limit ourselves to 2 detectors. One will check if @Column that is nullable is not annotated with @NotNull or vice versa, and the other will check if nullable @Column is annotated @Empty.
Implementation
We will use Maven to build our project. Here is our project layout:- impl
- src
- main
- java
- pl.com.it_crowd.findbugs
- EntityAnnotationDetector
- NotEmptyInconsistency
- NotNullInconsistency
- resources
- findbugs.xml
- messages.xml
- pom.xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>pl.com.it-crowd.findbugs</groupId>
<artifactId>itcrowd-findbugs</artifactId>
<version>1.0.0-SNAPSHOT</version>
<packaging>jar</packaging>
<dependencies>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>findbugs</artifactId>
<version>2.0.1</version>
</dependency>
</dependencies>
</project>
In pom.xml all we need is to define dependency on findbugs. Unfortunately I couldn't find sources anywhere around in maven repositories, so you probably have to checkout source code from findbugs VCS.<FindbugsPlugin xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="findbugsplugin.xsd"
pluginid="pl.com.it_crowd.findbugs.NotNullInconsistency" provider="IT Crowd" website="http://itcrowd.pl/itcrowd-findbugs">
<Detector class="pl.com.it_crowd.findbugs.NotNullInconsistency" reports="NOT_NULL_INCONSISTENCY"/>
<Detector class="pl.com.it_crowd.findbugs.NotEmptyInconsistency" reports="NOT_EMPTY_INCONSISTENCY"/>
<BugPattern type="NOT_NULL_INCONSISTENCY" abbrev="NNI" category="CORRECTNESS"/>
<BugPattern type="NOT_EMPTY_INCONSISTENCY" abbrev="NEI" category="CORRECTNESS"/>
</FindbugsPlugin>
File findbugs.xml is main configuration file for the plugin. We register here our detectors and bug patterns.
If you wonder where te findbugsplugin.xsd is, check findbugs.jar.<?xml version="1.0" encoding="UTF-8"?>
<MessageCollection xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="messagecollection.xsd">
<Plugin>
<ShortDescription>FindBugs IT Crowd domain annotation checks Plugin</ShortDescription>
<Details>IT Crowd domain annotation checks</Details>
</Plugin>
<Detector class="pl.com.it_crowd.findbugs.NotNullInconsistency">
<Details>
NotNullInconsistency
</Details>
</Detector>
<Detector class="pl.com.it_crowd.findbugs.NotEmptyInconsistency">
<Details>
NotEmptyInconsistency
</Details>
</Detector>
<BugPattern type="NOT_NULL_INCONSISTENCY">
<ShortDescription>@NotNull inconsistency</ShortDescription>
<LongDescription>@NotNull used for nullable column or not-null column misses @NotNull annotation on {1}</LongDescription>
<Details>
<![CDATA[
<p>Nullable properties should not be annotated @NotNull, while not nullable properties should.</p>
]]>
</Details>
</BugPattern>
<BugPattern type="NOT_EMPTY_INCONSISTENCY">
<ShortDescription>@NotEmpty inconsistency</ShortDescription>
<LongDescription>@NotEmpty used for nullable column or not-null String/array column misses @NotEmpty annotation on {1}</LongDescription>
<Details>
<![CDATA[
<p>Nullable properties should not be annotated @NotEmpty, while not nullable String/array properties should.</p>
]]>
</Details>
</BugPattern>
<BugCode abbrev="NNI">NotNull Inconsistency</BugCode>
<BugCode abbrev="NEI">NotEmpty Inconsistency</BugCode>
</MessageCollection>
Inside messages.xml we give description to the entire plugin, every decorator and each bug that is reported by our plugin. Note that the order is important, first you define plugin, then all detectors, then bugPatterns and funally bugCodes. If you wonder where the messagecolection.xsd is , check findbugs.jar.package pl.com.it_crowd.findbugs;
import edu.umd.cs.findbugs.BugInstance;
import edu.umd.cs.findbugs.BugReporter;
import edu.umd.cs.findbugs.BytecodeScanningDetector;
import org.apache.bcel.classfile.AnnotationEntry;
import org.apache.bcel.classfile.Field;
import org.apache.bcel.classfile.FieldOrMethod;
import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.Method;
public abstract class EntityAnnotationDetector extends BytecodeScanningDetector {
// ------------------------------ FIELDS ------------------------------
protected BugReporter bugReporter;
protected boolean entity;
// --------------------------- CONSTRUCTORS ---------------------------
public EntityAnnotationDetector(BugReporter bugReporter)
{
this.bugReporter = bugReporter;
}
// -------------------------- OTHER METHODS --------------------------
@Override
public void visit(JavaClass obj)
{
for (AnnotationEntry entry : obj.getAnnotationEntries()) {
entity |= "Ljavax/persistence/Entity;".equals(entry.getAnnotationType());
}
super.visit(obj);
}
@Override
public void visit(Field obj)
{
if (!entity) {
return;
}
if (isInvalid(obj)) {
bugReporter.reportBug(new BugInstance(this, getBugType(), HIGH_PRIORITY).addClass(this).addField(this));
}
}
@Override
public void visit(Method obj)
{
if (!entity) {
return;
}
if (isInvalid(obj)) {
bugReporter.reportBug(new BugInstance(this, getBugType(), HIGH_PRIORITY).addClass(this).addMethod(this));
}
}
protected abstract String getBugType();
protected abstract boolean isInvalid(FieldOrMethod obj);
}
Our detectors have very similar logic so we will push it to abstract super class called EntityAnnotationDetector. Decorators must implement either Decorator or Decorator2 interface. There are several base classes that implement those interfaces. We will use BytecodeScanningDetector.There are tons of visit methods, so checkout the sources and see yourself what suits you best. We need
to visit class, field and method. Note that the same detector instance will be used for all classes. First the visit(JavaClass) method will be invoked, so we can see if the class is entity. If not then we will not do the checks inside visit(Method) and visit(Field).
As you can seem in FindBugs we don't operate on class types, but rather on strings.
If we find a bug then we report it using bugReporter, that was passed to the constructor (we should store it in some field).
We always have to invoke "addClass" on BugInstance. If the bug is associated with field we invoke "addField"; if bug is associated with method then we invoke "addMethod". There are appropriate "addXX" methods for other cases. This will allow users to get better coordinates of bug when reading report.
You may wonder why "addField" or "addMethod" accepts "this" which is our Detector. It's because BugInstance can extract currently processed member, line or whatever from PreorderVisitor (which our Detector implements).
package pl.com.it_crowd.findbugs;
import edu.umd.cs.findbugs.BugReporter;
import org.apache.bcel.classfile.AnnotationEntry;
import org.apache.bcel.classfile.ElementValuePair;
import org.apache.bcel.classfile.Field;
import org.apache.bcel.classfile.FieldOrMethod;
import org.apache.bcel.classfile.Method;
import org.apache.bcel.generic.Type;
public class NotEmptyInconsistency extends EntityAnnotationDetector {
// --------------------------- CONSTRUCTORS ---------------------------
/**
* Construct a ClassNodeDetector. The bugReporter is passed to the
* constructor and stored in a protected final field.
*
* @param bugReporter the BugReporter that bug should be reporter to.
*/
public NotEmptyInconsistency(BugReporter bugReporter)
{
super(bugReporter);
}
@Override
protected String getBugType()
{
return "NOT_EMPTY_INCONSISTENCY";
}
protected boolean isInvalid(FieldOrMethod obj)
{
boolean columnAnnotationPresent = false;
boolean notEmptyAnnotationPresent = false;
boolean notNullColumn = false;
for (AnnotationEntry entry : obj.getAnnotationEntries()) {
if ("Lorg/hibernate/validator/constraints/NotEmpty;".equals(entry.getAnnotationType())) {
notEmptyAnnotationPresent = true;
} else if ("Ljavax/persistence/Column;".equals(entry.getAnnotationType())) {
columnAnnotationPresent = true;
for (ElementValuePair pair : entry.getElementValuePairs()) {
notNullColumn |= "nullable".equals(pair.getNameString()) && "false".equalsIgnoreCase(pair.getValue().stringifyValue());
}
}
}
Type type;
if (obj instanceof Field) {
type = ((Field) obj).getType();
} else {
type = ((Method) obj).getReturnType();
}
boolean isString = "java.lang.String".equals(type.toString());
return columnAnnotationPresent && (notNullColumn && isString && !notEmptyAnnotationPresent || !notNullColumn && notEmptyAnnotationPresent);
}
}
NotEmptyInconsistency reports NOT_EMPTY_INCONSISTENCY (this corresponds to value in findbugs.xml and messages.xml). Let's discuss isInvalid method.
It accepts FieldOrMethod param. We iterate over it's AnnotationEntries which reflect Annotations (@Column,@NotNull,@MinMax,etc.). Unfortunately we cannot be type safe here and have to rely on strings here. Within the loop we see if there is @NotEmpty annotation on field or method and we check for presence of @Column annotation and its "nullable" attribute value. Finally we make decision if annotations are consistent or not.
Please note that @NotEmpty is also used for collections and arrays, so we should probably modify this method a bit.
NotNullInconsistency class is very similar, even simpler, because it doesn't need to check for method return value type or field type, so we will skip it here.
Testing
I've found that FindBugs is very immute to testing. It is full of final classes that can't be mocked with Mockito, so currently I have no idea how to do unit testing here. If you know how, please let me know. To work around this we will also have separate Maven module for tests:- tests
- src
- main
- java
- pl.com.it_crowd.findbugs
- NotEntity
- SampleEntity
- SampleEntity2
- SampleEntity3
- pom.xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>pl.com.it-crowd.findbugs</groupId>
<artifactId>tests</artifactId>
<version>1.0.0-SNAPSHOT</version>
<dependencies>
<dependency>
<groupId>javax.persistence</groupId>
<artifactId>persistence-api</artifactId>
<version>1.0</version>
</dependency>
<dependency>
<groupId>javax.validation</groupId>
<artifactId>validation-api</artifactId>
<version>1.0.0.GA</version>
</dependency>
<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-validator</artifactId>
<version>4.2.0.Final</version>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>2.5.2</version>
<configuration>
<plugins>
<plugin>
<groupId>pl.com.it-crowd.findbugs</groupId>
<artifactId>itcrowd-findbugs</artifactId>
<version>1.0.0-SNAPSHOT</version>
</plugin>
</plugins>
<visitors>NotNullInconsistency,NotEmptyInconsistency</visitors>
</configuration>
</plugin>
</plugins>
</build>
</project>
What is important here is the "plugins" in findbugs-maven-plugin configuration.
For testing purposes we want to limit number of Detectors used to our custom ones. For that we use "visitors" in maven plugin configuration.
That's it. Simple, isn't it? Yes, I know, "everything is simple once it stops being difficult". The code is available at https://github.com/it-crowd/itcrowd-domain-findbugs. I'm waiting for your comments.
No comments:
Post a Comment