Saturday, January 30, 2010

DiskDigger

It was a wonderful Saturday morning. I rose up and started my morning really slowly, was staying at a friend's house. At some point I took my laptop, plugged in my USB stick and prepared to do some useful things with my time.

USB is not responding, oh well that happens every now and then, lets replug, still not working... A small alarm indicator went in my head. Lets try the other port, computer recognize the stick, the free/used space match, but... OH MY GOD, NO FILES!!!

*Sigh* just two months ago I lost a 250g disk to a hardware malfunction... I've decided to run chkdsk, and indeed, errors were found, but there was insufficient disk space on the stick for chkdsk do finish his thing (and a good thing that!). It was a mistake to use chkdsk, I should have used a recovery software right away.

The file system on the stick went caput :-(

From google, I got to checking The Free Country, an excellent source of free software. After trying several utilities from the Data Recovery section, I found this excellent utility: DiskDigger

Unfortunately at this point all I was left to work with was scattered blocks of that on the hard drive, so simple undelete procedures didn't work. DiskDigger however, managed to recover most of my important files, the docs and pictures especially.

Now I'm left with the annoying task of sorting through the unnamed files... This really ruined my Saturday.

Blah...

Wednesday, January 20, 2010

Parent/Child Hibernate pitfall

Lets say we have these two interfaces:
package com.codeark.parentchild;

import java.util.Set;

public interface Parent {
Set<Child> getChildren();
}

And
package com.codeark.parentchild;

public interface Child {
Parent getParent();
}

And here are their implementations, annotated for persistence:
package com.codeark.parentchild;

import java.util.HashSet;
import java.util.Set;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.OneToMany;
import javax.persistence.Table;

@Entity
@Table(name="parents")
public class ParentImpl implements Parent {

private Set<Child> children;
private Long id;

public ParentImpl() {
this.children = new HashSet<Child>();
}

@Override
@OneToMany(targetEntity=ChildImpl.class, mappedBy="parent", cascade=CascadeType.ALL)
public Set<Child> getChildren() {
return children;
}

public void setChildren(Set<Child> children) {
this.children = children;
}

@Id
@GeneratedValue(strategy = GenerationType.AUTO)
public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}
} 


And the child implementation:
package com.codeark.parentchild;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.Table;

@Entity
@Table(name="children")
public class ChildImpl implements Child {

private Parent parent;
private Long id;

public ChildImpl() {

}

@Override
@ManyToOne(targetEntity=ParentImpl.class)
public Parent getParent() {
return parent;
}

public void setParent(Parent parent) {
this.parent = parent;
}

@Id
@GeneratedValue(strategy = GenerationType.AUTO)
public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}
}
So, where is the pitfall? Well, there are probably a few lying about, but the one I ran into will become apparent shortly. First, consider this code snippet:
Parent p = new ParentImpl();
Child c = new ChildImpl();

p.getChildren().add(c);
((ChildImpl)c).setParent(p);

// persist to database
DAOFactory.getParentDAO().persist(p);

Now, another programmer joined the show, and decided to have his own Child implementation:
package com.codeark.parentchild;

public class AnotherChildImpl implements Child {

private Parent parent;

public AnotherChildImpl() {

}

protected void someOtherNewMethods() {

}

@Override
public Parent getParent() {
return parent;
}

public void setParent(Parent p) {
this.parent = p;
}
}

A perfectly legal implementation, lets have a look at another code snippet:
Parent p = new ParentImpl();
Child c = new ChildImpl();
Child c1 = new AnotherChildImpl();

p.getChildren().add(c);
p.getChildren().add(c1);
((ChildImpl)c).setParent(p);
((AnotherChildImpl)c1).setParent(p);

// persist to database
DAOFactory.getParentDAO().persist(p);

What do you think will happen now? This code will compile, but during runtime Hibernate will hit us with an IllegalArgumentException in class: com.codeark.parentchild.ChildImpl, getter method of property: id
org.hibernate.PropertyAccessException: IllegalArgumentException occurred calling getter of com.codeark.parentchild.ChildImpl.id...

This is a result of me telling hibernate to expect a ChildImpl (targetEntity) while in practice that collection contained AnotherChildImpl class, which hibernate knows nothing about.

Why is this so bad?

This exception will not occur until the program tries to persist a ParentImpl with an AnotherChildImpl bomb waiting to explode in its children collection. That event can happen far into the (runtime) future. Although I doubt such a bug will make it to production if you have decent integration and QA, its still within the realm of possibilities; it would be best if we do what ever we can to prevent that eventuality.

Here is what I did to prevent this...

First I changed the parent Interface:
package com.codeark.parentchild;

import java.util.Set;

public interface Parent {
Set<? extends Child> getChildren();
}
Note that our children collection is now of an unknown type bounded by Child, instead of just Child.

Then I changed ParentImpl:
package com.codeark.parentchild;

import java.util.HashSet;
import java.util.Set;

import javax.persistence.CascadeType;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.OneToMany;
import javax.persistence.Table;

@Entity
@Table(name="parents")
public final class ParentImpl implements Parent {

private Set<ChildImpl> children;
private Long id;

public ParentImpl() {
this.children = new HashSet<ChildImpl>();
}

@Override
@OneToMany(mappedBy="parent", cascade=CascadeType.ALL)
public Set<ChildImpl> getChildren() {
return children;
}

public void setChildren(Set<ChildImpl> children) {
this.children = children;
}

@Id
@GeneratedValue(strategy = GenerationType.AUTO)
public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}
}
Our HashSet collection is now of ChildImpl type instead of just Child, I've changed that throughout the class's internals and external API. Our class still complies with Parent's contract, since we've changed Parent.getChildren() from Set<Child> to Set<? extends Child>

As an added bonus, we no longer need the targetEntity=ChildImpl.class annotation for ParentImpl.getChildren().

Last small caveat, I've marked the class as "final" since subclassing it without proper annotations/mapping might expose us to similar problems.

Another important change; in ChildImpl class:
package com.codeark.parentchild;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.Table;

@Entity
@Table(name="children")
public final class ChildImpl implements Child {

private ParentImpl parent;
private Long id;

public ChildImpl() {

}

@Override
@ManyToOne
public ParentImpl getParent() {
return parent;
}

public void setParent(ParentImpl parent) {
this.parent = parent;
}

@Id
@GeneratedValue(strategy = GenerationType.AUTO)
public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}
}
All the Parent references were changed to ParentImpl, so I wouldn't face a similar problem if someone tries to add a non persistent parent to ChildImpl.

Lets look at our buggy code again:
ParentImpl p = new ParentImpl();
ChildImpl c = new ChildImpl();
Child c1 = new AnotherChildImpl();

p.getChildren().add(c);
p.getChildren().add(c1);  // A Compile time error here!
Now the "other" programmer will get a compile time error, while trying to add his new (and improved) class to your parent Implementation, and the world is a slightly better place now.