Skip to content

Commit

Permalink
Merge pull request #1056 from pod4lib/add-cops
Browse files Browse the repository at this point in the history
Add new rubocop rules and run autocorrect
  • Loading branch information
cbeer authored Jun 17, 2024
2 parents aaf12cc + 602a504 commit 18f803c
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 33 deletions.
104 changes: 104 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -384,3 +384,107 @@ Performance/StringInclude: # new in 1.7
Enabled: true
Performance/Sum: # new in 1.8
Enabled: true
Gemspec/DevelopmentDependencies: # new in 1.44
Enabled: true
Lint/DuplicateMatchPattern: # new in 1.50
Enabled: true
Lint/ItWithoutArgumentsInBlock: # new in 1.59
Enabled: true
Lint/LiteralAssignmentInCondition: # new in 1.58
Enabled: true
Lint/MixedCaseRange: # new in 1.53
Enabled: true
Lint/RedundantRegexpQuantifiers: # new in 1.53
Enabled: true
Lint/UselessRescue: # new in 1.43
Enabled: true
Metrics/CollectionLiteralLength: # new in 1.47
Enabled: true
Style/ComparableClamp: # new in 1.44
Enabled: true
Style/ConcatArrayLiterals: # new in 1.41
Enabled: true
Style/DataInheritance: # new in 1.49
Enabled: true
Style/DirEmpty: # new in 1.48
Enabled: true
Style/ExactRegexpMatch: # new in 1.51
Enabled: true
Style/FileEmpty: # new in 1.48
Enabled: true
Style/MapIntoArray: # new in 1.63
Enabled: true
Style/MapToSet: # new in 1.42
Enabled: true
Style/MinMaxComparison: # new in 1.42
Enabled: true
Style/RedundantArrayConstructor: # new in 1.52
Enabled: true
Style/RedundantCurrentDirectoryInPath: # new in 1.53
Enabled: true
Style/RedundantDoubleSplatHashBraces: # new in 1.41
Enabled: true
Style/RedundantFilterChain: # new in 1.52
Enabled: true
Style/RedundantHeredocDelimiterQuotes: # new in 1.45
Enabled: true
Style/RedundantLineContinuation: # new in 1.49
Enabled: true
Style/RedundantRegexpArgument: # new in 1.53
Enabled: true
Style/RedundantRegexpConstructor: # new in 1.52
Enabled: true
Style/ReturnNilInPredicateMethodDefinition: # new in 1.53
Enabled: true
Style/SendWithLiteralMethodName: # new in 1.64
Enabled: true
Style/SingleLineDoEndBlock: # new in 1.57
Enabled: true
Style/SuperArguments: # new in 1.64
Enabled: true
Style/SuperWithArgsParentheses: # new in 1.58
Enabled: true
Style/YAMLFileRead: # new in 1.53
Enabled: true
Rails/DangerousColumnNames: # new in 2.21
Enabled: true
Rails/EnvLocal: # new in 2.22
Enabled: true
Rails/RedundantActiveRecordAllMethod: # new in 2.21
Enabled: true
Rails/ResponseParsedBody: # new in 2.18
Enabled: true
Rails/SelectMap: # new in 2.21
Enabled: true
Rails/ThreeStateBooleanColumn: # new in 2.19
Enabled: true
Rails/UnusedRenderContent: # new in 2.21
Enabled: true
Rails/WhereRange: # new in 2.25
Enabled: true
Performance/MapMethodChain: # new in 1.19
Enabled: true
Capybara/ClickLinkOrButtonStyle: # new in 2.19
Enabled: true
Capybara/MatchStyle: # new in 2.17
Enabled: true
Capybara/RedundantWithinFind: # new in 2.20
Enabled: true
Capybara/RSpec/HaveSelector: # new in 2.19
Enabled: true
FactoryBot/AssociationStyle: # new in 2.23
Enabled: true
FactoryBot/ExcessiveCreateList: # new in 2.25
Enabled: true
FactoryBot/FactoryAssociationWithStrategy: # new in 2.23
Enabled: true
FactoryBot/IdSequence: # new in 2.24
Enabled: true
FactoryBot/RedundantFactoryOption: # new in 2.23
Enabled: true
RSpecRails/MinitestAssertions: # new in 2.17
Enabled: true
RSpecRails/NegationBeValid: # new in 2.23
Enabled: true
RSpecRails/TravelAround: # new in 2.19
Enabled: true
4 changes: 2 additions & 2 deletions app/jobs/generate_delta_dump_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ def perform(organization)
end
end
oai_writer.finalize
delta_dump.public_send(:oai_xml).attach(io: File.open(oai_writer.oai_file),
filename: human_readable_filename(base_name, :oai_xml, oai_file_counter))
delta_dump.oai_xml.attach(io: File.open(oai_writer.oai_file),
filename: human_readable_filename(base_name, :oai_xml, oai_file_counter))

oai_file_counter += 1
# Save the dump once for every 100 oai files to free up file handles
Expand Down
8 changes: 4 additions & 4 deletions app/jobs/generate_full_dump_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ def perform(organization)

if oai_writer.bytes_written?
oai_writer.finalize
full_dump.public_send(:oai_xml).attach(io: File.open(oai_writer.oai_file),
filename: human_readable_filename(
base_name, :oai_xml, oai_file_counter
))
full_dump.oai_xml.attach(io: File.open(oai_writer.oai_file),
filename: human_readable_filename(
base_name, :oai_xml, oai_file_counter
))
end

oai_file_counter += 1
Expand Down
8 changes: 3 additions & 5 deletions app/jobs/generate_interstream_delta_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,9 @@ def perform(stream)
# Attach Files
current_stream_dump.create_interstream_delta unless current_stream_dump.interstream_delta

current_stream_dump.interstream_delta.public_send(:marc21).attach(io: File.open(mrc_tempfile), filename: "#{base_name}.mrc")
current_stream_dump.interstream_delta.public_send(:marcxml).attach(io: File.open(xml_tempfile), filename: "#{base_name}.xml")
# rubocop:disable Layout/LineLength
current_stream_dump.interstream_delta.public_send(:deletes).attach(io: File.open(delete_tempfile), filename: "#{base_name}.del.txt")
# rubocop:enable Layout/LineLength
current_stream_dump.interstream_delta.marc21.attach(io: File.open(mrc_tempfile), filename: "#{base_name}.mrc")
current_stream_dump.interstream_delta.marcxml.attach(io: File.open(xml_tempfile), filename: "#{base_name}.xml")
current_stream_dump.interstream_delta.deletes.attach(io: File.open(delete_tempfile), filename: "#{base_name}.del.txt")
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
end
6 changes: 3 additions & 3 deletions app/models/stream.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ def current_dump_ids(from_date: nil, until_date: nil)
.or(NormalizedDump.where(full_dump_id: full_dump_id))
.published
.order(created_at: :asc)
dumps_query = dumps_query.where('created_at >= ?', Time.zone.parse(from_date).beginning_of_day) if from_date.present?
dumps_query = dumps_query.where('created_at <= ?', Time.zone.parse(until_date).end_of_day) if until_date.present?
dumps_query = dumps_query.where(created_at: Time.zone.parse(from_date).beginning_of_day..) if from_date.present?
dumps_query = dumps_query.where(created_at: ..Time.zone.parse(until_date).end_of_day) if until_date.present?

dumps_query.pluck(:id)
end
Expand All @@ -115,7 +115,7 @@ def previous_default_stream_history(datetime = nil)

organization.default_stream_histories
.order(end_time: :desc)
.where('end_time < ?', default_stream_history.start_time)
.where(end_time: ...default_stream_history.start_time)
.first
end

Expand Down
6 changes: 3 additions & 3 deletions spec/features/normalized_download_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
visit normalized_data_organization_stream_path(organization, stream)

# Note, this won't work in a driver other that Rack::Test w/o some other magic
click_link "#{organization.slug}-2020-01-01-full-marc21.mrc.gz"
click_on "#{organization.slug}-2020-01-01-full-marc21.mrc.gz"
marc = MarcRecordService.marc_reader(StringIO.new(page.body), :marc21_gzip)

expect(page.response_headers['Content-Disposition']).to eq 'attachment'
Expand All @@ -43,7 +43,7 @@
visit normalized_data_organization_stream_path(organization, stream)

# Note, this won't work in a driver other that Rack::Test w/o some other magic
click_link "#{organization.slug}-2020-01-01-full-marc21.mrc.gz"
click_on "#{organization.slug}-2020-01-01-full-marc21.mrc.gz"
records = MarcRecordService.marc_reader(StringIO.new(page.body), :marc21_gzip)

records.each do |marc|
Expand All @@ -57,7 +57,7 @@

expect do
# Note, this won't work in a driver other that Rack::Test w/o some other magic
click_link "#{organization.slug}-2020-01-01-full-marc21.mrc.gz"
click_on "#{organization.slug}-2020-01-01-full-marc21.mrc.gz"
end.to change(Ahoy::Event, :count).by(1)

expect(Ahoy::Event.last.properties.with_indifferent_access).to include(
Expand Down
30 changes: 15 additions & 15 deletions spec/features/oai_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@

it 'renders an error if no verb is supplied' do
visit oai_url
expect(page).to have_selector('error[code="badVerb"]')
expect(page).to have_css('error[code="badVerb"]')
end

it 'renders an error if an unknown verb is supplied' do
visit oai_url(verb: 'Oops')
expect(page).to have_selector('error[code="badVerb"]')
expect(page).to have_css('error[code="badVerb"]')
end

it 'renders the time the request was submitted in iso8601 format' do
Expand All @@ -60,7 +60,7 @@

it 'renders the request params and root url as an element' do
visit oai_url(verb: 'ListSets')
expect(page).to have_selector('request[verb="ListSets"]', text: oai_url)
expect(page).to have_css('request[verb="ListSets"]', text: oai_url)
end

context 'when the verb is ListSets' do
Expand All @@ -83,7 +83,7 @@

it 'renders an error if unknown params are supplied' do
visit oai_url(verb: 'ListSets', foo: 'bar')
expect(page).to have_selector('error[code="badArgument"]')
expect(page).to have_css('error[code="badArgument"]')
end
end

Expand Down Expand Up @@ -129,7 +129,7 @@

it 'renders an error if unknown params are supplied' do
visit oai_url(verb: 'Identify', foo: 'bar')
expect(page).to have_selector('error[code="badArgument"]')
expect(page).to have_css('error[code="badArgument"]')
end
end

Expand All @@ -151,12 +151,12 @@
it 'renders an error if an unknown identifier is supplied' do
pending 'single item requests are not yet implemented'
visit oai_url(verb: 'ListMetadataFormats', identifier: 'fake')
expect(page).to have_selector('error[code="idDoesNotExist"]')
expect(page).to have_css('error[code="idDoesNotExist"]')
end

it 'renders an error if unknown params are supplied' do
visit oai_url(verb: 'ListMetadataFormats', foo: 'bar')
expect(page).to have_selector('error[code="badArgument"]')
expect(page).to have_css('error[code="badArgument"]')
end
end

Expand All @@ -178,7 +178,7 @@
it 'renders a header indicating records are deleted' do
token = OaiConcern::ResumptionToken.new(set: organization.default_stream.id.to_s, page: '2')
visit oai_url(verb: 'ListRecords', resumptionToken: token.encode)
expect(page).to have_selector('header[status="deleted"]')
expect(page).to have_css('header[status="deleted"]')
end

it 'renders the set membership of each item' do
Expand Down Expand Up @@ -227,22 +227,22 @@

it 'renders an error if unknown params are supplied' do
visit oai_url(verb: 'ListRecords', metadataPrefix: 'marc21', foo: 'bar')
expect(page).to have_selector('error[code="badArgument"]')
expect(page).to have_css('error[code="badArgument"]')
end

it 'renders an error if no metadata prefix is supplied' do
visit oai_url(verb: 'ListRecords')
expect(page).to have_selector('error[code="badArgument"]')
expect(page).to have_css('error[code="badArgument"]')
end

it 'renders an error if an unsupported metadata prefix is supplied' do
visit oai_url(verb: 'ListRecords', metadataPrefix: 'foo')
expect(page).to have_selector('error[code="cannotDisseminateFormat"]')
expect(page).to have_css('error[code="cannotDisseminateFormat"]')
end

it 'renders an error if the request results in an empty result set' do
visit oai_url(verb: 'ListRecords', metadataPrefix: 'marc21', set: '392487')
expect(page).to have_selector('error[code="noRecordsMatch"]')
expect(page).to have_css('error[code="noRecordsMatch"]')
end

context 'when a resumption token is supplied' do
Expand All @@ -253,17 +253,17 @@

it 'renders an error if any other argument is also supplied' do
visit oai_url(verb: 'ListRecords', resumptionToken: OaiConcern::ResumptionToken.new(set: '1').encode, from: '2020-01-01')
expect(page).to have_selector('error[code="badArgument"]')
expect(page).to have_css('error[code="badArgument"]')
end

it 'renders an error if the resumption token is not valid' do
visit oai_url(verb: 'ListRecords', resumptionToken: 'foo')
expect(page).to have_selector('error[code="badResumptionToken"]')
expect(page).to have_css('error[code="badResumptionToken"]')
end

it 'renders an error if the requested page of records does not exist' do
visit oai_url(verb: 'ListRecords', resumptionToken: OaiConcern::ResumptionToken.new(page: '999').encode)
expect(page).to have_selector('error[code="badResumptionToken"]')
expect(page).to have_css('error[code="badResumptionToken"]')
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/jobs/generate_interstream_delta_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@

# Deltas should contain one addition (we find two instances of 'record' in the xml for <record> and </record>)
xml = stream.current_full_dump.interstream_delta.marcxml.open { |file| File.readlines(file) }.to_s
expect(xml.scan(/record/).count).to be(2)
expect(xml.scan('record').count).to be(2)

# Deltas should contain two deletions
deletes = stream.current_full_dump.interstream_delta.deletes.open { |file| File.readlines(file) }
Expand Down

0 comments on commit 18f803c

Please sign in to comment.