diff options
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 12 | ||||
| -rw-r--r-- | app/models/incoming_message.rb | 20 | ||||
| -rw-r--r-- | app/models/info_request.rb | 9 | ||||
| -rw-r--r-- | app/models/raw_email.rb | 13 | ||||
| -rw-r--r-- | db/migrate/067_factor_out_raw_email.rb | 38 | ||||
| -rw-r--r-- | db/schema.rb | 8 | ||||
| -rw-r--r-- | spec/controllers/general_controller_spec.rb | 2 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 16 | ||||
| -rw-r--r-- | spec/controllers/track_controller_spec.rb | 6 | ||||
| -rw-r--r-- | spec/controllers/user_controller_spec.rb | 2 | ||||
| -rw-r--r-- | spec/fixtures/incoming_messages.yml | 17 | ||||
| -rw-r--r-- | spec/fixtures/raw_emails.yml | 19 | ||||
| -rw-r--r-- | spec/models/incoming_message_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/request_mailer_spec.rb | 2 | ||||
| -rw-r--r-- | vendor/plugins/attr_lazy/init.rb | 9 | ||||
| -rw-r--r-- | vendor/plugins/attr_lazy/lib/attr_lazy.rb | 94 |
16 files changed, 114 insertions, 155 deletions
diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index dae07aa8a..16e1cc621 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: admin_request_controller.rb,v 1.18 2008-09-13 18:01:27 francis Exp $ +# $Id: admin_request_controller.rb,v 1.19 2008-09-22 22:08:43 francis Exp $ class AdminRequestController < ApplicationController layout "admin" @@ -127,12 +127,12 @@ class AdminRequestController < ApplicationController @incoming_message = IncomingMessage.find(params[:incoming_message_id]) @info_request = @incoming_message.info_request - raw_data = @incoming_message + deleted_incoming_message = @incoming_message incoming_message_id = @incoming_message.id @incoming_message.fully_destroy @incoming_message.info_request.log_event("destroy_incoming", - { :editor => admin_http_auth_user(), :raw_data => raw_data }) + { :editor => admin_http_auth_user(), :deleted_incoming_message => deleted_incoming_message }) flash[:notice] = 'Incoming message successfully destroyed.' redirect_to request_admin_url(@info_request) @@ -152,10 +152,10 @@ class AdminRequestController < ApplicationController redirect_to request_admin_url(incoming_message.info_request) end - raw_email = incoming_message.raw_data - mail = TMail::Mail.parse(raw_email) + raw_email_data = incoming_message.raw_email.data + mail = TMail::Mail.parse(raw_email_data) mail.base64_decode - destination_request.receive(mail, raw_email) + destination_request.receive(mail, raw_email_data) incoming_message.fully_destroy diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 0a4453a85..54e7b4f7f 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -19,7 +19,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: incoming_message.rb,v 1.147 2008-09-22 14:22:30 francis Exp $ +# $Id: incoming_message.rb,v 1.148 2008-09-22 22:08:44 francis Exp $ # TODO # Move some of the (e.g. quoting) functions here into rblib, as they feel @@ -167,26 +167,23 @@ class IncomingMessage < ActiveRecord::Base belongs_to :info_request validates_presence_of :info_request - validates_presence_of :raw_data + validates_presence_of :raw_email has_many :outgoing_message_followups, :foreign_key => 'incoming_message_followup_id', :class_name => 'OutgoingMessage' has_many :info_request_events # never really has many, but could in theory - # Some emails are large (10Mb), making things like search results and the - # front page list of requests slow to display as the data is transferred from - # the database. - attr_lazy :raw_data - + belongs_to :raw_email + # Return the structured TMail::Mail object # Documentation at http://i.loveruby.net/en/projects/tmail/doc/ def mail - if @mail.nil? && !self.raw_data.nil? + if @mail.nil? && !self.raw_email.nil? # Hack round bug in TMail's MIME decoding. Example request which provokes it: # http://www.whatdotheyknow.com/request/reviews_of_unduly_lenient_senten#incoming-4830 # Report of TMail bug: # http://rubyforge.org/tracker/index.php?func=detail&aid=21810&group_id=4512&atid=17370 - copy_of_raw_data = self.raw_data.gsub(/; boundary=\s+"/ims,'; boundary="') + copy_of_raw_data = self.raw_email.data.gsub(/; boundary=\s+"/ims,'; boundary="') @mail = TMail::Mail.parse(copy_of_raw_data) @mail.base64_decode @@ -484,7 +481,7 @@ class IncomingMessage < ActiveRecord::Base # Returns body text from main text part of email, converted to UTF-8, with uudecode removed def get_main_body_text - # Cached as loading raw_data can be quite huge, and need this for just + # Cached as loading raw_email can be quite huge, and need this for just # search results if self.cached_main_body_text.nil? text = self.get_main_body_text_internal @@ -848,6 +845,9 @@ class IncomingMessage < ActiveRecord::Base info_request_event.track_things_sent_emails.each { |a| a.destroy } info_request_event.user_info_request_sent_alerts.each { |a| a.destroy } info_request_event.destroy + if !self.raw_email.nil? + self.raw_email.destroy + end self.destroy end end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 67faa312a..4e1c24980 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -23,7 +23,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: info_request.rb,v 1.140 2008-09-22 14:22:30 francis Exp $ +# $Id: info_request.rb,v 1.141 2008-09-22 22:08:44 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -237,7 +237,7 @@ public end # A new incoming email to this request - def receive(email, raw_email, override_stop_new_responses = false) + def receive(email, raw_email_data, override_stop_new_responses = false) # See if new responses are prevented for spam reasons if self.stop_new_responses && !override_stop_new_responses RequestMailer.deliver_stopped_responses(self, email) @@ -248,8 +248,11 @@ public incoming_message = IncomingMessage.new ActiveRecord::Base.transaction do - incoming_message.raw_data = raw_email + 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! self.awaiting_description = true diff --git a/app/models/raw_email.rb b/app/models/raw_email.rb new file mode 100644 index 000000000..992e6c143 --- /dev/null +++ b/app/models/raw_email.rb @@ -0,0 +1,13 @@ +# models/raw_email.rb: +# The fat part of models/incoming_message.rb +# +# Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. +# Email: francis@mysociety.org; WWW: http://www.mysociety.org/ +# +# $Id: raw_email.rb,v 1.1 2008-09-22 22:08:44 francis Exp $ + +class RawEmail < ActiveRecord::Base + has_one :incoming_message +end + + diff --git a/db/migrate/067_factor_out_raw_email.rb b/db/migrate/067_factor_out_raw_email.rb new file mode 100644 index 000000000..be4153938 --- /dev/null +++ b/db/migrate/067_factor_out_raw_email.rb @@ -0,0 +1,38 @@ +class FactorOutRawEmail < ActiveRecord::Migration + def self.up + create_table :raw_emails do |t| + t.column :data, :text, :null => false + end + + add_column :incoming_messages, :raw_email_id, :integer, :null => true + change_column :incoming_messages, :raw_data, :text, :null => true + + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + execute "ALTER TABLE incoming_messages ADD CONSTRAINT fk_incoming_messages_raw_email FOREIGN KEY (raw_email_id) REFERENCES raw_emails(id)" + end + + batch_size = 10 + 0.step(IncomingMessage.count, batch_size) do |i| + IncomingMessage.find(:all, :limit => batch_size, :offset => i, :order => :id).each do |incoming_message| + if incoming_message.raw_email_id.nil? + STDERR.puts "doing incoming_message id " + incoming_message.id.to_s + ActiveRecord::Base.transaction do + raw_email = RawEmail.new + raw_email.data = incoming_message.raw_data + incoming_message.raw_email = raw_email + incoming_message.raw_data = nil + raw_email.save! + incoming_message.save! + end + end + end + end + + change_column :incoming_messages, :raw_email_id, :integer, :null => false + remove_column :incoming_messages, :raw_data + end + + def self.down + raise "down migration not implemented" + end +end diff --git a/db/schema.rb b/db/schema.rb index 496df6f0c..bce1b4d10 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -9,7 +9,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 66) do +ActiveRecord::Schema.define(:version => 67) do create_table "acts_as_xapian_jobs", :force => true do |t| t.string "model", :null => false @@ -31,11 +31,11 @@ ActiveRecord::Schema.define(:version => 66) do create_table "incoming_messages", :force => true do |t| t.integer "info_request_id", :null => false - t.text "raw_data", :null => false t.datetime "created_at", :null => false t.datetime "updated_at", :null => false t.text "cached_attachment_text" t.text "cached_main_body_text" + t.integer "raw_email_id", :null => false end create_table "info_request_events", :force => true do |t| @@ -139,6 +139,10 @@ ActiveRecord::Schema.define(:version => 66) do t.text "notes" end + create_table "raw_emails", :force => true do |t| + t.binary "data", :null => false + end + create_table "track_things", :force => true do |t| t.integer "tracking_user_id", :null => false t.string "track_query", :null => false diff --git a/spec/controllers/general_controller_spec.rb b/spec/controllers/general_controller_spec.rb index 23b7ba1b1..5390468ee 100644 --- a/spec/controllers/general_controller_spec.rb +++ b/spec/controllers/general_controller_spec.rb @@ -7,7 +7,7 @@ end describe GeneralController, "when searching" do integrate_views - fixtures :users, :outgoing_messages, :incoming_messages, :info_requests, :info_request_events, :public_bodies, :comments + fixtures :users, :outgoing_messages, :incoming_messages, :raw_emails, :info_requests, :info_request_events, :public_bodies, :comments before do # XXX - what is proper way to do this only once? diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index a6dcd62e0..45ab739c2 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -30,7 +30,7 @@ end describe RequestController, "when showing one request" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views it "should be successful" do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' @@ -212,7 +212,7 @@ end describe RequestController, "when viewing an individual response for reply/followup" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages, :comments # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views it "should ask for login if you are logged in as wrong person" do session[:user_id] = users(:silly_name_user).id @@ -229,7 +229,7 @@ end describe RequestController, "when classifying an individual response" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages, :comments # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views it "should require login" do post :describe_state, :incoming_message => { :described_state => "rejected" }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :last_info_request_event_id => info_request_events(:useless_incoming_message_event).id, :submitted_describe_state => 1 @@ -275,7 +275,7 @@ end describe RequestController, "when sending a followup message" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views it "should require login" do post :show_response, :outgoing_message => { :body => "What a useless response! You suck." }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1 @@ -331,7 +331,7 @@ end describe RequestController, "sending overdue request alerts" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views it "should send an overdue alert mail to creators of overdue requests" do RequestMailer.alert_overdue_requests @@ -358,7 +358,7 @@ end describe RequestController, "sending unclassified new response reminder alerts" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages, :comments # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views it "should send an alert" do RequestMailer.alert_new_response_reminders @@ -384,7 +384,7 @@ end describe RequestController, "clarification required alerts" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views it "should send an alert" do ir = info_requests(:fancy_dog_request) @@ -417,7 +417,7 @@ end describe RequestController, "comment alerts" do integrate_views - fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages, :comments # all needed as integrating views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views it "should send an alert" do # delete ficture comment and make new one, so is in last month (as diff --git a/spec/controllers/track_controller_spec.rb b/spec/controllers/track_controller_spec.rb index 173ffab7e..68064b908 100644 --- a/spec/controllers/track_controller_spec.rb +++ b/spec/controllers/track_controller_spec.rb @@ -2,7 +2,7 @@ require File.dirname(__FILE__) + '/../spec_helper' describe TrackController, "when making a new track on a request" do integrate_views - fixtures :info_requests, :outgoing_messages, :incoming_messages, :info_request_events, :users + fixtures :info_requests, :outgoing_messages, :incoming_messages, :raw_emails, :info_request_events, :users it "should require login when making new track" do get :track_request, :url_title => info_requests(:fancy_dog_request).url_title, :feed => 'track' @@ -22,7 +22,7 @@ end describe TrackController, "when sending alerts for a track" do integrate_views - fixtures :info_requests, :outgoing_messages, :incoming_messages, :info_request_events, :users, :track_things, :track_things_sent_emails + fixtures :info_requests, :outgoing_messages, :incoming_messages, :raw_emails, :info_request_events, :users, :track_things, :track_things_sent_emails it "should send alerts" do TrackMailer.alert_tracks @@ -58,7 +58,7 @@ end describe TrackController, "when viewing RSS feed for a track" do integrate_views - fixtures :info_requests, :outgoing_messages, :incoming_messages, :info_request_events, :users, :track_things, :comments, :public_bodies + fixtures :info_requests, :outgoing_messages, :incoming_messages, :raw_emails, :info_request_events, :users, :track_things, :comments, :public_bodies it "should get the RSS feed" do track_thing = track_things(:track_fancy_dog_request) diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index a0079ed9e..e883dc62e 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -5,7 +5,7 @@ require File.dirname(__FILE__) + '/../spec_helper' describe UserController, "when showing a user" do integrate_views - fixtures :users, :outgoing_messages, :incoming_messages, :info_requests, :info_request_events + fixtures :users, :outgoing_messages, :incoming_messages, :raw_emails, :info_requests, :info_request_events it "should be successful" do get :show, :url_name => "bob_smith" diff --git a/spec/fixtures/incoming_messages.yml b/spec/fixtures/incoming_messages.yml index 819a54bdc..e15a466ca 100644 --- a/spec/fixtures/incoming_messages.yml +++ b/spec/fixtures/incoming_messages.yml @@ -2,21 +2,6 @@ useless_incoming_message: id: 1 info_request_id: 101 updated_at: 2007-11-13 18:09:20.042061 - raw_data: | - From: FOI Person <foiperson@localhost> - To: Bob Smith <bob@localhost> - Date: Tue, 13 Nov 2007 11:39:55 +0000 - NoCc: foi+request-1-4b571715@cat - Bcc: - Subject: Re: Freedom of Information Request - Why do you have such a fancy dog? - Reply-To: - In-Reply-To: <471f1eae5d1cb_7347..fdbe67386163@cat.tmail> - - No way! I'm not going to tell you that in a month of Thursdays. - - The Geraldine Quango - - On Wed, Oct 24, 2007 at 11:30:06AM +0100, Bob Smith wrote: - > Why do you have such a fancy dog? + raw_email_id: 1 created_at: 2007-11-13 18:09:20.042061 diff --git a/spec/fixtures/raw_emails.yml b/spec/fixtures/raw_emails.yml new file mode 100644 index 000000000..6e43b1a9a --- /dev/null +++ b/spec/fixtures/raw_emails.yml @@ -0,0 +1,19 @@ +useless_raw_email: + id: 1 + data: | + From: FOI Person <foiperson@localhost> + To: Bob Smith <bob@localhost> + Date: Tue, 13 Nov 2007 11:39:55 +0000 + NoCc: foi+request-1-4b571715@cat + Bcc: + Subject: Re: Freedom of Information Request - Why do you have such a fancy dog? + Reply-To: + In-Reply-To: <471f1eae5d1cb_7347..fdbe67386163@cat.tmail> + + No way! I'm not going to tell you that in a month of Thursdays. + + The Geraldine Quango + + On Wed, Oct 24, 2007 at 11:30:06AM +0100, Bob Smith wrote: + > Why do you have such a fancy dog? + diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 55c042141..63a1680c4 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -1,7 +1,7 @@ require File.dirname(__FILE__) + '/../spec_helper' describe IncomingMessage, " when dealing with incoming mail" do - fixtures :incoming_messages + fixtures :incoming_messages, :raw_emails before do @im = incoming_messages(:useless_incoming_message) diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index 2a882b7c7..d7baf0719 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -1,7 +1,7 @@ require File.dirname(__FILE__) + '/../spec_helper' describe RequestMailer, " when receiving incoming mail" do - fixtures :info_requests, :incoming_messages, :users, :public_bodies + fixtures :info_requests, :incoming_messages, :raw_emails, :users, :public_bodies before do diff --git a/vendor/plugins/attr_lazy/init.rb b/vendor/plugins/attr_lazy/init.rb deleted file mode 100644 index 7bd84b894..000000000 --- a/vendor/plugins/attr_lazy/init.rb +++ /dev/null @@ -1,9 +0,0 @@ -# attr_lazy/init.rb: -# -# Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. -# Email: francis@mysociety.org; WWW: http://www.mysociety.org/ -# -# $Id: init.rb,v 1.1 2008-07-17 03:30:55 francis Exp $ - -require 'attr_lazy' - diff --git a/vendor/plugins/attr_lazy/lib/attr_lazy.rb b/vendor/plugins/attr_lazy/lib/attr_lazy.rb deleted file mode 100644 index eaa700ff9..000000000 --- a/vendor/plugins/attr_lazy/lib/attr_lazy.rb +++ /dev/null @@ -1,94 +0,0 @@ -# Taken from here, initial code at top (the refactoring doesn't work). -# http://refactormycode.com/codes/219-activerecord-lazy-attribute-loading-plugin-for-rails - -module AttrLazy - - def self.included(base_class) - base_class.extend(ClassMethods) - end - - module ClassMethods - def attr_lazy(*parameters) - columns = parameters - cattr_accessor :attr_lazy_configuration - self.attr_lazy_configuration = { - :columns => columns - } - - # prevent barfing if the plugin is reloaded/loaded twice - unless self.respond_to? :column_names_for_join_base_without_attr_lazy - self.extend AttrLazy::SingletonMethods - class << self - alias_method_chain :construct_finder_sql, :attr_lazy - alias_method_chain :column_names_for_join_base, :attr_lazy - end - end - - include AttrLazy::InstanceMethods - columns.each do |col| - class_eval("def #{col}; read_lazy_attribute :#{col}; end", __FILE__, __LINE__) - end - - end - - def column_names_for_join_base - column_names - end - - def column_names_for_join_base_with_attr_lazy - @column_names_for_join_base ||= columns.collect{|c| - c.name unless attr_lazy_configuration[:columns].include?(c.name.to_sym) - }.compact - end - end - - module SingletonMethods - - def construct_finder_sql_with_attr_lazy(options) - options = {:select => unlazy_column_list}.merge(options) - construct_finder_sql_without_attr_lazy(options) - end - - def unlazy_column_list - @unlazy ||= columns.collect do |c| - "#{quoted_table_name}.#{connection.quote_column_name(c.name)}" unless attr_lazy_columns.include?(c.name.to_sym) - end.compact.join ',' - end - - def attr_lazy_columns - attr_lazy_configuration[:columns] - end - - end - - module InstanceMethods - - def read_lazy_attribute(att) - @lazy_attribute_values ||= {} - if attribute_names.include?(att.to_s) - read_attribute att - else - @lazy_attribute_values[att] ||= self.class.find(id, :select => att)[att] - end - end - - end - -end - -class ActiveRecord::Associations::ClassMethods::JoinDependency::JoinBase - def column_names_with_alias - unless @column_names_with_alias - @column_names_with_alias = [] - ([active_record.primary_key] + (active_record.column_names_for_join_base - [active_record.primary_key])).each_with_index do |column_name, i| - @column_names_with_alias << [column_name, "#{ aliased_prefix }_r#{ i }"] - end - end - return @column_names_with_alias - end -end - -ActiveRecord::Base.send :include, AttrLazy - - - |