Skip to content

Commit

Permalink
validates doi name only when doi doesn't exist already
Browse files Browse the repository at this point in the history
DOI exists -> validation of metadata with DOI name containing a + sign should pass
DOI does not exist -> validation of metadata with DOI name containing a + sign should fail

fix datacite/bracco#189
  • Loading branch information
kjgarza committed Apr 13, 2019
1 parent 196c7c9 commit 0e35199
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
6 changes: 5 additions & 1 deletion app/controllers/dois_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,11 @@ def show
def validate
logger = Logger.new(STDOUT)
# logger.info safe_params.inspect
@doi = Doi.new(safe_params.merge(only_validate: true))

doi = Doi.where(doi: params.dig(:data,:attributes,:doi)).first
exists = doi.present?

@doi = Doi.new(safe_params.merge(only_validate: true, exists: exists))

This comment has been minimized.

Copy link
@mfenner

mfenner Apr 15, 2019

Contributor

Why create new DOI when DOI already exists?


authorize! :validate, @doi

Expand Down
3 changes: 2 additions & 1 deletion app/models/doi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class Doi < ActiveRecord::Base

attribute :regenerate, :boolean, default: false
attribute :only_validate, :boolean, default: false
attribute :exists, :boolean, default: false
attribute :should_validate, :boolean, default: false
attribute :agency, :string, default: "DataCite"

Expand All @@ -77,7 +78,7 @@ class Doi < ActiveRecord::Base
# validates_presence_of :url, if: :is_registered_or_findable?

# from https://www.crossref.org/blog/dois-and-matching-regular-expressions/ but using uppercase
validates_format_of :doi, :with => /\A10\.\d{4,5}\/[-\._;()\/:a-zA-Z0-9\*~\$\=]+\z/, :on => :create
validates_format_of :doi, :with => /\A10\.\d{4,5}\/[-\._;()\/:a-zA-Z0-9\*~\$\=]+\z/, :on => :create, unless: :exists

This comment has been minimized.

Copy link
@mfenner

mfenner Apr 15, 2019

Contributor

@kjgarza I don't think we need this. If the DOI exists, we will do an update rather than a create.

validates_format_of :url, :with => /\A(ftp|http|https):\/\/[\S]+/ , if: :url?, message: "URL is not valid"
validates_uniqueness_of :doi, message: "This DOI has already been taken", unless: :only_validate
validates :last_landing_page_status, numericality: { only_integer: true }, if: :last_landing_page_status?
Expand Down
52 changes: 52 additions & 0 deletions spec/requests/dois_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,36 @@
end
end

context 'when doi has unpermitted characters' do
let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) }
let(:valid_attributes) do
{
"data" => {
"type" => "dois",
"attributes" => {
"doi" => "10.14454/107+03",
"url" => "http://www.bl.uk/pdf/patspec.pdf",
"xml" => xml,
"source" => "test",
"event" => "publish"
}
}
}
end

before { post '/dois', params: valid_attributes.to_json, headers: headers }

it 'returns validation error' do
expect(json.dig('errors')).to eq([{"source"=>"doi", "title"=>"Is invalid"}])
end


it 'returns status code 422' do
expect(response).to have_http_status(422)
end

end

context 'creators no xml' do
let(:creators) { [{ "name"=>"Ollomi, Benjamin" }, { "name"=>"Duran, Patrick" }] }
let(:valid_attributes) do
Expand Down Expand Up @@ -1600,6 +1630,28 @@
end
end

context 'validatation fails with unpermitted characters in new DOI' do
let(:xml) { ::Base64.strict_encode64(File.read(file_fixture('datacite.xml'))) }
let(:params) do
{
"data" => {
"type" => "dois",
"attributes" => {
"doi" => "10.14454/107+03",
"xml" => xml,
}
}
}
end

before { post '/dois/validate', params: params.to_json, headers: headers }

it 'returns validation error' do
expect(json.dig('errors')).to eq([{"source"=>"doi", "title"=>"Is invalid"}])
end

end

context 'validates schema 3' do
let(:xml) { ::Base64.strict_encode64(File.read(file_fixture('datacite_schema_3.xml'))) }
let(:params) do
Expand Down

0 comments on commit 0e35199

Please sign in to comment.