Skip to content

Commit

Permalink
Remove depreciated code
Browse files Browse the repository at this point in the history
  • Loading branch information
@gbp
gbp committed Dec 22, 2022
1 parent efe8d52 commit b8fceb2
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 179 deletions.
74 changes: 15 additions & 59 deletions app/models/foi_attachment.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,41 +49,9 @@ class FoiAttachment < ApplicationRecord
BODY_MAX_TRIES = 3
BODY_MAX_DELAY = 5

def directory
if file.attached?
warn <<~DEPRECATION.squish
[DEPRECATION] FoiAttachment#directory shouldn't be used when using
`ActiveStorage` backed file stores. This method will be removed
in 0.42.
DEPRECATION
return
end

base_dir = File.expand_path(File.join(File.dirname(__FILE__), "../../cache", "attachments_#{Rails.env}"))
return File.join(base_dir, self.hexdigest[0..2])
end

def filepath
if file.attached?
warn <<~DEPRECATION.squish
[DEPRECATION] FoiAttachment#filepath shouldn't be used when using
`ActiveStorage` backed file stores. This method will be removed
in 0.42.
DEPRECATION
return
end

File.join(self.directory, self.hexdigest)
end

def delete_cached_file!
@cached_body = nil

if file.attached?
file.purge
elsif File.exist?(filepath)
File.delete(filepath)
end
file.purge if file.attached?
end

def body=(d)
Expand All @@ -101,33 +69,21 @@ def body=(d)
end

# raw body, encoded as binary
def body
if @cached_body.nil?
tries = 0
delay = 1
begin
if file.attached?
@cached_body = file.download
else
@cached_body = File.open(filepath, "rb" ) { |file| file.read }
end
rescue Errno::ENOENT, ActiveStorage::FileNotFoundError
# we've lost our cached attachments for some reason. Reparse them.
if tries > BODY_MAX_TRIES
raise
else
sleep delay
end
tries += 1
delay *= 2
delay = BODY_MAX_DELAY if delay > BODY_MAX_DELAY
force = true
self.incoming_message.parse_raw_email!(force)
reload
retry
end
def body(tries: 0, delay: 1)
return @cached_body if @cached_body

if file.attached?
@cached_body = file.download
else
# we've lost our cached attachments for some reason. Reparse them.
raise if tries > BODY_MAX_TRIES
sleep [delay, BODY_MAX_DELAY].min

self.incoming_message.parse_raw_email!(true)
reload

body(tries: tries + 1, delay: delay * 2)
end
return @cached_body
end

# body as UTF-8 text, with scrubbing of invalid chars if needed
Expand Down
49 changes: 2 additions & 47 deletions app/models/raw_email.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,45 +69,6 @@ def empty_from_field?
mail.from_addrs.nil? || mail.from_addrs.size == 0
end

def directory
if file.attached?
warn <<~DEPRECATION.squish
[DEPRECATION] RawEmail#directory shouldn't be used when using
`ActiveStorage` backed file stores. This method will be removed
in 0.42.
DEPRECATION
return
end

if request_id.empty?
raise "Failed to find the id number of the associated request: has it been saved?"
end

if Rails.env.test?
File.join(Rails.root, 'files/raw_email_test')
else
File.join(AlaveteliConfiguration::raw_emails_location,
request_id[0..2], request_id)
end
end

def filepath
if file.attached?
warn <<~DEPRECATION.squish
[DEPRECATION] RawEmail#filepath shouldn't be used when using
`ActiveStorage` backed file stores. This method will be removed
in 0.42.
DEPRECATION
return
end

if incoming_message_id.empty?
raise "Failed to find the id number of the associated incoming message: has it been saved?"
end

File.join(directory, incoming_message_id)
end

def mail
@mail ||= mail!
end
Expand All @@ -126,9 +87,7 @@ def data=(d)
end

def data
return @data ||= file.download if file.attached?

File.open(filepath, "rb").read
@data ||= file.download if file.attached?
end

def data_as_text
Expand Down Expand Up @@ -172,10 +131,6 @@ def incoming_message_id
end

def destroy_file_representation!
if file.attached?
file.purge
elsif File.exist?(filepath)
File.delete(filepath)
end
file.purge if file.attached?
end
end
10 changes: 0 additions & 10 deletions lib/configuration.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,16 +134,6 @@ module AlaveteliConfiguration
# rubocop:enable Layout/LineLength
end

def self.raw_emails_location
warn <<~DEPRECATION.squish
[DEPRECATION] AlaveteliConfiguration.raw_emails_location will be removed
in 0.42. You should have `ActiveStorage` configured but you still haven't
migrated all your old files. Please run `bin/rails storage:migrate` to
migrate
DEPRECATION
super
end

def self.method_missing(name)
key = name.to_s.upcase
if DEFAULTS.has_key?(key.to_sym)
Expand Down
79 changes: 16 additions & 63 deletions spec/models/raw_email_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,6 @@

RSpec.describe RawEmail do

# DEPRECATION: remove for release 0.42
class LegacyRawEmail < RawEmail
# This replicates how we used to store RawEmail before switching to
# ActiveStorage
def data=(d)
FileUtils.mkdir_p(directory) unless File.exist?(directory)
File.atomic_write(filepath) do |file|
file.binmode
file.write(d)
end
end
end

let(:legacy_raw_email) do
LegacyRawEmail.create!(
incoming_message: FactoryBot.create(:incoming_message),
data: 'Hello world'
)
end

def roundtrip_data(raw_email, data)
raw_email.data = data
raw_email.save!
Expand All @@ -40,28 +20,13 @@ def roundtrip_data(raw_email, data)
end

describe 'before destroy callbacks' do
shared_examples :destory_file_examples do
it 'should delete the directory' do
raw_email.run_callbacks(:destroy)
expect(File.exist?(raw_email.filepath)).to eq(false)
end

it 'should only delete the directory if it exists' do
expect(File).to receive(:delete).once.and_call_original
raw_email.run_callbacks(:destroy)
expect { raw_email.run_callbacks(:destroy) }.
not_to raise_error
end
end

context 'with active storage' do
let(:raw_email) { FactoryBot.create(:incoming_message).raw_email }
include_examples :destory_file_examples
end
let(:raw_email) { FactoryBot.create(:incoming_message).raw_email }

context 'without active storage' do
let(:raw_email) { legacy_raw_email }
include_examples :destory_file_examples
it 'should only delete the directory if it exists' do
expect(File).to receive(:delete).once.and_call_original
raw_email.run_callbacks(:destroy)
expect { raw_email.run_callbacks(:destroy) }.
not_to raise_error
end
end

Expand Down Expand Up @@ -219,33 +184,21 @@ def test_real(fixture_file, expected)
end

describe '#data' do
let(:raw_email) { FactoryBot.create(:incoming_message).raw_email }

shared_examples :data_unchanged_examples do
it 'roundtrips data unchanged' do
data = roundtrip_data(raw_email, "Hello, world!")
expect(data).to eq("Hello, world!")
end

it 'returns an unchanged binary string with a valid encoding if the data is non-ascii and non-utf-8' do
data = roundtrip_data(raw_email, "\xA0")

expect(data.encoding.to_s).to eq('ASCII-8BIT')
expect(data.valid_encoding?).to be true
data = data.force_encoding('UTF-8')
expect(data).to eq("\xA0")
end
it 'roundtrips data unchanged' do
data = roundtrip_data(raw_email, "Hello, world!")
expect(data).to eq("Hello, world!")
end

context 'with active storage' do
let(:raw_email) { FactoryBot.create(:incoming_message).raw_email }
include_examples :data_unchanged_examples
end
it 'returns an unchanged binary string with a valid encoding if the data is non-ascii and non-utf-8' do
data = roundtrip_data(raw_email, "\xA0")

context 'without active storage' do
let(:raw_email) { legacy_raw_email }
include_examples :data_unchanged_examples
expect(data.encoding.to_s).to eq('ASCII-8BIT')
expect(data.valid_encoding?).to be true
data = data.force_encoding('UTF-8')
expect(data).to eq("\xA0")
end

end

describe '#data_as_text' do
Expand Down

0 comments on commit b8fceb2

Please sign in to comment.