[codex] Improve dynamic table validation and colnames sync

+file/fillClass.m

@@ -164,7 +164,11 @@
         superClassProps, ...
         class, ...
         inherited);
-    setterFcns = file.fillSetters(setdiff(nonInherited, union(readonly, hiddenAndReadonly)), classprops);
+    setterFcns = file.fillSetters( ...
+        setdiff(nonInherited, union(readonly, hiddenAndReadonly)), ...
+        classprops, ...
+        name, ...
+        namespace);
     validatorFcns = file.fillValidators(allProperties, classprops, namespace, namespace.getFullClassName(name), inherited);
     exporterFcns = file.fillExport(nonInherited, class, superclassNames{1}, required, classprops);
     methodBody = strjoin({constructorBody...

+file/fillConstructor.m

@@ -28,7 +28,7 @@
     end
 
     % Add custom validation for DynamicTable and its descendant classes
-    if isDynamicTableDescendant(name, namespace)
+    if file.isDynamicTableDescendant(name, namespace)
         constructorElements{end+1} = '    types.util.dynamictable.checkConfig(obj);';
     end
 
@@ -270,26 +270,6 @@
     docString = char( strjoin(docString, newline) );
 end
 
-function tf = isDynamicTableDescendant(name, namespace)
-% Check if name is DynamicTable or if name is for a type that inherits from DynamicTable
-
-    tf = false;
-
-    if strcmp(name, 'DynamicTable')
-        tf = true;
-        return
-    end
-
-    ancestry = namespace.getRootBranch(name);
-    for iAncestor = 1:length(ancestry)
-        ParentRaw = ancestry{iAncestor};
-        % this is always true, we just use the proper index as typedefs may vary.
-        typeDefInd = isKey(ParentRaw, namespace.TYPEDEF_KEYS);
-        ancestorName = ParentRaw(namespace.TYPEDEF_KEYS{typeDefInd});
-        tf = tf || strcmp(ancestorName, 'DynamicTable');
-    end
-end
-
 % Todo: Mostly duplicate code from file.fillProps. Should consolidate
 function typeStr = getTypeStr(prop)
     if ischar(prop)
@@ -344,4 +324,4 @@ function assertValidRefType(referenceType)
         word (1,:) char
     end
     word(1) = upper(word(1));
-end
+end

+file/fillCustomConstraint.m

@@ -34,6 +34,13 @@
                 '    end\n', ...
                 'end'] );
 
+        case "DynamicTable"
+            customConstraintStr = sprintf( [...
+                'function checkCustomConstraint(obj)\n', ...
+                '    checkCustomConstraint@types.untyped.MetaClass(obj)\n', ...
+                '    types.util.dynamictable.checkConfig(obj)\n', ...
+                'end'] );
+
         otherwise
             customConstraintStr = '';
     end

+file/fillSetters.m

@@ -1,9 +1,9 @@
-function fsstr = fillSetters(propnames, classprops)
+function fsstr = fillSetters(propnames, classprops, typeName, namespace)
 fsstr = cell(size(propnames));
 for i=1:length(propnames)
     nm = propnames{i};
     prop = classprops(nm);
-    postsetFunctionStr = resolvePostsetFunction(nm, prop);
+    postsetFunctionStr = resolvePostsetFunction(nm, prop, typeName, namespace);
     if isempty(postsetFunctionStr)
         fsstr{i} = strjoin({...
             ['function set.' nm '(obj, val)']...
@@ -22,9 +22,10 @@
 fsstr = strjoin(fsstr, newline);
 end
 
-function postsetFunctionStr = resolvePostsetFunction(propname, prop)
+function postsetFunctionStr = resolvePostsetFunction(propName, prop, typeName, namespace)
 
-    postsetFunctionStr = '';
+    hookInfo = file.getPropertyHooks(propName, prop, typeName, namespace);
+    postsetStatements = hookInfo.PostsetStatements;
 
     if isa(prop, 'file.Attribute')
 
@@ -38,11 +39,11 @@
 
             conditionStr = sprintf(...
                 'if isempty(obj.%s) && ~isempty(obj.%s)', ...
-                parentname, propname);
+                parentname, propName);
 
             warnIfDependencyMissingString = sprintf(...
                 'obj.warnIfAttributeDependencyMissing(''%s'', ''%s'')', ...
-                propname, parentname);
+                propName, parentname);
 
             syncPromotedDatasetAttributeString = '';
             if prop.promoted_to_container
@@ -53,19 +54,27 @@
                     '    elseif ~isempty(obj.%1$s.%2$s)\n' ...
                     '        obj.%3$s = obj.%1$s.%2$s;\n' ...
                     '    end\n' ...
-                    'end'], parentname, prop.name, propname);
+                    'end'], parentname, prop.name, propName);
             end
-
-            postsetLines = {...
-                sprintf('function postset_%s(obj)', propname), ...
-                file.addSpaces(conditionStr, 4), ...
-                file.addSpaces(warnIfDependencyMissingString, 8), ...
-                file.addSpaces('end', 4), ...
-                'end'};
+
+            postsetStatements = [postsetStatements, ...
+                {conditionStr}, ...
+                {file.addSpaces(warnIfDependencyMissingString, 4)}, ...
+                {'end'}];
             if ~isempty(syncPromotedDatasetAttributeString)
-                postsetLines = [postsetLines(1:end-1), {file.addSpaces(syncPromotedDatasetAttributeString, 4)}, postsetLines(end)];
+                postsetStatements{end+1} = syncPromotedDatasetAttributeString;
             end
-            postsetFunctionStr = strjoin(postsetLines, newline);
         end
     end
+
+    if isempty(postsetStatements)
+        postsetFunctionStr = '';
+        return
+    end
+
+    postsetBody = strjoin(postsetStatements, newline);
+    postsetLines = {sprintf('function postset_%s(obj)', propName), ...
+        file.addSpaces(postsetBody, 4), ...
+        'end'};
+    postsetFunctionStr = strjoin(postsetLines, newline);
 end

+file/fillValidators.m

@@ -21,6 +21,9 @@
             validationBody = fillDtypeValidation(nm, prop, namespaceReg);
         end
 
+        hookInfo = file.getPropertyHooks(nm, prop, classNameParts(className), namespaceReg);
+        validationBody = appendLines(validationBody, hookInfo.ValidatorLines);
+
         headerStr = ['function val = validate_' nm '(obj, val)'];
         if isempty(validationBody)
             functionStr = [headerStr newline 'end'];
@@ -32,6 +35,25 @@
     end
 end
 
+function typeName = classNameParts(className)
+    classNameSplit = strsplit(className, '.');
+    typeName = classNameSplit{end};
+end
+
+function combinedLines = appendLines(validationBody, additionalLines)
+    if isempty(additionalLines)
+        combinedLines = validationBody;
+        return
+    end
+
+    validationLines = {};
+    if ~isempty(strtrim(validationBody))
+        validationLines = {strtrim(validationBody)};
+    end
+
+    combinedLines = strjoin([validationLines, additionalLines], newline);
+end
+
 function unitValidationStr = fillUnitValidation(name, prop, namespaceReg)
     unitValidationStr = '';
     if ~isscalar(prop) || (isa(prop, 'file.Link') && prop.isConstrainedSet)

+file/getPropertyHooks.m

@@ -0,0 +1,28 @@
+function hooks = getPropertyHooks(propName, prop, typeName, namespace)
+%GETPROPERTYHOOKS Return custom generator hooks for property validators/setters.
+
+    hooks = struct( ...
+        'ValidatorLines', {{}}, ...
+        'PostsetStatements', {{}});
+
+    fullClassName = namespace.getFullClassName(typeName);
+
+    if strcmp(fullClassName, 'types.hdmf_common.DynamicTable') ...
+            && strcmp(propName, 'colnames')
+        hooks.ValidatorLines = { ...
+            'val = types.util.dynamictable.validateColnames(val);' };
+    end
+
+    if file.isDynamicTableDescendant(typeName, namespace) ...
+            && isSchemaDefinedDynamicTableColumn(propName, prop)
+        hooks.PostsetStatements = { ...
+            sprintf('types.util.dynamictable.syncNamedColumn(obj, ''%s'');', propName) };
+    end
+end
+
+function tf = isSchemaDefinedDynamicTableColumn(propName, prop)
+    tf = isa(prop, 'file.Dataset') ...
+        && ~prop.isConstrainedSet ...
+        && ~strcmp(propName, 'id') ...
+        && ~endsWith(propName, '_index');
+end

+file/isDynamicTableDescendant.m

@@ -0,0 +1,18 @@
+function tf = isDynamicTableDescendant(name, namespace)
+%ISDYNAMICTABLEDESCENDANT Check if a type inherits from DynamicTable.
+
+    tf = false;
+
+    if strcmp(name, 'DynamicTable')
+        tf = true;
+        return
+    end
+
+    ancestry = namespace.getRootBranch(name);
+    for iAncestor = 1:length(ancestry)
+        parentRaw = ancestry{iAncestor};
+        typeDefIndex = isKey(parentRaw, namespace.TYPEDEF_KEYS);
+        ancestorName = parentRaw(namespace.TYPEDEF_KEYS{typeDefIndex});
+        tf = tf || strcmp(ancestorName, 'DynamicTable');
+    end
+end

+tests/+unit/dynamicTableTest.m

@@ -16,14 +16,21 @@ function testEmptyTable(testCase)
                 'description', 'test dynamic table column',...
                 'id',  types.hdmf_common.ElementIdentifiers('data', (0:2)'));
 
-            % validate that properties checking works (start_time is a
-            % property of TimeIntervals)
-            testCase.verifyError(@()types.core.TimeIntervals(...
-                'description', 'test error', ...
-                'start_time', types.hdmf_common.VectorData( ...
+            % Verify that start_time is added to colnames when creating an
+            % empty VectorData column for the start_time property
+            try
+                timeIntervals = types.core.TimeIntervals(...
+                    'description', 'test error', ...
+                    'start_time', types.hdmf_common.VectorData( ...
                     'description', 'start time column', ...
-                    'data', (1:5)')), ...
-                'NWB:DynamicTable:CheckConfig:ColumnNamesMismatch');
+                    'data', (1:5)' ...
+                    ));
+                testCase.verifyTrue(contains(timeIntervals.colnames, 'start_time'), ...
+                    'Expected colnames to include start_time')
+            catch
+                testCase.verifyFail(['Failed to create an empty TimeIntervals ' ...
+                    'table with start_time column.'])
+            end
 
             % validate that "vectordata" set checking works.
             testCase.verifyError(@()types.core.TimeIntervals(...
@@ -350,6 +357,59 @@ function testAddDataPipeWithWrongHeightToDynamicTable(testCase)
                 @() dynamicTable.addColumn('columnB', columnB), ...
                 'NWB:DynamicTable:AddColumn:MissingRows');
         end
+
+        function testDuplicateColnamesFailValidation(testCase)
+            testCase.verifyError( ...
+                @() types.hdmf_common.DynamicTable( ...
+                    'description', 'test table with duplicate colnames', ...
+                    'colnames', {'columnA', 'columnA'}), ...
+                'NWB:DynamicTable:DuplicateColumnNames');
+        end
+
+        function testDirectAssignmentSyncsNamedColumnsToColnames(testCase)
+            dynamicTable = types.core.TimeIntervals( ...
+                'description', 'test direct assignment');
+
+            dynamicTable.start_time = types.hdmf_common.VectorData( ...
+                'description', 'start time column', ...
+                'data', single((1:3)'));
+            dynamicTable.stop_time = types.hdmf_common.VectorData( ...
+                'description', 'stop time column', ...
+                'data', single((2:4)'));
+
+            testCase.verifyEqual(dynamicTable.colnames, {'start_time', 'stop_time'});
+        end
+
+        function testCheckConfigDetectsColumnsMissingFromColnames(testCase)
+            dynamicTable = types.hdmf_common.DynamicTable( ...
+                'description', 'test table with unregistered column', ...
+                'colnames', {'columnA'});
+
+            dynamicTable.vectordata.set('columnB', types.hdmf_common.VectorData( ...
+                'description', 'column b', ...
+                'data', (1:3)'));
+
+            testCase.verifyError( ...
+                @() types.util.dynamictable.checkConfig(dynamicTable), ...
+                'NWB:DynamicTable:CheckConfig:ColumnNamesMismatch');
+        end
+
+        function testExportDetectsColumnsMissingFromColnames(testCase)
+            fileName = testCase.getRandomFilename();
+            nwb = tests.factory.NWBFile();
+
+            dynamicTable = types.hdmf_common.DynamicTable( ...
+                'description', 'test table with unregistered column', ...
+                'colnames', {'columnA'});
+            dynamicTable.vectordata.set('columnB', types.hdmf_common.VectorData( ...
+                'description', 'column b', ...
+                'data', (1:3)'));
+
+            nwb.acquisition.set('DynamicTable', dynamicTable);
+
+            testCase.verifyError(@() nwbExport(nwb, fileName), ...
+                'NWB:CustomConstraintUnfulfilled');
+        end
     end
 
     methods (Static, Access=private)