save(validate: false) still validates on flush

[docbee-fschrader]

Expected Behavior

It should be possible to save a domain that has fields that are invalid based on grails validation by setting validate:false

Actual Behaviour

The validation is triggered on flush and the domain isn't persisted

Steps To Reproduce

I created an example project (based on the demo project generated on the grails website) with an integration test to demonstrate the issue. To run the test, create a postgresql database testDb for user test with password test.

Environment Information

• Grails 7.0.4
• Java 17.0.15
• PostgreSQL 17

Example Application

https://github.com/docbee-fschrader/grails-7-validation-bug

Version

7.0.4

[jdaugherty]

This has been brought up before; if you need to save it, save it with validate(false) and then discard the object so it's not in the session anymore. It's a design feature of grails to flush any objects currently in the session - calling discard should remove it from the session and allow the behavior you desire.

[docbee-fschrader]

The same test runs without errors in grails 6 though. So there has been some change in the behavior, making it difficult to upgrade from to grails 7. It is fine that the objects are flushed, but maybe they don't have to be validated if they have been saved with validate:false?

[jdaugherty]

apache/grails-data-mapping#1155 is the ticket documenting this change. apache/grails-data-mapping#1838 & grails/grails-data-hibernate5#913 are the pull requests for what caused this change in behavior. It was a bug fix to ensure objects were validated correctly.

If you change the save() to:

        example.save(validate: false, flush: true)
        example.discard()

the test passes in Grails 7. Are you saving invalid data that often in your application?

[docbee-fschrader]

Thanks for searching the issue that caused the change, I can see why a change was necessary.

There are only a few cases in the application I'm working on where invalid data is saved. I'd be fine discarding the domain in that specific case.

However we have a lot of cases where we skip validation because some of our validators are expensive and we know the object we're saving is valid. It would be very inconvenient (and bad for our code quality) to add a discard in all those places. That's why I was asking, whether it is really necessary to validate all domains (including the ones saved with validate:false), when the transaction is flushed.

[jdaugherty]

Playing devil's advocate here: how do we know what's valid & what's not (or in other words, which are expensive and which are not)? It looks like the bug fix was once skip validate was applied, it was applying to all domains (clearly wrong).

It sounds like you're asking for a way to opt out of specific domain validation or a way to turn off validation for an entire transaction, yes? I'm assuming the expensive validations are mostly closures? Wouldn't it be easier to handle this at the validator level, or more specifically the constraint level? You could add a custom constraint that invokes your closures based on a local thread variable and then you could decide when to opt-in or opt-out based on setting that variable.

[jdaugherty]

Separate from further discussion, I am going to look at disabling further validation once an object was saved with validate: false. Having input on the previous comment would help me understand if further changes also need to be made here.

[docbee-fschrader]

What would be ideal, would be a way of disabling validation for a specific domain in a transaction.

Most of our validators, that we want to skip are closures, some are also custom constraints.

An example for a case where we skip validation: We have a root object with children. One of the children is updated (we would validate the child in that case). However we need to update a cache property (e.g. a modified date) that doesn't have a validator on the root domain. We want to skip validation for the root domain in that case because it has expensive validators for other properties that haven't been updated.

[jdaugherty]

I was thinking of several features after this discussion:

1. save(validate: false) => flags that specific instance to not validate on flush
2. @Transactional will have a new attribute skipValidateOnFlush - false is the default. if true, we could either skip validation on all flushes or only at the end of the transaction
3. @Transactional will have a new attribute skipValidateFor - defaults to empty list. Takes a list of domain classes that will simply not validate at all on save or flush.
4. Suggest that the user modifies their custom constraint to bypass that validation based on their desired behavior.

Thoughts on these options:

• The problem with 1. is how do you handle cascades?
• For 2, we would need to decide if its only the end of transaction flush or any flush (thus, only calling save explicitly would validate).
• 3 is probably the easiest since it's on / off and very clear what happens.
• obviously easier for Grails, inconvenient for end users.

It sounds like 3 would solve the general problem here though & it's actually the most well defined. Do you agree?

[docbee-fschrader]

My thoughts:

1. That would be my preference because it would restore the behavior from grails 6 and require no changes when upgrading to grails 7. As to your question, how to handle cascades: Should that maybe be decided by the domain mapping property cascadeValidate?
2. If I were to use the attribute, I would expect it to skip validation on all flushes. That option would require changes in the process of updating an app from grails 6 to grails 7. My question for that option would be, whether it's possible to set the attribute for a withNewTransaction closure?
3. I agree, that it's clear, but it can potentially require a lot of changes when updating an app from grails 6 to grails 7. There could be cases, where it isn't decidable when starting the transaction, whether validation for a domain should be skipped or not. A simple example of that would be a method that allows to update two properties and only skips validation, if one of the properties isn't updated (more complex instances of that scenario exist in the project I'm working on). Options 1. and 2. seem more flexible because the decision of validating or not is made when save is called.
4. As an end user I agree it would be inconvenient 😅