Skip to content

fix: correct array handling in Schema.resolve #106

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ehennestad wants to merge 1 commit into from
+102 −31
Open

fix: correct array handling in Schema.resolve #106

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
72 changes: 41 additions & 31 deletions code/internal/+openminds/+abstract/Schema.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,41 +117,34 @@
% options.IsEmbedded = false - Todo?
end

instance = obj; % Initialize output
for i = 1:numel(obj)
if ~obj(i).IsReference % Instance is resolved (not a reference)
if options.NumLinksToResolve == 0
fprintf('Instance is already resolved.\n')
instance(i) = obj(i);
return
else
options.NumLinksToResolve = options.NumLinksToResolve-1;
nvPairs = namedargs2cell(options);
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
else
if isfield(options, 'LinkResolver')
resolver = options.LinkResolver;
else
resolver = openminds.internal.getLinkResolver([obj(i).id]);
end
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

if isempty(resolver)
error(...
'openMINDS:LinkResolver:NotFound', ...
'No link resolver found for object with id "%s".', obj(i).id);
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

obj(i) = resolver.resolve(obj(i), "NumLinksToResolve", options.NumLinksToResolve);
obj(i).IsReference = false; % Update state: mark as resolved
end
% An instance that is already resolved and has no depth left
% to spend needs no work.
end
instance = obj; % Set output
end
Expand Down Expand Up @@ -668,6 +661,23 @@
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)
Expand Down
61 changes: 61 additions & 0 deletions tools/tests/unitTests/ResolverTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,67 @@ function testResolveWithNumLinksToResolve(testCase)
testCase.verifyEqual(authors.givenName, "Mock");
end

function testResolveArrayContinuesPastResolvedElement(testCase)
% Every element of an array must be considered. An element that is
% already resolved must not stop the loop, or references later in
% the array are silently left unresolved.

mockResolver = ommtest.helper.mock.MockLinkResolver();
openminds.registerLinkResolver(mockResolver);

resolvedPerson = openminds.core.Person();
resolvedPerson.givenName = "Already";

referencePerson = openminds.core.Person('id', 'https://mock.io/person_after');

instances = [resolvedPerson, referencePerson];
instances.resolve();

testCase.verifyEqual(instances(1).givenName, "Already", ...
'The resolved element should be left alone.')
testCase.verifyEqual(instances(2).givenName, "Mock", ...
'The reference after a resolved element should still be resolved.')
end

function testResolveDepthIsPerArrayElement(testCase)
% The link depth is a budget for each element of the array, not a
% budget shared across the whole array. Resolving the links of one
% element must not exhaust the depth available to the next.

mockResolver = ommtest.helper.mock.MockLinkResolver();
openminds.registerLinkResolver(mockResolver);

firstDataset = ResolverTest.createDatasetWithAuthors( ...
openminds.core.Person('id', 'https://mock.io/author_first'), ...
"First Dataset");
secondDataset = ResolverTest.createDatasetWithAuthors( ...
openminds.core.Person('id', 'https://mock.io/author_second'), ...
"Second Dataset");

datasets = [firstDataset, secondDataset];
datasets.resolve( ...
'NumLinksToResolve', ResolverTest.datasetAuthorResolveDepth());

firstAuthor = ResolverTest.getDatasetAuthors(datasets(1));
secondAuthor = ResolverTest.getDatasetAuthors(datasets(2));

testCase.verifyEqual(firstAuthor.givenName, "Mock")
testCase.verifyEqual(secondAuthor.givenName, "Mock", ...
'The second element should get the same depth budget as the first.')
end

function testResolveIsQuiet(testCase)
% Resolving an instance that is already resolved is a no-op and must
% not write to the command window.

resolvedPerson = openminds.core.Person();
resolvedPerson.givenName = "Already";

output = evalc('resolvedPerson.resolve();');
testCase.verifyEmpty(strtrim(output), ...
'resolve should not print to the command window.')
end

function testResolveMultipleLinkedInstances(testCase)
% Test resolving a node with multiple linked instances
mockResolver = ommtest.helper.mock.MockLinkResolver();
Expand Down
Loading