[PATCH conductor 2/6] Fixed restoring deployment state on db rollback

jprovazn at redhat.com jprovazn at redhat.com
Tue Jun 5 13:43:39 UTC 2012


From: Jan Provaznik <jprovazn at redhat.com>

AR transaction doesn't handle restoring object's state
on rollback by default. Till now I used rollback_active_record_state!
to restore the state but it turned out that this doesn't work in all cases.

In short the problem is that if you use some other save calls inside transaction,
restore might not be restored properly.

So instead of trying to restore deployment's state, new deployment object is created.

If the deployment's launch transaction si rolled back,
deployment's id and new_record attrs should be set as for
---
 src/app/controllers/deployments_controller.rb |    6 ++++++
 src/app/models/deployment.rb                  |   20 +++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
index 77b7cb8..5447377 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -159,6 +159,12 @@ class DeploymentsController < ApplicationController
         format.js { render :partial => 'properties' }
         format.json { render :json => @deployment, :status => :created }
       else
+        # if rollback was done, we create new @deployment object instead of
+        # trying restoring the original @deployment's state
+        # TODO: replace with 'initialize_dup' method after upgrading
+        # to newer Rails
+        @deployment = @deployment.copy_as_new
+
         # TODO: put deployment's errors into flash or display inside page?
         format.html do
           flash.now[:warning] = t "deployments.flash.warning.failed_to_launch"
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 0cf1137..55e557a 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -212,13 +212,13 @@ class Deployment < ActiveRecord::Base
 
   def create_and_launch(session, user)
     begin
-      rollback_active_record_state! do
-        # deployment's attrs are not reset on rollback,
-        # rollback_active_record_state! takes care of this
-        transaction do
-          create_instances_with_params!(session, user)
-          launch!(user)
-        end
+      # this method doesn't restore record state if this transaction
+      # fails, wrapping transaction in rollback_active_record_state!
+      # is not sufficient because restore then doesn't work if
+      # you have some nested save operations inside the transaction
+      transaction do
+        create_instances_with_params!(session, user)
+        launch!(user)
       end
       return true
     rescue
@@ -486,6 +486,12 @@ class Deployment < ActiveRecord::Base
     end
   end
 
+  def copy_as_new
+    d = Deployment.new(self.attributes)
+    d.errors.merge!(self.errors)
+    d
+  end
+
   PRESET_FILTERS_OPTIONS = []
 
   private
-- 
1.7.7.6




More information about the aeolus-devel mailing list