[macro-support-review] Refactoring broken parse_content and creating pipeline manager method

Rodrigo Souto
1 parent d1d530b1
Showing 4 changed files with 79 additions and 2 deletions Show diff stats
app/helpers/application_helper.rb
lib/noosfero/plugin.rb
lib/noosfero/plugin/manager.rb
test/unit/plugin_manager_test.rb
@@ -1407,6 +1407,9 @@ module ApplicationHelper
   def filter_html(html, source)
     if @plugins
       html = convert_macro(html, source)
+      #TODO This parse should be done through the macro infra, but since there
+      #     are old things that do not support it we are keeping this hot spot.
+      html = @plugins.pipeline(:parse_content, html, source).first
     end
     html
   end
@@ -252,8 +252,8 @@ class Noosfero::Plugin
  
   # -> Parse and possibly make changes of content (article, block, etc) during HTML rendering
   # returns = content as string after parser and changes
-  def parse_content(args)
-    args
+  def parse_content(html, source)
+    [html, source]
   end
  
   # -> Adds links to the admin panel
@@ -53,6 +53,17 @@ class Noosfero::Plugin::Manager
     result
   end
  
+
+  def pipeline(event, *args)
+    each do |plugin|
+      result = plugin.send(event, *args)
+      result = result.kind_of?(Array) ? result : [result]
+      raise ArgumentError, "Pipeline broken by #{plugin.class.name} on #{event} with #{result.length} arguments instead of #{args.length}." if result.length != args.length
+      args = result
+    end
+    args.length < 2 ? args.first : args
+  end
+
   def parse_macro(macro_name, macro, source = nil)
     macro_instance = enabled_macros[macro_name] || default_macro
     macro_instance.convert(macro, source)
@@ -194,4 +194,67 @@ class PluginManagerTest &lt; ActiveSupport::TestCase
     assert_equal 'My name is Macro2!', manager.parse_macro(Plugin1::Macro2.identifier, macro)
   end
  
+  should 'dispatch event in a pipeline sequence' do
+    class Plugin1 < Noosfero::Plugin
+      def transform(v1, v2)
+        v = 2
+        [v1 * v, v2 * v]
+      end
+    end
+
+    class Plugin2 < Noosfero::Plugin
+      def transform(v1, v2)
+        v = 5
+        [v1 * v, v2 * v]
+      end
+    end
+
+    environment.enable_plugin(Plugin1)
+    environment.enable_plugin(Plugin2)
+
+    assert_equal [10, 20], manager.pipeline(:transform, 1, 2)
+  end
+
+  should 'be able to pipeline with single arguments' do
+    class Plugin1 < Noosfero::Plugin
+      def transform(value)
+        value * 2
+      end
+    end
+
+    class Plugin2 < Noosfero::Plugin
+      def transform(value)
+        value * 5
+      end
+    end
+
+    environment.enable_plugin(Plugin1)
+    environment.enable_plugin(Plugin2)
+
+    assert_equal 10, manager.pipeline(:transform, 1)
+  end
+
+  should 'raise if pipeline is broken' do
+    class Plugin1 < Noosfero::Plugin
+      def transform(v1, v2)
+        v = 2
+        [v1 * v, v2 * v]
+      end
+    end
+
+    class Plugin2 < Noosfero::Plugin
+      def transform(v1, v2)
+        v = 5
+        [v1 * v, v2 * v, 666]
+      end
+    end
+
+    environment.enable_plugin(Plugin1)
+    environment.enable_plugin(Plugin2)
+
+    assert_raise ArgumentError do
+      manager.pipeline(:transform, 1, 2)
+    end
+  end
+
 end
...	...	@@ -1407,6 +1407,9 @@ module ApplicationHelper
1407	1407	def filter_html(html, source)
1408	1408	if @plugins
1409	1409	html = convert_macro(html, source)
	1410	+ #TODO This parse should be done through the macro infra, but since there
	1411	+ # are old things that do not support it we are keeping this hot spot.
	1412	+ html = @plugins.pipeline(:parse_content, html, source).first
1410	1413	end
1411	1414	html
1412	1415	end
...	...
...	...	@@ -252,8 +252,8 @@ class Noosfero::Plugin
252	252
253	253	# -> Parse and possibly make changes of content (article, block, etc) during HTML rendering
254	254	# returns = content as string after parser and changes
255		- def parse_content(args)
256		- args
	255	+ def parse_content(html, source)
	256	+ [html, source]
257	257	end
258	258
259	259	# -> Adds links to the admin panel
...	...
...	...	@@ -53,6 +53,17 @@ class Noosfero::Plugin::Manager
53	53	result
54	54	end
55	55
	56	+
	57	+ def pipeline(event, *args)
	58	+ each do \|plugin\|
	59	+ result = plugin.send(event, *args)
	60	+ result = result.kind_of?(Array) ? result : [result]
	61	+ raise ArgumentError, "Pipeline broken by #{plugin.class.name} on #{event} with #{result.length} arguments instead of #{args.length}." if result.length != args.length
	62	+ args = result
	63	+ end
	64	+ args.length < 2 ? args.first : args
	65	+ end
	66	+
56	67	def parse_macro(macro_name, macro, source = nil)
57	68	macro_instance = enabled_macros[macro_name] \|\| default_macro
58	69	macro_instance.convert(macro, source)
...	...
...	...	@@ -194,4 +194,67 @@ class PluginManagerTest < ActiveSupport::TestCase
194	194	assert_equal 'My name is Macro2!', manager.parse_macro(Plugin1::Macro2.identifier, macro)
195	195	end
196	196
	197	+ should 'dispatch event in a pipeline sequence' do
	198	+ class Plugin1 < Noosfero::Plugin
	199	+ def transform(v1, v2)
	200	+ v = 2
	201	+ [v1 * v, v2 * v]
	202	+ end
	203	+ end
	204	+
	205	+ class Plugin2 < Noosfero::Plugin
	206	+ def transform(v1, v2)
	207	+ v = 5
	208	+ [v1 * v, v2 * v]
	209	+ end
	210	+ end
	211	+
	212	+ environment.enable_plugin(Plugin1)
	213	+ environment.enable_plugin(Plugin2)
	214	+
	215	+ assert_equal [10, 20], manager.pipeline(:transform, 1, 2)
	216	+ end
	217	+
	218	+ should 'be able to pipeline with single arguments' do
	219	+ class Plugin1 < Noosfero::Plugin
	220	+ def transform(value)
	221	+ value * 2
	222	+ end
	223	+ end
	224	+
	225	+ class Plugin2 < Noosfero::Plugin
	226	+ def transform(value)
	227	+ value * 5
	228	+ end
	229	+ end
	230	+
	231	+ environment.enable_plugin(Plugin1)
	232	+ environment.enable_plugin(Plugin2)
	233	+
	234	+ assert_equal 10, manager.pipeline(:transform, 1)
	235	+ end
	236	+
	237	+ should 'raise if pipeline is broken' do
	238	+ class Plugin1 < Noosfero::Plugin
	239	+ def transform(v1, v2)
	240	+ v = 2
	241	+ [v1 * v, v2 * v]
	242	+ end
	243	+ end
	244	+
	245	+ class Plugin2 < Noosfero::Plugin
	246	+ def transform(v1, v2)
	247	+ v = 5
	248	+ [v1 * v, v2 * v, 666]
	249	+ end
	250	+ end
	251	+
	252	+ environment.enable_plugin(Plugin1)
	253	+ environment.enable_plugin(Plugin2)
	254	+
	255	+ assert_raise ArgumentError do
	256	+ manager.pipeline(:transform, 1, 2)
	257	+ end
	258	+ end
	259	+
197	260	end
...	...