diff --git a/+tests/+unit/+types/+doubles/TypeWithFailingValidator.m b/+tests/+unit/+types/+doubles/TypeWithFailingValidator.m new file mode 100644 index 000000000..e6f2c9796 --- /dev/null +++ b/+tests/+unit/+types/+doubles/TypeWithFailingValidator.m @@ -0,0 +1,47 @@ +classdef TypeWithFailingValidator < types.untyped.MetaClass +% TypeWithFailingValidator - Test double for exercising MetaClass.validateProperties. +% Provides a property whose validator always fails and one whose validator +% always passes, plus a thin wrapper that invokes the protected +% validateProperties method from outside the class hierarchy. + + properties + validProperty + invalidProperty + coercingProperty + datetimeProperty + end + + methods + function value = validate_validProperty(~, value) + % Always valid; returns the value unchanged. + end + + function validate_invalidProperty(~, ~) + error('NWB:Test:InvalidPropertyValue', ... + 'This property value is never valid.') + end + + function value = validate_coercingProperty(~, value) + % Simulates a validator that coerces the input (e.g., dtype + % conversion). Returns the value as double regardless of input type. + value = double(value); + end + + function value = validate_datetimeProperty(~, value) + value = types.util.checkDtype('datetimeProperty', 'datetime', value); + end + + function runValidateProperties(obj, fullpath) + obj.validateProperties(fullpath) + end + end + + methods (Access = protected) + function str = getFooter(~) + % Override the inherited footer, which inspects required + % properties assuming a `types.` namespace this test double does + % not have. + str = ''; + end + end +end diff --git a/+tests/+unit/+types/MetaClassValidatePropertiesTest.m b/+tests/+unit/+types/MetaClassValidatePropertiesTest.m new file mode 100644 index 000000000..74aa382ea --- /dev/null +++ b/+tests/+unit/+types/MetaClassValidatePropertiesTest.m @@ -0,0 +1,73 @@ +classdef MetaClassValidatePropertiesTest < matlab.unittest.TestCase +% MetaClassValidatePropertiesTest - Unit tests for MetaClass.validateProperties, +% the export-time guard that re-runs property validators so that values which +% bypassed strict validation cannot be written back out to a file. + + methods (Test) + function testInvalidPropertyValueRaisesError(testCase) + testType = tests.unit.types.doubles.TypeWithFailingValidator(); + testType.invalidProperty = 1; + + testCase.verifyError( ... + @() testType.runValidateProperties('/some/path'), ... + 'NWB:Export:InvalidPropertyValue') + end + + function testErrorIncludesPropertyLocationAndCause(testCase) + testType = tests.unit.types.doubles.TypeWithFailingValidator(); + testType.invalidProperty = 1; + + try + testType.runValidateProperties('/some/path') + testCase.verifyFail('Expected an error for the invalid property value.') + catch exception + testCase.verifyEqual( ... + exception.identifier, 'NWB:Export:InvalidPropertyValue') + testCase.verifyTrue(contains(exception.message, 'invalidProperty')) + testCase.verifyTrue(contains(exception.message, '/some/path')) + % The original validator error is preserved as a cause. + testCase.verifyNotEmpty(exception.cause) + testCase.verifyEqual( ... + exception.cause{1}.identifier, 'NWB:Test:InvalidPropertyValue') + end + end + + function testEmptyPropertyIsNotValidated(testCase) + % An unset (empty) property is skipped even though its validator + % would fail, because empty optional properties are not exported. + testType = tests.unit.types.doubles.TypeWithFailingValidator(); + + testCase.verifyWarningFree( ... + @() testType.runValidateProperties('/some/path')) + end + + function testValidPropertyValuePasses(testCase) + testType = tests.unit.types.doubles.TypeWithFailingValidator(); + testType.validProperty = 42; + + testCase.verifyWarningFree( ... + @() testType.runValidateProperties('/some/path')) + end + + function testCoercingValidatorRaisesError(testCase) + % A validator that changes the MATLAB class must produce an + % error, because the writer would receive a value whose type does + % not match what strict validation accepts. + testType = tests.unit.types.doubles.TypeWithFailingValidator(); + testType.coercingProperty = int32(5); % int32 -> double on validate + + testCase.verifyError( ... + @() testType.runValidateProperties('/some/path'), ... + 'NWB:Export:InvalidPropertyValue') + end + + function testDatetimeFormatterNormalizationPasses(testCase) + testType = tests.unit.types.doubles.TypeWithFailingValidator(); + testType.datetimeProperty = {datetime(2020, 1, 1, ... + 'Format', 'dd-MMM-uuuu HH:mm:ss')}; + + testCase.verifyWarningFree( ... + @() testType.runValidateProperties('/some/path')) + end + end +end diff --git a/+types/+untyped/MetaClass.m b/+types/+untyped/MetaClass.m index a8e8959e4..1cab590ea 100644 --- a/+types/+untyped/MetaClass.m +++ b/+types/+untyped/MetaClass.m @@ -57,6 +57,7 @@ writer = io.backend.base.Writer.ensure(writer); obj.throwErrorIfCustomConstraintUnfulfilled(fullpath) obj.throwErrorIfMissingRequiredProps(fullpath) + obj.validateProperties(fullpath) obj.metaClass_fullPath = fullpath; %find reference properties propnames = properties(obj); @@ -269,6 +270,65 @@ function throwErrorIfCustomConstraintUnfulfilled(obj, fullpath) class(obj), fullpath, ME.message) end end + + function validateProperties(obj, fullpath) + % validateProperties - Re-run property validators before writing to file. + % Ensures property values that bypassed strict validation (for + % example, values read permissively from a file that does not + % conform to the schema) are not written back out as a new, invalid + % file. Validators run in the default (strict) context here, so a + % schema violation raises an error rather than a warning. + previousValidationContext = matnwb.common.validation.internal.context('write'); + cleanupValidationContext = onCleanup(@() matnwb.common.validation.internal.context(previousValidationContext)); + + if isempty(fullpath) + fullpath = 'root'; + end + + propertyNames = properties(obj); + for iProperty = 1:numel(propertyNames) + propertyName = propertyNames{iProperty}; + propertyValue = obj.(propertyName); + validatorName = ['validate_' propertyName]; + + % Validate only set properties that have a generated + % validator. An empty value represents an unset optional + % property, which is not written on export. + if ~isempty(propertyValue) && ismethod(obj, validatorName) + warnState = warning('error', 'NWB:CheckDataType:NeedsManualConversion'); + warnCleanupObj = onCleanup(@() warning(warnState)); + try + try + validatedValue = feval(validatorName, obj, propertyValue); + if ~strcmp(class(validatedValue), class(propertyValue)) + error('NWB:Export:PropertyValueRequiresNormalization', ... + ['Property "%s" would be converted by its validator. ' ... + 'Assign it via its setter (strict validation) before export.'], ... + propertyName); + end + catch MEValidator + if any(strcmp(MEValidator.identifier, ... + {'MATLAB:maxlhs', 'MATLAB:TooManyOutputs'})) + % Validator does not provide an output. Call + % again without requesting a normalized value. + feval(validatorName, obj, propertyValue); + else + rethrow(MEValidator) + end + end + catch ME + newException = MException( ... + 'NWB:Export:InvalidPropertyValue', ... + ['The value of property "%s" for type "%s" at ', ... + 'file location "%s" is not valid according to ', ... + 'the schema and cannot be exported:\n%s'], ... + propertyName, class(obj), fullpath, ME.message); + newException = newException.addCause(ME); + throw(newException) + end + end + end + end end methods