Skip to content

Commit

Permalink
Reworking the merge conflict resolution between 1.10.x and dev for th…
Browse files Browse the repository at this point in the history 
…e Core.Common and Projections migrations
  • Loading branch information
@BenedekFarkas
BenedekFarkas committed Mar 7, 2024
1 parent 81b9a60 commit fcf960f
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 142 deletions.
150 changes: 61 additions & 89 deletions src/Orchard.Web/Core/Common/Migrations.cs
View file
Original file line number Diff line number Diff line change
@@ -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<IdentityPartRecord> _identityPartRepository;
private readonly ISessionFactoryHolder _sessionFactoryHolder;
private readonly ShellSettings _shellSettings;

private HashSet<string> _existingIndexNames = new HashSet<string>();


public Migrations(
IRepository<IdentityPartRecord> identityPartRepository,
ISessionFactoryHolder sessionFactoryHolder,
ShellSettings shellSettings) {
/// <summary>
/// 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.
/// </summary>
public bool IsUpgradingFromOrchard_1_10_x_Version_7 { get; set; }
public bool IsUpgradingFromOrchard_1_10_x_Version_8 { get; set; }

public Migrations(IRepository<IdentityPartRecord> identityPartRepository) {
_identityPartRepository = identityPartRepository;
_sessionFactoryHolder = sessionFactoryHolder;
_shellSettings = shellSettings;
}


Expand Down Expand Up @@ -167,31 +164,67 @@ 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;
}

// This change was originally UpdateFrom7 on 1.10.x and UpdateFrom6 on dev.
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)));
Expand All @@ -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<DataRow>();

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}");
}
}
}
}
80 changes: 27 additions & 53 deletions src/Orchard.Web/Modules/Orchard.Projections/Migrations.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Linq;
using Orchard.ContentManagement.MetaData;
Expand All @@ -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;

Expand All @@ -17,22 +15,23 @@ public class Migrations : DataMigrationImpl {
private readonly IRepository<MemberBindingRecord> _memberBindingRepository;
private readonly IRepository<LayoutRecord> _layoutRepository;
private readonly IRepository<PropertyRecord> _propertyRecordRepository;
private readonly ISessionFactoryHolder _sessionFactoryHolder;
private readonly ShellSettings _shellSettings;

private HashSet<string> _existingColumnNames = new HashSet<string>();
/// <summary>
/// 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.
/// </summary>
public bool IsUpgradingFromOrchard_1_10_x_Version_6 { get; set; }

public Migrations(
IRepository<MemberBindingRecord> memberBindingRepository,
IRepository<LayoutRecord> layoutRepository,
IRepository<PropertyRecord> propertyRecordRepository,
ISessionFactoryHolder sessionFactoryHolder,
ShellSettings shellSettings) {
IRepository<PropertyRecord> propertyRecordRepository) {
_memberBindingRepository = memberBindingRepository;
_layoutRepository = layoutRepository;
_propertyRecordRepository = propertyRecordRepository;
_sessionFactoryHolder = sessionFactoryHolder;
_shellSettings = shellSettings;

T = NullLocalizer.Instance;
}
Expand Down Expand Up @@ -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<string>("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<string>("RewriteOutputCondition", c => c.Unlimited())
);
Expand All @@ -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<string>("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<DataRow>();

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}");
}
}
}

0 comments on commit fcf960f

Please sign in to comment.