Skip to content

Grails 3.2.9 validate() method of domain object returns False when it used to return True on Grails 3.1.12 #14615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
4 tasks done
rafajaw opened this issue May 3, 2017 · 8 comments

Comments

@rafajaw
Copy link

rafajaw commented May 3, 2017

Task List

  • Steps to reproduce provided
  • Stacktrace (if present) provided
  • Example that reproduces the problem uploaded to Github
  • Full description of the issue provided (see below)

Steps to Reproduce

1 - Create a Domain class called Parent with a property Child.
2 - Create a Domain class called Child with dummy string as property and a back-reference as static belongsTo = [ parent : Parent ];
3 - Set a constraint on Child to parent( nullable: false ).
4 - Use databinding on parent to populated the child's dummy string.
5 - Call validate() method on parent - it will return TRUE on grails 3.1.12 and FALSE on 3.2.9.

Expected Behaviour

The call to validate() method on parent should always return TRUE for backwards compatibility and not breaking existing code.

Actual Behaviour

The call to validate() method on parent returns different values depending on the Grails version running.

Environment Information

  • Operating System: Windows 10 64 bits
  • Grails Version: 3.1.12 and 3.2.9 confirmed for inconsistency.
  • JDK Version: jdk1.8.0_121
  • Container Version (If Applicable): Tomcat8

Example Application

Example running with Grails 3.1.12 https://github.com/rafajaw/grails-validate-issue-a
Example running with Grails 3.2.9 https://github.com/rafajaw/grails-validate-issue-b

Additional notes

In further investigation we can see that validate() method fails in Grails 3.2.9 because the constraint "parent( nullable: false )" wasn't met in the child object.

However, if we call parent.save() then the back-reference from child to parent gets populated properly and the validate() method begins to return TRUE even for Grails 3.2.9. Which is not always the use-case, in some use-cases we want to explicitly call validate on the object without saving it.

@graemerocher
Copy link
Contributor

This statement:

"The call to validate() method on parent should always return TRUE for backwards compatibility and not breaking existing code."

Is incorrect, backwards compatibility is clearly important, but not a reason to not fix valid bugs and this may well have been one.

Having said that we will investigate and see if this is indeed a bug

@rafajaw
Copy link
Author

rafajaw commented May 4, 2017

I agree with you.

I honestly think this is a bug, because validate() returns FALSE but a call to save() returns TRUE and, as far as I've read the docs, save() internally calls validate() on its own, but maybe the thing is that save() will first set the back-reference object first and then the call to validate() will succeed. I will try to look for this in the source.

@sdelamo
Copy link
Contributor

sdelamo commented May 8, 2017

I have created a pullrequest for your repository with Grails 3.1.12:

rafajaw/grails-validate-issue-a#1

Another pull request for your repository with Grails 3.2.9:

rafajaw/grails-validate-issue-b#1

As you will see in the tests. Both versions behave the same way. I am going to close this issue.

Please @rafajaw if you are able to create reproducible failing tests, add them to those repositories and reopen the issue.

We published a guide about domain class validation which may be useful:

http://guides.grails.org/grails-test-domain-class-constraints/guide/index.html

@sdelamo sdelamo closed this as completed May 8, 2017
@rafajaw
Copy link
Author

rafajaw commented May 16, 2017

Hey @sdelamo, sorry for the late reply. The Tests passes indeed, but it fails on actual program execution, as an example I created a brand new project, created both Parent and Child domain classes, and put the following in BootStrap.groovy:
`package newtest

class BootStrap {

def init = { servletContext ->
	
    // This passes in Test, lets try in actual program execution.
    def map = [
            "child": ["dummy": "populated"]
    ]
    Parent parent  =  new Parent( map )
    println "parent.validate() result is -> " + parent.validate()
	
}
def destroy = {
}

}
`
And indeed the result is the following:

parent.validate() result is -> false

It really takes one minute to verify this, I don't know why the test is passing, but it's not working as expected during normal program flow, which might be an even bigger problem since it is passing the test.

@rafajaw
Copy link
Author

rafajaw commented May 23, 2017

Hi @sdelamo, don't you think this issue should be reopened? I can't create a Test showing it but I can create a very very simple app which shows it. Which IMO is even worse, because the Test passes but it won't work as expected.

@sdelamo
Copy link
Contributor

sdelamo commented May 23, 2017

I've uploaded a repository with the issue:

https://github.com/grails-core-issues-forks/grails-core-issue-10621

Workaround: You could use something like this:

       def map = [
                "child": ["dummy": "populated"]
        ]
        def child = new Child(map.child)
        Parent parent  =  new Parent(child: child)
        child.parent = parent
       assert parent.validate()

@sdelamo sdelamo reopened this May 23, 2017
@sdelamo sdelamo self-assigned this May 23, 2017
@rafajaw
Copy link
Author

rafajaw commented May 25, 2017

Thanks @sdelamo! It's really cool that you took the time to dig into this. Unfortunately for me, the proposed workaround is not practical in my current setup. Do you foresee a fix in the next releases?

@rafajaw
Copy link
Author

rafajaw commented Jul 18, 2017

The proposed workaround is not practical in most situations because most of the time we are making use of the autobinding feature with deep nested objects. I feel my team is about regret choosing Grails for our project mainly because of this inconsistency between releases. I think if there were enough tests, this one would get caught. Anyway, I know you are doing and exceptional job for the community @sdelamo and you are not obligated to help anyone out there, but it would be great if you could give us a hint if this is something that you plan to look up further, or maybe I should dig into this myself, or should I drop back to my previous working version of Grails (which unfortunally lacks some features in this one but that I can deal with).

@jameskleeh jameskleeh transferred this issue from apache/grails-core Nov 1, 2018
@sdelamo sdelamo removed their assignment Jul 23, 2020
@jdaugherty jdaugherty transferred this issue from apache/grails-data-mapping Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants