From 5b0d0b30fd2e3ffb8d1f07d58843d80c16d59151 Mon Sep 17 00:00:00 2001 From: graemerocher Date: Wed, 21 Mar 2012 12:58:18 +0100 Subject: [PATCH] partial fix and tests for GRAILS-8922 "Hibernate AssertionFailure when flushing with a new invalid child object in a hasMany or hasOne relationship" --- .../AbstractSavePersistentMethod.java | 25 +++- .../hibernate/InvalidChildFlushingSpec.groovy | 135 ++++++++++++++++++ 2 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 grails-test-suite-persistence/src/test/groovy/org/codehaus/groovy/grails/orm/hibernate/InvalidChildFlushingSpec.groovy diff --git a/grails-hibernate/src/main/groovy/org/codehaus/groovy/grails/orm/hibernate/metaclass/AbstractSavePersistentMethod.java b/grails-hibernate/src/main/groovy/org/codehaus/groovy/grails/orm/hibernate/metaclass/AbstractSavePersistentMethod.java index 4057f98bce2..a77fc9caba3 100644 --- a/grails-hibernate/src/main/groovy/org/codehaus/groovy/grails/orm/hibernate/metaclass/AbstractSavePersistentMethod.java +++ b/grails-hibernate/src/main/groovy/org/codehaus/groovy/grails/orm/hibernate/metaclass/AbstractSavePersistentMethod.java @@ -28,6 +28,7 @@ import java.util.Set; import java.util.regex.Pattern; +import org.apache.commons.beanutils.PropertyUtils; import org.codehaus.groovy.grails.commons.*; import org.codehaus.groovy.grails.lifecycle.ShutdownOperations; import org.codehaus.groovy.grails.orm.hibernate.HibernateDatastore; @@ -36,6 +37,7 @@ import org.codehaus.groovy.grails.validation.CascadingValidator; import org.grails.datastore.mapping.engine.event.ValidationEvent; import org.hibernate.SessionFactory; +import org.springframework.beans.BeanUtils; import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.InvalidPropertyException; @@ -174,7 +176,7 @@ protected Object doInvokeInternal(final Object target, Object[] arguments) { } if (errors.hasErrors()) { - handleValidationError(target,errors); + handleValidationError(domainClass,target,errors); boolean shouldFail = shouldFail(application, domainClass); if (argsMap != null && argsMap.containsKey(ARGUMENT_FAIL_ON_ERROR)) { shouldFail = GrailsClassUtils.getBooleanFromMap(ARGUMENT_FAIL_ON_ERROR, argsMap); @@ -278,13 +280,28 @@ private void autoRetrieveAssocations(GrailsDomainClass domainClass, Object targe * if a validation error occurs. If save() is called again and validation passes the code will check if there * is a manual flush mode and flush manually if necessary * + * * @param target The target object that failed validation - * @param errors The Errors instance - * @return This method will return null signaling a validation failure + * @param o + *@param errors The Errors instance @return This method will return null signaling a validation failure */ - protected Object handleValidationError(final Object target, Errors errors) { + protected Object handleValidationError(GrailsDomainClass domainClass, final Object target, Errors errors) { // if a validation error occurs set the object to read-only to prevent a flush setObjectToReadOnly(target); + if(domainClass instanceof DefaultGrailsDomainClass) { + DefaultGrailsDomainClass dgdc = (DefaultGrailsDomainClass) domainClass; + List associations = dgdc.getAssociations(); + for (GrailsDomainClassProperty association : associations) { + if(association.isOneToOne() || association.isManyToOne()) { + BeanWrapper bean = new BeanWrapperImpl(target); + Object propertyValue = bean.getPropertyValue(association.getName()); + if(propertyValue != null) { + setObjectToReadOnly(propertyValue); + } + + } + } + } setErrorsOnInstance(target, errors); return null; } diff --git a/grails-test-suite-persistence/src/test/groovy/org/codehaus/groovy/grails/orm/hibernate/InvalidChildFlushingSpec.groovy b/grails-test-suite-persistence/src/test/groovy/org/codehaus/groovy/grails/orm/hibernate/InvalidChildFlushingSpec.groovy new file mode 100644 index 00000000000..6e634a9511f --- /dev/null +++ b/grails-test-suite-persistence/src/test/groovy/org/codehaus/groovy/grails/orm/hibernate/InvalidChildFlushingSpec.groovy @@ -0,0 +1,135 @@ +package org.codehaus.groovy.grails.orm.hibernate + +import grails.persistence.Entity +import spock.lang.Issue +import spock.lang.Ignore +import spock.lang.FailsWith +import org.hibernate.cfg.NotYetImplementedException +import org.hibernate.AssertionFailure +import org.hibernate.FlushMode + +/** + */ +@Issue('GRAILS-8922') +class InvalidChildFlushingSpec extends GormSpec{ + + @Issue('GRAILS-8922') + void "test one-to-many with new invalid child object and call to save() on child object"() { + + given:"A domain with a one-to-many association" + def author = new InvalidChildFlushingAuthor(name: 'author1') + author.save(failOnError: true, flush: true) + + def book = new InvalidChildFlushingBook(author: author, name: 'invalid') + author.addToBooks(book) + + + + when:"An object with an invalid child is saved and the session is flushed" + assert !book.save() + + + then:"The flush mode is manual" + session.flushMode == FlushMode.MANUAL + + when:"The session is flushed" + session.flush() + + // problem continues to exist in Hibernate, if this starts passing problem fixed in Hibernate + then:"An error thrown" + thrown AssertionFailure + } + + + @Issue('GRAILS-8922') + void "test one-to-one with new invalid child object"() { + given:"A domain with a one-to-one association" + def face = new InvalidChildFlushingFace() + face.save(failOnError: true, flush: true) + + def nose = new InvalidChildFlushingNose(face: face, size: 'invalid') + face.nose = nose + + when:"An object with an invalid child is saved and the session is flushed" + + assert !nose.save() + + + + then:"No error occurs" + nose != null + InvalidChildFlushingNose.count () == 0 + session.flushMode == FlushMode.MANUAL + + when:"The session is flushed" + session.flush() + + // problem continues to exist in Hibernate, if this starts passing problem fixed in Hibernate + then:"An error thrown" + thrown AssertionFailure + } + + @Issue('GRAILS-8922') + void "test one-to-one with existing valid child object changed to invalid"() { + given:"A domain with a one-to-one association" + def face = new InvalidChildFlushingFace() + face.save(failOnError: true, flush: true) + + def nose = new InvalidChildFlushingNose(face: face, size: 'valid') + face.nose = nose + nose.save(failOnError: true, flush: true) + + when:"An object with an invalid child is updated and the session is flushed" + nose.size = 'invalid' + assert !nose.save() + + session.flush() + + then:"No errors occured" + nose != null + } + + @Override + List getDomainClasses() { + return [InvalidChildFlushingAuthor, InvalidChildFlushingBook, InvalidChildFlushingFace, InvalidChildFlushingNose] + } +} +@Entity +class InvalidChildFlushingAuthor { + String name + + static hasMany = [books:InvalidChildFlushingBook] +} + +@Entity +class InvalidChildFlushingBook { + static belongsTo = [author:InvalidChildFlushingAuthor] + + String name + + static constraints = { + name(validator: {it != 'invalid'}) + } +} + +@Entity +class InvalidChildFlushingFace { + static hasOne = [nose:InvalidChildFlushingNose] + + static constraints = { + nose(nullable: true) + } + +} + +@Entity +class InvalidChildFlushingNose { + static belongsTo = [face:InvalidChildFlushingFace] + + String size + + static constraints = { + size(validator: {it != 'invalid'}) + } +} +