From d8c8e5bb3a6d441ab0afacd72d0ba3597e098382 Mon Sep 17 00:00:00 2001 From: Syphax bouazzouni Date: Thu, 21 Sep 2023 19:43:40 +0200 Subject: [PATCH] Fix: display contact for get submissions (#45) * add get submission all including all properties test * extract and use submission_include_params where we use submission.bring * use retrieve_submissions helper in the :acronym/submissions endpoint --- Gemfile.lock | 18 ++- controllers/admin_controller.rb | 4 +- controllers/ontologies_controller.rb | 20 +-- .../ontology_submissions_controller.rb | 21 ++-- helpers/application_helper.rb | 2 - helpers/submission_helper.rb | 35 +++--- .../test_ontology_submissions_controller.rb | 118 +++++++++++++++++- 7 files changed, 158 insertions(+), 60 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 44537959..4a35ef0a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/ncbo/ncbo_ontology_recommender.git - revision: 83e835de368bc9f19da800a477982e0ad770900d + revision: f440ae855a217807fead1d20629a0f187997b973 branch: master specs: ncbo_ontology_recommender (0.0.1) @@ -11,7 +11,7 @@ GIT GIT remote: https://github.com/ontoportal-lirmm/goo.git - revision: 1d78bde5a711d05475da0459308c7db074af5e21 + revision: 0c0dba92e28fd8c8261db1b3183946e0cf183b53 branch: development specs: goo (0.0.2) @@ -53,7 +53,7 @@ GIT GIT remote: https://github.com/ontoportal-lirmm/ontologies_linked_data.git - revision: 4c89c8346766d23e09b24c8e29750bf3a91e6b53 + revision: 1204ede68ed0a5af5e3fb355172496d5e0134544 branch: development specs: ontologies_linked_data (0.0.1) @@ -173,10 +173,9 @@ GEM retriable (>= 2.0, < 4.a) rexml webrick - googleauth (1.7.0) + googleauth (1.8.0) faraday (>= 0.17.3, < 3.a) jwt (>= 1.4, < 3.0) - memoist (~> 0.16) multi_json (~> 1.11) os (>= 0.9, < 2.0) signet (>= 0.16, < 2.a) @@ -205,7 +204,6 @@ GEM net-imap net-pop net-smtp - memoist (0.16.2) method_source (1.0.0) mime-types (3.5.1) mime-types-data (~> 3.2015) @@ -231,7 +229,7 @@ GEM net-protocol net-ssh (7.2.0) netrc (0.11.0) - newrelic_rpm (9.4.2) + newrelic_rpm (9.5.0) oj (2.18.5) omni_logger (0.1.4) logger @@ -248,7 +246,7 @@ GEM rack (>= 0.4) rack-attack (6.6.1) rack (>= 1.0, < 3) - rack-cache (1.14.0) + rack-cache (1.13.0) rack (>= 0.4) rack-cors (1.0.6) rack (>= 1.6.0) @@ -294,7 +292,7 @@ GEM rubyzip (2.3.2) rufus-scheduler (2.0.24) tzinfo (>= 0.3.22) - signet (0.17.0) + signet (0.18.0) addressable (~> 2.8) faraday (>= 0.17.5, < 3.a) jwt (>= 1.5, < 3.0) @@ -326,7 +324,7 @@ GEM net-ssh (>= 2.8.0) systemu (2.6.5) temple (0.10.2) - tilt (2.2.0) + tilt (2.3.0) timeout (0.4.0) trailblazer-option (0.1.2) tzinfo (2.0.6) diff --git a/controllers/admin_controller.rb b/controllers/admin_controller.rb index 7ae6d800..747def93 100644 --- a/controllers/admin_controller.rb +++ b/controllers/admin_controller.rb @@ -68,7 +68,7 @@ class AdminController < ApplicationController latest = ont.latest_submission(status: :any) error 404, "Ontology #{params["acronym"]} contains no submissions" if latest.nil? check_last_modified(latest) - latest.bring(*OntologySubmission.goo_attrs_to_load(includes_param)) + latest.bring(*submission_include_params) NcboCron::Models::OntologySubmissionParser.new.queue_submission(latest, actions) halt 204 end @@ -84,7 +84,7 @@ class AdminController < ApplicationController latest = ont.latest_submission(status: :any) end check_last_modified(latest) if latest - latest.bring(*OntologySubmission.goo_attrs_to_load(includes_param)) if latest + latest.bring(*submission_include_params) if latest reply(latest || {}) end diff --git a/controllers/ontologies_controller.rb b/controllers/ontologies_controller.rb index 99c0ce68..58518420 100644 --- a/controllers/ontologies_controller.rb +++ b/controllers/ontologies_controller.rb @@ -38,22 +38,12 @@ class OntologiesController < ApplicationController else latest = ont.latest_submission(status: :any) end - check_last_modified(latest) if latest - # When asking to display all metadata, we are using bring_remaining which is more performant than including all metadata (remove this when the query to get metadata will be fixed) - if latest - if includes_param.first == :all - # Bring what we need to display all attr of the submission - latest.bring_remaining - latest.bring(*submission_attributes_all) - else - includes = OntologySubmission.goo_attrs_to_load(includes_param) - - includes << {:contact=>[:name, :email]} if includes.find{|v| v.is_a?(Hash) && v.keys.first.eql?(:contact)} - latest.bring(*includes) - end + if latest + check_last_modified(latest) + latest.bring(*submission_include_params) end - #remove the whole previous if block and replace by it: latest.bring(*OntologySubmission.goo_attrs_to_load(includes_param)) if latest + reply(latest || {}) end @@ -63,7 +53,7 @@ class OntologiesController < ApplicationController patch '/:acronym/latest_submission' do ont = Ontology.find(params["acronym"]).first error 422, "You must provide an existing `acronym` to patch" if ont.nil? - + submission = ont.latest_submission(status: :any) submission.bring(*OntologySubmission.attributes) diff --git a/controllers/ontology_submissions_controller.rb b/controllers/ontology_submissions_controller.rb index 7de7a3dc..77302c8d 100644 --- a/controllers/ontology_submissions_controller.rb +++ b/controllers/ontology_submissions_controller.rb @@ -29,17 +29,14 @@ class OntologySubmissionsController < ApplicationController error 422, "Ontology #{params["acronym"]} does not exist" unless ont check_last_modified_segment(LinkedData::Models::OntologySubmission, [ont.acronym]) check_access(ont) - if includes_param.first == :all - # When asking to display all metadata, we are using bring_remaining which is more performant than including all metadata (remove this when the query to get metadata will be fixed) - ont.bring(submission_attributes_all) - - ont.submissions.each do |sub| - sub.bring_remaining - end - else - ont.bring(submissions: OntologySubmission.goo_attrs_to_load(includes_param)) - end - reply ont.submissions.sort {|a,b| b.submissionId.to_i <=> a.submissionId.to_i } # descending order of submissionId + options = { + also_include_views: params["also_include_views"], + status: (params["include_status"] || "ANY"), + ontology: params["acronym"] + } + subs = retrieve_submissions(options) + + reply subs.sort {|a,b| b.submissionId.to_i <=> a.submissionId.to_i } # descending order of submissionId end ## @@ -58,7 +55,7 @@ class OntologySubmissionsController < ApplicationController ont.bring(:submissions) ont_submission = ont.submission(params["ontology_submission_id"]) error 404, "`submissionId` not found" if ont_submission.nil? - ont_submission.bring(*OntologySubmission.goo_attrs_to_load(includes_param)) + ont_submission.bring(*submission_include_params) reply ont_submission end diff --git a/helpers/application_helper.rb b/helpers/application_helper.rb index 5d6d1b0a..dbddbc5b 100644 --- a/helpers/application_helper.rb +++ b/helpers/application_helper.rb @@ -363,8 +363,6 @@ def retrieve_latest_submissions(options = {}) latest_submissions = page? ? submissions : {} # latest_submission doest not work with pagination submissions.each do |sub| - # To retrieve all metadata, but slow when a lot of ontologies - sub.bring_remaining if includes_param.first == :all unless page? next if include_ready?(options) && !sub.ready? next if sub.ontology.nil? diff --git a/helpers/submission_helper.rb b/helpers/submission_helper.rb index c1ee5dd3..07f82138 100644 --- a/helpers/submission_helper.rb +++ b/helpers/submission_helper.rb @@ -3,6 +3,18 @@ module Sinatra module Helpers module SubmissionHelper + def submission_include_params + # When asking to display all metadata, we are using bring_remaining on each submission. Slower but best way to retrieve all attrs + includes = OntologySubmission.goo_attrs_to_load(includes_param) + if includes.find{|v| v.is_a?(Hash) && v.keys.include?(:ontology)} + includes << {:ontology=>[:administeredBy, :acronym, :name, :viewingRestriction, :group, :hasDomain,:notes, :reviews, :projects,:acl, :viewOf]} + end + + if includes.find{|v| v.is_a?(Hash) && v.keys.include?(:contact)} + includes << {:contact=>[:name, :email]} + end + includes + end def submission_attributes_all out = [LinkedData::Models::OntologySubmission.embed_values_hash] @@ -16,14 +28,17 @@ def submission_attributes_all def retrieve_submissions(options) status = (options[:status] || "RDF").to_s.upcase status = "RDF" if status.eql?("READY") + ontology_acronym = options[:ontology] any = status.eql?("ANY") include_views = options[:also_include_views] || false includes, page, size, order_by, _ = settings_params(LinkedData::Models::OntologySubmission) includes << :submissionStatus unless includes.include?(:submissionStatus) submissions_query = LinkedData::Models::OntologySubmission + submissions_query = submissions_query.where(ontology: [acronym: ontology_acronym]) if ontology_acronym + if any - submissions_query = submissions_query.where + submissions_query = submissions_query.where unless ontology_acronym else submissions_query = submissions_query.where({ submissionStatus: [code: status] }) end @@ -32,24 +47,8 @@ def retrieve_submissions(options) submissions_query = submissions_query.filter(Goo::Filter.new(ontology: [:viewOf]).unbound) unless include_views submissions_query = submissions_query.filter(filter) if filter? - # When asking to display all metadata, we are using bring_remaining on each submission. Slower but best way to retrieve all attrs - if includes_param.first == :all - includes = [:submissionId, { :contact => [:name, :email], - :ontology => [:administeredBy, :acronym, :name, :summaryOnly, :ontologyType, :viewingRestriction, :acl, - :group, :hasDomain, :views, :viewOf, :flat, :notes, :reviews, :projects], - :submissionStatus => [:code], :hasOntologyLanguage => [:acronym], :metrics => [:classes, :individuals, :properties] }, - :submissionStatus] - else - if includes.find { |v| v.is_a?(Hash) && v.keys.include?(:ontology) } - includes << { :ontology => [:administeredBy, :acronym, :name, :viewingRestriction, :group, :hasDomain, :notes, :reviews, :projects, :acl, :viewOf] } - end - - if includes.find { |v| v.is_a?(Hash) && v.keys.include?(:contact) } - includes << { :contact => [:name, :email] } - end - end - submissions = submissions_query.include(includes) + submissions = submissions_query.include(submission_include_params) if page? submissions.page(page, size).all else diff --git a/test/controllers/test_ontology_submissions_controller.rb b/test/controllers/test_ontology_submissions_controller.rb index 9ee81257..7d4ed7ef 100644 --- a/test/controllers/test_ontology_submissions_controller.rb +++ b/test/controllers/test_ontology_submissions_controller.rb @@ -39,6 +39,12 @@ def self._create_onts ont.save end + def setup + delete_ontologies_and_submissions + ont = Ontology.new(acronym: @@acronym, name: @@name, administeredBy: [@@user]) + ont.save + end + def test_submissions_for_given_ontology num_onts_created, created_ont_acronyms = create_ontologies_and_submissions(ont_count: 1) ontology = created_ont_acronyms.first @@ -196,7 +202,7 @@ def test_download_acl_only end def test_submissions_pagination - num_onts_created, created_ont_acronyms = create_ontologies_and_submissions(ont_count: 2, submission_count: 2) + num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: 2, submission_count: 2) get "/submissions" assert last_response.ok? @@ -210,4 +216,114 @@ def test_submissions_pagination submissions = MultiJson.load(last_response.body) assert_equal 1, submissions["collection"].length end + + + def test_submissions_default_includes + ontology_count = 5 + num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: ontology_count, submission_count: 1, submissions_to_process: []) + + submission_default_attributes = LinkedData::Models::OntologySubmission.hypermedia_settings[:serialize_default].map(&:to_s) + + get("/submissions?display_links=false&display_context=false&include_status=ANY") + assert last_response.ok? + submissions = MultiJson.load(last_response.body) + + assert_equal ontology_count, submissions.size + assert(submissions.all? { |sub| submission_default_attributes.eql?(submission_keys(sub)) }) + + get("/ontologies/#{created_ont_acronyms.first}/submissions?display_links=false&display_context=false") + + assert last_response.ok? + submissions = MultiJson.load(last_response.body) + assert_equal 1, submissions.size + assert(submissions.all? { |sub| submission_default_attributes.eql?(submission_keys(sub)) }) + end + + def test_submissions_all_includes + ontology_count = 5 + num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: ontology_count, submission_count: 1, submissions_to_process: []) + def submission_all_attributes + attrs = OntologySubmission.goo_attrs_to_load([:all]) + embed_attrs = attrs.select { |x| x.is_a?(Hash) }.first + + attrs.delete_if { |x| x.is_a?(Hash) }.map(&:to_s) + embed_attrs.keys.map(&:to_s) + end + get("/submissions?include=all&display_links=false&display_context=false") + + assert last_response.ok? + submissions = MultiJson.load(last_response.body) + assert_equal ontology_count, submissions.size + + assert(submissions.all? { |sub| submission_all_attributes.sort.eql?(submission_keys(sub).sort) }) + assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) }) + + get("/ontologies/#{created_ont_acronyms.first}/submissions?include=all&display_links=false&display_context=false") + + assert last_response.ok? + submissions = MultiJson.load(last_response.body) + assert_equal 1, submissions.size + + assert(submissions.all? { |sub| submission_all_attributes.sort.eql?(submission_keys(sub).sort) }) + assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) }) + + get("/ontologies/#{created_ont_acronyms.first}/latest_submission?include=all&display_links=false&display_context=false") + assert last_response.ok? + sub = MultiJson.load(last_response.body) + + assert(submission_all_attributes.sort.eql?(submission_keys(sub).sort)) + assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id]))) + + get("/ontologies/#{created_ont_acronyms.first}/submissions/1?include=all&display_links=false&display_context=false") + assert last_response.ok? + sub = MultiJson.load(last_response.body) + + assert(submission_all_attributes.sort.eql?(submission_keys(sub).sort)) + assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id]))) + end + + def test_submissions_custom_includes + ontology_count = 5 + num_onts_created, created_ont_acronyms, ontologies = create_ontologies_and_submissions(ont_count: ontology_count, submission_count: 1, submissions_to_process: []) + include = 'ontology,contact,submissionId' + + get("/submissions?include=#{include}&display_links=false&display_context=false") + + assert last_response.ok? + submissions = MultiJson.load(last_response.body) + assert_equal ontology_count, submissions.size + assert(submissions.all? { |sub| include.split(',').eql?(submission_keys(sub)) }) + assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) }) + + get("/ontologies/#{created_ont_acronyms.first}/submissions?include=#{include}&display_links=false&display_context=false") + + assert last_response.ok? + submissions = MultiJson.load(last_response.body) + assert_equal 1, submissions.size + assert(submissions.all? { |sub| include.split(',').eql?(submission_keys(sub)) }) + assert(submissions.all? { |sub| sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id])) }) + + get("/ontologies/#{created_ont_acronyms.first}/latest_submission?include=#{include}&display_links=false&display_context=false") + assert last_response.ok? + sub = MultiJson.load(last_response.body) + assert(include.split(',').eql?(submission_keys(sub))) + assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id]))) + + get("/ontologies/#{created_ont_acronyms.first}/submissions/1?include=#{include}&display_links=false&display_context=false") + assert last_response.ok? + sub = MultiJson.load(last_response.body) + assert(include.split(',').eql?(submission_keys(sub))) + assert(sub["contact"] && (sub["contact"].first.nil? || sub["contact"].first.keys.eql?(%w[name email id]))) + end + + def test_submissions_param_include + skip('only for local development regrouping a set of tests') + test_submissions_default_includes + test_submissions_all_includes + test_submissions_custom_includes + end + + private + def submission_keys(sub) + sub.to_hash.keys - %w[@id @type id] + end end