Skip to content

Commit

Permalink
Guard agains incomplete related_dois (#1069)
Browse files Browse the repository at this point in the history
## Purpose
Some related_identifiers do not have all the required fields in the database due to changes inthe schema.

closes: #1066, #1067, #1068

## Approach
Make sure all related_identifiers is a hash and has all the required fields

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue)

- [ ] New feature (non-breaking change which adds functionality)

- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Reviewer, please remember our [guidelines](https://datacite.atlassian.net/wiki/spaces/TEC/pages/1168375809/Pull+Request+Guidelines):

- Be humble in the language and feedback you give, ask don't tell.
- Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
- Offer suggestions on how to improve code e.g. simplification or expanding clarity.
- Ensure you give reasons for the changes you are proposing.
  • Loading branch information
jrhoads authored Dec 11, 2023
2 parents c8e812a + df07508 commit 4df4d77
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 1 deletion.
16 changes: 15 additions & 1 deletion app/models/doi/indexer/related_doi_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,27 @@

module Doi::Indexer
class RelatedDoiIndexer
REQUIRED_KEYS = %w[
relatedIdentifier
relatedIdentifierType
relationType
]

def initialize(related_identifiers)
@related_identifiers = Array.wrap(related_identifiers)
@related_dois = nil
end

def is_related_doi?(related)
related.is_a?(Hash) &&
REQUIRED_KEYS.all? { |k| related.key?(k) } &&
related.fetch("relatedIdentifierType", nil) == "DOI"
end

def related_dois
@related_dois ||= @related_identifiers.select { |r| r.fetch("relatedIdentifierType", nil) == "DOI" }
@related_dois ||= @related_identifiers.select do |r|
is_related_doi?(r)
end
end

def related_grouped_by_id
Expand Down
118 changes: 118 additions & 0 deletions spec/models/doi/related_doi_indexer_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@

# frozen_string_literal: true

require "rails_helper"

RSpec.describe Doi::Indexer::RelatedDoiIndexer do
describe "related_dois with different input" do
let(:good_related_identifier) do
{
"relatedIdentifier": "10.1234/5678",
"relatedIdentifierType": "DOI",
"relationType": "IsPartOf",
}.with_indifferent_access
end

it "handles nil input" do
expect(described_class.new(nil).related_dois).to eq([])
end

it "handles empty_string input" do
expect(described_class.new("").related_dois).to eq([])
end

it "handles a list with empty_string as input" do
expect(described_class.new([""]).related_dois).to eq([])
end


it "handles array of hashes with all required keys" do
expect(described_class.new([good_related_identifier]).related_dois).to eq(
[good_related_identifier])
end

it "handles single hash with all required keys" do
expect(described_class.new(good_related_identifier).related_dois).to eq(
[good_related_identifier])
end

it "exclude DOIs with if a required key is missing" do
expect(described_class.new(
good_related_identifier.except("relatedIdentifier")
).related_dois).to eq([])
expect(described_class.new(
good_related_identifier.except("relatedIdentifierType")
).related_dois).to eq([])
expect(described_class.new(
good_related_identifier.except("relationType")
).related_dois).to eq([])
end
end

describe "relation_type grouping" do
let(:related_identifiers) do
[
{
"relatedIdentifier": "10.1234/5678",
"relatedIdentifierType": "DOI",
"relationType": "IsPartOf",
"resourceTypeGeneral": "Dataset",
}.with_indifferent_access,
{
"relatedIdentifier": "10.1234/9999",
"relatedIdentifierType": "DOI",
"relationType": "HasVersion",
"resourceTypeGeneral": "Text",
}.with_indifferent_access,
{
"relatedIdentifier": "10.1234/9999",
"relatedIdentifierType": "DOI",
"relationType": "References",
"resourceTypeGeneral": "Text",
}.with_indifferent_access
]
end

it "can accept an array of valid identifiers" do
expect(described_class.new(related_identifiers).related_dois).to eq(related_identifiers)
end

it "groups related_dois by relatedIdentifier" do
expect(described_class.new(related_identifiers).relation_types_gouped_by_id).to eq(
{
"10.1234/5678" => ["is_part_of"],
"10.1234/9999" => ["has_version", "references"],
}
)
end

it "groups related_dois by relatedIdentifier" do
expect(described_class.new(related_identifiers).related_grouped_by_id).to eq(
{
"10.1234/5678" => [
{
"relatedIdentifier" => "10.1234/5678",
"relatedIdentifierType" => "DOI",
"relationType" => "IsPartOf",
"resourceTypeGeneral" => "Dataset",
}
],
"10.1234/9999" => [
{
"relatedIdentifier" => "10.1234/9999",
"relatedIdentifierType" => "DOI",
"relationType" => "HasVersion",
"resourceTypeGeneral" => "Text",
},
{
"relatedIdentifier" => "10.1234/9999",
"relatedIdentifierType" => "DOI",
"relationType" => "References",
"resourceTypeGeneral" => "Text",
}
],
}
)
end
end
end

0 comments on commit 4df4d77

Please sign in to comment.