From 573d4b9a7dcc1a31fd1c3dbcb16ec58584b2136b Mon Sep 17 00:00:00 2001 From: Kristian Garza Date: Tue, 15 Oct 2019 18:41:08 +0200 Subject: [PATCH 1/3] fixes error wrong code https://github.com/datacite/lupo/issues/264 creating a new DOI reuqires to have client id but that was is obtianed in safe_params which also doesn not require auth --- app/controllers/dois_controller.rb | 1 + spec/requests/dois_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/controllers/dois_controller.rb b/app/controllers/dois_controller.rb index c12eb9762..ad1925445 100644 --- a/app/controllers/dois_controller.rb +++ b/app/controllers/dois_controller.rb @@ -291,6 +291,7 @@ def validate def create logger = Logger.new(STDOUT) # logger.info safe_params.inspect + fail CanCan::AuthorizationNotPerformed unless current_user.present? @doi = Doi.new(safe_params) diff --git a/spec/requests/dois_spec.rb b/spec/requests/dois_spec.rb index fbe290aff..bb5dcd927 100644 --- a/spec/requests/dois_spec.rb +++ b/spec/requests/dois_spec.rb @@ -966,6 +966,30 @@ end end + context 'when the request is valid no password' do + let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) } + let(:valid_attributes) do + { + "data" => { + "type" => "dois", + "attributes" => { + "doi" => "10.14454/10703", + "url" => "http://www.bl.uk/pdf/patspec.pdf", + "xml" => xml, + "source" => "test", + "event" => "publish" + } + } + } + end + + it 'fails to create a Doi' do + post '/dois', valid_attributes + + expect(last_response.status).to eq(401) + end + end + context 'when the request is valid random doi' do let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) } let(:valid_attributes) do From be35b99272b5f3238703b1d65312a3871498935e Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Wed, 16 Oct 2019 14:43:47 +0200 Subject: [PATCH 2/3] Fix serialisation of errors Errors should not capitalize and lowercase all the rest of the remaining text. Prevents misleading errors specifically when dealing with uppercase/lowercase attribute names. Changed additional error message tests to match new serialisation. --- .../concerns/error_serializable.rb | 2 +- app/validators/xml_schema_validator.rb | 6 +-- db/schema.rb | 29 ++++++++---- .../datacite_malformed_creator_name_type.xml | 44 +++++++++++++++++++ spec/requests/dois_spec.rb | 40 +++++++++++++---- 5 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 spec/fixtures/files/datacite_malformed_creator_name_type.xml diff --git a/app/controllers/concerns/error_serializable.rb b/app/controllers/concerns/error_serializable.rb index a47bcd289..52748a762 100644 --- a/app/controllers/concerns/error_serializable.rb +++ b/app/controllers/concerns/error_serializable.rb @@ -9,7 +9,7 @@ def serialize_errors(errors) source = err.keys.first Array.wrap(err.values.first).each do |title| - sum << { source: source, title: title.is_a?(String) ? title.capitalize : title.to_s } + sum << { source: source, title: title.is_a?(String) ? title.sub(/^./, &:upcase) : title.to_s } end sum diff --git a/app/validators/xml_schema_validator.rb b/app/validators/xml_schema_validator.rb index 672779af9..ed762aa3f 100644 --- a/app/validators/xml_schema_validator.rb +++ b/app/validators/xml_schema_validator.rb @@ -39,16 +39,16 @@ def validate_each(record, attribute, value) record.errors[:xml] << "Schema #{record.schema_version} is no longer supported" return false end - + filepath = Bundler.rubygems.find_name('bolognese').first.full_gem_path + "/resources/#{kernel}/metadata.xsd" schema = Nokogiri::XML::Schema(open(filepath)) - + schema.validate(Nokogiri::XML(value, nil, 'UTF-8')).reduce({}) do |sum, error| location, level, source, text = error.message.split(": ", 4) line, column = location.split(":", 2) title = text.to_s.strip + " at line #{line}, column #{column}" if line.present? source = source.split("}").last[0..-2] if line.present? - source = schema_attributes(source) if source.present? + source = schema_attributes(source) if source.present? record.errors[source.to_sym] << title end rescue Nokogiri::XML::SyntaxError => e diff --git a/db/schema.rb b/db/schema.rb index 9b5211759..e4197710c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ ActiveRecord::Schema.define(version: 2019_08_07_002912) do - create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin ROW_FORMAT=DYNAMIC", force: :cascade do |t| + create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin", force: :cascade do |t| t.string "name", limit: 191, null: false t.string "record_type", null: false t.bigint "record_id", null: false @@ -22,7 +22,7 @@ t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true end - create_table "active_storage_blobs", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin ROW_FORMAT=DYNAMIC", force: :cascade do |t| + create_table "active_storage_blobs", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin", force: :cascade do |t| t.string "key", limit: 191, null: false t.string "filename", limit: 191, null: false t.string "content_type", limit: 191 @@ -85,7 +85,7 @@ t.index ["prefixes"], name: "FKE7FBD674AF86A1C7" end - create_table "audits", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin ROW_FORMAT=DYNAMIC", force: :cascade do |t| + create_table "audits", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin", force: :cascade do |t| t.integer "auditable_id" t.string "auditable_type" t.integer "associated_id" @@ -107,6 +107,17 @@ t.index ["user_id", "user_type"], name: "user_index" end + create_table "contacts", options: "ENGINE=InnoDB DEFAULT CHARSET=latin1", force: :cascade do |t| + t.bigint "allocator" + t.string "email" + t.string "given_name" + t.string "family_name" + t.string "role" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["allocator"], name: "fk_rails_5c598567a8" + end + create_table "datacentre", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.text "comments", limit: 4294967295 t.string "system_email", null: false @@ -128,8 +139,7 @@ t.text "url" t.string "software", limit: 191 t.text "description" - t.string "client_type", limit: 191, default: "repository" - t.string "doaj_journal_id", limit: 191 + t.string "client_type", limit: 191 t.json "issn" t.json "certificate" t.json "repository_type" @@ -211,7 +221,7 @@ t.index ["url"], name: "index_dataset_on_url", length: 100 end - create_table "events", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=DYNAMIC", force: :cascade do |t| + create_table "events", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", force: :cascade do |t| t.text "uuid", null: false t.text "subj_id", null: false t.text "obj_id" @@ -238,7 +248,7 @@ t.index ["uuid"], name: "index_events_on_uuid", unique: true, length: 36 end - create_table "media", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "media", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.string "media_type", limit: 80 t.datetime "updated" @@ -250,7 +260,7 @@ t.index ["url"], name: "index_media_on_url", length: 100 end - create_table "metadata", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "metadata", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.integer "metadata_version" t.integer "version" @@ -262,7 +272,7 @@ t.index ["dataset"], name: "FKE52D7B2F4D3D6B1B" end - create_table "prefix", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + create_table "prefix", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 ROW_FORMAT=COMPACT", force: :cascade do |t| t.datetime "created" t.string "prefix", limit: 80, null: false t.integer "version" @@ -301,6 +311,7 @@ add_foreign_key "allocator_prefixes", "allocator", column: "allocator", name: "FKE7FBD67446EBD781" add_foreign_key "allocator_prefixes", "prefix", column: "prefixes", name: "FKE7FBD674AF86A1C7" + add_foreign_key "contacts", "allocator", column: "allocator" add_foreign_key "datacentre", "allocator", column: "allocator", name: "FK6695D60546EBD781" add_foreign_key "datacentre_prefixes", "datacentre", column: "datacentre", name: "FK13A1B3BA47B5F5FF" add_foreign_key "datacentre_prefixes", "prefix", column: "prefixes", name: "FK13A1B3BAAF86A1C7" diff --git a/spec/fixtures/files/datacite_malformed_creator_name_type.xml b/spec/fixtures/files/datacite_malformed_creator_name_type.xml new file mode 100644 index 000000000..f7a00e71a --- /dev/null +++ b/spec/fixtures/files/datacite_malformed_creator_name_type.xml @@ -0,0 +1,44 @@ + + + 10.25670/oi2018-001on + + Učinkovitost kanabinoidov pri zdravljenju raka + mit ali resnica + Therapeutic Efficacy of Cannabinoids in Cancer Treatment + Myth or Fact + + + + Grošelj, Blaž + Blaž + Grošelj + Onkološki inštitut Ljubljana + + + Onkološki inštitut Ljubljana + 2018 + Journal Article + + 610 Medical sciences; Medicine + cancer + cannabinoids + + + 2018-06-20 + + + 1581-3215 + + + The use of cannabis derivatives (cannabinoids) in oncology is sometimes indicated in the symptomatic treatment of nausea and vomiting, in pain management and in some neuropsychiatric disorders. In the recent years, there has been a growing interest in determining the anticancer effects of cannabinoids. Based on our clinical experience, we know that many patients use cannabinoids, among them also those with the aim of curing their disease. But despite the extensive and convincing data on the anticancer properties of cannabinoids from in vitro studies on cell cultures and studies on animal models, these properties have not (yet) been confirmed in a clinical trial. This paper summarises the currently available data on the potential of cannabinoid use in the clinical practice of oncology. + Onkologija, leto XXII, št. 1, junij 2018 + Onkologija, Volume XXII, No. 1, June 2018 + + sl + + pdf + + + Creative Commons Attribution 4.0 International license (CC BY 4.0) + + \ No newline at end of file diff --git a/spec/requests/dois_spec.rb b/spec/requests/dois_spec.rb index fbe290aff..764e69a54 100644 --- a/spec/requests/dois_spec.rb +++ b/spec/requests/dois_spec.rb @@ -338,7 +338,7 @@ it 'updates the doi' do put "/dois/#{doi.doi}", valid_attributes, admin_headers - + expect(last_response.status).to eq(200) expect(json.dig('data', 'attributes', 'sizes')).to eq(sizes) end @@ -386,7 +386,7 @@ put "/dois/#{doi.doi}", valid_attributes, headers expect(last_response.status).to eq(422) - expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) + expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). Expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) end end @@ -502,7 +502,7 @@ put "/dois/#{doi_id}", valid_attributes, headers expect(last_response.status).to eq(422) - expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) + expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). Expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) end end @@ -528,7 +528,7 @@ put "/dois/#{doi_id}", valid_attributes, headers expect(last_response.status).to eq(422) - expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) + expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). Expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) end end @@ -616,7 +616,7 @@ patch "/dois/#{doi.doi}", valid_attributes, headers expect(last_response.status).to eq(422) - expect(json['errors']).to eq([{"source"=>"titles", "title"=>"Title 'submitted chemical data for inchikey=yapqbxqyljrxsa-uhfffaoysa-n' should be an object instead of a string."}]) + expect(json['errors']).to eq([{"source"=>"titles", "title"=>"Title 'Submitted chemical data for InChIKey=YAPQBXQYLJRXSA-UHFFFAOYSA-N' should be an object instead of a string."}]) end end @@ -1216,7 +1216,7 @@ } HEREDOC ) } - + let(:valid_attributes) do { "data" => { @@ -1975,7 +1975,7 @@ post '/dois', not_valid_attributes, headers expect(last_response.status).to eq(422) - expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) + expect(json["errors"]).to eq([{"source"=>"creators", "title"=>"Missing child element(s). Expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0"}]) end end @@ -2068,7 +2068,7 @@ expect(last_response.status).to eq(200) expect(json['errors'].size).to eq(1) - expect(json['errors'].first).to eq("source"=>"creators", "title"=>"Missing child element(s). expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0") + expect(json['errors'].first).to eq("source"=>"creators", "title"=>"Missing child element(s). Expected is ( {http://datacite.org/schema/kernel-4}creator ). at line 4, column 0") end end @@ -2091,7 +2091,29 @@ expect(last_response.status).to eq(200) expect(json['errors'].size).to eq(1) - expect(json['errors'].first).to eq("source"=>"creatorName", "title"=>"This element is not expected. expected is ( {http://datacite.org/schema/kernel-4}affiliation ). at line 16, column 0") + expect(json['errors'].first).to eq("source"=>"creatorName", "title"=>"This element is not expected. Expected is ( {http://datacite.org/schema/kernel-4}affiliation ). at line 16, column 0") + end + end + + context 'when attribute type names are wrong' do + let(:xml) { ::Base64.strict_encode64(File.read(file_fixture('datacite_malformed_creator_name_type.xml'))) } + let(:params) do + { + "data" => { + "type" => "dois", + "attributes" => { + "doi" => "10.14454/10703", + "xml" => xml, + } + } + } + end + + it 'validates types are in right format' do + post '/dois/validate', params, headers + + expect(last_response.status).to eq(200) + expect(json['errors'].first).to eq("source"=>"creatorName', attribute 'nameType","title"=>"[facet 'enumeration'] The value 'personal' is not an element of the set {'Organizational', 'Personal'}. at line 12, column 0") end end From d8d1d825163928d8ff282a07159987970c5d8d33 Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Tue, 22 Oct 2019 18:14:46 +0200 Subject: [PATCH 3/3] Export should always include deleted across pages --- app/controllers/export_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/export_controller.rb b/app/controllers/export_controller.rb index 3c8c85660..a856dbaac 100644 --- a/app/controllers/export_controller.rb +++ b/app/controllers/export_controller.rb @@ -141,7 +141,7 @@ def organizations page_num = 2 while page_num <= total_pages page = { size: 1000, number: page_num } - response = Client.query(nil, page: page) + response = Client.query(nil, page: page, include_deleted: true) clients = clients + response.records.to_a page_num += 1 end