From fcf960f7e37ecebeb40ddb64c747aaa29272a109 Mon Sep 17 00:00:00 2001 From: Benedek Farkas Date: Thu, 7 Mar 2024 16:50:58 +0100 Subject: [PATCH] Reworking the merge conflict resolution between 1.10.x and dev for the Core.Common and Projections migrations --- src/Orchard.Web/Core/Common/Migrations.cs | 150 +++++++----------- .../Modules/Orchard.Projections/Migrations.cs | 80 ++++------ 2 files changed, 88 insertions(+), 142 deletions(-) diff --git a/src/Orchard.Web/Core/Common/Migrations.cs b/src/Orchard.Web/Core/Common/Migrations.cs index 2c2d9e1dc24..65565f04531 100644 --- a/src/Orchard.Web/Core/Common/Migrations.cs +++ b/src/Orchard.Web/Core/Common/Migrations.cs @@ -1,30 +1,27 @@ using System; -using System.Collections.Generic; -using System.Data; using System.Linq; using Orchard.ContentManagement.MetaData; using Orchard.Core.Common.Models; using Orchard.Core.Contents.Extensions; using Orchard.Data; using Orchard.Data.Migration; -using Orchard.Environment.Configuration; namespace Orchard.Core.Common { public class Migrations : DataMigrationImpl { private readonly IRepository _identityPartRepository; - private readonly ISessionFactoryHolder _sessionFactoryHolder; - private readonly ShellSettings _shellSettings; - private HashSet _existingIndexNames = new HashSet(); - - - public Migrations( - IRepository identityPartRepository, - ISessionFactoryHolder sessionFactoryHolder, - ShellSettings shellSettings) { + /// + /// When upgrading from "1.10.x" branch code committed after 1.10.3 to "dev" branch code or 1.11, merge + /// conflicts between "1.10.x" and "dev" caused by running the same migration steps in a different order need to + /// be resolved by instructing this migration to decide which steps need to be executed. If you're upgrading + /// under these conditions and your pre-upgrade migration version is 7 or 8, use HostComponents.config to + /// override one of these properties to true. + /// + public bool IsUpgradingFromOrchard_1_10_x_Version_7 { get; set; } + public bool IsUpgradingFromOrchard_1_10_x_Version_8 { get; set; } + + public Migrations(IRepository identityPartRepository) { _identityPartRepository = identityPartRepository; - _sessionFactoryHolder = sessionFactoryHolder; - _shellSettings = shellSettings; } @@ -167,22 +164,60 @@ public int UpdateFrom5() { return 6; } - // This step's logic is executed together with the original logic from UpdateFrom7 and UpdateFrom8 and in - // UpdateFrom8 to make sure that an upgrade is possible from version 6 of both 1.10.x and dev, which both - // included these steps, but in a different order. - public int UpdateFrom6() => 7; - - // See UpdateFrom6 for explanation. - public int UpdateFrom7() => 8; - - // See UpdateFrom6 for explanation. - public int UpdateFrom8() { + public int UpdateFrom6() { AddIndexForIdentityPartRecordIdentifier(); - AddIndexesForCommonPartOwner(); + return 7; + } + public int UpdateFrom7() { AddIndexForCommonPartRecordContainerId(); + return 8; + } + + public int UpdateFrom8() { + if (IsUpgradingFromOrchard_1_10_x_Version_7) { + AddIndexForIdentityPartRecordIdentifier(); + } + else if (IsUpgradingFromOrchard_1_10_x_Version_8) { + AddIndexForCommonPartRecordContainerId(); + } + else { + // This change was originally UpdateFrom6 on 1.10.x and UpdateFrom8 on dev. + + // Studying SQL Server query execution plans we noticed that when the system tries to find content items for + // requests such as "The items of type TTT owned by me, ordered from the most recent" the existing indexes + // are not used. SQL Server does an index scan on the Primary key for CommonPartRecord. This may lead to + // annoying deadlocks when there are two concurrent transactions that are doing both this kind of query as + // well as an update (or insert) in the CommonPartRecord. Tests show that this can be easily fixed by adding + // a non-clustered index with these keys: OwnerId, {one of PublishedUTC, ModifiedUTC, CreatedUTC}. That + // means we need three indexes (one for each DateTime) to support ordering on either of them. + + // The queries we analyzed look like (in pseudo sql) + // SELECT TOP (N) * + // FROM + // ContentItemVersionRecord this_ + // inner join ContentItemRecord contentite1_ on this_.ContentItemRecord_id=contentite1_.Id + // inner join CommonPartRecord commonpart2_ on contentite1_.Id=commonpart2.Id + // left outer join ContentTypeRecord contenttyp6_ on contentite1_.ContentType_id=contenttyp6_.Id + // WHERE + // contentite1.ContentType_id = {TTT} + // and commonpart2_.OwnerId = {userid} + // and this_.Published = 1 + // ORDER BY + // commonpart2_PublishedUtc desc + var createdUtcIndexName = $"IDX_{nameof(CommonPartRecord)}_OwnedBy_ByCreation"; + var modifiedUtcIndexName = $"IDX_{nameof(CommonPartRecord)}_OwnedBy_ByModification"; + var publishedUtcIndexName = $"IDX_{nameof(CommonPartRecord)}_OwnedBy_ByPublication"; + + SchemaBuilder.AlterTable(nameof(CommonPartRecord), table => { + table.CreateIndex(createdUtcIndexName, nameof(CommonPartRecord.OwnerId), nameof(CommonPartRecord.CreatedUtc)); + table.CreateIndex(modifiedUtcIndexName, nameof(CommonPartRecord.OwnerId), nameof(CommonPartRecord.ModifiedUtc)); + table.CreateIndex(publishedUtcIndexName, nameof(CommonPartRecord.OwnerId), nameof(CommonPartRecord.PublishedUtc)); + }); + } + return 9; } @@ -190,8 +225,6 @@ public int UpdateFrom8() { private void AddIndexForIdentityPartRecordIdentifier() { var indexName = $"IDX_{nameof(IdentityPartRecord)}_{nameof(IdentityPartRecord.Identifier)}"; - if (IndexExists(nameof(IdentityPartRecord), indexName)) return; - SchemaBuilder.AlterTable(nameof(IdentityPartRecord), table => table.CreateIndex( indexName, nameof(IdentityPartRecord.Identifier))); @@ -201,69 +234,8 @@ private void AddIndexForIdentityPartRecordIdentifier() { private void AddIndexForCommonPartRecordContainerId() { var indexName = $"IDX_{nameof(CommonPartRecord)}_Container_id"; - if (IndexExists(nameof(CommonPartRecord), indexName)) return; - // Container_Id is used in several queries like a foreign key. SchemaBuilder.AlterTable(nameof(CommonPartRecord), table => table.CreateIndex(indexName, "Container_id")); } - - // This change was originally UpdateFrom6 on 1.10.x and UpdateFrom8 on dev. - private void AddIndexesForCommonPartOwner() { - // Studying SQL Server query execution plans we noticed that when the system tries to find content items for - // requests such as "The items of type TTT owned by me, ordered from the most recent" the existing indexes - // are not used. SQL Server does an index scan on the Primary key for CommonPartRecord. This may lead to - // annoying deadlocks when there are two concurrent transactions that are doing both this kind of query as - // well as an update (or insert) in the CommonPartRecord. Tests show that this can be easily fixed by adding - // a non-clustered index with these keys: OwnerId, {one of PublishedUTC, ModifiedUTC, CreatedUTC}. That - // means we need three indexes (one for each DateTime) to support ordering on either of them. - - // The queries we analyzed look like (in pseudo sql) - // SELECT TOP (N) * - // FROM - // ContentItemVersionRecord this_ - // inner join ContentItemRecord contentite1_ on this_.ContentItemRecord_id=contentite1_.Id - // inner join CommonPartRecord commonpart2_ on contentite1_.Id=commonpart2.Id - // left outer join ContentTypeRecord contenttyp6_ on contentite1_.ContentType_id=contenttyp6_.Id - // WHERE - // contentite1.ContentType_id = {TTT} - // and commonpart2_.OwnerId = {userid} - // and this_.Published = 1 - // ORDER BY - // commonpart2_PublishedUtc desc - var createdUtcIndexName = $"IDX_{nameof(CommonPartRecord)}_OwnedBy_ByCreation"; - var modifiedUtcIndexName = $"IDX_{nameof(CommonPartRecord)}_OwnedBy_ByModification"; - var publishedUtcIndexName = $"IDX_{nameof(CommonPartRecord)}_OwnedBy_ByPublication"; - - if (IndexExists(nameof(CommonPartRecord), createdUtcIndexName)) return; - - SchemaBuilder.AlterTable(nameof(CommonPartRecord), table => { - table.CreateIndex(createdUtcIndexName, nameof(CommonPartRecord.OwnerId), nameof(CommonPartRecord.CreatedUtc)); - table.CreateIndex(modifiedUtcIndexName, nameof(CommonPartRecord.OwnerId), nameof(CommonPartRecord.ModifiedUtc)); - table.CreateIndex(publishedUtcIndexName, nameof(CommonPartRecord.OwnerId), nameof(CommonPartRecord.PublishedUtc)); - }); - } - - private bool IndexExists(string tableName, string indexName) { - var tenantTablesPrefix = string.IsNullOrEmpty(_shellSettings.DataTablePrefix) - ? string.Empty : $"{_shellSettings.DataTablePrefix}_"; - - if (!_existingIndexNames.Any()) { - // Database-agnostic way of checking the existence of an index. - using (var session = _sessionFactoryHolder.GetSessionFactory().OpenSession()) { - var connection = session.Connection ?? throw new InvalidOperationException( - "The database connection object should derive from DbConnection to check if an index exists."); - - var indexes = connection.GetSchema("Indexes").Rows.Cast(); - - if (!string.IsNullOrEmpty(tenantTablesPrefix)) { - indexes = indexes.Where(row => row["TABLE_NAME"].ToString().StartsWith(tenantTablesPrefix)); - } - - _existingIndexNames = indexes.Select(row => $"{row["TABLE_NAME"]}.{row["INDEX_NAME"]}").ToHashSet(); - } - } - - return _existingIndexNames.Contains($"{SchemaBuilder.TableDbName(tableName)}.{tenantTablesPrefix}{indexName}"); - } } -} \ No newline at end of file +} diff --git a/src/Orchard.Web/Modules/Orchard.Projections/Migrations.cs b/src/Orchard.Web/Modules/Orchard.Projections/Migrations.cs index a14f315e0f7..cbf33106f9a 100644 --- a/src/Orchard.Web/Modules/Orchard.Projections/Migrations.cs +++ b/src/Orchard.Web/Modules/Orchard.Projections/Migrations.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Data; using System.Linq; using Orchard.ContentManagement.MetaData; @@ -8,7 +7,6 @@ using Orchard.Core.Title.Models; using Orchard.Data; using Orchard.Data.Migration; -using Orchard.Environment.Configuration; using Orchard.Localization; using Orchard.Projections.Models; @@ -17,22 +15,23 @@ public class Migrations : DataMigrationImpl { private readonly IRepository _memberBindingRepository; private readonly IRepository _layoutRepository; private readonly IRepository _propertyRecordRepository; - private readonly ISessionFactoryHolder _sessionFactoryHolder; - private readonly ShellSettings _shellSettings; - private HashSet _existingColumnNames = new HashSet(); + /// + /// When upgrading from "1.10.x" branch code committed after 1.10.3 to "dev" branch code or 1.11, merge + /// conflicts between "1.10.x" and "dev" caused by running the same migration steps in a different order need to + /// be resolved by instructing this migration to decide which steps need to be executed. If you're upgrading + /// under these conditions and your pre-upgrade migration version is 6, use HostComponents.config to override + /// this property to true. + /// + public bool IsUpgradingFromOrchard_1_10_x_Version_6 { get; set; } public Migrations( IRepository memberBindingRepository, IRepository layoutRepository, - IRepository propertyRecordRepository, - ISessionFactoryHolder sessionFactoryHolder, - ShellSettings shellSettings) { + IRepository propertyRecordRepository) { _memberBindingRepository = memberBindingRepository; _layoutRepository = layoutRepository; _propertyRecordRepository = propertyRecordRepository; - _sessionFactoryHolder = sessionFactoryHolder; - _shellSettings = shellSettings; T = NullLocalizer.Instance; } @@ -383,24 +382,32 @@ public int UpdateFrom4() { return 5; } - // This step's logic is now executed in UpdateFrom6 as MigratePropertyRecordToRewriteOutputCondition to make - // sure that it's executed even when upgrading from version 6 of 1.10.x, which (as opposed to dev) executed the - // changes that make up AddLayoutRecordGuid in UpdateFrom6. - public int UpdateFrom5() => 6; + public int UpdateFrom5() { + MigratePropertyRecordToRewriteOutputCondition(); - // See UpdateFrom5 for explanation. - public int UpdateFrom6() { - AddLayoutRecordGuid(); + return 6; + } - MigratePropertyRecordToRewriteOutputCondition(); + public int UpdateFrom6() { + if (IsUpgradingFromOrchard_1_10_x_Version_6) { + MigratePropertyRecordToRewriteOutputCondition(); + } + else { + // This change was originally UpdateFrom5 on 1.10.x and UpdateFrom6 on dev. + SchemaBuilder.AlterTable("LayoutRecord", table => + table.AddColumn("GUIdentifier", column => column.WithLength(68))); + + var layoutRecords = _layoutRepository.Table.Where(l => l.GUIdentifier == null || l.GUIdentifier == "").ToList(); + foreach (var layout in layoutRecords) { + layout.GUIdentifier = Guid.NewGuid().ToString(); + } + } return 7; } // This change was originally in UpdateFrom5 on dev, but didn't exist on 1.10.x. private void MigratePropertyRecordToRewriteOutputCondition() { - if (ColumnExists("PropertyRecord", "RewriteOutputCondition")) return; - SchemaBuilder.AlterTable("PropertyRecord", table => table .AddColumn("RewriteOutputCondition", c => c.Unlimited()) ); @@ -411,38 +418,5 @@ private void MigratePropertyRecordToRewriteOutputCondition() { if (property.RewriteOutput) property.RewriteOutputCondition = "true"; #pragma warning restore CS0618 // Type or member is obsolete } - - // This change was originally UpdateFrom5 on 1.10.x and UpdateFrom6 on dev. - private void AddLayoutRecordGuid() { - if (ColumnExists("LayoutRecord", "GUIdentifier")) return; - - SchemaBuilder.AlterTable("LayoutRecord", table => - table.AddColumn("GUIdentifier", column => column.WithLength(68))); - - var layoutRecords = _layoutRepository.Table.Where(l => l.GUIdentifier == null || l.GUIdentifier == "").ToList(); - foreach (var layout in layoutRecords) { - layout.GUIdentifier = Guid.NewGuid().ToString(); - } - } - - private bool ColumnExists(string tableName, string columnName) { - if (!_existingColumnNames.Any()) { - // Database-agnostic way of checking the existence of a column. - using (var session = _sessionFactoryHolder.GetSessionFactory().OpenSession()) { - var connection = session.Connection ?? throw new InvalidOperationException( - "The database connection object should derive from DbConnection to check if a column exists."); - - var columns = connection.GetSchema("Columns").Rows.Cast(); - - if (!string.IsNullOrEmpty(_shellSettings.DataTablePrefix)) { - columns = columns.Where(row => row["TABLE_NAME"].ToString().StartsWith($"{_shellSettings.DataTablePrefix}_")); - } - - _existingColumnNames = columns.Select(row => $"{row["TABLE_NAME"]}.{row["COLUMN_NAME"]}").ToHashSet(); - } - } - - return _existingColumnNames.Contains($"{SchemaBuilder.TableDbName(tableName)}.{columnName}"); - } } }