diff --git a/src/Umbraco.Core/Migrations/IMigration.cs b/src/Umbraco.Core/Migrations/IMigration.cs
index 53b7874b3a..c929234f77 100644
--- a/src/Umbraco.Core/Migrations/IMigration.cs
+++ b/src/Umbraco.Core/Migrations/IMigration.cs
@@ -7,6 +7,9 @@ namespace Umbraco.Core.Migrations
///
public interface IMigration : IDiscoverable
{
+ ///
+ /// Executes the migration.
+ ///
void Migrate();
}
}
diff --git a/src/Umbraco.Core/Migrations/IMigrationContext.cs b/src/Umbraco.Core/Migrations/IMigrationContext.cs
index 4db1b07b63..80ba78b6de 100644
--- a/src/Umbraco.Core/Migrations/IMigrationContext.cs
+++ b/src/Umbraco.Core/Migrations/IMigrationContext.cs
@@ -24,8 +24,13 @@ namespace Umbraco.Core.Migrations
ISqlContext SqlContext { get; }
///
- /// Gets the expression index.
+ /// Gets or sets the expression index.
///
int Index { get; set; }
+
+ ///
+ /// Gets or sets a value indicating whether an expression is being built.
+ ///
+ bool BuildingExpression { get; set; }
}
}
diff --git a/src/Umbraco.Core/Migrations/IncompleteMigrationExpressionException.cs b/src/Umbraco.Core/Migrations/IncompleteMigrationExpressionException.cs
new file mode 100644
index 0000000000..91d1838d6f
--- /dev/null
+++ b/src/Umbraco.Core/Migrations/IncompleteMigrationExpressionException.cs
@@ -0,0 +1,28 @@
+using System;
+
+namespace Umbraco.Core.Migrations
+{
+ ///
+ /// Represents errors that occurs when a migration exception is not executed.
+ ///
+ ///
+ /// Migration expression such as Alter.Table(...).Do() *must* end with Do() else they are
+ /// not executed. When a non-executed expression is detected, an IncompleteMigrationExpressionException
+ /// is thrown.
+ ///
+ public class IncompleteMigrationExpressionException : Exception
+ {
+ ///
+ /// Initializes a new instance of the class.
+ ///
+ public IncompleteMigrationExpressionException()
+ { }
+
+ ///
+ /// Initializes a new instance of the class with a message.
+ ///
+ public IncompleteMigrationExpressionException(string message)
+ : base(message)
+ { }
+ }
+}
diff --git a/src/Umbraco.Core/Migrations/MigrationBase.cs b/src/Umbraco.Core/Migrations/MigrationBase.cs
index 9fbee0ed92..58edcae80a 100644
--- a/src/Umbraco.Core/Migrations/MigrationBase.cs
+++ b/src/Umbraco.Core/Migrations/MigrationBase.cs
@@ -62,42 +62,65 @@ namespace Umbraco.Core.Migrations
///
protected Sql Sql(string sql, params object[] args) => Context.SqlContext.Sql(sql, args);
- ///
+ ///
+ /// Executes the migration.
+ ///
public abstract void Migrate();
+ ///
+ void IMigration.Migrate()
+ {
+ Migrate();
+
+ // ensure there is no building expression
+ // ie we did not forget to .Do() an expression
+ if (Context.BuildingExpression)
+ throw new IncompleteMigrationExpressionException("The migration has run, but leaves an expression that has not run.");
+ }
+
+ // ensures we are not already building,
+ // ie we did not forget to .Do() an expression
+ private T BeginBuild(T builder)
+ {
+ if (Context.BuildingExpression)
+ throw new IncompleteMigrationExpressionException("Cannot create a new expression: the previous expression has not run.");
+ Context.BuildingExpression = true;
+ return builder;
+ }
+
///
/// Builds an Alter expression.
///
- public IAlterBuilder Alter => new AlterBuilder(Context);
+ public IAlterBuilder Alter => BeginBuild(new AlterBuilder(Context));
///
/// Builds a Create expression.
///
- public ICreateBuilder Create => new CreateBuilder(Context);
+ public ICreateBuilder Create => BeginBuild(new CreateBuilder(Context));
///
/// Builds a Delete expression.
///
- public IDeleteBuilder Delete => new DeleteBuilder(Context);
+ public IDeleteBuilder Delete => BeginBuild(new DeleteBuilder(Context));
///
/// Builds an Execute expression.
///
- public IExecuteBuilder Execute => new ExecuteBuilder(Context);
+ public IExecuteBuilder Execute => BeginBuild(new ExecuteBuilder(Context));
///
/// Builds an Insert expression.
///
- public IInsertBuilder Insert => new InsertBuilder(Context);
+ public IInsertBuilder Insert => BeginBuild(new InsertBuilder(Context));
///
/// Builds a Rename expression.
///
- public IRenameBuilder Rename => new RenameBuilder(Context);
+ public IRenameBuilder Rename => BeginBuild(new RenameBuilder(Context));
///
/// Builds an Update expression.
///
- public IUpdateBuilder Update => new UpdateBuilder(Context);
+ public IUpdateBuilder Update => BeginBuild(new UpdateBuilder(Context));
}
}
diff --git a/src/Umbraco.Core/Migrations/MigrationContext.cs b/src/Umbraco.Core/Migrations/MigrationContext.cs
index d0802c813d..da454fab03 100644
--- a/src/Umbraco.Core/Migrations/MigrationContext.cs
+++ b/src/Umbraco.Core/Migrations/MigrationContext.cs
@@ -4,20 +4,33 @@ using Umbraco.Core.Persistence;
namespace Umbraco.Core.Migrations
{
+ ///
+ /// Represents a migration context.
+ ///
internal class MigrationContext : IMigrationContext
{
+ ///
+ /// Initializes a new instance of the class.
+ ///
public MigrationContext(IUmbracoDatabase database, ILogger logger)
{
Database = database ?? throw new ArgumentNullException(nameof(database));
Logger = logger ?? throw new ArgumentNullException(nameof(logger));
}
+ ///
public ILogger Logger { get; }
+ ///
public IUmbracoDatabase Database { get; }
+ ///
public ISqlContext SqlContext => Database.SqlContext;
+ ///
public int Index { get; set; }
+
+ ///
+ public bool BuildingExpression { get; set; }
}
}
diff --git a/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs b/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs
index 4c5a1f1aa7..6ac92a07aa 100644
--- a/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs
+++ b/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs
@@ -50,6 +50,7 @@ namespace Umbraco.Core.Migrations
if (_executed)
throw new InvalidOperationException("This expression has already been executed.");
_executed = true;
+ Context.BuildingExpression = false;
var sql = GetSql();
diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs
index fb0f18700c..1edd54a510 100644
--- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs
+++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs
@@ -119,7 +119,7 @@ namespace Umbraco.Core.Migrations.Upgrade
Chain("{517CE9EA-36D7-472A-BF4B-A0D6FB1B8F89}"); // from 7.12.0
Chain("{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}"); // from 7.12.0
//Chain("{2C87AA47-D1BC-4ECB-8A73-2D8D1046C27F}"); // stephan added that one = merge conflict, remove
-
+
Chain("{8B14CEBD-EE47-4AAD-A841-93551D917F11}"); // add andy's after others, with a new target state
From("{CF51B39B-9B9A-4740-BB7C-EAF606A7BFBF}") // and provide a path out of andy's
.CopyChain("{39E5B1F7-A50B-437E-B768-1723AEC45B65}", "{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}", "{8B14CEBD-EE47-4AAD-A841-93551D917F11}"); // to next
@@ -141,6 +141,7 @@ namespace Umbraco.Core.Migrations.Upgrade
Chain("{8804D8E8-FE62-4E3A-B8A2-C047C2118C38}");
Chain("{23275462-446E-44C7-8C2C-3B8C1127B07D}");
Chain("{6B251841-3069-4AD5-8AE9-861F9523E8DA}");
+ Chain("{EE429F1B-9B26-43CA-89F8-A86017C809A3}");
//FINAL
diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigration.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigration.cs
index 89cb7e74ac..5dc5e0b6fe 100644
--- a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigration.cs
+++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigration.cs
@@ -18,7 +18,9 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0
// kill unused parentId column
Delete.ForeignKey("FK_cmsTags_cmsTags").OnTable(Constants.DatabaseSchema.Tables.Tag).Do();
- Delete.Column("ParentId").FromTable(Constants.DatabaseSchema.Tables.Tag);
+ Delete.Column("ParentId").FromTable(Constants.DatabaseSchema.Tables.Tag).Do();
}
}
+
+ // fixes TagsMigration that... originally failed to properly drop the ParentId column
}
diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigrationFix.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigrationFix.cs
new file mode 100644
index 0000000000..4ee95c9f58
--- /dev/null
+++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TagsMigrationFix.cs
@@ -0,0 +1,16 @@
+namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0
+{
+ public class TagsMigrationFix : MigrationBase
+ {
+ public TagsMigrationFix(IMigrationContext context)
+ : base(context)
+ { }
+
+ public override void Migrate()
+ {
+ // kill unused parentId column, if it still exists
+ if (ColumnExists(Constants.DatabaseSchema.Tables.Tag, "ParentId"))
+ Delete.Column("ParentId").FromTable(Constants.DatabaseSchema.Tables.Tag).Do();
+ }
+ }
+}
\ No newline at end of file
diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj
index 6762dc508c..2bfa52a80a 100644
--- a/src/Umbraco.Core/Umbraco.Core.csproj
+++ b/src/Umbraco.Core/Umbraco.Core.csproj
@@ -335,6 +335,7 @@
+
@@ -367,6 +368,7 @@
+
diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs
index b408d351d7..d06cf2244e 100644
--- a/src/Umbraco.Tests/Migrations/MigrationTests.cs
+++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs
@@ -1,16 +1,19 @@
using System;
using System.Data;
+using Moq;
+using NUnit.Framework;
using Semver;
using Umbraco.Core.Events;
-using Umbraco.Core.Logging;
using Umbraco.Core.Migrations;
using Umbraco.Core.Migrations.Upgrade;
using Umbraco.Core.Persistence;
using Umbraco.Core.Scoping;
using Umbraco.Core.Services;
+using ILogger = Umbraco.Core.Logging.ILogger;
namespace Umbraco.Tests.Migrations
{
+ [TestFixture]
public class MigrationTests
{
public class TestUpgrader : Upgrader
@@ -67,5 +70,68 @@ namespace Umbraco.Tests.Migrations
public IScopeContext Context { get; set; }
public ISqlContext SqlContext { get; set; }
}
+
+ [Test]
+ public void RunGoodMigration()
+ {
+ var migrationContext = new MigrationContext(Mock.Of(), Mock.Of());
+ IMigration migration = new GoodMigration(migrationContext);
+ migration.Migrate();
+ }
+
+ [Test]
+ public void DetectBadMigration1()
+ {
+ var migrationContext = new MigrationContext(Mock.Of(), Mock.Of());
+ IMigration migration = new BadMigration1(migrationContext);
+ Assert.Throws(() => migration.Migrate());
+ }
+
+ [Test]
+ public void DetectBadMigration2()
+ {
+ var migrationContext = new MigrationContext(Mock.Of(), Mock.Of());
+ IMigration migration = new BadMigration2(migrationContext);
+ Assert.Throws(() => migration.Migrate());
+ }
+
+ public class GoodMigration : MigrationBase
+ {
+ public GoodMigration(IMigrationContext context)
+ : base(context)
+ { }
+
+ public override void Migrate()
+ {
+ Execute.Sql("").Do();
+ }
+ }
+
+ public class BadMigration1 : MigrationBase
+ {
+ public BadMigration1(IMigrationContext context)
+ : base(context)
+ { }
+
+ public override void Migrate()
+ {
+ Alter.Table("foo"); // stop here, don't Do it
+ }
+ }
+
+ public class BadMigration2 : MigrationBase
+ {
+ public BadMigration2(IMigrationContext context)
+ : base(context)
+ { }
+
+ public override void Migrate()
+ {
+ Alter.Table("foo"); // stop here, don't Do it
+
+ // and try to start another one
+ Alter.Table("bar");
+ }
+ }
}
}