-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Introduce ValidatingEntityCallback
and deprecate ValidatingMongoEventListener
#4910
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
Conversation
rfelgent
commented
Mar 2, 2025
•
edited
Loading
edited
- You have read the Spring Data contribution guidelines.
- You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
- You submit test cases (unit or integration tests) that back your changes.
- You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).
* JSR-303 dependant entities validator. | ||
* <p> | ||
* When it is registered as Spring component its automatically invoked | ||
* after any {@link AbstractMongoEventListener} and before entities are saved in database. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mp911de , I copied the documentation from ValidatingMongoEventListener
and adjusted it for the ValidatingEntityCallback
.
Is the statement regarding automatic invocation still valid?
|
||
private static final Log LOG = LogFactory.getLog(ValidatingEntityCallback.class); | ||
|
||
// TODO: create a validation handler (similar to "AuditingHandler") an reference it from "ValidatingMongoEventListener" and "ValidatingMongoEventListener" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally, I could live with this implementation (and duplicate the validation code logic of ValidatingMongoEventListener
)
@Override | ||
public Object onBeforeSave(Object entity, Document document, String collection) { | ||
|
||
if (LOG.isDebugEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls tell me, is "debug" or "trace" the appropriate choice for bean validation ?
ValidatingEntityCallback
and deprecate ValidatingMongoEventListener
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot. For validation, we should also consider the reactive flow that should not throw exceptions but rather return Mono.error(e)
. I think it makes sense to extract a tiny superclass returning Set<ConstraintValitation>
and let the callback and event listener reuse those bits. We could then get rid of logging in each component.
Admittedly, it's not much code, but a few concerns that we do not require in subclasses.
Also, we should deprecate the event listener and ideally find a place in the reference documentation to mention Bean Validation support.
Let me know how comfortable you are addressing these aspects and what guidance I can provide you with.
agreed. I will add a parent class and make
agreed, should the introduction of a reactive-based entity validor be part of this pull request or another one?
agreed, should this be part of this pull request or another one ? |
Signed-off-by: felgentraeger <[email protected]>
Signed-off-by: felgentraeger <[email protected]>
Signed-off-by: felgentraeger <[email protected]>
Signed-off-by: felgentraeger <[email protected]>
Signed-off-by: felgentraeger <[email protected]>
46051a6
to
c0e7bfb
Compare
Let's handle everything right here, no need to spread this enhancement over multiple tickets as these changes sum up nicely in a single logical change. |
Thank you for your contribution. That's merged and polished now. I extracted a few bits and tweaked the documentation slightly, adding a reactive callback variant. |
Closes #4901 Original pull request: #4910 Signed-off-by: felgentraeger <[email protected]>