From cd41bd6779a388f2265b721fdfc74fcdcd611c29 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terceiro@colivre.coop.br>
Date: Tue, 27 Oct 2009 18:06:32 -0300
Subject: [PATCH] Rewriting feed updater code

---
 app/models/block.rb                                                  |   2 ++
 app/models/external_feed.rb                                          |  10 ++++++++--
 app/models/feed_reader_block.rb                                      |  27 +++++++++++++++++++++++++--
 db/migrate/075_add_new_feed_stuff.rb                                 |  40 ++++++++++++++++++++++++++++++++++++++++
 db/schema.rb                                                         |  31 +++++++++++++++++++++----------
 doc/README_FOR_APP                                                   |   3 ++-
 lib/feed_handler.rb                                                  |  76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------
 lib/feed_updater.rb                                                  |  87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 script/feed-updater                                                  |  35 +++++++++++++++++++++++------------
 script/production                                                    |   5 ++---
 test/factories.rb                                                    |  76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/test_helper.rb                                                  |   4 ++++
 test/unit/block_test.rb                                              |   7 +++++++
 test/unit/external_feed_test.rb                                      | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
 test/unit/feed_handler_test.rb                                       |  43 +++++++++++++++++++++++++++++++++++++++----
 test/unit/feed_reader_block_test.rb                                  |  92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 test/unit/feed_updater_test.rb                                       |  38 ++++++++++++++++++++++++++++++++++++++
 test/unit/tiny_mce_article_test.rb                                   |   6 ++++++
 vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb |   2 +-
 19 files changed, 614 insertions(+), 84 deletions(-)
 create mode 100644 db/migrate/075_add_new_feed_stuff.rb
 create mode 100644 lib/feed_updater.rb
 create mode 100644 test/factories.rb
 create mode 100644 test/unit/feed_updater_test.rb

diff --git a/app/models/block.rb b/app/models/block.rb
index b0db40a..77541a3 100644
--- a/app/models/block.rb
+++ b/app/models/block.rb
@@ -13,6 +13,8 @@ class Block < ActiveRecord::Base
   acts_as_having_settings
   settings_items :visible, :type => :boolean, :default => true
 
+  named_scope :enabled, :conditions => { :enabled => true }
+
   def visible?
     visible
   end
diff --git a/app/models/external_feed.rb b/app/models/external_feed.rb
index 9d8f8bb..6513346 100644
--- a/app/models/external_feed.rb
+++ b/app/models/external_feed.rb
@@ -5,6 +5,11 @@ class ExternalFeed < ActiveRecord::Base
   validates_presence_of :address, :if => lambda {|efeed| efeed.enabled}
   validates_uniqueness_of :blog_id
 
+  named_scope :enabled, :conditions => { :enabled => true }
+  named_scope :expired, lambda {
+    { :conditions => ['(fetched_at is NULL) OR (fetched_at < ?)', Time.now - FeedUpdater.update_interval] }
+  }
+
   def add_item(title, link, date, content)
     article = TinyMceArticle.new(:name => title, :profile => blog.profile, :body => content, :published_at => date, :source => link, :profile => blog.profile, :parent => blog)
     unless blog.children.exists?(:slug => article.slug)
@@ -16,9 +21,10 @@ class ExternalFeed < ActiveRecord::Base
     # do nothing
   end
   def finish_fetch
-    if self.only_once
-      self.enabled = 'false'
+    if self.only_once && self.update_errors.zero?
+      self.enabled = false
     end
+    self.fetched_at = Time.now
     self.save!
   end
 
diff --git a/app/models/feed_reader_block.rb b/app/models/feed_reader_block.rb
index 2d6e3bf..cc04bf8 100644
--- a/app/models/feed_reader_block.rb
+++ b/app/models/feed_reader_block.rb
@@ -1,14 +1,32 @@
 class FeedReaderBlock < Block
 
+  def initialize(attributes = nil)
+    data = attributes || {}
+    super({ :enabled => !data[:address].blank? }.merge(data))
+  end
+
   include DatesHelper
 
   settings_items :address, :type => :string
+  alias :orig_set_address :address=
+  def address=(new_address)
+    old_address = address
+    orig_set_address(new_address)
+    self.enabled = (old_address.blank? && !new_address.blank?) || (new_address && new_address != old_address) || false
+  end
+
   settings_items :limit, :type => :integer
-  settings_items :fetched_at, :type => :date
 
   settings_items :feed_title, :type => :string
   settings_items :feed_items, :type => :array
 
+  settings_items :update_errors, :type => :integer, :default => 0
+  settings_items :error_message, :type => :string
+
+  named_scope :expired, lambda {
+    { :conditions => [ '(fetched_at is NULL) OR (fetched_at < ?)', Time.now - FeedUpdater.update_interval] }
+  }
+
   before_create do |block|
     block.limit = 5
     block.feed_items = []
@@ -27,9 +45,13 @@ class FeedReaderBlock < Block
   end
 
   def formatted_feed_content
-    return "<ul>\n" +
+    if error_message.blank?
+      "<ul>\n" +
       self.feed_items[0..(limit-1)].map{ |item| "<li><a href='#{item[:link]}'>#{item[:title]}</a></li>" }.join("\n") +
       "</ul>"
+    else
+      '<p>' + error_message + '</p>'
+    end
   end
 
   def footer
@@ -49,6 +71,7 @@ class FeedReaderBlock < Block
     self.feed_title = nil
   end
   def finish_fetch
+    self.fetched_at = Time.now
     self.save!
   end
 
diff --git a/db/migrate/075_add_new_feed_stuff.rb b/db/migrate/075_add_new_feed_stuff.rb
new file mode 100644
index 0000000..51bffb5
--- /dev/null
+++ b/db/migrate/075_add_new_feed_stuff.rb
@@ -0,0 +1,40 @@
+class AddNewFeedStuff < ActiveRecord::Migration
+
+  def self.up
+    add_column :blocks, :enabled, :boolean, :default => true
+    execute('update blocks set enabled = (1=1)')
+
+    add_column :blocks, :created_at, :datetime
+    add_column :blocks, :updated_at, :datetime
+    add_column :blocks, :fetched_at, :datetime
+    execute("update blocks set created_at = '2009-10-23 17:00', updated_at = '2009-10-23 17:00'")
+
+    add_index :blocks, :enabled
+    add_index :blocks, :fetched_at
+    add_index :blocks, :type
+
+    add_column :external_feeds, :error_message, :text
+    add_column :external_feeds, :update_errors, :integer, :default => 0
+    execute('update external_feeds set update_errors = 0')
+
+    add_index :external_feeds, :enabled
+    add_index :external_feeds, :fetched_at
+  end
+
+  def self.down
+    remove_index :blocks, :enabled
+    remove_index :blocks, :fetched_at
+    remove_index :blocks, :type
+    remove_column :blocks, :enabled
+    remove_column :blocks, :updated_at
+    remove_column :blocks, :created_at
+    remove_column :blocks, :fetched_at
+
+    remove_index :external_feeds, :enabled
+    remove_index :external_feeds, :fetched_at
+    remove_column :external_feeds, :error_message
+    remove_column :external_feeds, :update_errors
+  end
+
+end
+
diff --git a/db/schema.rb b/db/schema.rb
index 942106e..4550de8 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 => 74) do
+ActiveRecord::Schema.define(:version => 75) do
 
   create_table "article_versions", :force => true do |t|
     t.integer  "article_id"
@@ -88,8 +88,8 @@ ActiveRecord::Schema.define(:version => 74) do
     t.boolean "virtual",     :default => false
   end
 
-  add_index "articles_categories", ["category_id"], :name => "index_articles_categories_on_category_id"
   add_index "articles_categories", ["article_id"], :name => "index_articles_categories_on_article_id"
+  add_index "articles_categories", ["category_id"], :name => "index_articles_categories_on_category_id"
 
   create_table "blocks", :force => true do |t|
     t.string   "title"
@@ -97,10 +97,16 @@ ActiveRecord::Schema.define(:version => 74) do
     t.string   "type"
     t.text     "settings"
     t.integer  "position"
-    t.datetime "last_updated"
+    t.boolean  "enabled",    :default => true
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.datetime "fetched_at"
   end
 
   add_index "blocks", ["box_id"], :name => "index_blocks_on_box_id"
+  add_index "blocks", ["enabled"], :name => "index_blocks_on_enabled"
+  add_index "blocks", ["fetched_at"], :name => "index_blocks_on_fetched_at"
+  add_index "blocks", ["type"], :name => "index_blocks_on_type"
 
   create_table "boxes", :force => true do |t|
     t.string  "owner_type"
@@ -108,7 +114,7 @@ ActiveRecord::Schema.define(:version => 74) do
     t.integer "position"
   end
 
-  add_index "boxes", ["owner_type", "owner_id"], :name => "index_boxes_on_owner_type_and_owner_id"
+  add_index "boxes", ["owner_id", "owner_type"], :name => "index_boxes_on_owner_type_and_owner_id"
 
   create_table "categories", :force => true do |t|
     t.string  "name"
@@ -172,15 +178,20 @@ ActiveRecord::Schema.define(:version => 74) do
 
   create_table "external_feeds", :force => true do |t|
     t.string   "feed_title"
+    t.date     "fetched_at"
     t.string   "address"
-    t.integer  "blog_id",                      :null => false
-    t.boolean  "enabled",    :default => true, :null => false
-    t.boolean  "only_once",  :default => true, :null => false
+    t.integer  "blog_id",                         :null => false
+    t.boolean  "enabled",       :default => true, :null => false
+    t.boolean  "only_once",     :default => true, :null => false
     t.datetime "created_at"
     t.datetime "updated_at"
-    t.datetime "fetched_at"
+    t.text     "error_message"
+    t.integer  "update_errors", :default => 0
   end
 
+  add_index "external_feeds", ["enabled"], :name => "index_external_feeds_on_enabled"
+  add_index "external_feeds", ["fetched_at"], :name => "index_external_feeds_on_fetched_at"
+
   create_table "favorite_enteprises_people", :id => false, :force => true do |t|
     t.integer "person_id"
     t.integer "enterprise_id"
@@ -281,9 +292,9 @@ ActiveRecord::Schema.define(:version => 74) do
 
   create_table "roles", :force => true do |t|
     t.string  "name"
-    t.text    "permissions"
     t.string  "key"
     t.boolean "system",         :default => false
+    t.text    "permissions"
     t.integer "environment_id"
   end
 
@@ -294,8 +305,8 @@ ActiveRecord::Schema.define(:version => 74) do
     t.datetime "created_at"
   end
 
-  add_index "taggings", ["taggable_id", "taggable_type"], :name => "index_taggings_on_taggable_id_and_taggable_type"
   add_index "taggings", ["tag_id"], :name => "index_taggings_on_tag_id"
+  add_index "taggings", ["taggable_id", "taggable_type"], :name => "index_taggings_on_taggable_id_and_taggable_type"
 
   create_table "tags", :force => true do |t|
     t.string  "name"
diff --git a/doc/README_FOR_APP b/doc/README_FOR_APP
index c377fdb..ad6aed4 100644
--- a/doc/README_FOR_APP
+++ b/doc/README_FOR_APP
@@ -23,12 +23,13 @@ You need to have git installed, as well as:
 * contacts: http://github.com/cardmagic/contacts/tree/master
 * iso-codes: http://pkg-isocodes.alioth.debian.org/
 * feedparser: http://packages.debian.org/sid/libfeedparser-ruby
+* Daemons - http://daemons.rubyforge.org/
 * Mongrel: http://mongrel.rubyforge.org/
 * tango-icon-theme: http://tango.freedesktop.org/Tango_Icon_Library
 
 There are Debian packages available for all of them but contacts. Try:
 
- # aptitude install subversion ruby rake libgettext-ruby1.8 libsqlite3-ruby rcov librmagick-ruby libredcloth-ruby libwill-paginate-ruby iso-codes libfeedparser-ruby libferret-ruby mongrel mongrel-cluster tango-icon-theme
+ # aptitude install subversion ruby rake libgettext-ruby1.8 libsqlite3-ruby rcov librmagick-ruby libredcloth-ruby libwill-paginate-ruby iso-codes libfeedparser-ruby libferret-ruby libdaemons-ruby mongrel mongrel-cluster tango-icon-theme
 
 contacts is bundled together with noosfero for now, so you don't need to install it.
 
diff --git a/lib/feed_handler.rb b/lib/feed_handler.rb
index d6ac291..ddfc54f 100644
--- a/lib/feed_handler.rb
+++ b/lib/feed_handler.rb
@@ -1,8 +1,25 @@
 require 'feedparser'
 require 'open-uri'
 
+# This class is responsible for processing feeds and pass the items to the
+# respective container.
+#
+# The <tt>max_errors</tt> attribute controls how many times it will retry in
+# case of failure. If a feed fails for <tt>max_errors+1</tt> times, it will be
+# disabled and the last error message will be recorder in the container.
+# The default value is *6*, if you need to change it you can do that in your
+# config/local.rb file like this:
+#
+#   FeedHandler.max_errors = 10
+#
+# For the update interval, see FeedUpdater.
 class FeedHandler
 
+  # The maximum number
+  cattr_accessor :max_errors
+
+  self.max_errors = 6
+
   def parse(content)
     raise FeedHandler::ParseError, "Content is nil" if content.nil?
     begin
@@ -16,42 +33,61 @@ class FeedHandler
     begin
       content = ""
       block = lambda { |s| content = s.read }
-      content = if is_web_address?(address)
-        open( address, "User-Agent" => "Noosfero/#{Noosfero::VERSION}", &block )
-      else
-        open_uri_original_open(address, &block)
-      end
+      content =
+        if RAILS_ENV == 'test' && File.exists?(address)
+          File.read(address)
+        else
+          if !valid_url?(address)
+            raise InvalidUrl.new("\"%s\" is not a valid URL" % address)
+          end
+          open(address, "User-Agent" => "Noosfero/#{Noosfero::VERSION}", &block)
+        end
       return content
     rescue Exception => ex
-      raise FeedHandler::FetchError, ex.to_s
+      raise FeedHandler::FetchError, ex.message
     end
   end
 
   def process(container)
-    container.class.transaction do
-      container.clear
-      content = fetch(container.address)
-      container.fetched_at = Time.now
-      parsed_feed = parse(content)
-      container.feed_title = parsed_feed.title
-      parsed_feed.items[0..container.limit-1].reverse.each do |item|
-        container.add_item(item.title, item.link, item.date, item.content)
+    RAILS_DEFAULT_LOGGER.info("Processing %s with id = %d" % [container.class.name, container.id])
+    begin
+      container.class.transaction do
+        actually_process_container(container)
+        container.update_errors = 0
+        container.finish_fetch
+      end
+    rescue Exception => exception
+      RAILS_DEFAULT_LOGGER.warn("Unknown error from %s ID %d\n%s" % [container.class.name, container.id, exception.to_s])
+      RAILS_DEFAULT_LOGGER.warn("Backtrace:\n%s" % exception.backtrace.join("\n"))
+      container.reload
+      container.update_errors += 1
+      container.error_message = exception.to_s
+      if container.update_errors > FeedHandler.max_errors
+        container.enabled = false
       end
       container.finish_fetch
     end
   end
 
+  class InvalidUrl < Exception; end
   class ParseError < Exception; end
   class FetchError < Exception; end
 
   protected
 
-  # extracted from the open implementation in the open-uri library
-  def is_web_address?(address)
-    address.respond_to?(:open) ||
-      address.respond_to?(:to_str) &&
-      (%r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ address) &&
-      URI.parse(address).respond_to?(:open)
+  def actually_process_container(container)
+    container.clear
+    content = fetch(container.address)
+    container.fetched_at = Time.now
+    parsed_feed = parse(content)
+    container.feed_title = parsed_feed.title
+    parsed_feed.items[0..container.limit-1].reverse.each do |item|
+      container.add_item(item.title, item.link, item.date, item.content)
+    end
+  end
+
+  def valid_url?(url)
+    url =~ URI.regexp('http') || url =~ URI.regexp('https')
   end
 
 end
diff --git a/lib/feed_updater.rb b/lib/feed_updater.rb
new file mode 100644
index 0000000..cc48b03
--- /dev/null
+++ b/lib/feed_updater.rb
@@ -0,0 +1,87 @@
+# to run by hand
+if $PROGRAM_NAME == __FILE__
+  require File.dirname(__FILE__) + '/../config/environment'
+end
+
+# This class implements the feed updater. To change how often a feed gets
+# updated, change FeedUpdater#update_interval in your config/local.rb file like
+# this:
+#
+#   FeedUpdater.update_interval = 24.hours
+#
+# You can also customize the time between update runs setting
+# FeedUpdater#daemon_sleep_interval. Give it an integer representing the number
+# of seconds to wait between runs in your config/local.rb:
+#
+#   FeedUpdater.daemon_sleep_interval = 10
+#
+# The feed updaters is controlled by script/feed-updater, which starts and
+# stops the process.
+class FeedUpdater
+
+  # indicates how much time one feed will be left without updates
+  # (ActiveSupport::Duration). Default: <tt>4.hours</tt>
+  cattr_accessor :update_interval
+  self.update_interval = 4.hours
+
+  # indicates for how much time the daemon sleeps before looking for new feeds
+  # to load (in seconds, an integer). Default: 30
+  cattr_accessor :daemon_sleep_interval
+  self.daemon_sleep_interval = 30
+
+  attr_accessor :running
+
+  def initialize
+    self.running = true
+  end
+
+  def start
+    ['TERM', 'INT'].each do |signal|
+      Signal.trap(signal) do
+        stop
+        RAILS_DEFAULT_LOGGER.info("Feed updater exiting gracefully ...")
+      end
+    end
+    run
+    RAILS_DEFAULT_LOGGER.info("Feed updater exited.")
+  end
+
+  def run
+    while running
+      process_round
+      wait
+    end
+  end
+
+  def wait
+    i = 0
+    while running && i < FeedUpdater.daemon_sleep_interval
+      sleep 1
+      i += 1
+    end
+  end
+
+  def stop
+    self.running = false
+  end
+
+  def process_round
+    feed_handler = FeedHandler.new
+    [FeedReaderBlock, ExternalFeed].each do |source|
+      if !running
+        break
+      end
+      source.enabled.expired.all.each do |container|
+        if !running
+          break
+        end
+        feed_handler.process(container)
+      end
+    end
+  end
+end
+
+# run the updater
+if ($PROGRAM_NAME == __FILE__)
+  FeedUpdater.new.start
+end
diff --git a/script/feed-updater b/script/feed-updater
index 13556e5..5ad61a0 100755
--- a/script/feed-updater
+++ b/script/feed-updater
@@ -1,14 +1,25 @@
 #!/usr/bin/env ruby
-require File.dirname(__FILE__) + '/../config/environment'
-
-(FeedReaderBlock.find(:all) + ExternalFeed.find(:all, :conditions => {:enabled => true})).each do |container|
-  unless container.address.nil?
-    begin
-      handler = FeedHandler.new
-      handler.process(container)
-    rescue Exception => ex
-      $stderr.puts("Unknown error from %s ID %d\n%s" % [container.class.name, container.id, ex.to_s])
-      $stderr.puts("Backtrace:\n%s" % ex.backtrace.join("\n"))
-    end
-  end
+
+# This is the Noosfero feed updater controller script. It starts and stops the
+# feed updater daemon, which is implemented in the FeedUpdater class.
+#
+# The role of this script is to just start/stop the daemon, write a PID file,
+# etc. The actual feed update logic is in FeedUpdater.
+
+require 'daemons'
+
+NOOSFERO_ROOT = File.expand_path(File.dirname(__FILE__) + '/../')
+
+options = {
+  :dir_mode   => :normal,
+  :dir        => File.dirname(__FILE__) + '/../tmp/pids',
+  :multiple   => false,
+  :backtrace  => true,
+  :monitor    => true,
+}
+
+Daemons.run_proc('feed-updater', options) do
+  require NOOSFERO_ROOT + '/config/environment'
+  FeedUpdater.new.start
 end
+
diff --git a/script/production b/script/production
index 480be76..4d6dbde 100755
--- a/script/production
+++ b/script/production
@@ -13,14 +13,13 @@ do_start() {
   fi
 
   ./script/ferret_server -e $RAILS_ENV start
-  sleep 1
+  ./script/feed-updater start
   mongrel_rails cluster::start
-  sleep 3
 }
 
 do_stop() {
   mongrel_rails cluster::stop
-  sleep 1
+  ./script/feed-updater stop
   ./script/ferret_server -e $RAILS_ENV stop
 }
 
diff --git a/test/factories.rb b/test/factories.rb
new file mode 100644
index 0000000..99c598f
--- /dev/null
+++ b/test/factories.rb
@@ -0,0 +1,76 @@
+module Noosfero::Factory
+  
+  def fast_create(name, attrs = {})
+    obj = build(name, attrs)
+    obj.attributes.keys.each do |attr|
+      if !obj.column_for_attribute(attr).null && obj.send(attr).nil?
+        obj.send("#{attr}=", factory_num_seq)
+      end
+    end
+    obj.save_without_validation!
+    obj
+  end
+
+  def create(name, attrs = {})
+    target = 'create_' + name.to_s
+    if respond_to?(target)
+      send(target, attrs)
+    else
+      obj = build(name, attrs)
+      obj.save!
+      obj
+    end
+  end
+
+  def build(name, attrs = {})
+    data = 
+      if respond_to?('defaults_for_' + name.to_s)
+        send('defaults_for_'+ name.to_s).merge(attrs)
+      else
+        attrs
+      end
+    eval(name.to_s.camelize).new(data)
+  end
+
+  def self.num_seq
+    @num_seq ||= 0
+    @num_seq += 1
+    @num_seq
+  end
+
+  protected
+
+  def factory_num_seq
+    Noosfero::Factory.num_seq
+  end
+
+  ###############################################
+  # Blog
+  ###############################################
+  def create_blog
+    profile = Profile.create!(:identifier => 'testuser' + factory_num_seq.to_s, :name => 'Test user')
+    Blog.create!(:name => 'blog', :profile => profile)
+  end
+
+  ###############################################
+  # ExternalFeed
+  ###############################################
+  def defaults_for_external_feed
+    { :address => RAILS_ROOT + '/test/fixtures/files/feed.xml' }
+  end
+
+  def create_external_feed(attrs = {})
+    feed = build(:external_feed, attrs)
+    feed.blog = create_blog
+    feed.save!
+    feed
+  end
+
+  ###############################################
+  # FeedReaderBlock
+  ###############################################
+  def defaults_for_feed_reader_block
+    { :address => RAILS_ROOT + '/test/fixtures/files/feed.xml' }
+  end
+
+end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 1d7f022..f1902f5 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -6,6 +6,7 @@ require 'tidy'
 require 'hpricot'
 
 require 'noosfero/test'
+require File.dirname(__FILE__) + '/factories'
 
 FileUtils.rm_rf(File.join(RAILS_ROOT, 'index', 'test'))
 
@@ -36,6 +37,8 @@ class Test::Unit::TestCase
 
   # Add more helper methods to be used by all tests here...
 
+  include Noosfero::Factory
+
   include AuthenticatedTestHelper
 
   fixtures :environments, :roles
@@ -253,6 +256,7 @@ class ActionController::IntegrationTest
     follow_redirect!
     assert_not_equal '/account/login', path
   end
+
 end
 
 Profile
diff --git a/test/unit/block_test.rb b/test/unit/block_test.rb
index b91e1bc..f85d299 100644
--- a/test/unit/block_test.rb
+++ b/test/unit/block_test.rb
@@ -73,4 +73,11 @@ class BlockTest < Test::Unit::TestCase
      assert_equal( "block-id-#{b.id}", b.cache_keys)
   end
 
+  should 'list enabled blocks' do
+    block1 = Block.create!(:title => 'test 1')
+    block2 = Block.create!(:title => 'test 2', :enabled => false)
+    assert_includes Block.enabled, block1
+    assert_not_includes Block.enabled, block2
+  end
+
 end
diff --git a/test/unit/external_feed_test.rb b/test/unit/external_feed_test.rb
index a0a3a32..2c9880d 100644
--- a/test/unit/external_feed_test.rb
+++ b/test/unit/external_feed_test.rb
@@ -2,33 +2,29 @@ require File.dirname(__FILE__) + '/../test_helper'
 
 class ExternalFeedTest < ActiveSupport::TestCase
 
-  def setup
-    @profile = create_user('test-person').person
-    @blog = Blog.create!(:name => 'test-blog', :profile => @profile)
-  end
-  attr_reader :profile, :blog
-
   should 'require blog' do
-    e = ExternalFeed.new(:address => 'http://localhost')
-    assert !e.valid?
-    e.blog = blog
-    assert e.save!
+    e = build(:external_feed, :blog => nil)
+    e.valid?
+    assert e.errors[:blog_id]
+    e.blog = create_blog
+    e.valid?
+    assert !e.errors[:blog_id]
   end
 
-  should 'belongs to blog' do
-    e = ExternalFeed.create!(:address => 'http://localhost', :blog => blog)
-    e.reload
+  should 'belong to blog' do
+    blog = create_blog
+    e = build(:external_feed, :blog => blog)
     assert_equal blog, e.blog
   end
 
   should 'not add same item twice' do
-    e = ExternalFeed.create!(:address => 'http://localhost', :blog => blog)
+    e = create(:external_feed)
     assert e.add_item('Article title', 'http://orig.link.invalid', Time.now, 'Content for external post')
     assert !e.add_item('Article title', 'http://orig.link.invalid', Time.now, 'Content for external post')
     assert_equal 1, e.blog.posts.size
   end
 
-  should 'nothing when clear' do
+  should 'do nothing when clear' do
     assert_respond_to ExternalFeed.new, :clear
   end
 
@@ -37,27 +33,99 @@ class ExternalFeedTest < ActiveSupport::TestCase
   end
 
   should 'disable external feed if fetch only once on finish fetch' do
-    e = ExternalFeed.create(:address => 'http://localhost', :blog => blog, :only_once => true, :enabled => true)
-    assert e.enabled
-    assert e.finish_fetch
-    assert !e.enabled
+    e = build(:external_feed, :only_once => true, :enabled => true)
+    e.stubs(:save!)
+    e.finish_fetch
+    assert_equal false, e.enabled
+  end
+
+  should 'not disable after finish fetch if there are errors' do
+    e = build(:external_feed, :only_once => true, :update_errors => 1)
+    e.stubs(:save!)
+    e.finish_fetch
+    assert_equal true, e.enabled
+  end
+
+  should 'be enabled by default' do
+    assert ExternalFeed.new.enabled
   end
 
   should 'add items to blog as posts' do
     handler = FeedHandler.new
-    e = ExternalFeed.create!(:address => 'test/fixtures/files/feed.xml', :blog => blog, :enabled => true)
+    e = create(:external_feed)
     handler.process(e)
     assert_equal ["Last POST", "Second POST", "First POST"], e.blog.posts.map{|i| i.title}
   end
 
   should 'require address if enabled' do
-    e = ExternalFeed.new(:blog => blog, :enabled => true)
+    e = ExternalFeed.new(:enabled => true)
     assert !e.valid?
+    assert e.errors[:address]
   end
 
   should 'not require address if disabled' do
-    e = ExternalFeed.new(:blog => blog, :enabled => false)
-    assert e.valid?
+    e = ExternalFeed.new(:enabled => false, :address => nil)
+    e.valid?
+    assert !e.errors[:address]
+  end
+
+  should 'list enabled external feeds' do
+    e1 = fast_create(:external_feed, :enabled => true)
+    e2 = fast_create(:external_feed, :enabled => false)
+    assert_includes ExternalFeed.enabled, e1
+    assert_not_includes ExternalFeed.enabled, e2
+  end
+
+  should 'have an empty error message by default' do
+    assert ExternalFeed.new.error_message.blank?, 'new external feed must have empty error message'
+  end
+
+  should 'have empty fetch date by default' do
+    assert_nil ExternalFeed.new.fetched_at
+  end
+  should 'set fetch date when finishing fetch' do
+    feed = ExternalFeed.new
+    feed.stubs(:save!)
+    feed.finish_fetch
+    assert_not_nil feed.fetched_at
+  end
+
+  should 'expire feeds after a certain period' do
+    # save current time
+    now = Time.now
+
+    # Noosfero is configured to update feeds every 4 hours
+    FeedUpdater.stubs(:update_interval).returns(4.hours)
+
+    expired = fast_create(:external_feed)
+    not_expired = fast_create(:external_feed)
+
+    # 5 hours ago
+    Time.stubs(:now).returns(now  - 5.hours)
+    expired.finish_fetch
+
+    # 3 hours ago
+    Time.stubs(:now).returns(now - 3.hours)
+    not_expired.finish_fetch
+
+    # now one feed should be expired and the not the other
+    Time.stubs(:now).returns(now)
+    expired_list = ExternalFeed.expired
+    assert_includes expired_list, expired
+    assert_not_includes expired_list, not_expired
+  end
+
+  should 'consider recently-created instance as expired' do
+    new = fast_create(:external_feed)
+    assert_includes ExternalFeed.expired, new
+  end
+
+  should 'have an update errors counter' do
+    assert_equal 3, ExternalFeed.new(:update_errors => 3).update_errors
+  end
+
+  should 'have 0 update errors by default' do
+    assert_equal 0, ExternalFeed.new.update_errors
   end
 
 end
diff --git a/test/unit/feed_handler_test.rb b/test/unit/feed_handler_test.rb
index 62ec222..49a9e70 100644
--- a/test/unit/feed_handler_test.rb
+++ b/test/unit/feed_handler_test.rb
@@ -4,9 +4,12 @@ class FeedHandlerTest < Test::Unit::TestCase
 
   def setup
     @handler = FeedHandler.new
-    @container = FeedReaderBlock.create!(:box_id => 99999, :address => 'test/fixtures/files/feed.xml')
+    @container = nil
+  end
+  attr_reader :handler
+  def container
+    @container ||= fast_create(:feed_reader_block)
   end
-  attr_reader :handler, :container
 
   should 'fetch feed content' do
     content = handler.fetch(container.address)
@@ -68,15 +71,47 @@ class FeedHandlerTest < Test::Unit::TestCase
     handler.process(container)
   end
 
-  should 'finish_fetch after processing' do
+  should 'finish fetch after processing' do
+    container.expects(:finish_fetch)
+    handler.process(container)
+  end
+
+  should 'finish fetch even in case of crash' do
+    container.expects(:clear).raises(Exception.new("crash"))
     container.expects(:finish_fetch)
     handler.process(container)
   end
 
   should 'identifies itself as noosfero user agent' do
-    handler = FeedHandler.new
     handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"}, anything).returns('bli content')
     assert_equal 'bli content', handler.fetch('http://site.org/feed.xml')
   end
 
+  [:external_feed, :feed_reader_block].each do |container_class|
+
+    should "reset the errors count after a successfull run (#{container_class})" do
+      container = fast_create(container_class, :update_errors => 1, :address => RAILS_ROOT + '/test/fixtures/files/feed.xml')
+      handler.expects(:actually_process_container).with(container)
+      handler.process(container)
+      assert_equal 0, container.update_errors
+    end
+
+    should "set error message and disable in case of errors (#{container_class})" do
+      FeedHandler.stubs(:max_errors).returns(4)
+
+      container = fast_create(container_class)
+      handler.stubs(:actually_process_container).with(container).raises(Exception.new("crash"))
+
+      # in the first 4 errors, we are ok
+      4.times { handler.process(container) }
+      assert !container.error_message.blank?, 'should set the error message for the first <max_errors> errors (%s)' % container_class
+      assert container.enabled, 'must keep container enabled during the first <max_errors> errors (%s)' % container_class
+
+      # 5 errors it too much
+      handler.process(container)
+      assert !container.error_message.blank?, 'must set error message in container after <max_errors> errors (%s)' % container_class
+      assert !container.enabled, 'must disable continer after <max_errors> errors (%s)' % container_class
+    end
+  end
+
 end
diff --git a/test/unit/feed_reader_block_test.rb b/test/unit/feed_reader_block_test.rb
index 1a0f83e..aace9d5 100644
--- a/test/unit/feed_reader_block_test.rb
+++ b/test/unit/feed_reader_block_test.rb
@@ -5,12 +5,9 @@ class FeedReaderBlockTest < ActiveSupport::TestCase
   include DatesHelper
 
   def setup
-    @feed = FeedReaderBlock.new
-    @fetched_at = Time.now
-    @feed.fetched_at = @fetched_at
-    @feed.save!
+    @feed = fast_create(:feed_reader_block)
   end
-  attr_reader :feed, :fetched_at
+  attr_reader :feed
 
   should 'default describe' do
     assert_not_equal Block.description, FeedReaderBlock.description
@@ -56,8 +53,10 @@ class FeedReaderBlockTest < ActiveSupport::TestCase
   end
 
   should 'display last fetched date' do
+    now = Time.now
     feed.feed_items = ['one', 'two']
-    assert_equal "Updated: #{show_date(@fetched_at)}", feed.footer
+    feed.fetched_at = now
+    assert_equal "Updated: #{show_date(now)}", feed.footer
   end
 
   should 'clear feed title and items' do
@@ -73,6 +72,16 @@ class FeedReaderBlockTest < ActiveSupport::TestCase
     feed.finish_fetch
   end
 
+  should 'set fetched_at when finishing a fetch' do
+    feed.stubs(:save!)
+    feed.finish_fetch
+    assert_not_nil feed.fetched_at
+  end
+
+  should 'have empty fetched_at by default' do
+    assert_nil feed.fetched_at
+  end
+
   should 'display the latest post first' do
     %w[ first-post second-post last-post ].each do |i|
       feed.add_item(i, "http://localhost/#{i}", Date.today, "some contet for #{i}")
@@ -91,6 +100,77 @@ class FeedReaderBlockTest < ActiveSupport::TestCase
     assert_no_tag_in_string feed.formatted_feed_content, :tag => 'a', :attributes => { :href => 'http://localhost/first-post' }, :content => 'first-post'
   end
 
+  should 'have empty error message by default' do
+    assert FeedReaderBlock.new.error_message.blank?, 'new feed reader block expected to have empty error message'
+  end
+
+  should "display error message as content when it's the case" do
+    msg = "there was a problem"
+    feed.error_message = msg
+    assert_match(msg, feed.content)
+  end
 
+  should 'expire after a period' do
+    # save current time
+    now = Time.now
+    expired = FeedReaderBlock.create!
+    not_expired = FeedReaderBlock.create!
+
+    # Noosfero is configured to update feeds every 4 hours
+    FeedUpdater.stubs(:update_interval).returns(4.hours)
+
+    # 5 hours ago
+    Time.stubs(:now).returns(now  - 5.hours)
+    expired.finish_fetch
+
+    # 3 hours ago
+    Time.stubs(:now).returns(now - 3.hours)
+    not_expired.finish_fetch
+
+    # now one block should be expired and the not the other
+    Time.stubs(:now).returns(now)
+    expired_list = FeedReaderBlock.expired
+    assert_includes expired_list, expired
+    assert_not_includes expired_list, not_expired
+  end
+
+  should 'consider recently-created as expired' do
+    # feed is created in setup
+    assert_includes FeedReaderBlock.expired, feed
+  end
+
+  should 'have an update errors counter' do
+    assert_equal 5, FeedReaderBlock.new(:update_errors => 5).update_errors
+  end
+
+  should 'have 0 errors by default' do
+    assert_equal 0, FeedReaderBlock.new.update_errors
+  end
+
+  should 'be disabled by default' do
+    assert_equal false, FeedReaderBlock.new.enabled
+  end
+
+  should 'be enabled when address is filled' do
+    reader = build(:feed_reader_block, :address => 'http://www.example.com/feed')
+    assert_equal true, reader.enabled
+  end
+
+  should 'be disabled when address is empty' do
+    reader = build(:feed_reader_block, :enabled => true, :address => 'http://www.example.com/feed')
+    reader.address = nil
+    assert_equal false, reader.enabled
+  end
+
+  should 're-enable when address is changed' do
+    reader = build(:feed_reader_block, :address => 'http://www.example.com/feed')
+    reader.enabled = false
+
+    reader.address = 'http://www.example.com/feed'
+    assert_equal false, reader.enabled, 'must not enable when setting to the same address'
+
+    reader.address = 'http://www.acme.com/feed'
+    assert_equal true, reader.enabled, 'must enable when setting to new address'
+  end
 
 end
diff --git a/test/unit/feed_updater_test.rb b/test/unit/feed_updater_test.rb
new file mode 100644
index 0000000..c764aa0
--- /dev/null
+++ b/test/unit/feed_updater_test.rb
@@ -0,0 +1,38 @@
+require File.dirname(__FILE__) + '/../test_helper'
+
+class FeedUpdaterTest < Test::Unit::TestCase
+
+  should 'be running by default' do
+    assert_equal true, FeedUpdater.new.running
+  end
+
+  should 'unset running when stopped' do
+    updater = FeedUpdater.new
+    updater.expects(:running=).with(false)
+    updater.stop
+  end
+
+  should 'sleep in intervals of one second' do
+    FeedUpdater.stubs(:daemon_sleep_interval).returns(30)
+    updater = FeedUpdater.new
+    updater.expects(:sleep).with(1).times(30)
+    updater.wait
+  end
+
+  should 'not sleep when stopped' do
+    FeedUpdater.stubs(:daemon_sleep_interval).returns(30)
+    updater = FeedUpdater.new
+    updater.stubs(:running).returns(true).then.returns(true).then.returns(false)
+    updater.expects(:sleep).with(1).times(2)
+    updater.wait
+  end
+
+  should 'process until it is stopped' do
+    updater = FeedUpdater.new
+    updater.stubs(:running).returns(true).then.returns(true).then.returns(false)
+    updater.expects(:process_round).times(2)
+    updater.expects(:wait).times(2)
+    updater.run
+  end
+
+end
diff --git a/test/unit/tiny_mce_article_test.rb b/test/unit/tiny_mce_article_test.rb
index 8c39f63..65396b6 100644
--- a/test/unit/tiny_mce_article_test.rb
+++ b/test/unit/tiny_mce_article_test.rb
@@ -43,4 +43,10 @@ class TinyMceArticleTest < Test::Unit::TestCase
     article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "the <!-- comment --> article ...")
     assert_equal "the <!-- comment --> article ...", article.body
   end
+
+  should 'convert entities characters to UTF-8 instead of ISO-8859-1' do
+    article = TinyMceArticle.create!(:profile => profile, :name => 'teste ' + Time.now.to_s, :body => '<a title="inform&#225;tica">link</a>')
+    assert(article.body.is_utf8?, "%s expected to be valid UTF-8 content" % article.body.inspect)
+  end
+
 end
diff --git a/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb b/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb
index afd00ba..923fb10 100644
--- a/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb
+++ b/vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb
@@ -22,7 +22,7 @@ HTML::WhiteListSanitizer.module_eval do
       if !options[:attributes].include?(attr_name) || contains_bad_protocols?(attr_name, value)
         node.attributes.delete(attr_name)
       else
-        node.attributes[attr_name] = attr_name == 'style' ? sanitize_css(value) : CGI::escapeHTML(CGI::unescapeHTML(value))
+        node.attributes[attr_name] = attr_name == 'style' ? sanitize_css(value) : CGI::escapeHTML(value.gsub('&amp;', '&'))
       end
     end
   end
--
libgit2 0.21.2