Skip to content

DATAMONGO-941 - Add support for Update $min. #244

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

Closed
wants to merge 4 commits into from

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants