From 91a0ee2c93d0a136bad6b0ed8caf44368ea2f625 Mon Sep 17 00:00:00 2001 From: AndyButland Date: Thu, 12 Jul 2018 20:52:02 +0100 Subject: [PATCH] Refactored to object graph reference of fallback language to just use id --- src/Umbraco.Core/Models/ILanguage.cs | 4 +- src/Umbraco.Core/Models/Language.cs | 10 ++-- .../Persistence/Factories/LanguageFactory.cs | 9 +--- .../Implement/LanguageRepository.cs | 15 +----- .../Repositories/LanguageRepositoryTest.cs | 18 +++---- .../PublishedContentLanuageVariantTests.cs | 8 ++-- .../src/views/languages/edit.controller.js | 20 -------- .../src/views/languages/edit.html | 2 +- .../views/languages/overview.controller.js | 10 ++++ .../src/views/languages/overview.html | 2 +- src/Umbraco.Web/Editors/LanguageController.cs | 47 ++++++++----------- .../Models/ContentEditing/Language.cs | 4 +- .../PublishedValueLanguageFallback.cs | 46 +++++++++--------- 13 files changed, 80 insertions(+), 115 deletions(-) diff --git a/src/Umbraco.Core/Models/ILanguage.cs b/src/Umbraco.Core/Models/ILanguage.cs index f02bd33d2b..8d1c092e13 100644 --- a/src/Umbraco.Core/Models/ILanguage.cs +++ b/src/Umbraco.Core/Models/ILanguage.cs @@ -35,9 +35,9 @@ namespace Umbraco.Core.Models bool Mandatory { get; set; } /// - /// Defines the fallback language that can be used in multi-lingual scenarios to provide + /// Defines the id of a fallback language that can be used in multi-lingual scenarios to provide /// content if the requested language does not have it published. /// - ILanguage FallbackLanguage { get; set; } + int? FallbackLanguageId { get; set; } } } diff --git a/src/Umbraco.Core/Models/Language.cs b/src/Umbraco.Core/Models/Language.cs index 42c305d492..5fcd5cd50e 100644 --- a/src/Umbraco.Core/Models/Language.cs +++ b/src/Umbraco.Core/Models/Language.cs @@ -19,7 +19,7 @@ namespace Umbraco.Core.Models private string _cultureName; private bool _isDefaultVariantLanguage; private bool _mandatory; - private ILanguage _fallbackLanguage; + private int? _fallbackLanguageId; public Language(string isoCode) { @@ -33,7 +33,7 @@ namespace Umbraco.Core.Models public readonly PropertyInfo CultureNameSelector = ExpressionHelper.GetPropertyInfo(x => x.CultureName); public readonly PropertyInfo IsDefaultVariantLanguageSelector = ExpressionHelper.GetPropertyInfo(x => x.IsDefaultVariantLanguage); public readonly PropertyInfo MandatorySelector = ExpressionHelper.GetPropertyInfo(x => x.Mandatory); - public readonly PropertyInfo FallbackLanguageSelector = ExpressionHelper.GetPropertyInfo(x => x.FallbackLanguage); + public readonly PropertyInfo FallbackLanguageSelector = ExpressionHelper.GetPropertyInfo(x => x.FallbackLanguageId); } /// @@ -74,10 +74,10 @@ namespace Umbraco.Core.Models set => SetPropertyValueAndDetectChanges(value, ref _mandatory, Ps.Value.MandatorySelector); } - public ILanguage FallbackLanguage + public int? FallbackLanguageId { - get => _fallbackLanguage; - set => SetPropertyValueAndDetectChanges(value, ref _fallbackLanguage, Ps.Value.FallbackLanguageSelector); + get => _fallbackLanguageId; + set => SetPropertyValueAndDetectChanges(value, ref _fallbackLanguageId, Ps.Value.FallbackLanguageSelector); } } } diff --git a/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs b/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs index c805ae7f5a..7ab36d15d6 100644 --- a/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs @@ -8,7 +8,7 @@ namespace Umbraco.Core.Persistence.Factories { public static ILanguage BuildEntity(LanguageDto dto) { - var lang = new Language(dto.IsoCode) { CultureName = dto.CultureName, Id = dto.Id, IsDefaultVariantLanguage = dto.IsDefaultVariantLanguage, Mandatory = dto.Mandatory }; + var lang = new Language(dto.IsoCode) { CultureName = dto.CultureName, Id = dto.Id, IsDefaultVariantLanguage = dto.IsDefaultVariantLanguage, Mandatory = dto.Mandatory, FallbackLanguageId = dto.FallbackLanguageId }; // reset dirty initial properties (U4-1946) lang.ResetDirtyProperties(false); return lang; @@ -16,17 +16,12 @@ namespace Umbraco.Core.Persistence.Factories public static LanguageDto BuildDto(ILanguage entity) { - var dto = new LanguageDto { CultureName = entity.CultureName, IsoCode = entity.IsoCode, IsDefaultVariantLanguage = entity.IsDefaultVariantLanguage, Mandatory = entity.Mandatory }; + var dto = new LanguageDto { CultureName = entity.CultureName, IsoCode = entity.IsoCode, IsDefaultVariantLanguage = entity.IsDefaultVariantLanguage, Mandatory = entity.Mandatory, FallbackLanguageId = entity.FallbackLanguageId }; if (entity.HasIdentity) { dto.Id = short.Parse(entity.Id.ToString(CultureInfo.InvariantCulture)); } - if (entity.FallbackLanguage != null) - { - dto.FallbackLanguageId = entity.FallbackLanguage.Id; - } - return dto; } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs index 96bb088f2b..af5d28c18e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs @@ -54,7 +54,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // get languages var dtos = Database.Fetch(sql); var languages = dtos.Select(ConvertFromDto).ToList(); - PopulateFallbackLanguages(dtos, languages); // initialize the code-id map lock (_codeIdMap) @@ -77,9 +76,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var translator = new SqlTranslator(sqlClause, query); var sql = translator.Translate(); var dtos = Database.Fetch(sql); - var languages = dtos.Select(ConvertFromDto).ToList(); - PopulateFallbackLanguages(dtos, languages); - return languages; + return dtos.Select(ConvertFromDto).ToList(); } #endregion @@ -203,16 +200,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return entity; } - private static void PopulateFallbackLanguages(List dtos, IList languages) - { - foreach (var dto in dtos.Where(x => x.FallbackLanguageId.HasValue)) - { - var language = languages.Single(x => x.Id == dto.Id); - // ReSharper disable once PossibleInvalidOperationException (DTOs with fallback languages have already been filtered in the loop condition) - language.FallbackLanguage = languages.Single(x => x.Id == dto.FallbackLanguageId.Value); - } - } - public ILanguage GetByIsoCode(string isoCode) { TypedCachePolicy.GetAllCached(PerformGetAll); // ensure cache is populated, in a non-expensive way diff --git a/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs index bda899789d..a63bf5e08d 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/LanguageRepositoryTest.cs @@ -47,7 +47,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(language.HasIdentity, Is.True); Assert.That(language.CultureName, Is.EqualTo("en-US")); Assert.That(language.IsoCode, Is.EqualTo("en-US")); - Assert.That(language.FallbackLanguage, Is.Null); + Assert.That(language.FallbackLanguageId, Is.Null); } } @@ -63,7 +63,7 @@ namespace Umbraco.Tests.Persistence.Repositories var language = (ILanguage)new Language(au.Name) { CultureName = au.DisplayName, - FallbackLanguage = repository.Get(1) + FallbackLanguageId = 1 }; repository.Save(language); @@ -75,7 +75,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(language.HasIdentity, Is.True); Assert.That(language.CultureName, Is.EqualTo(au.DisplayName)); Assert.That(language.IsoCode, Is.EqualTo(au.Name)); - Assert.That(language.FallbackLanguage.IsoCode, Is.EqualTo("en-US")); + Assert.That(language.FallbackLanguageId, Is.EqualTo(1)); } } @@ -193,7 +193,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(languageBR.Id, Is.EqualTo(6)); //With 5 existing entries the Id should be 6 Assert.IsFalse(languageBR.IsDefaultVariantLanguage); Assert.IsFalse(languageBR.Mandatory); - Assert.IsNull(languageBR.FallbackLanguage); + Assert.IsNull(languageBR.FallbackLanguageId); } } @@ -215,7 +215,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(languageBR.Id, Is.EqualTo(6)); //With 5 existing entries the Id should be 6 Assert.IsTrue(languageBR.IsDefaultVariantLanguage); Assert.IsTrue(languageBR.Mandatory); - Assert.IsNull(languageBR.FallbackLanguage); + Assert.IsNull(languageBR.FallbackLanguageId); } } @@ -232,14 +232,14 @@ namespace Umbraco.Tests.Persistence.Repositories var languageBR = new Language("pt-BR") { CultureName = "pt-BR", - FallbackLanguage = repository.Get(1) + FallbackLanguageId = 1 }; repository.Save(languageBR); // Assert Assert.That(languageBR.HasIdentity, Is.True); Assert.That(languageBR.Id, Is.EqualTo(6)); //With 5 existing entries the Id should be 6 - Assert.That(languageBR.FallbackLanguage.IsoCode, Is.EqualTo("en-US")); + Assert.That(languageBR.FallbackLanguageId, Is.EqualTo(1)); } } @@ -284,7 +284,7 @@ namespace Umbraco.Tests.Persistence.Repositories var language = repository.Get(5); language.IsoCode = "pt-BR"; language.CultureName = "pt-BR"; - language.FallbackLanguage = repository.Get(1); + language.FallbackLanguageId = 1; repository.Save(language); @@ -294,7 +294,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(languageUpdated, Is.Not.Null); Assert.That(languageUpdated.IsoCode, Is.EqualTo("pt-BR")); Assert.That(languageUpdated.CultureName, Is.EqualTo("pt-BR")); - Assert.That(languageUpdated.FallbackLanguage.IsoCode, Is.EqualTo("en-US")); + Assert.That(languageUpdated.FallbackLanguageId, Is.EqualTo(1)); } } diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentLanuageVariantTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentLanuageVariantTests.cs index ea77310977..22eb4bd799 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentLanuageVariantTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentLanuageVariantTests.cs @@ -39,15 +39,15 @@ namespace Umbraco.Tests.PublishedContent { new Language("en-US") { Id = 1, CultureName = "English", IsDefaultVariantLanguage = true }, new Language("fr") { Id = 2, CultureName = "French" }, - new Language("es") { Id = 3, CultureName = "Spanish" }, - new Language("it") { Id = 4, CultureName = "Italian" }, + new Language("es") { Id = 3, CultureName = "Spanish", FallbackLanguageId = 1 }, + new Language("it") { Id = 4, CultureName = "Italian", FallbackLanguageId = 3 }, new Language("de") { Id = 5, CultureName = "German" } }; - languages[2].FallbackLanguage = languages[0]; - languages[3].FallbackLanguage = languages[2]; var localizationService = Mock.Get(serviceContext.LocalizationService); localizationService.Setup(x => x.GetAllLanguages()).Returns(languages); + localizationService.Setup(x => x.GetLanguageById(It.IsAny())) + .Returns((int id) => languages.SingleOrDefault(y => y.Id == id)); localizationService.Setup(x => x.GetLanguageByIsoCode(It.IsAny())) .Returns((string c) => languages.SingleOrDefault(y => y.IsoCode == c)); } diff --git a/src/Umbraco.Web.UI.Client/src/views/languages/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/languages/edit.controller.js index 79972725fc..523ef867cf 100644 --- a/src/Umbraco.Web.UI.Client/src/views/languages/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/languages/edit.controller.js @@ -98,31 +98,11 @@ }); } - function setCultureForFallbackLanguage(lang) { - for (var i = 0; i < vm.availableLanguages.length; i++) { - if (vm.availableLanguages[i].id === lang.id) { - lang.culture = vm.availableLanguages[i].culture; - break; - } - } - } - function save() { if (formHelper.submitForm({ scope: $scope })) { vm.page.saveButtonState = "busy"; - // Handle selection of no fall-back language (should pass null) - if (!vm.language.fallbackLanguage.id) { - vm.language.fallbackLanguage = null; - } - - // We need to attach the ISO code to the fall-back language to pass - // server-side validation. - if (vm.language.fallbackLanguage) { - setCultureForFallbackLanguage(vm.language.fallbackLanguage); - } - languageResource.save(vm.language).then(function (lang) { formHelper.resetForm({ scope: $scope }); diff --git a/src/Umbraco.Web.UI.Client/src/views/languages/edit.html b/src/Umbraco.Web.UI.Client/src/views/languages/edit.html index 5570b901e2..a2217a6649 100644 --- a/src/Umbraco.Web.UI.Client/src/views/languages/edit.html +++ b/src/Umbraco.Web.UI.Client/src/views/languages/edit.html @@ -67,7 +67,7 @@
diff --git a/src/Umbraco.Web.UI.Client/src/views/languages/overview.controller.js b/src/Umbraco.Web.UI.Client/src/views/languages/overview.controller.js index c8a728d3aa..a5c446dfb5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/languages/overview.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/languages/overview.controller.js @@ -13,6 +13,16 @@ vm.editLanguage = editLanguage; vm.deleteLanguage = deleteLanguage; + vm.getLanguageById = function(id) { + for (var i = 0; i < vm.languages.length; i++) { + if (vm.languages[i].id === id) { + return vm.languages[i]; + } + } + + return null; + }; + function init() { vm.loading = true; diff --git a/src/Umbraco.Web.UI.Client/src/views/languages/overview.html b/src/Umbraco.Web.UI.Client/src/views/languages/overview.html index f53326a491..3b75fa62bd 100644 --- a/src/Umbraco.Web.UI.Client/src/views/languages/overview.html +++ b/src/Umbraco.Web.UI.Client/src/views/languages/overview.html @@ -36,7 +36,7 @@ - {{vm.labels.general}} {{vm.labels.mandatory}} - {{vm.labels.fallsbackTo}}: {{language.fallbackLanguage.name}} + {{vm.labels.fallsbackTo}}: {{vm.getLanguageById(language.fallbackLanguageId).name}} x.FallbackLanguage?.Id == language.Id)) + 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)); @@ -147,20 +147,21 @@ namespace Umbraco.Web.Editors CultureName = culture.DisplayName, IsDefaultVariantLanguage = language.IsDefaultVariantLanguage, Mandatory = language.Mandatory, + FallbackLanguageId = language.FallbackLanguageId }; - AssociateFallbackLanguage(language, newLang); Services.LocalizationService.Save(newLang); return Mapper.Map(newLang); } found.Mandatory = language.Mandatory; found.IsDefaultVariantLanguage = language.IsDefaultVariantLanguage; - AssociateFallbackLanguage(language, found); + found.FallbackLanguageId = language.FallbackLanguageId; - if (DoesUpdatedFallbackLanguageCreateACircularPath(found)) + string selectedFallbackLanguageCultureName; + if (DoesUpdatedFallbackLanguageCreateACircularPath(found, out selectedFallbackLanguageCultureName)) { - ModelState.AddModelError("FallbackLanguage", "The selected fall back language '" + found.FallbackLanguage.CultureName + "' would create a circular path."); + ModelState.AddModelError("FallbackLanguage", "The selected fall back language '" + selectedFallbackLanguageCultureName + "' would create a circular path."); throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); } @@ -168,43 +169,35 @@ namespace Umbraco.Web.Editors return Mapper.Map(found); } - private static void AssociateFallbackLanguage(Language submittedLanguage, ILanguage languageToCreateOrUpdate) + private bool DoesUpdatedFallbackLanguageCreateACircularPath(ILanguage language, out string selectedFallbackLanguageCultureName) { - if (submittedLanguage.FallbackLanguage == null) - { - languageToCreateOrUpdate.FallbackLanguage = null; - return; - } - - var fallbackLanguageCulture = CultureInfo.GetCultureInfo(submittedLanguage.FallbackLanguage.IsoCode); - languageToCreateOrUpdate.FallbackLanguage = new Core.Models.Language(fallbackLanguageCulture.Name) - { - Id = submittedLanguage.FallbackLanguage.Id, - CultureName = fallbackLanguageCulture.DisplayName - }; - } - - private bool DoesUpdatedFallbackLanguageCreateACircularPath(ILanguage language) - { - if (language.FallbackLanguage == null) + if (language.FallbackLanguageId.HasValue == false) { + selectedFallbackLanguageCultureName = string.Empty; return false; } var languages = Services.LocalizationService.GetAllLanguages().ToArray(); - var fallbackLanguage = language.FallbackLanguage; - while (fallbackLanguage != null) + var fallbackLanguageId = language.FallbackLanguageId; + while (fallbackLanguageId.HasValue) { - if (fallbackLanguage.Id == language.Id) + 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; } - fallbackLanguage = languages.Single(x => x.Id == fallbackLanguage.Id).FallbackLanguage; + fallbackLanguageId = GetLanguageFromCollectionById(languages, fallbackLanguageId.Value).FallbackLanguageId; } + selectedFallbackLanguageCultureName = string.Empty; return false; } + + private static ILanguage GetLanguageFromCollectionById(IEnumerable languages, int id) + { + return languages.Single(x => x.Id == id); + } } } diff --git a/src/Umbraco.Web/Models/ContentEditing/Language.cs b/src/Umbraco.Web/Models/ContentEditing/Language.cs index 309e111e32..7693ee836e 100644 --- a/src/Umbraco.Web/Models/ContentEditing/Language.cs +++ b/src/Umbraco.Web/Models/ContentEditing/Language.cs @@ -25,7 +25,7 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "isMandatory")] public bool Mandatory { get; set; } - [DataMember(Name = "fallbackLanguage")] - public Language FallbackLanguage { get; set; } + [DataMember(Name = "fallbackLanguageId")] + public int? FallbackLanguageId { get; set; } } } diff --git a/src/Umbraco.Web/Models/PublishedContent/PublishedValueLanguageFallback.cs b/src/Umbraco.Web/Models/PublishedContent/PublishedValueLanguageFallback.cs index b6dc9f4244..d6e8db83f4 100644 --- a/src/Umbraco.Web/Models/PublishedContent/PublishedValueLanguageFallback.cs +++ b/src/Umbraco.Web/Models/PublishedContent/PublishedValueLanguageFallback.cs @@ -107,13 +107,6 @@ namespace Umbraco.Web.Models.PublishedContent return base.GetValue(content, alias, culture, segment, defaultValue, recurse, fallbackPriority); } - private static bool ValueIsNotNullEmptyOrDefault(T value, T defaultValue) - { - return value != null && - string.IsNullOrEmpty(value.ToString()) == false && - value.Equals(defaultValue) == false; - } - private bool TryGetValueFromFallbackLanguage(IPublishedProperty property, string culture, string segment, T defaultValue, out T value) { if (string.IsNullOrEmpty(culture)) @@ -123,22 +116,23 @@ namespace Umbraco.Web.Models.PublishedContent } var language = _localizationService.GetLanguageByIsoCode(culture); - if (language.FallbackLanguage == null) + if (language.FallbackLanguageId.HasValue == false) { value = defaultValue; return false; } - var fallbackLanguage = language.FallbackLanguage; - while (fallbackLanguage != null) + var fallbackLanguageId = language.FallbackLanguageId; + while (fallbackLanguageId.HasValue) { + var fallbackLanguage = GetLanguageById(fallbackLanguageId.Value); value = property.Value(fallbackLanguage.IsoCode, segment, defaultValue); if (ValueIsNotNullEmptyOrDefault(value, defaultValue)) { return true; } - fallbackLanguage = GetNextFallbackLanguage(fallbackLanguage); + fallbackLanguageId = fallbackLanguage.FallbackLanguageId; } value = defaultValue; @@ -154,22 +148,23 @@ namespace Umbraco.Web.Models.PublishedContent } var language = _localizationService.GetLanguageByIsoCode(culture); - if (language.FallbackLanguage == null) + if (language.FallbackLanguageId.HasValue == false) { value = defaultValue; return false; } - var fallbackLanguage = language.FallbackLanguage; - while (fallbackLanguage != null) + var fallbackLanguageId = language.FallbackLanguageId; + while (fallbackLanguageId.HasValue) { + var fallbackLanguage = GetLanguageById(fallbackLanguageId.Value); value = content.Value(alias, fallbackLanguage.IsoCode, segment, defaultValue); if (ValueIsNotNullEmptyOrDefault(value, defaultValue)) { return true; } - fallbackLanguage = GetNextFallbackLanguage(fallbackLanguage); + fallbackLanguageId = fallbackLanguage.FallbackLanguageId; } value = defaultValue; @@ -185,34 +180,39 @@ namespace Umbraco.Web.Models.PublishedContent } var language = _localizationService.GetLanguageByIsoCode(culture); - if (language.FallbackLanguage == null) + if (language.FallbackLanguageId.HasValue == false) { value = defaultValue; return false; } - var fallbackLanguage = language.FallbackLanguage; - while (fallbackLanguage != null) + var fallbackLanguageId = language.FallbackLanguageId; + while (fallbackLanguageId.HasValue) { + var fallbackLanguage = GetLanguageById(fallbackLanguageId.Value); value = content.Value(alias, fallbackLanguage.IsoCode, segment, defaultValue, recurse); if (ValueIsNotNullEmptyOrDefault(value, defaultValue)) { return true; } - fallbackLanguage = GetNextFallbackLanguage(fallbackLanguage); + fallbackLanguageId = fallbackLanguage.FallbackLanguageId; } value = defaultValue; return false; } - private ILanguage GetNextFallbackLanguage(ILanguage fallbackLanguage) + private ILanguage GetLanguageById(int id) { - // Ensure reference to next fall-back language is loaded if it exists - fallbackLanguage = _localizationService.GetLanguageById(fallbackLanguage.Id); + return _localizationService.GetLanguageById(id); + } - return fallbackLanguage.FallbackLanguage; + private static bool ValueIsNotNullEmptyOrDefault(T value, T defaultValue) + { + return value != null && + string.IsNullOrEmpty(value.ToString()) == false && + value.Equals(defaultValue) == false; } } }