From 62c5418f3b4a5b7a7d1d7e9979a746e278b79c5b Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terceiro@colivre.coop.br>
Date: Fri, 5 Feb 2010 15:35:36 -0300
Subject: [PATCH] Rendering a "not found" message for missing docs

---
 app/controllers/public/doc_controller.rb |  5 +++++
 app/models/doc_item.rb                   |  1 +
 app/models/doc_section.rb                |  9 +++++++--
 app/models/doc_topic.rb                  |  3 +++
 test/functional/doc_controller_test.rb   |  6 ++++++
 test/unit/doc_section_test.rb            |  6 ++++--
 test/unit/doc_topic_test.rb              |  7 +++++++
 7 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/app/controllers/public/doc_controller.rb b/app/controllers/public/doc_controller.rb
index 20dad86..75747d9 100644
--- a/app/controllers/public/doc_controller.rb
+++ b/app/controllers/public/doc_controller.rb
@@ -19,6 +19,11 @@ class DocController < PublicController
     @topic = @section.find(params[:topic])
   end
 
+  rescue_from DocItem::NotFound, :with => :not_found
+  def not_found
+    render_not_found
+  end
+
   protected
 
   def load_toc
diff --git a/app/models/doc_item.rb b/app/models/doc_item.rb
index 6738260..43d3dc7 100644
--- a/app/models/doc_item.rb
+++ b/app/models/doc_item.rb
@@ -1,4 +1,5 @@
 class DocItem
+  class  NotFound < Exception; end
   attr_accessor :id, :title, :text, :language
   def initialize(attrs = {})
     attrs.each do |name,value|
diff --git a/app/models/doc_section.rb b/app/models/doc_section.rb
index 694f074..68b8f82 100644
--- a/app/models/doc_section.rb
+++ b/app/models/doc_section.rb
@@ -20,7 +20,12 @@ class DocSection < DocItem
     if id.blank?
       root(language)
     else
-      all(language, force).find {|item| item.id == id }
+      section = all(language, force).find {|item| item.id == id }
+      if section
+        section
+      else
+        raise DocItem::NotFound
+      end
     end
   end
 
@@ -50,7 +55,7 @@ class DocSection < DocItem
     [
       File.join(dir, "#{id}#{language_suffix}.xhtml"),
       File.join(dir, "#{id}.en.xhtml")
-    ].find {|file| File.exist?(file) }
+    ].find {|file| File.exist?(file) } || raise(DocItem::NotFound)
   end
 
   def load_items
diff --git a/app/models/doc_topic.rb b/app/models/doc_topic.rb
index 2b93e62..ae4a3e8 100644
--- a/app/models/doc_topic.rb
+++ b/app/models/doc_topic.rb
@@ -1,5 +1,8 @@
 class DocTopic < DocItem
   def self.loadfile(file)
+    if !File.exist?(file)
+      raise DocItem::NotFound
+    end
     lines = File.readlines(file) 
     title = _find_title(lines)
     File.basename(file) =~ /(.*)\.([^\.]+)\.xhtml$/
diff --git a/test/functional/doc_controller_test.rb b/test/functional/doc_controller_test.rb
index 6bd168d..9c82f55 100644
--- a/test/functional/doc_controller_test.rb
+++ b/test/functional/doc_controller_test.rb
@@ -40,5 +40,11 @@ class DocControllerTest < ActionController::TestCase
     assert_equal 'pt', assigns(:topic).language
   end
 
+  should 'bail out gracefully for unexisting sections or topics' do
+    assert_nothing_raised do
+      get :section, :section => 'something-very-unlikely'
+      get :section, :section => 'something-very-unlikely', :topic => 'other-thing-very-unlikely'
+    end
+  end
 
 end
diff --git a/test/unit/doc_section_test.rb b/test/unit/doc_section_test.rb
index 18e73ca..a0c2bcc 100644
--- a/test/unit/doc_section_test.rb
+++ b/test/unit/doc_section_test.rb
@@ -91,8 +91,10 @@ class DocSectionTest < ActiveSupport::TestCase
     end
   end
 
-  should 'not load null section (the root) for unexisting sections' do
-    assert_nil DocSection.find('something-very-unlikely')
+  should 'raise DocItem::NotFound when loading unexisting section' do
+    assert_raise DocItem::NotFound do
+      DocSection.find('something-very-unlikely')
+    end
   end
 
 end
diff --git a/test/unit/doc_topic_test.rb b/test/unit/doc_topic_test.rb
index 745bc6d..47d9fb2 100644
--- a/test/unit/doc_topic_test.rb
+++ b/test/unit/doc_topic_test.rb
@@ -18,4 +18,11 @@ class DocTopicTest < ActiveSupport::TestCase
     assert_equal 'pt', doc.language
     assert_equal 'Teste da documentação', doc.title
   end
+
+  should 'raise DocTopic::NotFound when trying to load an unexisting topic' do
+    assert_raise DocItem::NotFound do
+      DocTopic.loadfile('/path/to/unexisting/file.en.xhtml')
+    end
+  end
+
 end
--
libgit2 0.21.2