From b5bc40fc1b4df1cadcf3698f8f1f28dafff47370 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 12 Sep 2026 18:27:30 +0200 Subject: [PATCH] fix: take an identifier-only mixed-type link from the instance library A property that holds several types held a bare @id as a MixedTypeReference, whatever the identifier named. When it named a controlled instance, the collection added it as a node, and saving failed because a reference of unknown type is not a metadata type. Controlled terms and IRI strings in the same properties are already taken from the library at construction; a struct carrying only a library IRI now is too, and a lookup the library cannot serve keeps the link as a reference and says why. The serializer no longer queues a reference as a document of its own, at any recursion depth. That closes the same failure for Node.serialize. ParcellationTerminologyVersion and QuantitativeRelationAssessment leave the known round-trip gaps, since the reason they were listed is gone. Co-Authored-By: Claude Fable 5.1 --- code/+openminds/+base/MixedTypeSet.m | 44 +++++++++++++++---- code/+openminds/+base/Serializer.m | 11 ++++- .../+ommtest/+helper/knownRoundTripGap.m | 7 +-- .../tests/unitTests/ControlledInstanceTest.m | 37 ++++++++++++++++ tools/tests/unitTests/SerializationTest.m | 21 +++++++++ 5 files changed, 104 insertions(+), 16 deletions(-) diff --git a/code/+openminds/+base/MixedTypeSet.m b/code/+openminds/+base/MixedTypeSet.m index ec2e9234c..fa2721923 100644 --- a/code/+openminds/+base/MixedTypeSet.m +++ b/code/+openminds/+base/MixedTypeSet.m @@ -279,16 +279,12 @@ end if isfield(structure, 'at_id') % Linked instance - % Support initializing an Instance from a struct with - % an @id. This will act as a placeholder for an - % unresolved linked instance, and the link needs to be - % resolved externally in order to put a real instance in place. - instance = openminds.internal.MixedTypeReference(structure.at_id); - + instance = obj.initializeFromIdentifier(structure.at_id); + elseif isfield(structure, 'x_id') % Linked instance - % Variation of above - instance = openminds.internal.MixedTypeReference(structure.x_id); - + % Variation of above, from a raw jsondecode + instance = obj.initializeFromIdentifier(structure.x_id); + elseif isfield(structure, 'at_type') % Embedded instance instance = openminds.fromTypeName(structure.at_type); instance = instance.fromStruct(structure); @@ -300,6 +296,36 @@ strjoin(fieldnames(structure), ', '), strjoin(obj(1).ALLOWED_TYPES, ', ')) end end + + function instance = initializeFromIdentifier(obj, identifier) + % initializeFromIdentifier - Initialize a linked instance from an identifier alone + % + % A link that carries only an identifier names a node that is not + % here. When the identifier is a controlled instance IRI, the node + % is in the instance library, which every reader has, so it is + % taken from there, as a controlled term built from a bare + % identifier is. Any other link is held as a reference until it is + % resolved externally. So is a controlled instance the library + % turns out not to hold, as happens when an IRI and the library + % file it points to spell the name differently; the lookup can + % fail in several ways, and all of them mean the same thing here, + % so the cause is reported rather than matched. + + if openminds.utility.isInstanceIRI(identifier) + try + instance = openminds.instanceFromIRI(identifier); + mustBeOneOf(instance, obj(1).ALLOWED_TYPES) + return + catch cause + warning('openMINDS:MixedTypeSet:ControlledInstanceNotFound', ... + ['The controlled instance "%s" could not be taken from ', ... + 'the instance library, so the link is kept as a ', ... + 'reference. %s'], identifier, cause.message) + end + end + + instance = openminds.internal.MixedTypeReference(identifier); + end end methods diff --git a/code/+openminds/+base/Serializer.m b/code/+openminds/+base/Serializer.m index bdc706e0f..58cc605bc 100644 --- a/code/+openminds/+base/Serializer.m +++ b/code/+openminds/+base/Serializer.m @@ -13,7 +13,9 @@ % % Linked instances are always written as references and queued to be % emitted as documents of their own, subject to the configured recursion -% depth. Embedded instances are written inline and carry no identifier. +% depth. A linked instance that is itself a reference is written as one +% and not queued. Embedded instances are written inline and carry no +% identifier. % % USAGE: % ------ @@ -338,6 +340,13 @@ function enqueueDocument(obj, instance) % Queue a referenced instance for emission as its own document. + % + % A reference stands for a node that is not here and carries + % nothing to write, so it is never a document of its own. + + if instance.isReference() + return + end childDepth = obj.CurrentDepth + 1; if childDepth > obj.SerializationConfiguration.RecursionDepth diff --git a/tools/tests/+ommtest/+helper/knownRoundTripGap.m b/tools/tests/+ommtest/+helper/knownRoundTripGap.m index ede3ef4d5..4297a5425 100644 --- a/tools/tests/+ommtest/+helper/knownRoundTripGap.m +++ b/tools/tests/+ommtest/+helper/knownRoundTripGap.m @@ -23,12 +23,7 @@ reason = ""; - 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" + if 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 " + ... diff --git a/tools/tests/unitTests/ControlledInstanceTest.m b/tools/tests/unitTests/ControlledInstanceTest.m index 0eebe3124..a3fb132f6 100644 --- a/tools/tests/unitTests/ControlledInstanceTest.m +++ b/tools/tests/unitTests/ControlledInstanceTest.m @@ -62,6 +62,43 @@ function testFromNameCarriesFileIdentifier(testCase) testCase.verifyNotEqual(string(instance.name), "") end + function testIdentifierOnlyLinkToControlledInstanceIsLookedUp(testCase) + % A document that links a property holding several types to a + % controlled instance by identifier alone describes nothing about + % it, so the instance is taken from the library, as a controlled + % term built from a bare identifier is. Held as a reference of + % unknown type instead, it could not be saved. + + licenseIRI = "https://openminds.om-i.org/instances/licenses/CC-BY-4.0"; + + atlasVersion = openminds.sands.atlas.AnatomicalAtlasVersion(); + atlasVersion.usageCondition = struct('at_id', licenseIRI); + + license = atlasVersion.usageCondition(1); + testCase.verifyClass(license, 'openminds.core.data.License') + testCase.verifyFalse(license.isReference()) + testCase.verifyEqual(string(license.id), licenseIRI) + testCase.verifyNotEqual(string(license.fullName), "") + end + + function testIdentifierOnlyLinkToUnknownControlledInstanceStaysReference(testCase) + % The library may not hold the instance an identifier points to, + % as when the two spell a name differently. The link is then kept + % as a reference and the reader is told, rather than the read + % failing. + + missingIRI = "https://openminds.om-i.org/instances/licenses/no-such-license"; + + atlasVersion = openminds.sands.atlas.AnatomicalAtlasVersion(); + testCase.verifyWarning( ... + @() atlasVersion.set("usageCondition", struct('at_id', missingIRI)), ... + 'openMINDS:MixedTypeSet:ControlledInstanceNotFound') + + link = atlasVersion.usageCondition(1); + testCase.verifyClass(link, 'openminds.internal.MixedTypeReference') + testCase.verifyEqual(string(link.id), missingIRI) + end + function testGetControlledInstanceRemote(testCase, instanceSpecification, versionNumber) jsonStr = openminds.internal.getControlledInstance(... instanceSpecification{:}, versionNumber, "FileSource", "github"); diff --git a/tools/tests/unitTests/SerializationTest.m b/tools/tests/unitTests/SerializationTest.m index e11602c86..6309b1d7e 100644 --- a/tools/tests/unitTests/SerializationTest.m +++ b/tools/tests/unitTests/SerializationTest.m @@ -214,6 +214,27 @@ function testCircularGraphSerializesAsReference(testCase) 'Each node should appear exactly once, as its own document.') end + function testReferenceInLinkedPropertyGetsNoDocument(testCase) + % A linked value that is a reference stands for a node that is not + % here. It is written as a reference in the linking document and + % gets no document of its own, whether or not its type is known, + % however deep the serializer is allowed to recurse. + + dataset = openminds.core.Dataset("fullName", "Referencing dataset"); + dataset.digitalIdentifier = openminds.internal.MixedTypeReference( ... + "https://graph.example/instances/doi-001"); + dataset.documentation = openminds.core.DOI( ... + "id", "https://graph.example/instances/doi-002", "IsReference", true); + + serializer = openminds.internal.serializer.JsonLdSerializer( ... + 'RecursionDepth', 2); + documents = string(serializer.serialize(dataset)); + + testCase.assertNumElements(documents, 1) + testCase.verifySubstring(documents, 'instances/doi-001') + testCase.verifySubstring(documents, 'instances/doi-002') + end + function testPropertyHoldingSeveralTypesSerializes(testCase) % A property that accepts several types may hold instances of more % than one of them at once. Those instances cannot be concatenated