Skip to content

Conversation

@christophstrobl
Copy link
Member

We now offer dedicated methods for min (Number/Date) on Update.
As BigInteger and BigDecimal values are converted to String before storing in MongoDB it is not possible to use for the min operation. Therefore we type check both the given value and the property it should be applied to and raise an error if it fails.

This PR ist a first draft for discussion and should not be directly merged.

We now offer dedicated methods for ‘min’ (Number/Date) on Update. As BigInteger and BigDecimal values are converted to String before storing in MongoDB it is not possible to use for the min operation. Therefore we type check both the given value and the property it should be applied to and raise an error if it fails.
@thomasdarimont
Copy link

Rebased on master and adjusted the PR quite a bit.

Moved value field from KeywordContext into Keyword directly since it was always accessed directly. Made KeywordContext immutable. Validation now works with Keywords directly instead of using the KeywordContext. Introduced static factory method keywordFor(...) for constructing keywords.
Moved construction of validators to KeywordFactory. Reduced nesting in Keyword validation.
@thomasdarimont
Copy link

I adjusted the PR accordingly - any other issues?

@odrotbohm
Copy link
Member

The changes to the QueryMapper look rather involved. When I suggested the Keyword to take the responsibility for validating I was rather assuming something like this:

class Keyword { // might need additional parameters void validate(PersistentProperty<?> property, Object value) {} }

A factory method for keywords could now either use the non-validating default instance or detect things like $min or $max to use a dedicated subclass (e.g. ComparableValueEnsuringKeyword) that would instantaneously reject the value if it doesn't match the implemented requirements.

Am I overseeing something that justifies the additional complexity?

@christophstrobl
Copy link
Member Author

I did an alternate approach without Validators in an another branch. We still can move the verification to Keyword - thoughts?

@mp911de
Copy link
Member

mp911de commented Apr 8, 2016

Superseded by DATAMONGO-1404 and #353.

@mp911de mp911de closed this Apr 8, 2016
@mp911de mp911de deleted the issue/DATAMONGO-941 branch May 26, 2020 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants