diff --git a/src/Orchard.Tests/ContentManagement/DefaultContentManagerTests.cs b/src/Orchard.Tests/ContentManagement/DefaultContentManagerTests.cs index f7d9fe4d9..90461827c 100644 --- a/src/Orchard.Tests/ContentManagement/DefaultContentManagerTests.cs +++ b/src/Orchard.Tests/ContentManagement/DefaultContentManagerTests.cs @@ -89,8 +89,10 @@ namespace Orchard.Tests.ContentManagement { _manager = _container.Resolve(); } - 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; + } } } diff --git a/src/Orchard.Tests/DataMigration/DataMigrationTests.cs b/src/Orchard.Tests/DataMigration/DataMigrationTests.cs index af20fa4c7..ede3adc5c 100644 --- a/src/Orchard.Tests/DataMigration/DataMigrationTests.cs +++ b/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().As(); builder.RegisterType().As(); _session = _sessionFactory.OpenSession(); - builder.RegisterInstance(new DefaultContentManagerTests.TestSessionLocator(_session)).As(); + builder.RegisterInstance(new DefaultContentManagerTests.TestSessionLocator(_session)).As().As(); foreach(var type in dataMigrations) { builder.RegisterType(type).As(); } @@ -79,7 +80,7 @@ namespace Orchard.Tests.DataMigration { _container.Resolve(); _dataMigrationManager = _container.Resolve(); _repository = _container.Resolve>(); - + _transactionManager = _container.Resolve(); 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"); + } } } \ No newline at end of file diff --git a/src/Orchard/Data/Migration/DataMigrationManager.cs b/src/Orchard/Data/Migration/DataMigrationManager.cs index a850594a8..2c5ffd7c9 100644 --- a/src/Orchard/Data/Migration/DataMigrationManager.cs +++ b/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 dataMigrations, IRepository 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; - } } } diff --git a/src/Orchard/Data/SessionLocator.cs b/src/Orchard/Data/SessionLocator.cs index ffb83ab15..185365397 100644 --- a/src/Orchard/Data/SessionLocator.cs +++ b/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) {