feat: keep reified definition nodes in the graph, filter them on read

The step no longer removes anything. Unlinking a node once its text had been
read left the graph diverging from the source RDF for the sake of a
presentation problem, so it is reverted: like every other step of the
pipeline this one only appends, and the reified node keeps both its link and
its provenance.

The URI is instead kept out of Class#definition. skos:definition carries
whatever object the graph puts on it, so a reified node arrives as a bare URI
that no reader can use; the attribute now hands out only what is readable,
while `properties` still reports the raw predicate, node URI included.

A resource is only taken for a reified node when it lives in the concept's
own URI space. An external reference - a Wikipedia page, a DOI, a term in
another vocabulary - is a definition the ontology means to give and comes
through untouched. The namespace is taken from the concept rather than from
the submission because this runs inside the reader, once per class per page,
and reaching for the submission there would lazy load it.

Two details the implementation depends on. The reader is wrapped with
alias_method, not redefined: Goo generates it with define_method on the class
itself, so a bare `def definition` would replace it and silently drop the
language flattening and the AttributeNotLoaded guard. And the filter is
applied again in index_doc, because it builds from to_hash, which reads the
instance variables straight and bypasses every reader - without it a node URI
would still be indexed as the text of the definition.

Signed-off-by: BOUKERFA Mohamed El Amine <boukerfa.ma@gmail.com>

Author: maboukerfa

lib/ontologies_linked_data/models/class.rb

@@ -196,6 +196,60 @@ def obsolete
         @obsolete || false
       end
 
+      # skos:definition carries whatever object the graph puts on it, and
+      # thesauri that reify their definitions put the URI of a note node there
+      # instead of the text. That URI is not a definition anyone can read, so it
+      # is kept out of the attribute.
+      #
+      # Nothing is hidden from the graph: ResolveReifiedDefinitions asserts the
+      # text of those nodes as literals beside the URI without removing
+      # anything, and `properties` still reports the raw skos:definition
+      # objects, node URI included.
+      alias_method :definition_with_reified_nodes, :definition
+
+      def definition(*args)
+        self.class.reject_reified_nodes(definition_with_reified_nodes(*args),
+                                        self.class.uri_namespace(id))
+      end
+
+      # The reader hands back an Array, or a { language => values } Hash when
+      # languages are asked for - which is how the serializer calls it - or a
+      # lone value. All three shapes have to survive the filter, and a language
+      # left with nothing but a node URI drops out entirely.
+      def self.reject_reified_nodes(value, namespace)
+        case value
+        when Hash
+          value.transform_values { |values| reject_reified_nodes(values, namespace) }
+               .reject { |_, values| values.nil? || (values.respond_to?(:empty?) && values.empty?) }
+        when Array
+          value.reject { |v| reified_node?(v, namespace) }
+        else
+          reified_node?(value, namespace) ? nil : value
+        end
+      end
+
+      # A resource-valued definition is only taken for a reified node when it
+      # lives in the concept's own URI space. An external reference - a
+      # Wikipedia page, a DOI, a term in another vocabulary - is a definition
+      # the ontology means to give, and is left exactly as it is.
+      def self.reified_node?(value, namespace)
+        return false unless value.is_a?(RDF::Resource)
+        return true if value.is_a?(RDF::Node) # a blank node is internal by construction
+
+        !namespace.empty? && value.to_s.start_with?(namespace)
+      end
+
+      # Everything up to and including a URI's last separator, the way
+      # LinkedData::Utils::Triples.last_iri_fragment splits it. Taken from the
+      # concept rather than from the submission on purpose: this runs inside the
+      # reader, for every class of every page, and reaching for the submission
+      # there would lazy load it.
+      def self.uri_namespace(uri)
+        uri = uri.to_s
+        separator = uri.index('#') ? uri.rindex('#') : uri.rindex('/')
+        separator ? uri[0..separator] : ''
+      end
+
       def index_id()
         self.bring(:submission) if self.bring?(:submission)
         return nil unless self.submission
@@ -273,6 +327,13 @@ def index_doc(to_set=nil)
           doc[:obsolete] = self.obsolete.to_s
 
           all_attrs = self.to_hash
+          # to_hash reads the instance variables straight, so it bypasses the
+          # reader that drops reified definition nodes - filter here too, or a
+          # node URI gets indexed as the text of the definition.
+          if all_attrs.key?(:definition)
+            all_attrs[:definition] = self.class.reject_reified_nodes(all_attrs[:definition],
+                                                                     self.class.uri_namespace(id))
+          end
           std = [:id, :prefLabel, :notation, :synonym, :definition, :cui]
           multi_language_fields = [:prefLabel, :synonym, :definition]
           std.each do |att|

lib/ontologies_linked_data/services/submission_process/operations/submission_reified_definitions.rb

@@ -21,12 +21,11 @@ module Services
     # asserts the text back on the concept as a skos:definition literal, keeping
     # the language tag.
     #
-    # Once a node is read, the triple pointing at it is dropped, so the concept
-    # is left with its definition as text instead of a URI nothing can resolve.
-    # This is the one step of the pipeline that removes a source triple, and it
-    # removes only what it has replaced: a node whose text could not be read
-    # keeps its link, and the node's own triples are never touched - the
-    # provenance stays in the graph, reachable by URI.
+    # Nothing is removed: like every other step of the pipeline this one only
+    # appends, so the submission graph stays a faithful copy of the source RDF
+    # and the reified node keeps both its link and its provenance. A concept
+    # therefore carries two definition objects afterwards - the node URI it
+    # always had, and the text read out of it.
     class ResolveReifiedDefinitions < OntologySubmissionProcess
 
       # Concepts resolved per round trip to the triple store.
@@ -139,7 +138,7 @@ def process(logger, options = {})
       def resolve_reified_definitions(logger, file_path)
         properties = definition_properties
         save_in_file = file_path.nil? ? nil : File.join(File.dirname(file_path), 'definitions.ttl')
-        stats = { scanned: 0, asserted: 0, unlinked: 0, unresolved: 0 }
+        stats = { scanned: 0, asserted: 0, unresolved: 0 }
         fsave = nil
         after = nil
 
@@ -148,7 +147,7 @@ def resolve_reified_definitions(logger, file_path)
             concepts = reified_definition_concepts(properties, after)
             break if concepts.empty?
 
-            triples, resolved_links = resolve_concepts(logger, properties, concepts, stats)
+            triples = resolve_concepts(logger, properties, concepts, stats)
             stats[:scanned] += concepts.length
 
             unless triples.empty?
@@ -159,11 +158,6 @@ def resolve_reified_definitions(logger, file_path)
               stats[:asserted] += triples.length
             end
 
-            unless resolved_links.empty?
-              Goo.sparql_update_client.delete_data(resolved_links, graph: @submission.id)
-              stats[:unlinked] += resolved_links.length
-            end
-
             after = concepts.last
             break if concepts.length < PAGE_SIZE
           end
@@ -179,7 +173,6 @@ def report(logger, stats, save_in_file, time)
         else
           logger.info("Resolved #{stats[:asserted]} definitions out of the reified nodes of " \
                       "#{stats[:scanned]} concepts in #{time.round(2)} sec.")
-          logger.info("Dropped #{stats[:unlinked]} triples pointing at a node that has been read")
           logger.info("Saved resolved definitions in #{save_in_file}") if save_in_file && stats[:asserted].positive?
         end
 
@@ -205,10 +198,10 @@ def definition_properties
       end
 
       # One page of concepts whose definition is a resource instead of a literal,
-      # starting after the last concept of the previous page. Paging on the
-      # cursor rather than on an offset is what makes it safe to unlink resolved
-      # nodes as we go: the step shrinks the very set it is walking, and an
-      # OFFSET would skip over as many concepts as it just resolved.
+      # starting after the last concept of the previous page. A cursor rather
+      # than an OFFSET: the literals this step asserts never match the pattern
+      # below, but paging that survives a shrinking match set costs nothing here
+      # and does not have to be revisited if that ever stops being true.
       def reified_definition_concepts(properties, after)
         filters = ['!isLiteral(?node)']
         # IRIs order by their string value, so the cursor can compare on str().
@@ -231,16 +224,12 @@ def reified_definition_concepts(properties, after)
         concepts
       end
 
-      # Returns the definition triples to assert and the reified links they
-      # replace, which the caller drops from the graph.
       def resolve_concepts(logger, properties, concepts, stats)
         values = concepts.map(&:to_ntriples).join(' ')
         existing = existing_definitions(properties, values)
-        nodes, links = definition_nodes(properties, values)
         triples = []
-        resolved_links = []
 
-        nodes.each do |concept, concept_nodes|
+        definition_nodes(properties, values).each do |concept, concept_nodes|
           concept_nodes.each do |node, predicates|
             literals = definition_text(predicates)
 
@@ -259,23 +248,18 @@ def resolve_concepts(logger, properties, concepts, stats)
                                                            Goo.vocabulary(:skos)[:definition],
                                                            definition_literal(literal))
             end
-
-            # Read, so the concept no longer needs to point at it.
-            resolved_links.concat(links[[concept, node]].to_a)
           end
         end
 
-        [triples, resolved_links]
+        triples
       end
 
       # The literals hanging off the reified nodes of a page of concepts, as
-      # { concept => { node => { predicate => [literal] } } }, along with the
-      # triples that link each concept to each of its nodes, as
-      # { [concept, node] => #<Set: statements> }. Nodes carrying no literal at
-      # all are kept (empty), they are the ones to report.
+      # { concept => { node => { predicate => [literal] } } }. Nodes carrying no
+      # literal at all are kept (empty), they are the ones to report.
       def definition_nodes(properties, values)
         query = <<~SPARQL
-          SELECT ?concept ?definitionProperty ?node ?p ?o
+          SELECT ?concept ?node ?p ?o
           FROM #{@submission.id.to_ntriples}
           WHERE {
             VALUES ?concept { #{values} }
@@ -290,23 +274,13 @@ def definition_nodes(properties, values)
         SPARQL
 
         nodes = Hash.new { |h, k| h[k] = Hash.new { |h2, k2| h2[k2] = Hash.new { |h3, k3| h3[k3] = [] } } }
-        links = Hash.new { |h, k| h[k] = Set.new }
         Goo.sparql_query_client.query(query).each_solution do |sol|
           predicates = nodes[sol[:concept].to_s][sol[:node].to_s]
-          # The same concept can reach one node through more than one definition
-          # property, and each of those links has to go once the node is read.
-          # Only IRI nodes: DELETE DATA cannot name a blank node, so a blank
-          # reified definition gets its text read but keeps its link.
-          if sol[:node].is_a?(RDF::URI)
-            links[[sol[:concept].to_s, sol[:node].to_s]] << RDF::Statement.new(sol[:concept],
-                                                                              sol[:definitionProperty],
-                                                                              sol[:node])
-          end
           next if sol[:p].nil?
 
           predicates[sol[:p].to_s] << sol[:o] if text_literal?(sol[:o])
         end
-        [nodes, links]
+        nodes
       end
 
       # The definitions a page of concepts already holds as literals, so nothing

test/models/skos/test_resolve_reified_definitions.rb

@@ -90,10 +90,55 @@ def test_literal_handling
     assert_equal RDF::XSD.string, exotic.datatype
   end
 
+  # Backend-free: the reader-side filter that keeps node URIs out of the
+  # definition attribute, across the shapes the Goo reader can return - an
+  # Array, or a { language => values } Hash when languages are asked for, which
+  # is how the serializer calls it.
+  def test_reject_reified_nodes
+    cls = LinkedData::Models::Class
+    ns = cls.uri_namespace(REIFIED_CONCEPT)
+    node = RDF::URI.new(REIFIED_NODE)
+
+    assert_equal 'http://opendata.inrae.fr/thesaurusINRAE/', ns
+    assert_equal 'http://ex.org/onto#', cls.uri_namespace('http://ex.org/onto#c_1')
+
+    assert_equal ['texte'], cls.reject_reified_nodes([node, 'texte'], ns)
+    assert_empty cls.reject_reified_nodes([node], ns)
+    assert_empty cls.reject_reified_nodes([RDF::Node.new('b1')], ns), 'blank nodes go too'
+    assert_equal({ fr: ['texte'] }, cls.reject_reified_nodes({ fr: [node, 'texte'] }, ns))
+    assert_equal({ en: ['text'] }, cls.reject_reified_nodes({ fr: [node], en: ['text'] }, ns),
+                 'a language left with nothing readable drops out')
+    assert_nil cls.reject_reified_nodes(node, ns)
+    assert_equal 'texte', cls.reject_reified_nodes('texte', ns)
+
+    untouched = [node, 'texte']
+    cls.reject_reified_nodes(untouched, ns)
+    assert_equal [node, 'texte'], untouched, 'the filter must not mutate what it is given'
+  end
+
+  # A definition that points outside the concept's URI space is a reference the
+  # ontology means to give, not a reified node, and must come through untouched.
+  def test_external_definition_links_are_kept
+    cls = LinkedData::Models::Class
+    ns = cls.uri_namespace(REIFIED_CONCEPT)
+    node = RDF::URI.new(REIFIED_NODE)
+    wikipedia = RDF::URI.new('https://en.wikipedia.org/wiki/Crop_establishment')
+    doi = RDF::URI.new('https://doi.org/10.15454/QHFTMX')
+
+    assert_equal [wikipedia], cls.reject_reified_nodes([wikipedia], ns)
+    assert_equal [doi, 'texte'], cls.reject_reified_nodes([doi, 'texte'], ns)
+    assert_equal [wikipedia, 'texte'], cls.reject_reified_nodes([node, wikipedia, 'texte'], ns)
+    assert_equal({ '@none' => [wikipedia], fr: ['texte'] },
+                 cls.reject_reified_nodes({ '@none' => [wikipedia], fr: ['texte'] }, ns),
+                 'an untagged external link keeps its bucket')
+    assert_equal [node], cls.reject_reified_nodes([node], ''),
+                 'with no namespace to compare against, nothing is taken for a node'
+  end
+
   # The whole step against the triple store: a reified definition becomes a
-  # literal on the concept, the link to the node it replaces is dropped, the
-  # provenance is not mistaken for the text, and replaying the step does not
-  # duplicate anything.
+  # literal on the concept, nothing is removed from the graph, the provenance is
+  # not mistaken for the text, and replaying the step does not duplicate
+  # anything.
   def test_resolve_reified_definitions
     sub = submission
     sub.bring_remaining # the step writes its triples next to the master file
@@ -109,20 +154,23 @@ def test_resolve_reified_definitions
     assert_equal :fr, definition.language, 'the language of the reified text is kept'
     refute definition.value.start_with?(EXPECTED_SOURCE_START), 'dcterms:source is not the definition'
 
-    # The link to the node that has been read is gone: the text is the only
-    # definition of the concept now.
-    assert_equal 1, definition_objects(sub, REIFIED_CONCEPT).length
-    refute_includes definition_objects(sub, REIFIED_CONCEPT).map(&:to_s), REIFIED_NODE
-
-    # Only the link is dropped, the node keeps its own triples.
-    assert node_still_described?(sub, REIFIED_NODE),
-           'the provenance node stays in the graph, reachable by URI'
+    # The step only appends: the reified triple and the node it points at are
+    # both still there.
+    assert_includes definition_objects(sub, REIFIED_CONCEPT).map(&:to_s), REIFIED_NODE
+    assert node_still_described?(sub, REIFIED_NODE)
 
+    # ... but the attribute only hands out what is readable.
     RequestStore.store[:requested_lang] = :FR
     cls = LinkedData::Models::Class.find(REIFIED_CONCEPT).in(sub).include(:definition).first
     assert_equal 1, cls.definition.length
     assert cls.definition.first.to_s.start_with?(EXPECTED_TEXT_START),
            'the concept exposes its definition as text, not as a URI'
+    refute_includes cls.definition.map(&:to_s), REIFIED_NODE
+
+    # The raw predicate is still reachable, node URI included.
+    raw = LinkedData::Models::Class.find(REIFIED_CONCEPT).in(sub).include(:unmapped).first.properties
+    assert_includes raw[RDF::URI.new(SKOS_DEFINITION)].map(&:to_s), REIFIED_NODE,
+                    'properties reports the graph as it is'
 
     # Replayed on an already enriched graph, the step adds nothing.
     sub.resolve_reified_definitions(Logger.new(TestLogFile.new))