Skip to content

Commit

Permalink
Fix validator not validating on spec of builder data type (#1050)
Browse files Browse the repository at this point in the history
  • Loading branch information
rly committed Feb 5, 2024
1 parent 3fbe27d commit 46d81cb
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 4 deletions.
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')

0 comments on commit 46d81cb

Please sign in to comment.