From 25c87d76b8cbda43a71ade65a7cb49515efb32d9 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 13 Sep 2018 14:59:45 +0200 Subject: [PATCH] Fix merge --- .../Migrations/Install/DatabaseDataCreator.cs | 2 +- .../Migrations/Upgrade/UmbracoPlan.cs | 11 +-- .../V_8_0_0/UpdateDefaultMandatoryLanguage.cs | 6 +- .../Persistence/Dtos/LanguageDto.cs | 2 +- .../Persistence/Factories/LanguageFactory.cs | 4 +- .../Implement/LanguageRepository.cs | 15 ++++- .../Services/Implement/LocalizationService.cs | 24 +++++++ src/Umbraco.Web/Editors/LanguageController.cs | 67 ++++++++----------- 8 files changed, 78 insertions(+), 53 deletions(-) diff --git a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs index bfb8705480..5ec259ade4 100644 --- a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs @@ -232,7 +232,7 @@ namespace Umbraco.Core.Migrations.Install private void CreateLanguageData() { - _database.Insert(Constants.DatabaseSchema.Tables.Language, "id", false, new LanguageDto { Id = 1, IsoCode = "en-US", CultureName = "English (United States)", IsDefaultVariantLanguage = true }); + _database.Insert(Constants.DatabaseSchema.Tables.Language, "id", false, new LanguageDto { Id = 1, IsoCode = "en-US", CultureName = "English (United States)", IsDefault = true }); } private void CreateContentChildTypeData() diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index fb87a1ff4f..036fc99fa4 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -118,17 +118,18 @@ namespace Umbraco.Core.Migrations.Upgrade Chain("{EB34B5DC-BB87-4005-985E-D983EA496C38}"); // from 7.12.0 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 final + .CopyChain("{39E5B1F7-A50B-437E-B768-1723AEC45B65}", "{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}", "{8B14CEBD-EE47-4AAD-A841-93551D917F11}"); // to next // resume at {8B14CEBD-EE47-4AAD-A841-93551D917F11} ... - Chain("{guid1}"); // add stephan's after others, with a new target state + + Chain("{5F4597F4-A4E0-4AFE-90B5-6D2F896830EB}"); // add stephan's after others, with a new target state From("{2C87AA47-D1BC-4ECB-8A73-2D8D1046C27F}") // and provide a path out of stephan's - .Chain("{guid1}"); - #error is this OK? + .Chain("{5F4597F4-A4E0-4AFE-90B5-6D2F896830EB}"); // to next + // resume at {5F4597F4-A4E0-4AFE-90B5-6D2F896830EB} ... + //FINAL diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/UpdateDefaultMandatoryLanguage.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/UpdateDefaultMandatoryLanguage.cs index dd5fe7c369..b965bc71d2 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/UpdateDefaultMandatoryLanguage.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/UpdateDefaultMandatoryLanguage.cs @@ -26,7 +26,7 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 var defaultId = int.MaxValue; foreach (var dto in dtos) { - if (dto.IsDefaultVariantLanguage) + if (dto.IsDefault) { defaultId = dto.Id; break; @@ -38,8 +38,8 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 // update, so that language with that id is now default and mandatory var updateDefault = Sql() .Update(u => u - .Set(x => x.IsDefaultVariantLanguage, true) - .Set(x => x.Mandatory, true)) + .Set(x => x.IsDefault, true) + .Set(x => x.IsMandatory, true)) .Where(x => x.Id == defaultId); Database.Execute(updateDefault); diff --git a/src/Umbraco.Core/Persistence/Dtos/LanguageDto.cs b/src/Umbraco.Core/Persistence/Dtos/LanguageDto.cs index 5f3247bc49..488390f985 100644 --- a/src/Umbraco.Core/Persistence/Dtos/LanguageDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/LanguageDto.cs @@ -39,7 +39,7 @@ namespace Umbraco.Core.Persistence.Dtos /// [Column("isDefaultVariantLang")] [Constraint(Default = "0")] - public bool IsDefaultVariantLanguage { get; set; } + public bool IsDefault { get; set; } /// /// Gets or sets a value indicating whether the language is mandatory. diff --git a/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs b/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs index 9be2409a5e..ad58c5b570 100644 --- a/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Persistence.Factories { CultureName = dto.CultureName, Id = dto.Id, - IsDefault = dto.IsDefaultVariantLanguage, + IsDefault = dto.IsDefault, IsMandatory = dto.IsMandatory, FallbackLanguageId = dto.FallbackLanguageId }; @@ -28,7 +28,7 @@ namespace Umbraco.Core.Persistence.Factories { CultureName = entity.CultureName, IsoCode = entity.IsoCode, - IsDefaultVariantLanguage = entity.IsDefault, + IsDefault = entity.IsDefault, IsMandatory = entity.IsMandatory, FallbackLanguageId = entity.FallbackLanguageId }; diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs index 97535fe07c..2b3674700b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs @@ -136,9 +136,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // set all other entities to non-default // safe (no race cond) because the service locks languages var setAllDefaultToFalse = Sql() - .Update(u => u.Set(x => x.IsDefaultVariantLanguage, false)); + .Update(u => u.Set(x => x.IsDefault, false)); Database.Execute(setAllDefaultToFalse); } + + // fallback cycles are detected at service level + // insert var dto = LanguageFactory.BuildDto(entity); var id = Convert.ToInt32(Database.Insert(dto)); @@ -178,6 +181,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement throw new InvalidOperationException($"Cannot save the default language ({entity.IsoCode}) as non-default. Make another language the default language instead."); } + // fallback cycles are detected at service level + // update var dto = LanguageFactory.BuildDto(entity); Database.Update(dto); @@ -199,8 +204,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement throw new InvalidOperationException($"Cannot delete the default language ({entity.IsoCode})."); // We need to remove any references to the language if it's being used as a fall-back from other ones - #error rewirte - Database.Execute(Sql().Update(u => u.Set(x => x.FallbackLanguageId, null)).Where(x => x.FallbackLanguageId == entity.Id)); + var clearFallbackLanguage = Sql() + .Update(u => u + .Set(x => x.FallbackLanguageId, null)) + .Where(x => x.FallbackLanguageId == entity.Id); + + Database.Execute(clearFallbackLanguage); // delete base.PersistDeletedItem(entity); diff --git a/src/Umbraco.Core/Services/Implement/LocalizationService.cs b/src/Umbraco.Core/Services/Implement/LocalizationService.cs index ec328bdb9d..49a764b533 100644 --- a/src/Umbraco.Core/Services/Implement/LocalizationService.cs +++ b/src/Umbraco.Core/Services/Implement/LocalizationService.cs @@ -363,6 +363,16 @@ namespace Umbraco.Core.Services.Implement // write-lock languages to guard against race conds when dealing with default language scope.WriteLock(Constants.Locks.Languages); + // look for cycles - within write-lock + if (language.FallbackLanguageId.HasValue) + { + var languages = _languageRepository.GetMany().ToDictionary(x => x.Id, x => x); + if (!languages.ContainsKey(language.FallbackLanguageId.Value)) + throw new InvalidOperationException($"Cannot save language {language.IsoCode} with fallback id={language.FallbackLanguageId.Value} which is not a valid language id."); + if (CreatesCycle(language, languages)) + throw new InvalidOperationException($"Cannot save language {language.IsoCode} with fallback {languages[language.FallbackLanguageId.Value].IsoCode} as it would create a fallback cycle."); + } + var saveEventArgs = new SaveEventArgs(language); if (scope.Events.DispatchCancelable(SavingLanguage, this, saveEventArgs)) { @@ -380,6 +390,20 @@ namespace Umbraco.Core.Services.Implement } } + private bool CreatesCycle(ILanguage language, IDictionary languages) + { + // a new language is not referenced yet, so cannot be part of a cycle + if (!language.HasIdentity) return false; + + var id = language.FallbackLanguageId; + while (true) // assuming languages does not already contains a cycle, this must end + { + if (!id.HasValue) return false; // no fallback means no cycle + if (id.Value == language.Id) return true; // back to language = cycle! + id = languages[id.Value].FallbackLanguageId; // else keep chaining + } + } + /// /// Deletes a by removing it (but not its usages) from the db /// diff --git a/src/Umbraco.Web/Editors/LanguageController.cs b/src/Umbraco.Web/Editors/LanguageController.cs index 5a6848dea1..a3e613670b 100644 --- a/src/Umbraco.Web/Editors/LanguageController.cs +++ b/src/Umbraco.Web/Editors/LanguageController.cs @@ -77,11 +77,8 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(Request.CreateNotificationValidationErrorResponse(message)); } - if (langs.Any(x => x.FallbackLanguageId.HasValue && x.FallbackLanguageId.Value == language.Id)) - { - var message = $"Language '{language.CultureName}' is defined as a fall-back language for one or more other languages, and so cannot be deleted."; - throw new HttpResponseException(Request.CreateNotificationValidationErrorResponse(message)); - } + // service is happy deleting a language that's fallback for another language, + // will just remove it - so no need to check here Services.LocalizationService.Delete(language); @@ -121,7 +118,7 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); } - //create it + // create it (creating a new language cannot create a fallback cycle) var newLang = new Core.Models.Language(culture.Name) { CultureName = culture.DisplayName, @@ -147,46 +144,40 @@ namespace Umbraco.Web.Editors existing.IsDefault = language.IsDefault; existing.FallbackLanguageId = language.FallbackLanguageId; - string selectedFallbackLanguageCultureName; - if (DoesUpdatedFallbackLanguageCreateACircularPath(existing, out selectedFallbackLanguageCultureName)) + // modifying an existing language can create a fallback, verify + // note that the service will check again, dealing with race conds + if (existing.FallbackLanguageId.HasValue) { - ModelState.AddModelError("FallbackLanguage", "The selected fall back language '" + selectedFallbackLanguageCultureName + "' would create a circular path."); - throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); + var languages = Services.LocalizationService.GetAllLanguages().ToDictionary(x => x.Id, x => x); + if (!languages.ContainsKey(existing.FallbackLanguageId.Value)) + { + ModelState.AddModelError("FallbackLanguage", "The selected fall back language does not exist."); + throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); + } + if (CreatesCycle(existing, languages)) + { + ModelState.AddModelError("FallbackLanguage", $"The selected fall back language {languages[existing.FallbackLanguageId.Value].IsoCode} would create a circular path."); + throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); + } } Services.LocalizationService.Save(existing); return Mapper.Map(existing); - } - - private bool DoesUpdatedFallbackLanguageCreateACircularPath(ILanguage language, out string selectedFallbackLanguageCultureName) - { - if (language.FallbackLanguageId.HasValue == false) - { - selectedFallbackLanguageCultureName = string.Empty; - return false; - } - - var languages = Services.LocalizationService.GetAllLanguages().ToArray(); - var fallbackLanguageId = language.FallbackLanguageId; - while (fallbackLanguageId.HasValue) - { - if (fallbackLanguageId.Value == language.Id) - { - // We've found the current language in the path of fall back languages, so we have a circular path. - selectedFallbackLanguageCultureName = GetLanguageFromCollectionById(languages, fallbackLanguageId.Value).CultureName; - return true; - } - - fallbackLanguageId = GetLanguageFromCollectionById(languages, fallbackLanguageId.Value).FallbackLanguageId; - } - - selectedFallbackLanguageCultureName = string.Empty; - return false; } - private static ILanguage GetLanguageFromCollectionById(IEnumerable languages, int id) + // see LocalizationService + private bool CreatesCycle(ILanguage language, IDictionary languages) { - return languages.Single(x => x.Id == id); + // a new language is not referenced yet, so cannot be part of a cycle + if (!language.HasIdentity) return false; + + var id = language.FallbackLanguageId; + while (true) // assuming languages does not already contains a cycle, this must end + { + if (!id.HasValue) return false; // no fallback means no cycle + if (id.Value == language.Id) return true; // back to language = cycle! + id = languages[id.Value].FallbackLanguageId; // else keep chaining + } } } }