Fix nil crash in paths_to_root at parentless ancestors

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

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

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,