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) + )