enforce type for doi json fields. #331

Gemfile.lock

@@ -106,7 +106,7 @@ GEM
       latex-decode (~> 0.0)
     binding_of_caller (0.8.0)
       debug_inspector (>= 0.0.1)
-    bolognese (1.3.9)
+    bolognese (1.3.10)
       activesupport (>= 4.2.5, < 6)
       benchmark_methods (~> 0.7)
       bibtex-ruby (~> 4.1)
@@ -134,7 +134,7 @@ GEM
     builder (3.2.3)
     byebug (11.0.1)
     cancancan (2.3.0)
-    capybara (3.27.0)
+    capybara (3.28.0)
       addressable
       mini_mime (>= 0.1.3)
       nokogiri (~> 1.8)

app/controllers/dois_controller.rb

@@ -487,9 +487,6 @@ def safe_params
 
     fail JSON::ParserError, "You need to provide a payload following the JSONAPI spec" unless params[:data].present?
 
-    # default values for attributes stored as JSON
-    defaults = { data: { titles: [], descriptions: [], types: {}, dates: [], rightsList: [], creators: [], contributors: [] }}
-
     attributes = [
       :doi,
       :confirmDoi,
@@ -507,8 +504,8 @@ def safe_params
       { dates: [:date, :dateType, :dateInformation] },
       :subjects,
       { subjects: [:subject, :subjectScheme, :schemeUri, :valueUri, :lang] },
-      {
-        landingPage: [
+      :landingPage,
+      { landingPage: [
           :checked,
           :url,
           :status,
@@ -567,6 +564,9 @@ def safe_params
     ]
     relationships = [{ client: [data: [:type, :id]] }]
 
+    # default values for attributes stored as JSON
+    defaults = { data: { titles: [], descriptions: [], types: {}, container: {}, dates: [], subjects: [], rightsList: [], creators: [], contributors: [], sizes: [], formats: [], contentUrl: [], identifiers: [], relatedIdentifiers: [], fundingReferences: [], geoLocations: [] }}
+
     p = params.require(:data).permit(:type, :id, attributes: attributes, relationships: relationships).reverse_merge(defaults)
     client_id = p.dig("relationships", "client", "data", "id") || current_user.try(:client_id)
     p = p.fetch("attributes").merge(client_id: client_id)
@@ -619,7 +619,7 @@ def safe_params
     # merge attributes from xml into regular attributes
     # make sure we don't accidentally set any attributes to nil
     read_attrs_keys.each do |attr|
-      p.merge!(attr.to_s.underscore => p[attr].presence || meta[attr.to_s.underscore].presence || p[attr]) if p.has_key?(attr) || meta.has_key?(attr.to_s.underscore)
+      p.merge!(attr.to_s.underscore => p[attr] || meta[attr.to_s.underscore] || p[attr]) if p.has_key?(attr) || meta.has_key?(attr.to_s.underscore)
     end
     p.merge!(version_info: p[:version] || meta["version_info"]) if p.has_key?(:version_info) || meta["version_info"].present?
 

app/models/doi.rb

@@ -85,8 +85,18 @@ class Doi < ActiveRecord::Base
   validates :xml, presence: true, xml_schema: true, if: Proc.new { |doi| doi.validatable? }
   validate :check_dates, if: :dates?
   validate :check_rights_list, if: :rights_list?
+  validate :check_titles, if: :titles?
   validate :check_descriptions, if: :descriptions?
+  validate :check_types, if: :types?
+  validate :check_container, if: :container?
   validate :check_subjects, if: :subjects?
+  validate :check_creators, if: :creators?
+  validate :check_contributors, if: :contributors?
+  validate :check_landing_page, if: :landing_page?
+  validate :check_identifiers, if: :identifiers?
+  validate :check_related_identifiers, if: :related_identifiers?
+  validate :check_funding_references, if: :funding_references?
+  validate :check_geo_locations, if: :geo_locations?
 
   after_commit :update_url, on: [:create, :update]
   after_commit :update_media, on: [:create, :update]
@@ -727,6 +737,18 @@ def doi=(value)
     write_attribute(:doi, value.upcase) if value.present?
   end
 
+  def formats=(value)
+    write_attribute(:formats, Array.wrap(value))
+  end
+
+  def sizes=(value)
+    write_attribute(:sizes, Array.wrap(value))
+  end
+
+  def content_url=(value)
+    write_attribute(:content_url, Array.wrap(value))
+  end
+
   def identifier
     normalize_doi(doi, sandbox: !Rails.env.production?)
   end
@@ -787,8 +809,8 @@ def update_media
 
     media.delete_all
 
-    Array.wrap(content_url).each do |c|
-      media << Media.create(url: c, media_type: formats)
+    Array.wrap(content_url).each_with_index do |c, index|
+      media << Media.create(url: c, media_type: Array.wrap(formats)[index])
     end
   end
 
@@ -853,6 +875,12 @@ def check_rights_list
     end
   end
 
+  def check_titles
+    Array.wrap(titles).each do |t|
+      errors.add(:titles, "Title '#{t}' should be an object instead of a string.") unless t.is_a?(Hash)
+    end
+  end
+
   def check_descriptions
     Array.wrap(descriptions).each do |d|
       errors.add(:descriptions, "Description '#{d}' should be an object instead of a string.") unless d.is_a?(Hash)
@@ -865,6 +893,54 @@ def check_subjects
     end
   end
 
+  def check_creators
+    Array.wrap(creators).each do |c|
+      errors.add(:creators, "Creator '#{c}' should be an object instead of a string.") unless c.is_a?(Hash)
+    end
+  end
+
+  def check_contributors
+    Array.wrap(contributors).each do |c|
+      errors.add(:contributors, "Contributor '#{c}' should be an object instead of a string.") unless c.is_a?(Hash)
+    end
+  end
+
+  def check_identifiers
+    Array.wrap(identifiers).each do |i|
+      errors.add(:identifiers, "Identifier '#{i}' should be an object instead of a string.") unless i.is_a?(Hash)
+    end
+  end
+
+  def check_related_identifiers
+    Array.wrap(related_identifiers).each do |r|
+      errors.add(:related_identifiers, "Related identifier '#{r}' should be an object instead of a string.") unless r.is_a?(Hash)
+    end
+  end
+
+  def check_funding_references
+    Array.wrap(funding_references).each do |f|
+      errors.add(:funding_references, "Funding reference '#{f}' should be an object instead of a string.") unless f.is_a?(Hash)
+    end
+  end
+
+  def check_geo_locations
+    Array.wrap(geo_locations).each do |g|
+      errors.add(:geo_locations, "Geolocation '#{g}' should be an object instead of a string.") unless g.is_a?(Hash)
+    end
+  end
+
+  def check_landing_page
+    errors.add(:landing_page, "Landing page '#{landing_page}' should be an object instead of a string.") unless landing_page.is_a?(Hash)
+  end
+
+  def check_types
+    errors.add(:types, "Types '#{types}' should be an object instead of a string.") unless types.is_a?(Hash)
+  end
+
+  def check_container
+    errors.add(:container, "Container '#{container}' should be an object instead of a string.") unless container.is_a?(Hash)
+  end
+
   # to be used after DOIs were transferred to another DOI RA
   def self.delete_dois_by_prefix(prefix, options={})
     logger = Logger.new(STDOUT)

app/models/media.rb

@@ -45,7 +45,7 @@ def doi_id=(value)
   def set_defaults
     current_media = Media.where(dataset: dataset).order('media.created DESC').first
     self.version = current_media.present? ? current_media.version + 1 : 0
-    self.media_type = "text/plain " if media_type.blank?
+    self.media_type = "text/plain" if media_type.blank?
     self.updated = Time.zone.now.utc.iso8601
   end
 end

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
@@ -569,6 +569,64 @@
       end
     end
 
+    context 'when the title is changed wrong format' do
+      let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) }
+      let(:titles) { "Submitted chemical data for InChIKey=YAPQBXQYLJRXSA-UHFFFAOYSA-N" }
+      let(:valid_attributes) do
+        {
+          "data" => {
+            "type" => "dois",
+            "attributes" => {
+              "url" => "http://www.bl.uk/pdf/pat.pdf",
+              "xml" => xml,
+              "titles" => titles,
+              "event" => "publish"
+            }
+          }
+        }
+      end
+
+      it 'error' do
+        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."}])
+      end
+    end
+
+    context 'when the description is changed to empty' do
+      let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) }
+      let(:descriptions) { [] }
+      let(:valid_attributes) do
+        {
+          "data" => {
+            "type" => "dois",
+            "attributes" => {
+              "url" => "http://www.bl.uk/pdf/pat.pdf",
+              "xml" => xml,
+              "descriptions" => descriptions,
+              "event" => "publish"
+            }
+          }
+        }
+      end
+
+      it 'updates the record' do
+        patch "/dois/#{doi.doi}", valid_attributes, headers
+        puts last_response.body
+        expect(last_response.status).to eq(200)
+        expect(json.dig('data', 'attributes', 'url')).to eq("http://www.bl.uk/pdf/pat.pdf")
+        expect(json.dig('data', 'attributes', 'doi')).to eq(doi.doi.downcase)
+        expect(json.dig('data', 'attributes', 'descriptions')).to eq([])
+      end
+
+      it 'sets state to findable' do
+        patch "/dois/#{doi.doi}", valid_attributes, headers
+
+        expect(json.dig('data', 'attributes', 'state')).to eq("findable")
+      end
+    end
+
     context 'when a doi is created ignore reverting back' do
       let(:xml) { Base64.strict_encode64(file_fixture('datacite.xml').read) }
       let(:valid_attributes) do
@@ -1041,7 +1099,7 @@
 
       it 'updates the record' do
         patch "/dois/10.14454/8na3-9s47", valid_attributes, headers
-
+        puts last_response.body
         expect(last_response.status).to eq(201)
         expect(json.dig('data', 'attributes', 'url')).to eq("https://ors.datacite.org/doi:/10.14454/8na3-9s47")
         expect(json.dig('data', 'attributes', 'doi')).to eq("10.14454/8na3-9s47")
@@ -2174,6 +2232,7 @@
       it 'updates the Doi' do
         patch "/dois/#{doi.doi}", update_attributes, headers
 
+        expect(last_response.status).to eq(200)
         expect(json.dig('data', 'attributes', 'contentUrl')).to eq(content_url)
       end
     end
@@ -2330,7 +2389,7 @@
         patch "/dois/#{doi.doi}", update_attributes_again, headers
 
         expect(json.dig('data', 'attributes', 'descriptions').size).to eq(1)
-        expect(json.dig('data', 'attributes', 'container')).to be_nil
+        expect(json.dig('data', 'attributes', 'container')).to be_empty
       end
     end