0

I have a custom EmptyInterceptor that I use to set information on the creation date, last modification date, created by user, and last modified user by overriding onSave and onFlushDirty.

This system has worked pretty well for us but we just found an issue where we have code that is blindly calling the setters on an entity using some input data. In this case, even if the data has not changed, hibernate will fire our custom interceptor and we'll set the last modified user and date. This causes Hibernate to update the entity in the database. We know this because we have an insert and update trigger on this table. If we disable the interceptor, Hibernate will not update the entity in the database.

We'd like to avoid setting the user and date if the entity hasn't really changed so no update happens in those situations. I've read about the way Hibernate does its dirty entity checking and I have the previousState and currentState arrays passed into onFlushDirty and I can loop through to do this check myself. Is there any easier way to do that?

I looked at HibernateSession.isDirty() but that doesn't tell me if this particular entity has changed, just if there's a changed entity in the session.


UPDATE

It turns out that the offending code blindly calling setters was not the issue. It was the offending code setting a Collection of child objects instead of modifying the Collection that was already there. In this case, Hibernate thinks the entity has changed - or at least thinks so enough to call the Interceptor.

Community
  • 1
  • 1
Chris Williams
  • 11,647
  • 15
  • 60
  • 97
  • I'm not completely sure I'm understanding the question, but if the entity is created by yourself and have control over the interceptor: Why don't just override the `equals` method on the entity so each object can compare itself with any other object field by field, all fields except 'last update date' or any other way you like to compare them and so on the interceptor you just call the `object1.equals(object2);`. There you have the comparisson to check if some data changed and you can control wich specific data to check. – Santiago Alzate Feb 16 '17 at 19:57
  • @SEAang, I have the current entity as an object that is passed into the `onFlushDirty` method but I have nothing to compare it to. Hibernate doesn't provide me another copy of the entity that represents the original state. I do have an array that represents all of the original fields and another that represents the changed fields. I'm looping through that and comparing values. – Chris Williams Feb 16 '17 at 21:14

2 Answers2

1

From the design perspective, this check should really be done on the front-end/client side. If the front-end determines that the record is modified by the user, then it should submit an update to the server.

If you want to do this in the middle tier (server-side), then you should think about the lifecycle of a Hibernate entity: Transient, Persistent, Detached, Removed, and also think about how session.save() is different from session.merge() and session.saveOrUpdate().

Furthermore, you also have to consider the different design patterns when managing session, such as "session-per-operation" anti-pattern, session-per-request, or session-per-conversation patterns, etc...

If you have an open session, and your entity is already in the session (from another operation), then Hibernate can in fact do the dirty checking for you. But if you have a detached entity and that entity doesn't exist in the session, and say you do a merge, Hibernate will first fetch that entity from the data store (database) by issuing a SELECT and puts that managed entity in the persistent context, and then it will merge the two entities, the one you provide and the one hibernate fetches and puts in the persistence context, and then it checks to see if anything has changed, hence dirty-checking.

In your case, since you want to exclude the last-modified-user name and time from dirty checking, you might as well get the entity by ID (since you presumably have the ID on the detached entity) and then use equals() or hashCode() to do your own version of dirty checking, and if they are not, then call merge.

You may think this is an extra trip to the DB, but it's not, because even in normal cases, Hibernate still makes the extra trip to the DB, if the entity is not already in the persistent context (i.e. session), and if it is, and you do a get-by-id, Hibernate will just return to you what it already has in the session, and won't hit the DB.

This argument doesn't apply to saveOrUpdate, in which case, Hibernate will simply push the updates to the DB without dirty checking (if the entity is not already in the session), and if it is already in the session, it will throw an exception saying that the entity is already in the session.

raminr
  • 784
  • 3
  • 12
  • Good points but in the end, I consider this a Hibernate bug. It should not be calling an implementation of EmptyInterceptor if the entity has not changed. In this case, it was changed because a `Collection` of child entities was re-created rather than cleared and updated in-place. That type of change shouldn't be considered for `onFlushDirty`. – Chris Williams Feb 17 '17 at 20:14
  • It's not a bug. Hibernate is calling your intercepter because you declared that persistent events should be intercepted. Modifications to entities are actually translated into events. So every time you modify an entity, an event is queued up (if entity is marked as dirty), and all those events are used to compose the SQL. – raminr Feb 18 '17 at 15:22
  • If my interceptor doesn't modify the entity at all, nothing is written to the database. I define that as the object not being dirty. The API doc says `onFlushDirty` will be "Called when an object is detected to be dirty". This object isn't dirty, its relationship with other objects has changed. I'd say that `onFlushDirty` should never be called in this scenario. – Chris Williams Feb 19 '17 at 20:11
1

In case anyone needs to solve the same problem without changing the code that causes this issue, I implemented the compare as:

    /**
     * Called upon entity UPDATE. For our BaseEntity, populates updated by with logged in user ID and
     * updated date-time.
     * <p>
     * Note that this method is called even if an object has NOT changed but someone's called a setter on
     * a Collection related to a child object (as opposed to modifying the Collection itself). Because of
     * that, the comparisons below are necessary to make sure the entity has really changed before setting
     * the update date and user.
     *
     * @see org.hibernate.EmptyInterceptor#onFlushDirty(java.lang.Object, java.io.Serializable, java.lang.Object[], java.lang.Object[], java.lang.String[], org.hibernate.type.Type[])
     */
    @Override
    public boolean onFlushDirty(Object entity, Serializable id, Object[] currentState, Object[] previousState,
            String[] propertyNames, Type[] types)
    {

        boolean changed = false;

        if (entity instanceof BaseEntity) {
            logger.debug("onFlushDirty method called on " + entity.getClass().getCanonicalName());

            // Check to see if this entity really changed(see Javadoc above).
            boolean reallyChanged = false;
            for (int i = 0; i < propertyNames.length; i++) {
                // Don't care about the collection types because those can change and we still don't consider
                // this object changed when that happens.
                if (!(types[i] instanceof CollectionType)) {
                    boolean equals = Objects.equals(previousState[i], currentState[i]);
                    if (!equals) {
                        reallyChanged = true;
                        break;
                    }
                }
            }

            if (reallyChanged) {
                String userId = somehowGetUserIdForTheUserThatMadeTheRequest();
                Date dateTimeStamp = new Date();

                // Locate the correct field and update it.
                for (int i = 0; i < propertyNames.length; i++) {
                    if (UPDATE_BY_FIELD_NAME.equals(propertyNames[i])) {
                        currentState[i] = userId;
                        changed = true;
                    }

                    if (UPDATE_DATE_FIELD_NAME.equals(propertyNames[i])) {
                        currentState[i] = dateTimeStamp;
                        changed = true;
                    }
                }
            }
        }

        return changed;
    }
}
Chris Williams
  • 11,647
  • 15
  • 60
  • 97
  • 2
    Looks good. You should break out of the loop as soon as it encounters at least one thing has changed so you don't bother checking all the other properties. – raminr Feb 17 '17 at 14:25