aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--.gitignore1
-rw-r--r--app/controllers/application_controller.rb17
-rw-r--r--app/controllers/request_controller.rb2
-rw-r--r--app/models/info_request.rb15
-rw-r--r--app/models/post_redirect.rb2
-rw-r--r--app/models/raw_email.rb36
-rw-r--r--app/models/track_mailer.rb6
-rw-r--r--config/general.yml-example10
-rw-r--r--db/migrate/099_move_raw_email_to_filesystem.rb23
-rw-r--r--db/migrate/100_remove_redundant_raw_email_columns.rb14
-rwxr-xr-xscript/mailin2
-rw-r--r--spec/models/info_request_spec.rb2
-rw-r--r--spec/models/outgoing_mailer_spec.rb31
-rw-r--r--spec/models/raw_email_spec.rb4
-rw-r--r--spec/spec_helper.rb4
15 files changed, 143 insertions, 26 deletions
diff --git a/.gitignore b/.gitignore
index 8fabc641a..e8317395f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,3 +14,4 @@ moo.txt
TAGS
/vendor/plugins/*theme
/locale/model_attributes.rb
+/files/ \ No newline at end of file
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 405327952..fe9ed7ec7 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -46,12 +46,17 @@ class ApplicationController < ActionController::Base
# egrep "CONSUME MEMORY: [0-9]{7} KB" production.log
around_filter :record_memory
def record_memory
- File.read("/proc/#{Process.pid}/status").match(/VmRSS:\s+(\d+)/)
- rss_before_action = $1.to_i
- yield
- File.read("/proc/#{Process.pid}/status").match(/VmRSS:\s+(\d+)/)
- rss_after_action = $1.to_i
- logger.info("PID: #{Process.pid}\tCONSUME MEMORY: #{rss_after_action - rss_before_action} KB\tNow: #{rss_after_action} KB\t#{request.url}")
+ record_memory = MySociety::Config.get('DEBUG_RECORD_MEMORY', false)
+ if record_memory
+ File.read("/proc/#{Process.pid}/status").match(/VmRSS:\s+(\d+)/)
+ rss_before_action = $1.to_i
+ yield
+ File.read("/proc/#{Process.pid}/status").match(/VmRSS:\s+(\d+)/)
+ rss_after_action = $1.to_i
+ logger.info("PID: #{Process.pid}\tCONSUME MEMORY: #{rss_after_action - rss_before_action} KB\tNow: #{rss_after_action} KB\t#{request.url}")
+ else
+ yield
+ end
end
# Set cookie expiry according to "remember me" checkbox, as per "An easier
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 441ddc417..81dffaa80 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -19,7 +19,7 @@ class RequestController < ApplicationController
include RequestControllerCustomStates
@@custom_states_loaded = true
end
- rescue NameError
+ rescue MissingSourceFile
end
def show
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index dcef9e5b5..e6a520fd2 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -112,7 +112,7 @@ class InfoRequest < ActiveRecord::Base
include InfoRequestCustomStates
@@custom_states_loaded = true
end
- rescue NameError
+ rescue MissingSourceFile
end
# only check on create, so existing models with mixed case are allowed
@@ -347,14 +347,7 @@ public
# XXX this *should* also check outgoing message joined to is an initial
# request (rather than follow up)
def InfoRequest.find_by_existing_request(title, public_body_id, body)
- # XXX can add other databases here which have regexp_replace
- if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
- # Exclude spaces from the body comparison using regexp_replace
- return InfoRequest.find(:first, :conditions => [ "title = ? and public_body_id = ? and regexp_replace(outgoing_messages.body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", title, public_body_id, body ], :include => [ :outgoing_messages ] )
- else
- # For other databases (e.g. SQLite) not the end of the world being space-sensitive for this check
- return InfoRequest.find(:first, :conditions => [ "title = ? and public_body_id = ? and outgoing_messages.body = ?", title, public_body_id, body ], :include => [ :outgoing_messages ] )
- end
+ return InfoRequest.find(:first, :conditions => [ "title = ? and public_body_id = ? and outgoing_messages.body = ?", title, public_body_id, body ], :include => [ :outgoing_messages ] )
end
def find_existing_outgoing_message(body)
@@ -435,11 +428,11 @@ public
ActiveRecord::Base.transaction do
raw_email = RawEmail.new
- raw_email.data = raw_email_data
incoming_message.raw_email = raw_email
incoming_message.info_request = self
- raw_email.save!
incoming_message.save!
+ raw_email.data = raw_email_data
+ raw_email.save!
self.awaiting_description = true
self.log_event("response", { :incoming_message_id => incoming_message.id })
diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb
index 1ea3f1543..b111d019d 100644
--- a/app/models/post_redirect.rb
+++ b/app/models/post_redirect.rb
@@ -88,7 +88,7 @@ class PostRedirect < ActiveRecord::Base
# Called from cron job delete-old-things
def self.delete_old_post_redirects
- PostRedirect.delete_all "updated_at < (now() - interval '6 months')"
+ PostRedirect.delete_all "updated_at < (now() - interval '2 months')"
end
end
diff --git a/app/models/raw_email.rb b/app/models/raw_email.rb
index 0b70d1786..4e1a69456 100644
--- a/app/models/raw_email.rb
+++ b/app/models/raw_email.rb
@@ -6,7 +6,6 @@
# id :integer not null, primary key
# data_text :text
# data_binary :binary
-#
# models/raw_email.rb:
# The fat part of models/incoming_message.rb
@@ -21,17 +20,50 @@ class RawEmail < ActiveRecord::Base
has_one :incoming_message
+ before_destroy :destroy_file_representation!
# We keep the old data_text field (which is of type text) for backwards
# compatibility. We use the new data_binary field because only it works
# properly in recent versions of PostgreSQL (get seg faults escaping
# some binary strings).
+ def directory
+ request_id = self.incoming_message.info_request.id.to_s
+ File.join(MySociety::Config.get('RAW_EMAILS_LOCATION',
+ 'files/raw_emails'),
+ request_id[0..2], request_id)
+ end
+
+ def filepath
+ File.join(self.directory, self.incoming_message.id.to_s)
+ end
+
def data=(d)
- write_attribute(:data_binary, d)
+ if !File.exists?(self.directory)
+ FileUtils.mkdir_p self.directory
+ end
+ File.open(self.filepath, "wb") { |file|
+ file.write d
+ }
end
def data
+ if !File.exists?(self.filepath)
+ dbdata
+ else
+ File.open(self.filepath, "rb" ).read
+ end
+ end
+
+ def destroy_file_representation!
+ File.delete(self.filepath)
+ end
+
+ def dbdata=(d)
+ write_attribute(:data_binary, d)
+ end
+
+ def dbdata
d = read_attribute(:data_binary)
if !d.nil?
return d
diff --git a/app/models/track_mailer.rb b/app/models/track_mailer.rb
index 1b37709b9..6901a834d 100644
--- a/app/models/track_mailer.rb
+++ b/app/models/track_mailer.rb
@@ -26,7 +26,11 @@ class TrackMailer < ApplicationMailer
@body = { :user => user, :email_about_things => email_about_things, :unsubscribe_url => unsubscribe_url }
end
- # Send email alerts for tracked things.
+ # Send email alerts for tracked things. Never more than one email
+ # a day, nor about events which are more than a week old, nor
+ # events about which emails have been sent within the last two
+ # weeks.
+
# Useful query to run by hand to see how many alerts are due:
# User.find(:all, :conditions => [ "last_daily_track_email < ?", Time.now - 1.day ]).size
def self.alert_tracks
diff --git a/config/general.yml-example b/config/general.yml-example
index 8a4a0442e..fb2afd336 100644
--- a/config/general.yml-example
+++ b/config/general.yml-example
@@ -56,6 +56,10 @@ ADMIN_PASSWORD: 'passwordx'
CONTACT_EMAIL: 'postmaster@localhost'
CONTACT_NAME: 'Alaveteli Webmaster'
+# Where the raw incoming email data gets stored; make sure you back
+# this up!
+RAW_EMAILS_LOCATION: 'files/raw_emails'
+
# The base URL for admin pages.
# If not specified, it will default to the path to the admin controller,
# which is usually what you want. It is useful in situations where admin
@@ -83,3 +87,9 @@ STAGING_SITE: 1
RECAPTCHA_PUBLIC_KEY: 'x'
RECAPTCHA_PRIVATE_KEY: 'x'
+# For debugging memory problems. If true, the app logs
+# the memory use increase of the Ruby process due to the
+# request (Linux only). Since Ruby never returns memory to the OS, if the
+# existing process previously served a larger request, this won't
+# show any consumption for the later request.
+DEBUG_RECORD_MEMORY: false \ No newline at end of file
diff --git a/db/migrate/099_move_raw_email_to_filesystem.rb b/db/migrate/099_move_raw_email_to_filesystem.rb
new file mode 100644
index 000000000..ea77580e1
--- /dev/null
+++ b/db/migrate/099_move_raw_email_to_filesystem.rb
@@ -0,0 +1,23 @@
+class MoveRawEmailToFilesystem < ActiveRecord::Migration
+ def self.up
+ batch_size = 10
+ 0.step(RawEmail.count, batch_size) do |i|
+ RawEmail.find(:all, :limit => batch_size, :offset => i, :order => :id).each do |raw_email|
+ if !File.exists?(raw_email.filepath)
+ STDERR.puts "converting raw_email " + raw_email.id.to_s
+ raw_email.data = raw_email.dbdata
+ #raw_email.dbdata = nil
+ #raw_email.save!
+ end
+ end
+ end
+ end
+
+ def self.down
+ raise "safer not to have reverse migration scripts, and we never use them"
+ end
+end
+
+
+
+
diff --git a/db/migrate/100_remove_redundant_raw_email_columns.rb b/db/migrate/100_remove_redundant_raw_email_columns.rb
new file mode 100644
index 000000000..b8f9454f6
--- /dev/null
+++ b/db/migrate/100_remove_redundant_raw_email_columns.rb
@@ -0,0 +1,14 @@
+class RemoveRedundantRawEmailColumns < ActiveRecord::Migration
+ def self.up
+ ActiveRecord::Base.connection.execute("ALTER TABLE raw_emails DROP COLUMN data_text")
+ ActiveRecord::Base.connection.execute("ALTER TABLE raw_emails DROP COLUMN data_binary")
+ end
+
+ def self.down
+ raise "safer not to have reverse migration scripts, and we never use them"
+ end
+end
+
+
+
+
diff --git a/script/mailin b/script/mailin
index 738034c26..733eaff3d 100755
--- a/script/mailin
+++ b/script/mailin
@@ -22,7 +22,7 @@ then
# send error to administators (use mutt for MIME encoding)
SUBJ="Mail import error for $OPTION_DOMAIN"
BODY="There was an error code $ERROR_CODE returned by the RequestMailer.receive command in script/mailin. See attached for details. This might be quite serious, such as the database was down, or might be an email with corrupt headers that Rails is choking on. The email was returned with an exit code 75, which for Exim at least means the MTA will try again later. A well configured installation of this code will separately have had Exim make a backup copy of the email in a separate mailbox, just in case."
- echo "$BODY" | /usr/bin/mutt -s "$SUBJ" -a $OUTPUT -a $INPUT $OPTION_CONTACT_EMAIL
+ echo "$BODY" | /usr/bin/mutt -s "$SUBJ" -a $OUTPUT $INPUT -- $OPTION_CONTACT_EMAIL
# tell exim error was temporary, so try again later (no point bouncing message to authority)
rm -f $OUTPUT
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index 549296149..d0b0e0e32 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -40,7 +40,7 @@ describe InfoRequest do
describe " when emailing" do
- fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :comments
+ fixtures :info_requests, :info_request_events, :outgoing_messages, :public_bodies, :public_body_translations, :users, :comments
before do
@info_request = info_requests(:fancy_dog_request)
diff --git a/spec/models/outgoing_mailer_spec.rb b/spec/models/outgoing_mailer_spec.rb
index 3073c5ffd..83da7a553 100644
--- a/spec/models/outgoing_mailer_spec.rb
+++ b/spec/models/outgoing_mailer_spec.rb
@@ -6,6 +6,23 @@ describe OutgoingMailer, " when working out follow up addresses" do
# mocks. Put parts of the tests in spec/lib/tmail_extensions.rb
fixtures :info_requests, :incoming_messages, :raw_emails, :public_bodies, :public_body_translations
+ before do
+ # XXX this is a hack around the fact that our raw_email model
+ # is in transition to something that doesn't actually live in
+ # the database at all. The raw_email fixture saves to the
+ # model, the model then needs to be told to save itself on the
+ # filesystem.
+ raw_email = raw_emails(:useless_raw_email)
+ raw_email.data=raw_email.dbdata
+ end
+
+ after do
+ # And this is a hack around the fact that Rails fixtures don't
+ # have teardowns happen on them; we need to ensure no emails
+ # are left lying around
+ raw_emails(:useless_raw_email).destroy_file_representation!
+ end
+
it "should parse them right" do
ir = info_requests(:fancy_dog_request)
im = ir.incoming_messages[0]
@@ -67,8 +84,17 @@ describe OutgoingMailer, " when working out follow up addresses" do
end
describe OutgoingMailer, "when working out follow up subjects" do
- fixtures :info_requests, :incoming_messages, :outgoing_messages
-
+ fixtures :info_requests, :incoming_messages, :outgoing_messages, :raw_emails
+
+ before do
+ raw_email = raw_emails(:useless_raw_email)
+ raw_email.data=raw_email.dbdata
+ end
+
+ after do
+ raw_emails(:useless_raw_email).destroy_file_representation!
+ end
+
it "should prefix the title with 'Freedom of Information request -' for initial requests" do
ir = info_requests(:fancy_dog_request)
im = ir.incoming_messages[0]
@@ -105,6 +131,7 @@ describe OutgoingMailer, "when working out follow up subjects" do
it "should not add Re: prefix if there already is a lower case re: prefix" do
ir = info_requests(:fancy_dog_request)
im = ir.incoming_messages[0]
+ puts im.raw_email.data
om = outgoing_messages(:useless_outgoing_message)
om.incoming_message_followup = im
diff --git a/spec/models/raw_email_spec.rb b/spec/models/raw_email_spec.rb
index 6f3a8acd6..ff2830a62 100644
--- a/spec/models/raw_email_spec.rb
+++ b/spec/models/raw_email_spec.rb
@@ -3,6 +3,10 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
describe User, "manipulating a raw email" do
before do
@raw_email = RawEmail.new
+ incoming_message = mock_model(IncomingMessage)
+ info_request = mock_model(InfoRequest)
+ incoming_message.stub!(:info_request).and_return(info_request)
+ @raw_email.stub!(:incoming_message).and_return(incoming_message)
end
it 'putting data in comes back out' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 5e6d9ec4a..bbcc9aa23 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -13,6 +13,10 @@ config['ADMIN_PASSWORD'] = 'baz'
# tests assume 20 days
config['REPLY_LATE_AFTER_DAYS'] = 20
+# tests assume 20 days
+config['RAW_EMAILS_LOCATION'] = 'files/raw_emails_tests'
+
+
# Uncomment the next line to use webrat's matchers
#require 'webrat/integrations/rspec-rails'