work on top of - Fix inc/dec rejecting deltas outside a field's min_value/max_value

AUTHORS

@@ -266,3 +266,4 @@ that much better:
  * Terence Honles (https://github.com/terencehonles)
  * Sean Bermejo (https://github.com/seanbermejo)
  * Juan Gutierrez (https://github.com/juannyg)
+ * DSeaStar (https://github.com/DSeaStar)

docs/changelog.rst

@@ -9,7 +9,7 @@ Development
 - (Fill this out as you fix issues and develop your features).
 
 Changes in 1.0.0
-===========
+================
 - Add support for transaction through run_in_transaction (kudos to juannyG for this) #2569
 
   Some considerations:
@@ -33,6 +33,7 @@ Changes in 1.0.0
 - BREAKING CHANGE: The obsolete ``slaves`` and ``is_slave`` connection options were silently ignored since 2014 and will now raise ``ConnectionFailure`` if provided #2920.
 - BugFix - Calling .clear on a ListField wasn't being marked as changed (and flushed to db upon .save()) #2858
 - Improve error message in case a document assigned to a ReferenceField wasn't saved yet #1955
+- Fix inc/dec atomic updates rejecting deltas outside a field's min_value/max_value #2339
 - BugFix - Take `where()` into account when using `.modify()`, as in MyDocument.objects().where("this[field] >= this[otherfield]").modify(field='new') #2044
 
 Changes in 0.29.3

docs/guide/querying.rst

@@ -596,6 +596,7 @@ There are several different "modifiers" that you may use with these methods:
 * ``min`` -- update only if value is smaller
 * ``inc`` -- increment a value by a given amount
 * ``dec`` -- decrement a value by a given amount
+* ``mul`` -- multiply a value by a given amount
 * ``push`` -- append a value to a list
 * ``push_all`` -- append several values to a list
 * ``pop`` -- remove the first or last element of a list `depending on the value`_
@@ -604,6 +605,13 @@ There are several different "modifiers" that you may use with these methods:
 * ``add_to_set`` -- add value to a list only if its not in the list already
 * ``rename`` -- rename the key name
 
+.. note::
+
+    The operands passed to ``inc``, ``dec``, and ``mul`` are deltas or
+    multipliers, not replacement field values. MongoEngine therefore does not
+    validate them against the field's ``min_value`` or ``max_value`` as this would require a lookup. These
+    atomic updates can leave the stored value outside those bounds.
+
 .. _need to add upsert=True: http://docs.mongodb.org/manual/reference/operator/update/setOnInsert
 .. _depending on the value: http://docs.mongodb.org/manual/reference/operator/update/pop/
 

mongoengine/base/fields.py

@@ -236,7 +236,9 @@ def _to_mongo_safe_call(self, value, use_db_field=True, fields=None):
 
     def prepare_query_value(self, op, value):
         """Prepare a value that is being used in a query for PyMongo."""
-        if op in UPDATE_OPERATORS:
+        # Do not validate $inc/$mul operands against stored-value min/max bounds.
+        # dec is normalized to inc with a negative value before this point.
+        if op in UPDATE_OPERATORS and op not in ("inc", "mul"):
             self.validate(value)
         return value
 

mongoengine/queryset/transform.py

@@ -365,7 +365,8 @@ def update(_doc_cls=None, **update):
                     value = field.prepare_query_value(op, value)
             elif op == "unset":
                 value = 1
-            elif op == "inc":
+            # dec is normalized to inc above.
+            elif op in ("inc", "mul"):
                 value = field.prepare_query_value(op, value)
 
         if match:

tests/queryset/test_queryset.py

@@ -2148,6 +2148,55 @@ class BlogPost(Document):
         post.reload()
         assert post.hits == 11
 
+    def test_update_number_operators__operand_outside_bounds__bypasses_validation(self):
+        """inc, dec, and mul operands are not stored values and bypass bounds."""
+
+        class Counter(Document):
+            number = IntField(min_value=1, max_value=5)
+
+        Counter.drop_collection()
+        counter = Counter(number=1).save()
+
+        Counter.objects.update(inc__number=5)
+        counter.reload()
+        assert counter.number == 6
+
+        Counter.objects.update(dec__number=6)
+        counter.reload()
+        assert counter.number == 0
+
+        counter.number = 2
+        counter.save()
+
+        Counter.objects.update(mul__number=10)
+        counter.reload()
+        assert counter.number == 20
+
+    def test_update_inc__numeric_string__converts_to_number(self):
+        """inc converts a numeric string through IntField before updating."""
+
+        class Counter(Document):
+            number = IntField()
+
+        Counter.drop_collection()
+        counter = Counter(number=1).save()
+
+        Counter.objects.update(inc__number="5")
+        counter.reload()
+        assert counter.number == 6
+
+    def test_update_inc__string_field__relies_on_server_validation(self):
+        """MongoDB rejects inc after MongoEngine skips client-side validation."""
+
+        class Person(Document):
+            name = StringField()
+
+        Person.drop_collection()
+        Person(name="Alice").save()
+
+        with pytest.raises(OperationError):
+            Person.objects.update(inc__name=1)
+
     def test_update_decimalfield_operator(self):
         class BlogPost(Document):
             review = DecimalField()

tests/queryset/test_transform.py

@@ -1,6 +1,7 @@
 import unittest
 
 import pytest
+from bson.decimal128 import Decimal128
 from bson.son import SON
 
 from mongoengine import *
@@ -83,6 +84,48 @@ class BlogPost(Document):
         update = transform.update(BlogPost, push_all__tags=["mongo", "db"])
         assert update == {"$push": {"tags": {"$each": ["mongo", "db"]}}}
 
+    def test_transform_update_inc_dec_ignores_min_max(self):
+        """inc/dec pass a delta; min_value/max_value apply to stored values (#2339)."""
+
+        class Account(Document):
+            amount = FloatField(min_value=0, required=True)
+            count = IntField(min_value=0, max_value=100)
+            money = DecimalField(min_value=0)
+            money128 = Decimal128Field(min_value=0)
+
+        update = transform.update(Account, dec__amount=10)
+        assert update == {"$inc": {"amount": -10.0}}
+
+        update = transform.update(Account, inc__count=200)
+        assert update == {"$inc": {"count": 200}}
+
+        update = transform.update(Account, mul__count="200")
+        assert update == {"$mul": {"count": 200}}
+
+        update = transform.update(Account, dec__count=5)
+        assert update == {"$inc": {"count": -5}}
+
+        update = transform.update(Account, dec__money=3)
+        assert update == {"$inc": {"money": -3.0}}
+
+        update = transform.update(Account, dec__money128=3)
+        assert update == {"$inc": {"money128": Decimal128("-3")}}
+
+        with pytest.raises(ValidationError):
+            transform.update(Account, set__amount=-1)
+
+        with pytest.raises(ValidationError):
+            transform.update(Account, set__count=101)
+
+    def test_transform_update__inc_on_string_field__skips_validation(self):
+        """inc leaves nonnumeric field validation to MongoDB."""
+
+        class Person(Document):
+            name = StringField()
+
+        update = transform.update(Person, inc__name=1)
+        assert update == {"$inc": {"name": 1}}
+
     def test_transform_update_no_operator_default_to_set(self):
         """Ensure the differences in behvaior between 'push' and 'push_all'"""