Get date range searches working.

12 files changed, 109 insertions, 167 deletions

diff --git a/app/controllers/application.rb b/app/controllers/application.rb
index 79521e240..7275e53a7 100644
--- a/app/controllers/application.rb
+++ b/app/controllers/application.rb
@@ -6,7 +6,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: application.rb,v 1.40 2008-04-24 23:52:59 francis Exp $
+# $Id: application.rb,v 1.41 2008-04-30 00:37:50 francis Exp $
 
 
 class ApplicationController < ActionController::Base
@@ -127,7 +127,7 @@ class ApplicationController < ActionController::Base
         @http_auth_user = admin_http_auth_user
     end
 
-    # Convert URL name for sort by order, to Lucene query 
+    # Convert URL name for sort by order, to Xapian query 
     def order_to_sort_by(sortby)
         if sortby.nil?
             return [nil, true]
diff --git a/app/controllers/general_controller.rb b/app/controllers/general_controller.rb
index 9e2dccc9a..6ac91c605 100644
--- a/app/controllers/general_controller.rb
+++ b/app/controllers/general_controller.rb
@@ -5,7 +5,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: general_controller.rb,v 1.19 2008-04-24 23:52:59 francis Exp $
+# $Id: general_controller.rb,v 1.20 2008-04-30 00:37:50 francis Exp $
 
 class GeneralController < ApplicationController
 
@@ -47,14 +47,19 @@ class GeneralController < ApplicationController
             @query = nil
             render :action => "search"
         else
-            redirect_to search_url(:query => @query, :sortby => @sortby)
+            redirect_to search_url(@query, @sortby)
         end
     end
 
     # Actual search
     def search
-        query = params[:query]
-        sortby = params[:sortby]
+        combined = params[:combined]
+        sortby = nil
+        if combined.size > 1 and combined[-1] == 'newest'
+            sortby = 'newest'
+            combined = combined[0..-2]
+        end
+        query = combined.join("/")
         perform_search(query, sortby)
 
         #render :controller => "help", :action => "about"
diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb
index ab8915e5d..bc0bdbafe 100644
--- a/app/helpers/link_to_helper.rb
+++ b/app/helpers/link_to_helper.rb
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: link_to_helper.rb,v 1.25 2008-04-09 01:32:53 francis Exp $
+# $Id: link_to_helper.rb,v 1.26 2008-04-30 00:37:50 francis Exp $
 
 module LinkToHelper
 
@@ -81,6 +81,15 @@ module LinkToHelper
         link_to h(user.name), user_admin_url(user)
     end
 
+    # General pages
+    def search_url(query, sortby = nil)
+        if sortby.nil?
+            combined = query
+        else
+            combined = query + "/" + sortby
+        end
+        search_general_url(:combined => combined)
+    end
 
     # Admin pages
     def admin_url(relative_path)
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 99312986d..e75127667 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -21,7 +21,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: info_request.rb,v 1.98 2008-04-29 16:42:11 francis Exp $
+# $Id: info_request.rb,v 1.99 2008-04-30 00:37:50 francis Exp $
 
 require 'digest/sha1'
 require 'vendor/plugins/acts_as_xapian/lib/acts_as_xapian'
@@ -75,8 +75,8 @@ class InfoRequest < ActiveRecord::Base
             [InfoRequestEvent, PublicBody, User], query,
             :offset => offset, :limit => per_page,
             :sort_by_prefix => order,
-            :sort_by_ascending => ascending, 
-            :collapse_by_prefix => "request_collapse"
+            :sort_by_ascending => ascending
+#            :collapse_by_prefix => "request_collapse" # XXX fix this so off for email/RSS, on for web
         )
     end
 
diff --git a/app/views/general/search.rhtml b/app/views/general/search.rhtml
index a919bff45..929fa2871 100644
--- a/app/views/general/search.rhtml
+++ b/app/views/general/search.rhtml
@@ -20,14 +20,14 @@
 <% end %>
 
 <% if not @search_results.nil? %>
-    <%=link_to_unless @sortby.nil?, "Show most relevant results first", { :sortby => nil } %>
+    <%=link_to_unless @sortby.nil?, "Show most relevant results first", search_url(@query, nil) %>
     |
-    <%=link_to_unless @sortby == 'newest', "Newest results first", { :sortby => "newest" } %>
+    <%=link_to_unless @sortby == 'newest', "Newest results first", search_url(@query, 'newest') %>
 
     <h1><%=@title%></h1>
 
     <% if @search_spelling %>
-        <p id="did_you_mean">Did you mean: <%= link_to @search_spelling, search_url(:query => @search_spelling) %></p>
+        <p id="did_you_mean">Did you mean: <%= link_to @search_spelling, search_url(@search_spelling, @sortby) %></p>
     <% end %>
 
     <% if @search_results.empty? %>
@@ -59,7 +59,7 @@
     <li><strong>requested_from:home_office</strong> to restrict to requests from the <%= link_to "Home Office", show_public_body_url(:url_name => 'home_office') %>, typing the name as in the URL.
     <li><strong>requested_by:julian_todd</strong> to restrict to requests made by <%= link_to "Julian Todd", show_user_url(:url_name => 'julian_todd') %>, typing the name as in the URL.
     <li><strong>request:</strong> to restrict to a specific request, typing the title as in the URL.
-    <li>Read about <a href="http://lucene.apache.org/java/docs/queryparsersyntax.html">advanced search operators</a>, such as fuzziness and proximity.
+    <li>Read about <a href="http://www.xapian.org/docs/queryparser.html">advanced search operators</a>, such as proximity and wildcards.
     </ul>
 
     <table class="status_table">
diff --git a/app/views/request/show.rhtml b/app/views/request/show.rhtml
index ae9f2bda2..a13a129b5 100644
--- a/app/views/request/show.rhtml
+++ b/app/views/request/show.rhtml
@@ -51,7 +51,7 @@
         <!-- <h2>Some other requests made by <%= user_link(@info_request.user) %> to <%= public_body_link(@info_request.public_body) %></h2> -->
         <%= render :partial => 'sidebar_request_listing', :locals => { :info_requests => @info_requests_same_user_same_body } %>
         <% if @info_requests_same_user_same_body_more %>
-            <p><%= link_to "More such requests", search_url(:query => "variety:sent requested_from:" + @info_request.public_body.url_name + " requested_by:" + @info_request.user.url_name + " -request:" + @info_request.url_title, :sortby => "newest") %></p>
+            <p><%= link_to "More such requests", search_url("variety:sent requested_from:" + @info_request.public_body.url_name + " requested_by:" + @info_request.user.url_name + " -request:" + @info_request.url_title, "newest") %></p>
         <% end %>
     <% end %>
 
diff --git a/config/routes.rb b/config/routes.rb
index 07ba151e5..bafc7ff36 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: routes.rb,v 1.55 2008-04-18 01:57:43 francis Exp $
+# $Id: routes.rb,v 1.56 2008-04-30 00:37:51 francis Exp $
 
 ActionController::Routing::Routes.draw do |map|
 
@@ -19,7 +19,7 @@ ActionController::Routing::Routes.draw do |map|
         general.auto_complete_for_public_body_query 'auto_complete_for_public_body_query', :action => 'auto_complete_for_public_body_query'
 
         general.search_redirect '/search',      :action => 'search_redirect'
-        general.search '/search/:query/:sortby',      :action => 'search', :sortby => nil
+        general.search_general '/search/*combined',      :action => 'search'
 
         general.fai_test '/test', :action => 'fai_test'
     end
diff --git a/todo.txt b/todo.txt
index 0601490b4..2bf641e58 100644
--- a/todo.txt
+++ b/todo.txt
@@ -11,8 +11,8 @@ deployment:
     install it on the server
     make sure solr is stopped
 
-check that you aren't doing collapse_by for RSS / Email
-date range tips in search help
+XXX date range tips in search help
+check that you aren't doing collapse_by for RSS / Email (via full_search)
 check emails look OK
 
 remove all the scripts in scripts/*solr*
@@ -20,25 +20,25 @@ remove all the solr plugins and stuff
 
 http://localhost:3000/list - sent highlighted here
 
-You need to reload db after deleting stuff, how do we deal with that?
-
 highlight word docs text
 full_search still has html_highlight parameter
 
-follow up E re. blog
-needs tagline that v. quickly explains what site is on each page
-needs development text box
-
-
 Watch this one:
 http://www.whatdotheyknow.com/request/crime_statistics_for_2007_champi
 
-Group requests by response
-Make it totally awesome like in Tommy's screenshot
-Cluster solr patch - https://issues.apache.org/jira/browse/SOLR-236
+Design
+------
 
-Search for "health" crashes it (Solr?)
+needs tagline that v. quickly explains what site is on each page
+needs development text box
 
+Later Solr
+----------
+
+You need to reload Xapian processes after deleting stuff (and maybe other
+    times?), how do we deal with that? - maybe reload xapian db every x searches?
+Search for "health" crashes it (Solr?)
+Remove vendor/plugins/acts_as_solr
 
 FOI requests to use to test it
 ==============================
@@ -238,6 +238,7 @@ Quoting fixing TODO:
     http://www.whatdotheyknow.com/request/51/response/93
     http://www.whatdotheyknow.com/request/56/response/252
     http://www.whatdotheyknow.com/request/298/response/415
+    http://www.whatdotheyknow.com/request/119/response/447 # remove double blank lines
    
 Sources of public bodies
 ========================
diff --git a/vendor/plugins/acts_as_solr/lib/tasks/database.rake b/vendor/plugins/acts_as_solr/lib/tasks/database.rake
deleted file mode 100644
index c02dd3b69..000000000
--- a/vendor/plugins/acts_as_solr/lib/tasks/database.rake
+++ /dev/null
@@ -1,18 +0,0 @@
-require File.dirname(__FILE__) + '/../solr_fixtures'
-
-namespace :db do
-  namespace :fixtures do
-    desc "Load fixtures into the current environment's database. Load specific fixtures using FIXTURES=x,y"
-    task :load => :environment do
-      begin
-        ActsAsSolr::Post.execute(Solr::Request::Delete.new(:query => "*:*"))
-        ActsAsSolr::Post.execute(Solr::Request::Commit.new)
-        (ENV['FIXTURES'] ? ENV['FIXTURES'].split(/,/) : Dir.glob(File.join(RAILS_ROOT, 'test', 'fixtures', '*.{yml,csv}'))).each do |fixture_file|    
-          ActsAsSolr::SolrFixtures.load(File.basename(fixture_file, '.*'))
-        end 
-        puts "The fixtures loaded have been added to Solr"       
-      rescue
-      end
-    end
-  end
-end
\ No newline at end of file
diff --git a/vendor/plugins/acts_as_solr/lib/tasks/solr.rake b/vendor/plugins/acts_as_solr/lib/tasks/solr.rake
deleted file mode 100644
index d57be7e12..000000000
--- a/vendor/plugins/acts_as_solr/lib/tasks/solr.rake
+++ /dev/null
@@ -1,58 +0,0 @@
-require 'rubygems'
-require 'rake'
-require 'net/http'
-require 'active_record'
-require "#{File.dirname(__FILE__)}/../../config/environment.rb"
-
-namespace :solr do
-
-  desc 'Starts Solr. Options accepted: RAILS_ENV=your_env, PORT=XX. Defaults to development if none.'
-  task :start do
-    begin
-      n = Net::HTTP.new('localhost', SOLR_PORT)
-      n.request_head('/').value 
-
-    rescue Net::HTTPServerException #responding
-      puts "Port #{SOLR_PORT} in use" and return
-
-    rescue Errno::ECONNREFUSED #not responding
-      Dir.chdir(SOLR_PATH) do
-        pid = fork do
-          #STDERR.close
-          exec "java -Dsolr.data.dir=solr/data/#{ENV['RAILS_ENV']} -Djetty.port=#{SOLR_PORT} -jar start.jar"
-        end
-        sleep(5)
-        File.open("#{SOLR_PATH}/tmp/#{ENV['RAILS_ENV']}_pid", "w"){ |f| f << pid}
-        puts "#{ENV['RAILS_ENV']} Solr started successfully on #{SOLR_PORT}, pid: #{pid}."
-      end
-    end
-  end
-  
-  desc 'Stops Solr. Specify the environment by using: RAILS_ENV=your_env. Defaults to development if none.'
-  task :stop do
-    fork do
-      file_path = "#{SOLR_PATH}/tmp/#{ENV['RAILS_ENV']}_pid"
-      if File.exists?(file_path)
-        File.open(file_path, "r") do |f| 
-          pid = f.readline
-          Process.kill('TERM', pid.to_i)
-        end
-        File.unlink(file_path)
-        Rake::Task["solr:destroy_index"].invoke if ENV['RAILS_ENV'] == 'test'
-        puts "Solr shutdown successfully."
-      else
-        puts "Solr is not running.  I haven't done anything."
-      end
-    end
-  end
-  
-  desc 'Remove Solr index'
-  task :destroy_index do
-    raise "In production mode.  I'm not going to delete the index, sorry." if ENV['RAILS_ENV'] == "production"
-    if File.exists?("#{SOLR_PATH}/solr/data/#{ENV['RAILS_ENV']}")
-      Dir[ SOLR_PATH + "/solr/data/#{ENV['RAILS_ENV']}/index/*"].each{|f| File.unlink(f)}
-      Dir.rmdir(SOLR_PATH + "/solr/data/#{ENV['RAILS_ENV']}/index")
-      puts "Index files removed under " + ENV['RAILS_ENV'] + " environment"
-    end
-  end
-end
diff --git a/vendor/plugins/acts_as_solr/lib/tasks/test.rake b/vendor/plugins/acts_as_solr/lib/tasks/test.rake
deleted file mode 100644
index 6bb6cafbf..000000000
--- a/vendor/plugins/acts_as_solr/lib/tasks/test.rake
+++ /dev/null
@@ -1,7 +0,0 @@
-require 'active_record'
-
-namespace :test do
-  task :migrate do
-    ActiveRecord::Migrator.migrate("test/db/migrate/", ENV["VERSION"] ? ENV["VERSION"].to_i : nil)
-  end
-end
diff --git a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
index a0d88f721..4cb3d6a08 100644
--- a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
+++ b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: acts_as_xapian.rb,v 1.13 2008-04-29 16:42:11 francis Exp $
+# $Id: acts_as_xapian.rb,v 1.14 2008-04-30 00:37:51 francis Exp $
 
 # TODO:
 # Test :eager_load
@@ -192,7 +192,7 @@ module ActsAsXapian
     # Initialisation
     def ActsAsXapian.init(classname = nil, options = nil)
         if not classname.nil?
-            # store class and options for use later, when we open the db in late_init
+            # store class and options for use later, when we open the db in readable_init
             @@init_values.push([classname,options])
         end
 
@@ -205,64 +205,74 @@ module ActsAsXapian
         # XXX this gets made once for each acts_as_xapian. Oh well.
         @@stemmer = Xapian::Stem.new('english')
     end
-    # called only when we *need* to open the db
-    def ActsAsXapian.late_init
+
+    # Called only when we *need* to open the db
+    def ActsAsXapian.readable_init
         if @@db.nil?
             # basic Xapian objects
-            @@db = Xapian::Database.new()
             @@db = Xapian::Database.new(@@db_path)
             @@enquire = Xapian::Enquire.new(@@db)
 
-            # for queries
-            @@query_parser = Xapian::QueryParser.new
-            @@query_parser.stemmer = @@stemmer
-            @@query_parser.stemming_strategy = Xapian::QueryParser::STEM_SOME
-            @@query_parser.database = @@db
-            @@query_parser.default_op = Xapian::Query::OP_AND
-
-            @@terms_by_capital = {}
-            @@values_by_number = {}
-            @@values_by_prefix = {}
-
-            for init_value_pair in @@init_values
-                classname = init_value_pair[0]
-                options = init_value_pair[1]
-
-                # go through the various field types, and tell query parser about them,
-                # and error check them - i.e. check for consistency between models
-                @@query_parser.add_boolean_prefix("model", "M")
-                @@query_parser.add_boolean_prefix("modelid", "I")
-                for term in options[:terms]
-                    raise "Use a single capital letter for term code" if not term[1].match(/^[A-Z]$/)
-                    raise "M and I are reserved for use as the model/id term" if term[1] == "M" or term[1] == "I"
-                    raise "model and modelid are reserved for use as the model/id prefixes" if term[2] == "model" or term[2] == "modelid"
-                    raise "Z is reserved for stemming terms" if term[1] == "Z"
-                    raise "Already have code '" + term[1] + "' in another model but with different prefix '" + @@terms_by_capital[term[1]] + "'" if @@terms_by_capital.include?(term[1]) && @@terms_by_capital[term[1]] != term[2]
-                    @@terms_by_capital[term[1]] = term[2]
-                    @@query_parser.add_boolean_prefix(term[2], term[1])
-                end
-                for value in options[:values]
-                    raise "Value index '"+value[1].to_s+"' must be an integer, is " + value[1].class.to_s if value[1].class != 1.class
-                    raise "Already have value index '" + value[1].to_s + "' in another model but with different prefix '" + @@values_by_number[value[1]].to_s + "'" if @@values_by_number.include?(value[1]) && @@values_by_number[value[1]] != value[2]
-
-                    # date types are special, mark them so the first model they're seen for
-                    if !@@values_by_number.include?(value[1])
-                        if value[3] == :date 
-                            value_range = Xapian::DateValueRangeProcessor.new(value[1])
-                        elsif value[3] == :string 
-                            value_range = Xapian::StringValueRangeProcessor.new(value[1])
-                        elsif value[3] == :number
-                            value_range = Xapian::NumberValueRangeProcessor.new(value[1])
-                        else
-                            raise "Unknown value type '" + value[3].to_s + "'"
-                        end
-
-                        @@query_parser.add_valuerangeprocessor(value_range)
+            init_query_parser
+        end
+    end
+
+    # Make a new query parser
+    def ActsAsXapian.init_query_parser
+        # for queries
+        @@query_parser = Xapian::QueryParser.new
+        @@query_parser.stemmer = @@stemmer
+        @@query_parser.stemming_strategy = Xapian::QueryParser::STEM_SOME
+        @@query_parser.database = @@db
+        @@query_parser.default_op = Xapian::Query::OP_AND
+
+        @@terms_by_capital = {}
+        @@values_by_number = {}
+        @@values_by_prefix = {}
+        @@value_ranges_store = []
+
+        for init_value_pair in @@init_values
+            classname = init_value_pair[0]
+            options = init_value_pair[1]
+
+            # go through the various field types, and tell query parser about them,
+            # and error check them - i.e. check for consistency between models
+            @@query_parser.add_boolean_prefix("model", "M")
+            @@query_parser.add_boolean_prefix("modelid", "I")
+            for term in options[:terms]
+                raise "Use a single capital letter for term code" if not term[1].match(/^[A-Z]$/)
+                raise "M and I are reserved for use as the model/id term" if term[1] == "M" or term[1] == "I"
+                raise "model and modelid are reserved for use as the model/id prefixes" if term[2] == "model" or term[2] == "modelid"
+                raise "Z is reserved for stemming terms" if term[1] == "Z"
+                raise "Already have code '" + term[1] + "' in another model but with different prefix '" + @@terms_by_capital[term[1]] + "'" if @@terms_by_capital.include?(term[1]) && @@terms_by_capital[term[1]] != term[2]
+                @@terms_by_capital[term[1]] = term[2]
+                @@query_parser.add_boolean_prefix(term[2], term[1])
+            end
+            for value in options[:values]
+                raise "Value index '"+value[1].to_s+"' must be an integer, is " + value[1].class.to_s if value[1].class != 1.class
+                raise "Already have value index '" + value[1].to_s + "' in another model but with different prefix '" + @@values_by_number[value[1]].to_s + "'" if @@values_by_number.include?(value[1]) && @@values_by_number[value[1]] != value[2]
+
+                # date types are special, mark them so the first model they're seen for
+                if !@@values_by_number.include?(value[1])
+                    if value[3] == :date 
+                        value_range = Xapian::DateValueRangeProcessor.new(value[1])
+                    elsif value[3] == :string 
+                        value_range = Xapian::StringValueRangeProcessor.new(value[1])
+                    elsif value[3] == :number
+                        value_range = Xapian::NumberValueRangeProcessor.new(value[1])
+                    else
+                        raise "Unknown value type '" + value[3].to_s + "'"
                     end
 
-                    @@values_by_number[value[1]] = value[2]
-                    @@values_by_prefix[value[2]] = value[1]
+                    @@query_parser.add_valuerangeprocessor(value_range)
+
+                    # stop it being garbage collected, as
+                    # add_valuerangeprocessor ref is outside Ruby's GC
+                    @@value_ranges_store.push(value_range) 
                 end
+
+                @@values_by_number[value[1]] = value[2]
+                @@values_by_prefix[value[2]] = value[1]
             end
         end
     end
@@ -306,7 +316,7 @@ module ActsAsXapian
             sort_by_ascending = options[:sort_by_ascending] || true
             collapse_by_prefix = options[:collapse_by_prefix] || nil
 
-            ActsAsXapian.late_init
+            ActsAsXapian.readable_init
             if ActsAsXapian.db.nil?
                 raise "ActsAsXapian not initialized"
             end
@@ -328,7 +338,7 @@ module ActsAsXapian
                 ActsAsXapian.enquire.sort_by_value_then_relevance!(value, sort_by_ascending)
             end
             if collapse_by_prefix.nil?
-                ActsAsXapian.enquire.collapse_key = nil
+                ActsAsXapian.enquire.collapse_key = Xapian.BAD_VALUENO
             else
                 value = ActsAsXapian.values_by_prefix[collapse_by_prefix]
                 raise "couldn't find prefix '" + collapse_by_prefix + "'" if value.nil?