From 2158d74fafcf97d54a110eef29a0986620ffda21 Mon Sep 17 00:00:00 2001 From: Bret Curtis Date: Tue, 25 Aug 2026 15:32:37 +0200 Subject: [PATCH] Batch 3: SearchByTreeWalkingMixin sizeLimit and ModifyOp round-trip - SearchByTreeWalkingMixin.search now honors sizeLimit and stops appending matches once the requested count is reached (fixes #234). Truncation only -- the tree walk itself still runs to completion, because early cancellation would require an interruptible iterator the mixin does not expose. - 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 the ValueError: too many values to unpack round-trip failure (fixes #223). Regression tests for both. Full suite green on py3.11, 3.13, 3.14. --- docs/source/NEWS.rst | 12 ++++++++++++ ldaptor/delta.py | 2 +- ldaptor/entryhelpers.py | 9 ++++++++- ldaptor/test/test_delta.py | 28 ++++++++++++++++++++++++++++ ldaptor/test/test_inmemory.py | 15 +++++++++++++++ 5 files changed, 64 insertions(+), 2 deletions(-) diff --git a/docs/source/NEWS.rst b/docs/source/NEWS.rst index 60cf2a35..2f621ca6 100644 --- a/docs/source/NEWS.rst +++ b/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) diff --git a/ldaptor/delta.py b/ldaptor/delta.py index c381e3e0..0521da1b 100644 --- a/ldaptor/delta.py +++ b/ldaptor/delta.py @@ -41,7 +41,7 @@ def asLDAP(self): ] ), ] - ).toWire() + ) def __eq__(self, other): if not isinstance(other, self.__class__): diff --git a/ldaptor/entryhelpers.py b/ldaptor/entryhelpers.py index 4cad5ee8..80611ad9 100644 --- a/ldaptor/entryhelpers.py +++ b/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: diff --git a/ldaptor/test/test_delta.py b/ldaptor/test/test_delta.py index a9b70d45..5868ed30 100644 --- a/ldaptor/test/test_delta.py +++ b/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) diff --git a/ldaptor/test/test_inmemory.py b/ldaptor/test/test_inmemory.py index 4c23bdb6..4efa09d3 100644 --- a/ldaptor/test/test_inmemory.py +++ b/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")