grails / grails-core

The Grails Web Application Framework
http://grails.org
Apache License 2.0
2.78k stars 950 forks source link

Grails 4.0.3: Inherited domain with constraints throws error when validating on child property constraint #11527

Open Trebla7th opened 4 years ago

Trebla7th commented 4 years ago

When a domain hierarchy exists, having a constraint on the child class that checks a field of the child class will cause a grails error Property [xxx] is not a valid property of class example.<parentObject> when saving the child object.

This only occurs when the constraint is present, in the referenced example, if you remove the "unique" constraint on the child objects, they can be saved just fine. This worked in grails 3.

The error claims to come from the abstract parent object's validation, that the field does not exist on the object, even though it does exist on the concrete child object which was instantiated and saved.

Task List

Steps to Reproduce

  1. Run the referenced application
  2. Navigate to the "owner" controller default action
  3. See error in log

Remove the constraints from Cat and Dog classes, restart application and repeat steps, the error does not occur

In the attached application, PetObject is abstract, Dog extends PetObject and has an additional property that it belongs to called dogHouse

Dog class:

class Dog extends PetObject
{
  DogHouse dogHouse
  static belongsTo = DogHouse
  static constraints = {
    name(blank:false,unique:'dogHouse', maxSize:255)
  }
}

When attempting to create the object as follows

Dog dog = new Dog(name: "Rover", description: "Brown", petType: PetType.DOG, dogHouse: dogOwner)
dogOwner.addToDogs(dog)
dog.save()

The exception is thrown

Stack Trace:

IllegalArgumentException occurred when processing request: [GET] /owner/index
Property [dogHouse] is not a valid property of class example.PetObject. Stacktrace follows:

java.lang.reflect.InvocationTargetException: null
    at org.grails.core.DefaultGrailsControllerClass$ReflectionInvoker.invoke(DefaultGrailsControllerClass.java:211)
    at org.grails.core.DefaultGrailsControllerClass.invoke(DefaultGrailsControllerClass.java:188)
    at org.grails.web.mapping.mvc.UrlMappingsInfoHandlerAdapter.handle(UrlMappingsInfoHandlerAdapter.groovy:90)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
    at org.grails.web.servlet.mvc.GrailsWebRequestFilter.doFilterInternal(GrailsWebRequestFilter.java:77)
    at org.grails.web.filters.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:67)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IllegalArgumentException: Property [dogHouse] is not a valid property of class example.PetObject
    at org.grails.datastore.mapping.reflect.FieldEntityAccess$FieldEntityReflector.getPropertyReader(FieldEntityAccess.java:268)
    at org.grails.datastore.mapping.reflect.FieldEntityAccess$FieldEntityReflector.getProperty(FieldEntityAccess.java:286)
    at org.grails.datastore.gorm.validation.constraints.builtin.UniqueConstraint$_processValidate_closure1.doCall(UniqueConstraint.groovy:111)
    at org.grails.datastore.gorm.query.criteria.AbstractDetachedCriteria.build(AbstractDetachedCriteria.groovy:821)
    at grails.gorm.DetachedCriteria.build(DetachedCriteria.groovy:583)
    at org.grails.datastore.gorm.validation.constraints.builtin.UniqueConstraint.processValidate(UniqueConstraint.groovy:106)
    at org.grails.datastore.gorm.validation.constraints.AbstractConstraint.validate(AbstractConstraint.java:88)
    at grails.gorm.validation.DefaultConstrainedProperty.validate(DefaultConstrainedProperty.groovy:601)
    at grails.gorm.validation.PersistentEntityValidator.validatePropertyWithConstraint(PersistentEntityValidator.groovy:305)
    at grails.gorm.validation.PersistentEntityValidator.validate(PersistentEntityValidator.groovy:76)
    at org.grails.orm.hibernate.AbstractHibernateGormInstanceApi.save(AbstractHibernateGormInstanceApi.groovy:124)
    at org.grails.datastore.gorm.GormInstanceApi.save(GormInstanceApi.groovy:119)
    at org.grails.datastore.gorm.GormEntity$Trait$Helper.save(GormEntity.groovy:100)
    at example.OwnerController$_index_closure1.doCall(OwnerController.groovy:16)
    at grails.gorm.transactions.GrailsTransactionTemplate$2.doInTransaction(GrailsTransactionTemplate.groovy:94)
    at org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:140)
    at grails.gorm.transactions.GrailsTransactionTemplate.execute(GrailsTransactionTemplate.groovy:91)
    at org.grails.datastore.gorm.GormStaticApi.withTransaction(GormStaticApi.groovy:1014)
    at org.grails.datastore.gorm.GormStaticApi.withTransaction(GormStaticApi.groovy:966)
    at org.grails.datastore.gorm.GormStaticApi.withNewTransaction(GormStaticApi.groovy:927)
    at org.grails.datastore.gorm.GormEntity$Trait$Helper.withNewTransaction(GormEntity.groovy:954)
    at example.OwnerController.index(OwnerController.groovy:6)
    ... 13 common frames omitted

Expected Behaviour

Domain class should validate and save

Actual Behaviour

Error above is thrown

Environment Information

Example Application

https://github.com/Trebla7th/abstract-domain-validation-error

Trebla7th commented 4 years ago

Sorry for some of the extraneous things (like the type enum)... took me a while to figure out what was actually triggering the error.

billgonemad commented 4 years ago

I have also run into this bug while attempting to upgrade a grails 2.5 app to 4.0.3. Is there a work around at all?

abowsher commented 4 years ago

I am surprised that there's not more comments on this one, seems like it would affect a lot of people. We have sold our management on a Grails 4 upgrade, please don't let us down Grails!

niravassar commented 4 years ago

I see two areas where I dont think GORM features are being used properly.

  1. The constraint for unique:'dogHouse' is improperly used. By grails documentation, the value should be false or true. Here the field value is given. In order to define it correctly, it can be written as dogHouse(unique: true). When I alter the sample application with this change, it no longer gives an exception. Also I confirmed if you take out the unique constraint, the exception also goes away. With these two facts, it seems to me that the usage of it in the sample app is wrong.

  2. With an inheritance hierarchy for a PetObject and Dog, currently PetObject resides in the domain package. I turned on logSql with hibernate and the created table is pet_object. This indicates that that the PetObject is the class that is scanned to map. In addition, the unique constraint is the DSL is not translated to the database ddl. I believe the proper way is to move the PetObject to the groovy folder and leave Dog in domain. When I do this in the sample application, the dog table is correctly created in ddl, and all the constraints above are translated into ddl constraints.

As for the fact that it worked in Grails 3, this may be a consequence of hibernate and gorm not being as "smart" in previous versions, and now it requires a more clear definition of terms and placement of domain heirarchy.

niravassar commented 4 years ago

@puneetbehl please review the above and determine what we should do thanks

Trebla7th commented 4 years ago

I believe (1) is an incorrect interpretation. dogHouse should not be unique, but no two dogs can have the same name in the same doghouse. For example there can be "Rover" in dogHouse1 and "Rover" in dogHouse2. Perhaps this changed since grails 3, this application runs fine in grails 3.

For (2), I don't believe this is a constraint on the database, this is a grails constraint on the data model enforcing uniqueness within a parent object.

Again, this is a regression from grails 3 functionality (or possibly a change that I've misinterpreted). I am not in a location I can test your changes against samples to see if the resolve the issue until the end of next week.

Edit: Sorry, I'm rushing and didn't notice your comment about gorm/hibernate not being as smart. This is certainly possible, but I believe it's still a regression from documented functional intent.

Trebla7th commented 4 years ago

According to the grails docs - https://docs.grails.org/latest/ref/Constraints/unique.html

Uniqueness can be specified as in the example

You can also define multi-column unique constraints by declaring the other field(s) to be included as the parameter value. If there is one other field, specify its name, but if there are more than one use a List, for example:

Example:

group unique: 'department'
In this example the group name must be unique in one department but there might be groups with same name in different departments, i.e. the group name isn’t unique by itself.