Validate property values on export to prevent writing invalid files

+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

+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

+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