Hi,
plenty of tests and
functionalities of our apps fail after upgrade from 2.4.2 to 2.4.3.
This is mostly caused by command objects with methods which names start with
"getXxx". They are treated as validateable not nullable properties.
Not only command object has errors on those 'properties' but sometimes also
database queries or other not trivial operations are called just to check if method returns something. This
seems like a big change and probably not only for our apps. Is this not to
dangerous?
Maybe 'transient' methods should be supported in
@Validateable objects?
thanks, droggo
--
You received this message because you are subscribed to the Google Groups "Grails Dev Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to grails-dev-disc...@googlegroups.com.
To post to this group, send email to grails-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/grails-dev-discuss/63fe4332-f91f-423f-9cfd-6a8fb776f4ee%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
On August 1, 2014 at 3:05:44 PM, drog...@gmail.com (drog...@gmail.com) wrote:
Hi,
plenty of tests and functionalities of our apps fail after upgrade from 2.4.2 to 2.4.3.
This is mostly caused by command objects with methods which names start with "getXxx". They are treated as validateable not nullable properties.
When I replied earlier I was reading/typing on my phone and had not read the original note carefully enough. I apologize for that.
Command object properties are supposed to be nullable:false by default. If you define a method like getFirstName(), you are defining a property (that is just 1 way to define a property). There have been some bugs around this but that is the documented and intended behavior. One of those bugs was recently resolved and I suspect that you were taking advantage of the bug before and now that it is fixed, your code might need to be addressed to deal with that.
Not only command object has errors on those 'properties' but sometimes also database queries or other not trivial operations are called just to check if method returns something.
I don’t think that the framework every sends a query to the database in order to check if a method returns something. Can you elaborate on that?
Maybe 'transient' methods should be supported in @Validateable objects?
@Validateable
ManagerCommand {
Manager manager
Collection getEmployees(){
Employee.findAllByManagerAndActive(manager, true)
}
int getEmployeeCount(){
Employee.countAllByManagerAndActive(manager, true)
}
static constraints = {
manager nullable:false
}
}
I completely agree with Droggo, we use the command objects the same way as he does and this bug fix is a breaking change for us.
I think you should reconsider this or at least provide a way such as transients or something like that. If not, the only solution would be to rename this methods:
Instead of using manager.employees, manager.employeeCount. We can use manager.fetchEmployees(), manager.fetchEmployeeCount().
I prefer the first one.
Regards, Iván.
--
Sent from my Nexus 10
nullable: true
argument" - http://grails.org/doc/2.4.x/guide/upgradingFrom23.htmlOn August 3, 2014 at 11:00:48 AM, drog...@gmail.com (drog...@gmail.com) wrote:
I can create JIRA with tests for above if this will help
I started looking at this this afternoon. See https://jira.grails.org/browse/GRAILS-11625 and https://github.com/grails/grails-core/commit/2423424909cf933eddd85aee503a9dbe551b0791?w=true. If you want to work up any further tests to help validate the behavior, that would be great.
JSB
On August 4, 2014 at 1:48:38 AM, drog...@gmail.com (drog...@gmail.com) wrote:
If we add constraint "age validator:{val -> val > 0}" framework automatically adds "nullable:false" constraint. This happens both in 2.3.x and in 2.4.x. Albo for both @Validatateable and @Validateable(nullable = true). Theoretically it is not valid, because custom validator may not require nullable:false, and I couldn't find such functionality description in docs
If that is happening for @Validateable(nullable=true) then that is a bug and we can fix that but for @Validateable() (without nullable=true), I think that is the correct behavior. All properties are by default supposed to be configured with nullable:false unless you express otherwise.
Thank you for the fast change Jeff. I added small pull request to the Spec. I also described one more concern in the pull request description:
If we add constraint "age validator:{val -> val > 0}" framework automatically adds "nullable:false" constraint. This happens both in 2.3.x and in 2.4.x. Albo for both @Validatateable and @Validateable(nullable = true). Theoretically it is not valid, because custom validator may not require nullable:false, and I couldn't find such functionality description in docs
I think the assertion you added at https://github.com/grails/grails-core/commit/3dcab35ba5fcae0f38d1b97934077acfd0b0393b#diff-ad85ae603aa15d04fef1f20cb95c59deR166 is consistent with what happens but inconsistent with what should happen. I think the age property should in fact be nullable since the class is marked with @Validateable(nullable=true). Is that correct?
In all cases, if nullable:true or nullable:false is explicitly expressed in the constraints block, it should be respected.
For classes marked @Validateable() or @Validateable(nullable=false):
- Any property that is not configured with a nullable constraint should be configured with nullable:false
For classes marked with @Validateable(nullable=true):
- Any constrained property that is not configured with a nullable constraint should be configured with nullable:true
- Any unconstrained property should be left unconstrained and as such, that properties' value will not need to be retrieved at validation time.
What say ye? I have code worked up that satisfies those requirements.