Default nullable constraints for @Validateable

[Aaron]

Migrating to 2.4.0 and just realizing that this change is going to be a huge pain.

I appreciate that command objects and domain objects will work more or less the same, however for many command objects the default nullable behavior is preferable, especially if they aren't really backing a domain object in any way.

For example, we use command objects a lot for search parameters. So we'll have an InvoiceSearchCommand or something which has 5 or 6 fields which the user can search by. In that case, they are all optional so we have no constraints block at all. In 2.4, we are going to have to explicitly add each property to the constraints and mark it nullable: true. If someone adds a new property to the search command, they have to remember to add it to the constraints block as well.

I hate adding flags for the sake of compatibility, but this might be a case worth considering. I'm sure this change makes the importFrom work more reliably so perhaps it could inherit the domain class behavior when using importFrom only? I'm just trying to consider other reasonable options.

-Aaron

[zyro]

hmm.

i faced this just the reverse way recently when creating some
validateable objects for a rest api in 2.3.7 and was wondering why they
were nullable when not explicitly added to the constraints closure...

if it should be decided to add such a flag, please make sure the flag
works the same for @Validateable classes and domain objects (i.e. both
nullable true or false by default), and not some
config.validateablePropertiesNullableByDefault = true

zyro

> --
> 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
> <mailto:grails-dev-disc...@googlegroups.com>.
> To post to this group, send email to grails-de...@googlegroups.com
> <mailto:grails-de...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/grails-dev-discuss/b4e6afa6-58b8-4905-a4c8-d4c714711e79%40googlegroups.com
> <https://groups.google.com/d/msgid/grails-dev-discuss/b4e6afa6-58b8-4905-a4c8-d4c714711e79%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

[Aaron]

I just did a quick audit of our project. We have 238 command classes and only about 20% of them have a constraints block at all. 

I do agree that the default should hold regardless of whether or not a constraint is provided, for instance:

static constraints = {

    foo(minSize: 10)

}

should NOT change minSize to nullable: false (that just seems like a side effect).

Now, when using importFrom() on a domain object, that is where the problem comes in. Because the domain objects have the opposite behavior, you don't really get a proper import for the implicitly null properties on the domain object. You end up having to repeat yourself for the nullable: false properties or put explicit nullable: false entries into the domain object.

My first choice would be that when using importFrom() the default behavior mimic that of the domain class and essentially do what it's doing at them moment in 2.4.0. Otherwise, the default behavior of a command object should be nullable: true (even in cases where a constraint is provided such as minSize, etc.).

My second choice would be a config option for default parameters like you can do with domain classes:

grails.gorm.default.constraints = {
    '*'(nullable: true, size: 1..20)
}

So something like:

grails.validateable.default.constraints = {

    '*'(nullable: true)

}

would give us the 2.3.x behavior including the (annoying) behavior that adding ANY constraint effectively removes all of the defaults.

-Aaron

[Aaron]

Just curious if there's any opinion on this from the core team? 

If this behavior is going to stay as-is and no global override or default can be provided, I think many people will have a lot more of a challenge upgrading to 2.4 (ourselves included).

Rgds,

Aaron

[Jeff Scott Brown]

On May 18, 2014 at 9:15:00 PM, Aaron (lon...@gmail.com) wrote:
> Just curious if there's any opinion on this from the core team?
> 

We discussed this while evaluating https://jira.grails.org/browse/GRAILS-9686 and decided that the change was the right thing to do, but that it should not be included in a 2.3.8 or 2.3.9 release so we moved the change to 2.4 and described the new behavior in the upgrading section of the User Guide at http://grails.org/doc/2.4.0.RC2/guide/upgradingFrom23.html.

Over the years we have had people ask for the default to be nullable: true and folks ask for the default to be nullable: false.  The behavior in 2.4 is probably the best default but it won’t be optimal for every application of course.  We can look at options to allow the default to be configurable, but at this late hour it isn’t likely to be included in 2.4.0.



JSB
—
Jeff Scott Brown
jbr...@gopivotal.com

Find The Cause ~ Find The Cure
http://www.autismspeaks.org/

[Aaron]

Well, without the default this becomes a huge breaking change. As I started going through each of our command objects and adding the nullable: true, it just doesn't feel right given that (as I said before) the majority of them don't need it. I feel like I'm just repeating the property list in the constraints block. I think this is because we make heavy use of Command objects for 'search' pages where they tend to always be nullable.

I've opened a ticket, I might try to invest some time in fixing it if you guys are amenable to having a default option:

https://jira.grails.org/browse/GRAILS-11416

Thanks,

Aaron

[Jeff Scott Brown]

On May 19, 2014 at 2:39:23 PM, Aaron (lon...@gmail.com) wrote:
>
>
> I've opened a ticket, I might try to invest some time in fixing it if you
> guys are amenable to having a default option:
>
> https://jira.grails.org/browse/GRAILS-11416
>
> Thanks,
> Aaron
> 

Considering a configuration option to allow the default to be controlled is on the table.  I think it is probably a good idea.  Go ahead and invest the time if you like.  If you have questions, ask here.  

[Aaron]

It turned out to be a pretty simple implementation to add a nullable parameter to the Validateable annotation. That allows you to explicitly annotate a command object and indicate:

@Validateable(nullable = true)

I debated what to call the property (nullable, defaultNullable, ?) but nullable was most succinct. I think this would be good compromise and would make the upgrade much less painful and repetitive. Obviously if you don't provide the annotation, the default behavior is nullable false (added as a parameter to injectValidateableCode in DefaultASTValidateableHelper).

Thoughts?

On a different note, I have contributor access to grails-core but I've never actually used it to commit or merge a change directly. I'm not sure what the process or convention is for making direct contributions. Obviously, I could update my fork and open a pull request but that seems somewhat roundabout. 

-Aaron

[Jeff Scott Brown]

Send a pull request. Please don't commit anything for this to the 2.4.x or master branches right now. 

Thanks. 

JSB

Sent from my iPhone

--
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/b69774c8-22a2-4656-9e4e-0a53a2074156%40googlegroups.com.

[Aaron]

Sure:

https://github.com/grails/grails-core/pull/499

The change is pretty minimal and a test case is provided so hopefully this can be considered for a 2.4.0GA release.

If it gets the green light, I can submit a pull request for the grails-doc as well.

-Aaron

[Jeff Scott Brown]

I am looking at it from a phone right now but at a glance I think it is probably a good thing. One of us will look more closely tonight or tomorrow. 

Thanks. 

JSB

Sent from my iPhone

--
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/ecc5a9c9-41f1-406b-be6a-b982b813d5f0%40googlegroups.com.

[Jeff Scott Brown]

On May 19, 2014 at 4:43:19 PM, Aaron (lon...@gmail.com) wrote:
> 
> If it gets the green light, I can submit a pull request for the grails-doc
> as well.
> 

If you could submit a PR to the user guide today then we can likely include it.

Thanks for the help.