@HandleBeforeSave RepositoryEventHandler should be able to contain old entity [DATAREST-373]

[spring-projects-issues]

Marcel Overdijk opened DATAREST-373 and commented

In case of @HandleBeforeSave it's desired to have the entity object values of the old domain object being updated.

The use cases are:

• Need to re-encrypt a password when it's updated
• send a warning email when a particular values gets bigger than a given treshold (only first time)
• ..

So it would be an idea to support something like

@HandleBeforeSave
public void handleBeforeSave(Customer oldCustomer, Customer newCustomer) { .. }

To workaround this limitation I tried something like:

@Component
@RepositoryEventHandler(Customer.class)
public class CustomerEventHandler {

    private CustomerRepository customerRepository;

    @Autowired
    public CustomerEventHandler(CustomerRepository customerRepository) {
        this.customerRepository = customerRepository;
    }

    @HandleBeforeSave
    public void handleBeforeSave(Customer newCustomer) {
        Customer oldCustomer = customerRepository.findOne(customer.getId());
        System.out.println("handleBeforeSave :: new customer.name = " + newCustomer.getName());
        System.out.println("handleBeforeSave :: old customer.name = " + oldCustomer.getName());
    }
}

But the problem is that the old object contains the already the new values.
So it seems the new object (with updated values) was already attached to the current (Hibernate) session.

---

Affects: 2.1.2 (Dijkstra SR2)

Issue Links:

• DATAREST-678 @HandleBeforeSave seems like override jpa sessions
  ("is duplicated by")
• DATAREST-876 @Transactional annotation is not being enforced on repository methods
  ("is duplicated by")
• DATAREST-874 spring-data-rest JPA repository, transaction issue
  ("is duplicated by")

30 votes, 31 watchers

[spring-projects-issues]

Marcel Overdijk commented

See also discussion: http://stackoverflow.com/questions/25233089/how-to-get-old-entity-value-in-handlebeforesave-event-to-determine-if-a-propert

[spring-projects-issues]

Michael Sharman commented

I'm also encountering this problem, in my use case I want to implement a security check based on the previous values of the entity and unfortunately this can't be achieved via the @HandleBeforeSave event handler because there doesn't appear to be any way to access the properties of the previous object. Given the name HandleBeforeSave I think it is quite reasonable to expect be able to inspect the state before it is committed?

[spring-projects-issues]

Fotis Paraskevopoulos commented

Hello,

Did you guys find any workarounds to this issue? The stackoverflow link doesn't shed much light on the issue.

Best,
FP

[spring-projects-issues]

Fotis Paraskevopoulos commented

Anyone stumbling here can look at this

http://stackoverflow.com/a/29919009/162059

For a workaround of the OPs scenario.

[spring-projects-issues]

Najum Ali commented

Checkout my (quite simple) workaround here: http://stackoverflow.com/a/32115184/1351164

[spring-projects-issues]

Rowan Massey commented

This would be extremely useful to have. I unfortunately can't use the work around suggested with an interceptor, since I use mongodb

[spring-projects-issues]

Ry Lowry commented

I've also run into this problem. In my case I wanted to fire off some special actions when a status field changes. This would be a great feature to see added

[spring-projects-issues]

Augusto David Altman Quaranta commented

I've also the same problem! A fix for this would be great

[spring-projects-issues]

Oliver Drotbohm commented

Unfortunately I don't think there's much we can do here as there just is no such thing as an "old entity" in JPA. We get the existing one, apply the changes submitted through POST or PUT and hand that one back to the persistence provider. JPA is a tricky beast even, as it doesn't necessarily needs interaction with the repository to save changes as it does dirty checking on the EntityManager flush. The manual call to findOne(…) will just hit the first level cache and return the same object you already get handed to the event listener method. Even if you triggered a query to lookup the "old entity" you'd flush the pending changes with that, which you don't want at the same time.

I'll have a look at what we can do but I actually don't want to start apply store specific handling on the REST level. What I can imagine though is changing the flush mode for Spring Data REST driven transactions but I am not entirely sure this will get us where we want

[spring-projects-issues]

Augusto David Altman Quaranta commented

As a workaround what I did was to add a new json ignored field in my entity intended to hold some other field previous value.

@Entity
public class Reservation {
        // ...............................
	private int familySize;
	
	@JsonIgnore
	private int prevFamilySize; //Workaround for issue: https://jira.spring.io/browse/DATAREST-373
        // ...............................
}

In order to keep the prevFamilySize updated I set its value with the actual familySize on the HandleBeforeSave/HandleBeforeCreate events after every processing has been done (is the last thing I do on the events).

@HandleBeforeSave
public void beforeSaveReservation(Reservation r) {
        // ...............................
	r.setPrevFamilySize(r.getFamilySize());
}

This way I can assure that prevFamilySize will always hold the familySize previous value and then I can use it for whatever I want.

I hope this workaround helps. Maybe Spring could provide a way to abstract and automatize this mechanism.

The obvious downside is that you will be holding the previous value in the database, but if you can live with that it's a very simple solution.

Greetings, Augusto.-

[spring-projects-issues]

benkuly commented

Why not copy the old entity in RepositoryEntityController or PersistentEntityResourceHandlerMethodArgumentResolver before read the new entity and make it available for developers in RepositoryEventHandler?

[spring-projects-issues]

Jens Schauder commented

Copying "beans" is not at all easy. One pitfall is that one might navigate large parts of the database, just to make a copy. And in most cases, it looks like the copy isn't used turning this into a huge waste of CPU time and memory. I don't think this will work in the general case.

If at all it might make sense to provide a simple interface that allows loading the previous version, probably backed by the findeOne method of the underlying repository, but with a separate connection, so one gets the current state (assuming it isn't changed in another transaction by now)

[spring-projects-issues]

benkuly commented

Okay I totally agree with you.

Maybe I understand it wrong, but: We could detach the "old" entity from the transactionbefore it becomes "new" entity, so we can call findOne in EventHandlers to get the old entity?

[spring-projects-issues]

Oliver Drotbohm commented

Because that requires knowledge about the persistence technology that Spring Data REST must not have. Also, even if it had, if we detach it, we can't transparently resolve lazy loading proxies and we'd almost by definition run into LazyInitializationExceptions everywhere. Spring Data REST needs to ignore all of that and just handle the aggregate as opaque instance.

I think one thing we can investigate though is disabling the flushing just for the invocation of the event handler method so that implementations can at least call repository methods that return a DTO that basically represents a read-only view on the entity

[spring-projects-issues]

Jens Schauder commented

Re: disabling the flushing just for the invocation: I don't like the sound of that. This would change the behavior of the JPA implementation just in that spot, meaning that code behaves differently when it is called from this method vs every other place in an application. Sounds like a big violation of the "principle of least surprise" to me.

[spring-projects-issues]

Oliver Drotbohm commented

I get the point, but as evident from this ticket it might actually be a less surprise to Spring Data REST users. JPA is the odd one out here as no other store actually exposes this hard to control flushing behaviour. Actually the don't flush semantics are actually in line with the semantics of the callback: implicitly triggering the save in a "handle before save" one would be more surprising actually

[spring-projects-issues]

Petar Tahchiev commented

If passing the old entity is a problem, why don't just pass the dirty values as Map. That would probably require significantly more work, but it will overcome the JPA flush behavior

[spring-projects-issues]

benkuly commented

Are there any plans or progress to fix the issue?

[spring-projects-issues]

Jens Schauder commented

With the upcoming releases and the still unmade decision if and how to fix this there is currently no one working on this issue. Sorry

[spring-projects-issues]

Thomas Turrell-Croft commented

I don't believe that this is an issue that needs to be resolved in Spring Data REST.

My colleague, István Rátkai, showed me a nice solution for this problem.

@Data
@Entity
public class User {
 
  @Id
  @GeneratedValue
  private Long id;
 
  @JsonProperty(access = Access.WRITE_ONLY)
  private String password;
 
  @JsonIgnore
  @Transient
  private String storedPassword;
 
  @PostLoad
  public void postLoad() {
    storedPassword = password;
  }
}

It is now possible to access the previous and the current value for the password in a handler.

  @HandleBeforeSave
  public void handleUserBeforeSave(User user) {

    // if the new password was provided, it needs to be encoded before saving
    if (user.getPassword() != null 
        && !user.getPassword().equals(user.getStoredPassword())) {
      user.setPassword(passwordEncoder.encode(user.getPassword()));
    }
  }

I hope that is useful for other people. Perhaps it could be added to the Spring Data REST documentation?

[spring-projects-issues]

Tyler Eastman commented

If you're using Hibernate another workaround is to user Hibernate spi events and event listeners to do this, you can implement PreUpdateEventListener and then register your class with the EventListenerRegistry in the sessionFactory. I created a small spring library to handle all of the setup.  I initially started writing it as a pull request for Spring data rest, but it relies on Hibernate, so thought it may be better as it's own module.  https://github.com/teastman/spring-data-hibernate-event

It adds the annotation @HibernateEventListener which allow you to annotate any method where the first parameter is the entity you want to listen to, and the second parameter is the Hibernate event that you want to listen for. I've also added the static util function getPropertyIndex to more easily get access to the specific property you want to check, but you can also just look at the raw Hibernate event.

@HibernateEventListener
public void onUpdate(MyEntity entity, PreUpdateEvent event) {
  int index = getPropertyIndex(event, "name");
  if (event.getOldState()[index] != event.getState()[index]) {
    // The name changed.
  }
}

 Hope this helps someone.

 

 

[spring-projects-issues]

Michael Sharman commented

I ended up detaching the object from the EntityManager - like this:

@RepositoryEventHandler(User.class)
public class UserAclEventHandler {
	@Autowired
	UserService userService;

	@PersistenceContext
  	private EntityManager entityManager;

	@HandleBeforeSave
	public void handleUserSave( User incoming ) {
		
		entityManager.detach(incoming);
		
		User existing = userService.loadUserById( incoming.getId() );

		userService.checkCrudOperationAllowed( existing );
		userService.checkCrudOperationAllowed( incoming );
		
		entityManager.merge(incoming);
	}
}