From 40453cc3539b4c689aea482959a7f126727dfec8 Mon Sep 17 00:00:00 2001 From: Karol Hosiawa Date: Wed, 6 Apr 2011 15:43:25 +0200 Subject: [PATCH] moved Notices to a separate collection --- .gitignore | 1 + Gemfile | 3 +- Gemfile.lock | 30 ++++--- README.md | 11 ++- app/controllers/apps_controller.rb | 26 +++--- app/controllers/errs_controller.rb | 26 +++--- app/models/app.rb | 18 ++--- app/models/err.rb | 26 +++--- app/models/notice.rb | 81 ++++++++++++++----- app/views/apps/index.html.haml | 2 +- app/views/apps/show.html.haml | 2 +- app/views/errs/_table.html.haml | 2 +- app/views/errs/lighthouseapp_body.txt.erb | 2 +- app/views/errs/redmine_body.txt.erb | 2 +- app/views/errs/show.html.haml | 10 +-- app/views/mailer/err_notification.text.erb | 6 +- app/views/notices/_atom_entry.html.haml | 4 +- app/views/notices/_environment.html.haml | 2 +- app/views/notices/_summary.html.haml | 2 +- ...027_move_notices_to_separate_collection.rb | 22 +++++ lib/hoptoad.rb | 10 +-- lib/recurse.rb | 24 ++++++ lib/tasks/errbit/err_message.rake | 12 +++ lib/tasks/errbit/notices_counter.rake | 12 +++ spec/controllers/errs_controller_spec.rb | 50 ++++++------ spec/models/err_spec.rb | 71 ++++++++++++---- spec/models/notice_spec.rb | 52 +++++++----- 27 files changed, 342 insertions(+), 167 deletions(-) create mode 100644 db/migrate/20110422152027_move_notices_to_separate_collection.rb create mode 100644 lib/recurse.rb create mode 100644 lib/tasks/errbit/err_message.rake create mode 100644 lib/tasks/errbit/notices_counter.rake diff --git a/.gitignore b/.gitignore index 6137e7550..f976d0d4a 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ config/deploy.rb config/mongoid.yml .rvmrc *~ +*.rbc diff --git a/Gemfile b/Gemfile index f0f9cc63a..6b5d1b0cd 100644 --- a/Gemfile +++ b/Gemfile @@ -2,12 +2,13 @@ source 'http://rubygems.org' gem 'rails', '3.0.5' gem 'nokogiri' -gem 'mongoid', '~> 2.0.0.rc.7' +gem 'mongoid', '2.0.0.rc.8' gem 'haml' gem 'will_paginate' gem 'devise', '~> 1.1.8' gem 'lighthouse-api' gem 'redmine_client', :git => "git://github.com/oruen/redmine_client.git" +gem 'mongoid_rails_migrations' platform :ruby do gem 'bson_ext', '~> 1.2' diff --git a/Gemfile.lock b/Gemfile.lock index 69b33db00..4ecb153b7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -35,15 +35,15 @@ GEM activemodel (= 3.0.5) activesupport (= 3.0.5) activesupport (3.0.5) - addressable (2.2.4) + addressable (2.2.5) arel (2.0.9) bcrypt-ruby (2.1.4) - bson (1.2.4) - bson_ext (1.2.4) + bson (1.3.0) + bson_ext (1.3.0) builder (2.1.2) crack (0.1.8) - database_cleaner (0.6.5) - devise (1.1.8) + database_cleaner (0.6.7) + devise (1.1.9) bcrypt-ruby (~> 2.1.2) warden (~> 1.0.2) diff-lcs (1.1.2) @@ -58,23 +58,28 @@ GEM lighthouse-api (2.0) activeresource (>= 3.0.0) activesupport (>= 3.0.0) - mail (2.2.15) + mail (2.2.17) activesupport (>= 2.3.6) i18n (>= 0.4.0) mime-types (~> 1.16) treetop (~> 1.4.8) mime-types (1.16) - mongo (1.2.4) - bson (>= 1.2.4) - mongoid (2.0.0.rc.7) + mongo (1.3.0) + bson (>= 1.3.0) + mongoid (2.0.0.rc.8) activemodel (~> 3.0) mongo (~> 1.2) tzinfo (~> 0.3.22) will_paginate (~> 3.0.pre) + mongoid_rails_migrations (0.0.10) + activesupport (~> 3.0.0) + bundler (>= 0.9.19) + rails (~> 3.0.0) + railties (~> 3.0.0) nokogiri (1.4.4) polyglot (0.3.1) rack (1.2.2) - rack-mount (0.6.13) + rack-mount (0.6.14) rack (>= 1.0.0) rack-test (0.5.7) rack (>= 1.0) @@ -108,7 +113,7 @@ GEM thor (0.14.6) treetop (1.4.9) polyglot (>= 0.3.1) - tzinfo (0.3.25) + tzinfo (0.3.26) warden (1.0.3) rack (>= 1.0.0) webmock (1.6.2) @@ -126,7 +131,8 @@ DEPENDENCIES factory_girl_rails haml lighthouse-api - mongoid (~> 2.0.0.rc.7) + mongoid (= 2.0.0.rc.8) + mongoid_rails_migrations nokogiri rails (= 3.0.5) redmine_client! diff --git a/README.md b/README.md index e20a020d0..e3c8a0e0c 100644 --- a/README.md +++ b/README.md @@ -89,17 +89,24 @@ for you. Checkout [Hoptoad](http://hoptoadapp.com) from the guys over at 4. Enjoy! +Upgrading +--------- +*Note*: If upgrading from a version of Errbit that used Notices embedded in Errs please run: + + 1. git pull origin master ( assuming origin is the github.com/jdpace/errbit repo ) + 2. rake db:migrate + Lighthouseapp integration ------------------------- -* Account is the name of your subdomain, i.e. **litcafe** for project at http://litcafe.lighthouseapp.com/projects/73466-face/overview +* Account is the name of your subdomain, i.e. **litcafe** for project at http://litcafe.lighthouseapp.com/projects/73466-face/overview * Errbit uses token-based authentication. Get your API Token or visit [http://help.lighthouseapp.com/kb/api/how-do-i-get-an-api-token](http://help.lighthouseapp.com/kb/api/how-do-i-get-an-api-token) to learn how to get it. * Project id is number identifier of your project, i.e. **73466** for project at http://litcafe.lighthouseapp.com/projects/73466-face/overview Redmine integration ------------------------- -* Account is the host of your redmine installation, i.e. **http://redmine.org** +* Account is the host of your redmine installation, i.e. **http://redmine.org** * Errbit uses token-based authentication. Get your API Key or visit [http://www.redmine.org/projects/redmine/wiki/Rest_api#Authentication](http://www.redmine.org/projects/redmine/wiki/Rest_api#Authentication) to learn how to get it. * Project id is an identifier of your project, i.e. **chilliproject** for project at http://www.redmine.org/projects/chilliproject diff --git a/app/controllers/apps_controller.rb b/app/controllers/apps_controller.rb index 3bf60825a..f3a31cbab 100644 --- a/app/controllers/apps_controller.rb +++ b/app/controllers/apps_controller.rb @@ -1,12 +1,12 @@ class AppsController < ApplicationController - + before_filter :require_admin!, :except => [:index, :show] before_filter :find_app, :except => [:index, :new, :create] - + def index @apps = current_user.admin? ? App.all : current_user.apps.all end - + def show respond_to do |format| format.html do @@ -18,21 +18,21 @@ def show end end end - + def new @app = App.new @app.watchers.build @app.issue_tracker = IssueTracker.new end - + def edit @app.watchers.build if @app.watchers.none? @app.issue_tracker = IssueTracker.new if @app.issue_tracker.nil? end - + def create @app = App.new(params[:app]) - + if @app.save flash[:success] = 'Great success! Configure your app with the API key below' redirect_to app_path(@app) @@ -40,8 +40,8 @@ def create render :new end end - - def update + + def update if @app.update_attributes(params[:app]) flash[:success] = "Good news everyone! '#{@app.name}' was successfully updated." redirect_to app_path(@app) @@ -49,18 +49,18 @@ def update render :edit end end - + def destroy @app.destroy flash[:success] = "'#{@app.name}' was successfully destroyed." redirect_to apps_path end - + protected - + def find_app @app = App.find(params[:id]) - + # Mongoid Bug: could not chain: current_user.apps.find_by_id! # apparently finding by 'watchers.email' and 'id' is broken raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) diff --git a/app/controllers/errs_controller.rb b/app/controllers/errs_controller.rb index a5cae5637..3b18616a6 100644 --- a/app/controllers/errs_controller.rb +++ b/app/controllers/errs_controller.rb @@ -1,8 +1,8 @@ class ErrsController < ApplicationController - + before_filter :find_app, :except => [:index, :all] before_filter :find_err, :except => [:index, :all] - + def index app_scope = current_user.admin? ? App.all : current_user.apps respond_to do |format| @@ -14,14 +14,14 @@ def index end end end - + def all app_scope = current_user.admin? ? App.all : current_user.apps @errs = Err.for_apps(app_scope).ordered.paginate(:page => params[:page], :per_page => current_user.per_page) end - + def show - page = (params[:notice] || @err.notices.count) + page = (params[:notice] || @err.notices_count) page = 1 if page.to_i.zero? @notices = @err.notices.ordered.paginate(:page => page, :per_page => 1) @notice = @notices.first @@ -46,25 +46,25 @@ def clear_issue @err.update_attribute :issue_link, nil redirect_to app_err_path(@app, @err) end - + def resolve - # Deal with bug in mogoid where find is returning an Enumberable obj + # Deal with bug in mongoid where find is returning an Enumberable obj @err = @err.first if @err.respond_to?(:first) - + @err.resolve! - + flash[:success] = 'Great news everyone! The err has been resolved.' redirect_to :back rescue ActionController::RedirectBackError redirect_to app_path(@app) end - + protected - + def find_app @app = App.find(params[:app_id]) - + # Mongoid Bug: could not chain: current_user.apps.find_by_id! # apparently finding by 'watchers.email' and 'id' is broken raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) @@ -79,5 +79,5 @@ def set_tracker_params IssueTracker.default_url_options[:port] = request.port IssueTracker.default_url_options[:protocol] = request.scheme end - + end diff --git a/app/models/app.rb b/app/models/app.rb index 3709ae13f..fb71b370e 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -1,7 +1,7 @@ class App include Mongoid::Document include Mongoid::Timestamps - + field :name, :type => String field :api_key field :resolve_errs_on_deploy, :type => Boolean, :default => false @@ -21,29 +21,29 @@ class App embeds_many :deploys embeds_one :issue_tracker references_many :errs, :dependent => :destroy - + before_validation :generate_api_key, :on => :create - + validates_presence_of :name, :api_key validates_uniqueness_of :name, :allow_blank => true validates_uniqueness_of :api_key, :allow_blank => true validates_associated :watchers validate :check_issue_tracker - + accepts_nested_attributes_for :watchers, :allow_destroy => true, :reject_if => proc { |attrs| attrs[:user_id].blank? && attrs[:email].blank? } accepts_nested_attributes_for :issue_tracker, :allow_destroy => true, :reject_if => proc { |attrs| !%w( lighthouseapp redmine ).include?(attrs[:issue_tracker_type]) } - + # Mongoid Bug: find(id) on association proxies returns an Enumerator def self.find_by_id!(app_id) where(:_id => app_id).first || raise(Mongoid::Errors::DocumentNotFound.new(self,app_id)) end - + def self.find_by_api_key!(key) where(:api_key => key).first || raise(Mongoid::Errors::DocumentNotFound.new(self,key)) end - + def last_deploy_at deploys.last && deploys.last.created_at end @@ -58,9 +58,9 @@ def notify_on_deploys !(self[:notify_on_deploys] == false) end alias :notify_on_deploys? :notify_on_deploys - + protected - + def generate_api_key self.api_key ||= ActiveSupport::SecureRandom.hex end diff --git a/app/models/err.rb b/app/models/err.rb index 290a4829f..465258807 100644 --- a/app/models/err.rb +++ b/app/models/err.rb @@ -1,7 +1,7 @@ class Err include Mongoid::Document include Mongoid::Timestamps - + field :klass field :component field :action @@ -10,42 +10,44 @@ class Err field :last_notice_at, :type => DateTime field :resolved, :type => Boolean, :default => false field :issue_link, :type => String + field :notices_count, :type => Integer, :default => 0 + field :message index :last_notice_at index :app_id referenced_in :app - embeds_many :notices - + references_many :notices + validates_presence_of :klass, :environment - + scope :resolved, where(:resolved => true) scope :unresolved, where(:resolved => false) scope :ordered, order_by(:last_notice_at.desc) scope :in_env, lambda {|env| where(:environment => env)} scope :for_apps, lambda {|apps| where(:app_id.in => apps.all.map(&:id))} - + def self.for(attrs) app = attrs.delete(:app) app.errs.where(attrs).first || app.errs.create!(attrs) end - + def resolve! self.update_attributes!(:resolved => true) end - + def unresolved? !resolved? end - + def where where = component.dup where << "##{action}" if action.present? where end - + def message - notices.first.try(:message) || klass + super || klass end - -end \ No newline at end of file + +end diff --git a/app/models/notice.rb b/app/models/notice.rb index 83b5ce166..3d5a8c783 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -1,32 +1,37 @@ require 'hoptoad' +require 'recurse' class Notice include Mongoid::Document include Mongoid::Timestamps - + field :message field :backtrace, :type => Array field :server_environment, :type => Hash field :request, :type => Hash field :notifier, :type => Hash - - embedded_in :err, :inverse_of => :notices - + + referenced_in :err + index :err_id + after_create :cache_last_notice_at after_create :deliver_notification, :if => :should_notify? - + before_create :increase_counter_cache, :cache_message + before_save :sanitize + before_destroy :decrease_counter_cache + validates_presence_of :backtrace, :server_environment, :notifier - + scope :ordered, order_by(:created_at.asc) - + def self.from_xml(hoptoad_xml) hoptoad_notice = Hoptoad::V2.parse_xml(hoptoad_xml) app = App.find_by_api_key!(hoptoad_notice['api-key']) - + hoptoad_notice['request'] ||= {} hoptoad_notice['request']['component'] = 'unknown' if hoptoad_notice['request']['component'].blank? hoptoad_notice['request']['action'] = nil if hoptoad_notice['request']['action'].blank? - + err = Err.for({ :app => app, :klass => hoptoad_notice['error']['class'], @@ -36,7 +41,7 @@ def self.from_xml(hoptoad_xml) :fingerprint => hoptoad_notice['fingerprint'] }) err.update_attributes(:resolved => false) if err.resolved? - + err.notices.create!({ :message => hoptoad_notice['error']['message'], :backtrace => hoptoad_notice['error']['backtrace']['line'], @@ -45,35 +50,67 @@ def self.from_xml(hoptoad_xml) :notifier => hoptoad_notice['notifier'] }) end - + def request read_attribute(:request) || {} end - + def env_vars request['cgi-data'] || {} end - + def params request['params'] || {} end - + def session request['session'] || {} end - + def deliver_notification Mailer.err_notification(self).deliver end - + def cache_last_notice_at err.update_attributes(:last_notice_at => created_at) end - + protected - - def should_notify? - err.app.notify_on_errs? && Errbit::Config.email_at_notices.include?(err.notices.count) && err.app.watchers.any? + + def should_notify? + err.app.notify_on_errs? && Errbit::Config.email_at_notices.include?(err.notices.count) && err.app.watchers.any? + end + + + def increase_counter_cache + err.inc(:notices_count,1) + end + + def decrease_counter_cache + err.inc(:notices_count,-1) + end + + def cache_message + err.update_attribute(:message, message) if err.notices_count == 1 + end + + def sanitize + [:server_environment, :request, :notifier].each do |h| + send("#{h}=",sanitize_hash(send(h))) end - -end \ No newline at end of file + end + + def sanitize_hash(h) + h.recurse do + |h| h.inject({}) do |h,(k,v)| + if k.is_a?(String) + h[k.gsub(/\./,'.').gsub(/^\$/,'$')] = v + else + h[k] = v + end + h + end + end + end +end + diff --git a/app/views/apps/index.html.haml b/app/views/apps/index.html.haml index c636d814b..451918402 100644 --- a/app/views/apps/index.html.haml +++ b/app/views/apps/index.html.haml @@ -14,7 +14,7 @@ %td.name= link_to app.name, app_path(app) %td.deploy= app.last_deploy_at ? link_to( app.last_deploy_at.to_s(:micro), app_deploys_path(app)) : 'n/a' %td.count - - if app.errs.any? + - if app.errs.count > 0 = link_to app.errs.unresolved.count, app_errs_path(app) - else \- diff --git a/app/views/apps/show.html.haml b/app/views/apps/show.html.haml index 61b10a482..200ff75cb 100644 --- a/app/views/apps/show.html.haml +++ b/app/views/apps/show.html.haml @@ -50,7 +50,7 @@ - else %h3 No deploys -- if @app.errs.any? +- if @app.errs.count > 0 %h3.clear Errs = render 'errs/table', :errs => @errs - else diff --git a/app/views/errs/_table.html.haml b/app/views/errs/_table.html.haml index 28efc14f3..825ef7dff 100644 --- a/app/views/errs/_table.html.haml +++ b/app/views/errs/_table.html.haml @@ -18,7 +18,7 @@ %em= err.where %td.latest #{time_ago_in_words(last_notice_at err)} ago %td.deploy= err.app.last_deploy_at ? err.app.last_deploy_at.to_s(:micro) : 'n/a' - %td.count= link_to err.notices.count, app_err_path(err.app, err) + %td.count= link_to err.notices_count, app_err_path(err.app, err) %td.resolve= link_to image_tag("thumbs-up.png"), resolve_app_err_path(err.app, err), :title => "Resolve", :method => :put, :confirm => err_confirm, :class => 'resolve' if err.unresolved? - if errs.none? %tr diff --git a/app/views/errs/lighthouseapp_body.txt.erb b/app/views/errs/lighthouseapp_body.txt.erb index c0c6d8111..709d194f5 100644 --- a/app/views/errs/lighthouseapp_body.txt.erb +++ b/app/views/errs/lighthouseapp_body.txt.erb @@ -13,7 +13,7 @@ <%= notice.created_at.to_s(:micro) %> ### Similar ### - <%= (notice.err.notices.count - 1).to_s %> + <%= (notice.err.notices_count - 1).to_s %> ## Params ## <%= pretty_hash(notice.params) %> diff --git a/app/views/errs/redmine_body.txt.erb b/app/views/errs/redmine_body.txt.erb index aeff23cfe..2a7e975ec 100644 --- a/app/views/errs/redmine_body.txt.erb +++ b/app/views/errs/redmine_body.txt.erb @@ -18,7 +18,7 @@ h3. Occured h3. Similar -<%= (notice.err.notices.count - 1).to_s %> +<%= (notice.err.notices_count - 1).to_s %> h2. Params diff --git a/app/views/errs/show.html.haml b/app/views/errs/show.html.haml index ade5c90cd..f7fb535b1 100644 --- a/app/views/errs/show.html.haml +++ b/app/views/errs/show.html.haml @@ -20,7 +20,7 @@ %span= link_to 'resolve', resolve_app_err_path(@app, @err), :method => :put, :confirm => err_confirm, :class => 'resolve' %h4= @notice.try(:message) - + = will_paginate @notices, :param_name => :notice, :page_links => false, :class => 'notice-pagination' viewing occurrence #{@notices.current_page} of #{@notices.total_pages} @@ -36,19 +36,19 @@ viewing occurrence #{@notices.current_page} of #{@notices.total_pages} #summary %h3 Summary = render 'notices/summary', :notice => @notice - + #backtrace %h3 Backtrace = render 'notices/backtrace', :lines => @notice.backtrace - + #environment %h3 Environment = render 'notices/environment', :notice => @notice - + #params %h3 Parameters = render 'notices/params', :notice => @notice - + #session %h3 Session = render 'notices/session', :notice => @notice diff --git a/app/views/mailer/err_notification.text.erb b/app/views/mailer/err_notification.text.erb index 3d46554f6..dfd8a3159 100644 --- a/app/views/mailer/err_notification.text.erb +++ b/app/views/mailer/err_notification.text.erb @@ -1,7 +1,7 @@ An err has just occurred in <%= @notice.err.environment %>: <%= @notice.err.message %> -This err has occurred <%= pluralize @notice.err.notices.count, 'time' %>. You should really look into it here: +This err has occurred <%= pluralize @notice.err.notices_count, 'time' %>. You should really look into it here: <%= app_err_url(@app, @notice.err) %> - -<%= render :partial => 'signature' %> \ No newline at end of file + +<%= render :partial => 'signature' %> diff --git a/app/views/notices/_atom_entry.html.haml b/app/views/notices/_atom_entry.html.haml index 739686262..b11e19859 100644 --- a/app/views/notices/_atom_entry.html.haml +++ b/app/views/notices/_atom_entry.html.haml @@ -6,13 +6,13 @@ = link_to(notice.request['url'], notice.request['url']) %p %strong Where: - = notice.err.where + = notice.err.where %p %strong Occured: = notice.created_at.to_s(:micro) %p %strong Similar: - = notice.err.notices.count - 1 + = notice.err.notices_count - 1 %h3 Params %p= pretty_hash(notice.params) diff --git a/app/views/notices/_environment.html.haml b/app/views/notices/_environment.html.haml index ebf533ef8..8db2fb12b 100644 --- a/app/views/notices/_environment.html.haml +++ b/app/views/notices/_environment.html.haml @@ -2,5 +2,5 @@ %table.environment - notice.env_vars.each do |key,val| %tr - %th= key + %th= raw key %td.main= val \ No newline at end of file diff --git a/app/views/notices/_summary.html.haml b/app/views/notices/_summary.html.haml index 06889e691..4fe491c95 100644 --- a/app/views/notices/_summary.html.haml +++ b/app/views/notices/_summary.html.haml @@ -15,4 +15,4 @@ %td= notice.created_at.to_s(:micro) %tr %th Similar - %td= notice.err.notices.count - 1 \ No newline at end of file + %td= notice.err.notices_count - 1 \ No newline at end of file diff --git a/db/migrate/20110422152027_move_notices_to_separate_collection.rb b/db/migrate/20110422152027_move_notices_to_separate_collection.rb new file mode 100644 index 000000000..f3ea60ee6 --- /dev/null +++ b/db/migrate/20110422152027_move_notices_to_separate_collection.rb @@ -0,0 +1,22 @@ +class MoveNoticesToSeparateCollection < Mongoid::Migration + def self.up + # copy embedded Notices into a separate collection + mongo_db = Err.db + errs = mongo_db.collection("errs").find({ }, :fields => ["notices"]) + errs.each do |err| + next unless err['notices'] + e = Err.find(err['_id']) + puts "Copying notices for Err #{err['_id']}" + err['notices'].each do |notice| + e.notices.create!(notice) + end + mongo_db.collection("errs").update({ "_id" => err['_id']}, { "$unset" => { "notices" => 1}}) + end + Rake::Task["errbit:db:update_notices_count"].invoke + Rake::Task["errbit:db:update_err_message"].invoke + end + + def self.down + end + +end diff --git a/lib/hoptoad.rb b/lib/hoptoad.rb index ee6d79902..4bf64e65a 100644 --- a/lib/hoptoad.rb +++ b/lib/hoptoad.rb @@ -1,13 +1,13 @@ module Hoptoad module V2 require 'digest/md5' - + class ApiVersionError < StandardError def initialize super "Wrong API Version: Expecting v2.0" end end - + def self.parse_xml(xml) parsed = ActiveSupport::XmlMini.backend.parse(xml)['notice'] raise ApiVersionError unless parsed && parsed['version'] == '2.0' @@ -15,9 +15,9 @@ def self.parse_xml(xml) rekeyed['fingerprint'] = Digest::MD5.hexdigest(rekeyed['error']['backtrace'].to_s) rekeyed end - + private - + def self.rekey(node) if node.is_a?(Hash) && node.has_key?('var') && node.has_key?('key') {node['key'] => rekey(node['var'])} @@ -42,4 +42,4 @@ def self.rekey(node) end end end -end \ No newline at end of file +end diff --git a/lib/recurse.rb b/lib/recurse.rb new file mode 100644 index 000000000..7f5a4ee9a --- /dev/null +++ b/lib/recurse.rb @@ -0,0 +1,24 @@ +class Hash + + # Apply a block to hash, and recursively apply that block + # to each sub-hash or +types+. + # + # h = {:a=>1, :b=>{:b1=>1, :b2=>2}} + # g = h.recurse{|h| h.inject({}){|h,(k,v)| h[k.to_s] = v; h} } + # g #=> {"a"=>1, "b"=>{"b1"=>1, "b2"=>2}} + # + def recurse(*types, &block) + types = [self.class] if types.empty? + h = inject({}) do |hash, (key, value)| + case value + when *types + hash[key] = value.recurse(*types, &block) + else + hash[key] = value + end + hash + end + yield h + end + +end diff --git a/lib/tasks/errbit/err_message.rake b/lib/tasks/errbit/err_message.rake new file mode 100644 index 000000000..6f9e8c17f --- /dev/null +++ b/lib/tasks/errbit/err_message.rake @@ -0,0 +1,12 @@ +namespace :errbit do + + namespace :db do + desc "Updates Err#notices_count" + task :update_err_message => :environment do + puts "Updating err.message" + Err.all.each do |e| + e.update_attributes(:message => e.notices.first.message) if e.notices.first + end + end + end +end diff --git a/lib/tasks/errbit/notices_counter.rake b/lib/tasks/errbit/notices_counter.rake new file mode 100644 index 000000000..617ea447d --- /dev/null +++ b/lib/tasks/errbit/notices_counter.rake @@ -0,0 +1,12 @@ +namespace :errbit do + + namespace :db do + desc "Updates Err#notices_count" + task :update_notices_count => :environment do + puts "Updating err.notices_count" + Err.all.each do |e| + e.update_attributes(:notices_count => e.notices.count) + end + end + end +end diff --git a/spec/controllers/errs_controller_spec.rb b/spec/controllers/errs_controller_spec.rb index 86f8fb405..3d3149be0 100644 --- a/spec/controllers/errs_controller_spec.rb +++ b/spec/controllers/errs_controller_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe ErrsController do - + it_requires_authentication :for => { :index => :get, :all => :get, :show => :get, :resolve => :put }, :params => {:app_id => 'dummyid', :id => 'dummyid'} - + let(:app) { Factory(:app) } let(:err) { Factory(:err, :app => app) } - + describe "GET /errs" do render_views context 'when logged in as an admin' do @@ -31,7 +31,7 @@ response.should be_success response.body.should match(@err.message) end - + it "should handle lots of errors" do pending "Turning off long running spec" 1000.times { Factory :notice } @@ -55,7 +55,7 @@ end end end - + context 'when logged in as a user' do it 'gets a paginated list of unresolved errs for the users apps' do sign_in(user = Factory(:user)) @@ -68,7 +68,7 @@ end end end - + describe "GET /errs/all" do context 'when logged in as an admin' do it "gets a paginated list of all errs" do @@ -83,7 +83,7 @@ assigns(:errs).should == errs end end - + context 'when logged in as a user' do it 'gets a paginated list of all errs for the users apps' do sign_in(user = Factory(:user)) @@ -96,29 +96,29 @@ end end end - + describe "GET /apps/:app_id/errs/:id" do render_views - + before do 3.times { Factory(:notice, :err => err)} end - + context 'when logged in as an admin' do before do sign_in Factory(:admin) end - + it "finds the app" do get :show, :app_id => app.id, :id => err.id assigns(:app).should == app end - + it "finds the err" do get :show, :app_id => app.id, :id => err.id assigns(:err).should == err end - + it "successfully render page" do get :show, :app_id => app.id, :id => err.id response.should be_success @@ -131,9 +131,9 @@ err = Factory :err get :show, :app_id => err.app.id, :id => err.id - response.body.should_not button_matcher + response.body.should_not button_matcher end - + it "should exist for err's app with issue tracker" do tracker = Factory(:lighthouseapp_tracker) err = Factory(:err, :app => tracker.app) @@ -141,7 +141,7 @@ response.body.should button_matcher end - + it "should not exist for err with issue_link" do tracker = Factory(:lighthouseapp_tracker) err = Factory(:err, :app => tracker.app, :issue_link => "http://some.host") @@ -151,7 +151,7 @@ end end end - + context 'when logged in as a user' do before do sign_in(@user = Factory(:user)) @@ -160,12 +160,12 @@ @watcher = Factory(:user_watcher, :user => @user, :app => @watched_app) @watched_err = Factory(:err, :app => @watched_app) end - + it 'finds the err if the user is watching the app' do get :show, :app_id => @watched_app.to_param, :id => @watched_err.id assigns(:err).should == @watched_err end - + it 'raises a DocumentNotFound error if the user is not watching the app' do lambda { get :show, :app_id => @unwatched_err.app_id, :id => @unwatched_err.id @@ -173,17 +173,17 @@ end end end - + describe "PUT /apps/:app_id/errs/:id/resolve" do before do sign_in Factory(:admin) - + @err = Factory(:err) App.stub(:find).with(@err.app.id).and_return(@err.app) @err.app.errs.stub(:find).and_return(@err) @err.stub(:resolve!) end - + it 'finds the app and the err' do App.should_receive(:find).with(@err.app.id).and_return(@err.app) @err.app.errs.should_receive(:find).and_return(@err) @@ -191,17 +191,17 @@ assigns(:app).should == @err.app assigns(:err).should == @err end - + it "should resolve the issue" do @err.should_receive(:resolve!).and_return(true) put :resolve, :app_id => @err.app.id, :id => @err.id end - + it "should display a message" do put :resolve, :app_id => @err.app.id, :id => @err.id request.flash[:success].should match(/Great news/) end - + it "should redirect to the app page" do put :resolve, :app_id => @err.app.id, :id => @err.id response.should redirect_to(app_path(@err.app)) diff --git a/spec/models/err_spec.rb b/spec/models/err_spec.rb index 95f68e90d..5eaf12416 100644 --- a/spec/models/err_spec.rb +++ b/spec/models/err_spec.rb @@ -1,21 +1,21 @@ require 'spec_helper' describe Err do - + context 'validations' do it 'requires a klass' do err = Factory.build(:err, :klass => nil) err.should_not be_valid err.errors[:klass].should include("can't be blank") end - + it 'requires an environment' do err = Factory.build(:err, :environment => nil) err.should_not be_valid err.errors[:environment].should include("can't be blank") end end - + context '#for' do before do @app = Factory(:app) @@ -27,16 +27,16 @@ :environment => 'production' } end - + it 'returns the correct err if one already exists' do existing = Err.create(@conditions) Err.for(@conditions).should == existing end - + it 'assigns the returned err to the given app' do Err.for(@conditions).app.should == @app end - + it 'creates a new err if a matching one does not already exist' do Err.where(@conditions.except(:app)).exists?.should == false lambda { @@ -44,36 +44,47 @@ }.should change(Err,:count).by(1) end end - + context '#last_notice_at' do it "returns the created_at timestamp of the latest notice" do err = Factory(:err) err.last_notice_at.should be_nil - + notice1 = Factory(:notice, :err => err) err.last_notice_at.should == notice1.created_at - + notice2 = Factory(:notice, :err => err) err.last_notice_at.should == notice2.created_at end end - + context '#message' do + it "returns klass by default" do + err = Factory(:err) + err.message.should == err.klass + end + it 'returns the message from the first notice' do err = Factory(:err) notice1 = Factory(:notice, :err => err, :message => 'ERR 1') notice2 = Factory(:notice, :err => err, :message => 'ERR 2') err.message.should == notice1.message end + + it "adding a notice caches its message" do + err = Factory(:err) + lambda { + notice1 = Factory(:notice, :err => err, :message => 'ERR 1')}.should change(err, :message).from(err.klass).to('ERR 1') + end end - + context "#resolved?" do it "should start out as unresolved" do err = Err.new err.should_not be_resolved err.should be_unresolved end - + it "should be able to be resolved" do err = Factory(:err) err.should_not be_resolved @@ -81,7 +92,7 @@ err.reload.should be_resolved end end - + context "resolve!" do it "marks the err as resolved" do err = Factory(:err) @@ -89,7 +100,7 @@ err.resolve! err.should be_resolved end - + it "should throw an err if it's not successful" do err = Factory(:err) err.should_not be_resolved @@ -100,7 +111,7 @@ }.should raise_error(Mongoid::Errors::Validations) end end - + context "Scopes" do context "resolved" do it 'only finds resolved Errs' do @@ -110,7 +121,7 @@ Err.resolved.all.should_not include(unresolved) end end - + context "unresolved" do it 'only finds unresolved Errs' do resolved = Factory(:err, :resolved => true) @@ -130,4 +141,30 @@ end end end -end \ No newline at end of file + + context "notice counter cache" do + + before do + @app = Factory(:app) + @err = Factory(:err, :app => @app) + end + + it "#notices_count returns 0 by default" do + @err.notices_count.should == 0 + end + + it "adding a notice increases #notices_count by 1" do + lambda { + notice1 = Factory(:notice, :err => @err, :message => 'ERR 1')}.should change(@err, :notices_count).from(0).to(1) + end + + it "removing a notice decreases #notices_count by 1" do + notice1 = Factory(:notice, :err => @err, :message => 'ERR 1') + lambda { + @err.notices.first.destroy + }.should change(@err, :notices_count).from(1).to(0) + end + end + + +end diff --git a/spec/models/notice_spec.rb b/spec/models/notice_spec.rb index 2d8954210..07920b309 100644 --- a/spec/models/notice_spec.rb +++ b/spec/models/notice_spec.rb @@ -1,39 +1,39 @@ require 'spec_helper' describe Notice do - + context 'validations' do it 'requires a backtrace' do notice = Factory.build(:notice, :backtrace => nil) notice.should_not be_valid notice.errors[:backtrace].should include("can't be blank") end - + it 'requires the server_environment' do notice = Factory.build(:notice, :server_environment => nil) notice.should_not be_valid notice.errors[:server_environment].should include("can't be blank") end - + it 'requires the notifier' do notice = Factory.build(:notice, :notifier => nil) notice.should_not be_valid notice.errors[:notifier].should include("can't be blank") end end - + context '#from_xml' do before do @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read @app = Factory(:app, :api_key => 'APIKEY') Digest::MD5.stub(:hexdigest).and_return('fingerprintdigest') end - + it 'finds the correct app' do @notice = Notice.from_xml(@xml) @notice.err.app.should == @app end - + it 'finds the correct err for the notice' do Err.should_receive(:for).with({ :app => @app, @@ -46,7 +46,7 @@ err.notices.stub(:create!) @notice = Notice.from_xml(@xml) end - + it 'marks the err as unresolve if it was previously resolved' do Err.should_receive(:for).with({ :app => @app, @@ -61,56 +61,70 @@ @notice.err.should == err @notice.err.should_not be_resolved end - + it 'should create a new notice' do @notice = Notice.from_xml(@xml) @notice.should be_persisted end - + it 'assigns an err to the notice' do @notice = Notice.from_xml(@xml) @notice.err.should be_a(Err) end - + it 'captures the err message' do @notice = Notice.from_xml(@xml) @notice.message.should == 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' end - + it 'captures the backtrace' do @notice = Notice.from_xml(@xml) @notice.backtrace.size.should == 73 @notice.backtrace.last['file'].should == '[GEM_ROOT]/bin/rake' end - + it 'captures the server_environment' do @notice = Notice.from_xml(@xml) @notice.server_environment['environment-name'].should == 'development' end - + it 'captures the request' do @notice = Notice.from_xml(@xml) @notice.request['url'].should == 'http://example.org/verify' @notice.request['params']['controller'].should == 'application' end - + it 'captures the notifier' do @notice = Notice.from_xml(@xml) @notice.notifier['name'].should == 'Hoptoad Notifier' end - it "should handle params withour 'request' section" do + it "should handle params without 'request' section" do @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read lambda { Notice.from_xml(@xml) }.should_not raise_error end end - + + describe "key sanitization" do + before do + @hash = { "some.key" => { "$nested.key" => {"$Path" => "/", "some$key" => "key"}}} + @hash_sanitized = { "some.key" => { "$nested.key" => {"$Path" => "/", "some$key" => "key"}}} + end + [:server_environment, :request, :notifier].each do |key| + it "replaces . with . and $ with $ in keys used in #{key}" do + err = Factory(:err) + notice = Factory(:notice, :err => err, key => @hash) + notice.send(key).should == @hash_sanitized + end + end + end + describe "email notifications" do before do @app = Factory(:app_with_watcher) @err = Factory(:err, :app => @app) end - + Errbit::Config.email_at_notices.each do |threshold| it "sends an email notification after #{threshold} notice(s)" do @err.notices.stub(:count).and_return(threshold) @@ -120,5 +134,5 @@ end end end - -end \ No newline at end of file + +end