Batch 3: SearchByTreeWalkingMixin sizeLimit and ModifyOp round-trip

docs/source/NEWS.rst

@@ -64,6 +64,18 @@ Fixes
   iterator's Deferred was silently discarded, so back ends whose
   ``children`` / ``subtree`` methods actually behaved asynchronously
   returned an empty result set (#62).
+- ``SearchByTreeWalkingMixin.search`` now honors ``sizeLimit``,
+  truncating the result set to the requested number of matches.
+  Early termination of the tree walk itself would require an
+  interruptible iterator, which the current mixin does not expose, so
+  the whole tree is still walked -- but the network/client cost is
+  bounded (#234).
+- ``delta.Modification.asLDAP`` no longer returns wire-encoded bytes;
+  it returns the ``BERSequence`` object that
+  ``LDAPModifyRequest.modification`` and ``ModifyOp.fromLDAP`` already
+  document and expect. This fixes ``ValueError: too many values to
+  unpack (expected 2)`` when round-tripping
+  ``ModifyOp -> asLDAP -> fromLDAP`` (#223).
 
 
 21.2.0 (2021-02-28)

ldaptor/delta.py

@@ -41,7 +41,7 @@ def asLDAP(self):
                     ]
                 ),
             ]
-        ).toWire()
+        )
 
     def __eq__(self, other):
         if not isinstance(other, self.__class__):

ldaptor/entryhelpers.py

@@ -297,10 +297,17 @@ def iterateSelf(callback):
         else:
             matchCallback = callback
 
-        # gather results, send them
+        # ponytail: sizeLimit truncates results but still walks the whole tree;
+        # early-stop needs an iterator that supports cancellation, which the
+        # current subtree/children mixin doesn't.
+        matched = [0]
+
         def _tryMatch(entry):
+            if sizeLimit and matched[0] >= sizeLimit:
+                return
             if entry.match(filterObject):
                 matchCallback(entry)
+                matched[0] += 1
 
         d = defer.maybeDeferred(iterator, callback=_tryMatch)
         if callback is None:

ldaptor/test/test_delta.py

@@ -643,3 +643,31 @@ def testEquality_List_False(self):
         a = delta.Add("k", ["b", "c", "d"])
         b = ["b", "c", "d"]
         self.assertNotEqual(a, b)
+
+
+class TestModifyOpRoundtrip(unittest.TestCase):
+    """
+    ``ModifyOp.asLDAP`` must produce an ``LDAPModifyRequest`` that
+    ``ModifyOp.fromLDAP`` can parse back into an equal ``ModifyOp``.
+
+    Previously ``Modification.asLDAP`` called ``.toWire()``, which left
+    ``LDAPModifyRequest.modification`` as a list of raw bytes rather than
+    the list of ``BERSequence`` objects that both ``fromLDAP`` and the
+    ``LDAPModifyRequest`` docstring assume, so the round-trip raised
+    ``ValueError: too many values to unpack (expected 2)`` (#223).
+    """
+
+    def test_add_delete_replace_roundtrip(self):
+        # Use bytes values so equality matches after the wire trip;
+        # LDAP attribute values are bytes at rest and fromLDAP does not
+        # re-decode them to str (that ambiguity is a separate concern).
+        original = delta.ModifyOp(
+            "cn=xander,dc=example,dc=com",
+            [
+                delta.Add("mail", [b"a@example.com", b"b@example.com"]),
+                delta.Delete("description"),
+                delta.Replace("cn", [b"Xander"]),
+            ],
+        )
+        parsed = delta.ModifyOp.fromLDAP(original.asLDAP())
+        self.assertEqual(parsed, original)

ldaptor/test/test_inmemory.py

@@ -382,6 +382,21 @@ def testSearch_withoutCallback(self):
         )
         return d
 
+    def testSearch_sizeLimit_truncatesResults(self):
+        """
+        A search with sizeLimit=N returns at most N entries. Previously
+        sizeLimit was silently ignored (#234).
+        """
+        d = self.root.search(filterText="(|(cn=foo)(cn=bar))", sizeLimit=1)
+        d.addCallback(lambda actual: self.assertEqual(len(actual), 1))
+        return d
+
+    def testSearch_sizeLimitZero_returnsAll(self):
+        """sizeLimit=0 means no limit (LDAP convention)."""
+        d = self.root.search(filterText="(|(cn=foo)(cn=bar))", sizeLimit=0)
+        d.addCallback(lambda actual: self.assertEqual(len(actual), 2))
+        return d
+
     def test_move_noChildren_sameSuperior(self):
         d = self.empty.move("ou=moved,dc=example,dc=com")