Sunday, February 7, 2010

The Perls and Perils of BeanUtils copyProperties

I’m still working on my Lazy Map example and it’s been a long time since I posted so I decided I should write a quick small post. I decided to talk about BeanUtils.copyProperties and how it can be misused.

When you need to copy a large number of members from one Data Transfer Object or Value Object (DTO/VO) to another, copyProperties is indeed a powerful and useful tool. However as the adage goes ”with great power comes a great responsibility”, this utility has a dark side. In my opinion it’s a terrible side effect that is often misunderstood by the average developer, hence the title of this article.

So, what does it do and why would I be interested in using it anyway? Well, as the name implies, it simple copies the members from one bean to another, matching the getter and setter names to do so. It only has three Run Time Exceptions. These exceptions are IllegalAccessException, IllegalArgumentException, and InvocationTargetException.

IllegalAccessException - if the caller does not have access to the property accessor method
IllegalArgumentException - if the dest or orig argument is null or if the dest property type is different from the source type and the relevant converter has not been registered.
InvocationTargetException - if the property accessor method throws an exception
Potentially, It saves time by allowing a developer to write one line of code versus 10 or even 20 lines of code.

You might be asking yourself “How could something so good have any kind of side effect that would be considered bad?” I personally have found that this class has got to be one of the biggest misused utilities I have ever seen. Copying beans, when there is no contract such as an Interface or an Abstract class involved usually means you might get something you didn’t expect. An Anonymous User hinted at the problem in one of his comments… Refactoring. Say the DTO/VO’s you are going to copy have similar member names, but no contract and you copy them… several months later someone is working on code in some part of the program and doesn’t like the name so changes it. Most IDEA such as Intellij or Eclipse provide this feature, but in their defense, there is just no way to know that the code is now broken. If there was a interface or Abstract class it would have corrected it.

Here is an example of a failure. The following two DTO/VO objects dont exacly match because someone deleted a member. The test catches this error, but as you can see, the test really isnt a good test. For this example, I will be using beanutils 1.8.2, collections 3.2.1, lang 2.4, logging 1.1.1, and junit 4.8.1

import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

public class ValueObjectA {
private String firstname;
private String lastname;
private String cv;
private String linkedin;
private String blog;
private int age;
private boolean hacker;

public ValueObjectA() { }

public ValueObjectA(String firstname, String lastname, String cv, String linkedin, String blog, int age, boolean hacker) {
this.firstname = firstname;
this.lastname = lastname;
this.cv = cv;
this.linkedin = linkedin;
this.blog = blog;
this.age = age;
this.hacker = hacker;
}

public String getFirstname() {
return firstname;
}

public void setFirstname(String firstname) {
this.firstname = firstname;
}

public String getLastname() {
return lastname;
}

public void setLastname(String lastname) {
this.lastname = lastname;
}

public String getCv() {
return cv;
}

public void setCv(String cv) {
this.cv = cv;
}

public String getLinkedin() {
return linkedin;
}

public void setLinkedin(String linkedin) {
this.linkedin = linkedin;
}

public String getBlog() {
return blog;
}

public void setBlog(String blog) {
this.blog = blog;
}

public int getAge() {
return age;
}

public void setAge(int age) {
this.age = age;
}

public boolean isHacker() {
return hacker;
}

public void setHacker(boolean hacker) {
this.hacker = hacker;
}

@Override
public boolean equals(Object o) {
return EqualsBuilder.reflectionEquals(this, o);

}

@Override
public int hashCode() {
return HashCodeBuilder.reflectionHashCode(this);
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}
}


import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

public class ValueObjectB {
private String firstname;
private String lastname;
private String cv;
private String linkedin;
private int age;
private boolean hacker;

public ValueObjectB() { }

public ValueObjectB(String firstname, String lastname, String cv, String linkedin, int age, boolean hacker) {
this.firstname = firstname;
this.lastname = lastname;
this.cv = cv;
this.linkedin = linkedin;
this.age = age;
this.hacker = hacker;
}

public String getFirstname() {
return firstname;
}

public void setFirstname(String firstname) {
this.firstname = firstname;
}

public String getLastname() {
return lastname;
}

public void setLastname(String lastname) {
this.lastname = lastname;
}

public String getCv() {
return cv;
}

public void setCv(String cv) {
this.cv = cv;
}

public String getLinkedin() {
return linkedin;
}

public void setLinkedin(String linkedin) {
this.linkedin = linkedin;
}

public int getAge() {
return age;
}

public void setAge(int age) {
this.age = age;
}

public boolean isHacker() {
return hacker;
}

public void setHacker(boolean hacker) {
this.hacker = hacker;
}

@Override
public boolean equals(Object o) {
return EqualsBuilder.reflectionEquals(this, o);

}

@Override
public int hashCode() {
return HashCodeBuilder.reflectionHashCode(this);
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}    
}


So, if for some reason you could not make you're DTO/VO's implement an interface or extend an abstract class, you could set up a test like this:

import org.apache.commons.beanutils.BeanUtils;
import org.apache.commons.collections.CollectionUtils;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import org.junit.Test;
import static org.junit.matchers.JUnitMatchers.hasItem;

import java.lang.reflect.InvocationTargetException;
import java.util.Collection;
import java.util.Map;

public class BeanUtilsExample {

@Test
public void myTest() throws InvocationTargetException, IllegalAccessException, NoSuchMethodException {

ValueObjectA userA = new ValueObjectA("Philip", "Senger", "http://www.visualcv.com/philipsenger", "http://www.linkedin.com/in/philipsenger", "http://www.apachecommonstipsandtricks.blogspot.com/", 42, true);
ValueObjectB userB = new ValueObjectB();
BeanUtils.copyProperties(userB, userA);

Map<String, String> a = BeanUtils.describe(userA);
Map<String, String> b = BeanUtils.describe(userB);

for (String keyB : b.keySet()) {
assertThat(a.keySet(), hasItem(keyB));
}

for (String keyA : a.keySet()) {
assertThat(b.keySet(), hasItem(keyA));
}

// or you could test this way.

Collection disjunction = CollectionUtils.disjunction(a.keySet(), b.keySet());
assertThat( disjunction.size(), is(0));

}
}


If you didn't want to set yourself up for failure or couldn't include a unit test like the one above, here is one way to change the two DTO/VO's.

public interface ValueObject {
String getFirstname();

void setFirstname(String firstname);

String getLastname();

void setLastname(String lastname);

String getCv();

void setCv(String cv);

String getLinkedin();

void setLinkedin(String linkedin);

String getBlog();

void setBlog(String blog);

int getAge();

void setAge(int age);

boolean isHacker();

void setHacker(boolean hacker);
}

import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

public class ValueObjectA implements ValueObject {
private String firstname;
private String lastname;
private String cv;
private String linkedin;
private String blog;
private int age;
private boolean hacker;

public ValueObjectA() {
}

public ValueObjectA(String firstname, String lastname, String cv, String linkedin, String blog, int age, boolean hacker) {
this.firstname = firstname;
this.lastname = lastname;
this.cv = cv;
this.linkedin = linkedin;
this.blog = blog;
this.age = age;
this.hacker = hacker;
}

public String getFirstname() {
return firstname;
}

public void setFirstname(String firstname) {
this.firstname = firstname;
}

public String getLastname() {
return lastname;
}

public void setLastname(String lastname) {
this.lastname = lastname;
}

public String getCv() {
return cv;
}

public void setCv(String cv) {
this.cv = cv;
}

public String getLinkedin() {
return linkedin;
}

public void setLinkedin(String linkedin) {
this.linkedin = linkedin;
}

public String getBlog() {
return blog;
}

public void setBlog(String blog) {
this.blog = blog;
}

public int getAge() {
return age;
}

public void setAge(int age) {
this.age = age;
}

public boolean isHacker() {
return hacker;
}

public void setHacker(boolean hacker) {
this.hacker = hacker;
}

@Override
public boolean equals(Object o) {
return EqualsBuilder.reflectionEquals(this, o);

}

@Override
public int hashCode() {
return HashCodeBuilder.reflectionHashCode(this);
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}
}

import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

public class ValueObjectB implements ValueObject {
private String firstname;
private String lastname;
private String cv;
private String linkedin;
private String blog;
private int age;
private boolean hacker;

public ValueObjectB() { }

public ValueObjectB(String firstname, String lastname, String cv, String linkedin, String blog, int age, boolean hacker) {
this.firstname = firstname;
this.lastname = lastname;
this.cv = cv;
this.linkedin = linkedin;
this.blog = blog;
this.age = age;
this.hacker = hacker;
}

public String getFirstname() {
return firstname;
}

public void setFirstname(String firstname) {
this.firstname = firstname;
}

public String getLastname() {
return lastname;
}

public void setLastname(String lastname) {
this.lastname = lastname;
}

public String getCv() {
return cv;
}

public void setCv(String cv) {
this.cv = cv;
}

public String getLinkedin() {
return linkedin;
}

public void setLinkedin(String linkedin) {
this.linkedin = linkedin;
}

public String getBlog() {
return blog;
}

public void setBlog(String blog) {
this.blog = blog;
}

public int getAge() {
return age;
}

public void setAge(int age) {
this.age = age;
}

public boolean isHacker() {
return hacker;
}

public void setHacker(boolean hacker) {
this.hacker = hacker;
}

@Override
public boolean equals(Object o) {
return EqualsBuilder.reflectionEquals(this, o);

}

@Override
public int hashCode() {
return HashCodeBuilder.reflectionHashCode(this);
}

@Override
public String toString() {
return ToStringBuilder.reflectionToString(this);
}    
}


In conclusion, this command is powerful, but if you use it, I advise you to consider using a Interface or an Abstract class. Hope you enjoy this post, and feel free to drop me an email.

Author: Philip A Senger