diff --git a/lib/ontologies_linked_data/models/class.rb b/lib/ontologies_linked_data/models/class.rb index 7dec5fad..27e886d1 100644 --- a/lib/ontologies_linked_data/models/class.rb +++ b/lib/ontologies_linked_data/models/class.rb @@ -704,7 +704,14 @@ def append_if_not_there_already(path, r) end def traverse_path_to_root(parents, paths, path_i, tree = false, roots = nil) - return if (tree && parents.length == 0) + # Terminal condition: a class with no parents under the submission's tree + # property is the top of this path, so there is nothing left to append. + # Callers that pass `roots` (path_to_root / #tree) normally stop earlier in + # tree_root? below; callers that do not (the paths_to_root endpoint) rely on + # this guard, because the only other sentinel here is owl:Thing -- which + # owlapi asserts on top-level OWL classes but which never appears under + # skos:broader (SKOS) or metadata:treeView (OBO). + return if parents.nil? || parents.empty? recursions = [path_i] recurse_on_path = [false] @@ -737,13 +744,14 @@ def traverse_path_to_root(parents, paths, path_i, tree = false, roots = nil) end if !p.loaded_attributes.include?(:parents) - # fail safely + # fail safely -- abandon this path only, the sibling paths in this + # frame are unaffected and must still be traversed logger = LinkedData::Parser.logger || Logger.new($stderr) logger.error("Class #{p.id.to_s} from #{p.submission.id} cannot load parents") - return + next end - traverse_path_to_root(p.parents.dup, paths, rec_i, tree=tree, roots=roots) + traverse_path_to_root((p.parents || []).dup, paths, rec_i, tree=tree, roots=roots) end end end diff --git a/test/models/test_class.rb b/test/models/test_class.rb index 8a3e4129..3efbe210 100644 --- a/test/models/test_class.rb +++ b/test/models/test_class.rb @@ -454,4 +454,71 @@ def test_xml_literal_serialization assert_equal String, xml_comment.class assert_equal comment, xml_comment end + + # Companion to test_bro_paths_to_root, for the OBO tree property. BRO cannot + # cover this: owlapi asserts `rdfs:subClassOf owl:Thing` on top-level OWL + # classes, and the traversal treats owl:Thing as a stop sentinel. OBO + # submissions walk metadata:treeView, where no such sentinel exists, so the + # traversal has to terminate on a genuinely parentless ancestor instead. + # Regressing that recurses into an empty parents array and raises + # "undefined method `id' for nil:NilClass" in append_if_not_there_already. + def test_obo_paths_to_root_terminates_at_parentless_class + if !LinkedData::Models::Ontology.find("HPPATHSTEST").first + submission_parse("HPPATHSTEST", "HP paths_to_root test", "./test/data/ontology_files/hp.obo", 124, + process_rdf: true, index_search: false, + run_metrics: false) + end + os = LinkedData::Models::Ontology.find("HPPATHSTEST").first.latest_submission(status: [:rdf]) + + assert_equal 'http://data.bioontology.org/metadata/treeView', + LinkedData::Models::Class.tree_view_property(os).to_s, + 'expected the OBO tree property, otherwise this duplicates the OWL test' + + # Find a class with a parent that itself has no parents -- one hop is enough + # to make the traversal walk into the parentless ancestor. Candidates are + # sorted so the same class is picked on every backend (row order is not + # guaranteed and differs between 4store/AG/GraphDB/Virtuoso). + candidates = [] + LinkedData::Models::Class.in(os).include(parents: [:prefLabel]).page(1, 500).each do |cls| + next if cls.parents.nil? || cls.parents.empty? + + cls.parents.each do |parent| + # owl:Thing has no parents either, but the traversal stops on it via the + # sentinel, so it does not exercise the empty-parents branch. + next if parent.id.to_s['#Thing'] + + parent.bring(:parents) if parent.bring?(:parents) + next unless parent.parents.nil? || parent.parents.empty? + + candidates << cls.id.to_s + break + end + end + target_id = candidates.sort.first + refute_nil target_id, 'expected an OBO class whose parent is parentless' + target = LinkedData::Models::Class.find(RDF::URI.new(target_id)).in(os).first + + paths = target.paths_to_root + refute_empty paths, 'expected at least one path to root' + + saw_parentless_terminal = false + paths.each do |path| + refute_includes path, nil, 'path contains a nil node' + assert_equal target.id.to_s, path.last.id.to_s, 'path must end at the requested class' + + root = path.first + root.bring(:parents) if root.bring?(:parents) + root_parents = root.parents || [] + saw_parentless_terminal ||= root_parents.empty? + + # A path may also stop on the owl:Thing sentinel, which owlapi asserts on + # top-level classes; what must hold is that nothing traversable is left. + assert_empty root_parents.reject { |p| p.id.to_s['#Thing'] }, + "path must terminate where no parent is left to follow, got #{root.id}" + end + + assert saw_parentless_terminal, + 'expected at least one path to terminate at a class with no parents at all -- ' \ + 'that is the case that used to raise' + end end diff --git a/test/models/test_skos_submission.rb b/test/models/test_skos_submission.rb index 601c9565..3b676c94 100644 --- a/test/models/test_skos_submission.rb +++ b/test/models/test_skos_submission.rb @@ -217,6 +217,54 @@ def test_skos_class_tree assert seen_target, 'target class not found within its own tree' end + # Guards the terminal condition of the paths_to_root traversal. The traversal + # must stop when it reaches a class with no parents under the submission's tree + # property; it cannot rely on an owl:Thing sentinel, which never appears under + # skos:broader (or metadata:treeView for OBO), nor on `roots` -- the + # /classes/:cls/paths_to_root endpoint passes none. Regressing this recurses + # into an empty parents array and raises + # "undefined method `id' for nil:NilClass" in append_if_not_there_already. + def test_skos_paths_to_root_terminates_at_parentless_concept + sub = before_suite + roots = sub.roots + refute_empty roots, 'expected SKOS roots' + + # Descend one level from a root so the path is non-trivial (root -> target). + # Ids are sorted so the same concept is picked on every backend -- row order + # is not guaranteed and differs between 4store/AG/GraphDB/Virtuoso. + child_ids = [] + roots.sort_by { |r| r.id.to_s }.each do |r| + LinkedData::Models::Class.in(sub).models([r]).include(children: [:prefLabel]).all + child_ids += r.children.map { |c| c.id.to_s } + end + target_id = child_ids.sort.first + refute_nil target_id, 'expected a SKOS root with at least one child' + target = LinkedData::Models::Class.find(RDF::URI.new(target_id)).in(sub).first + + paths = target.paths_to_root + refute_empty paths, 'expected at least one path to root' + + saw_parentless_terminal = false + paths.each do |path| + refute_includes path, nil, 'path contains a nil node' + assert_equal target.id.to_s, path.last.id.to_s, 'path must end at the requested class' + + root = path.first + root.bring(:parents) if root.bring?(:parents) + root_parents = root.parents || [] + saw_parentless_terminal ||= root_parents.empty? + + # skos:broader has no owl:Thing sentinel, so the only way to stop is to run + # out of parents; the reject keeps this symmetric with the OBO guard. + assert_empty root_parents.reject { |p| p.id.to_s['#Thing'] }, + "path must terminate where no parent is left to follow, got #{root.id}" + end + + assert saw_parentless_terminal, + 'expected at least one path to terminate at a concept with no broader -- ' \ + 'that is the case that used to raise' + end + # N+1 guard for the class-tree endpoint (SKOS). Same contract as the OWL guard # in test_class.rb: serializing a tree must resolve the submission's languages # once, not once per node (get_languages `bring?` guard,