Improve exception handling of web app when the requested resource does not exist

10 files changed, 216 insertions, 227 deletions

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb
index 02e2a7085df..717eb23a751 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb
@@ -30,22 +30,6 @@ class Api::ApiController < ApplicationController
     end
   end
 
-  #
-  # Override the error handling defined in parent ApplicationController
-  #
-  rescue_from Exception, :with => :render_error
-  rescue_from ApiException, :with => :render_error
-  rescue_from Errors::BadRequest, :with => :render_bad_request
-  rescue_from ActionController::UnknownAction, :with => :render_not_found
-  rescue_from ActionController::RoutingError, :with => :render_not_found
-  rescue_from ActionController::UnknownController, :with => :render_not_found
-  rescue_from ActiveRecord::RecordNotFound, :with => :render_not_found
-  rescue_from Errors::NotFound, :with => :render_not_found
-  rescue_from Errors::AccessDenied, :with => :render_access_denied
-
-  
-  protected
-
   def text_not_supported
     "Not supported"
   end
@@ -94,8 +78,9 @@ class Api::ApiController < ApplicationController
   #
   #
 
-  def render_error(message, status)
-    logger.error("Fail to render: #{request.url}", message) if status==500
+  def render_error(exception, status=500)
+    message = exception.respond_to?('message') ? Api::Utils.exception_message(exception) : exception.to_s
+    java_facade.logError("Fail to render: #{request.url}\n#{message}") if status==500
     respond_to do |format|
       format.json { render :json => error_to_json(status, message), :status => status }
       format.xml { render :xml => error_to_xml(status, message), :status => status }
@@ -103,12 +88,14 @@ class Api::ApiController < ApplicationController
     end
   end
 
-  def render_not_found(error)
-    render_error(error.message, 404)
+  def render_not_found(exception)
+    message = exception.respond_to?('message') ? exception.message : exception.to_s
+    render_error(message, 404)
   end
 
-  def render_bad_request(error)
-    render_error(error.message, 400)
+  def render_bad_request(exception)
+    message = exception.respond_to?('message') ? exception.message : exception.to_s
+    render_error(message, 400)
   end
 
   def render_access_denied
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
index c799f91a95b..3f59f2c24ac 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
@@ -18,31 +18,30 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
 #
 class ApplicationController < ActionController::Base
-  
+
   include AuthenticatedSystem
   include NeedAuthorization::Helper
-  
+
   before_filter :check_database_version, :set_locale, :check_authentication
 
-  unless ActionController::Base.consider_all_requests_local
-    rescue_from Exception, :with => :render_error
-    rescue_from Errors::BadRequest, :with => :render_bad_request
-    rescue_from ActionController::UnknownAction, :with => :render_not_found
-    rescue_from ActionController::RoutingError, :with => :render_not_found
-    rescue_from ActionController::UnknownController, :with => :render_not_found
-    rescue_from ActiveRecord::RecordNotFound, :with => :render_not_found
-    rescue_from Errors::NotFound, :with => :render_not_found
-    rescue_from Errors::AccessDenied, :with => :render_access_denied # See lib/authenticated_system.rb#access_denied()
-  end
+  rescue_from Exception, :with => :render_error
+  rescue_from Errors::BadRequest, :with => :render_bad_request
+  rescue_from ActionController::UnknownAction, :with => :render_not_found
+  rescue_from ActionController::RoutingError, :with => :render_not_found
+  rescue_from ActionController::UnknownController, :with => :render_not_found
+  rescue_from ActiveRecord::RecordInvalid, :with => :render_error
+  rescue_from ActiveRecord::RecordNotFound, :with => :render_not_found
+  rescue_from Errors::NotFound, :with => :render_not_found
+  rescue_from Errors::AccessDenied, :with => :render_access_denied # See lib/authenticated_system.rb#access_denied()
 
   def self.root_context
     ActionController::Base.relative_url_root || ''
   end
-    
+
   def java_facade
     Java::OrgSonarServerUi::JRubyFacade.getInstance()
   end
-  
+
   def available_locales
     # see config/initializers/available_locales.rb
     AVAILABLE_LOCALES
@@ -98,16 +97,13 @@ class ApplicationController < ActionController::Base
       access_denied
     end
   end
-  
+
   # i18n
   def message(key, options={})
     Api::Utils.message(key, options)
   end
 
 
-
-
-
   #
   #
   # ERROR HANDLING
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/cloud_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/cloud_controller.rb
index e46df170ef4..80d16f1ca3a 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/cloud_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/cloud_controller.rb
@@ -24,9 +24,7 @@ class CloudController < ApplicationController
   def index
     resource_key = params[:id]
     @project = resource_key ? Project.by_key(resource_key) : nil
-    if @project.nil?
-      return render :text => "Resource [#{project_key}] not found", :status => 404
-    end
+    not_found("Project not found") unless @project
     access_denied unless has_role?(:user, @project)
     @snapshot=@project.last_snapshot
 
@@ -40,28 +38,28 @@ class CloudController < ApplicationController
     snapshot_conditions='snapshots.islast=:islast AND snapshots.scope=:scope AND snapshots.qualifier!=:test_qualifier AND 
       (snapshots.id=:sid OR (snapshots.root_snapshot_id=:root_sid AND snapshots.path LIKE :path))'
     snapshot_values={
-      :islast => true, 
-      :scope => 'FIL', 
-      :test_qualifier => 'UTS',
-      :sid => @snapshot.id, 
-      :root_sid => (@snapshot.root_snapshot_id || @snapshot.id),
-      :path => "#{@snapshot.path}#{@snapshot.id}.%"
+        :islast => true,
+        :scope => 'FIL',
+        :test_qualifier => 'UTS',
+        :sid => @snapshot.id,
+        :root_sid => (@snapshot.root_snapshot_id || @snapshot.id),
+        :path => "#{@snapshot.path}#{@snapshot.id}.%"
     }
-    
+
     @snapshots=Snapshot.find(:all, :conditions => [snapshot_conditions, snapshot_values], :include => 'project', :order => 'projects.name')
-        
+
     size_measures=ProjectMeasure.find(:all,
-      :select => 'project_measures.id,project_measures.value,project_measures.metric_id,project_measures.snapshot_id,project_measures.rule_id,project_measures.rule_priority,project_measures.text_value,project_measures.characteristic_id,project_measures.alert_status',
-      :joins => :snapshot,
-      :conditions => [snapshot_conditions + " AND project_measures.metric_id=#{@size_metric.id}", snapshot_values],
-      :order => 'project_measures.value')
-    
+                                      :select => 'project_measures.id,project_measures.value,project_measures.metric_id,project_measures.snapshot_id,project_measures.rule_id,project_measures.rule_priority,project_measures.text_value,project_measures.characteristic_id,project_measures.alert_status',
+                                      :joins => :snapshot,
+                                      :conditions => [snapshot_conditions + " AND project_measures.metric_id=#{@size_metric.id}", snapshot_values],
+                                      :order => 'project_measures.value')
+
     color_measures=ProjectMeasure.find(:all,
-        :select => 'project_measures.id,project_measures.value,project_measures.metric_id,project_measures.snapshot_id,project_measures.rule_id,project_measures.rule_priority,project_measures.text_value,project_measures.characteristic_id,project_measures.alert_status',
-        :joins => :snapshot,
-        :conditions => [snapshot_conditions + " AND project_measures.metric_id=#{@color_metric.id}", snapshot_values],
-        :order => 'project_measures.value')
-        
+                                       :select => 'project_measures.id,project_measures.value,project_measures.metric_id,project_measures.snapshot_id,project_measures.rule_id,project_measures.rule_priority,project_measures.text_value,project_measures.characteristic_id,project_measures.alert_status',
+                                       :joins => :snapshot,
+                                       :conditions => [snapshot_conditions + " AND project_measures.metric_id=#{@color_metric.id}", snapshot_values],
+                                       :order => 'project_measures.value')
+
     @size_measure_by_sid={}, @color_measure_by_sid={}
     size_measures.each do |m|
       @size_measure_by_sid[m.snapshot_id]=m
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/components_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/components_controller.rb
index 2588dd36453..24a8635f16e 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/components_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/components_controller.rb
@@ -21,7 +21,7 @@ class ComponentsController < ApplicationController
 
   helper :metrics, :components
 
-  verify :method => :post, :only => :update_default_treemap_metrics  
+  verify :method => :post, :only => :update_default_treemap_metrics
 
   before_filter :admin_required, :only => :update_default_treemap_metrics
 
@@ -32,10 +32,11 @@ class ComponentsController < ApplicationController
     @components_configuration = Sonar::ComponentsConfiguration.new
 
     @project = Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless has_role?(:user, @project)
     @snapshot = @project.last_snapshot
     @snapshots = Snapshot.find(:all, :include => 'project', :conditions => ['snapshots.parent_snapshot_id=? and snapshots.qualifier<>? and projects.qualifier<>?', @snapshot.id, Snapshot::QUALIFIER_UNIT_TEST_CLASS, Snapshot::QUALIFIER_UNIT_TEST_CLASS])
-    
+
     @columns = @components_configuration.selected_columns
     metrics = @components_configuration.homepage_metrics
 
@@ -54,9 +55,9 @@ class ComponentsController < ApplicationController
     render(:update) do |page|
       page.replace_html 'treemap', @treemap.generate_html
       page.replace_html 'treemap_gradient', :partial => 'components/treemap_gradient',
-        :locals => { :color_metric => @treemap.color_metric}
+                        :locals => {:color_metric => @treemap.color_metric}
       page.replace_html 'treemap_set_default', :partial => 'components/treemap_set_default',
-        :locals => { :controller => 'components', :size_metric => params[:size_metric], :color_metric => params[:color_metric], :rid => @snapshot.project_id }
+                        :locals => {:controller => 'components', :size_metric => params[:size_metric], :color_metric => params[:color_metric], :rid => @snapshot.project_id}
     end
   end
 
@@ -70,13 +71,13 @@ class ComponentsController < ApplicationController
 
   def refresh_configure
     render :update do |page|
-      page.replace_html("rule_id_#{@rule.id}", :partial => 'rule', :locals => { :rule => @rule })
+      page.replace_html("rule_id_#{@rule.id}", :partial => 'rule', :locals => {:rule => @rule})
     end
   end
 
   def measures_by_snapshot(snapshots, measures)
     snapshot_by_id = {}
-    snapshots.each {|s| snapshot_by_id[s.id]=s}
+    snapshots.each { |s| snapshot_by_id[s.id]=s }
     hash={}
     measures.each do |m|
       if m && m.snapshot_id && snapshot_by_id[m.snapshot_id]
@@ -88,9 +89,9 @@ class ComponentsController < ApplicationController
   end
 
   def component_measures(snapshots, metrics)
-    sids = snapshots.collect{|s| s.id}
+    sids = snapshots.collect { |s| s.id }
     if sids && sids.size>0
-      mids = metrics.collect{|metric| metric.id}
+      mids = metrics.collect { |metric| metric.id }
       measures=[]
 
       page_size=950
@@ -100,11 +101,11 @@ class ComponentsController < ApplicationController
       page_count.times do |page_index|
         page_sids=sids[page_index*page_size...(page_index+1)*page_size]
         measures.concat(ProjectMeasure.find(:all, :conditions => {
-          'snapshot_id' => page_sids,
-          'metric_id' => mids,
-          'rule_id' => nil,
-          'rule_priority' => nil,
-          'characteristic_id' => nil}))
+            'snapshot_id' => page_sids,
+            'metric_id' => mids,
+            'rule_id' => nil,
+            'rule_priority' => nil,
+            'characteristic_id' => nil}))
       end
       measures
     else
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboard_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboard_controller.rb
index f86fc2901e8..1a05eac678a 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboard_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboard_controller.rb
@@ -38,7 +38,7 @@ class DashboardController < ApplicationController
     # TODO display error page if no dashboard or no resource
     load_resource()
     load_dashboard()
-      load_widget_definitions()
+    load_widget_definitions()
     unless @dashboard
       redirect_to home_path
     end
@@ -56,7 +56,7 @@ class DashboardController < ApplicationController
       dashboard.column_layout=params[:layout]
       dashboard.save!
       columns=dashboard.column_layout.split('-')
-      dashboard.widgets.find(:all, :conditions => ["column_index > ?",columns.size()]).each do |widget|
+      dashboard.widgets.find(:all, :conditions => ["column_index > ?", columns.size()]).each do |widget|
         widget.column_index=columns.size()
         widget.save
       end
@@ -83,7 +83,7 @@ class DashboardController < ApplicationController
         end
       end
     end
-    @dashboard.widgets.reject{|w| all_ids.include?(w.id)}.each do |w|
+    @dashboard.widgets.reject { |w| all_ids.include?(w.id) }.each do |w|
       w.destroy
     end
     render :json => {:status => 'ok'}
@@ -95,12 +95,12 @@ class DashboardController < ApplicationController
     if dashboard.editable_by?(current_user)
       definition=java_facade.getWidget(params[:widget])
       if definition
-        first_column_widgets=dashboard.widgets.select{|w| w.column_index==1}.sort_by{|w| w.row_index}
+        first_column_widgets=dashboard.widgets.select { |w| w.column_index==1 }.sort_by { |w| w.row_index }
         new_widget=dashboard.widgets.create(:widget_key => definition.getId(),
-                                           :name => definition.getTitle(),
-                                           :column_index => 1,
-                                           :row_index => 1,
-                                           :configured => !definition.hasRequiredProperties())
+                                            :name => definition.getTitle(),
+                                            :column_index => 1,
+                                            :row_index => 1,
+                                            :configured => !definition.hasRequiredProperties())
         widget_id=new_widget.id
         first_column_widgets.each_with_index do |w, index|
           w.row_index=index+2
@@ -133,7 +133,7 @@ class DashboardController < ApplicationController
     if errors_by_property_key.empty?
       widget.configured=true
       widget.save
-      widget.properties.each {|p| p.save}
+      widget.properties.each { |p| p.save }
       render :update do |page|
         page.redirect_to(url_for(:action => :configure, :did => widget.dashboard_id, :id => params[:id]))
       end
@@ -181,17 +181,13 @@ class DashboardController < ApplicationController
 
   def load_resource
     @resource=Project.by_key(params[:id])
-    if @resource.nil?
-      # TODO display error page
-      redirect_to home_path
-      return false
-    end
+    not_found("Resource not found") unless @resource
     access_denied unless has_role?(:user, @resource)
     @snapshot = @resource.last_snapshot
-    @project=@resource  # variable name used in old widgets
+    @project=@resource # variable name used in old widgets
   end
 
-  def load_authorized_widget_definitions()
+  def load_authorized_widget_definitions
     if @resource
       @widget_definitions = java_facade.getWidgets(@resource.scope, @resource.qualifier, @resource.language)
       @widget_definitions=@widget_definitions.select do |widget|
@@ -210,10 +206,10 @@ class DashboardController < ApplicationController
   def load_widget_definitions(filter_on_category=nil)
     @widget_definitions=java_facade.getWidgets()
     @widget_categories=[]
-    @widget_definitions.each {|definition| @widget_categories<<definition.getWidgetCategories()}
+    @widget_definitions.each { |definition| @widget_categories<<definition.getWidgetCategories() }
     @widget_categories=@widget_categories.flatten.uniq.sort
     unless filter_on_category.blank?
-      @widget_definitions=@widget_definitions.select{|definition| definition.getWidgetCategories().to_a.include?(filter_on_category)}
+      @widget_definitions=@widget_definitions.select { |definition| definition.getWidgetCategories().to_a.include?(filter_on_category) }
     end
   end
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/drilldown_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/drilldown_controller.rb
index 809914e1739..ca66a93d92d 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/drilldown_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/drilldown_controller.rb
@@ -37,7 +37,7 @@ class DrilldownController < ApplicationController
     else
       selected_rids=[]
     end
-    selected_rids=selected_rids.map{|r|r.to_i}
+    selected_rids=selected_rids.map { |r| r.to_i }
 
 
     # options
@@ -96,16 +96,16 @@ class DrilldownController < ApplicationController
     else
       @selected_rids=[]
     end
-    @selected_rids=@selected_rids.map{|r|r.to_i}
+    @selected_rids=@selected_rids.map { |r| r.to_i }
 
 
     # options for Drilldown
     options={:exclude_zero_value => true, :period => @period}
     if @rule
-      params[:rule]=@rule.key  # workaround for SONAR-1767 : the javascript hash named "rp" in the HTML source must contain the rule key, but not the rule id
+      params[:rule]=@rule.key # workaround for SONAR-1767 : the javascript hash named "rp" in the HTML source must contain the rule key, but not the rule id
       options[:rule_id]=@rule.id
     end
-    
+
     # load data
     @drilldown = Drilldown.new(@project, @metric, @selected_rids, options)
     @snapshot=@drilldown.snapshot
@@ -126,11 +126,11 @@ class DrilldownController < ApplicationController
     else
       # No filter -> loads all the rules
       metrics=[
-        Metric.by_key("#{metric_prefix}blocker_violations"),
-        Metric.by_key("#{metric_prefix}critical_violations"),
-        Metric.by_key("#{metric_prefix}major_violations"),
-        Metric.by_key("#{metric_prefix}minor_violations"),
-        Metric.by_key("#{metric_prefix}info_violations")
+          Metric.by_key("#{metric_prefix}blocker_violations"),
+          Metric.by_key("#{metric_prefix}critical_violations"),
+          Metric.by_key("#{metric_prefix}major_violations"),
+          Metric.by_key("#{metric_prefix}minor_violations"),
+          Metric.by_key("#{metric_prefix}info_violations")
       ]
       @rule_measures = @snapshot.rule_measures(metrics)
     end
@@ -143,9 +143,8 @@ class DrilldownController < ApplicationController
   def init_project
     project_key = params[:id]
     @project = project_key ? Project.by_key(project_key) : nil
-    if @project.nil?
-      render :text => "Project [#{project_key}] not found", :status => 404
-    end
+    # For security reasons, we must not return 404 not found. It would be an information that the resource exists.
+    not_found("Resource not found") unless @project
   end
 
   def select_metric(metric_key, default_key)
@@ -176,7 +175,7 @@ class DrilldownController < ApplicationController
     hash
   end
 
-  def display_metric_viewers?(resource,metric_key)
+  def display_metric_viewers?(resource, metric_key)
     return true if resource.file?
     java_facade.getResourceTabsForMetric(resource.scope, resource.qualifier, resource.language, metric_key).each do |tab|
       tab.getUserRoles().each do |role|
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/plugins/resource_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/plugins/resource_controller.rb
index b8967a29821..6d9ce960255 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/plugins/resource_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/plugins/resource_controller.rb
@@ -24,7 +24,7 @@ class Plugins::ResourceController < ApplicationController
 
   def index
     @project = ::Project.by_key(params[:id])
-    return redirect_to home_url if @project.nil?
+    not_found("Not found") unless @project
 
     @snapshot=@project.last_snapshot
 
@@ -32,7 +32,7 @@ class Plugins::ResourceController < ApplicationController
     @page_proxy=java_facade.getPage(page_id)
 
     return redirect_to(home_path) unless @page_proxy
-    
+
     authorized=@page_proxy.getUserRoles().size==0
     unless authorized
       @page_proxy.getUserRoles().each do |role|
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/project_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/project_controller.rb
index de14ba3a606..55a85677bfe 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/project_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/project_controller.rb
@@ -18,17 +18,18 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
 #
 class ProjectController < ApplicationController
-  verify :method => :post, :only => [  :set_links, :set_exclusions, :delete_exclusions ], :redirect_to => { :action => :index }
-  verify :method => :delete, :only => [ :delete ], :redirect_to => { :action => :index }
+  verify :method => :post, :only => [:set_links, :set_exclusions, :delete_exclusions], :redirect_to => {:action => :index}
+  verify :method => :delete, :only => [:delete], :redirect_to => {:action => :index}
 
   SECTION=Navigation::SECTION_RESOURCE
-  
+
   def index
-    redirect_to :overwrite_params => {:controller => :dashboard, :action => 'index'} 
+    redirect_to :overwrite_params => {:controller => :dashboard, :action => 'index'}
   end
 
   def deletion
     @project=Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
 
     @snapshot=@project.last_snapshot
@@ -49,32 +50,35 @@ class ProjectController < ApplicationController
 
   def history
     @project=Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
 
     if !(@project.project? || @project.view? || @project.subview?)
       redirect_to :action => 'index', :id => params[:id]
     end
-    
+
     @snapshot=@project.last_snapshot
-    @snapshots = Snapshot.find(:all, :conditions => ["status='P' AND project_id=?", @project.id], 
+    @snapshots = Snapshot.find(:all, :conditions => ["status='P' AND project_id=?", @project.id],
                                :include => 'events', :order => 'snapshots.created_at DESC')
   end
 
   def delete_snapshot_history
     project=Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
-    
+
     sid = params[:snapshot_id]
     if sid
       Snapshot.update_all("status='U'", ["id=? or root_snapshot_id=(?)", sid.to_i, sid.to_i])
       flash[:notice] = message('project_history.snapshot_deleted')
     end
-    
+
     redirect_to :action => 'history', :id => project.id
   end
 
   def links
     @project=Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
 
     @snapshot=@project.last_snapshot
@@ -85,6 +89,7 @@ class ProjectController < ApplicationController
 
   def set_links
     project = Project.by_key(params[:project_id])
+    not_found("Project not found") unless project
     access_denied unless is_admin?(project)
 
     project.links.clear
@@ -109,9 +114,10 @@ class ProjectController < ApplicationController
     redirect_to :action => 'links', :id => project.id
   end
 
-  
+
   def settings
     @project=Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
 
     @snapshot=@project.last_snapshot
@@ -122,7 +128,7 @@ class ProjectController < ApplicationController
     @category=params[:category] ||= 'general'
     @properties_per_category={}
     definitions = java_facade.getPropertyDefinitions()
-    properties = definitions.getProperties().select {|property| (@project.module? && property.module()) || (@project.project? && property.project())}
+    properties = definitions.getProperties().select { |property| (@project.module? && property.module()) || (@project.project? && property.project()) }
     properties.each do |property|
       category = definitions.getCategory(property.key())
       @properties_per_category[category]||=[]
@@ -138,18 +144,19 @@ class ProjectController < ApplicationController
     conditions={:resource_id => @snapshot.project_id}
     conditions[:category]=@category unless @category.blank?
 
-    category_names=@categories.map{|cat| cat.name}
+    category_names=@categories.map { |cat| cat.name }
     @events=Event.find(:all, :conditions => conditions, :order => 'event_date desc').select do |event|
       category_names.include?(event.category)
     end
-    render :action => 'events', :layout => ! request.xhr?
+    render :action => 'events', :layout => !request.xhr?
   end
 
 
   def exclusions
     @project=Project.by_key(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
-  
+
     @snapshot=@project.last_snapshot
     if !@project.project? && !@project.module?
       redirect_to :action => 'index', :id => params[:id]
@@ -158,14 +165,15 @@ class ProjectController < ApplicationController
 
   def set_exclusions
     @project = Project.find(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
 
-    patterns=params['patterns'].reject{|p| p.blank?}.uniq
+    patterns=params['patterns'].reject { |p| p.blank? }.uniq
     if patterns.empty?
       Property.clear('sonar.exclusions', @project.id)
     else
       # Trim spaces in patterns before merging into one String - see http://jira.codehaus.org/browse/SONAR-2261
-      Property.set('sonar.exclusions', patterns.collect{|x| x.strip}.join(','), @project.id)
+      Property.set('sonar.exclusions', patterns.collect { |x| x.strip }.join(','), @project.id)
     end
     flash[:notice]='Filters added'
     redirect_to :action => 'exclusions', :id => @project.id
@@ -173,8 +181,9 @@ class ProjectController < ApplicationController
 
   def delete_exclusions
     @project = Project.find(params[:id])
+    not_found("Project not found") unless @project
     access_denied unless is_admin?(@project)
-    
+
     Property.clear('sonar.exclusions', @project.id)
     flash[:notice]='Filters deleted'
     redirect_to :action => 'exclusions', :id => @project.id
@@ -182,8 +191,9 @@ class ProjectController < ApplicationController
 
   def update_version
     snapshot=Snapshot.find(params[:sid])
-    return access_denied unless is_admin?(snapshot)
-  
+    not_found("Snapshot not found") unless snapshot
+    access_denied unless is_admin?(snapshot)
+
     unless params[:version_name].blank?
       if Event.already_exists(snapshot.id, params[:version_name], EventCategory::KEY_VERSION)
         flash[:error] = message('project_history.version_already_exists', :params => params[:version_name])
@@ -197,41 +207,43 @@ class ProjectController < ApplicationController
         # And then we update/create the event on each snapshot
         if snapshot.event(EventCategory::KEY_VERSION)
           # This is an update: we update all the related events
-          Event.update_all( {:name => params[:version_name]},
-                            ["category = ? AND snapshot_id IN (?)", EventCategory::KEY_VERSION, snapshots.map{|s| s.id}])
+          Event.update_all({:name => params[:version_name]},
+                           ["category = ? AND snapshot_id IN (?)", EventCategory::KEY_VERSION, snapshots.map { |s| s.id }])
           flash[:notice] = message('project_history.version_updated', :params => params[:version_name])
         else
           # We create an event for every concerned snapshot
           snapshots.each do |snapshot|
-            event = Event.create!(:name => params[:version_name], :snapshot => snapshot, 
-                              :resource_id => snapshot.project_id, :category => EventCategory::KEY_VERSION,
-                              :event_date => snapshot.created_at)
+            event = Event.create!(:name => params[:version_name], :snapshot => snapshot,
+                                  :resource_id => snapshot.project_id, :category => EventCategory::KEY_VERSION,
+                                  :event_date => snapshot.created_at)
           end
           flash[:notice] = message('project_history.version_created', :params => params[:version_name])
         end
       end
     end
-  
+
     redirect_to :action => 'history', :id => snapshot.root_project_id
   end
 
   def delete_version
     snapshot=Snapshot.find(params[:sid])
-    return access_denied unless is_admin?(snapshot)
-    
+    not_found("Snapshot not found") unless snapshot
+    access_denied unless is_admin?(snapshot)
+
     event = snapshot.event(EventCategory::KEY_VERSION)
     old_version_name = event.name
     events = find_events(event)
-    Event.delete(events.map {|e| e.id})
-  
+    Event.delete(events.map { |e| e.id })
+
     flash[:notice] = message('project_history.version_removed', :params => old_version_name)
     redirect_to :action => 'history', :id => snapshot.root_project_id
   end
 
   def new_event
     snapshot=Snapshot.find(params[:sid])
-    return access_denied unless is_admin?(snapshot)
-    
+    not_found("Snapshot not found") unless snapshot
+    access_denied unless is_admin?(snapshot)
+
     @event = Event.new(:snapshot => snapshot, :resource => snapshot.resource)
     @categories=EventCategory.categories(false)
     @categories << EventCategory.other_category
@@ -240,8 +252,8 @@ class ProjectController < ApplicationController
 
   def create_event
     event = Event.new(params[:event])
-    return access_denied unless is_admin?(event.resource)
-    
+    access_denied unless is_admin?(event.resource)
+
     if Event.already_exists(event.snapshot_id, event.name, event.category)
       flash[:error] = message('project_history.event_already_exists', :params => [event.name, event.category])
     else
@@ -255,23 +267,25 @@ class ProjectController < ApplicationController
       end
       flash[:notice] = message('project_history.event_created', :params => event.name)
     end
-    
+
     redirect_to :action => 'history', :id => event.resource_id
   end
 
   def edit_event
     @event = Event.find(params[:id])
-    return access_denied unless is_admin?(@event.resource)
-  
+    not_found("Event not found") unless @event
+    access_denied unless is_admin?(@event.resource)
+
     @categories=EventCategory.categories(false)
     @categories << EventCategory.other_category
     render :partial => 'edit_event'
   end
-  
+
   def update_event
     event = Event.find(params[:event][:id])
-    return access_denied unless is_admin?(event.resource)
-    
+    not_found("Event not found") unless event
+    access_denied unless is_admin?(event.resource)
+
     if Event.already_exists(event.snapshot_id, params[:event][:name], params[:event][:category])
       flash[:error] = message('project_history.event_already_exists', :params => [event.name, event.category])
     else
@@ -283,29 +297,30 @@ class ProjectController < ApplicationController
       end
       flash[:notice] = message('project_history.event_updated')
     end
-    
+
     redirect_to :action => 'history', :id => event.resource_id
   end
 
   def delete_event
     event = Event.find(params[:id])
-    return access_denied unless is_admin?(event.resource)
-    
+    not_found("Event not found") unless event
+    access_denied unless is_admin?(event.resource)
+
     name = event.name
     resource_id = event.resource_id
     events = find_events(event)
-    Event.delete(events.map {|e| e.id})
-      
+    Event.delete(events.map { |e| e.id })
+
     flash[:notice] = message('project_history.event_deleted', :params => name)
     redirect_to :action => 'history', :id => resource_id
   end
 
   protected
-  
+
   def find_project_snapshots(root_snapshot_id)
     snapshots = Snapshot.find(:all, :include => 'events', :conditions => ["(root_snapshot_id = ? OR id = ?) AND scope = 'PRJ'", root_snapshot_id, root_snapshot_id])
   end
-  
+
   # Returns all an array that contains the given event + all the events that are the same, but which are attached on the submodules
   def find_events(event)
     events = []
@@ -314,7 +329,7 @@ class ProjectController < ApplicationController
     description = event.description
     snapshots = find_project_snapshots(event.snapshot_id)
     snapshots.each do |snapshot|
-      snapshot.events.reject {|e| e.name!=name || e.category!=category}.each do |event|
+      snapshot.events.reject { |e| e.name!=name || e.category!=category }.each do |event|
         events << event
       end
     end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
index cf399666698..abe5ef95921 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
@@ -25,46 +25,43 @@ class ResourceController < ApplicationController
 
   SECTION=Navigation::SECTION_RESOURCE
   helper :dashboard, SourceHelper
-  
+
   def index
     @resource = Project.by_key(params[:id])
-
-    if (@resource && has_role?(:user, @resource))
-      params[:layout]='false'
-      @snapshot=@resource.last_snapshot
-
-      load_extensions()
-
-      if @extension
-        if (@extension.getId()=='violations')
-          render_violations()
-        elsif (@extension.getId()=='coverage')
-          render_coverage()
-        elsif (@extension.getId()=='source')
-          render_source()
-        elsif (@extension.getId()=='duplications')
-          render_duplications()
-        else
-          render_extension()
-        end
+    not_found("Resource not found") unless @resource
+    access_denied unless has_role?(:user, @resource)
+
+    params[:layout]='false'
+    @snapshot=@resource.last_snapshot
+
+    load_extensions()
+
+    if @extension
+      if (@extension.getId()=='violations')
+        render_violations()
+      elsif (@extension.getId()=='coverage')
+        render_coverage()
+      elsif (@extension.getId()=='source')
+        render_source()
+      elsif (@extension.getId()=='duplications')
+        render_duplications()
       else
-        render_nothing()
+        render_extension()
       end
     else
-      access_denied
+      render_nothing()
     end
   end
-  
+
   def show_duplication_snippet
     resource = Project.by_key(params[:id])
-    if (resource && has_role?(:user, resource))
-      original_resource = Project.by_key(params[:original_resource_id])
-      render :partial => 'duplications_source_snippet', 
-             :locals => {:resource => resource, :original_resource => original_resource, :from_line => params[:from_line].to_i, :to_line => params[:to_line].to_i, :lines_count => params[:lines_count].to_i, 
-                         :group_index => params[:group_index], :external => (resource.root_id != original_resource.root_id)}
-    else
-      access_denied
-    end
+    not_found("Resource not found") unless resource
+    access_denied unless has_role?(:user, resource)
+
+    original_resource = Project.by_key(params[:original_resource_id])
+    render :partial => 'duplications_source_snippet',
+           :locals => {:resource => resource, :original_resource => original_resource, :from_line => params[:from_line].to_i, :to_line => params[:to_line].to_i, :lines_count => params[:lines_count].to_i,
+                       :group_index => params[:group_index], :external => (resource.root_id != original_resource.root_id)}
   end
 
 
@@ -86,13 +83,13 @@ class ResourceController < ApplicationController
     end
 
     if !params[:tab].blank?
-      @extension=@extensions.find{|extension| extension.getId()==params[:tab]}
+      @extension=@extensions.find { |extension| extension.getId()==params[:tab] }
 
     elsif !params[:metric].blank?
       metric=Metric.by_key(params[:metric])
-      @extension=@extensions.find{|extension| extension.getDefaultTabForMetrics().include?(metric.key)}
+      @extension=@extensions.find { |extension| extension.getDefaultTabForMetrics().include?(metric.key) }
     end
-    @extension=@extensions.find{|extension| extension.isDefaultTab()} if @extension==nil
+    @extension=@extensions.find { |extension| extension.isDefaultTab() } if @extension==nil
   end
 
   def load_sources
@@ -112,7 +109,7 @@ class ResourceController < ApplicationController
         line.author=@authors_by_line[index+1]
 
         date_string=@dates_by_line[index+1]
-        line.datetime=(date_string ? Java::OrgSonarApiUtils::DateUtils.parseDateTime(date_string): nil)
+        line.datetime=(date_string ? Java::OrgSonarApiUtils::DateUtils.parseDateTime(date_string) : nil)
       end
     end
   end
@@ -132,7 +129,7 @@ class ResourceController < ApplicationController
   def render_coverage
     load_sources()
     @display_coverage = true
-    @display_it_coverage = (! @snapshot.measure('it_coverage').nil?)
+    @display_it_coverage = (!@snapshot.measure('it_coverage').nil?)
     @expandable = (@lines!=nil)
     if @lines
       metric = Metric.by_key(params[:coverage_filter]||params[:metric])
@@ -143,7 +140,7 @@ class ResourceController < ApplicationController
       @conditions_by_line = load_distribution("#{it_prefix}conditions_by_line")
       @covered_conditions_by_line = load_distribution("#{it_prefix}covered_conditions_by_line")
 
-      @hits_by_line.each_pair do |line_id,hits|
+      @hits_by_line.each_pair do |line_id, hits|
         line = @lines[line_id-1]
         if line
           line.hits = hits.to_i
@@ -155,7 +152,7 @@ class ResourceController < ApplicationController
       if @snapshot.measure("#{it_prefix}conditions_by_line").nil?
         # TODO remove this code when branch_coverage_hits_data is fully removed from CoreMetrics
         deprecated_branches_by_line = load_distribution("#{it_prefix}branch_coverage_hits_data")
-        deprecated_branches_by_line.each_pair do |line_id,label|
+        deprecated_branches_by_line.each_pair do |line_id, label|
           line = @lines[line_id-1]
           if line
             line.deprecated_conditions_label = label
@@ -170,33 +167,33 @@ class ResourceController < ApplicationController
           'it_lines_to_cover'==@coverage_filter || 'it_coverage'==@coverage_filter || 'it_line_coverage'==@coverage_filter ||
           'new_it_lines_to_cover'==@coverage_filter || 'new_it_coverage'==@coverage_filter || 'new_it_line_coverage'==@coverage_filter)
         @coverage_filter = "#{it_prefix}lines_to_cover"
-        filter_lines{|line| line.hits && line.after(to)}
+        filter_lines { |line| line.hits && line.after(to) }
 
       elsif ('uncovered_lines'==@coverage_filter || 'new_uncovered_lines'==@coverage_filter ||
-            'it_uncovered_lines'==@coverage_filter || 'new_it_uncovered_lines'==@coverage_filter)
+          'it_uncovered_lines'==@coverage_filter || 'new_it_uncovered_lines'==@coverage_filter)
         @coverage_filter = "#{it_prefix}uncovered_lines"
-        filter_lines{|line| line.hits && line.hits==0 && line.after(to)}
+        filter_lines { |line| line.hits && line.hits==0 && line.after(to) }
 
       elsif ('conditions_to_cover'==@coverage_filter || 'branch_coverage'==@coverage_filter ||
-            'new_conditions_to_cover'==@coverage_filter || 'new_branch_coverage'==@coverage_filter ||
-            'it_conditions_to_cover'==@coverage_filter || 'it_branch_coverage'==@coverage_filter ||
-            'new_it_conditions_to_cover' == @coverage_filter || 'new_it_branch_coverage'==@coverage_filter)
+          'new_conditions_to_cover'==@coverage_filter || 'new_branch_coverage'==@coverage_filter ||
+          'it_conditions_to_cover'==@coverage_filter || 'it_branch_coverage'==@coverage_filter ||
+          'new_it_conditions_to_cover' == @coverage_filter || 'new_it_branch_coverage'==@coverage_filter)
         @coverage_filter="#{it_prefix}conditions_to_cover"
-        filter_lines{|line| line.conditions && line.conditions>0 && line.after(to)}
+        filter_lines { |line| line.conditions && line.conditions>0 && line.after(to) }
 
       elsif ('uncovered_conditions' == @coverage_filter || 'new_uncovered_conditions' == @coverage_filter ||
-            'it_uncovered_conditions'==@coverage_filter || 'new_it_uncovered_conditions' == @coverage_filter)
+          'it_uncovered_conditions'==@coverage_filter || 'new_it_uncovered_conditions' == @coverage_filter)
         @coverage_filter="#{it_prefix}uncovered_conditions"
-        filter_lines{|line| line.conditions && line.covered_conditions && line.covered_conditions<line.conditions && line.after(to)}
+        filter_lines { |line| line.conditions && line.covered_conditions && line.covered_conditions<line.conditions && line.after(to) }
       end
     end
     render :action => 'index', :layout => !request.xhr?
   end
-  
-  
+
+
   def render_duplications
     duplications_data = @snapshot.measure('duplications_data');
-    
+
     # create duplication groups
     @duplication_groups = []
     if duplications_data && duplications_data.measure_data && duplications_data.measure_data.data
@@ -207,7 +204,7 @@ class ResourceController < ApplicationController
         parse_duplications_old_format(dups, @duplication_groups)
       end
     end
-    
+
     # And sort them 
     @duplication_groups.each do |group|
       group.sort! do |dup1, dup2|
@@ -233,8 +230,8 @@ class ResourceController < ApplicationController
         end
       end
     end
-    @duplication_groups.sort! {|group1, group2| group1[0][:from_line].to_i <=> group2[0][:from_line].to_i}
-    
+    @duplication_groups.sort! { |group1, group2| group1[0][:from_line].to_i <=> group2[0][:from_line].to_i }
+
     render :action => 'index', :layout => !request.xhr?
   end
 
@@ -256,7 +253,7 @@ class ResourceController < ApplicationController
       duplication_groups << dup_group
     end
   end
-  
+
   def parse_duplications_old_format(dups, duplication_groups)
     resource_by_key = {}
     dups.elements.each("duplications/duplication") do |dup|
@@ -273,8 +270,8 @@ class ResourceController < ApplicationController
       duplication_groups << group
     end
   end
-  
-  
+
+
   def render_violations
     load_sources()
     @display_violations=true
@@ -314,7 +311,7 @@ class ResourceController < ApplicationController
       end
     end
 
-    RuleFailure.find(:all, :include => ['rule', 'review' ], :conditions => [conditions] + values, :order => 'failure_level DESC').each do |violation|
+    RuleFailure.find(:all, :include => ['rule', 'review'], :conditions => [conditions] + values, :order => 'failure_level DESC').each do |violation|
       # sorted by severity => from blocker to info
       if violation.line && violation.line>0 && @lines
         @lines[violation.line-1].add_violation(violation)
@@ -324,12 +321,12 @@ class ResourceController < ApplicationController
     end
 
     if !@expanded && @lines
-      filter_lines{|line| line.violations?}
+      filter_lines { |line| line.violations? }
     end
     render :action => 'index', :layout => !request.xhr?
   end
-  
-  
+
+
   def render_source
     load_sources()
     filter_lines_by_date()
@@ -350,7 +347,7 @@ class ResourceController < ApplicationController
   end
 
   def filter_lines(&block)
-    @lines.each_with_index do |line,index|
+    @lines.each_with_index do |line, index|
       if yield(line)
         for i in index-4...index
           @lines[i].flag_as_highlight_context() if i>=0
@@ -429,15 +426,15 @@ class ResourceController < ApplicationController
     def deprecated_conditions_label=(label)
       if label
         @deprecated_conditions_label=label
-          if label=='0%'
-            @conditions=2
-            @covered_conditions=0
-          elsif label=='100%'
-            @conditions=2
-            @covered_conditions=2
-          else
-            @conditions=2
-            @covered_conditions=1
+        if label=='0%'
+          @conditions=2
+          @covered_conditions=0
+        elsif label=='100%'
+          @conditions=2
+          @covered_conditions=2
+        else
+          @conditions=2
+          @covered_conditions=1
         end
       end
     end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/widget_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/widget_controller.rb
index 0d3ba47c081..1c783aa92e9 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/widget_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/widget_controller.rb
@@ -28,15 +28,15 @@ class WidgetController < ApplicationController
     params[:layout]='false'
     render :action => 'index'
   end
- 
+
   private
 
   def load_resource
     @resource=Project.by_key(params[:resource])
-    not_found('Unknown resource') unless @resource
-    
-    @project=@resource
+    not_found("Resource not found") unless @resource
     access_denied unless has_role?(:user, @resource)
+
+    @project=@resource
     @snapshot = @resource.last_snapshot
   end