diff --git a/code/internal/+openminds/+abstract/ControlledTermBase.m b/code/internal/+openminds/+abstract/ControlledTermBase.m index b7ce6dd1..f9131e8e 100644 --- a/code/internal/+openminds/+abstract/ControlledTermBase.m +++ b/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. + + jsonLdKeywordFields = ["at_id", "x_id", "at_type", "x_type", "at_context", "x_context"]; + valueFields = setdiff(string(fieldnames(structure))', jsonLdKeywordFields); + hasValue = arrayfun(@(name) ... + ~openminds.abstract.ControlledTermBase.isEmptyValue(structure.(name)), valueFields); + tf = ~any(hasValue); + end + function tf = isEmptyValue(value) if isempty(value) tf = true; diff --git a/code/internal/+openminds/+internal/+utility/getStructIdentifier.m b/code/internal/+openminds/+internal/+utility/getStructIdentifier.m new file mode 100644 index 00000000..3061299f --- /dev/null +++ b/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 diff --git a/tools/tests/+ommtest/+helper/knownRoundTripGap.m b/tools/tests/+ommtest/+helper/knownRoundTripGap.m index 6040168e..c5ba8f64 100644 --- a/tools/tests/+ommtest/+helper/knownRoundTripGap.m +++ b/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", ... diff --git a/tools/tests/unitTests/ControlledTermTest.m b/tools/tests/unitTests/ControlledTermTest.m index fb4db624..dc07f088 100644 --- a/tools/tests/unitTests/ControlledTermTest.m +++ b/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"));