From 24f5d4a96d6318d99efca0bce4c169ed0dee8839 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 28 Aug 2026 01:51:32 +0200 Subject: [PATCH] refactor: add graph traversal core and move resolution onto it Resolution walked the instance graph by hand inside Schema.resolve, with no cycle detection and no way to store an instance that a resolver returned. This introduces the shared traversal machinery described in issue #69 and moves resolution onto it. TraversalCore holds the primitives every traversal of an instance graph needs: enumerating linked and embedded edges, unwrapping mixed type values, writing children back to a property, and tracking which nodes have been seen. The same traversal is currently written three times, in the serializer, in Schema.resolve and in loadInstances, each with its own mixed type handling and only one of them with cycle detection. BaseVisitor is the side-effecting protocol on top of it. Edges rather than nodes are the unit of work, which is a departure from the sketch in issue #69. A node hook cannot store a replacement, because replacing a child needs the parent, the property and the position within it. Since a reference whose type is unknown until it is probed can only be resolved by replacement, a node hook cannot express resolution at all. Returning the child list also keeps the remaining children in position. Resolution splits in two. AbstractLinkResolver is now an interface for fetching the data behind one kind of identifier, and no longer walks anything. ResolvingVisitor owns the traversal and selects a resolver for each reference it meets. Keeping the resolver as the visitor, as issue #69 proposed, cannot handle a graph holding references from more than one source, because the traversal would be bound to a single resolver. Schema.resolve reduces to constructing a visitor and calling it. The recursion, depth accounting and resolver lookup leave the class. The static resolve and canResolve methods become instance methods, and resolve is renamed resolveNode to separate it from Schema.resolve. Resolvers may now hold state such as clients and caches. The registry holds resolvers in a cell array rather than relying on matlab.mixin.Heterogeneous, which the resolvers no longer share. Breaking change for openminds-kg-sync: KGResolver must rename its static resolve to an instance method resolveNode, make canResolve an instance method, and drop the NumLinksToResolve argument, which the visitor now owns. Its existing distinction between populating a reference node and building a new instance carries over unchanged and is now what the protocol expects. Co-Authored-By: Claude Opus 5 --- .../+openminds/+abstract/BaseVisitor.m | 95 +++++++++++ code/internal/+openminds/+abstract/Schema.m | 82 +++++----- .../+internal/+graph/TraversalCore.m | 150 ++++++++++++++++++ .../+resolver/AbstractLinkResolver.m | 51 +++--- .../+internal/+resolver/InstanceResolver.m | 29 ++-- .../+resolver/LinkResolverRegistry.m | 75 +++------ .../+internal/+resolver/ResolvingVisitor.m | 86 ++++++++++ .../+openminds/+internal/MixedTypeReference.m | 11 -- .../+ommtest/+helper/+mock/MockLinkResolver.m | 12 +- .../+helper/+mock/ReplacingMockLinkResolver.m | 37 +++++ tools/tests/unitTests/ResolverTest.m | 70 +++++++- 11 files changed, 548 insertions(+), 150 deletions(-) create mode 100644 code/internal/+openminds/+abstract/BaseVisitor.m create mode 100644 code/internal/+openminds/+internal/+graph/TraversalCore.m create mode 100644 code/internal/+openminds/+internal/+resolver/ResolvingVisitor.m create mode 100644 tools/tests/+ommtest/+helper/+mock/ReplacingMockLinkResolver.m diff --git a/code/internal/+openminds/+abstract/BaseVisitor.m b/code/internal/+openminds/+abstract/BaseVisitor.m new file mode 100644 index 00000000..29dc9c43 --- /dev/null +++ b/code/internal/+openminds/+abstract/BaseVisitor.m @@ -0,0 +1,95 @@ +classdef (Abstract) BaseVisitor < openminds.internal.graph.TraversalCore +% BaseVisitor - Walk an instance graph and act on each node +% +% Subclass this for operations that inspect or modify an instance graph +% in place: resolving references, wiring up links after deserialization, +% validating. Each node is visited at most once per traversal, so a +% circular graph terminates. +% +% USAGE: +% ------ +% Implement the two edge methods. Each receives the parent node, the +% name of the property the edge belongs to, and the child instances on +% it, and returns the child instances to keep: +% +% classdef MyVisitor < openminds.abstract.BaseVisitor +% methods (Access = protected) +% function children = doForLinkedEdge(obj, parentNode, propertyName, children) +% for i = 1:numel(children) +% children{i} = obj.visit(children{i}); +% end +% end +% function children = doForEmbeddedEdge(obj, parentNode, propertyName, children) +% ... +% end +% end +% end +% +% Edges rather than nodes are the unit of work because a child cannot +% always be updated in place. A reference whose type is not known until +% it is probed has to be replaced by a new instance of the discovered +% type, and replacing it needs the parent, the property and the position +% in the property. Returning the child list also keeps the remaining +% children in their original positions. +% +% See also openminds.internal.graph.TraversalCore + + methods (Sealed) + function node = visit(obj, node) + % visit - Traverse a node and everything reachable from it + % + % The node is returned because visiting it may replace it. A + % caller holding the node directly, rather than through a + % property of a parent, has to take the returned value. + + arguments + obj (1,1) openminds.abstract.BaseVisitor + node (1,1) openminds.abstract.Schema + end + + if obj.wasVisited(node) + return + end + obj.markVisited(node); + + node = obj.onVisitNode(node); + + obj.traverseEdges(node, obj.getLinkedEdges(node), @obj.doForLinkedEdge); + obj.traverseEdges(node, obj.getEmbeddedEdges(node), @obj.doForEmbeddedEdge); + + obj.onLeaveNode(node); + end + end + + methods (Abstract, Access = protected) + children = doForLinkedEdge(obj, parentNode, propertyName, children) + % doForLinkedEdge - Act on the children of a linked property + + children = doForEmbeddedEdge(obj, parentNode, propertyName, children) + % doForEmbeddedEdge - Act on the children of an embedded property + end + + methods (Access = protected) % Optional hooks + function node = onVisitNode(~, node) + % onVisitNode - Act on a node before its edges are traversed + % + % Return a different instance to replace the node. + end + + function onLeaveNode(~, ~) + % onLeaveNode - Act on a node after its edges have been traversed + end + end + + methods (Access = private) + function traverseEdges(obj, node, edges, edgeFunction) + % traverseEdges - Apply an edge method to each edge and store the result + + for i = 1:numel(edges) + propertyName = edges(i).PropertyName; + children = edgeFunction(node, propertyName, edges(i).Children); + obj.setEdgeChildren(node, propertyName, children); + end + end + end +end diff --git a/code/internal/+openminds/+abstract/Schema.m b/code/internal/+openminds/+abstract/Schema.m index 6f61a9b0..5061855d 100644 --- a/code/internal/+openminds/+abstract/Schema.m +++ b/code/internal/+openminds/+abstract/Schema.m @@ -97,6 +97,13 @@ % - instance (openminds.abstract.Schema) - % an openMINDS typed metadata instance % + % Output Arguments: + % - instance - the resolved instance or instances. Resolving a + % reference whose type was not known produces an instance of a + % different class, so the result is returned rather than + % assigned in place. If an array resolves to more than one + % class, the result is a cell array. + % % - options (name-value pairs) - % Optional name-value pairs. Available options: % @@ -117,36 +124,31 @@ % options.IsEmbedded = false - Todo? end + visitorOptions = {"RemainingLinkDepth", options.NumLinksToResolve}; + if isfield(options, 'LinkResolver') + visitorOptions = [visitorOptions, {"LinkResolver", options.LinkResolver}]; + end + + % Results are collected in a cell array rather than assigned + % back into obj, because resolving a reference whose type was + % unknown produces an instance of a different class, which + % cannot be stored in an array of the original class. + resolved = cell(1, numel(obj)); for i = 1:numel(obj) - if obj(i).IsReference - resolver = obj(i).selectLinkResolver(options); - obj(i) = resolver.resolve(obj(i), ... - "NumLinksToResolve", options.NumLinksToResolve); - obj(i).IsReference = false; % Update state: mark as resolved - - elseif options.NumLinksToResolve > 0 - % The instance itself is resolved, so spend one unit of - % depth following its links. The remaining depth is - % derived per element rather than by decrementing - % options, which would leak the budget already spent on - % one element into the next. - childOptions = options; - childOptions.NumLinksToResolve = options.NumLinksToResolve - 1; - nvPairs = namedargs2cell(childOptions); - - linkedInstances = obj(i).getLinkedInstances(); - for j = 1:numel(linkedInstances) - linkedInstances{j}.resolve(nvPairs{:}); - end - embeddedInstances = obj(i).getEmbeddedInstances(); - for j = 1:numel(embeddedInstances) - embeddedInstances{j}.resolve(nvPairs{:}); - end - end - % An instance that is already resolved and has no depth left - % to spend needs no work. + % A fresh visitor per element, so the visited registry of + % one element does not stop a shared node from being + % resolved for the next. + visitor = openminds.internal.resolver.ResolvingVisitor(visitorOptions{:}); + resolved{i} = visitor.visit(obj(i)); + end + + resolvedClasses = cellfun(@class, resolved, 'UniformOutput', false); + if isscalar(unique(resolvedClasses)) + instance = [resolved{:}]; + else + % Instances of different types cannot form an object array + instance = resolved; end - instance = obj; % Set output end function str = serialize(obj, options) @@ -661,23 +663,6 @@ end end - methods (Access = private) - function resolver = selectLinkResolver(obj, options) - % selectLinkResolver - Resolver for this instance, from options or registry - - if isfield(options, 'LinkResolver') - resolver = options.LinkResolver; - else - resolver = openminds.internal.getLinkResolver(obj.id); - end - - if isempty(resolver) - error('openMINDS:LinkResolver:NotFound', ... - 'No link resolver found for object with id "%s".', obj.id); - end - end - end - methods (Access = private) % Introspective utility methods function tf = isSubsForProperty(obj, subs) @@ -847,6 +832,13 @@ function assignInstanceId(obj, id) end end + methods (Access = ?openminds.internal.resolver.ResolvingVisitor) + function markResolved(obj) + % markResolved - Record that this node is no longer a reference + obj.IsReference = false; + end + end + methods (Access = ?openminds.internal.mixin.CustomInstanceDisplay) function semanticName = getSemanticName(obj) % Using eval to ensure it also works for empty objects: diff --git a/code/internal/+openminds/+internal/+graph/TraversalCore.m b/code/internal/+openminds/+internal/+graph/TraversalCore.m new file mode 100644 index 00000000..1b4ab3a4 --- /dev/null +++ b/code/internal/+openminds/+internal/+graph/TraversalCore.m @@ -0,0 +1,150 @@ +classdef (Abstract) TraversalCore < handle +% TraversalCore - Shared graph traversal primitives for openMINDS visitors +% +% Provides the machinery every traversal of an instance graph needs: +% enumerating the linked and embedded edges of a node, unwrapping mixed +% type values, writing child values back to a property, and tracking +% which nodes have been seen. +% +% This exists because the same traversal was written three times, in the +% serializer, in Schema.resolve and in loadInstances, each with its own +% handling of mixed types and only one of them with cycle detection. +% +% Subclasses define the traversal protocol on top of these primitives. +% See openminds.abstract.BaseVisitor for the side-effecting protocol and +% openminds.abstract.BaseTransformer for the accumulating one. + + properties (Access = private) + % Identifiers of nodes seen so far. What "seen" means is decided by + % the protocol built on top: a visitor marks a node for the whole + % traversal, a transformer marks it only while its own subtree is + % being processed. + % Created in the constructor rather than as a default value, + % because a handle default would be shared by every instance. + VisitedNodeIds + end + + methods + function obj = TraversalCore() + obj.reset() + end + + function reset(obj) + % reset - Forget which nodes have been seen + % + % Call between independent traversals that share a visitor. + + obj.VisitedNodeIds = containers.Map( ... + 'KeyType', 'char', 'ValueType', 'logical'); + end + end + + methods (Access = protected) + function edges = getLinkedEdges(obj, node) + % getLinkedEdges - Edges of a node that point to linked instances + % + % Returns a struct array with fields PropertyName and Children, + % where Children is a cell array of openMINDS instances. + + edges = obj.getEdges(node, fieldnames(node.LINKED_PROPERTIES)); + end + + function edges = getEmbeddedEdges(obj, node) + % getEmbeddedEdges - Edges of a node that point to embedded instances + + edges = obj.getEdges(node, fieldnames(node.EMBEDDED_PROPERTIES)); + end + + function setEdgeChildren(~, node, propertyName, children) + % setEdgeChildren - Write child instances back to a property + % + % Assigning the property once, with the complete list, is what + % lets a traversal replace a child rather than only mutate it, + % and it keeps the remaining children in their original + % positions. + + arguments + ~ + node (1,1) openminds.abstract.Schema + propertyName (1,1) string + children cell + end + + if isempty(children) + node.(propertyName) = []; + return + end + + % Instances of one type go back as an object array. A mix of + % types has to stay a cell array, which is what the mixed type + % wrapper for the property expects. + childClasses = cellfun(@class, children, 'UniformOutput', false); + if isscalar(unique(childClasses)) + node.(propertyName) = [children{:}]; + else + node.(propertyName) = children; + end + end + + function tf = wasVisited(obj, node) + tf = obj.VisitedNodeIds.isKey(obj.nodeKey(node)); + end + + function markVisited(obj, node) + obj.VisitedNodeIds(obj.nodeKey(node)) = true; + end + + function unmarkVisited(obj, node) + key = obj.nodeKey(node); + if obj.VisitedNodeIds.isKey(key) + obj.VisitedNodeIds.remove(key); + end + end + end + + methods (Access = private) + function edges = getEdges(obj, node, propertyNames) + % getEdges - Collect the non-empty edges for a set of properties + + edges = struct('PropertyName', {}, 'Children', {}); + + for i = 1:numel(propertyNames) + propertyName = string(propertyNames{i}); + children = obj.getChildren(node, propertyName); + if isempty(children) + continue + end + edges(end+1) = struct( ... + 'PropertyName', propertyName, ... + 'Children', {children}); %#ok + end + end + + function children = getChildren(~, node, propertyName) + % getChildren - Property value as a cell array of instances + % + % Mixed type values are containers rather than instances, so the + % instance is taken out of each element. + + children = {}; + value = node.(propertyName); + + if isempty(value) + return + end + + if openminds.utility.isMixedInstance(value) + children = arrayfun(@(element) element.Instance, value, ... + 'UniformOutput', false); + elseif openminds.utility.isInstance(value) + children = num2cell(value); + end + end + end + + methods (Static, Access = private) + function key = nodeKey(node) + key = char(node.id); + end + end +end diff --git a/code/internal/+openminds/+internal/+resolver/AbstractLinkResolver.m b/code/internal/+openminds/+internal/+resolver/AbstractLinkResolver.m index a9a483d5..5e0bca4e 100644 --- a/code/internal/+openminds/+internal/+resolver/AbstractLinkResolver.m +++ b/code/internal/+openminds/+internal/+resolver/AbstractLinkResolver.m @@ -1,28 +1,41 @@ -classdef (Abstract) AbstractLinkResolver < handle & matlab.mixin.Heterogeneous -% AbstractLinkResolver - Abstract pattern for a LinkResolver class +classdef (Abstract) AbstractLinkResolver < handle +% AbstractLinkResolver - Turns a reference node into a populated instance % -% Concrete implementations must implement these methods -% - canResolve - Whether the class can resolve a given IRI -% - resolve - Return an resolved instance given an IRI +% A resolver knows how to fetch the data behind one kind of identifier. +% It does not walk the graph: traversal, link depth and cycle detection +% belong to openminds.internal.resolver.ResolvingVisitor, which selects +% a resolver for each reference it meets. That separation matters +% because one graph can hold references from several sources, and no +% single resolver can handle all of them. +% +% Concrete implementations must provide: +% - IRIPrefix The identifier prefix this resolver handles +% - canResolve Whether it can handle a given identifier +% - resolveNode Fetch or populate a single reference node +% +% RESOLVING IN PLACE OR BY REPLACEMENT: +% ------------------------------------- +% resolveNode returns the resolved instance, and callers must use the +% returned value rather than assuming the argument was modified. +% +% A reference whose type is known can be populated in place, and +% returning it unchanged is correct. A reference whose type is not known +% until it is probed cannot be, because an instance cannot change its +% class: the resolver has to build an instance of the discovered type +% and return that instead. Both are legitimate, and which one applies is +% a property of the reference rather than of the resolver. +% +% See also openminds.internal.resolver.ResolvingVisitor, openminds.registerLinkResolver properties (Constant, Abstract) IRIPrefix (1,1) string end - methods (Static, Abstract) - instance = resolve(IRI, options) - - tf = canResolve(IRI) - end + methods (Abstract) + instance = resolveNode(obj, instance) + % resolveNode - Fetch or populate a single reference node - methods(Sealed) - function tf = eq(obj, resolver) - if isempty(obj) - tf = isempty(resolver); - else - tf = arrayfun(@(x) isequal(x, resolver), obj) | ... - strcmp([obj.IRIPrefix], resolver.IRIPrefix); - end - end + tf = canResolve(obj, IRI) + % canResolve - Whether this resolver handles the given identifier end end diff --git a/code/internal/+openminds/+internal/+resolver/InstanceResolver.m b/code/internal/+openminds/+internal/+resolver/InstanceResolver.m index f37b5309..fa3431d7 100644 --- a/code/internal/+openminds/+internal/+resolver/InstanceResolver.m +++ b/code/internal/+openminds/+internal/+resolver/InstanceResolver.m @@ -1,35 +1,38 @@ classdef InstanceResolver < openminds.internal.resolver.AbstractLinkResolver -% Resolver for openMINDS controlled instances +% InstanceResolver - Resolves openMINDS controlled instances from the local library properties (Constant) IRIPrefix = openminds.constant.BaseURI("v1") + "/instances" % Todo: get from constant end - methods (Static) - function instance = resolve(instance, options) + methods + function instance = resolveNode(~, instance) arguments - instance (1,1) openminds.abstract.Schema % todo: support array - options.NumLinksToResolve = 0 %#ok %TODO + ~ + instance (1,1) openminds.abstract.Schema end + [typeEnum, instanceName] = openminds.utility.parseInstanceIRI(instance.id); instances = openminds.internal.listControlledInstances(typeEnum); + isMatch = instances.InstanceName == string(instanceName); - if any(isMatch) - % Todo: jsonld serializer - data = jsondecode(fileread(instances.Filepath(isMatch))); - else - error(['Could not find data for instance with IRI ', ... - '"%s"'], instance.id) + if ~any(isMatch) + error('openMINDS:LinkResolver:InstanceNotFound', ... + 'Could not find data for instance with IRI "%s"', instance.id) end + + % Todo: use the JSON-LD deserializer once it exists + data = jsondecode(fileread(instances.Filepath(isMatch))); instance.fromStruct(data); end - function tf = canResolve(IRI) + function tf = canResolve(~, IRI) % canResolve - Check whether this resolver can resolve an IRI arguments + ~ IRI (1,:) string end - tf = startsWith(IRI, openminds.constant.BaseURI("v1") + "/instances") || ... + tf = startsWith(IRI, openminds.constant.BaseURI("v1") + "/instances") | ... startsWith(IRI, openminds.constant.BaseURI("v4") + "/instances"); end end diff --git a/code/internal/+openminds/+internal/+resolver/LinkResolverRegistry.m b/code/internal/+openminds/+internal/+resolver/LinkResolverRegistry.m index c921f82f..e7e50625 100644 --- a/code/internal/+openminds/+internal/+resolver/LinkResolverRegistry.m +++ b/code/internal/+openminds/+internal/+resolver/LinkResolverRegistry.m @@ -2,7 +2,10 @@ % LinkResolverRegistry Singleton registry for LinkResolver instances. % properties (SetAccess = private) - LinkResolvers (1,:) {mustBeLinkResolverOrEmpty} + % Resolvers are held in a cell array because they are unrelated + % concrete classes. An object array would require them to share a + % heterogeneous root, which buys nothing here. + LinkResolvers (1,:) cell = {} end methods (Access = private) @@ -20,24 +23,16 @@ function addLinkResolver(obj, resolver) resolver (1,1) {mustBeA(resolver, "openminds.internal.resolver.AbstractLinkResolver")} end - if any(obj.LinkResolvers == resolver) + if obj.hasResolverForPrefix(resolver.IRIPrefix) % Already registered return end - if ~isempty(obj.LinkResolvers) - existingIRIPrefixes = [obj.LinkResolvers.IRIPrefix]; - if ismember(resolver.IRIPrefix, existingIRIPrefixes) - % Already registered - return - end - end + obj.LinkResolvers{end+1} = resolver; + end - if isempty(obj.LinkResolvers) - obj.LinkResolvers = resolver; - else - obj.LinkResolvers(end+1) = resolver; - end + function tf = hasResolverForPrefix(obj, iriPrefix) + tf = any( cellfun(@(r) r.IRIPrefix == iriPrefix, obj.LinkResolvers) ); end function resolver = getLinkResolver(obj, IRI) @@ -49,10 +44,11 @@ function addLinkResolver(obj, resolver) end resolver = []; - for r = obj.LinkResolvers - if r.canResolve(IRI(1)) % Assume all IRIs can be resolved by the same resolver - resolver = r; - obj.promoteResolver(r) + for i = 1:numel(obj.LinkResolvers) + candidate = obj.LinkResolvers{i}; + if candidate.canResolve(IRI(1)) % Assume all IRIs can be resolved by the same resolver + resolver = candidate; + obj.promoteResolver(i) break end end @@ -64,39 +60,31 @@ function addLinkResolver(obj, resolver) end function tf = hasLinkResolver(obj, name) - tf = any( arrayfun(@(x) isa(x, name), obj.LinkResolvers)); + tf = any( cellfun(@(r) isa(r, name), obj.LinkResolvers) ); end function reset(obj) - obj.LinkResolvers = []; + obj.LinkResolvers = {}; % Add the default resolver obj.addLinkResolver(openminds.internal.resolver.InstanceResolver()) end end methods (Access = private) - function promoteResolver(obj, resolver) - % moveResolverToFront - Reorder registry so resolver is first. + function promoteResolver(obj, index) + % promoteResolver - Reorder registry so the resolver at index is first arguments obj (1,1) openminds.internal.resolver.LinkResolverRegistry - resolver (1,1) {mustBeA(resolver, "openminds.internal.resolver.AbstractLinkResolver")} + index (1,1) double {mustBePositive, mustBeInteger} end - - % Find resolver index - idx = find(arrayfun(@(x) isequal(x, resolver), obj.LinkResolvers)); - if isempty(idx) - error('LinkResolverRegistry:ResolverNotFound', ... - 'Resolver is not registered in this registry.'); - end - - if idx == 1 + if index == 1 return % Already at front end - - % Reorder: put this resolver first, keep relative order of others - obj.LinkResolvers = [obj.LinkResolvers(idx), ... - obj.LinkResolvers([1:idx-1, idx+1:end])]; + + % Keep the relative order of the remaining resolvers + obj.LinkResolvers = [obj.LinkResolvers(index), ... + obj.LinkResolvers([1:index-1, index+1:end])]; end end @@ -111,18 +99,3 @@ function promoteResolver(obj, resolver) end end end - -function mustBeLinkResolverOrEmpty(value) -% This special validator is necessary for object construction, because it -% is not possible to create an empty object of an abstract class and as we -% want to ensure the values of the LinkResolvers is an implementation of -% the AbstractLinkResolver we also need to allow empty values. - if ~isempty(value) - actualType = arrayfun(@class, value, 'UniformOutput', false); - assert(... - isa(value, "openminds.internal.resolver.AbstractLinkResolver"), ... - 'openMINDS_MATLAB:LinkResolverRegistry:InvalidLinkResolver', ... - ['LinkResolver must be a concrete implementation ', ... - 'AbstractLinkResolver. Got %s instead'], strjoin(actualType, ', ')) - end -end \ No newline at end of file diff --git a/code/internal/+openminds/+internal/+resolver/ResolvingVisitor.m b/code/internal/+openminds/+internal/+resolver/ResolvingVisitor.m new file mode 100644 index 00000000..61684910 --- /dev/null +++ b/code/internal/+openminds/+internal/+resolver/ResolvingVisitor.m @@ -0,0 +1,86 @@ +classdef ResolvingVisitor < openminds.abstract.BaseVisitor +% ResolvingVisitor - Resolves the reference nodes of an instance graph +% +% Walks an instance graph and replaces reference nodes, which carry an +% identifier and nothing else, with populated instances. A resolver is +% selected for each reference from the resolver registry, so one graph +% may hold references from several sources. +% +% Following a link spends one unit of link depth. Descending into an +% embedded instance does not, because an embedded instance is part of +% its parent rather than a separate node. +% +% See also openminds.internal.resolver.AbstractLinkResolver + + properties + % Number of further links to follow. + RemainingLinkDepth (1,1) double {mustBeNonnegative, mustBeInteger} = 0 + + % Resolver to use instead of consulting the registry. Empty means + % a resolver is selected per reference. + LinkResolver = [] + end + + methods + function obj = ResolvingVisitor(options) + arguments + options.RemainingLinkDepth (1,1) double {mustBeNonnegative, mustBeInteger} = 0 + options.LinkResolver = [] + end + obj.RemainingLinkDepth = options.RemainingLinkDepth; + obj.LinkResolver = options.LinkResolver; + end + end + + methods (Access = protected) + function node = onVisitNode(obj, node) + % Resolve the node when it is an unresolved reference. + + if ~node.isUnresolved() + return + end + + resolver = obj.selectResolver(node); + node = resolver.resolveNode(node); + markResolved(node) + end + + function children = doForLinkedEdge(obj, ~, ~, children) + % Following a link costs one unit of depth. + + if obj.RemainingLinkDepth == 0 + return + end + + obj.RemainingLinkDepth = obj.RemainingLinkDepth - 1; + depthCleanup = onCleanup(@() obj.restoreLinkDepth()); + + for i = 1:numel(children) + children{i} = obj.visit(children{i}); + end + end + + function children = doForEmbeddedEdge(obj, ~, ~, children) + % An embedded instance is part of its parent, so descending into it + % does not spend link depth. + + for i = 1:numel(children) + children{i} = obj.visit(children{i}); + end + end + end + + methods (Access = private) + function resolver = selectResolver(obj, node) + if ~isempty(obj.LinkResolver) + resolver = obj.LinkResolver; + return + end + resolver = openminds.internal.getLinkResolver(node.id); + end + + function restoreLinkDepth(obj) + obj.RemainingLinkDepth = obj.RemainingLinkDepth + 1; + end + end +end diff --git a/code/internal/+openminds/+internal/MixedTypeReference.m b/code/internal/+openminds/+internal/MixedTypeReference.m index af2f76f5..e1f2d067 100644 --- a/code/internal/+openminds/+internal/MixedTypeReference.m +++ b/code/internal/+openminds/+internal/MixedTypeReference.m @@ -27,17 +27,6 @@ end end - methods - function instance = resolve(obj, options) - arguments - obj (1,:) openminds.abstract.Schema - options.NumLinksToResolve = 0 - end - resolver = openminds.internal.getLinkResolver([obj.id]); - instance = resolver.resolve(obj, "NumLinksToResolve", options.NumLinksToResolve); - end - end - methods (Hidden, Access = protected) % CustomDisplay - Method implementation function tf = isReference(~) tf = true; diff --git a/tools/tests/+ommtest/+helper/+mock/MockLinkResolver.m b/tools/tests/+ommtest/+helper/+mock/MockLinkResolver.m index 107579a5..a3cfdf5d 100644 --- a/tools/tests/+ommtest/+helper/+mock/MockLinkResolver.m +++ b/tools/tests/+ommtest/+helper/+mock/MockLinkResolver.m @@ -9,13 +9,8 @@ IRIPrefix = "https://mock.io/" end - methods (Static) - function instance = resolve(instance, options) - arguments - instance - options.NumLinksToResolve = 0 %#ok - end - + methods + function instance = resolveNode(~, instance) % Mock implementation - populate instance with fake data if isa(instance, 'openminds.core.Person') % Populate a Person with mock data @@ -36,8 +31,9 @@ % For any other type, just leave as-is (could add more types as needed) end - function tf = canResolve(IRI) + function tf = canResolve(~, IRI) arguments + ~ IRI (1,:) string end % This mock resolver can handle IRIs that start with mock.io diff --git a/tools/tests/+ommtest/+helper/+mock/ReplacingMockLinkResolver.m b/tools/tests/+ommtest/+helper/+mock/ReplacingMockLinkResolver.m new file mode 100644 index 00000000..a400d9b7 --- /dev/null +++ b/tools/tests/+ommtest/+helper/+mock/ReplacingMockLinkResolver.m @@ -0,0 +1,37 @@ +classdef ReplacingMockLinkResolver < openminds.internal.resolver.AbstractLinkResolver +%ReplacingMockLinkResolver Resolver that replaces rather than populates +% +% Mirrors the case where the type behind an identifier is not known +% until it is probed. Such a reference cannot be populated in place, +% because an instance cannot change its class, so the resolver builds an +% instance of the discovered type and returns that instead. + + properties (Constant) + IRIPrefix = "https://replacing.mock/" + end + + properties (SetAccess = private) + % Identifiers this resolver was asked to resolve, in order. + ResolvedIdentifiers (1,:) string = string.empty + end + + methods + function instance = resolveNode(obj, instance) + obj.ResolvedIdentifiers(end+1) = string(instance.id); + + replacement = openminds.core.Person(); + replacement.givenName = "Replaced"; + replacement.familyName = "Instance"; + instance = replacement; + end + + function tf = canResolve(obj, IRI) + arguments + obj %#ok + IRI (1,:) string + end + tf = all(startsWith(IRI, ... + ommtest.helper.mock.ReplacingMockLinkResolver.IRIPrefix)); + end + end +end diff --git a/tools/tests/unitTests/ResolverTest.m b/tools/tests/unitTests/ResolverTest.m index 52a50713..75f12831 100644 --- a/tools/tests/unitTests/ResolverTest.m +++ b/tools/tests/unitTests/ResolverTest.m @@ -18,7 +18,7 @@ function testRegistryInitialization(testCase) registry = openminds.internal.resolver.LinkResolverRegistry.instance(); testCase.verifyNotEmpty(registry.LinkResolvers); - testCase.verifyTrue(isa(registry.LinkResolvers(1), ... + testCase.verifyTrue(isa(registry.LinkResolvers{1}, ... 'openminds.internal.resolver.InstanceResolver')); end @@ -32,7 +32,7 @@ function testRegisterNewResolver(testCase) % Verify it's in the registry registry = openminds.internal.resolver.LinkResolverRegistry.instance(); - testCase.verifyTrue(any(registry.LinkResolvers == mockResolver)); + testCase.verifyTrue(any( cellfun(@(r) r == mockResolver, registry.LinkResolvers) )); end function testGetResolverForValidIRI(testCase) @@ -137,7 +137,7 @@ function testResolverRegistryPromotesUsedResolver(testCase) openminds.internal.getLinkResolver('https://mock.io/test_123'); % Verify mock resolver is now promoted to first position - testCase.verifyEqual(registry.LinkResolvers(1), mockResolver); + testCase.verifyEqual(registry.LinkResolvers{1}, mockResolver); end function testNoDuplicateResolvers(testCase) @@ -267,6 +267,70 @@ function testResolveIsQuiet(testCase) 'resolve should not print to the command window.') end + function testResolverReplacementIsWiredIntoParent(testCase) + % A resolver that cannot populate a reference in place returns a + % new instance. The traversal must put that instance on the parent + % property, or the resolved value is discarded. + + replacingResolver = ommtest.helper.mock.ReplacingMockLinkResolver(); + openminds.registerLinkResolver(replacingResolver); + + reference = openminds.core.Person( ... + 'id', 'https://replacing.mock/unknown_type_1'); + dataset = ResolverTest.createDatasetWithAuthors(reference, "Replacing Dataset"); + + dataset.resolve( ... + 'NumLinksToResolve', ResolverTest.datasetAuthorResolveDepth()); + + testCase.assertNotEmpty(replacingResolver.ResolvedIdentifiers, ... + 'The resolver was never asked to resolve the reference.') + + author = ResolverTest.getDatasetAuthors(dataset); + testCase.verifyEqual(author.givenName, "Replaced", ... + 'The instance returned by the resolver was not stored on the parent.') + end + + function testUnknownTypeReferenceResolvesByReplacement(testCase) + % A reference whose type is not known until it is probed cannot be + % populated in place, so resolving it must return a new instance of + % the discovered type. + + replacingResolver = ommtest.helper.mock.ReplacingMockLinkResolver(); + openminds.registerLinkResolver(replacingResolver); + + reference = openminds.internal.MixedTypeReference( ... + "https://replacing.mock/unknown_type_2"); + + resolved = reference.resolve(); + + testCase.verifyClass(resolved, 'openminds.core.actors.Person') + testCase.verifyEqual(resolved.givenName, "Replaced") + end + + function testResolveTerminatesOnCircularGraph(testCase) + % A circular instance graph must not be traversed forever. Each + % node is visited once, however much link depth is available. + + mockResolver = ommtest.helper.mock.MockLinkResolver(); + openminds.registerLinkResolver(mockResolver); + + firstType = openminds.core.data.ContentType(); + firstType.name = "first/type"; + secondType = openminds.core.data.ContentType(); + secondType.name = "second/type"; + + firstType.isBasedOn = secondType; + secondType.isBasedOn = firstType; + + % A depth far larger than the graph, so termination can only + % come from cycle detection rather than from exhausted depth. + firstType.resolve('NumLinksToResolve', 500); + + testCase.verifyEqual(firstType.name, "first/type", ... + 'The graph should be unchanged by resolving it.') + testCase.verifyEqual(secondType.name, "second/type") + end + function testResolveMultipleLinkedInstances(testCase) % Test resolving a node with multiple linked instances mockResolver = ommtest.helper.mock.MockLinkResolver();