Merge branch 'xapian-dcabo' into develop

5 files changed, 60 insertions, 38 deletions

diff --git a/.gitignore b/.gitignore
index 92831606a..ab1f3004f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,4 +16,6 @@ TAGS
 /vendor/plugins/*theme
 /locale/model_attributes.rb
 /files/
-public/download
\ No newline at end of file
+public/download
+general.yml
+
diff --git a/app/views/layouts/default.rhtml b/app/views/layouts/default.rhtml
index 4008760a1..675bb38de 100644
--- a/app/views/layouts/default.rhtml
+++ b/app/views/layouts/default.rhtml
@@ -34,7 +34,7 @@
         <!--[if LT IE 8]> 
         <style type="text/css">@import url("/stylesheets/ie7.css");</style> 
         <![endif]-->
-	<!-- the following method for customising CSS is deprecated -- see `doc/THEMES.md for detail -->
+	<!-- the following method for customising CSS is deprecated; see `doc/THEMES.md` for detail -->
 	<%= stylesheet_link_tag 'custom', :title => "Main", :rel => "stylesheet"  %>
         <% if force_registration_on_new_request %>
         <%= stylesheet_link_tag 'jquery.fancybox-1.3.4', :rel => "stylesheet"  %>
diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb
index c23ff6e27..932966dfb 100644
--- a/spec/models/xapian_spec.rb
+++ b/spec/models/xapian_spec.rb
@@ -14,7 +14,6 @@ describe User, " when indexing users with Xapian" do
     it "should search by 'about me' text" do
         user = users(:bob_smith_user)
 
-        rebuild_xapian_index
           # def InfoRequest.full_search(models, query, order, ascending, collapse, per_page, page)
         xapian_object = InfoRequest.full_search([User], "stuff", 'created_at', true, nil, 100, 1)
         xapian_object.results.size.should == 1
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 20e630a26..ecb67a3b4 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -78,11 +78,11 @@ def load_file_fixture(file_name)
 end
 
 def rebuild_xapian_index
-    # XXX could for speed call ActsAsXapian.rebuild_index directly, but would
-    # need model name list, and would need to fix acts_as_xapian so can call writes
-    # and reads mixed up (it asserts where it thinks it can't do this)
-    rebuild_name = File.dirname(__FILE__) + '/../script/rebuild-xapian-index'
-    Kernel.system(rebuild_name) or raise "failed to launch #{rebuild_name}, error bitcode #{$?}, exit status: #{$?.exitstatus}"
+    verbose = false
+    # safe_rebuild=true, which involves forking to avoid memory leaks, doesn't work well with rspec. 
+    # unsafe is significantly faster, and we can afford possible memory leaks while testing.
+    safe_rebuild = false
+    ActsAsXapian.rebuild_index(["PublicBody", "User", "InfoRequestEvent"].map{|m| m.constantize}, verbose, safe_rebuild) 
 end
 
 def update_xapian_index
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 4671b79da..0af49dffd 100644
--- a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
+++ b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
@@ -35,7 +35,6 @@ module ActsAsXapian
         @@db = nil
         @@db_path = nil
         @@writable_db = nil
-        @@writable_suffix = nil
         @@init_values = []
         $acts_as_xapian_class_var_init = true
     end
@@ -217,7 +216,6 @@ module ActsAsXapian
         prepare_environment
 
         full_path = @@db_path + suffix
-        raise "writable_suffix/suffix inconsistency" if @@writable_suffix && @@writable_suffix != suffix
 
         # for indexing
         @@writable_db = Xapian::flint_open(full_path, Xapian::DB_CREATE_OR_OPEN)
@@ -225,7 +223,6 @@ module ActsAsXapian
         @@term_generator.set_flags(Xapian::TermGenerator::FLAG_SPELLING, 0)
         @@term_generator.database = @@writable_db
         @@term_generator.stemmer = @@stemmer
-        @@writable_suffix = suffix
     end
 
     ######################################################################
@@ -580,16 +577,20 @@ module ActsAsXapian
                 STDERR.puts(detail.backtrace.join("\n") + "\nFAILED ActsAsXapian.update_index job #{id} #{$!} " + (job.nil? ? "" : "model " + job.model + " id " + job.model_id.to_s))
             end
         end
-    end
         
+        # We close the database when we're finished to remove the lock file. Since writable_init 
+        # reopens it and recreates the environment every time we don't need to do further cleanup 
+        ActsAsXapian.writable_db.close
+    end
+    
     # You must specify *all* the models here, this totally rebuilds the Xapian
     # database.  You'll want any readers to reopen the database after this.
     #
     # Incremental update_index calls above are suspended while this rebuild
     # happens (i.e. while the .new database is there) - any index update jobs
     # are left in the database, and will run after the rebuild has finished.
-    def ActsAsXapian.rebuild_index(model_classes, verbose = false)
-        raise "when rebuilding all, please call as first and only thing done in process / task" if not ActsAsXapian.writable_db.nil?
+    def ActsAsXapian.rebuild_index(model_classes, verbose = false, safe_rebuild = true)
+        #raise "when rebuilding all, please call as first and only thing done in process / task" if not ActsAsXapian.writable_db.nil?
 
         prepare_environment
         
@@ -600,7 +601,47 @@ module ActsAsXapian
             FileUtils.rm_r(new_path)
         end
 
-        # Index everything 
+        # Index everything
+        if safe_rebuild
+            _rebuild_index_safely(model_classes, verbose)
+        else
+            # Save time by running the indexing in one go and in-process
+            ActsAsXapian.writable_init(".new")
+            for model_class in model_classes
+                STDOUT.puts("ActsAsXapian.rebuild_index: Rebuilding #{model_class.to_s}") if verbose
+                model_class.find(:all).each do |model|
+                  STDOUT.puts("ActsAsXapian.rebuild_index      #{model_class} #{model.id}") if verbose
+                  model.xapian_index
+                end
+            end
+            # make sure everything is written and close
+            ActsAsXapian.writable_db.flush
+            ActsAsXapian.writable_db.close
+        end            
+
+        # Rename into place
+        old_path = ActsAsXapian.db_path
+        temp_path = ActsAsXapian.db_path + ".tmp"
+        if File.exist?(temp_path)
+            raise "temporary database found " + temp_path + " which is not Xapian flint database, please delete for me" if not File.exist?(File.join(temp_path, "iamflint"))
+            FileUtils.rm_r(temp_path)
+        end
+        if File.exist?(old_path)
+            FileUtils.mv old_path, temp_path
+        end
+        FileUtils.mv new_path, old_path
+
+        # Delete old database
+        if File.exist?(temp_path)
+            raise "old database now at " + temp_path + " is not Xapian flint database, please delete for me" if not File.exist?(File.join(temp_path, "iamflint"))
+            FileUtils.rm_r(temp_path)
+        end
+
+        # You'll want to restart your FastCGI or Mongrel processes after this,
+        # so they get the new db
+    end
+
+    def ActsAsXapian._rebuild_index_safely(model_classes, verbose)
         batch_size = 1000
         for model_class in model_classes
             model_class_count = model_class.count
@@ -621,13 +662,14 @@ module ActsAsXapian
                     # (so doc ids and so on aren't preserved across the fork)
                     ActsAsXapian.writable_init(".new")
                     STDOUT.puts("ActsAsXapian.rebuild_index: New batch. #{model_class.to_s} from #{i} to #{i + batch_size} of #{model_class_count} pid #{Process.pid.to_s}") if verbose
-                    models = model_class.find(:all, :limit => batch_size, :offset => i, :order => :id)
-                    for model in models
+                    model_class.find(:all, :limit => batch_size, :offset => i, :order => :id).each do |model|
                       STDOUT.puts("ActsAsXapian.rebuild_index      #{model_class} #{model.id}") if verbose
                       model.xapian_index
                     end
                     # make sure everything is written
                     ActsAsXapian.writable_db.flush
+                    # close database
+                    ActsAsXapian.writable_db.close
                     # database connection won't survive a fork, so shut it down
                     ActiveRecord::Base.connection.disconnect!
                     # brutal exit, so other shutdown code not run (for speed and safety)
@@ -636,27 +678,6 @@ module ActsAsXapian
 
             end
         end
-
-        # Rename into place
-        old_path = ActsAsXapian.db_path
-        temp_path = ActsAsXapian.db_path + ".tmp"
-        if File.exist?(temp_path)
-            raise "temporary database found " + temp_path + " which is not Xapian flint database, please delete for me" if not File.exist?(File.join(temp_path, "iamflint"))
-            FileUtils.rm_r(temp_path)
-        end
-        if File.exist?(old_path)
-            FileUtils.mv old_path, temp_path
-        end
-        FileUtils.mv new_path, old_path
-
-        # Delete old database
-        if File.exist?(temp_path)
-            raise "old database now at " + temp_path + " is not Xapian flint database, please delete for me" if not File.exist?(File.join(temp_path, "iamflint"))
-            FileUtils.rm_r(temp_path)
-        end
-
-        # You'll want to restart your FastCGI or Mongrel processes after this,
-        # so they get the new db
     end
 
     ######################################################################