#19580: Executing data migrations transactionally

Work Item: 19580

--HG--
branch : 1.x

src/Orchard.Tests/ContentManagement/DefaultContentManagerTests.cs

@@ -89,8 +89,10 @@ namespace Orchard.Tests.ContentManagement {
             _manager = _container.Resolve<IContentManager>();
         }
 
-        public class TestSessionLocator : ISessionLocator, ITransactionManager {
+        public class TestSessionLocator : ISessionLocator, ITransactionManager, IDisposable {
             private readonly ISession _session;
+            private ITransaction _transaction;
+            private bool _cancelled;
 
             public TestSessionLocator(ISession session) {
                 _session = session;
@@ -100,16 +102,64 @@ namespace Orchard.Tests.ContentManagement {
                 return _session;
             }
 
-            public void Demand() {
+            void ITransactionManager.Demand() {
+                EnsureSession();
+
+                if (_transaction == null) {
+                    _transaction = _session.BeginTransaction(IsolationLevel.ReadCommitted);
+                }
             }
 
-            public void RequireNew() {
+            void ITransactionManager.RequireNew() {
+                ((ITransactionManager)this).RequireNew(IsolationLevel.ReadCommitted);
             }
 
-            public void RequireNew(IsolationLevel level) {
+            void ITransactionManager.RequireNew(IsolationLevel level) {
+                EnsureSession();
+
+                if (_cancelled) {
+                    _transaction.Rollback();
+                    _transaction.Dispose();
+                    _transaction = null;
+                }
+                else {
+                    if (_transaction != null) {
+                        _transaction.Commit();
+                    }
+                }
+
+                _transaction = _session.BeginTransaction(level);
             }
 
-            public void Cancel() {
+            void ITransactionManager.Cancel() {
+                _cancelled = true;
+            }
+
+            void IDisposable.Dispose() {
+                if (_transaction != null) {
+                    try {
+                        if (!_cancelled) {
+                            _transaction.Commit();
+                        }
+                        else {
+                            _transaction.Rollback();
+                        }
+
+                        _transaction.Dispose();
+                    }
+                    catch (Exception e) {
+                    }
+                    finally {
+                        _transaction = null;
+                        _cancelled = false;
+                    }
+                }
+            }
+
+            private void EnsureSession() {
+                if (_session != null) {
+                    return;
+                }
             }
         }
 

src/Orchard.Tests/DataMigration/DataMigrationTests.cs

@@ -32,6 +32,7 @@ namespace Orchard.Tests.DataMigration {
 
         private ISessionFactory _sessionFactory;
         private ISession _session;
+        private ITransactionManager _transactionManager;
 
         [TestFixtureSetUp]
         public void CreateDb() {
@@ -71,7 +72,7 @@ namespace Orchard.Tests.DataMigration {
             builder.RegisterType<StubParallelCacheContext>().As<IParallelCacheContext>();
             builder.RegisterType<StubAsyncTokenProvider>().As<IAsyncTokenProvider>();
             _session = _sessionFactory.OpenSession();
-            builder.RegisterInstance(new DefaultContentManagerTests.TestSessionLocator(_session)).As<ISessionLocator>();
+            builder.RegisterInstance(new DefaultContentManagerTests.TestSessionLocator(_session)).As<ISessionLocator>().As<ITransactionManager>();
             foreach(var type in dataMigrations) {
                 builder.RegisterType(type).As<IDataMigration>();
             }
@@ -79,7 +80,7 @@ namespace Orchard.Tests.DataMigration {
             _container.Resolve<IExtensionManager>();
             _dataMigrationManager = _container.Resolve<IDataMigrationManager>();
             _repository = _container.Resolve<IRepository<DataMigrationRecord>>();
-
+            _transactionManager = _container.Resolve<ITransactionManager>();
             InitDb();
         }
 
@@ -212,6 +213,31 @@ namespace Orchard.Tests.DataMigration {
                 return 999;
             }
         }
+
+
+        public class DataMigrationTransactional : DataMigrationImpl {
+            public override Feature Feature {
+                get { return new Feature() { Descriptor = new FeatureDescriptor { Id = "Feature1", Extension = new ExtensionDescriptor { Id = "Module1" } } }; }
+            }
+
+            public int Create() {
+                SchemaBuilder.CreateTable("FOO", table =>
+                    table.Column("Id", DbType.Int32, column =>
+                        column.PrimaryKey().Identity()));
+
+                return 1;
+            }
+
+            public int UpdateFrom1() {
+                throw new Exception();
+
+                return 2;
+            }
+
+            public int UpdateFrom2() {
+                return 3;
+            }
+        }
         
         public class DataMigrationSimpleBuilder : DataMigrationImpl {
             public override Feature Feature {
@@ -433,5 +459,22 @@ Features:
             _dataMigrationManager.Update("Feature1");
         }
 
+        [Test]
+        public void DataMigrationShouldBeTransactional() {
+            Init(new[] { typeof(DataMigrationTransactional) });
+
+            _folders.Manifests.Add("Module1", @"
+Name: Module1
+Version: 0.1
+OrchardVersion: 1
+Features:
+    Feature1: 
+        Description: Feature
+");
+
+            _dataMigrationManager.Update("Feature1");
+            Assert.That(_repository.Table.Count(), Is.EqualTo(0));
+            _dataMigrationManager.Update("Feature1");
+        }
     }
 }

src/Orchard/Data/Migration/DataMigrationManager.cs

@@ -20,18 +20,21 @@ namespace Orchard.Data.Migration {
         private readonly IExtensionManager _extensionManager;
         private readonly IDataMigrationInterpreter _interpreter;
         private readonly IContentDefinitionManager _contentDefinitionManager;
+        private readonly ITransactionManager _transactionManager;
 
         public DataMigrationManager(
             IEnumerable<IDataMigration> dataMigrations, 
             IRepository<DataMigrationRecord> dataMigrationRepository,
             IExtensionManager extensionManager,
             IDataMigrationInterpreter interpreter,
-            IContentDefinitionManager contentDefinitionManager) {
+            IContentDefinitionManager contentDefinitionManager,
+            ITransactionManager transactionManager) {
             _dataMigrations = dataMigrations;
             _dataMigrationRepository = dataMigrationRepository;
             _extensionManager = extensionManager;
             _interpreter = interpreter;
             _contentDefinitionManager = contentDefinitionManager;
+            _transactionManager = transactionManager;
 
             Logger = NullLogger.Instance;
         }
@@ -76,7 +79,7 @@ namespace Orchard.Data.Migration {
 
             // apply update methods to each migration class for the module
             foreach ( var migration in migrations ) {
-                // copy the objet for the Linq query
+                // copy the object for the Linq query
                 var tempMigration = migration;
                 
                 // get current version for this migration
@@ -87,39 +90,49 @@ namespace Orchard.Data.Migration {
                     current = dataMigrationRecord.Version.Value;
                 }
 
-                // do we need to call Create() ?
-                if(current == 0) {
-                    // try to resolve a Create method
+                try {
+                    _transactionManager.RequireNew();
 
-                    var createMethod = GetCreateMethod(migration);
-                    if(createMethod != null) {
-                        current = (int)createMethod.Invoke(migration, new object[0]);
+                    // do we need to call Create() ?
+                    if (current == 0) {
+                        // try to resolve a Create method
+
+                        var createMethod = GetCreateMethod(migration);
+                        if (createMethod != null) {
+                            current = (int) createMethod.Invoke(migration, new object[0]);
+                        }
                     }
-                }
 
-                var lookupTable = CreateUpgradeLookupTable(migration);
+                    var lookupTable = CreateUpgradeLookupTable(migration);
 
-                while(lookupTable.ContainsKey(current)) {
-                    try {
-                        Logger.Information("Applying migration for {0} from version {1}", feature, current);
-                        current = (int)lookupTable[current].Invoke(migration, new object[0]);
+                    while (lookupTable.ContainsKey(current)) {
+                        try {
+                            Logger.Information("Applying migration for {0} from version {1}", feature, current);
+                            current = (int) lookupTable[current].Invoke(migration, new object[0]);
+                        }
+                        catch (Exception ex) {
+                            Logger.Error(ex, "An unexpected error orccured while applying migration on {0} from version {1}", feature, current);
+                            throw;
+                        }
                     }
-                    catch (Exception ex) {
-                        Logger.Error(ex, "An unexpected error orccured while applying migration on {0} from version {1}", feature, current);
-                        throw;
+
+                    // if current is 0, it means no upgrade/create method was found or succeeded 
+                    if (current == 0) {
+                        continue;
                     }
+                    if (dataMigrationRecord == null) {
+                        _dataMigrationRepository.Create(new DataMigrationRecord { Version = current, DataMigrationClass = migration.GetType().FullName });
+                    }
+                    else {
+                        dataMigrationRecord.Version = current;
+                    }
+
+                    _transactionManager.RequireNew();
+                }
+                catch (Exception e) {
+                    _transactionManager.Cancel();
                 }
 
-                // if current is 0, it means no upgrade/create method was found or succeeded 
-                if (current == 0) {
-                    continue;
-                }
-                if (dataMigrationRecord == null) {
-                    _dataMigrationRepository.Create(new DataMigrationRecord {Version = current, DataMigrationClass = migration.GetType().FullName});
-                }
-                else {
-                    dataMigrationRecord.Version = current;
-                }
             }
         }
 

src/Orchard/Data/SessionLocator.cs

@@ -36,7 +36,7 @@ namespace Orchard.Data {
             return _session;
         }
 
-        void ITransactionManager.Demand() {
+        public void Demand() {
             EnsureSession();
 
             if (_transaction == null) {
@@ -45,11 +45,11 @@ namespace Orchard.Data {
             }
         }
 
-        void ITransactionManager.RequireNew() {
-            ((ITransactionManager)this).RequireNew(IsolationLevel.ReadCommitted);
+        public void RequireNew() {
+            RequireNew(IsolationLevel.ReadCommitted);
         }
 
-        void ITransactionManager.RequireNew(IsolationLevel level) {
+        public void RequireNew(IsolationLevel level) {
             EnsureSession();
 
             if (_cancelled) {
@@ -67,12 +67,12 @@ namespace Orchard.Data {
             _transaction = _session.BeginTransaction(level);
         }
 
-        void ITransactionManager.Cancel() {
+        public void Cancel() {
             Logger.Debug("Transaction cancelled flag set");
             _cancelled = true;
         }
 
-        void IDisposable.Dispose() {
+        public void Dispose() {
             if (_transaction != null) {
                 try {
                     if (!_cancelled) {