Merge pull request #439 from bli/merge-with-lyfeyaj

supporting has_many reification based on @lyfeyaj and @NullVoxPopuli 's works

.gitignore

@@ -15,3 +15,5 @@ Gemfile.lock
 vendor/*
 .idea
 .rvmrc
+.tags
+.tags_sorted_by_file

README.md

@@ -12,7 +12,7 @@ There's an excellent [RailsCast on implementing Undo with Paper Trail](http://ra
 * Allows you to get at every version, including the original, even once destroyed.
 * Allows you to get at every version even if the schema has since changed.
 * Allows you to get at the version as of a particular time.
-* Option to automatically restore `has_one` associations as they were at the time.
+* Option to automatically restore `has_one`, `has_many` and `has_many :through` associations as they were at the time.
 * Automatically records who was responsible via your controller.  PaperTrail calls `current_user` by default, if it exists, but you can have it call any method you like.
 * Allows you to set who is responsible at model-level (useful for migrations).
 * Allows you to store arbitrary model-level metadata with each version (useful for filtering versions).
@@ -605,14 +605,9 @@ end
 
 ## Associations
 
-I haven't yet found a good way to get PaperTrail to automatically restore associations when you reify a model.  See [here for a little more info](http://airbladesoftware.com/notes/undo-and-redo-with-papertrail).
+PaperTrail can restore three types of associations: Has-One, Has-Many, and Has-Many-Through. In order to do this, you will need to create the `version_associations` table, either at installation time with the rails generate paper_trail:install --with-associations option or manually. PaperTrail will store in that table additional information to correlate versions of the association and versions of the model when the associated record is changed. When reifying the model, PaperTrail can use this table, together with the `transaction_id` to find the correct version of the association and reify it. The `transaction_id` is a unique id for version records created in the same transaction. It is used to associate the version of the model and the version of the association that are created in the same transaction.
 
-If you can think of a good way to achieve this, please let me know.
-
-
-## Has-One Associations
-
-PaperTrail can restore `:has_one` associations as they were at (actually, 3 seconds before) the time.
+To restore Has-One associations as they were at the time, pass option `:has_one => true` to `reify`. To restore Has-Many and Has-Many-Through associations, use option `:has_many => true`.  For example:
 
 ```ruby
 class Location < ActiveRecord::Base
@@ -636,21 +631,55 @@ end
 >> t.location.latitude              # 12.345
 ```
 
-The implementation is complicated by the edge case where the parent and child are updated in one go, e.g. in one web request or database transaction.  PaperTrail doesn't know about different models being updated "together", so you can't ask it definitively to get the child as it was before the joint parent-and-child update.
-
-The correct solution is to make PaperTrail aware of requests or transactions (c.f. [Efficiency's transaction ID middleware](http://github.com/efficiency20/ops_middleware/blob/master/lib/e20/ops/middleware/transaction_id_middleware.rb)).  In the meantime we work around the problem by finding the child as it was a few seconds before the parent was updated.  By default we go 3 seconds before but you can change this by passing the desired number of seconds to the `:has_one` option:
+If the parent and child are updated in one go, PaperTrail can use the aforementioned `transaction_id` to reify the models as they were before the transaction (instead of before the update to the model).
 
 ```ruby
->> t = treasure.versions.last.reify(:has_one => 1)  # look back 1 second instead of 3
+>> treasure.amount                  # 100
+>> treasure.location.latitude       # 12.345
+
+>> Treasure.transaction do
+>> treasure.location.update_attributes :latitude => 54.321
+>> treasure.update_attributes :amount => 153
+>> end
+
+>> t = treasure.versions.last.reify(:has_one => true)
+>> t.amount                         # 100
+>> t.location.latitude              # 12.345, instead of 54.321
 ```
 
-If you are shuddering, take solace from knowing PaperTrail opts out of these shenanigans by default. This means your `:has_one` associated objects will be the live ones, not the ones the user saw at the time.  Since PaperTrail doesn't auto-restore `:has_many` associations (I can't get it to work) or `:belongs_to` (I ran out of time looking at `:has_many`), this at least makes your associations wrong consistently ;)
+By default, PaperTrail excludes an associated record from the reified parent model if the associated record exists in the live model but did not exist as at the time the version was created. This is usually what you want if you just want to look at the reified version. But if you want to persist it, it would be better to pass in option `:mark_for_destruction => true` so that the associated record is included and marked for destruction.
 
+```ruby
+class Widget < ActiveRecord::Base
+  has_paper_trail
+  has_one :wotsit, autosave: true
+end
 
+class Wotsit < ActiveRecord::Base
+  has_paper_trail
+  belongs_to :widget
+end
+
+>> widget = Widget.create(:name => 'widget_0')
+>> widget.update_attributes(:name => 'widget_1')
+>> widget.create_wotsit(:name => 'wotsit')
 
-## Has-Many-Through Associations
+>> widget_0 = widget.versions.last.reify(:has_one => true)
+>> widget_0.wotsit                                  # nil
+
+>> widget_0 = widget.versions.last.reify(:has_one => true, :mark_for_destruction => true)
+>> widget_0.wotsit.marked_for_destruction?          # true
+>> widget_0.save!
+>> widget.reload.wotsit                             # nil
+```
+
+**Caveats:**
+
+  1. PaperTrail can't restore an association properly if the association record can be updated to replace its parent model (by replacing the foreign key)  
+  2. Currently PaperTrail only support single `version_associations` table. The implication is that you can only use a single table to store the versions for all related models. Sorry for those who use multiple version tables.
+  3. PaperTrail only reifies the first level of associations, i.e., it does not reify any associations of its associations, and so on.
+  4. PaperTrail relies on the callbacks on the association model (and the :through association model for Has-Many-Through associations) to record the versions and the relationship between the versions. If the association is changed without invoking the callbacks, Reification won't work. Below are some examples:  
 
-PaperTrail can track most changes to the join table.  Specifically it can track all additions but it can only track removals which fire the `after_destroy` callback on the join table.  Here are some examples:
 
 Given these models:
 
@@ -681,13 +710,14 @@ Then each of the following will store authorship versions:
 >> @book.authors.create :name => 'Tolstoy'
 >> @book.authorships.last.destroy
 >> @book.authorships.clear
+>> @book.author_ids = [@solzhenistyn.id, @dostoyevsky.id]
 ```
 
 But none of these will:
 
 ```ruby
 >> @book.authors.delete @tolstoy
->> @book.author_ids = [@solzhenistyn.id, @dostoyevsky.id]
+>> @book.author_ids = []
 >> @book.authors = []
 ```
 
@@ -712,9 +742,6 @@ end
 
 See [issue 113](https://github.com/airblade/paper_trail/issues/113) for a discussion about this.
 
-There may be a way to store authorship versions, probably using association callbacks, no matter how the collection is manipulated but I haven't found it yet.  Let me know if you do.
-
-There has been some discussion of how to implement PaperTrail to fully track HABTM associations. See [pull 90](https://github.com/airblade/paper_trail/pull/90) for an implementation that has worked for some.
 
 ## Storing metadata
 

gemfiles/3.0.gemfile

@@ -21,6 +21,13 @@ group :development, :test do
   # To do proper transactional testing with ActiveSupport::TestCase on MySQL
   gem 'database_cleaner', '~> 1.2.0'
 
+  # Allow time travel in testing. timecop is only supported after 1.9.2 but does a better cleanup at 'return'
+  if RUBY_VERSION < "1.9.2"
+    gem 'delorean'
+  else
+    gem 'timecop'
+  end
+
   platforms :ruby do 
     gem 'sqlite3', '~> 1.2'
     gem 'mysql2', '~> 0.3'

lib/generators/paper_trail/install_generator.rb

@@ -11,12 +11,23 @@ class InstallGenerator < ::Rails::Generators::Base
     desc 'Generates (but does not run) a migration to add a versions table.'
 
     def create_migration_file
-      migration_template 'create_versions.rb', 'db/migrate/create_versions.rb'
-      migration_template 'add_object_changes_to_versions.rb', 'db/migrate/add_object_changes_to_versions.rb' if options.with_changes?
+      add_paper_trail_migration('create_versions')
+      add_paper_trail_migration('add_object_changes_to_versions') if options.with_changes?
+      add_paper_trail_migration('create_version_associations')
+      add_paper_trail_migration('add_transaction_id_column_to_versions')
     end
 
     def self.next_migration_number(dirname)
       ::ActiveRecord::Generators::Base.next_migration_number(dirname)
     end
+
+    protected
+    def add_paper_trail_migration(template)
+      migration_dir = File.expand_path('db/migrate')
+
+      if !self.class.migration_exists?(migration_dir, template)
+        migration_template "#{template}.rb", "db/migrate/#{template}.rb"
+      end
+    end
   end
 end

lib/generators/paper_trail/templates/add_transaction_id_column_to_versions.rb

@@ -0,0 +1,11 @@
+class AddTransactionIdColumnToVersions < ActiveRecord::Migration
+  def self.up
+    add_column :versions, :transaction_id, :integer
+    add_index :versions, [:transaction_id]
+  end
+
+  def self.down
+    remove_index :versions, [:transaction_id]
+    remove_column :versions, :transaction_id
+  end
+end

lib/generators/paper_trail/templates/create_version_associations.rb

@@ -0,0 +1,17 @@
+class CreateVersionAssociations < ActiveRecord::Migration
+  def self.up
+    create_table :version_associations do |t|
+      t.integer  :version_id
+      t.string   :foreign_key_name, :null => false
+      t.integer  :foreign_key_id
+    end
+    add_index :version_associations, [:version_id]
+    add_index :version_associations, [:foreign_key_name, :foreign_key_id], :name => 'index_version_associations_on_foreign_key'
+  end
+
+  def self.down
+    remove_index :version_associations, [:version_id]
+    remove_index :version_associations, :name => 'index_version_associations_on_foreign_key'
+    drop_table :version_associations
+  end
+end

lib/paper_trail.rb

@@ -94,6 +94,18 @@ def self.active_record_protected_attributes?
     @active_record_protected_attributes ||= ::ActiveRecord::VERSION::MAJOR < 4 || !!defined?(ProtectedAttributes)
   end
 
+  def self.transaction?
+    ActiveRecord::Base.connection.open_transactions > 0
+  end
+
+  def self.transaction_id
+    paper_trail_store[:transaction_id]
+  end
+
+  def self.transaction_id=(id)
+    paper_trail_store[:transaction_id] = id
+  end
+
   private
 
   # Thread-safe hash to hold PaperTrail's data.

lib/paper_trail/frameworks/active_record/models/paper_trail/version_association.rb

@@ -0,0 +1,7 @@
+require 'paper_trail/version_association_concern'
+
+module PaperTrail
+  class VersionAssociation < ::ActiveRecord::Base
+    include PaperTrail::VersionAssociationConcern
+  end
+end

lib/paper_trail/has_paper_trail.rb

@@ -78,6 +78,10 @@ def has_paper_trail(options = {})
           after_update  :clear_version_instance!
         end
         after_destroy :record_destroy, :if => :save_version? if options_on.empty? || options_on.include?(:destroy)
+
+        # Reset the transaction id when the transaction is closed
+        after_commit :reset_transaction_id
+        after_rollback :reset_transaction_id
         after_rollback :clear_rolled_back_versions
       end
 
@@ -231,6 +235,16 @@ def without_versioning(method = nil)
         self.class.paper_trail_on! if paper_trail_was_enabled
       end
 
+      # Utility method for reifying. Anything executed inside the block will appear like a new record
+      def appear_as_new_record
+        instance_eval {
+          alias :old_new_record? :new_record?
+          alias :new_record? :present?
+        }
+        yield
+        instance_eval { alias :new_record? :old_new_record? }
+      end
+
       # Temporarily overwrites the value of whodunnit and then executes the provided block.
       def whodunnit(value)
         raise ArgumentError, 'expected to receive a block' unless block_given?
@@ -266,8 +280,9 @@ def source_version
       def record_create
         if paper_trail_switched_on?
           data = {
-            :event     => paper_trail_event || 'create',
-            :whodunnit => PaperTrail.whodunnit
+            :event          => paper_trail_event || 'create',
+            :whodunnit      => PaperTrail.whodunnit,
+            :transaction_id => PaperTrail.transaction_id
           }
           if respond_to?(:created_at)
             data[PaperTrail.timestamp_field] = created_at
@@ -276,17 +291,20 @@ def record_create
             data[:object_changes] = self.class.paper_trail_version_class.object_changes_col_is_json? ? changes_for_paper_trail :
               PaperTrail.serializer.dump(changes_for_paper_trail)
           end
-          send(self.class.versions_association_name).create! merge_metadata(data)
+          version = send(self.class.versions_association_name).create! merge_metadata(data)
+          set_transaction_id(version)
+          save_associations(version)
         end
       end
 
       def record_update
         if paper_trail_switched_on? && changed_notably?
           object_attrs = object_attrs_for_paper_trail(item_before_change)
           data = {
-            :event     => paper_trail_event || 'update',
-            :object    => self.class.paper_trail_version_class.object_col_is_json? ? object_attrs : PaperTrail.serializer.dump(object_attrs),
-            :whodunnit => PaperTrail.whodunnit
+            :event          => paper_trail_event || 'update',
+            :object         => self.class.paper_trail_version_class.object_col_is_json? ? object_attrs : PaperTrail.serializer.dump(object_attrs),
+            :whodunnit      => PaperTrail.whodunnit,
+            :transaction_id => PaperTrail.transaction_id
           }
           if respond_to?(:updated_at)
             data[PaperTrail.timestamp_field] = updated_at
@@ -295,7 +313,9 @@ def record_update
             data[:object_changes] = self.class.paper_trail_version_class.object_changes_col_is_json? ? changes_for_paper_trail :
               PaperTrail.serializer.dump(changes_for_paper_trail)
           end
-          send(self.class.versions_association_name).create merge_metadata(data)
+          version = send(self.class.versions_association_name).create merge_metadata(data)
+          set_transaction_id(version)
+          save_associations(version)
         end
       end
 
@@ -319,17 +339,45 @@ def record_destroy
         if paper_trail_switched_on? and not new_record?
           object_attrs = object_attrs_for_paper_trail(item_before_change)
           data = {
-            :item_id   => self.id,
-            :item_type => self.class.base_class.name,
-            :event     => paper_trail_event || 'destroy',
-            :object    => self.class.paper_trail_version_class.object_col_is_json? ? object_attrs : PaperTrail.serializer.dump(object_attrs),
-            :whodunnit => PaperTrail.whodunnit
+            :item_id        => self.id,
+            :item_type      => self.class.base_class.name,
+            :event          => paper_trail_event || 'destroy',
+            :object         => self.class.paper_trail_version_class.object_col_is_json? ? object_attrs : PaperTrail.serializer.dump(object_attrs),
+            :whodunnit      => PaperTrail.whodunnit,
+            :transaction_id => PaperTrail.transaction_id
           }
-          send("#{self.class.version_association_name}=", self.class.paper_trail_version_class.create(merge_metadata(data)))
+          version = self.class.paper_trail_version_class.create(merge_metadata(data))
+          send("#{self.class.version_association_name}=", version)
           send(self.class.versions_association_name).send :load_target
+          set_transaction_id(version)
+          save_associations(version)
+        end
+      end
+
+      def save_associations(version)
+        self.class.reflect_on_all_associations(:belongs_to).each do |assoc|
+          if assoc.klass.paper_trail_enabled_for_model?
+            PaperTrail::VersionAssociation.create(
+              :version_id => version.id,
+              :foreign_key_name => assoc.foreign_key,
+              :foreign_key_id => self.send(assoc.foreign_key)
+            )
+          end
+        end
+      end
+
+      def set_transaction_id(version)
+        if PaperTrail.transaction? && PaperTrail.transaction_id.nil?
+           PaperTrail.transaction_id = version.id
+           version.transaction_id = version.id
+           version.save
         end
       end
 
+      def reset_transaction_id
+        PaperTrail.transaction_id = nil
+      end
+
       def merge_metadata(data)
         # First we merge the model-level metadata in `meta`.
         paper_trail_options[:meta].each do |k,v|

lib/paper_trail/version_association_concern.rb

@@ -0,0 +1,13 @@
+require 'active_support/concern'
+
+module PaperTrail
+  module VersionAssociationConcern
+    extend ::ActiveSupport::Concern
+
+    included do
+      belongs_to :version
+
+      attr_accessible :version_id, :foreign_key_name, :foreign_key_id if PaperTrail.active_record_protected_attributes?
+    end
+  end
+end