From f68cdf43e1071d15c2964a3938e5fb4bfd006fbf Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 2 Jul 2015 17:19:42 +0200 Subject: [PATCH] Fixes: U4-6099 cmsLanguageText.languageId needs to be a foreign key to the umbracoLanguage table --- src/Umbraco.Core/Constants-Conventions.cs | 1 + src/Umbraco.Core/Models/DictionaryItem.cs | 16 ++--- src/Umbraco.Core/Models/IDictionaryItem.cs | 2 +- .../Models/Rdbms/DictionaryDto.cs | 4 +- .../Models/Rdbms/LanguageTextDto.cs | 1 + .../Persistence/DatabaseSchemaHelper.cs | 14 +++-- .../Initial/DatabaseSchemaCreation.cs | 4 +- .../Migrations/MigrationExpressionBase.cs | 27 +++++++++ .../Expressions/DeleteDataExpression.cs | 2 +- .../Expressions/InsertDataExpression.cs | 25 +------- .../Expressions/UpdateDataExpression.cs | 4 +- ...reignKeysForLanguageAndDictionaryTables.cs | 58 +++++++++++++++++++ .../Repositories/DictionaryRepository.cs | 5 +- .../Services/LocalizationService.cs | 5 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Persistence/BaseTableByTableTest.cs | 1 + .../Services/LocalizationServiceTests.cs | 4 +- src/umbraco.cms/businesslogic/Dictionary.cs | 8 +-- 18 files changed, 122 insertions(+), 60 deletions(-) create mode 100644 src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddForeignKeysForLanguageAndDictionaryTables.cs diff --git a/src/Umbraco.Core/Constants-Conventions.cs b/src/Umbraco.Core/Constants-Conventions.cs index c1d5bd35e3..3456ff1cfa 100644 --- a/src/Umbraco.Core/Constants-Conventions.cs +++ b/src/Umbraco.Core/Constants-Conventions.cs @@ -27,6 +27,7 @@ namespace Umbraco.Core /// /// The root id for all top level dictionary items /// + [Obsolete("There is no dictionary root item id anymore, it is simply null")] public const string DictionaryItemRootId = "41c7638d-f529-4bff-853e-59a0c2fb1bde"; } diff --git a/src/Umbraco.Core/Models/DictionaryItem.cs b/src/Umbraco.Core/Models/DictionaryItem.cs index e09c370053..7e59726c80 100644 --- a/src/Umbraco.Core/Models/DictionaryItem.cs +++ b/src/Umbraco.Core/Models/DictionaryItem.cs @@ -14,22 +14,22 @@ namespace Umbraco.Core.Models [DataContract(IsReference = true)] public class DictionaryItem : Entity, IDictionaryItem { - private Guid _parentId; + private Guid? _parentId; private string _itemKey; private IEnumerable _translations; public DictionaryItem(string itemKey) - : this(new Guid(Constants.Conventions.Localization.DictionaryItemRootId), itemKey) + : this(null, itemKey) {} - public DictionaryItem(Guid parentId, string itemKey) + public DictionaryItem(Guid? parentId, string itemKey) { _parentId = parentId; _itemKey = itemKey; _translations = new List(); } - private static readonly PropertyInfo ParentIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ParentId); + private static readonly PropertyInfo ParentIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ParentId); private static readonly PropertyInfo ItemKeySelector = ExpressionHelper.GetPropertyInfo(x => x.ItemKey); private static readonly PropertyInfo TranslationsSelector = ExpressionHelper.GetPropertyInfo>(x => x.Translations); @@ -37,7 +37,7 @@ namespace Umbraco.Core.Models /// Gets or Sets the Parent Id of the Dictionary Item /// [DataMember] - public Guid ParentId + public Guid? ParentId { get { return _parentId; } set @@ -95,11 +95,7 @@ namespace Umbraco.Core.Models { base.AddingEntity(); - Key = Guid.NewGuid(); - - //If ParentId is not set we should default to the root parent id - if(ParentId == Guid.Empty) - _parentId = new Guid(Constants.Conventions.Localization.DictionaryItemRootId); + Key = Guid.NewGuid(); } } diff --git a/src/Umbraco.Core/Models/IDictionaryItem.cs b/src/Umbraco.Core/Models/IDictionaryItem.cs index e10a80e831..ed88acfbec 100644 --- a/src/Umbraco.Core/Models/IDictionaryItem.cs +++ b/src/Umbraco.Core/Models/IDictionaryItem.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Models /// Gets or Sets the Parent Id of the Dictionary Item /// [DataMember] - Guid ParentId { get; set; } + Guid? ParentId { get; set; } /// /// Gets or sets the Key for the Dictionary Item diff --git a/src/Umbraco.Core/Models/Rdbms/DictionaryDto.cs b/src/Umbraco.Core/Models/Rdbms/DictionaryDto.cs index 18fd16b658..712d1937a9 100644 --- a/src/Umbraco.Core/Models/Rdbms/DictionaryDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/DictionaryDto.cs @@ -19,7 +19,9 @@ namespace Umbraco.Core.Models.Rdbms public Guid UniqueId { get; set; } [Column("parent")] - public Guid Parent { get; set; } + [NullSetting(NullSetting = NullSettings.Null)] + [ForeignKey(typeof(DictionaryDto), Column = "id")] + public Guid? Parent { get; set; } [Column("key")] [Length(1000)] diff --git a/src/Umbraco.Core/Models/Rdbms/LanguageTextDto.cs b/src/Umbraco.Core/Models/Rdbms/LanguageTextDto.cs index d062df4eae..87329fbd4c 100644 --- a/src/Umbraco.Core/Models/Rdbms/LanguageTextDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/LanguageTextDto.cs @@ -14,6 +14,7 @@ namespace Umbraco.Core.Models.Rdbms public int PrimaryKey { get; set; } [Column("languageId")] + [ForeignKey(typeof(LanguageDto), Column = "id")] public int LanguageId { get; set; } [Column("UniqueId")] diff --git a/src/Umbraco.Core/Persistence/DatabaseSchemaHelper.cs b/src/Umbraco.Core/Persistence/DatabaseSchemaHelper.cs index ca64775880..55c1fe505f 100644 --- a/src/Umbraco.Core/Persistence/DatabaseSchemaHelper.cs +++ b/src/Umbraco.Core/Persistence/DatabaseSchemaHelper.cs @@ -151,6 +151,13 @@ namespace Umbraco.Core.Persistence _db.Update("SET id = @IdAfter WHERE id = @IdBefore AND userLogin = @Login", new { IdAfter = 0, IdBefore = 1, Login = "admin" }); } + //Loop through index statements and execute sql + foreach (var sql in indexSql) + { + int createdIndex = _db.Execute(new Sql(sql)); + _logger.Info(string.Format("Create Index sql {0}:\n {1}", createdIndex, sql)); + } + //Loop through foreignkey statements and execute sql foreach (var sql in foreignSql) { @@ -158,12 +165,7 @@ namespace Umbraco.Core.Persistence _logger.Info(string.Format("Create Foreign Key sql {0}:\n {1}", createdFk, sql)); } - //Loop through index statements and execute sql - foreach (var sql in indexSql) - { - int createdIndex = _db.Execute(new Sql(sql)); - _logger.Info(string.Format("Create Index sql {0}:\n {1}", createdIndex, sql)); - } + transaction.Complete(); } diff --git a/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs b/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs index ce2a53ea95..a864bf21a4 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs @@ -48,8 +48,8 @@ namespace Umbraco.Core.Persistence.Migrations.Initial {7, typeof (DataTypeDto)}, {8, typeof (DataTypePreValueDto)}, {9, typeof (DictionaryDto)}, - {10, typeof (LanguageTextDto)}, - {11, typeof (LanguageDto)}, + {10, typeof (LanguageDto)}, + {11, typeof (LanguageTextDto)}, {12, typeof (DomainDto)}, {13, typeof (LogDto)}, {14, typeof (MacroDto)}, diff --git a/src/Umbraco.Core/Persistence/Migrations/MigrationExpressionBase.cs b/src/Umbraco.Core/Persistence/Migrations/MigrationExpressionBase.cs index e7535e78d4..7dde31d3e3 100644 --- a/src/Umbraco.Core/Persistence/Migrations/MigrationExpressionBase.cs +++ b/src/Umbraco.Core/Persistence/Migrations/MigrationExpressionBase.cs @@ -53,5 +53,32 @@ namespace Umbraco.Core.Persistence.Migrations /// to ensure they are not executed twice. /// internal string Name { get; set; } + + protected string GetQuotedValue(object val) + { + if (val == null) return "NULL"; + + var type = val.GetType(); + + switch (Type.GetTypeCode(type)) + { + case TypeCode.Boolean: + return ((bool)val) ? "1" : "0"; + case TypeCode.Single: + case TypeCode.Double: + case TypeCode.Decimal: + case TypeCode.SByte: + case TypeCode.Int16: + case TypeCode.Int32: + case TypeCode.Int64: + case TypeCode.Byte: + case TypeCode.UInt16: + case TypeCode.UInt32: + case TypeCode.UInt64: + return val.ToString(); + default: + return SqlSyntaxContext.SqlSyntaxProvider.GetQuotedValue(val.ToString()); + } + } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Migrations/Syntax/Delete/Expressions/DeleteDataExpression.cs b/src/Umbraco.Core/Persistence/Migrations/Syntax/Delete/Expressions/DeleteDataExpression.cs index 2131a7b106..73966f6e93 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Syntax/Delete/Expressions/DeleteDataExpression.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Syntax/Delete/Expressions/DeleteDataExpression.cs @@ -44,7 +44,7 @@ namespace Umbraco.Core.Persistence.Migrations.Syntax.Delete.Expressions whereClauses.Add(string.Format("{0} {1} {2}", SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName(item.Key), item.Value == null ? "IS" : "=", - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedValue(item.Value.ToString()))); + GetQuotedValue(item.Value))); } deleteItems.Add(string.Format(SqlSyntaxContext.SqlSyntaxProvider.DeleteData, diff --git a/src/Umbraco.Core/Persistence/Migrations/Syntax/Insert/Expressions/InsertDataExpression.cs b/src/Umbraco.Core/Persistence/Migrations/Syntax/Insert/Expressions/InsertDataExpression.cs index 330e22bb93..568087733b 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Syntax/Insert/Expressions/InsertDataExpression.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Syntax/Insert/Expressions/InsertDataExpression.cs @@ -60,29 +60,6 @@ namespace Umbraco.Core.Persistence.Migrations.Syntax.Insert.Expressions return string.Join(",", insertItems); } - private string GetQuotedValue(object val) - { - var type = val.GetType(); - - switch (Type.GetTypeCode(type)) - { - case TypeCode.Boolean: - return ((bool) val) ? "1" : "0"; - case TypeCode.Single: - case TypeCode.Double: - case TypeCode.Decimal: - case TypeCode.SByte: - case TypeCode.Int16: - case TypeCode.Int32: - case TypeCode.Int64: - case TypeCode.Byte: - case TypeCode.UInt16: - case TypeCode.UInt32: - case TypeCode.UInt64: - return val.ToString(); - default: - return SqlSyntaxContext.SqlSyntaxProvider.GetQuotedValue(val.ToString()); - } - } + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Migrations/Syntax/Update/Expressions/UpdateDataExpression.cs b/src/Umbraco.Core/Persistence/Migrations/Syntax/Update/Expressions/UpdateDataExpression.cs index a5a3204974..fd470dd7f2 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Syntax/Update/Expressions/UpdateDataExpression.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Syntax/Update/Expressions/UpdateDataExpression.cs @@ -32,7 +32,7 @@ namespace Umbraco.Core.Persistence.Migrations.Syntax.Update.Expressions { updateItems.Add(string.Format("{0} = {1}", SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName(item.Key), - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedValue(item.Value.ToString()))); + GetQuotedValue(item.Value))); } if (IsAllRows) @@ -46,7 +46,7 @@ namespace Umbraco.Core.Persistence.Migrations.Syntax.Update.Expressions whereClauses.Add(string.Format("{0} {1} {2}", SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName(item.Key), item.Value == null ? "IS" : "=", - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedValue(item.Value.ToString()))); + GetQuotedValue(item.Value))); } } return string.Format(SqlSyntaxContext.SqlSyntaxProvider.UpdateData, diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddForeignKeysForLanguageAndDictionaryTables.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddForeignKeysForLanguageAndDictionaryTables.cs new file mode 100644 index 0000000000..1657cacc8b --- /dev/null +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddForeignKeysForLanguageAndDictionaryTables.cs @@ -0,0 +1,58 @@ +using System; +using System.Data; +using System.Linq; +using Umbraco.Core.Configuration; +using Umbraco.Core.Logging; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenThreeZero +{ + [Migration("7.3.0", 14, GlobalSettings.UmbracoMigrationName)] + public class AddForeignKeysForLanguageAndDictionaryTables : MigrationBase + { + public AddForeignKeysForLanguageAndDictionaryTables(ISqlSyntaxProvider sqlSyntax, ILogger logger) + : base(sqlSyntax, logger) + { + } + + public override void Up() + { + var constraints = SqlSyntax.GetConstraintsPerColumn(Context.Database).Distinct().ToArray(); + + //if the FK doesn't exist + if (constraints.Any(x => x.Item1.InvariantEquals("cmsLanguageText") && x.Item2.InvariantEquals("umbracoLanguage") && x.Item3.InvariantEquals("FK_cmsLanguageText_umbracoLanguage_id")) == false) + { + //Somehow, a language text item might end up with a language Id of zero or one that no longer exists + //before we add the foreign key + foreach (var pk in Context.Database.Query( + "SELECT cmsLanguageText.pk FROM cmsLanguageText WHERE cmsLanguageText.languageId NOT IN (SELECT umbracoLanguage.id FROM umbracoLanguage)")) + { + Delete.FromTable("cmsLanguageText").Row(new { pk = pk }); + } + + //now we need to create a foreign key + Create.ForeignKey("FK_cmsLanguageText_umbracoLanguage_id").FromTable("cmsLanguageText").ForeignColumn("languageId") + .ToTable("umbracoLanguage").PrimaryColumn("id").OnDeleteOrUpdate(Rule.None); + + Alter.Table("cmsDictionary").AlterColumn("parent").AsGuid().Nullable(); + + //set the parent to null if it equals the default dictionary item root id + foreach (var pk in Context.Database.Query("SELECT pk FROM cmsDictionary WHERE parent NOT IN (SELECT id FROM cmsDictionary)")) + { + Update.Table("cmsDictionary").Set(new { parent = (Guid?)null }).Where(new { pk = pk }); + } + + Create.ForeignKey("FK_cmsDictionary_cmsDictionary_id").FromTable("cmsDictionary").ForeignColumn("parent") + .ToTable("cmsDictionary").PrimaryColumn("id").OnDeleteOrUpdate(Rule.None); + } + + + + } + + public override void Down() + { + throw new System.NotImplementedException(); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs index cf3193943b..1e84814d1b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs @@ -103,12 +103,9 @@ namespace Umbraco.Core.Persistence.Repositories return new List(); } - /// - /// Returns the Top Level Parent Guid Id - /// protected override Guid NodeObjectTypeId { - get { return new Guid(Constants.Conventions.Localization.DictionaryItemRootId); } + get { throw new NotImplementedException(); } } #endregion diff --git a/src/Umbraco.Core/Services/LocalizationService.cs b/src/Umbraco.Core/Services/LocalizationService.cs index bd695b65e5..59b77085af 100644 --- a/src/Umbraco.Core/Services/LocalizationService.cs +++ b/src/Umbraco.Core/Services/LocalizationService.cs @@ -18,7 +18,6 @@ namespace Umbraco.Core.Services /// public class LocalizationService : RepositoryService, ILocalizationService { - private static readonly Guid RootParentId = new Guid(Constants.Conventions.Localization.DictionaryItemRootId); [Obsolete("Use the constructors that specify all dependencies instead")] public LocalizationService() @@ -93,7 +92,7 @@ namespace Umbraco.Core.Services } } - var item = new DictionaryItem(parentId.HasValue ? parentId.Value : RootParentId, key); + var item = new DictionaryItem(parentId, key); if (defaultValue.IsNullOrWhiteSpace() == false) { @@ -188,7 +187,7 @@ namespace Umbraco.Core.Services { using (var repository = RepositoryFactory.CreateDictionaryRepository(UowProvider.GetUnitOfWork())) { - var query = Query.Builder.Where(x => x.ParentId == RootParentId); + var query = Query.Builder.Where(x => x.ParentId == null); var items = repository.GetByQuery(query); return items; diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c6883c6a94..b70653a7d3 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -381,6 +381,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs b/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs index 2467bf8a54..a05956e109 100644 --- a/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs +++ b/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs @@ -258,6 +258,7 @@ namespace Umbraco.Tests.Persistence using (Transaction transaction = Database.GetTransaction()) { DatabaseSchemaHelper.CreateTable(); + DatabaseSchemaHelper.CreateTable(); DatabaseSchemaHelper.CreateTable(); //transaction.Complete(); diff --git a/src/Umbraco.Tests/Services/LocalizationServiceTests.cs b/src/Umbraco.Tests/Services/LocalizationServiceTests.cs index 40597dc285..fd0fd1437d 100644 --- a/src/Umbraco.Tests/Services/LocalizationServiceTests.cs +++ b/src/Umbraco.Tests/Services/LocalizationServiceTests.cs @@ -179,7 +179,7 @@ namespace Umbraco.Tests.Services Assert.Greater(item.Id, 0); Assert.IsTrue(item.HasIdentity); - Assert.AreEqual(new Guid(Constants.Conventions.Localization.DictionaryItemRootId), item.ParentId); + Assert.IsFalse(item.ParentId.HasValue); Assert.AreEqual("Testing123", item.ItemKey); Assert.AreEqual(1, item.Translations.Count()); } @@ -197,7 +197,7 @@ namespace Umbraco.Tests.Services Assert.IsNotNull(item); Assert.Greater(item.Id, 0); Assert.IsTrue(item.HasIdentity); - Assert.AreEqual(new Guid(Constants.Conventions.Localization.DictionaryItemRootId), item.ParentId); + Assert.IsFalse(item.ParentId.HasValue); Assert.AreEqual("Testing12345", item.ItemKey); var allLangs = ServiceContext.LocalizationService.GetAllLanguages(); Assert.Greater(allLangs.Count(), 0); diff --git a/src/umbraco.cms/businesslogic/Dictionary.cs b/src/umbraco.cms/businesslogic/Dictionary.cs index 21d9bc516c..8e76122175 100644 --- a/src/umbraco.cms/businesslogic/Dictionary.cs +++ b/src/umbraco.cms/businesslogic/Dictionary.cs @@ -93,8 +93,8 @@ namespace umbraco.cms.businesslogic [Obsolete("This is no longer used and will be removed from the codebase in future versions")] public bool IsTopMostItem() - { - return _dictionaryItem.ParentId == new Guid(Constants.Conventions.Localization.DictionaryItemRootId); + { + return _dictionaryItem.ParentId.HasValue == false; } /// @@ -105,9 +105,9 @@ namespace umbraco.cms.businesslogic get { //EnsureCache(); - if (_parent == null) + if (_parent == null && _dictionaryItem.ParentId.HasValue) { - var p = ApplicationContext.Current.Services.LocalizationService.GetDictionaryItemById(_dictionaryItem.ParentId); + var p = ApplicationContext.Current.Services.LocalizationService.GetDictionaryItemById(_dictionaryItem.ParentId.Value); if (p == null) {