Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/did/did_matlab_python_bridge.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ classes:
- name: database
type: class
matlab_path: "+did/database.m"
matlab_last_sync_hash: "ec1e733"
matlab_last_sync_hash: "9b36124"
status: ported
python_path: "did/database.py"
python_class: "Database"
Expand All @@ -59,6 +59,14 @@ classes:
MissingOptionalDependencyWarning gated on the
DID_FORCE_VALIDATION_WARNINGS environment variable.

2026-09-09: bumped through DID-matlab 9b36124 (PR #200). Only
check_files needed a change -- the fail-open on a required file
declared in file_list with nothing bound to it in file_info, same
shape as PR #182's step-1 fail-open, one step down. See the checkfiles
method entry for the port. The rest of PR #200 was MATLAB-only: a
try/catch that read file_info before file_list under one arm, which
Python does not share.

Reviewed 2026-08-29 through MATLAB c0a9507, nothing to port. Two
MATLAB fixes landed in this window and Python has neither defect:

Expand Down Expand Up @@ -768,6 +776,20 @@ classes:
meant a coverage check could not tell that isfilenamematch and
canfindonefile were bridged at all. Split 2026-08-28.

2026-09-09 (DID-matlab#200 / #199): step 3 used to loop over the
actual file_info entries and fall through to is_valid = True when a
required name had none -- the same fail-open PR #182 closed for
step 1, one step further down. Ported here: check_files now
separates the two absences, keeping the file_list message for a
name missing from the list and adding a new one for a name that IS
in the file_list with nothing bound for it (naming the file and
file_info). The other half of DID-matlab#200, an independent read
of file_list before file_info, is not a bug on the Python side --
_validate_files already reads the two fields independently and did
not share MATLAB's try/catch shape -- so no code change beyond
check_files was needed. Coverage:
tests/test_validation.py:TestFileValidationDiagnosis.

- name: isfilenamematch
kind: static
python_name: is_filename_match
Expand Down
29 changes: 26 additions & 3 deletions src/did/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,15 @@ def check_files(

Returns ``(is_valid, error_message)``. Mirrors MATLAB
``database.checkfiles``.

Steps 1 and 3 report DIFFERENT ABSENCES. A schema-required name can be
missing from the document's file_list (step 1), or it can be there in the
file_list -- declared, correctly -- with nothing bound to it in file_info
(step 3). The second is the common one, and step 3 used to fall out of its
empty match loop without raising: a required file was absent and the
document was accepted anyway. That is the same fail-open MATLAB PR #182
closed for step 1, one step further down, and DID-matlab#199 / #200 closed
for step 3.
"""
expected_unique = sorted(set(expected_names))
actual_unique = sorted(set(actual_file_names))
Expand All @@ -665,9 +674,23 @@ def check_files(
if not _is_truthy(expected_value):
continue
item_name = expected_names[index]
for file_index, actual in enumerate(actual_file_names):
if not is_filename_match(item_name, actual):
continue
matches = [
file_index
for file_index, actual in enumerate(actual_file_names)
if is_filename_match(item_name, actual)
]
if not matches:
# The name IS in the document's file_list -- step 1 above
# established that -- but nothing is bound to it in file_info, so
# there is no file to look for. Say that, and say it about
# file_info: the file_list is correct here, and blaming it is what
# cost the debugging time in DID-matlab#199.
return False, (
f'Required file "{item_name}" is declared in the file_list of '
f"{doc_name} but no file is bound to that name "
f"(files.file_info has no entry for it)"
)
for file_index in matches:
entry = files[file_index] if file_index < len(files) else {}
locations = entry.get("locations") if isinstance(entry, dict) else None
if not can_find_one_file(locations):
Expand Down
137 changes: 137 additions & 0 deletions tests/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,3 +404,140 @@ def test_the_url_survives_the_round_trip(self):
entries = entries if isinstance(entries, list) else [entries]
locations.extend(item.get("location") for item in entries)
self.assertIn(self.URL, locations)


class TestFileValidationDiagnosis(_DatabaseTestCase):
"""WHICH FIELD a file-validation failure blames.

check_files' step 3 used to loop over the actual file_info entries: a
required name declared in the file_list with nothing bound to it in
file_info matched nothing and fell through to is_valid = True. That is
the same fail-open MATLAB PR #182 closed for step 1, one step further
down; DID-matlab#199 closed it for step 3. Python's _validate_files
already read file_info and file_list independently -- MATLAB's own bug
from that PR was a try/catch shape Python does not share -- but the
fail-open in check_files itself was shared, so the port is that fix.
"""

def _doc_with_file_info(self, file_info, file_list=None):
"""A demoFile document whose files section is set by the caller.

demoFile declares filename1.ext and filename2.ext, both
mustbenotempty.
"""
doc = Document("demoFile", **{"demoFile.value": 1})
doc.document_properties["files"]["file_info"] = file_info
if file_list is not None:
doc.document_properties["files"]["file_list"] = file_list
return doc

def _assert_refused(self, doc):
with self.assertRaises(ValidationError) as caught:
self.db.add_docs([doc])
self.assertEqual(
caught.exception.identifier,
"DID:Database:ValidationFiles",
f"refused for the wrong reason: {caught.exception}",
)
return str(caught.exception)

def test_empty_file_info_blames_the_unbound_file_not_the_file_list(self):
# The reported case. file_list declares both names and is correct;
# nothing is bound to them. file_info = [] used to fall out of the
# empty match loop and be accepted (Python) or throw and get the
# file_list blamed (MATLAB).
doc = self._doc_with_file_info([])

msg = self._assert_refused(doc)

self.assertIn(
"filename1.ext",
msg,
"the message must name the file that is not bound",
)
self.assertIn(
"file_info",
msg,
"the message must name the field that is actually empty",
)
self.assertNotIn(
"from the file_list",
msg,
(
"the file_list declares the file and is correct, so the "
f"message must not send the reader there: {msg}"
),
)

def test_name_missing_from_file_list_names_the_absent_name(self):
# A document whose file_list is genuinely incomplete: it holds
# filename1.ext and not filename2.ext. The absent name is
# filename2.ext, and that is what the message must say.
doc = self._doc_with_file_info([], file_list=["filename1.ext"])

msg = self._assert_refused(doc)

self.assertIn(
"file_list",
msg,
"a name absent from the file_list IS a file_list problem",
)
self.assertIn(
"filename2.ext",
msg,
"the message must name the file that is absent from the list",
)
self.assertNotIn(
"filename1.ext",
msg,
(
"filename1.ext is in the file_list; only filename2.ext is "
f"absent from it: {msg}"
),
)

def test_document_with_no_files_field_still_validates(self):
# Most classes declare no files at all, and their documents have no
# files section. That must keep working.
doc = Document("demoA", **{"demoA.value": 1})
self.assertNotIn(
"files",
doc.document_properties,
"precondition: a demoA document has no files section",
)
self.db.add_docs([doc])
self.assertIsNotNone(self.db.get_docs(doc.id()))

def test_declared_optional_files_need_not_be_bound(self):
# demoSeries declares both of its files mustbenotempty = 0, so a
# document that binds neither is valid: the file_list is correct
# and nothing it declares is required.
doc = Document("demoSeries", **{"demoSeries.value": 1})
doc.document_properties["files"]["file_info"] = []
self.db.add_docs([doc])
self.assertIsNotNone(self.db.get_docs(doc.id()))

def test_bound_required_files_validate(self):
# The other side of the tightening: a document that binds what its
# schema requires still validates. Validation does no file I/O, so
# the locations need not exist yet.
doc = Document("demoFile", **{"demoFile.value": 1})
doc.add_file("filename1.ext", "placeholder", ingest=0, delete_original=0)
doc.add_file("filename2.ext", "placeholder", ingest=0, delete_original=0)
self.db.add_docs([doc])
self.assertIsNotNone(self.db.get_docs(doc.id()))

def test_check_files_directly_reports_unbound_required_file(self):
# The unit-level counterpart, calling check_files without a database.
is_valid, message = check_files(
expected_names=["filename1.ext", "filename2.ext"],
must_have_value=[1, 1],
actual_file_names=[],
doc_name="demoFile doc X",
files=[],
actual_file_list=["filename1.ext", "filename2.ext"],
)
self.assertFalse(is_valid)
self.assertIn("filename1.ext", message)
self.assertIn("file_info", message)
self.assertNotIn("from the file_list", message)
Loading