From 1ae64b58585daaea1c509f60e85acbb8a30a101c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 22:45:47 +0000 Subject: [PATCH] open_doc: do not ask a two-argument handler for a series member Port of DID-matlab's handlerTakesContext gate (3299ab8, #188). A series member has no location of its own, so the handler is given the series manifest's location and the member's uid in the context. A handler that takes only (dest_path, source_path) never sees that uid and cannot know which member is wanted. Whatever it writes would be cached under the member's uid as the wrong bytes. MATLAB's fetchSeriesMemberBytes does not call such a handler. Python did, and only caught the case where the bytes happened to equal the manifest's. _handler_takes_context is factored out of _dispatch_custom_file_handler, mirroring MATLAB's handlerTakesContext. _open_series_member now raises DID:SQLITEDB:FileSeries:MemberNotHere for a two-argument handler, as it does with no handler, without calling it. This changes behaviour for callers: a two-argument handler that used to serve series members now gets MemberNotHere. The test that asserted the old behaviour is flipped. The add path is unchanged: two-argument handlers still ingest, as in MATLAB. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Dn4gCNCU2jUGdmfPVCAGK --- ..._matlab_python_bridge_implementations.yaml | 33 +++++---- src/did/implementations/sqlitedb.py | 72 +++++++++++++----- tests/test_open_doc_file_series.py | 73 ++++++++++++++++++- 3 files changed, 145 insertions(+), 33 deletions(-) diff --git a/src/did/did_matlab_python_bridge_implementations.yaml b/src/did/did_matlab_python_bridge_implementations.yaml index 74171ae..ef106ae 100644 --- a/src/did/did_matlab_python_bridge_implementations.yaml +++ b/src/did/did_matlab_python_bridge_implementations.yaml @@ -658,21 +658,28 @@ classes: TypeError (MATLAB's fallback is the two-argument contract). - name: handlerTakesContext - python_name: ~ + python_name: _handler_takes_context kind: static decision_log: > - Not separated out. Its rule lives inline in - _dispatch_custom_file_handler. DIVERGENCE, recorded 2026-10-23: - MATLAB's fetchSeriesMemberBytes calls handlerTakesContext and - refuses to ask a two-argument handler for a series member at all, - since it would only see the manifest's location. Python's - _open_series_member has no such gate: it offers the member to - any handler, and a two-argument handler that fetches the manifest - again is caught afterwards by the same-bytes check (see - filesAreIdentical). The outcome is a refusal either way, but - Python spends one fetch to reach it, and a two-argument handler - that returns something else is accepted where MATLAB never asks. - Not changed here; that is a behaviour port, not a bridge fix. + Same rule as dispatch: three or more positional parameters, or + *args. A callable whose signature cannot be inspected answers False, + MATLAB's safe default when nargin fails. Factored out of + _dispatch_custom_file_handler 2026-10-23, as MATLAB names it once, + and used for the gate it exists for: _open_series_member, like + MATLAB's fetchSeriesMemberBytes (3299ab8, #188), does not ask a + two-argument handler for a series member at all, because such a + handler sees only the manifest's location and cannot know which + member is wanted. The open raises + DID:SQLITEDB:FileSeries:MemberNotHere, as with no handler. + This closes the divergence recorded here earlier the same day: + Python offered the member to any handler and caught a re-fetched + manifest only afterwards, and a two-argument handler that returned + other bytes had them cached under the member's uid. The behaviour + change is visible to callers: a two-argument handler that used to + serve members now gets MemberNotHere and must take a context. + One difference remains in dispatch, not in the gate: there an + uninspectable callable is tried with three arguments, then two on + TypeError, rather than given the two-argument contract outright. - name: filesAreIdentical python_name: ~ diff --git a/src/did/implementations/sqlitedb.py b/src/did/implementations/sqlitedb.py index 97efe69..7122238 100644 --- a/src/did/implementations/sqlitedb.py +++ b/src/did/implementations/sqlitedb.py @@ -1992,6 +1992,21 @@ def _open_series_member(self, series_locations, custom_file_handler): f"custom_file_handler was supplied.", ) + # A two-argument handler is told nothing but the manifest's location, + # so it has no way to learn which member it is being asked for: + # anything it produced would be the wrong bytes under this uid. It is + # not asked at all -- the member is simply not here, as when no + # handler was supplied. Mirrors MATLAB's fetchSeriesMemberBytes gate. + if not self._handler_takes_context(custom_file_handler): + raise FileAccessError( + "DID:SQLITEDB:FileSeries:MemberNotHere", + f'Series member "{filename}" of series "{stem}" (uid ' + f'"{member_uid}") is not on this machine, and the ' + f"custom_file_handler takes only (dest_path, source_path), so " + f"it cannot be told which member to fetch. A handler that " + f"accepts a third, context argument is given the member's uid.", + ) + # Offer every location of the manifest to the handler, remote first # so a handler that can answer from the manifest's store is not # given a local copy it can only re-download from itself. @@ -2343,6 +2358,42 @@ def _handler_context(context): return None return dict(context) + @staticmethod + def _handler_takes_context(handler): + """Will ``handler`` be given the context? + + True when it declares three or more positional parameters, or takes + ``*args``. A handler declared with two gets the two-argument call and + never sees a context. Mirrors MATLAB's + ``did.implementations.sqlitedb.handlerTakesContext``: the rule + _dispatch_custom_file_handler dispatches by, named once so a caller + with nothing useful to say to a two-argument handler -- the series + member fetch in _open_series_member, whose whole message is in the + context -- can ask rather than re-derive it. + + A callable whose signature cannot be inspected answers False, as + MATLAB's does when ``nargin`` fails: the safe default is the old + contract. + """ + import inspect + + try: + signature = inspect.signature(handler) + except (TypeError, ValueError): + return False + # Count positional-capable parameters (POSITIONAL_ONLY / + # POSITIONAL_OR_KEYWORD / VAR_POSITIONAL). + positional = 0 + for p in signature.parameters.values(): + if p.kind == inspect.Parameter.VAR_POSITIONAL: + return True + if p.kind in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + ): + positional += 1 + return positional >= 3 + @staticmethod def _dispatch_custom_file_handler(handler, dest_path, source_path, context): """Call ``handler`` with two or three arguments per its signature. @@ -2351,7 +2402,8 @@ def _dispatch_custom_file_handler(handler, dest_path, source_path, context): ``*args``) is called with ``(dest_path, source_path, context)``. A handler declared with two inputs is called without context, so the older two-argument signature still works. Mirrors MATLAB's - ``did.implementations.sqlitedb.dispatchCustomFileHandler``. + ``did.implementations.sqlitedb.dispatchCustomFileHandler``; the rule + itself is :meth:`_handler_takes_context`. """ import inspect @@ -2359,7 +2411,7 @@ def _dispatch_custom_file_handler(handler, dest_path, source_path, context): handler(dest_path, source_path) return try: - signature = inspect.signature(handler) + inspect.signature(handler) except (TypeError, ValueError): # Builtin or C-implemented callable: fall back to positional try. try: @@ -2369,21 +2421,7 @@ def _dispatch_custom_file_handler(handler, dest_path, source_path, context): handler(dest_path, source_path) return - params = list(signature.parameters.values()) - # Count positional-capable parameters (POSITIONAL_ONLY / - # POSITIONAL_OR_KEYWORD / VAR_POSITIONAL). - positional = 0 - has_var_positional = False - for p in params: - if p.kind in ( - inspect.Parameter.POSITIONAL_ONLY, - inspect.Parameter.POSITIONAL_OR_KEYWORD, - ): - positional += 1 - elif p.kind == inspect.Parameter.VAR_POSITIONAL: - has_var_positional = True - - if has_var_positional or positional >= 3: + if SQLiteDB._handler_takes_context(handler): handler(dest_path, source_path, context) else: handler(dest_path, source_path) diff --git a/tests/test_open_doc_file_series.py b/tests/test_open_doc_file_series.py index e5c1b18..e4642d2 100644 --- a/tests/test_open_doc_file_series.py +++ b/tests/test_open_doc_file_series.py @@ -399,7 +399,14 @@ def test_each_member_is_fetched_under_its_own_uid(self): fetched = [context["uid"] for _dest, _src, context in calls] self.assertEqual(fetched, self.member_uids) - def test_a_two_argument_handler_is_still_called(self): + def test_a_two_argument_handler_is_not_asked_for_a_member(self): + """A two-argument handler is given only the manifest's location, so + it cannot know which member is wanted; whatever it wrote would be + cached under the member's uid as the wrong bytes. MATLAB's + fetchSeriesMemberBytes does not call it (handlerTakesContext), and + neither does Python: the member is not here, as with no handler. + This test used to assert the opposite -- that the handler was called + and its bytes returned -- which was the divergence.""" doc = self.stored_series(count=2, remote_manifest=True) self.evict_members() calls = [] @@ -409,12 +416,33 @@ def handler(dest_path, source_path): with open(dest_path, "wb") as handle: handle.write(b"two-arg") + with self.assertRaises(FileAccessError) as caught: + self.db.open_doc(doc.id(), "chunkdata.bin_1", custom_file_handler=handler) + + self.assertEqual( + caught.exception.identifier, "DID:SQLITEDB:FileSeries:MemberNotHere" + ) + self.assertIn("cannot be told which member", str(caught.exception)) + self.assertEqual(calls, [], "a two-argument handler is never called") + + def test_a_varargs_handler_is_asked_with_the_context(self): + doc = self.stored_series(count=2, remote_manifest=True) + self.evict_members() + calls = [] + + def handler(*args): + calls.append(args) + with open(args[0], "wb") as handle: + handle.write(b"varargs") + file_obj = self.db.open_doc( doc.id(), "chunkdata.bin_1", custom_file_handler=handler ) - self.assertEqual(self.read(file_obj), b"two-arg") - self.assertEqual(calls, [REMOTE_MANIFEST]) + self.assertEqual(self.read(file_obj), b"varargs") + self.assertEqual(len(calls), 1) + self.assertEqual(len(calls[0]), 3) + self.assertEqual(calls[0][2]["uid"], self.member_uids[0]) def test_a_fetched_member_is_kept_and_not_fetched_again(self): doc = self.stored_series(count=2, remote_manifest=True) @@ -882,3 +910,42 @@ def test_no_handler_still_raises_manifest_not_local(self): if __name__ == "__main__": unittest.main() + + +class TestHandlerTakesContext(unittest.TestCase): + """The one rule both the dispatch and the series-member gate use.""" + + def test_two_positional_parameters_do_not_take_context(self): + self.assertFalse(SQLiteDB._handler_takes_context(lambda dest, src: None)) + + def test_three_positional_parameters_take_context(self): + self.assertTrue( + SQLiteDB._handler_takes_context(lambda dest, src, context: None) + ) + + def test_varargs_takes_context(self): + self.assertTrue(SQLiteDB._handler_takes_context(lambda *args: None)) + self.assertTrue(SQLiteDB._handler_takes_context(lambda dest, *rest: None)) + + def test_keyword_only_parameters_do_not_count(self): + self.assertFalse( + SQLiteDB._handler_takes_context(lambda dest, src, *, context=None: None) + ) + + def test_a_bound_method_counts_without_self(self): + class Handler: + def two(self, dest, src): + pass + + def three(self, dest, src, context): + pass + + self.assertFalse(SQLiteDB._handler_takes_context(Handler().two)) + self.assertTrue(SQLiteDB._handler_takes_context(Handler().three)) + + def test_an_uninspectable_callable_gets_the_old_contract(self): + # MATLAB: when nargin(handler) fails, the safe default is n = 2. + with mock.patch("inspect.signature", side_effect=ValueError("no sig")): + self.assertFalse( + SQLiteDB._handler_takes_context(lambda dest, src, context: None) + )