diff --git a/code/internal/+openminds/+abstract/ControlledTerm.m b/code/internal/+openminds/+abstract/ControlledTerm.m index 5beb166b..fb6bae36 100644 --- a/code/internal/+openminds/+abstract/ControlledTerm.m +++ b/code/internal/+openminds/+abstract/ControlledTerm.m @@ -35,7 +35,7 @@ propValues.id (1,1) string end - obj.initializeControlledTerm(instanceSpec, propValues) + obj = obj.initializeControlledTerm(instanceSpec, propValues); end end end diff --git a/code/internal/+openminds/+abstract/ControlledTermBase.m b/code/internal/+openminds/+abstract/ControlledTermBase.m index 14d69693..81fc29c2 100644 --- a/code/internal/+openminds/+abstract/ControlledTermBase.m +++ b/code/internal/+openminds/+abstract/ControlledTermBase.m @@ -19,7 +19,13 @@ end methods (Access = protected) - function initializeControlledTerm(obj, instanceSpec, propValues) + function obj = initializeControlledTerm(obj, instanceSpec, propValues) + % initializeControlledTerm - Populate one or more terms from a spec + % + % The object array is returned because a struct array spec + % produces one term per element. Expanding obj inside this method + % only grows the local copy, so a caller that ignores the return + % value keeps just the first element. if isstring(instanceSpec) && isscalar(instanceSpec) && instanceSpec == "" instanceSpec = string.empty; end diff --git a/code/internal/+openminds/+abstract/private/controlledTerms/v2/ControlledTerm.m b/code/internal/+openminds/+abstract/private/controlledTerms/v2/ControlledTerm.m index fe4ed0c3..c1c19d1f 100644 --- a/code/internal/+openminds/+abstract/private/controlledTerms/v2/ControlledTerm.m +++ b/code/internal/+openminds/+abstract/private/controlledTerms/v2/ControlledTerm.m @@ -32,7 +32,7 @@ propValues.id (1,1) string end - obj.initializeControlledTerm(instanceSpec, propValues) + obj = obj.initializeControlledTerm(instanceSpec, propValues); end end end diff --git a/code/internal/+openminds/+abstract/private/controlledTerms/v3/ControlledTerm.m b/code/internal/+openminds/+abstract/private/controlledTerms/v3/ControlledTerm.m index 5beb166b..fb6bae36 100644 --- a/code/internal/+openminds/+abstract/private/controlledTerms/v3/ControlledTerm.m +++ b/code/internal/+openminds/+abstract/private/controlledTerms/v3/ControlledTerm.m @@ -35,7 +35,7 @@ propValues.id (1,1) string end - obj.initializeControlledTerm(instanceSpec, propValues) + obj = obj.initializeControlledTerm(instanceSpec, propValues); end end end diff --git a/tools/tests/+ommtest/+helper/knownRoundTripGap.m b/tools/tests/+ommtest/+helper/knownRoundTripGap.m index c5ba8f64..ede3ef4d 100644 --- a/tools/tests/+ommtest/+helper/knownRoundTripGap.m +++ b/tools/tests/+ommtest/+helper/knownRoundTripGap.m @@ -23,9 +23,16 @@ reason = ""; - if ismember(typeName, multiValuedControlledInstanceGap()) - reason = "Multi-valued properties linking to controlled instances " + ... - "lose all but the first entry on reload."; + if ismember(typeName, ["ParcellationTerminologyVersion", "QuantitativeRelationAssessment"]) + reason = "Serializing a property that holds an unresolved " + ... + "MixedTypeReference fails, because the meta type registry does " + ... + "not recognise it as a metadata type."; + + elseif typeName == "ChemicalSubstance" + reason = "A controlled instance whose name contains characters that " + ... + "are not valid in a MATLAB identifier cannot be looked up from a " + ... + "bare reference, so it reloads as an empty term with a new " + ... + "blank node identifier."; elseif typeName == "TermSuggestion" reason = "addExistingTerminology is typed as an openMINDS type but " + ... @@ -35,23 +42,3 @@ "at all, so fixing it means changing the generator."; end end - -function typeNames = multiValuedControlledInstanceGap() -% Types holding a multi-valued property that links to controlled instances. -% -% 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", ... - "ContentType", "CustomAnnotation", "DataAnalysis", "DataCopy", ... - "DatasetVersion", "Dependency", "File", "FileBundle", ... - "FilePathPattern", "GenericComputation", "LocalFile", ... - "ModelValidation", "Optimization", "ParcellationTerminologyVersion", ... - "QuantitativeRelationAssessment", "Setup", "Simulation", ... - "SoftwareVersion", "SubjectGroup", "SubjectGroupState", ... - "SubjectState", "TissueSample", "TissueSampleCollection", ... - "TissueSampleCollectionState", "TissueSampleState", ... - "ValidationTest", "Visualization"]; -end diff --git a/tools/tests/unitTests/ControlledTermTest.m b/tools/tests/unitTests/ControlledTermTest.m index dc07f088..167d96f6 100644 --- a/tools/tests/unitTests/ControlledTermTest.m +++ b/tools/tests/unitTests/ControlledTermTest.m @@ -30,6 +30,38 @@ function testUserDefinedTermSurvivesDeserialization(testCase) testCase.verifyEqual(string(term.id), "_:a-user-defined-term") end + function testStructArrayProducesOneTermPerElement(testCase) + % A multi-valued property deserializes to a struct array of + % references. Each element must become its own term, or every + % entry after the first is lost. + + references = struct('at_id', { ... + "https://openminds.om-i.org/instances/laterality/left", ... + "https://openminds.om-i.org/instances/laterality/right"}); + + terms = openminds.controlledterms.Laterality(references); + + testCase.assertNumElements(terms, 2) + testCase.verifyEqual(string(terms(1).id), ... + "https://openminds.om-i.org/instances/laterality/left") + testCase.verifyEqual(string(terms(2).id), ... + "https://openminds.om-i.org/instances/laterality/right") + end + + function testMultiValuedControlledPropertyKeepsEveryEntry(testCase) + % The same case reached through a property rather than the + % constructor, which is how deserialization gets there. + + references = struct('at_id', { ... + "https://openminds.om-i.org/instances/laterality/left", ... + "https://openminds.om-i.org/instances/laterality/right"}); + + annotation = openminds.sands.AtlasAnnotation(); + annotation.laterality = references; + + testCase.verifyNumElements(annotation.laterality, 2) + end + function testReferenceToKnownTermIsLookedUp(testCase) % A document carrying only an identifier describes nothing, so the % term is populated from the controlled instance library instead.