From 6a7221a9da5907305f1e335e6718e9b9e5f97aba Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Thu, 25 May 2017 06:34:15 +0900
Subject: [PATCH] Use URI Template (RFC 6570) for interpolating `search_url`

---
 app/models/agents/peak_detector_agent.rb       | 18 ++++++++++++++++--
 .../peak_detector_agent/_show.html.erb         |  3 ++-
 spec/models/agents/peak_detector_agent_spec.rb |  8 ++++++++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/app/models/agents/peak_detector_agent.rb b/app/models/agents/peak_detector_agent.rb
index 327b6e40..126f08d2 100644
--- a/app/models/agents/peak_detector_agent.rb
+++ b/app/models/agents/peak_detector_agent.rb
@@ -4,6 +4,8 @@ module Agents
   class PeakDetectorAgent < Agent
     cannot_be_scheduled!
 
+    DEFAULT_SEARCH_URL = 'https://twitter.com/search?q={q}'
+
     description <<-MD
       The Peak Detector Agent will watch for peaks in an event stream.  When a peak is detected, the resulting Event will have a payload message of `message`.  You can include extractions in the message, for example: `I saw a bar of: {{foo.bar}}`, have a look at the [Wiki](https://github.com/cantino/huginn/wiki/Formatting-Events-using-Liquid) for details.
 
@@ -15,7 +17,7 @@ module Agents
 
       You may set `min_events` for the minimal number of accumulated events before the agent starts detecting.
 
-      You may set `search_url` to point to something else than Twitter search. Default value is `https://twitter.com/search?q=%{q}` where `%{q}` will be replaced with group name.
+      You may set `search_url` to point to something else than Twitter search, using the URI Template syntax defined in [RFC 6570](https://tools.ietf.org/html/rfc6570). Default value is `#{DEFAULT_SEARCH_URL}` where `{q}` will be replaced with group name.
     MD
 
     event_description <<-MD
@@ -33,6 +35,15 @@ module Agents
       unless options['expected_receive_period_in_days'].present? && options['message'].present? && options['value_path'].present? && options['min_events'].present?
         errors.add(:base, "expected_receive_period_in_days, value_path, min_events and message are required")
       end
+      begin
+        tmpl = search_url
+      rescue => e
+        errors.add(:base, "search_url must be a valid URI template: #{e.message}")
+      else
+        unless tmpl.keys.include?('q')
+          errors.add(:base, "search_url must include a variable named 'q'")
+        end
+      end
     end
 
     def default_options
@@ -42,7 +53,6 @@ module Agents
         'value_path' => "count",
         'message' => "A peak of {{count}} was found in {{filter}}",
         'min_events' => '4',
-        'search_url' => 'https://twitter.com/search?q=%{q}'
       }
     end
 
@@ -58,6 +68,10 @@ module Agents
       end
     end
 
+    def search_url
+      Addressable::Template.new(options[:search_url].presence || DEFAULT_SEARCH_URL)
+    end
+
     private
 
     def check_for_peak(group, event)
diff --git a/app/views/agents/agent_views/peak_detector_agent/_show.html.erb b/app/views/agents/agent_views/peak_detector_agent/_show.html.erb
index 2f5f4a81..8c6bf641 100644
--- a/app/views/agents/agent_views/peak_detector_agent/_show.html.erb
+++ b/app/views/agents/agent_views/peak_detector_agent/_show.html.erb
@@ -3,9 +3,10 @@
 <% end %>
 <% if @agent.memory[:data] && @agent.memory[:data].length > 0 %>
   <h3>Recent Trends</h3>
+  <% search_url = @agent.search_url %>
   <% @agent.memory[:data].each.with_index do |(group_name, data), index| %>
     <div class="filter-group counts">
-      <div class='filter'><%= link_to group_name.to_s, format(@agent.options[:search_url].presence || @agent.default_options['search_url'], q: CGI::escape(group_name.to_s)), :target => "blank" %></div>
+      <div class='filter'><%= link_to group_name.to_s, search_url.expand(q: group_name.to_s).to_s, target: '_blank' %></div>
 
       <div class="chart-container group-<%= index.to_s %>">
         <div class="y-axis"></div>
diff --git a/spec/models/agents/peak_detector_agent_spec.rb b/spec/models/agents/peak_detector_agent_spec.rb
index ec000dd1..75f4aecf 100644
--- a/spec/models/agents/peak_detector_agent_spec.rb
+++ b/spec/models/agents/peak_detector_agent_spec.rb
@@ -99,5 +99,13 @@ describe Agents::PeakDetectorAgent do
       @agent.options['value_path'] = ""
       expect(@agent).not_to be_valid
     end
+
+    it "should validate search_url" do
+      @agent.options['search_url'] = 'https://twitter.com/'
+      expect(@agent).not_to be_valid
+
+      @agent.options['search_url'] = 'https://twitter.com/{q}'
+      expect(@agent).to be_valid
+    end
   end
 end