diff --git a/lib/ontologies_linked_data/concerns/concepts/concept_in_scheme.rb b/lib/ontologies_linked_data/concerns/concepts/concept_in_scheme.rb index ba3592d2..12a19fa8 100644 --- a/lib/ontologies_linked_data/concerns/concepts/concept_in_scheme.rb +++ b/lib/ontologies_linked_data/concerns/concepts/concept_in_scheme.rb @@ -16,7 +16,13 @@ def inScheme?(scheme) def load_is_in_scheme(schemes = []) included = schemes.select { |s| inScheme?(s) } - included = [self.submission.get_main_concept_scheme] if included.empty? && schemes&.empty? + # isInActiveScheme is a SKOS-only concept and is only serialized for + # SKOS submissions. Skip the main-concept-scheme lookup for non-SKOS + # (OWL/OBO) submissions, where it would run a Scheme query per node + # that always returns nothing and is then discarded. + if included.empty? && schemes&.empty? && submission.skos? + included = [self.submission.get_main_concept_scheme] + end @isInActiveScheme = included end diff --git a/lib/ontologies_linked_data/concerns/ontology_submissions/skos/skos_submission_schemes.rb b/lib/ontologies_linked_data/concerns/ontology_submissions/skos/skos_submission_schemes.rb index b275d4cd..7dc0f148 100644 --- a/lib/ontologies_linked_data/concerns/ontology_submissions/skos/skos_submission_schemes.rb +++ b/lib/ontologies_linked_data/concerns/ontology_submissions/skos/skos_submission_schemes.rb @@ -10,8 +10,12 @@ def get_main_concept_scheme(default_return: ontology_uri) end end + # Memoized: the tree/paths endpoints resolve isInActiveScheme once per + # node, so without this the same SKOS::Scheme query fires ~N times per + # request (one triplestore miss + N-1 Redis cache hits). Cache the + # result on the submission instance for the life of the request. def all_concepts_schemes - LinkedData::Models::SKOS::Scheme.in(self).all + @all_concepts_schemes ||= LinkedData::Models::SKOS::Scheme.in(self).all end end end diff --git a/lib/ontologies_linked_data/serializers/json.rb b/lib/ontologies_linked_data/serializers/json.rb index 187b460d..e83720f5 100644 --- a/lib/ontologies_linked_data/serializers/json.rb +++ b/lib/ontologies_linked_data/serializers/json.rb @@ -96,7 +96,10 @@ def self.get_languages(submission, user_languages) result_lang = user_languages if submission - submission.bring :naturalLanguage + # Guard the load: goo's #bring re-queries every call, and this runs once + # per serialized object. Without the guard, a response of N objects sharing + # a submission issues N naturalLanguage queries instead of 1. + submission.bring(:naturalLanguage) if submission.bring?(:naturalLanguage) languages = get_submission_languages(submission.naturalLanguage) # intersection of the two arrays , if the requested language is not :all result_lang = user_languages == :all ? languages : Array(user_languages) & languages diff --git a/test/models/test_class.rb b/test/models/test_class.rb index dd6dcf64..6029d8a6 100644 --- a/test/models/test_class.rb +++ b/test/models/test_class.rb @@ -296,6 +296,46 @@ def test_bro_tree_has_children assert checked > 0, "expected at least one node with a child-count aggregate" end + # N+1 guard for the class-tree endpoint (OWL). Serializing a tree must resolve + # the submission's languages once, not once per node -- before the get_languages + # `bring?` guard (ncbo/ontologies_linked_data#302) serialization issued ~one + # naturalLanguage query per serialized node. Asserts serialization query + # fan-out stays sub-linear in node count so the N+1 cannot silently return. + def test_bro_tree_serialize_no_n_plus_1 + if !LinkedData::Models::Ontology.find("BROTEST123").first + submission_parse("BROTEST123", "SOME BROTEST Bla", "./test/data/ontology_files/BRO_v3.2.owl", 123, + process_rdf: true, index_search: false, + run_metrics: false, reasoning: true) + end + os = LinkedData::Models::Ontology.find("BROTEST123").first.latest_submission(status: [:rdf]) + # The hypermedia links serialized per node need submission.ontology.acronym, + # exactly as the controller has it loaded; preload so serialization runs. + os.bring(:ontology) if os.bring?(:ontology) + os.ontology.bring(:acronym) if os.ontology.bring?(:acronym) + + statistical_Text_Analysis = "http://bioontology.org/ontologies/BiomedicalResourceOntology.owl#Statistical_Text_Analysis" + display_attrs = [:prefLabel, :hasChildren, :children, :obsolete, :subClassOf] + cls = LinkedData::Models::Class.find(RDF::URI.new(statistical_Text_Analysis)).in(os).include(display_attrs).first + tree_root = cls.tree + + node_count = 0 + stack = [tree_root] + until stack.empty? + n = stack.pop + node_count += 1 + stack.concat(n.children) + end + assert_operator node_count, :>=, 3, "tree too small to be a meaningful N+1 guard" + + serialize_queries = count_sparql_queries do + LinkedData::Serializers::JSON.serialize([tree_root], only: display_attrs) + end + + assert_operator serialize_queries, :<, node_count, + "serialization issued #{serialize_queries} SPARQL queries for #{node_count} tree nodes -- " \ + "looks like a per-node N+1 (see get_languages guard in ncbo/ontologies_linked_data#302)" + end + def test_include_ancestors if !LinkedData::Models::Ontology.find("BROTEST123").first diff --git a/test/models/test_ontology_common.rb b/test/models/test_ontology_common.rb index 431a4ba4..1bcaf4cd 100644 --- a/test/models/test_ontology_common.rb +++ b/test/models/test_ontology_common.rb @@ -1,9 +1,38 @@ require_relative "../test_case" require 'rack' +# STOPGAP query counter for N+1 regression guards. +# +# TODO(de-fork): replace with Goo::TestHelpers.assert_sparql_queries once the +# de-fork SPARQL query-counting framework lands on the goo branch this app +# tracks (ncbo/goo `development`). That helper does not exist there yet, so we +# count Goo::SPARQL::Client#query invocations directly. Inert unless a counting +# block armed the thread-local, so it adds nothing in normal runs. +module GooQueryCountSpy + def query(*args, **kwargs, &blk) + c = Thread.current[:test_goo_query_count] + Thread.current[:test_goo_query_count] = c + 1 unless c.nil? + super + end +end +unless Goo::SPARQL::Client.ancestors.include?(GooQueryCountSpy) + Goo::SPARQL::Client.prepend(GooQueryCountSpy) +end + module LinkedData class TestOntologyCommon < LinkedData::TestCase + # Count Goo SPARQL queries (cache hits + store round-trips) issued by the + # block. Counts query fan-out -- what an N+1 inflates -- independent of + # cache state. See GooQueryCountSpy above. + def count_sparql_queries + Thread.current[:test_goo_query_count] = 0 + yield + Thread.current[:test_goo_query_count] + ensure + Thread.current[:test_goo_query_count] = nil + end + def create_count_mapping count = LinkedData::Models::MappingCount.where.all.length unless count > 2 diff --git a/test/models/test_skos_submission.rb b/test/models/test_skos_submission.rb index 5ea94663..3a445419 100644 --- a/test/models/test_skos_submission.rb +++ b/test/models/test_skos_submission.rb @@ -182,5 +182,46 @@ def test_skos_class_tree assert seen_target, 'target class not found within its own tree' 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, + # ncbo/ontologies_linked_data#302). Asserts serialization query fan-out stays + # sub-linear in node count. + def test_skos_tree_serialize_no_n_plus_1 + sub = before_suite + sub.bring(:ontology) if sub.bring?(:ontology) + sub.ontology.bring(:acronym) if sub.ontology.bring?(:acronym) + + roots = sub.roots + roots.each { |r| LinkedData::Models::Class.in(sub).models([r]).include(children: [:prefLabel]).all } + # Pick the bushiest root so the target's tree (root + its siblings) has enough + # nodes to make the query-count guard meaningful on this small fixture. + root = roots.reject { |r| r.children.empty? }.max_by { |r| r.children.length } + refute_nil root, 'expected a SKOS root with children' + target = LinkedData::Models::Class.find(root.children.first.id).in(sub).first + refute_nil target, 'expected a SKOS root with at least one child' + + display_attrs = [:prefLabel, :hasChildren, :children, :obsolete, :subClassOf] + + LinkedData::Models::Class.concept_is_in_attributes + tree_root = target.tree + + node_count = 0 + stack = [tree_root] + until stack.empty? + n = stack.pop + node_count += 1 + stack.concat(n.children) + end + assert_operator node_count, :>=, 2, 'tree too small to be a meaningful N+1 guard' + + serialize_queries = count_sparql_queries do + LinkedData::Serializers::JSON.serialize([tree_root], only: display_attrs) + end + + assert_operator serialize_queries, :<, node_count, + "serialization issued #{serialize_queries} SPARQL queries for #{node_count} tree nodes -- " \ + "looks like a per-node N+1 (see get_languages guard in ncbo/ontologies_linked_data#302)" + end end