Skip to content

Commit

Permalink
Delete a newly created page if the version import fails
Browse files Browse the repository at this point in the history
  • Loading branch information
bensheldon committed May 21, 2019
1 parent fe59864 commit 20ed3a0
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 21 deletions.
52 changes: 31 additions & 21 deletions app/jobs/import_versions_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,33 +76,43 @@ def import_record(record)

return if existing && @import.skip_existing_records?

version = version_for_record(record, existing, @import.update_behavior)
version.page = page
begin
version = version_for_record(record, existing, @import.update_behavior)
version.page = page

if version.uri.nil?
if record['content']
# TODO: upload content
raise Api::NotImplementedError, 'Raw content uploading not implemented yet.'
if version.uri.nil?
if record['content']
# TODO: upload content
raise Api::NotImplementedError, 'Raw content uploading not implemented yet.'
end
elsif !Archiver.already_archived?(version.uri) || !version.version_hash
result = Archiver.archive(version.uri, expected_hash: version.version_hash)
version.version_hash = result[:hash]
if result[:url] != version.uri
version.source_metadata['original_url'] = version.uri
version.uri = result[:url]
end
end
elsif !Archiver.already_archived?(version.uri) || !version.version_hash
result = Archiver.archive(version.uri, expected_hash: version.version_hash)
version.version_hash = result[:hash]
if result[:url] != version.uri
version.source_metadata['original_url'] = version.uri
version.uri = result[:url]

if @import.skip_unchanged_versions? && version_changed?(version)
warn "Skipped version identical to previous. URL: #{page.url}, capture_time: #{version.capture_time}, source_type: #{version.source_type}"
return
end
end

if @import.skip_unchanged_versions? && version_changed?(version)
warn "Skipped version identical to previous. URL: #{page.url}, capture_time: #{version.capture_time}, source_type: #{version.source_type}"
return
end
version.validate!
version.update_different_attribute(save: false)
version.save

version.validate!
version.update_different_attribute(save: false)
version.save
@added << version unless existing
rescue StandardError
# Delete a newly created page if the version import fails
if page.uuid_previously_changed? && page.versions.empty?
page.maintainers = []
page.destroy!
end

@added << version unless existing
raise
end
end

def version_for_record(record, existing_version = nil, update_behavior = 'replace')
Expand Down
36 changes: 36 additions & 0 deletions test/jobs/import_versions_job_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,42 @@ def teardown
assert_equal(original_data['source_metadata'], versions(:page1_v1).source_metadata, 'source_metadata was changed')
end

test 'does not add a page if there is an error processing versions' do
bad_page_url = 'http://badpage.com'

import = Import.create_with_data(
{
user: users(:alice)
},
[
{
page_url: bad_page_url,
page_title: 'Bad Page',
site_agency: 'Bad Agency',
site_name: 'Bad Administration',
capture_time: 5.days.ago,
uri: 'https://test-bucket.s3.amazonaws.com/example-v1',
version_hash: 'INVALID_HASH',
source_type: 'versionista',
source_metadata: { test_meta: 'data' }
}
].map(&:to_json).join("\n")
)

archive_stub = proc do |_url|
raise StandardError
end

Archiver.stub :already_archived?, false do
Archiver.stub :archive, archive_stub do
ImportVersionsJob.perform_now(import)

imported_page = Page.find_by(url: bad_page_url)
assert_nil imported_page, 'Should not import page with bad versions'
end
end
end

test 'replaces an existing version if requested' do
page_versions_count = pages(:home_page).versions.count

Expand Down
1 change: 1 addition & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
ENV['RAILS_ENV'] ||= 'test'
require File.expand_path('../config/environment', __dir__)
require 'rails/test_help'
require 'minitest/autorun'
require 'webmock/minitest'

class ActiveSupport::TestCase
Expand Down

0 comments on commit 20ed3a0

Please sign in to comment.