Skip to content

fix: take an identifier-only mixed-type link from the instance library #196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ehennestad wants to merge 1 commit into from
Open

fix: take an identifier-only mixed-type link from the instance library #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 35 additions & 9 deletions code/+openminds/+base/MixedTypeSet.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
11 changes: 10 additions & 1 deletion code/+openminds/+base/Serializer.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
% ------
Expand Down Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions tools/tests/+ommtest/+helper/knownRoundTripGap.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 " + ...
Expand Down
37 changes: 37 additions & 0 deletions tools/tests/unitTests/ControlledInstanceTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
21 changes: 21 additions & 0 deletions tools/tests/unitTests/SerializationTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading