26

We are working on Java web project based on JPA 2, Hibernate, Spring 3 and JSF 2 running in Tomcat 7. We are using Oracle 11g as database.

We are currently holding a debate on approaches to populate database constraint violations as user-friendly messages to the UI. More or less we see two ways, both are not really satisfying. Could somebody give some advise?

Approach 1 - Validate programmatically and throw specific exception

In CountryService.java each Unique constraint will be validated and a corresponding exception is thrown. The exceptions are handled individually in a backing bean.

Advantage: Easy to understand and maintain. Specific User messages possible.

Disadvantage: A lot of code just for having nice messages. Basically all DB Constraints are written again in the application. A lot of queries - unnecessary db load.

@Service("countryService")
public class CountryServiceImpl implements CountryService {

    @Inject
    private CountryRepository countryRepository;

    @Override
    public Country saveCountry(Country country) throws NameUniqueViolationException,  IsoCodeUniqueViolationException, UrlUniqueViolationException {
        if (!isUniqueNameInDatabase(country)) {
            throw new NameUniqueViolationException();
        }
        if (!isUniqueUrl(country)) {
            throw new UrlUniqueViolationException();
        }
        if (!isUniqueIsoCodeInDatabase(country)) {
            throw new IsoCodeUniqueViolationException();
        }
        return countryRepository.save(country);
    }
}

In the View's Backing Bean you handle the exceptions:

@Component
@Scope(value = "view")
public class CountryBean {

    private Country country;

    @Inject
    private CountryService countryService;

    public void saveCountryAction() {
        try {
            countryService.saveCountry(country);
        } catch (NameUniqueViolationException e) {
            FacesContext.getCurrentInstance().addMessage("name", new FacesMessage("A country with the same name already exists."));
        } catch (IsoCodeUniqueViolationException e) {
            FacesContext.getCurrentInstance().addMessage("isocode", new FacesMessage("A country with the same isocode already exists."));
        } catch (UrlUniqueViolationException e) {
            FacesContext.getCurrentInstance().addMessage("url", new FacesMessage("A country with the same url already exists."));
        } catch (DataIntegrityViolationException e) {
             // update: in case of concurrent modfications. should not happen often
             FacesContext.getCurrentInstance().addMessage(null, new FacesMessage("The country could not be saved."));
        }
    }
}

Approach 2 - Let the database detect constraint violations

Advantage: No boiler plate code. No unnecessary queries to db. No duplication of data constraint logic.

Disadvantage: Dependencies to constraint names in DB, so generation of Schema through hibernate not possible. Mechanism needed to bind messages to input components (e.g. for highlighting).

public class DataIntegrityViolationExceptionsAdvice {
    public void afterThrowing(DataIntegrityViolationException ex) throws DataIntegrityViolationException {

        // extract the affected database constraint name:
        String constraintName = null;
        if ((ex.getCause() != null) && (ex.getCause() instanceof ConstraintViolationException)) {
            constraintName = ((ConstraintViolationException) ex.getCause()).getConstraintName();
        }

        // create a detailed message from the constraint name if possible
        String message = ConstraintMsgKeyMappingResolver.map(constraintName);
        if (message != null) {
            throw new DetailedConstraintViolationException(message, ex);
        }
        throw ex;
    }
}
Cœur
  • 37,241
  • 25
  • 195
  • 267
fischermatte
  • 3,327
  • 4
  • 42
  • 52
  • 1
    You are still relying on the database to detect constraint violations in Approach 1, in the event that a user saves a duplicate country after the unique check but before the save of a second user. – qxn Mar 23 '12 at 16:22
  • 1
    we are aware of the concurrent issue here. it is not mandatory for our use cases. it is enough, if in 90% of the cases the message is specific. if in exceptional cases, a more generic message is triggered by db doesn't matter. – fischermatte Mar 23 '12 at 16:29

3 Answers3

16

Approach 1 will not work in a concurrent scenario! -- There is always a change that somebody else insert a new database record after you checked but before you added your database record. (except you use isolation level serializable, but this is highly unlikely)

So you have to handle the DB constraint violation exceptions. But I recommend to catch the database exception that indicates the unique violation and throw a more meaning full like you suggested in Approach 1.

Ralph
  • 118,862
  • 56
  • 287
  • 383
  • 1
    as i wrote to ken, the concurrent issue doesn't matter. in those exceptional cases a more generic message (violation triggered by db will be shown). updated approach 1. – fischermatte Mar 23 '12 at 16:31
  • Anyway, this will not change my suggested approach. – Ralph Mar 23 '12 at 16:47
  • 1
    What I don't like about it, is that normally during development hibernate will generate the constraint names. With approach 2 we need to maintain the names seprately through additional scripts. – fischermatte Mar 23 '12 at 16:52
  • 1
    The Unique Constraint has a name Attribute! `@javax.persistence.UniqueConstraint(name, columnName)` – Ralph Mar 23 '12 at 17:07
  • 3
    as much as i know hibernate ignores the name attribute on UniqueConstraint. it won't be used when generating the schema. – fischermatte Mar 25 '12 at 14:34
  • ...just to confirm the previous statement, I used `@Table(name = "users", uniqueConstraints = {@UniqueConstraint(name = "users_unique_email", columnNames = {"email"})})` with my `User` entity, and then tried to fetch it via `getConstraintName()` while persisting user with duplicate email. It heavily depends on underlying RDBMS, because I get different constraints names for different databases (particularly in case of Postgres - I get `null` and have to parse `sqlException` to figure out what field was exactly duplicated, which can be a pain if I decide to add more than one unique constraint). – escudero380 Sep 17 '20 at 18:35
15

This may also be an option, and might be less costly because you only do checks for a detailed exception if you can't save outright:

try {
    return countryRepository.save(country);
}
catch (DataIntegrityViolationException ex) {
    if (!isUniqueNameInDatabase(country)) {
        throw new NameUniqueViolationException();
    }
    if (!isUniqueUrl(country)) {
        throw new UrlUniqueViolationException();
    }
    if (!isUniqueIsoCodeInDatabase(country)) {
        throw new IsoCodeUniqueViolationException();
    }
    throw ex;
}
qxn
  • 17,162
  • 3
  • 49
  • 72
  • 3
    Thx, defintely better than approach 1! Only drawback i see is, that you manually need to flush after calling `countryRepository.save(country)`. – fischermatte Mar 27 '12 at 07:47
  • 2
    Nice, but keep in mind that you will not be able to catch the exception if save is inside a method annotated with @Transactional – abarazal May 12 '21 at 12:18
1

To avoid boilerplate I treat DataIntegrityViolationException in ExceptionInfoHandler, finding DB constraints occurrences in root cause message and convert it into i18n message via map. See code here: https://stackoverflow.com/a/42422568/548473

naXa stands with Ukraine
  • 35,493
  • 19
  • 190
  • 259
Grigory Kislin
  • 16,647
  • 10
  • 125
  • 197