Warn instead of error when reading values invalid per the NWB schema

+io/createParsedType.m

@@ -24,9 +24,9 @@
 
     [lastWarningMessage, lastWarningID] = lastwarn('', ''); % Clear last warning
 
-    previousValidationContext = types.util.validationContext('read');
+    previousValidationContext = matnwb.common.validation.internal.context("read");
     validationContextCleanupObj = onCleanup( ...
-        @() types.util.validationContext(previousValidationContext));
+        @() matnwb.common.validation.internal.context(previousValidationContext));
 
     try
         typeInstance = feval(typeName, varargin{:}); % Create the type.

+matnwb/+common/+validation/+internal/ValidationContext.m

@@ -0,0 +1,23 @@
+classdef ValidationContext
+% ValidationContext - Runtime context for schema validation reporting.
+%   ValidationContext identifies when schema validation is running so
+%   matnwb.common.validation.reportSchemaViolation can choose whether a
+%   violation should warn or error.
+%
+%   READ is used while constructing MatNWB objects from existing files.
+%   Schema violations warn in this context so files with non-conforming
+%   values remain readable.
+%
+%   EDIT is the default context for user-created or modified in-memory
+%   objects. Schema violations error unless a validator explicitly requests
+%   warning behavior.
+%
+%   WRITE is used while exporting NWB files. Schema violations always error
+%   in this context so MatNWB does not write invalid files.
+
+    enumeration
+        READ
+        EDIT
+        WRITE
+    end
+end

+matnwb/+common/+validation/+internal/context.m

@@ -0,0 +1,23 @@
+function previousContext = context(newContext)
+% context - Get or set the process-local schema validation context.
+
+    arguments
+        newContext matnwb.common.validation.internal.ValidationContext = ...
+            matnwb.common.validation.internal.ValidationContext.empty
+    end
+
+    persistent activeContext
+
+    if isempty(activeContext)
+        activeContext = matnwb.common.validation.internal.ValidationContext.EDIT;
+    end
+
+    previousContext = activeContext;
+
+    if ~isempty(newContext)
+        assert(isscalar(newContext), ...
+            'NWB:Validation:InvalidContext', ...
+            'Validation context must be scalar.')
+        activeContext = newContext;
+    end
+end

+matnwb/+common/+validation/isReadContext.m

@@ -0,0 +1,4 @@
+function tf = isReadContext()
+    import matnwb.common.validation.internal.ValidationContext
+    tf = matnwb.common.validation.internal.context() == ValidationContext.READ;
+end

+matnwb/+common/+validation/reportSchemaViolation.m

@@ -0,0 +1,44 @@
+function reportSchemaViolation(errorId, message, causes, options)
+% reportSchemaViolation - Raise a schema-validation issue as an error or warning.
+%   By default, schema violations are errors. During read/deserialization,
+%   they are warnings so existing files remain readable. Callers can request
+%   warning behavior for specific backwards-compatible validators, but write
+%   validation always errors.
+%
+%   Callers pass a fully formed, context-neutral message describing the
+%   violation. Optional CAUSES (an array of MException objects) are attached
+%   as causes when erroring, and their messages are appended to the text when
+%   warning, because warnings cannot carry structured causes.
+
+    arguments
+        errorId (1,1) string
+        message (1,1) string
+        causes (1,:) MException = MException.empty(1, 0)
+        options.WarnInsteadOfError (1,1) logical = false
+    end
+
+    import matnwb.common.validation.internal.ValidationContext
+
+    validationContext = matnwb.common.validation.internal.context();
+    isReadContext = validationContext == ValidationContext.READ;
+    isWriteContext = validationContext == ValidationContext.WRITE;
+
+    if ~isWriteContext && (isReadContext || options.WarnInsteadOfError)
+        lenientGuidance = ['The non-conforming value is kept. If you maintain ' ...
+            'this data, consider correcting it before export.'];
+
+        fullMessage = message;
+        for iCause = 1:numel(causes)
+            fullMessage = fullMessage + " " + string(causes(iCause).message);
+        end
+        fullMessage = fullMessage + " " + lenientGuidance;
+
+        warning(errorId, '%s', fullMessage)
+    else
+        exception = MException(errorId, '%s', message);
+        for iCause = 1:numel(causes)
+            exception = exception.addCause(causes(iCause));
+        end
+        throw(exception)
+    end
+end

+tests/+unit/+common/+validation/ReportSchemaViolationTest.m

@@ -0,0 +1,77 @@
+classdef ReportSchemaViolationTest < matlab.unittest.TestCase
+% ReportSchemaViolationTest - Unit tests for schema violation reporting.
+
+    methods (TestMethodTeardown)
+        function resetValidationContext(~)
+            matnwb.common.validation.internal.context("edit");
+        end
+    end
+
+    methods (Test)
+        function testEditContextRaisesErrorByDefault(testCase)
+            matnwb.common.validation.internal.context("edit");
+            cause = MException( ...
+                'NWB:Test:Cause', 'The nested validation failed.');
+
+            try
+                matnwb.common.validation.reportSchemaViolation( ...
+                    'NWB:Test:SchemaViolation', ...
+                    "The value does not match the schema.", cause)
+                testCase.verifyFail( ...
+                    'Expected reportSchemaViolation to throw an error.')
+            catch exception
+                testCase.verifyEqual( ...
+                    exception.identifier, 'NWB:Test:SchemaViolation')
+                testCase.verifyEqual( ...
+                    exception.message, 'The value does not match the schema.')
+                testCase.verifyNumElements(exception.cause, 1)
+                testCase.verifyEqual( ...
+                    exception.cause{1}.identifier, 'NWB:Test:Cause')
+            end
+        end
+
+        function testReadContextWarnsWithGuidanceAndCauseMessages(testCase)
+            matnwb.common.validation.internal.context("read");
+            cause = MException( ...
+                'NWB:Test:Cause', 'The nested validation failed.');
+
+            lastwarn('')
+            testCase.verifyWarning( ...
+                @() matnwb.common.validation.reportSchemaViolation( ...
+                    'NWB:Test:SchemaViolation', ...
+                    "The value does not match the schema.", cause), ...
+                'NWB:Test:SchemaViolation')
+
+            [warningMessage, warningId] = lastwarn;
+            testCase.verifyEqual(warningId, 'NWB:Test:SchemaViolation')
+            testCase.verifySubstring( ...
+                warningMessage, 'The value does not match the schema.')
+            testCase.verifySubstring( ...
+                warningMessage, 'The nested validation failed.')
+            testCase.verifySubstring( ...
+                warningMessage, 'The non-conforming value is kept.')
+        end
+
+        function testWarnInsteadOfErrorWarnsInEditContext(testCase)
+            matnwb.common.validation.internal.context("edit");
+
+            testCase.verifyWarning( ...
+                @() matnwb.common.validation.reportSchemaViolation( ...
+                    'NWB:Test:SchemaViolation', ...
+                    "The value does not match the schema.", ...
+                    WarnInsteadOfError=true), ...
+                'NWB:Test:SchemaViolation')
+        end
+
+        function testWriteContextRaisesErrorWithWarnInsteadOfError(testCase)
+            matnwb.common.validation.internal.context("write");
+
+            testCase.verifyError( ...
+                @() matnwb.common.validation.reportSchemaViolation( ...
+                    'NWB:Test:SchemaViolation', ...
+                    "The value does not match the schema.", ...
+                    WarnInsteadOfError=true), ...
+                'NWB:Test:SchemaViolation')
+        end
+    end
+end

+tests/+unit/+io/testCreateParsedType.m

@@ -34,20 +34,17 @@ function testCreateTypeWithInvalidInputs(testCase)
         end
 
         function testCreateTypeWithInvalidPropertyValue(testCase)
+            % An invalid property value is read permissively: createParsedType
+            % warns and keeps the value instead of failing the read (#776).
             testPath = 'some/dataset/path';
             testType = 'types.hdmf_common.VectorIndex';
             kwargs = {'data', 'text is not numeric indices'};
 
-            try
-                io.createParsedType(testPath, testType, kwargs{:});
-                testCase.verifyFail('Expected io.createParsedType to throw an error.');
-            catch exception
-                testCase.verifyEqual(...
-                    exception.identifier, ...
-                    'NWB:createParsedType:TypeCreationFailed');
-                testCase.verifyNotEmpty(strfind(exception.message, testType));
-                testCase.verifyNotEmpty(strfind(exception.message, testPath));
-            end
+            type = testCase.verifyWarning(...
+                @() io.createParsedType(testPath, testType, kwargs{:}), ...
+                'NWB:CheckDataType:InvalidConversion');
+
+            testCase.verifyClass(type, testType)
         end
 
         function testCreateDynamicTableWithDuplicateColnamesWarns(testCase)
@@ -93,9 +90,9 @@ function testCreateDynamicTableWithColumnNamesMismatchWarns(testCase)
         end
 
         function testCheckConfigDoesNotRevalidateDuplicateColnamesOnRead(testCase)
-            previousValidationContext = types.util.validationContext('read');
+            previousValidationContext = matnwb.common.validation.internal.context("read");
             cleanupContext = onCleanup( ...
-                @() types.util.validationContext(previousValidationContext));
+                @() matnwb.common.validation.internal.context(previousValidationContext));
 
             warningState = warning('off', 'NWB:DynamicTable:DuplicateColumnNames');
             cleanupWarning = onCleanup(@() warning(warningState));

+tests/+unit/+types/+validators/CheckConstraintTest.m

@@ -0,0 +1,39 @@
+classdef CheckConstraintTest < matlab.unittest.TestCase
+% CheckConstraintTest - Unit tests for types.util.checkConstraint.
+%   A value whose type is not among the allowed constrained types is an
+%   error on construction and a warning (the value is kept) when reading a
+%   file.
+
+    methods (Test)
+        function testValueMatchingNoConstrainedTypeErrorsInStrictContext(testCase)
+            testCase.verifyError( ...
+                @() types.util.checkConstraint('group', 'item', struct(), ...
+                    {'types.hdmf_common.VectorData'}, 5), ...
+                'NWB:CheckConstraint:InvalidType')
+        end
+
+        function testValueMatchingNoConstrainedTypeWarnsInReadContext(testCase)
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            value = testCase.verifyWarning( ...
+                @() types.util.checkConstraint('group', 'item', struct(), ...
+                    {'types.hdmf_common.VectorData'}, 5), ...
+                'NWB:CheckConstraint:InvalidType');
+            testCase.verifyEqual(value, 5)
+        end
+
+        function testReadContextProbesConstrainedTypesStrictly(testCase)
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            value = testCase.verifyWarningFree( ...
+                @() types.util.checkConstraint('group', 'item', struct(), ...
+                    {'types.hdmf_common.VectorData', 'double'}, 5));
+
+            testCase.verifyEqual(value, 5)
+        end
+    end
+end

+tests/+unit/+types/+validators/CheckDtypeTest.m

@@ -202,6 +202,45 @@ function testAnyDtypeRejectsUnsupportedType(testCase)
                 @() types.util.checkDtype('invalidValue', 'any', {struct()}), ...
                 'NWB:CheckDType:InvalidType')
         end
+
+        function testAnyDtypeWarnsForUnsupportedTypeInReadContext(testCase)
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            invalidValue = {struct()};
+
+            value = testCase.verifyWarning( ...
+                @() types.util.checkDtype('invalidValue', 'any', invalidValue), ...
+                'NWB:CheckDType:InvalidType');
+
+            testCase.verifyEqual(value, invalidValue)
+        end
+
+        function testStrictModeErrorsInReadContext(testCase)
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            testCase.verifyError( ...
+                @() types.util.checkDtype( ...
+                    'invalidValue', 'any', {struct()}, Mode="strict"), ...
+                'NWB:CheckDType:InvalidType')
+        end
+
+        function testInvalidConversionWarnsInReadContext(testCase)
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            invalidValue = single(realmax('single'));
+
+            value = testCase.verifyWarning( ...
+                @() types.util.checkDtype('precisionLossError', 'uint64', invalidValue), ...
+                'NWB:CheckDataType:InvalidConversion');
+
+            testCase.verifyEqual(value, invalidValue)
+        end
     end
 
     methods (Static)

+tests/+unit/+types/+validators/CheckTypeTest.m

@@ -0,0 +1,51 @@
+classdef CheckTypeTest < matlab.unittest.TestCase
+% CheckTypeTest - Unit tests for types.util.checkType.
+%   A value of the wrong neurodata type is an error on construction and a
+%   warning (the value is kept) when reading a file. An unrecognized
+%   expected type is an internal error and stays an error in both contexts.
+
+    methods (Test)
+        function testMatchingTypePasses(testCase)
+            value = types.hdmf_common.VectorData( ...
+                'description', 'a column', 'data', (1:3)');
+            testCase.verifyWarningFree( ...
+                @() types.util.checkType( ...
+                    'col', 'types.hdmf_common.VectorData', value))
+        end
+
+        function testWrongTypeErrorsInStrictContext(testCase)
+            value = types.hdmf_common.VectorData( ...
+                'description', 'a column', 'data', (1:3)');
+            testCase.verifyError( ...
+                @() types.util.checkType( ...
+                    'region', 'types.hdmf_common.DynamicTableRegion', value), ...
+                'NWB:CheckType:InvalidNeurodataType')
+        end
+
+        function testWrongTypeWarnsInReadContext(testCase)
+            value = types.hdmf_common.VectorData( ...
+                'description', 'a column', 'data', (1:3)');
+
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            testCase.verifyWarning( ...
+                @() types.util.checkType( ...
+                    'region', 'types.hdmf_common.DynamicTableRegion', value), ...
+                'NWB:CheckType:InvalidNeurodataType')
+        end
+
+        function testUnknownExpectedTypeAlwaysErrors(testCase)
+            % An unrecognized expected type indicates an internal problem,
+            % not a non-conforming file, so it stays an error on read.
+            previousContext = matnwb.common.validation.internal.context("read");
+            cleanup = onCleanup( ...
+                @() matnwb.common.validation.internal.context(previousContext));
+
+            testCase.verifyError( ...
+                @() types.util.checkType('x', 'types.core.NotARealType', 5), ...
+                'NWB:CheckType:UnknownNeurodataType')
+        end
+    end
+end