From 0bc83942d802107d4d162977694892ec842e6981 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 28 Aug 2026 02:31:44 +0200 Subject: [PATCH] refactor: add a JSON-LD deserializer symmetric with the serializer Serialization had an architecture and deserialization had a function. loadInstances read files, stripped a hardcoded vocabulary prefix, dispatched types, built instances and wired links between them, with a hand-rolled traversal that had no cycle detection and no extension point for another format. BaseDeserializer is the counterpart to BaseSerializer. Subclasses implement parseToStructs for one format; the base class handles type dispatch, instance construction and link wiring. JsonLdDeserializer implements the JSON-LD case and accepts several documents at once. Link wiring moves to LinkWiringVisitor, built on the traversal core introduced earlier in this stack, so it inherits cycle detection and positional replacement rather than reimplementing traversal. A node that cannot be turned into an instance was reported with warning(ME.message), one warning per node, which lost the error identifier and told the caller nothing about which node was lost or how much of the document was missing. Such nodes are now collected and reported once, by identifier, under an identified warning. Callers that cannot use a partial result can ask for an error instead. Resolving the type stays outside that handling. A type in an unknown namespace means the document was written for a different model version, which is a document-level problem, and reporting it per node would leave the caller with an empty result and a warning rather than a failure. loadInstances is now file reading and deserializer selection, and goes from 144 lines to 48. Co-Authored-By: Claude Opus 5 --- .../+internal/+serializer/BaseDeserializer.m | 133 ++++++++++++++ .../+serializer/JsonLdDeserializer.m | 51 ++++++ .../+internal/+serializer/LinkWiringVisitor.m | 76 ++++++++ .../+internal/+store/loadInstances.m | 165 ++++-------------- tools/tests/unitTests/DeserializerTest.m | 149 ++++++++++++++++ 5 files changed, 444 insertions(+), 130 deletions(-) create mode 100644 code/internal/+openminds/+internal/+serializer/BaseDeserializer.m create mode 100644 code/internal/+openminds/+internal/+serializer/JsonLdDeserializer.m create mode 100644 code/internal/+openminds/+internal/+serializer/LinkWiringVisitor.m create mode 100644 tools/tests/unitTests/DeserializerTest.m diff --git a/code/internal/+openminds/+internal/+serializer/BaseDeserializer.m b/code/internal/+openminds/+internal/+serializer/BaseDeserializer.m new file mode 100644 index 00000000..39cb6521 --- /dev/null +++ b/code/internal/+openminds/+internal/+serializer/BaseDeserializer.m @@ -0,0 +1,133 @@ +classdef (Abstract) BaseDeserializer < handle +% BaseDeserializer - Turns serialized data into linked openMINDS instances +% +% The counterpart to BaseSerializer. Subclasses implement parseToStructs +% for one serialization format; this class provides the parts that do +% not depend on the format: dispatching each node to its type, building +% the instances, and wiring the references between them. +% +% USAGE: +% ------ +% Subclasses implement one method: +% +% classdef MyDeserializer < openminds.internal.serializer.BaseDeserializer +% methods (Access = protected) +% function rawStructs = parseToStructs(obj, data) +% ... +% end +% end +% end +% +% See also openminds.internal.serializer.BaseSerializer, +% openminds.internal.serializer.LinkWiringVisitor + + properties (Access = protected) + % What to do with a node that cannot be turned into an instance. + % Warning reports them together once; error raises on the first. + UnreadableNodePolicy (1,1) string ... + {mustBeMember(UnreadableNodePolicy, ["warning", "error"])} = "warning" + end + + methods + function instances = deserialize(obj, data) + % deserialize - Build linked instances from serialized data + % + % Returns a cell array of openMINDS instances. Nodes that cannot + % be read are left out and reported. + + rawStructs = obj.parseToStructs(data); + [instances, unreadable] = obj.instantiateAll(rawStructs); + + obj.reportUnreadableNodes(unreadable); + + if isempty(instances) + return + end + + visitor = openminds.internal.serializer.LinkWiringVisitor(instances); + for i = 1:numel(instances) + visitor.visit(instances{i}); + end + end + end + + methods (Abstract, Access = protected) + rawStructs = parseToStructs(obj, data) + % parseToStructs - Format-specific parse into a cell array of structs + end + + methods (Access = protected) + function [instances, unreadable] = instantiateAll(~, rawStructs) + % instantiateAll - Build one instance per node of the document + + instances = cell(1, numel(rawStructs)); + unreadable = struct('Identifier', {}, 'Reason', {}); + + for i = 1:numel(rawStructs) + node = rawStructs{i}; + + if ~isfield(node, 'at_type') + unreadable(end+1) = struct( ... + 'Identifier', nodeIdentifier(node), ... + 'Reason', "the node has no @type"); %#ok + continue + end + + % Resolving the type is deliberately outside the try. A + % type in an unknown namespace means the whole document + % was written for a different model version, which is a + % document-level problem rather than one bad node, and + % reporting it as a skipped node would leave the caller + % with an empty result and a warning. + typeEnum = openminds.enum.Types.fromAtType(node.at_type); + + try + instances{i} = feval(typeEnum.ClassName, node); + catch ME + unreadable(end+1) = struct( ... + 'Identifier', nodeIdentifier(node), ... + 'Reason', string(ME.message)); %#ok + end + end + + instances = instances(~cellfun(@isempty, instances)); + end + + function reportUnreadableNodes(obj, unreadable) + % reportUnreadableNodes - Report every node that had to be skipped + % + % Reporting once, with the identifier of each node, makes it + % possible to tell how much of a document was lost. Reporting + % each node separately as it failed buried that. + + if isempty(unreadable) + return + end + + details = arrayfun( ... + @(entry) sprintf(' %s: %s', entry.Identifier, entry.Reason), ... + unreadable, 'UniformOutput', false); + + message = sprintf('%d of the nodes in the data could not be read:\n%s', ... + numel(unreadable), strjoin(details, newline)); + + if obj.UnreadableNodePolicy == "error" + error('openMINDS:Deserializer:UnreadableNodes', '%s', message) + else + warning('openMINDS:Deserializer:UnreadableNodes', '%s', message) + end + end + end +end + +function identifier = nodeIdentifier(node) +% Best available name for a node that could not be read. + + if isfield(node, 'at_id') + identifier = string(node.at_id); + elseif isfield(node, 'x_id') + identifier = string(node.x_id); + else + identifier = ""; + end +end diff --git a/code/internal/+openminds/+internal/+serializer/JsonLdDeserializer.m b/code/internal/+openminds/+internal/+serializer/JsonLdDeserializer.m new file mode 100644 index 00000000..62101d13 --- /dev/null +++ b/code/internal/+openminds/+internal/+serializer/JsonLdDeserializer.m @@ -0,0 +1,51 @@ +classdef JsonLdDeserializer < openminds.internal.serializer.BaseDeserializer +% JsonLdDeserializer - Reads openMINDS instances from JSON-LD +% +% The counterpart to JsonLdSerializer. Accepts either a single document +% or several, and both a document holding one instance and a collection +% document holding an @graph. +% +% USAGE: +% ------ +% deserializer = openminds.internal.serializer.JsonLdDeserializer(); +% instances = deserializer.deserialize(jsonText); +% +% See also openminds.internal.serializer.JsonLdSerializer + + properties (Constant) + DefaultFileExtension = ".jsonld" + end + + methods + function obj = JsonLdDeserializer(options) + arguments + options.UnreadableNodePolicy (1,1) string ... + {mustBeMember(options.UnreadableNodePolicy, ["warning", "error"])} = "warning" + end + obj.UnreadableNodePolicy = options.UnreadableNodePolicy; + end + end + + methods (Access = protected) + function rawStructs = parseToStructs(~, data) + % parseToStructs - Decode one or more JSON-LD documents + + arguments + ~ + data (1,:) string + end + + rawStructs = {}; + + for i = 1:numel(data) + decoded = openminds.internal.serializer.jsonld2struct(data(i)); + + if ~iscell(decoded) + decoded = num2cell(decoded); + end + + rawStructs = [rawStructs, reshape(decoded, 1, [])]; %#ok + end + end + end +end diff --git a/code/internal/+openminds/+internal/+serializer/LinkWiringVisitor.m b/code/internal/+openminds/+internal/+serializer/LinkWiringVisitor.m new file mode 100644 index 00000000..bf0428a3 --- /dev/null +++ b/code/internal/+openminds/+internal/+serializer/LinkWiringVisitor.m @@ -0,0 +1,76 @@ +classdef LinkWiringVisitor < openminds.abstract.BaseVisitor +% LinkWiringVisitor - Replaces reference stubs with the instances they name +% +% After a document is parsed, each node is built on its own and its +% linked properties hold stubs carrying an identifier and nothing else. +% This visitor walks the instances and swaps each stub for the instance +% with that identifier, so the separate nodes of a document become one +% connected graph. +% +% A stub naming a controlled instance that is not part of the document +% is resolved from the local instance library instead. +% +% See also openminds.internal.serializer.BaseDeserializer + + properties (Access = private) + % Identifier to instance, for everything in the document + InstancesById containers.Map + end + + methods + function obj = LinkWiringVisitor(instances) + arguments + instances cell + end + + obj.InstancesById = containers.Map( ... + 'KeyType', 'char', 'ValueType', 'any'); + + for i = 1:numel(instances) + obj.InstancesById(char(instances{i}.id)) = instances{i}; + end + end + end + + methods (Access = protected) + function children = doForLinkedEdge(obj, ~, ~, children) + for i = 1:numel(children) + children{i} = obj.wireChild(children{i}); + obj.visit(children{i}); + end + end + + function children = doForEmbeddedEdge(obj, ~, ~, children) + % An embedded instance is written inline, so it is never a stub. + % It can still hold stubs of its own. + + for i = 1:numel(children) + obj.visit(children{i}); + end + end + end + + methods (Access = private) + function child = wireChild(obj, child) + % wireChild - Swap a stub for the instance it names + + if ~child.isUnresolved() + return + end + + identifier = char(child.id); + + if obj.InstancesById.isKey(identifier) + child = obj.InstancesById(identifier); + return + end + + % Not part of the document. A controlled instance can still be + % resolved from the local library; anything else stays a stub + % for the caller to resolve. + if openminds.utility.isInstanceIRI(child.id) + child = openminds.instanceFromIRI(child.id); + end + end + end +end diff --git a/code/internal/+openminds/+internal/+store/loadInstances.m b/code/internal/+openminds/+internal/+store/loadInstances.m index e347834f..d7163f6d 100644 --- a/code/internal/+openminds/+internal/+store/loadInstances.m +++ b/code/internal/+openminds/+internal/+store/loadInstances.m @@ -1,143 +1,48 @@ -function instances = loadInstances(filePath)%, options) -%loadInstances Load metadata instances from file(s) - -% Todo: -% - Add documentation -% - Test that this works in different cases. Single files, collection of -% files, -% - Generalize to work for multiple file formats. +function instances = loadInstances(filePath) +%loadInstances Load metadata instances from one or more files +% +% instances = loadInstances(filePath) reads the given files and returns +% a cell array of openMINDS instances, with the references between them +% resolved to the instances themselves. +% +% The format is chosen from the file extension. Parsing, type dispatch +% and link wiring are the deserializer's work; this function only reads +% the files and picks the deserializer. +% +% Input Arguments: +% filePath - One or more paths to metadata files. +% +% Output Arguments: +% instances - Cell array of openminds.abstract.Schema instances. +% +% See also openminds.internal.serializer.JsonLdDeserializer arguments - filePath (1,:) string = "" - % options.RecursionDepth = 1 - end - - import openminds.internal.serializer.jsonld2struct - - [~, ~, serializationFormat] = fileparts(filePath(1)); - - switch serializationFormat - case ".jsonld" - - % Read one or more files - str = arrayfun(@fileread, filePath, 'UniformOutput', false); - - % Produce a cell array of instances represented as structs - if isscalar(str) - structInstances = jsonld2struct(str); - if ~iscell(structInstances); structInstances={structInstances};end - else - structInstances = cellfun(@jsonld2struct, str, 'UniformOutput', false); - end - - % Create instance objects - instances = cell(size(structInstances)); - for i = 1:numel(structInstances) - - thisInstance = structInstances{i}; - - if ~isfield(thisInstance, 'at_type') - continue % Todo: Why skip? - % instances{i} = struct('id', thisInstance.at_id); - else - openMindsType = thisInstance.at_type; - - typeEnum = openminds.enum.Types.fromAtType(openMindsType); - assert( strcmp( typeEnum.TypeURI, openMindsType), ... - "Instance type does not match schema type. This " + ... - "is not supposed to happen, please report!") - - try - instances{i} = feval(typeEnum.ClassName, thisInstance); - catch ME - warning(ME.message) - end - end - end - - isEmpty = cellfun(@(c) isempty(c), instances); - instances(isEmpty) = []; - - instanceIds = cellfun(@(instance) instance.id, instances, 'UniformOutput', false); - instanceIds = string(instanceIds); - - % Link instances / Resolve linked objects... - for i = 1:numel(instances) - resolveLinks(instances{i}, instanceIds, instances) - end - - otherwise - error('Unkown input format') + filePath (1,:) string = string.empty end - if ~nargout - clear str - end -end - -function resolveLinks(instance, instanceIds, instanceCollection) -%resolveLinks Resolve linked types, i.e replace an @id with the actual -% instance object. - - if isstruct(instance) % Instance is not resolvable (E.g belongs to remote collection) + instances = {}; + if isempty(filePath) return end - metaType = openminds.internal.meta.fromInstance(instance); - - for i = 1:metaType.NumProperties - thisPropertyName = metaType.PropertyNames{i}; - if metaType.isPropertyWithLinkedType(thisPropertyName) - linkedInstances = instance.(thisPropertyName); - - resolvedInstances = cell(size(linkedInstances)); + deserializer = selectDeserializer(filePath(1)); - for j = 1:numel(linkedInstances) - if openminds.utility.isMixedInstance(linkedInstances(j)) - try - instanceId = linkedInstances(j).Instance.id; - catch - instanceId = linkedInstances(j).Instance; - end - else - instanceId = linkedInstances(j).id; - end - - isMatchedInstance = instanceIds == string(instanceId); - - if any(isMatchedInstance) - resolvedInstances{j} = instanceCollection{isMatchedInstance}; - resolveLinks(resolvedInstances{j}, instanceIds, instanceCollection) - else - % Check if instance is a controlled instance - if startsWith(instanceId, "https://openminds.ebrains.eu/instances/") - resolvedInstances{j} = openminds.instanceFromIRI(instanceId); - end - end - end + documents = arrayfun(@(path) string(fileread(path)), filePath); + instances = deserializer.deserialize(documents); +end - try - resolvedInstances = [resolvedInstances{:}]; - catch - assert(isa(resolvedInstances, 'cell'), ... - 'Expected resolved instances to be a cell array') - end +function deserializer = selectDeserializer(filePath) +% Pick a deserializer from the file extension. - if ~isempty(resolvedInstances) - instance.(thisPropertyName) = resolvedInstances; - end - - elseif metaType.isPropertyWithEmbeddedType(thisPropertyName) - embeddedInstances = instance.(thisPropertyName); + [~, ~, fileExtension] = fileparts(filePath); - for j = 1:numel(embeddedInstances) - if openminds.utility.isMixedInstance(embeddedInstances(j)) - embeddedInstance = embeddedInstances(j).Instance; - else - embeddedInstance = embeddedInstances(j); - end - resolveLinks(embeddedInstance, instanceIds, instanceCollection) - end - end + switch lower(fileExtension) + case ".jsonld" + deserializer = openminds.internal.serializer.JsonLdDeserializer(); + otherwise + error('openMINDS:LoadInstances:UnsupportedFormat', ... + ['Unsupported metadata file format "%s". ', ... + 'Supported formats: .jsonld'], fileExtension) end end diff --git a/tools/tests/unitTests/DeserializerTest.m b/tools/tests/unitTests/DeserializerTest.m new file mode 100644 index 00000000..1221e39c --- /dev/null +++ b/tools/tests/unitTests/DeserializerTest.m @@ -0,0 +1,149 @@ +classdef DeserializerTest < matlab.unittest.TestCase +% DeserializerTest - Unit tests for reading openMINDS instances from JSON-LD +% +% See also openminds.internal.serializer.JsonLdDeserializer + + properties (Constant, Access = private) + TypeIRI = "https://openminds.om-i.org/types/" + end + + methods (Test) + function testCollectionDocumentIsRead(testCase) + document = DeserializerTest.collectionDocument( ... + DeserializerTest.personNode("_:person-1", "Ada")); + + instances = testCase.deserialize(document); + + testCase.assertNumElements(instances, 1) + testCase.verifyEqual(instances{1}.givenName, "Ada") + end + + function testLinksBetweenNodesAreWired(testCase) + % Nodes are built one at a time, so a linked property first holds + % a stub. Deserialization must swap the stub for the instance. + + document = DeserializerTest.collectionDocument( ... + sprintf(['{"@id": "_:contact-1", "@type": "%sContactInformation", ', ... + '"email": "ada@example.org"}'], DeserializerTest.TypeIRI), ... + sprintf(['{"@id": "_:person-1", "@type": "%sPerson", ', ... + '"givenName": "Ada", "contactInformation": [{"@id": "_:contact-1"}]}'], ... + DeserializerTest.TypeIRI)); + + instances = testCase.deserialize(document); + person = DeserializerTest.findByClass(instances, 'openminds.core.actors.Person'); + + testCase.assertNotEmpty(person) + testCase.verifyEqual(person.contactInformation.email, "ada@example.org") + end + + function testLinksAreWiredAcrossDocuments(testCase) + % Instances loaded from separate files form one graph, which is + % what a folder of one file per instance relies on. + + firstDocument = DeserializerTest.collectionDocument( ... + sprintf(['{"@id": "_:contact-2", "@type": "%sContactInformation", ', ... + '"email": "grace@example.org"}'], DeserializerTest.TypeIRI)); + secondDocument = DeserializerTest.collectionDocument( ... + sprintf(['{"@id": "_:person-2", "@type": "%sPerson", ', ... + '"givenName": "Grace", "contactInformation": [{"@id": "_:contact-2"}]}'], ... + DeserializerTest.TypeIRI)); + + instances = testCase.deserialize([firstDocument, secondDocument]); + person = DeserializerTest.findByClass(instances, 'openminds.core.actors.Person'); + + testCase.assertNotEmpty(person) + testCase.verifyEqual(person.contactInformation.email, "grace@example.org") + end + + function testCircularDocumentTerminates(testCase) + % Two nodes referring to each other must not be wired forever. + + document = DeserializerTest.collectionDocument( ... + sprintf(['{"@id": "_:type-a", "@type": "%sContentType", ', ... + '"name": "a/type", "isBasedOn": [{"@id": "_:type-b"}]}'], ... + DeserializerTest.TypeIRI), ... + sprintf(['{"@id": "_:type-b", "@type": "%sContentType", ', ... + '"name": "b/type", "isBasedOn": [{"@id": "_:type-a"}]}'], ... + DeserializerTest.TypeIRI)); + + instances = testCase.deserialize(document); + + testCase.assertNumElements(instances, 2) + first = DeserializerTest.findById(instances, "_:type-a"); + testCase.verifyEqual(first.isBasedOn.name, "b/type") + end + + function testNodeWithoutTypeIsReported(testCase) + % A node with no @type cannot be turned into an instance. Skipping + % it silently hides how much of a document was lost. + + document = DeserializerTest.collectionDocument( ... + '{"@id": "_:untyped-1", "name": "no type here"}', ... + DeserializerTest.personNode("_:person-3", "Ada")); + + instances = testCase.verifyWarning( ... + @() testCase.deserialize(document), ... + 'openMINDS:Deserializer:UnreadableNodes'); + + testCase.verifyNumElements(instances, 1, ... + 'The readable node should still be returned.') + end + + function testUnreadableNodesCanBeAnError(testCase) + % A caller that cannot work with a partial result can ask for the + % read to fail instead. + + document = DeserializerTest.collectionDocument( ... + '{"@id": "_:untyped-2", "name": "no type here"}'); + + deserializer = openminds.internal.serializer.JsonLdDeserializer( ... + "UnreadableNodePolicy", "error"); + + testCase.verifyError(@() deserializer.deserialize(document), ... + 'openMINDS:Deserializer:UnreadableNodes') + end + end + + methods (Access = private) + function instances = deserialize(~, documents) + deserializer = openminds.internal.serializer.JsonLdDeserializer(); + instances = deserializer.deserialize(documents); + end + end + + methods (Static, Access = private) + function document = collectionDocument(varargin) + % Wrap node documents in a collection document with an @graph. + + document = sprintf( ... + '{"@context": {"@vocab": "https://openminds.om-i.org/props/"}, "@graph": [%s]}', ... + strjoin(varargin, ', ')); + document = string(document); + end + + function node = personNode(identifier, givenName) + node = sprintf('{"@id": "%s", "@type": "%sPerson", "givenName": "%s"}', ... + identifier, DeserializerTest.TypeIRI, givenName); + end + + function instance = findByClass(instances, className) + instance = []; + for i = 1:numel(instances) + if isa(instances{i}, className) + instance = instances{i}; + return + end + end + end + + function instance = findById(instances, identifier) + instance = []; + for i = 1:numel(instances) + if string(instances{i}.id) == identifier + instance = instances{i}; + return + end + end + end + end +end