Commit cd41bd6779a388f2265b721fdfc74fcdcd611c29

Authored by Antonio Terceiro
1 parent 91e0208c

Rewriting feed updater code

  * transformed script/feed-updater into a controller script. It starts
    and stops together with the production system (script/production)
  * moved the update (daemon) logic into FeedUpdater class. It knows which
    feeds must be updated and when, and when it should stop running.
  * concentrated the fetch (download) logic into FeedHandler class. It
    knows how to update a specific feed.
  * implemented the concept of "enabled" and "expired" in both
    ExternalFeed and FeedReaderBlock. The feed updater looks for feeds
    that are both enabled *and* expired to update.
  * Disabled feed reader blocks get re-enabled when their address is
    changed.
  * fixed a bug that made some feeds crash when using PostgreSQL
    (CGI::unescapeHTML transforms Numeric Character References into
    iso-8859-1 data and PostgreSQL won't accept that into a UTF-8
    database)
  * Removed sleep(1) calls from script/production, they don't seem to be
    useful
  * Added an index for the type column in `blocks` table.

(ActionItem1243)
app/models/block.rb
@@ -13,6 +13,8 @@ class Block < ActiveRecord::Base @@ -13,6 +13,8 @@ class Block < ActiveRecord::Base
13 acts_as_having_settings 13 acts_as_having_settings
14 settings_items :visible, :type => :boolean, :default => true 14 settings_items :visible, :type => :boolean, :default => true
15 15
  16 + named_scope :enabled, :conditions => { :enabled => true }
  17 +
16 def visible? 18 def visible?
17 visible 19 visible
18 end 20 end
app/models/external_feed.rb
@@ -5,6 +5,11 @@ class ExternalFeed < ActiveRecord::Base @@ -5,6 +5,11 @@ class ExternalFeed < ActiveRecord::Base
5 validates_presence_of :address, :if => lambda {|efeed| efeed.enabled} 5 validates_presence_of :address, :if => lambda {|efeed| efeed.enabled}
6 validates_uniqueness_of :blog_id 6 validates_uniqueness_of :blog_id
7 7
  8 + named_scope :enabled, :conditions => { :enabled => true }
  9 + named_scope :expired, lambda {
  10 + { :conditions => ['(fetched_at is NULL) OR (fetched_at < ?)', Time.now - FeedUpdater.update_interval] }
  11 + }
  12 +
8 def add_item(title, link, date, content) 13 def add_item(title, link, date, content)
9 article = TinyMceArticle.new(:name => title, :profile => blog.profile, :body => content, :published_at => date, :source => link, :profile => blog.profile, :parent => blog) 14 article = TinyMceArticle.new(:name => title, :profile => blog.profile, :body => content, :published_at => date, :source => link, :profile => blog.profile, :parent => blog)
10 unless blog.children.exists?(:slug => article.slug) 15 unless blog.children.exists?(:slug => article.slug)
@@ -16,9 +21,10 @@ class ExternalFeed &lt; ActiveRecord::Base @@ -16,9 +21,10 @@ class ExternalFeed &lt; ActiveRecord::Base
16 # do nothing 21 # do nothing
17 end 22 end
18 def finish_fetch 23 def finish_fetch
19 - if self.only_once  
20 - self.enabled = 'false' 24 + if self.only_once && self.update_errors.zero?
  25 + self.enabled = false
21 end 26 end
  27 + self.fetched_at = Time.now
22 self.save! 28 self.save!
23 end 29 end
24 30
app/models/feed_reader_block.rb
1 class FeedReaderBlock < Block 1 class FeedReaderBlock < Block
2 2
  3 + def initialize(attributes = nil)
  4 + data = attributes || {}
  5 + super({ :enabled => !data[:address].blank? }.merge(data))
  6 + end
  7 +
3 include DatesHelper 8 include DatesHelper
4 9
5 settings_items :address, :type => :string 10 settings_items :address, :type => :string
  11 + alias :orig_set_address :address=
  12 + def address=(new_address)
  13 + old_address = address
  14 + orig_set_address(new_address)
  15 + self.enabled = (old_address.blank? && !new_address.blank?) || (new_address && new_address != old_address) || false
  16 + end
  17 +
6 settings_items :limit, :type => :integer 18 settings_items :limit, :type => :integer
7 - settings_items :fetched_at, :type => :date  
8 19
9 settings_items :feed_title, :type => :string 20 settings_items :feed_title, :type => :string
10 settings_items :feed_items, :type => :array 21 settings_items :feed_items, :type => :array
11 22
  23 + settings_items :update_errors, :type => :integer, :default => 0
  24 + settings_items :error_message, :type => :string
  25 +
  26 + named_scope :expired, lambda {
  27 + { :conditions => [ '(fetched_at is NULL) OR (fetched_at < ?)', Time.now - FeedUpdater.update_interval] }
  28 + }
  29 +
12 before_create do |block| 30 before_create do |block|
13 block.limit = 5 31 block.limit = 5
14 block.feed_items = [] 32 block.feed_items = []
@@ -27,9 +45,13 @@ class FeedReaderBlock &lt; Block @@ -27,9 +45,13 @@ class FeedReaderBlock &lt; Block
27 end 45 end
28 46
29 def formatted_feed_content 47 def formatted_feed_content
30 - return "<ul>\n" + 48 + if error_message.blank?
  49 + "<ul>\n" +
31 self.feed_items[0..(limit-1)].map{ |item| "<li><a href='#{item[:link]}'>#{item[:title]}</a></li>" }.join("\n") + 50 self.feed_items[0..(limit-1)].map{ |item| "<li><a href='#{item[:link]}'>#{item[:title]}</a></li>" }.join("\n") +
32 "</ul>" 51 "</ul>"
  52 + else
  53 + '<p>' + error_message + '</p>'
  54 + end
33 end 55 end
34 56
35 def footer 57 def footer
@@ -49,6 +71,7 @@ class FeedReaderBlock &lt; Block @@ -49,6 +71,7 @@ class FeedReaderBlock &lt; Block
49 self.feed_title = nil 71 self.feed_title = nil
50 end 72 end
51 def finish_fetch 73 def finish_fetch
  74 + self.fetched_at = Time.now
52 self.save! 75 self.save!
53 end 76 end
54 77
db/migrate/075_add_new_feed_stuff.rb 0 → 100644
@@ -0,0 +1,40 @@ @@ -0,0 +1,40 @@
  1 +class AddNewFeedStuff < ActiveRecord::Migration
  2 +
  3 + def self.up
  4 + add_column :blocks, :enabled, :boolean, :default => true
  5 + execute('update blocks set enabled = (1=1)')
  6 +
  7 + add_column :blocks, :created_at, :datetime
  8 + add_column :blocks, :updated_at, :datetime
  9 + add_column :blocks, :fetched_at, :datetime
  10 + execute("update blocks set created_at = '2009-10-23 17:00', updated_at = '2009-10-23 17:00'")
  11 +
  12 + add_index :blocks, :enabled
  13 + add_index :blocks, :fetched_at
  14 + add_index :blocks, :type
  15 +
  16 + add_column :external_feeds, :error_message, :text
  17 + add_column :external_feeds, :update_errors, :integer, :default => 0
  18 + execute('update external_feeds set update_errors = 0')
  19 +
  20 + add_index :external_feeds, :enabled
  21 + add_index :external_feeds, :fetched_at
  22 + end
  23 +
  24 + def self.down
  25 + remove_index :blocks, :enabled
  26 + remove_index :blocks, :fetched_at
  27 + remove_index :blocks, :type
  28 + remove_column :blocks, :enabled
  29 + remove_column :blocks, :updated_at
  30 + remove_column :blocks, :created_at
  31 + remove_column :blocks, :fetched_at
  32 +
  33 + remove_index :external_feeds, :enabled
  34 + remove_index :external_feeds, :fetched_at
  35 + remove_column :external_feeds, :error_message
  36 + remove_column :external_feeds, :update_errors
  37 + end
  38 +
  39 +end
  40 +
@@ -9,7 +9,7 @@ @@ -9,7 +9,7 @@
9 # 9 #
10 # It's strongly recommended to check this file into your version control system. 10 # It's strongly recommended to check this file into your version control system.
11 11
12 -ActiveRecord::Schema.define(:version => 74) do 12 +ActiveRecord::Schema.define(:version => 75) do
13 13
14 create_table "article_versions", :force => true do |t| 14 create_table "article_versions", :force => true do |t|
15 t.integer "article_id" 15 t.integer "article_id"
@@ -88,8 +88,8 @@ ActiveRecord::Schema.define(:version =&gt; 74) do @@ -88,8 +88,8 @@ ActiveRecord::Schema.define(:version =&gt; 74) do
88 t.boolean "virtual", :default => false 88 t.boolean "virtual", :default => false
89 end 89 end
90 90
91 - add_index "articles_categories", ["category_id"], :name => "index_articles_categories_on_category_id"  
92 add_index "articles_categories", ["article_id"], :name => "index_articles_categories_on_article_id" 91 add_index "articles_categories", ["article_id"], :name => "index_articles_categories_on_article_id"
  92 + add_index "articles_categories", ["category_id"], :name => "index_articles_categories_on_category_id"
93 93
94 create_table "blocks", :force => true do |t| 94 create_table "blocks", :force => true do |t|
95 t.string "title" 95 t.string "title"
@@ -97,10 +97,16 @@ ActiveRecord::Schema.define(:version =&gt; 74) do @@ -97,10 +97,16 @@ ActiveRecord::Schema.define(:version =&gt; 74) do
97 t.string "type" 97 t.string "type"
98 t.text "settings" 98 t.text "settings"
99 t.integer "position" 99 t.integer "position"
100 - t.datetime "last_updated" 100 + t.boolean "enabled", :default => true
  101 + t.datetime "created_at"
  102 + t.datetime "updated_at"
  103 + t.datetime "fetched_at"
101 end 104 end
102 105
103 add_index "blocks", ["box_id"], :name => "index_blocks_on_box_id" 106 add_index "blocks", ["box_id"], :name => "index_blocks_on_box_id"
  107 + add_index "blocks", ["enabled"], :name => "index_blocks_on_enabled"
  108 + add_index "blocks", ["fetched_at"], :name => "index_blocks_on_fetched_at"
  109 + add_index "blocks", ["type"], :name => "index_blocks_on_type"
104 110
105 create_table "boxes", :force => true do |t| 111 create_table "boxes", :force => true do |t|
106 t.string "owner_type" 112 t.string "owner_type"
@@ -108,7 +114,7 @@ ActiveRecord::Schema.define(:version =&gt; 74) do @@ -108,7 +114,7 @@ ActiveRecord::Schema.define(:version =&gt; 74) do
108 t.integer "position" 114 t.integer "position"
109 end 115 end
110 116
111 - add_index "boxes", ["owner_type", "owner_id"], :name => "index_boxes_on_owner_type_and_owner_id" 117 + add_index "boxes", ["owner_id", "owner_type"], :name => "index_boxes_on_owner_type_and_owner_id"
112 118
113 create_table "categories", :force => true do |t| 119 create_table "categories", :force => true do |t|
114 t.string "name" 120 t.string "name"
@@ -172,15 +178,20 @@ ActiveRecord::Schema.define(:version =&gt; 74) do @@ -172,15 +178,20 @@ ActiveRecord::Schema.define(:version =&gt; 74) do
172 178
173 create_table "external_feeds", :force => true do |t| 179 create_table "external_feeds", :force => true do |t|
174 t.string "feed_title" 180 t.string "feed_title"
  181 + t.date "fetched_at"
175 t.string "address" 182 t.string "address"
176 - t.integer "blog_id", :null => false  
177 - t.boolean "enabled", :default => true, :null => false  
178 - t.boolean "only_once", :default => true, :null => false 183 + t.integer "blog_id", :null => false
  184 + t.boolean "enabled", :default => true, :null => false
  185 + t.boolean "only_once", :default => true, :null => false
179 t.datetime "created_at" 186 t.datetime "created_at"
180 t.datetime "updated_at" 187 t.datetime "updated_at"
181 - t.datetime "fetched_at" 188 + t.text "error_message"
  189 + t.integer "update_errors", :default => 0
182 end 190 end
183 191
  192 + add_index "external_feeds", ["enabled"], :name => "index_external_feeds_on_enabled"
  193 + add_index "external_feeds", ["fetched_at"], :name => "index_external_feeds_on_fetched_at"
  194 +
184 create_table "favorite_enteprises_people", :id => false, :force => true do |t| 195 create_table "favorite_enteprises_people", :id => false, :force => true do |t|
185 t.integer "person_id" 196 t.integer "person_id"
186 t.integer "enterprise_id" 197 t.integer "enterprise_id"
@@ -281,9 +292,9 @@ ActiveRecord::Schema.define(:version =&gt; 74) do @@ -281,9 +292,9 @@ ActiveRecord::Schema.define(:version =&gt; 74) do
281 292
282 create_table "roles", :force => true do |t| 293 create_table "roles", :force => true do |t|
283 t.string "name" 294 t.string "name"
284 - t.text "permissions"  
285 t.string "key" 295 t.string "key"
286 t.boolean "system", :default => false 296 t.boolean "system", :default => false
  297 + t.text "permissions"
287 t.integer "environment_id" 298 t.integer "environment_id"
288 end 299 end
289 300
@@ -294,8 +305,8 @@ ActiveRecord::Schema.define(:version =&gt; 74) do @@ -294,8 +305,8 @@ ActiveRecord::Schema.define(:version =&gt; 74) do
294 t.datetime "created_at" 305 t.datetime "created_at"
295 end 306 end
296 307
297 - add_index "taggings", ["taggable_id", "taggable_type"], :name => "index_taggings_on_taggable_id_and_taggable_type"  
298 add_index "taggings", ["tag_id"], :name => "index_taggings_on_tag_id" 308 add_index "taggings", ["tag_id"], :name => "index_taggings_on_tag_id"
  309 + add_index "taggings", ["taggable_id", "taggable_type"], :name => "index_taggings_on_taggable_id_and_taggable_type"
299 310
300 create_table "tags", :force => true do |t| 311 create_table "tags", :force => true do |t|
301 t.string "name" 312 t.string "name"
doc/README_FOR_APP
@@ -23,12 +23,13 @@ You need to have git installed, as well as: @@ -23,12 +23,13 @@ You need to have git installed, as well as:
23 * contacts: http://github.com/cardmagic/contacts/tree/master 23 * contacts: http://github.com/cardmagic/contacts/tree/master
24 * iso-codes: http://pkg-isocodes.alioth.debian.org/ 24 * iso-codes: http://pkg-isocodes.alioth.debian.org/
25 * feedparser: http://packages.debian.org/sid/libfeedparser-ruby 25 * feedparser: http://packages.debian.org/sid/libfeedparser-ruby
  26 +* Daemons - http://daemons.rubyforge.org/
26 * Mongrel: http://mongrel.rubyforge.org/ 27 * Mongrel: http://mongrel.rubyforge.org/
27 * tango-icon-theme: http://tango.freedesktop.org/Tango_Icon_Library 28 * tango-icon-theme: http://tango.freedesktop.org/Tango_Icon_Library
28 29
29 There are Debian packages available for all of them but contacts. Try: 30 There are Debian packages available for all of them but contacts. Try:
30 31
31 - # 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 32 + # 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
32 33
33 contacts is bundled together with noosfero for now, so you don't need to install it. 34 contacts is bundled together with noosfero for now, so you don't need to install it.
34 35
lib/feed_handler.rb
1 require 'feedparser' 1 require 'feedparser'
2 require 'open-uri' 2 require 'open-uri'
3 3
  4 +# This class is responsible for processing feeds and pass the items to the
  5 +# respective container.
  6 +#
  7 +# The <tt>max_errors</tt> attribute controls how many times it will retry in
  8 +# case of failure. If a feed fails for <tt>max_errors+1</tt> times, it will be
  9 +# disabled and the last error message will be recorder in the container.
  10 +# The default value is *6*, if you need to change it you can do that in your
  11 +# config/local.rb file like this:
  12 +#
  13 +# FeedHandler.max_errors = 10
  14 +#
  15 +# For the update interval, see FeedUpdater.
4 class FeedHandler 16 class FeedHandler
5 17
  18 + # The maximum number
  19 + cattr_accessor :max_errors
  20 +
  21 + self.max_errors = 6
  22 +
6 def parse(content) 23 def parse(content)
7 raise FeedHandler::ParseError, "Content is nil" if content.nil? 24 raise FeedHandler::ParseError, "Content is nil" if content.nil?
8 begin 25 begin
@@ -16,42 +33,61 @@ class FeedHandler @@ -16,42 +33,61 @@ class FeedHandler
16 begin 33 begin
17 content = "" 34 content = ""
18 block = lambda { |s| content = s.read } 35 block = lambda { |s| content = s.read }
19 - content = if is_web_address?(address)  
20 - open( address, "User-Agent" => "Noosfero/#{Noosfero::VERSION}", &block )  
21 - else  
22 - open_uri_original_open(address, &block)  
23 - end 36 + content =
  37 + if RAILS_ENV == 'test' && File.exists?(address)
  38 + File.read(address)
  39 + else
  40 + if !valid_url?(address)
  41 + raise InvalidUrl.new("\"%s\" is not a valid URL" % address)
  42 + end
  43 + open(address, "User-Agent" => "Noosfero/#{Noosfero::VERSION}", &block)
  44 + end
24 return content 45 return content
25 rescue Exception => ex 46 rescue Exception => ex
26 - raise FeedHandler::FetchError, ex.to_s 47 + raise FeedHandler::FetchError, ex.message
27 end 48 end
28 end 49 end
29 50
30 def process(container) 51 def process(container)
31 - container.class.transaction do  
32 - container.clear  
33 - content = fetch(container.address)  
34 - container.fetched_at = Time.now  
35 - parsed_feed = parse(content)  
36 - container.feed_title = parsed_feed.title  
37 - parsed_feed.items[0..container.limit-1].reverse.each do |item|  
38 - container.add_item(item.title, item.link, item.date, item.content) 52 + RAILS_DEFAULT_LOGGER.info("Processing %s with id = %d" % [container.class.name, container.id])
  53 + begin
  54 + container.class.transaction do
  55 + actually_process_container(container)
  56 + container.update_errors = 0
  57 + container.finish_fetch
  58 + end
  59 + rescue Exception => exception
  60 + RAILS_DEFAULT_LOGGER.warn("Unknown error from %s ID %d\n%s" % [container.class.name, container.id, exception.to_s])
  61 + RAILS_DEFAULT_LOGGER.warn("Backtrace:\n%s" % exception.backtrace.join("\n"))
  62 + container.reload
  63 + container.update_errors += 1
  64 + container.error_message = exception.to_s
  65 + if container.update_errors > FeedHandler.max_errors
  66 + container.enabled = false
39 end 67 end
40 container.finish_fetch 68 container.finish_fetch
41 end 69 end
42 end 70 end
43 71
  72 + class InvalidUrl < Exception; end
44 class ParseError < Exception; end 73 class ParseError < Exception; end
45 class FetchError < Exception; end 74 class FetchError < Exception; end
46 75
47 protected 76 protected
48 77
49 - # extracted from the open implementation in the open-uri library  
50 - def is_web_address?(address)  
51 - address.respond_to?(:open) ||  
52 - address.respond_to?(:to_str) &&  
53 - (%r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ address) &&  
54 - URI.parse(address).respond_to?(:open) 78 + def actually_process_container(container)
  79 + container.clear
  80 + content = fetch(container.address)
  81 + container.fetched_at = Time.now
  82 + parsed_feed = parse(content)
  83 + container.feed_title = parsed_feed.title
  84 + parsed_feed.items[0..container.limit-1].reverse.each do |item|
  85 + container.add_item(item.title, item.link, item.date, item.content)
  86 + end
  87 + end
  88 +
  89 + def valid_url?(url)
  90 + url =~ URI.regexp('http') || url =~ URI.regexp('https')
55 end 91 end
56 92
57 end 93 end
lib/feed_updater.rb 0 → 100644
@@ -0,0 +1,87 @@ @@ -0,0 +1,87 @@
  1 +# to run by hand
  2 +if $PROGRAM_NAME == __FILE__
  3 + require File.dirname(__FILE__) + '/../config/environment'
  4 +end
  5 +
  6 +# This class implements the feed updater. To change how often a feed gets
  7 +# updated, change FeedUpdater#update_interval in your config/local.rb file like
  8 +# this:
  9 +#
  10 +# FeedUpdater.update_interval = 24.hours
  11 +#
  12 +# You can also customize the time between update runs setting
  13 +# FeedUpdater#daemon_sleep_interval. Give it an integer representing the number
  14 +# of seconds to wait between runs in your config/local.rb:
  15 +#
  16 +# FeedUpdater.daemon_sleep_interval = 10
  17 +#
  18 +# The feed updaters is controlled by script/feed-updater, which starts and
  19 +# stops the process.
  20 +class FeedUpdater
  21 +
  22 + # indicates how much time one feed will be left without updates
  23 + # (ActiveSupport::Duration). Default: <tt>4.hours</tt>
  24 + cattr_accessor :update_interval
  25 + self.update_interval = 4.hours
  26 +
  27 + # indicates for how much time the daemon sleeps before looking for new feeds
  28 + # to load (in seconds, an integer). Default: 30
  29 + cattr_accessor :daemon_sleep_interval
  30 + self.daemon_sleep_interval = 30
  31 +
  32 + attr_accessor :running
  33 +
  34 + def initialize
  35 + self.running = true
  36 + end
  37 +
  38 + def start
  39 + ['TERM', 'INT'].each do |signal|
  40 + Signal.trap(signal) do
  41 + stop
  42 + RAILS_DEFAULT_LOGGER.info("Feed updater exiting gracefully ...")
  43 + end
  44 + end
  45 + run
  46 + RAILS_DEFAULT_LOGGER.info("Feed updater exited.")
  47 + end
  48 +
  49 + def run
  50 + while running
  51 + process_round
  52 + wait
  53 + end
  54 + end
  55 +
  56 + def wait
  57 + i = 0
  58 + while running && i < FeedUpdater.daemon_sleep_interval
  59 + sleep 1
  60 + i += 1
  61 + end
  62 + end
  63 +
  64 + def stop
  65 + self.running = false
  66 + end
  67 +
  68 + def process_round
  69 + feed_handler = FeedHandler.new
  70 + [FeedReaderBlock, ExternalFeed].each do |source|
  71 + if !running
  72 + break
  73 + end
  74 + source.enabled.expired.all.each do |container|
  75 + if !running
  76 + break
  77 + end
  78 + feed_handler.process(container)
  79 + end
  80 + end
  81 + end
  82 +end
  83 +
  84 +# run the updater
  85 +if ($PROGRAM_NAME == __FILE__)
  86 + FeedUpdater.new.start
  87 +end
script/feed-updater
1 #!/usr/bin/env ruby 1 #!/usr/bin/env ruby
2 -require File.dirname(__FILE__) + '/../config/environment'  
3 -  
4 -(FeedReaderBlock.find(:all) + ExternalFeed.find(:all, :conditions => {:enabled => true})).each do |container|  
5 - unless container.address.nil?  
6 - begin  
7 - handler = FeedHandler.new  
8 - handler.process(container)  
9 - rescue Exception => ex  
10 - $stderr.puts("Unknown error from %s ID %d\n%s" % [container.class.name, container.id, ex.to_s])  
11 - $stderr.puts("Backtrace:\n%s" % ex.backtrace.join("\n"))  
12 - end  
13 - end 2 +
  3 +# This is the Noosfero feed updater controller script. It starts and stops the
  4 +# feed updater daemon, which is implemented in the FeedUpdater class.
  5 +#
  6 +# The role of this script is to just start/stop the daemon, write a PID file,
  7 +# etc. The actual feed update logic is in FeedUpdater.
  8 +
  9 +require 'daemons'
  10 +
  11 +NOOSFERO_ROOT = File.expand_path(File.dirname(__FILE__) + '/../')
  12 +
  13 +options = {
  14 + :dir_mode => :normal,
  15 + :dir => File.dirname(__FILE__) + '/../tmp/pids',
  16 + :multiple => false,
  17 + :backtrace => true,
  18 + :monitor => true,
  19 +}
  20 +
  21 +Daemons.run_proc('feed-updater', options) do
  22 + require NOOSFERO_ROOT + '/config/environment'
  23 + FeedUpdater.new.start
14 end 24 end
  25 +
script/production
@@ -13,14 +13,13 @@ do_start() { @@ -13,14 +13,13 @@ do_start() {
13 fi 13 fi
14 14
15 ./script/ferret_server -e $RAILS_ENV start 15 ./script/ferret_server -e $RAILS_ENV start
16 - sleep 1 16 + ./script/feed-updater start
17 mongrel_rails cluster::start 17 mongrel_rails cluster::start
18 - sleep 3  
19 } 18 }
20 19
21 do_stop() { 20 do_stop() {
22 mongrel_rails cluster::stop 21 mongrel_rails cluster::stop
23 - sleep 1 22 + ./script/feed-updater stop
24 ./script/ferret_server -e $RAILS_ENV stop 23 ./script/ferret_server -e $RAILS_ENV stop
25 } 24 }
26 25
test/factories.rb 0 → 100644
@@ -0,0 +1,76 @@ @@ -0,0 +1,76 @@
  1 +module Noosfero::Factory
  2 +
  3 + def fast_create(name, attrs = {})
  4 + obj = build(name, attrs)
  5 + obj.attributes.keys.each do |attr|
  6 + if !obj.column_for_attribute(attr).null && obj.send(attr).nil?
  7 + obj.send("#{attr}=", factory_num_seq)
  8 + end
  9 + end
  10 + obj.save_without_validation!
  11 + obj
  12 + end
  13 +
  14 + def create(name, attrs = {})
  15 + target = 'create_' + name.to_s
  16 + if respond_to?(target)
  17 + send(target, attrs)
  18 + else
  19 + obj = build(name, attrs)
  20 + obj.save!
  21 + obj
  22 + end
  23 + end
  24 +
  25 + def build(name, attrs = {})
  26 + data =
  27 + if respond_to?('defaults_for_' + name.to_s)
  28 + send('defaults_for_'+ name.to_s).merge(attrs)
  29 + else
  30 + attrs
  31 + end
  32 + eval(name.to_s.camelize).new(data)
  33 + end
  34 +
  35 + def self.num_seq
  36 + @num_seq ||= 0
  37 + @num_seq += 1
  38 + @num_seq
  39 + end
  40 +
  41 + protected
  42 +
  43 + def factory_num_seq
  44 + Noosfero::Factory.num_seq
  45 + end
  46 +
  47 + ###############################################
  48 + # Blog
  49 + ###############################################
  50 + def create_blog
  51 + profile = Profile.create!(:identifier => 'testuser' + factory_num_seq.to_s, :name => 'Test user')
  52 + Blog.create!(:name => 'blog', :profile => profile)
  53 + end
  54 +
  55 + ###############################################
  56 + # ExternalFeed
  57 + ###############################################
  58 + def defaults_for_external_feed
  59 + { :address => RAILS_ROOT + '/test/fixtures/files/feed.xml' }
  60 + end
  61 +
  62 + def create_external_feed(attrs = {})
  63 + feed = build(:external_feed, attrs)
  64 + feed.blog = create_blog
  65 + feed.save!
  66 + feed
  67 + end
  68 +
  69 + ###############################################
  70 + # FeedReaderBlock
  71 + ###############################################
  72 + def defaults_for_feed_reader_block
  73 + { :address => RAILS_ROOT + '/test/fixtures/files/feed.xml' }
  74 + end
  75 +
  76 +end
test/test_helper.rb
@@ -6,6 +6,7 @@ require &#39;tidy&#39; @@ -6,6 +6,7 @@ require &#39;tidy&#39;
6 require 'hpricot' 6 require 'hpricot'
7 7
8 require 'noosfero/test' 8 require 'noosfero/test'
  9 +require File.dirname(__FILE__) + '/factories'
9 10
10 FileUtils.rm_rf(File.join(RAILS_ROOT, 'index', 'test')) 11 FileUtils.rm_rf(File.join(RAILS_ROOT, 'index', 'test'))
11 12
@@ -36,6 +37,8 @@ class Test::Unit::TestCase @@ -36,6 +37,8 @@ class Test::Unit::TestCase
36 37
37 # Add more helper methods to be used by all tests here... 38 # Add more helper methods to be used by all tests here...
38 39
  40 + include Noosfero::Factory
  41 +
39 include AuthenticatedTestHelper 42 include AuthenticatedTestHelper
40 43
41 fixtures :environments, :roles 44 fixtures :environments, :roles
@@ -253,6 +256,7 @@ class ActionController::IntegrationTest @@ -253,6 +256,7 @@ class ActionController::IntegrationTest
253 follow_redirect! 256 follow_redirect!
254 assert_not_equal '/account/login', path 257 assert_not_equal '/account/login', path
255 end 258 end
  259 +
256 end 260 end
257 261
258 Profile 262 Profile
test/unit/block_test.rb
@@ -73,4 +73,11 @@ class BlockTest &lt; Test::Unit::TestCase @@ -73,4 +73,11 @@ class BlockTest &lt; Test::Unit::TestCase
73 assert_equal( "block-id-#{b.id}", b.cache_keys) 73 assert_equal( "block-id-#{b.id}", b.cache_keys)
74 end 74 end
75 75
  76 + should 'list enabled blocks' do
  77 + block1 = Block.create!(:title => 'test 1')
  78 + block2 = Block.create!(:title => 'test 2', :enabled => false)
  79 + assert_includes Block.enabled, block1
  80 + assert_not_includes Block.enabled, block2
  81 + end
  82 +
76 end 83 end
test/unit/external_feed_test.rb
@@ -2,33 +2,29 @@ require File.dirname(__FILE__) + &#39;/../test_helper&#39; @@ -2,33 +2,29 @@ require File.dirname(__FILE__) + &#39;/../test_helper&#39;
2 2
3 class ExternalFeedTest < ActiveSupport::TestCase 3 class ExternalFeedTest < ActiveSupport::TestCase
4 4
5 - def setup  
6 - @profile = create_user('test-person').person  
7 - @blog = Blog.create!(:name => 'test-blog', :profile => @profile)  
8 - end  
9 - attr_reader :profile, :blog  
10 -  
11 should 'require blog' do 5 should 'require blog' do
12 - e = ExternalFeed.new(:address => 'http://localhost')  
13 - assert !e.valid?  
14 - e.blog = blog  
15 - assert e.save! 6 + e = build(:external_feed, :blog => nil)
  7 + e.valid?
  8 + assert e.errors[:blog_id]
  9 + e.blog = create_blog
  10 + e.valid?
  11 + assert !e.errors[:blog_id]
16 end 12 end
17 13
18 - should 'belongs to blog' do  
19 - e = ExternalFeed.create!(:address => 'http://localhost', :blog => blog)  
20 - e.reload 14 + should 'belong to blog' do
  15 + blog = create_blog
  16 + e = build(:external_feed, :blog => blog)
21 assert_equal blog, e.blog 17 assert_equal blog, e.blog
22 end 18 end
23 19
24 should 'not add same item twice' do 20 should 'not add same item twice' do
25 - e = ExternalFeed.create!(:address => 'http://localhost', :blog => blog) 21 + e = create(:external_feed)
26 assert e.add_item('Article title', 'http://orig.link.invalid', Time.now, 'Content for external post') 22 assert e.add_item('Article title', 'http://orig.link.invalid', Time.now, 'Content for external post')
27 assert !e.add_item('Article title', 'http://orig.link.invalid', Time.now, 'Content for external post') 23 assert !e.add_item('Article title', 'http://orig.link.invalid', Time.now, 'Content for external post')
28 assert_equal 1, e.blog.posts.size 24 assert_equal 1, e.blog.posts.size
29 end 25 end
30 26
31 - should 'nothing when clear' do 27 + should 'do nothing when clear' do
32 assert_respond_to ExternalFeed.new, :clear 28 assert_respond_to ExternalFeed.new, :clear
33 end 29 end
34 30
@@ -37,27 +33,99 @@ class ExternalFeedTest &lt; ActiveSupport::TestCase @@ -37,27 +33,99 @@ class ExternalFeedTest &lt; ActiveSupport::TestCase
37 end 33 end
38 34
39 should 'disable external feed if fetch only once on finish fetch' do 35 should 'disable external feed if fetch only once on finish fetch' do
40 - e = ExternalFeed.create(:address => 'http://localhost', :blog => blog, :only_once => true, :enabled => true)  
41 - assert e.enabled  
42 - assert e.finish_fetch  
43 - assert !e.enabled 36 + e = build(:external_feed, :only_once => true, :enabled => true)
  37 + e.stubs(:save!)
  38 + e.finish_fetch
  39 + assert_equal false, e.enabled
  40 + end
  41 +
  42 + should 'not disable after finish fetch if there are errors' do
  43 + e = build(:external_feed, :only_once => true, :update_errors => 1)
  44 + e.stubs(:save!)
  45 + e.finish_fetch
  46 + assert_equal true, e.enabled
  47 + end
  48 +
  49 + should 'be enabled by default' do
  50 + assert ExternalFeed.new.enabled
44 end 51 end
45 52
46 should 'add items to blog as posts' do 53 should 'add items to blog as posts' do
47 handler = FeedHandler.new 54 handler = FeedHandler.new
48 - e = ExternalFeed.create!(:address => 'test/fixtures/files/feed.xml', :blog => blog, :enabled => true) 55 + e = create(:external_feed)
49 handler.process(e) 56 handler.process(e)
50 assert_equal ["Last POST", "Second POST", "First POST"], e.blog.posts.map{|i| i.title} 57 assert_equal ["Last POST", "Second POST", "First POST"], e.blog.posts.map{|i| i.title}
51 end 58 end
52 59
53 should 'require address if enabled' do 60 should 'require address if enabled' do
54 - e = ExternalFeed.new(:blog => blog, :enabled => true) 61 + e = ExternalFeed.new(:enabled => true)
55 assert !e.valid? 62 assert !e.valid?
  63 + assert e.errors[:address]
56 end 64 end
57 65
58 should 'not require address if disabled' do 66 should 'not require address if disabled' do
59 - e = ExternalFeed.new(:blog => blog, :enabled => false)  
60 - assert e.valid? 67 + e = ExternalFeed.new(:enabled => false, :address => nil)
  68 + e.valid?
  69 + assert !e.errors[:address]
  70 + end
  71 +
  72 + should 'list enabled external feeds' do
  73 + e1 = fast_create(:external_feed, :enabled => true)
  74 + e2 = fast_create(:external_feed, :enabled => false)
  75 + assert_includes ExternalFeed.enabled, e1
  76 + assert_not_includes ExternalFeed.enabled, e2
  77 + end
  78 +
  79 + should 'have an empty error message by default' do
  80 + assert ExternalFeed.new.error_message.blank?, 'new external feed must have empty error message'
  81 + end
  82 +
  83 + should 'have empty fetch date by default' do
  84 + assert_nil ExternalFeed.new.fetched_at
  85 + end
  86 + should 'set fetch date when finishing fetch' do
  87 + feed = ExternalFeed.new
  88 + feed.stubs(:save!)
  89 + feed.finish_fetch
  90 + assert_not_nil feed.fetched_at
  91 + end
  92 +
  93 + should 'expire feeds after a certain period' do
  94 + # save current time
  95 + now = Time.now
  96 +
  97 + # Noosfero is configured to update feeds every 4 hours
  98 + FeedUpdater.stubs(:update_interval).returns(4.hours)
  99 +
  100 + expired = fast_create(:external_feed)
  101 + not_expired = fast_create(:external_feed)
  102 +
  103 + # 5 hours ago
  104 + Time.stubs(:now).returns(now - 5.hours)
  105 + expired.finish_fetch
  106 +
  107 + # 3 hours ago
  108 + Time.stubs(:now).returns(now - 3.hours)
  109 + not_expired.finish_fetch
  110 +
  111 + # now one feed should be expired and the not the other
  112 + Time.stubs(:now).returns(now)
  113 + expired_list = ExternalFeed.expired
  114 + assert_includes expired_list, expired
  115 + assert_not_includes expired_list, not_expired
  116 + end
  117 +
  118 + should 'consider recently-created instance as expired' do
  119 + new = fast_create(:external_feed)
  120 + assert_includes ExternalFeed.expired, new
  121 + end
  122 +
  123 + should 'have an update errors counter' do
  124 + assert_equal 3, ExternalFeed.new(:update_errors => 3).update_errors
  125 + end
  126 +
  127 + should 'have 0 update errors by default' do
  128 + assert_equal 0, ExternalFeed.new.update_errors
61 end 129 end
62 130
63 end 131 end
test/unit/feed_handler_test.rb
@@ -4,9 +4,12 @@ class FeedHandlerTest &lt; Test::Unit::TestCase @@ -4,9 +4,12 @@ class FeedHandlerTest &lt; Test::Unit::TestCase
4 4
5 def setup 5 def setup
6 @handler = FeedHandler.new 6 @handler = FeedHandler.new
7 - @container = FeedReaderBlock.create!(:box_id => 99999, :address => 'test/fixtures/files/feed.xml') 7 + @container = nil
  8 + end
  9 + attr_reader :handler
  10 + def container
  11 + @container ||= fast_create(:feed_reader_block)
8 end 12 end
9 - attr_reader :handler, :container  
10 13
11 should 'fetch feed content' do 14 should 'fetch feed content' do
12 content = handler.fetch(container.address) 15 content = handler.fetch(container.address)
@@ -68,15 +71,47 @@ class FeedHandlerTest &lt; Test::Unit::TestCase @@ -68,15 +71,47 @@ class FeedHandlerTest &lt; Test::Unit::TestCase
68 handler.process(container) 71 handler.process(container)
69 end 72 end
70 73
71 - should 'finish_fetch after processing' do 74 + should 'finish fetch after processing' do
  75 + container.expects(:finish_fetch)
  76 + handler.process(container)
  77 + end
  78 +
  79 + should 'finish fetch even in case of crash' do
  80 + container.expects(:clear).raises(Exception.new("crash"))
72 container.expects(:finish_fetch) 81 container.expects(:finish_fetch)
73 handler.process(container) 82 handler.process(container)
74 end 83 end
75 84
76 should 'identifies itself as noosfero user agent' do 85 should 'identifies itself as noosfero user agent' do
77 - handler = FeedHandler.new  
78 handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"}, anything).returns('bli content') 86 handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"}, anything).returns('bli content')
79 assert_equal 'bli content', handler.fetch('http://site.org/feed.xml') 87 assert_equal 'bli content', handler.fetch('http://site.org/feed.xml')
80 end 88 end
81 89
  90 + [:external_feed, :feed_reader_block].each do |container_class|
  91 +
  92 + should "reset the errors count after a successfull run (#{container_class})" do
  93 + container = fast_create(container_class, :update_errors => 1, :address => RAILS_ROOT + '/test/fixtures/files/feed.xml')
  94 + handler.expects(:actually_process_container).with(container)
  95 + handler.process(container)
  96 + assert_equal 0, container.update_errors
  97 + end
  98 +
  99 + should "set error message and disable in case of errors (#{container_class})" do
  100 + FeedHandler.stubs(:max_errors).returns(4)
  101 +
  102 + container = fast_create(container_class)
  103 + handler.stubs(:actually_process_container).with(container).raises(Exception.new("crash"))
  104 +
  105 + # in the first 4 errors, we are ok
  106 + 4.times { handler.process(container) }
  107 + assert !container.error_message.blank?, 'should set the error message for the first <max_errors> errors (%s)' % container_class
  108 + assert container.enabled, 'must keep container enabled during the first <max_errors> errors (%s)' % container_class
  109 +
  110 + # 5 errors it too much
  111 + handler.process(container)
  112 + assert !container.error_message.blank?, 'must set error message in container after <max_errors> errors (%s)' % container_class
  113 + assert !container.enabled, 'must disable continer after <max_errors> errors (%s)' % container_class
  114 + end
  115 + end
  116 +
82 end 117 end
test/unit/feed_reader_block_test.rb
@@ -5,12 +5,9 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase @@ -5,12 +5,9 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase
5 include DatesHelper 5 include DatesHelper
6 6
7 def setup 7 def setup
8 - @feed = FeedReaderBlock.new  
9 - @fetched_at = Time.now  
10 - @feed.fetched_at = @fetched_at  
11 - @feed.save! 8 + @feed = fast_create(:feed_reader_block)
12 end 9 end
13 - attr_reader :feed, :fetched_at 10 + attr_reader :feed
14 11
15 should 'default describe' do 12 should 'default describe' do
16 assert_not_equal Block.description, FeedReaderBlock.description 13 assert_not_equal Block.description, FeedReaderBlock.description
@@ -56,8 +53,10 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase @@ -56,8 +53,10 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase
56 end 53 end
57 54
58 should 'display last fetched date' do 55 should 'display last fetched date' do
  56 + now = Time.now
59 feed.feed_items = ['one', 'two'] 57 feed.feed_items = ['one', 'two']
60 - assert_equal "Updated: #{show_date(@fetched_at)}", feed.footer 58 + feed.fetched_at = now
  59 + assert_equal "Updated: #{show_date(now)}", feed.footer
61 end 60 end
62 61
63 should 'clear feed title and items' do 62 should 'clear feed title and items' do
@@ -73,6 +72,16 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase @@ -73,6 +72,16 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase
73 feed.finish_fetch 72 feed.finish_fetch
74 end 73 end
75 74
  75 + should 'set fetched_at when finishing a fetch' do
  76 + feed.stubs(:save!)
  77 + feed.finish_fetch
  78 + assert_not_nil feed.fetched_at
  79 + end
  80 +
  81 + should 'have empty fetched_at by default' do
  82 + assert_nil feed.fetched_at
  83 + end
  84 +
76 should 'display the latest post first' do 85 should 'display the latest post first' do
77 %w[ first-post second-post last-post ].each do |i| 86 %w[ first-post second-post last-post ].each do |i|
78 feed.add_item(i, "http://localhost/#{i}", Date.today, "some contet for #{i}") 87 feed.add_item(i, "http://localhost/#{i}", Date.today, "some contet for #{i}")
@@ -91,6 +100,77 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase @@ -91,6 +100,77 @@ class FeedReaderBlockTest &lt; ActiveSupport::TestCase
91 assert_no_tag_in_string feed.formatted_feed_content, :tag => 'a', :attributes => { :href => 'http://localhost/first-post' }, :content => 'first-post' 100 assert_no_tag_in_string feed.formatted_feed_content, :tag => 'a', :attributes => { :href => 'http://localhost/first-post' }, :content => 'first-post'
92 end 101 end
93 102
  103 + should 'have empty error message by default' do
  104 + assert FeedReaderBlock.new.error_message.blank?, 'new feed reader block expected to have empty error message'
  105 + end
  106 +
  107 + should "display error message as content when it's the case" do
  108 + msg = "there was a problem"
  109 + feed.error_message = msg
  110 + assert_match(msg, feed.content)
  111 + end
94 112
  113 + should 'expire after a period' do
  114 + # save current time
  115 + now = Time.now
  116 + expired = FeedReaderBlock.create!
  117 + not_expired = FeedReaderBlock.create!
  118 +
  119 + # Noosfero is configured to update feeds every 4 hours
  120 + FeedUpdater.stubs(:update_interval).returns(4.hours)
  121 +
  122 + # 5 hours ago
  123 + Time.stubs(:now).returns(now - 5.hours)
  124 + expired.finish_fetch
  125 +
  126 + # 3 hours ago
  127 + Time.stubs(:now).returns(now - 3.hours)
  128 + not_expired.finish_fetch
  129 +
  130 + # now one block should be expired and the not the other
  131 + Time.stubs(:now).returns(now)
  132 + expired_list = FeedReaderBlock.expired
  133 + assert_includes expired_list, expired
  134 + assert_not_includes expired_list, not_expired
  135 + end
  136 +
  137 + should 'consider recently-created as expired' do
  138 + # feed is created in setup
  139 + assert_includes FeedReaderBlock.expired, feed
  140 + end
  141 +
  142 + should 'have an update errors counter' do
  143 + assert_equal 5, FeedReaderBlock.new(:update_errors => 5).update_errors
  144 + end
  145 +
  146 + should 'have 0 errors by default' do
  147 + assert_equal 0, FeedReaderBlock.new.update_errors
  148 + end
  149 +
  150 + should 'be disabled by default' do
  151 + assert_equal false, FeedReaderBlock.new.enabled
  152 + end
  153 +
  154 + should 'be enabled when address is filled' do
  155 + reader = build(:feed_reader_block, :address => 'http://www.example.com/feed')
  156 + assert_equal true, reader.enabled
  157 + end
  158 +
  159 + should 'be disabled when address is empty' do
  160 + reader = build(:feed_reader_block, :enabled => true, :address => 'http://www.example.com/feed')
  161 + reader.address = nil
  162 + assert_equal false, reader.enabled
  163 + end
  164 +
  165 + should 're-enable when address is changed' do
  166 + reader = build(:feed_reader_block, :address => 'http://www.example.com/feed')
  167 + reader.enabled = false
  168 +
  169 + reader.address = 'http://www.example.com/feed'
  170 + assert_equal false, reader.enabled, 'must not enable when setting to the same address'
  171 +
  172 + reader.address = 'http://www.acme.com/feed'
  173 + assert_equal true, reader.enabled, 'must enable when setting to new address'
  174 + end
95 175
96 end 176 end
test/unit/feed_updater_test.rb 0 → 100644
@@ -0,0 +1,38 @@ @@ -0,0 +1,38 @@
  1 +require File.dirname(__FILE__) + '/../test_helper'
  2 +
  3 +class FeedUpdaterTest < Test::Unit::TestCase
  4 +
  5 + should 'be running by default' do
  6 + assert_equal true, FeedUpdater.new.running
  7 + end
  8 +
  9 + should 'unset running when stopped' do
  10 + updater = FeedUpdater.new
  11 + updater.expects(:running=).with(false)
  12 + updater.stop
  13 + end
  14 +
  15 + should 'sleep in intervals of one second' do
  16 + FeedUpdater.stubs(:daemon_sleep_interval).returns(30)
  17 + updater = FeedUpdater.new
  18 + updater.expects(:sleep).with(1).times(30)
  19 + updater.wait
  20 + end
  21 +
  22 + should 'not sleep when stopped' do
  23 + FeedUpdater.stubs(:daemon_sleep_interval).returns(30)
  24 + updater = FeedUpdater.new
  25 + updater.stubs(:running).returns(true).then.returns(true).then.returns(false)
  26 + updater.expects(:sleep).with(1).times(2)
  27 + updater.wait
  28 + end
  29 +
  30 + should 'process until it is stopped' do
  31 + updater = FeedUpdater.new
  32 + updater.stubs(:running).returns(true).then.returns(true).then.returns(false)
  33 + updater.expects(:process_round).times(2)
  34 + updater.expects(:wait).times(2)
  35 + updater.run
  36 + end
  37 +
  38 +end
test/unit/tiny_mce_article_test.rb
@@ -43,4 +43,10 @@ class TinyMceArticleTest &lt; Test::Unit::TestCase @@ -43,4 +43,10 @@ class TinyMceArticleTest &lt; Test::Unit::TestCase
43 article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "the <!-- comment --> article ...") 43 article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "the <!-- comment --> article ...")
44 assert_equal "the <!-- comment --> article ...", article.body 44 assert_equal "the <!-- comment --> article ...", article.body
45 end 45 end
  46 +
  47 + should 'convert entities characters to UTF-8 instead of ISO-8859-1' do
  48 + article = TinyMceArticle.create!(:profile => profile, :name => 'teste ' + Time.now.to_s, :body => '<a title="inform&#225;tica">link</a>')
  49 + assert(article.body.is_utf8?, "%s expected to be valid UTF-8 content" % article.body.inspect)
  50 + end
  51 +
46 end 52 end
vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb
@@ -22,7 +22,7 @@ HTML::WhiteListSanitizer.module_eval do @@ -22,7 +22,7 @@ HTML::WhiteListSanitizer.module_eval do
22 if !options[:attributes].include?(attr_name) || contains_bad_protocols?(attr_name, value) 22 if !options[:attributes].include?(attr_name) || contains_bad_protocols?(attr_name, value)
23 node.attributes.delete(attr_name) 23 node.attributes.delete(attr_name)
24 else 24 else
25 - node.attributes[attr_name] = attr_name == 'style' ? sanitize_css(value) : CGI::escapeHTML(CGI::unescapeHTML(value)) 25 + node.attributes[attr_name] = attr_name == 'style' ? sanitize_css(value) : CGI::escapeHTML(value.gsub('&amp;', '&'))
26 end 26 end
27 end 27 end
28 end 28 end