Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validator not validating on spec of builder data type #1050

Merged
merged 3 commits into from
Feb 5, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Fixed retrieving the correct path for a `HERD` zip file on read. [#1046](https://github.com/hdmf-dev/hdmf/pull/1046)
- Fixed internal links in docstrings and tutorials. @stephprince [#1031](https://github.com/hdmf-dev/hdmf/pull/1031)
- Fixed issue with creating documentation links to classes in docval arguments. @rly [#1036](https://github.com/hdmf-dev/hdmf/pull/1036)
- Fixed issue with validator not validating against the spec that defines the data type of the builder. @rly [#1050](https://github.com/hdmf-dev/hdmf/pull/1050)

## HDMF 3.12.0 (January 16, 2024)

Expand Down
21 changes: 17 additions & 4 deletions src/hdmf/validate/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,8 @@ def __validate_child_builder(self, child_spec, child_builder, parent_builder):
yield self.__construct_illegal_link_error(child_spec, parent_builder)
return # do not validate illegally linked objects
child_builder = child_builder.builder
for child_validator in self.__get_child_validators(child_spec):
child_builder_data_type = child_builder.attributes.get(self.spec.type_key())
for child_validator in self.__get_child_validators(child_spec, child_builder_data_type):
yield from child_validator.validate(child_builder)

def __construct_illegal_link_error(self, child_spec, parent_builder):
Expand All @@ -557,7 +558,7 @@ def __construct_illegal_link_error(self, child_spec, parent_builder):
def __cannot_be_link(spec):
return not isinstance(spec, LinkSpec) and not spec.linkable

def __get_child_validators(self, spec):
def __get_child_validators(self, spec, builder_data_type):
"""Returns the appropriate list of validators for a child spec

Due to the fact that child specs can both inherit a data type via data_type_inc
Expand All @@ -572,9 +573,21 @@ def __get_child_validators(self, spec):
returned. If the spec is a LinkSpec, no additional Validator is returned
because the LinkSpec cannot add or modify fields and the target_type will be
validated by the Validator returned from the ValidatorMap.

For example, if the spec is:
{'doc': 'Acquired, raw data.', 'quantity': '*', 'data_type_inc': 'NWBDataInterface'}
then the returned validators will be:
- a validator for the spec for the builder data type
- a validator for the spec for data_type_def: NWBDataInterface
- a validator for the above spec which might have extended properties
on top of data_type_def: NWBDataInterface
"""
if _resolve_data_type(spec) is not None:
yield self.vmap.get_validator(_resolve_data_type(spec))
if builder_data_type is not None:
yield self.vmap.get_validator(builder_data_type)

spec_data_type = _resolve_data_type(spec)
if spec_data_type is not None:
yield self.vmap.get_validator(spec_data_type)

if isinstance(spec, GroupSpec):
yield GroupValidator(spec, self.vmap)
Expand Down
52 changes: 52 additions & 0 deletions tests/unit/validator_tests/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -1217,3 +1217,55 @@ def test_empty_int_dataset(self):
DatasetBuilder(name='dataInt', data=[], dtype='int') # <-- Empty int dataset
])
self.runBuilderRoundTrip(builder)


class TestValidateSubspec(ValidatorTestBase):
"""When a subtype satisfies a subspec, the validator should also validate
that the subtype satisfies its spec.
"""

def getSpecs(self):
dataset_spec = DatasetSpec('A dataset', data_type_def='Foo')
sub_dataset_spec = DatasetSpec(
doc='A subtype of Foo',
data_type_def='Bar',
data_type_inc='Foo',
attributes=[
AttributeSpec(name='attr1', doc='an example attribute', dtype='text')
],
)
spec = GroupSpec(
doc='A group that contains a Foo',
data_type_def='Baz',
datasets=[
DatasetSpec(doc='Child Dataset', data_type_inc='Foo'),
])
return (spec, dataset_spec, sub_dataset_spec)

def test_validate_subtype(self):
"""Test that when spec A contains dataset B, and C is a subtype of B, using a C builder is valid.
"""
builder = GroupBuilder(
name='my_baz',
attributes={'data_type': 'Baz'},
datasets=[
DatasetBuilder(name='bar', attributes={'data_type': 'Bar', 'attr1': 'value'})
],
)
result = self.vmap.validate(builder)
self.assertEqual(len(result), 0)

def test_validate_subtype_error(self):
"""Test that when spec A contains dataset B, and C is a subtype of B, using a C builder validates
against spec C.
"""
builder = GroupBuilder(
name='my_baz',
attributes={'data_type': 'Baz'},
datasets=[
DatasetBuilder(name='bar', attributes={'data_type': 'Bar'})
],
)
result = self.vmap.validate(builder)
self.assertEqual(len(result), 1)
self.assertValidationError(result[0], MissingError, name='Bar/attr1')
Loading