Do not use AllArgsConstructor in your public API

Introduction

Do you think about compatibility of your public API when you modify classes from it? It is especially easy to miss out that something incompatibly changed when you are using Lombok. If you use AllArgsConstructor annotation it will cause many problems.

What is the problem?

Let’s define simple class with AllArgsConstructor:

@Data
@AllArgsConstructor
public class Person {
    private final String firstName;
    private final String lastName;
    private Integer age;
}

 

Now we can use generated constructor in spock test:

def 'use generated allArgsConstructor'() {
    when:
        Person p = new Person('John', 'Smith', 30)
    then:
        with(p) {
            firstName == 'John'
            lastName == 'Smith'
            age == 30
        }
}

And the test is green.

Let’s add new optional field to our Person class – email:

@Data
@AllArgsConstructor
public class Person {
    private final String firstName;
    private final String lastName;
    private Integer age;
    private String email;
}

Adding optional field is considered compatible change. But our test fails…

groovy.lang.GroovyRuntimeException: Could not find matching constructor for: com.github.alien11689.allargsconstructor.Person(java.lang.String, java.lang.String, java.lang.Integer)

How to solve this problem?

After adding field add previous constructor

If you still want to use AllArgsConstructor you have to ensure compatibility by adding previous version of constructor on your own:

@Data
@AllArgsConstructor
public class Person {
    private final String firstName;
    private final String lastName;
    private Integer age;
    private String email;

    public Person(String firstName, String lastName, Integer age) {
        this(firstName, lastName, age, null);
    }
}

And now our test again passes.

Annotation lombok.Data is enough

If you use only Data annotation, then constructor, with only mandatory (final) fields, will be generated. It is because Data implies RequiredArgsConstructor:

@Data
public class Person {
    private final String firstName;
    private final String lastName;
    private Integer age;
}
class PersonTest extends Specification {
    def 'use generated requiredFieldConstructor'() {
        when:
            Person p = new Person('John', 'Smith')
            p.age = 30
        then:
            with(p) {
                firstName == 'John'
                lastName == 'Smith'
                age == 30
            }
    }
}

After adding new field email test still passes.

Use Builder annotation

Annotation Builder generates for us PersonBuilder class which helps us create new Person:

@Data
@Builder
public class Person {
    private final String firstName;
    private final String lastName;
    private Integer age;
}
class PersonTest extends Specification {
    def 'use builder'() {
        when: Person p = Person.builder()
            .firstName('John')
            .lastName('Smith')
            .age(30).build()
        then: with(p) {
            firstName == 'John'
            lastName == 'Smith'
            age == 30
        }
    }
}

After adding email field test still passes.

Conclusion

If you use AllArgsConstructor you have to be sure what are you doing and know issues related to its compatibility. In my opinion the best option is not to use this annotation at all and instead stay with Data or Builder annotation.

Sources are available here.

You May Also Like

JBoss Envers and Spring transaction managers

I've stumbled upon a bug with my configuration for JBoss Envers today, despite having integration tests all over the application. I have to admit, it casted a dark shadow of doubt about the value of all the tests for a moment. I've been practicing TDD since 2005, and frankly speaking, I should have been smarter than that.

My fault was simple. I've started using Envers the right way, with exploratory tests and a prototype. Then I've deleted the prototype and created some integration tests using in-memory H2 that looked more or less like this example:

@Test
public void savingAndUpdatingPersonShouldCreateTwoHistoricalVersions() {
    //given
    Person person = createAndSavePerson();
    String oldFirstName = person.getFirstName();
    String newFirstName = oldFirstName + "NEW";

    //when
    updatePersonWithNewName(person, newFirstName);

    //then
    verifyTwoHistoricalVersionsWereSaved(oldFirstName, newFirstName);
}

private Person createAndSavePerson() {
    Transaction transaction = session.beginTransaction();
    Person person = PersonFactory.createPerson();
    session.save(person);
    transaction.commit();
    return person;
}    

private void updatePersonWithNewName(Person person, String newName) {
    Transaction transaction = session.beginTransaction();
    person.setFirstName(newName);
    session.update(person);
    transaction.commit();
}

private void verifyTwoHistoricalVersionsWereSaved(String oldFirstName, String newFirstName) {
    List<Object[]> personRevisions = getPersonRevisions();
    assertEquals(2, personRevisions.size());
    assertEquals(oldFirstName, ((Person)personRevisions.get(0)[0]).getFirstName());
    assertEquals(newFirstName, ((Person)personRevisions.get(1)[0]).getFirstName());
}

private List<Object[]> getPersonRevisions() {
    Transaction transaction = session.beginTransaction();
    AuditReader auditReader = AuditReaderFactory.get(session);
    List<Object[]> personRevisions = auditReader.createQuery()
            .forRevisionsOfEntity(Person.class, false, true)
            .getResultList();
    transaction.commit();
    return personRevisions;
}

Because Envers inserts audit data when the transaction is commited (in a new temporary session), I thought I have to create and commit the transaction manually. And that is true to some point.

My fault was that I didn't have an end-to-end integration/acceptance test, that would call to entry point of the application (in this case a service which is called by GWT via RPC), because then I'd notice, that the Spring @Transactional annotation, and calling transaction.commit() are two, very different things.

Spring @Transactional annotation will use a transaction manager configured for the application. Envers on the other hand is used by subscribing a listener to hibernate's SessionFactory like this:

<bean id="sessionFactory" class="org.springframework.orm.hibernate3.annotation.AnnotationSessionFactoryBean" >        
...
 <property name="eventListeners">
     <map key-type="java.lang.String" value-type="org.hibernate.event.EventListeners">
         <entry key="post-insert" value-ref="auditEventListener"/>
         <entry key="post-update" value-ref="auditEventListener"/>
         <entry key="post-delete" value-ref="auditEventListener"/>
         <entry key="pre-collection-update" value-ref="auditEventListener"/>
         <entry key="pre-collection-remove" value-ref="auditEventListener"/>
         <entry key="post-collection-recreate" value-ref="auditEventListener"/>
     </map>
 </property>
</bean>

<bean id="auditEventListener" class="org.hibernate.envers.event.AuditEventListener" />

Envers creates and collects something called AuditWorkUnits whenever you update/delete/insert audited entities, but audit tables are not populated until something calls AuditProcess.beforeCompletion, which makes sense. If you are using org.hibernate.transaction.JDBCTransaction manually, this is called on commit() when notifying all subscribed javax.transaction.Synchronization objects (and enver's AuditProcess is one of them).

The problem was, that I used a wrong transaction manager.

<bean id="transactionManager" class="org.springframework.jdbc.datasource.DataSourceTransactionManager" >
    <property name="dataSource" ref="dataSource"/>
</bean>

This transaction manager doesn't know anything about hibernate and doesn't use org.hibernate.transaction.JDBCTransaction. While Synchronization is an interface from javax.transaction package, DataSourceTransactionManager doesn't use it (maybe because of simplicity, I didn't dig deep enough in org.springframework.jdbc.datasource), and thus Envers works fine except not pushing the data to the database.

Which is the whole point of using Envers.

Use right tools for the task, they say. The whole problem is solved by using a transaction manager that is well aware of hibernate underneath.

<bean id="transactionManager" class="org.springframework.orm.hibernate3.HibernateTransactionManager" >
    <property name="sessionFactory" ref="sessionFactory"/>
</bean>

Lesson learned: always make sure your acceptance tests are testing the right thing. If there is a doubt about the value of your tests, you just don't have enough of them,

JCE keystore and untrusted sites

Recently at work I was in need of connecting to a web service exposed via HTTPS. I've been doing this from inside Servicemix 3.3.1, which may seem a bit inhibiting, but that was a requirement. Nevertheless I've been trying my luck with the included ser...Recently at work I was in need of connecting to a web service exposed via HTTPS. I've been doing this from inside Servicemix 3.3.1, which may seem a bit inhibiting, but that was a requirement. Nevertheless I've been trying my luck with the included ser...