fix: preserve controlled term values when deserializing

code/internal/+openminds/+abstract/ControlledTermBase.m

@@ -47,12 +47,7 @@ function initializeControlledTerm(obj, instanceSpec, propValues)
                         obj(numInstances) = feval(class(obj));
                     end
                     for i = 1:numel(instanceSpec)
-                        if isfield(instanceSpec(i), 'at_id')
-                            iri = instanceSpec(i).at_id;
-                        elseif isfield(instanceSpec(i), 'x_id')
-                            iri = instanceSpec(i).x_id;
-                        end
-                        obj(i).deserializeFromName(iri);
+                        obj(i).initializeFromStructure(instanceSpec(i));
                     end
                 else
                     error('openMINDS:ControlledTerm:InvalidInput', ...
@@ -81,6 +76,36 @@ function initializeControlledTerm(obj, instanceSpec, propValues)
     end
 
     methods (Access = private)
+        function initializeFromStructure(obj, structure)
+        % initializeFromStructure - Populate this term from a decoded document
+        %
+        %   A structure that carries property values is a serialized
+        %   instance and is authoritative: its values are used as they
+        %   stand. That is the only way a term defined by a user, which by
+        %   definition is not in the controlled instance library, can
+        %   survive being written out and read back.
+        %
+        %   A structure that carries nothing but an identifier is a
+        %   reference. There is no data to take from it, so the term is
+        %   looked up in the controlled instance library instead.
+
+            identifier = openminds.internal.utility.getStructIdentifier(structure);
+
+            if openminds.abstract.ControlledTermBase.isBareReference(structure)
+                obj.deserializeFromName(identifier);
+                return
+            end
+
+            obj.fromStruct(structure);
+
+            % fromStruct assigns the identifier from an at_id or x_id
+            % field, but a document written without identifiers has
+            % neither, and the constructor has already generated one.
+            if ~openminds.abstract.ControlledTermBase.isEmptyValue(identifier)
+                obj.assignInstanceId(identifier);
+            end
+        end
+
         function deserializeFromName(obj, instanceName)
 
             import openminds.internal.getControlledInstance
@@ -158,6 +183,23 @@ function deserializeFromName(obj, instanceName)
             typeName = string(typeName);
         end
 
+        function tf = isBareReference(structure)
+        % isBareReference - True when a document carries no property values
+        %
+        %   Such a document points at a term without describing it, so
+        %   there is nothing to populate the instance from. The document
+        %   may still list every property with an empty value, which is
+        %   what a serializer writes for an unpopulated term when it
+        %   includes empty properties, so the test is on values rather
+        %   than on the presence of fields.
+
+            valueFields = setdiff(string(fieldnames(structure))', ...
+                openminds.internal.utility.jsonLdKeywordFields());
+            hasValue = arrayfun(@(name) ...
+                ~openminds.abstract.ControlledTermBase.isEmptyValue(structure.(name)), valueFields);
+            tf = ~any(hasValue);
+        end
+
         function tf = isEmptyValue(value)
             if isempty(value)
                 tf = true;

code/internal/+openminds/+internal/+utility/getStructIdentifier.m

@@ -0,0 +1,25 @@
+function identifier = getStructIdentifier(S)
+%getStructIdentifier Identifier carried by a decoded JSON-LD node, if any
+%
+%   identifier = getStructIdentifier(S) returns the @id of a decoded node
+%   as a string. The field is at_id when the node was decoded by this
+%   library and x_id when it came from MATLAB's jsondecode; both are
+%   accepted. Returns "" when the node carries no identifier.
+
+    arguments
+        S (1,1) struct
+    end
+
+    if isfield(S, 'at_id')
+        identifier = string(S.at_id);
+    elseif isfield(S, 'x_id')
+        identifier = string(S.x_id);
+    else
+        identifier = "";
+    end
+
+    % A null in the document decodes to [], which string() keeps empty.
+    if isempty(identifier)
+        identifier = "";
+    end
+end

code/internal/+openminds/+internal/+utility/jsonLdKeywordFields.m

@@ -0,0 +1,11 @@
+function keywordFields = jsonLdKeywordFields()
+%jsonLdKeywordFields Field names that carry JSON-LD keywords in a decoded node
+%
+%   keywordFields = jsonLdKeywordFields() returns the struct field names a
+%   decoded JSON-LD node uses for @id, @type and @context, in both the at_
+%   form written by this library's decoder and the x_ form written by
+%   MATLAB's jsondecode. Every other field of a decoded node is a property
+%   value.
+
+    keywordFields = ["at_id", "x_id", "at_type", "x_type", "at_context", "x_context"];
+end

tools/tests/+ommtest/+helper/knownRoundTripGap.m

@@ -5,9 +5,9 @@
 %   explaining why the given openMINDS type currently fails to round trip
 %   through JSON-LD, or an empty string if the type is expected to succeed.
 %
-%   Every entry here is a defect in the library, not in the test. This list
-%   is expected to shrink. When a fix lands, the corresponding entry must
-%   be removed so the round-trip test starts guarding the fixed behaviour.
+%   Every entry here is a defect, not a property of the test. This list is
+%   expected to shrink. When a fix lands, the corresponding entry must be
+%   removed so the round-trip test starts guarding the fixed behaviour.
 %
 %   Input Arguments:
 %     typeName - Short name of an openMINDS type, e.g. "Person".
@@ -23,32 +23,25 @@
 
     reason = "";
 
-    if isControlledTermType(typeName)
-        reason = "Controlled terms defined by the user lose every property " + ...
-            "on reload. ControlledTermBase/initializeControlledTerm discards " + ...
-            "the decoded struct and passes only the identifier to " + ...
-            "deserializeFromName, which finds no matching controlled instance " + ...
-            "and returns an empty object.";
-        return
-    end
-
-    if ismember(typeName, residualGapTypes())
+    if ismember(typeName, multiValuedControlledInstanceGap())
         reason = "Multi-valued properties linking to controlled instances " + ...
             "lose all but the first entry on reload.";
-    end
-end
 
-function tf = isControlledTermType(typeName)
-    className = openminds.enum.Types(typeName).ClassName;
-    tf = any(ismember(superclasses(className), {'openminds.abstract.ControlledTerm'}));
+    elseif typeName == "TermSuggestion"
+        reason = "addExistingTerminology is typed as an openMINDS type but " + ...
+            "is not registered in LINKED_PROPERTIES, so it serializes inline " + ...
+            "without a type or an identifier and cannot be read back. The " + ...
+            "generated controlled term classes do not declare that constant " + ...
+            "at all, so fixing it means changing the generator.";
+    end
 end
 
-function typeNames = residualGapTypes()
-% Types that fail for reasons other than the controlled term defect.
+function typeNames = multiValuedControlledInstanceGap()
+% Types holding a multi-valued property that links to controlled instances.
 %
-%   Unlike the controlled term case there is no clean structural predicate
-%   for these, so they are listed explicitly. Determined by sweeping every
-%   type through save and load; see the round-trip test for the procedure.
+%   There is no clean structural predicate for these, because many types
+%   with such a property round trip correctly, so they are listed
+%   explicitly. Determined by sweeping every type through save and load.
 
     typeNames = [ ...
         "Accessibility", "AtlasAnnotation", "ChemicalSubstance", ...

tools/tests/unitTests/ControlledTermTest.m

@@ -10,6 +10,39 @@ function testKnownInstanceCreatesLightweightReference(testCase)
                 openminds.constant.BaseURI + "/instances/contributionType/authoring")
         end
 
+        function testUserDefinedTermSurvivesDeserialization(testCase)
+        % A controlled term defined by a user is not in the controlled
+        % instance library, so there is nothing to look it up by. Its
+        % values have to be taken from the document it was read from.
+
+            structure = struct( ...
+                'at_id', "_:a-user-defined-term", ...
+                'at_type', "https://openminds.om-i.org/types/Species", ...
+                'name', "Novel species", ...
+                'definition', "A species that is not in the library.", ...
+                'synonym', {{'first synonym', 'second synonym'}});
+
+            term = openminds.controlledterms.Species(structure);
+
+            testCase.verifyEqual(term.name, "Novel species")
+            testCase.verifyEqual(term.definition, "A species that is not in the library.")
+            testCase.verifyEqual(term.synonym, ["first synonym", "second synonym"])
+            testCase.verifyEqual(string(term.id), "_:a-user-defined-term")
+        end
+
+        function testReferenceToKnownTermIsLookedUp(testCase)
+        % A document carrying only an identifier describes nothing, so the
+        % term is populated from the controlled instance library instead.
+
+            structure = struct( ...
+                'at_id', "https://openminds.om-i.org/instances/species/homoSapiens");
+
+            term = openminds.controlledterms.Species(structure);
+
+            testCase.verifyEqual(term.name, "Homo sapiens")
+            testCase.verifyNotEmpty(term.definition)
+        end
+
         function testOlderControlledTermPropertiesAreAccepted(testCase)
             sourceText = fileread(testCase.getControlledTermBasePath("v2"));