Unit testing Java data classes immutability with the Mutability Detector

Updated post can be found on my new blog site.

In all our project, we use data classes which, by definition, contain data (fields) but no (business) logic.

According to the best coding practices, a data class should preferably be immutable because immutability means thread safety. Main reference here is Joshua Bloch’s Effective Java book; this Yegor Bugayenko’s post is also very interesting reading.

An immutable class has several interesting properties:

  • it should be not sub-classable (i.e. it should be final or it should have a static factory method and a private constructor)
  • all fields should be private (to prevent direct access)
  • all fields should be written once (at instance creation time) (i.e. they should be final and without setters)
  • all mutable type (like java.util.Date) fields should be protected to prevent client write access by reference

An example of immutable class is the following:

public final class ImmutableBean {

private final String aStr;
private final int anInt;

public ImmutableBean(String aStr, int anInt) {
this.aStr = aStr;
this.anInt = anInt;
}

public String getAStr() {
return aStr;
}

public int getAnInt() {
return anInt;
}
}

Note: as frequent in Java, there is a lot of boilerplate code which hides the immutability definitions.

Libraries like Project Lombok makes our life easier because we can use the @Value annotation to easily define an immutable class as follows:

@Value
public class LombokImmutableBean {
String aStr;
int anInt;
}

which is a lot more more readable.

Should we (unit) test a class to check its immutability?

In a perfect world, the answer is no.

With the help of our preferred IDE automatic code generation features or with libraries like Lombok it is not difficult to add immutability to a class.

But in a real world, human errors can be happen, when we create the class or when we (or may be a junior member of the team) modify the class later on. What happen if a new field is added without final and a setter is generated by using IDE code generator? The class is no more immutable.

It is important to guarantee that the class is and remains immutable along all project lifetime.

And with the help of the Mutability Detector we can easily create a test to check the immutability status of a class.

As usual, Maven/Gradle dependencies can be found on Maven Central.

To test our ImmutableBean we can create the following jUnit test class:

import static org.mutabilitydetector.unittesting.MutabilityAssert.assertImmutable;

public class ImmutableBeanTest {

@Test
public void testClassIsImmutable() {
assertImmutable(ImmutableBean.class);
}
}

the test will fail if the class is not immutable.

For example, if a field is not final and it has a setter method, the test fails and the error message is very descriptive:

org.mutabilitydetector.unittesting.MutabilityAssertionError:
Expected: it.gualtierotesta.testsolutions.general.beans.ImmutableBean to be IMMUTABLE
but: it.gualtierotesta.testsolutions.general.beans.ImmutableBean is actually NOT_IMMUTABLE
Reasons:
Field is not final, if shared across threads the Java Memory Model will not guarantee it is initialised before it is read.
[Field: aStr, Class: it.gualtierotesta.testsolutions.general.beans.ImmutableBean]
Field [aStr] can be reassigned within method [setaStr]
[Field: aStr, Class: it.gualtierotesta.testsolutions.general.beans.ImmutableBean]

The complete project can be found on my Test Solutions gallery project on GitHub. See module general.

The approach I suggest is to use Lombok without any immutability test. If Lombok cannot be used (for example in a legacy project), use the Mutability Detector to assert that the class is really immutable.

Vaadin dependencies in Maven projects

Updated post can be found on my new blog site.

The Vaadin framework has several dependencies but not all of them should be included in our war/ear artifacts.

The following table shows all Vaadin version 7.6/7.7 main modules and their meaning and usage

Module Description and usage
server This is the core of the framework. It has the following (transitive) dependencies: vaadin-shared and vaadin-sass-compiler
themes Compiled version of the standard Vaadin themes
client-compiled Compiled version of the standard Vaadin widgets set
client Vaadin and GWT classes for widgets
client-compiler Widgets compiler based on GWT Google Web Toolkit
push Optional module. It includes the support for push protocols (server to client) thanks to the Atmosphere framework
shared Common modules code. It is included as dependency in the server module
sass-compiler SASS to CSS compiler, used at build time and at run-time (“on-the-fly” compilation). It is included as dependency in the server module

Depending on the project requirements, the above modules should be included or not as project dependencies. We can identify two possible scenarios:

  1. Project without a custom widget set. It can have a custom theme
  2. Project with a custom widget set

In the first case (without a custom widget set) we need the following modules:

  • server
  • themes
  • push (optional)
  • client-compiled

while, if we have a custom widget set, we need to compile the widgets so the dependencies become:

  • server
  • themes
  • push (optional)
  • client (for build only)
  • client-compiler (for build only)

Note: the compiled custom widgets are included in our artifact

The following table summarizes the Maven dependencies:

Module ArtifactId Scope Required?
server vaadin-server compile yes
themes vaadin-themes compile yes
client-compiled vaadin-client-compiled runtime only if the project does not use custom widget set
client vaadin-client provided only with custom widget set
client-compiler vaadin-client-compiler provided only with custom widget sett. See also note below.
push vaadin-push compile optional
shared vaadin-shared vaadin-server dependency. No need to be specified in the pom.xml
sass-compiler vaadin-sass-compiler vaadin-server dependency. No need to be specified in the pom.xml

Note: the vaadin-client-compiler dependency is automatically included in the classpath by the Vaadin Maven plugin (vaadin-maven-plugin) when the custom widgets set should be compiled.

Tutorial: Correct SLF4J logging usage and how to check it

SLF4J is a very popular logging facade but, like all libraries we use, there is a chance that we use it in a wrong or at least in a not optimal way.

In this tutorial we will list common logging errors and how we can detect them using FindBugs. We will also mention PMD and Sonar Squid checks when relevant.

We will use two external FindBugs plugins which add logging detectors to FindBugs.

The first one is a SLF4J only plugin by Kengo Toda which contains SLF4J detectors only.

The second plugin is the popular FB Contrib which contains, among many others, some logging detectors.

For how to use FindBugs plugins, please refer to the following posts:

Note: in all examples we will assume the following imports:

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

1. Logger definition

Wrong way:

W1a. Logger log = LoggerFactory.getLogger(MyClass.class);
W1b. private Logger logger = LoggerFactory.getLogger(MyClass.class);
W1c. static Logger LOGGER = LoggerFactory.getLogger(AnotherClass.class);

Correct way:

C1a. private static final Logger LOGGER = LoggerFactory.getLogger(MyClass.class);
C1b. private final Logger logger = LoggerFactory.getLogger(getClass());

General rule: the logger should be final and private because there are no reasons to share it with other classes or to re-assign it.

On the contrary there is no general agreement if the logger should be static or not. SLF4J plugin favors non static version (C1b) while PMD (“LoggerIsNotStaticFinal” rule) and Sonar (squid rule S1312) prefer a static logger (C1a) so both options should be considered as valid.

Additional info:

Please note that

  • in the static version (C1a), the logger name is usually in uppercase characters as all constant fields. If not, PMD will report a “VariableNamingConventions” violation.
  • in both cases, the suggested name is “logger/LOGGER” and not “log/LOG” because some naming conventions avoid too short names (less than four characters). Moreover log is the verb, more suited for a method name.
  • the W1c is wrong because we are referring to a class (AnotherClass) which is not the class where the logger is defined. In the 99% of the cases, this is due to a copy & paste from one class to another.

Related FindBugs (SLF4J plugin) checks:

  • SLF4J_LOGGER_SHOULD_BE_PRIVATE
  • SLF4J_LOGGER_SHOULD_BE_NON_STATIC
  • SLF4J_LOGGER_SHOULD_BE_FINAL
  • SLF4J_ILLEGAL_PASSED_CLASS

 

2. Format string

Wrong way:

W2a. LOGGER.info("Obj=" + myObj);
W2b. LOGGER.info(String.format(“Obj=%s”, myObj));

Correct way:

C2. LOGGER.info("Obj={}",myObj);

General rule: the format string (the first argument) should be constant, without any string concatenation. Dynamic contents (the myObj value in the example) should be added using the placeholders (the ‘{}’ ).

Motivation is simple: we should delay logging message creation after the logger has established if the message should be logged or not, depending on the current logging level. If we use string concatenation, message is built any way, regardless the logging level which is a waste of CPU and memory resources.

Related FindBugs (SLF4J plugin) checks:

  • SLF4J_FORMAT_SHOULD_BE_CONST Format should be constant
  • SLF4J_SIGN_ONLY_FORMAT Format string should not contain placeholders only

Related FindBugs (FB Contrib plugin) checks:

  • LO_APPENDED_STRING_IN_FORMAT_STRING Method passes a concatenated string to SLF4J’s format string

 

3. Placeholder arguments

Wrong way:

W3a. LOGGER.info("Obj={}",myObj.getSomeBigField());
W3b. LOGGER.info("Obj={}",myObj.toString());
W3c. LOGGER.info("Obj={}",myObj, anotherObj);
W3d. LOGGER.info("Obj={} another={}",myObj);

Correct way:

C3a. LOGGER.info("Obj={}",myObj);
C3b. LOGGER.info("Obj={}",myObj.log());

General rule: the placeholder should be an object (C3a), not a method return value (W3a) in order to post-pone its evaluation after logging level analysis (see previous paragraph). In W3a example, the mehod getSomeBigField() will be always called, regardless the logging level. For the same reason, we should avoid W3b which is semantically equivalent to C3a but it always incurs in the toString() method invocation.

Solutions W3c and W3d are wrong because the number of placeholders in the format string does not match the number of placeholders arguments.

Solution C3b could be somehow misleading because it includes a method invocation but it could be useful whenever the myObj contains several fields (for example it is a big JPA entity) but we do not want to log all its contents.

For example, let’s consider the following class:

public class Person {
private String id;
private String name;
private String fullName;
private Date birthDate;
private Object address;
private Map<String, String> attributes;
private List phoneNumbers;

its toString() method will most probably include all fields. Using the solution C3a, all their values will be printed in the log file.

If you do not need all this data, it is useful to define a helper method like the following:

public String log() {
return String.format("Person: id=%s name=%s", this.id, this.name);
}

which prints relevant information only. This solution is also CPU and memory lighter than toString().

What is relevant ? It depends on the application and on the object type. For a JPA entity, I usually include in the log() method the ID field (in order to let me find the record in the DB if I need all columns data) and, may be, one or two important fields.

For no reason, passwords fields and/or sensitive info (phone numbers,…) should be logged. This is an additional reason to not log using toString().

Related FindBugs (SLF4J plugin) checks:

  • SLF4J_PLACE_HOLDER_MISMATCH

 

4. Debug messages

IMPORTANT: rule #4 (see 5 rules article) guide us to use a guarded debug logging

if (LOGGER.isDebugEnabled()) {
LOGGER.debug(“Obj={}”, myObj);
}

Using SLF4J, if the placeholder argument is an object reference (see solutions C3a/C3b), we can use avoid the if in order to keep the code cleaner.

So it is safe to use the following:

LOGGER.debug(“Obj={}”, myObj);

 

5. Exceptions

Proper exceptions logging is an important support for problems analysis but it is easy to neglect its usefulness.

Wrong way:

W5a. catch (SomeException ex) { LOGGER.error(ex);}..
W5b. catch (SomeException ex) { LOGGER.error("Error:" + ex.getMessage());}..

Correct way:

C5. catch (SomeException ex) { LOGGER.error("Read operation failed: id={}", idRecord, ex);}..`

General rules:

  1. Do not remove the stack trace information by using getMessage() (see W5b) and not the complete exception. The stack trace often includes the real cause of the problem which is easily another exception raised by the underlying code. Logging only the message will prevent us to discover the real cause of the problem.
  2. Do show significant (for the human which will analyze the log file) information in the logging message showing a text explaining what we wanted to perform while the exception was raised (not the exception kind or messages like “error”: we know already something bad happened). What we need to know is what we were doing and on which data.

The C5 example tells we were trying to read the record with a specific ID whose value has been written in the log with the message.

Please note that C5 use one placeholder in the format string but there are two additional arguments. This is not an error but a special pattern which is recognized by SLF4J as an exception logging case: the last argument (ex in the C5 example) is considered by SLF4J as a Throwable (exception) so it should be not included in the format string.

Related FindBugs (SLF4J plugin) checks:

  • SLF4J_MANUALLY_PROVIDED_MESSAGE: the message should not be based on Exception getMessage()