From 4cbd0d9178d846c4a7ffe980b233ab0b43c9e3bf Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 12 Sep 2018 11:47:04 +0200 Subject: [PATCH] Remove mandatory constraint on default language --- .../Migrations/Install/DatabaseDataCreator.cs | 2 +- .../Migrations/Upgrade/UmbracoPlan.cs | 1 + .../Upgrade/V_8_0_0/AddLockObjects.cs | 12 +- .../V_8_0_0/UpdateDefaultMandatoryLanguage.cs | 48 ++++++++ .../Persistence/Constants-Locks.cs | 43 +++++++ .../Persistence/Factories/LanguageFactory.cs | 18 ++- .../Implement/LanguageRepository.cs | 107 ++++++++++-------- .../Services/Implement/LocalizationService.cs | 6 + src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../src/views/languages/edit.controller.js | 5 +- .../src/views/languages/edit.html | 2 +- .../Umbraco/config/lang/en_us.xml | 2 +- src/Umbraco.Web/Editors/LanguageController.cs | 64 ++++------- 13 files changed, 213 insertions(+), 98 deletions(-) create mode 100644 src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/UpdateDefaultMandatoryLanguage.cs diff --git a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs index ac74e6ee66..bfb8705480 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)" }); + _database.Insert(Constants.DatabaseSchema.Tables.Language, "id", false, new LanguageDto { Id = 1, IsoCode = "en-US", CultureName = "English (United States)", IsDefaultVariantLanguage = true }); } private void CreateContentChildTypeData() diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index 48ac39a630..09846ed6e2 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -117,6 +117,7 @@ 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}"); //FINAL diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLockObjects.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLockObjects.cs index d595c70fa0..7c0b26dd53 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLockObjects.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/AddLockObjects.cs @@ -1,4 +1,5 @@ -using Umbraco.Core.Persistence.Dtos; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Dtos; namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 { @@ -23,11 +24,18 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 private void EnsureLockObject(int id, string name) { - var db = Database; + EnsureLockObject(Database, id, name); + } + + internal static void EnsureLockObject(IUmbracoDatabase db, int id, string name) + { + // not if it already exists var exists = db.Exists(id); if (exists) return; + // be safe: delete old umbracoNode lock objects if any db.Execute($"DELETE FROM umbracoNode WHERE id={id};"); + // then create umbracoLock object db.Execute($"INSERT umbracoLock (id, name, value) VALUES ({id}, '{name}', 1);"); } 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 new file mode 100644 index 0000000000..dd5fe7c369 --- /dev/null +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/UpdateDefaultMandatoryLanguage.cs @@ -0,0 +1,48 @@ +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Dtos; + +namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 +{ + public class UpdateDefaultMandatoryLanguage : MigrationBase + { + public UpdateDefaultMandatoryLanguage(IMigrationContext context) + : base(context) + { } + + public override void Migrate() + { + // add the new languages lock object + AddLockObjects.EnsureLockObject(Database, Constants.Locks.Languages, "Languages"); + + // get all existing languages + var selectDtos = Sql() + .Select() + .From(); + + var dtos = Database.Fetch(selectDtos); + + // get the id of the language which is already the default one, if any, + // else get the lowest language id, which will become the default language + var defaultId = int.MaxValue; + foreach (var dto in dtos) + { + if (dto.IsDefaultVariantLanguage) + { + defaultId = dto.Id; + break; + } + + if (dto.Id < defaultId) defaultId = dto.Id; + } + + // 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)) + .Where(x => x.Id == defaultId); + + Database.Execute(updateDefault); + } + } +} diff --git a/src/Umbraco.Core/Persistence/Constants-Locks.cs b/src/Umbraco.Core/Persistence/Constants-Locks.cs index 6f5d4bb0dc..1dcd2408e7 100644 --- a/src/Umbraco.Core/Persistence/Constants-Locks.cs +++ b/src/Umbraco.Core/Persistence/Constants-Locks.cs @@ -3,17 +3,60 @@ namespace Umbraco.Core { static partial class Constants { + /// + /// Defines lock objects. + /// public static class Locks { + /// + /// All servers. + /// public const int Servers = -331; + + /// + /// All content and media types. + /// public const int ContentTypes = -332; + + /// + /// The entire content tree, i.e. all content items. + /// public const int ContentTree = -333; + + /// + /// The entire media tree, i.e. all media items. + /// public const int MediaTree = -334; + + /// + /// The entire member tree, i.e. all members. + /// public const int MemberTree = -335; + + /// + /// All media types. + /// public const int MediaTypes = -336; + + /// + /// All member types. + /// public const int MemberTypes = -337; + + /// + /// All domains. + /// public const int Domains = -338; + + /// + /// All key-values. + /// public const int KeyValues = -339; + + /// + /// All languages. + /// + public const int Languages = -340; } } } diff --git a/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs b/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs index 7b24411498..30bd9c68ed 100644 --- a/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/LanguageFactory.cs @@ -8,7 +8,14 @@ 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 + }; + // reset dirty initial properties (U4-1946) lang.ResetDirtyProperties(false); return lang; @@ -16,7 +23,14 @@ 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 + }; + if (entity.HasIdentity) dto.Id = short.Parse(entity.Id.ToString(CultureInfo.InvariantCulture)); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs index 740015683a..ae5d9ae8b8 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs @@ -54,11 +54,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // get languages var languages = Database.Fetch(sql).Select(ConvertFromDto).OrderBy(x => x.Id).ToList(); - // fix inconsistencies: there has to be a default language, and it has to be mandatory - var defaultLanguage = languages.FirstOrDefault(x => x.IsDefaultVariantLanguage) ?? languages.First(); - defaultLanguage.IsDefaultVariantLanguage = true; - defaultLanguage.Mandatory = true; - // initialize the code-id map lock (_codeIdMap) { @@ -131,70 +126,83 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistNewItem(ILanguage entity) { - if (entity.IsoCode.IsNullOrWhiteSpace() || entity.CultureInfo == null || entity.CultureName.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The required language data is missing"); + // validate iso code and culture name + if (entity.IsoCode.IsNullOrWhiteSpace() || entity.CultureName.IsNullOrWhiteSpace()) + throw new InvalidOperationException("Cannot save a language without an ISO code and a culture name."); - ((EntityBase)entity).AddingEntity(); + ((EntityBase) entity).AddingEntity(); + // deal with entity becoming the new default entity if (entity.IsDefaultVariantLanguage) { - //if this entity is flagged as the default, we need to set all others to false - Database.Execute(Sql().Update(u => u.Set(x => x.IsDefaultVariantLanguage, false))); - //We need to clear the whole cache since all languages will be updated - IsolatedCache.ClearAllCache(); + // 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)); + Database.Execute(setAllDefaultToFalse); } - ; + // insert var dto = LanguageFactory.BuildDto(entity); - var id = Convert.ToInt32(Database.Insert(dto)); entity.Id = id; - entity.ResetDirtyProperties(); - } protected override void PersistUpdatedItem(ILanguage entity) { - if (entity.IsoCode.IsNullOrWhiteSpace() || entity.CultureInfo == null || entity.CultureName.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The required language data is missing"); + // validate iso code and culture name + if (entity.IsoCode.IsNullOrWhiteSpace() || entity.CultureName.IsNullOrWhiteSpace()) + throw new InvalidOperationException("Cannot save a language without an ISO code and a culture name."); - ((EntityBase)entity).UpdatingEntity(); + ((EntityBase) entity).UpdatingEntity(); if (entity.IsDefaultVariantLanguage) { - //if this entity is flagged as the default, we need to set all others to false - Database.Execute(Sql().Update(u => u.Set(x => x.IsDefaultVariantLanguage, false))); - //We need to clear the whole cache since all languages will be updated - IsolatedCache.ClearAllCache(); + // deal with entity becoming the new default entity + + // 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)); + Database.Execute(setAllDefaultToFalse); } - + else + { + // deal with the entity not being default anymore + // which is illegal - another entity has to become default + var selectDefaultId = Sql() + .Select(x => x.Id) + .From() + .Where(x => x.IsDefaultVariantLanguage); + + var defaultId = Database.ExecuteScalar(selectDefaultId); + if (entity.Id == defaultId) + throw new InvalidOperationException($"Cannot save the default language ({entity.IsoCode}) as non-default. Make another language the default language instead."); + } + + // update var dto = LanguageFactory.BuildDto(entity); - Database.Update(dto); - entity.ResetDirtyProperties(); - - //Clear the cache entries that exist by key/iso - IsolatedCache.ClearCacheItem(RepositoryCacheKeys.GetKey(entity.IsoCode)); - IsolatedCache.ClearCacheItem(RepositoryCacheKeys.GetKey(entity.CultureName)); } protected override void PersistDeletedItem(ILanguage entity) { - //we need to validate that we can delete this language - if (entity.IsDefaultVariantLanguage) - throw new InvalidOperationException($"Cannot delete the default language ({entity.IsoCode})"); + // validate that the entity is not the default language. + // safe (no race cond) because the service locks languages - var count = Database.ExecuteScalar(Sql().SelectCount().From()); - if (count == 1) - throw new InvalidOperationException($"Cannot delete the default language ({entity.IsoCode})"); + var selectDefaultId = Sql() + .Select(x => x.Id) + .From() + .Where(x => x.IsDefaultVariantLanguage); + var defaultId = Database.ExecuteScalar(selectDefaultId); + if (entity.Id == defaultId) + throw new InvalidOperationException($"Cannot delete the default language ({entity.IsoCode})."); + + // delete base.PersistDeletedItem(entity); - - //Clear the cache entries that exist by key/iso - IsolatedCache.ClearCacheItem(RepositoryCacheKeys.GetKey(entity.IsoCode)); - IsolatedCache.ClearCacheItem(RepositoryCacheKeys.GetKey(entity.CultureName)); } #endregion @@ -262,17 +270,20 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private ILanguage GetDefault() { // get all cached, non-cloned - var all = TypedCachePolicy.GetAllCached(PerformGetAll); + var languages = TypedCachePolicy.GetAllCached(PerformGetAll).ToList(); + var language = languages.FirstOrDefault(x => x.IsDefaultVariantLanguage); + if (language != null) return language; + + // this is an anomaly, the service/repo should ensure it cannot happen + Logger.Warn("There is no default language. Fix this anomaly by editing the language table in database and setting one language as the default language."); + + // still, don't kill the site, and return "something" ILanguage first = null; - foreach (var language in all) + foreach (var l in languages) { - // if one language is default, return - if (language.IsDefaultVariantLanguage) - return language; - // keep track of language with lowest id - if (first == null || language.Id < first.Id) - first = language; + if (first == null || l.Id < first.Id) + first = l; } return first; diff --git a/src/Umbraco.Core/Services/Implement/LocalizationService.cs b/src/Umbraco.Core/Services/Implement/LocalizationService.cs index 663ecf586c..e136cb5a68 100644 --- a/src/Umbraco.Core/Services/Implement/LocalizationService.cs +++ b/src/Umbraco.Core/Services/Implement/LocalizationService.cs @@ -360,6 +360,9 @@ namespace Umbraco.Core.Services.Implement { using (var scope = ScopeProvider.CreateScope()) { + // write-lock languages to guard against race conds when dealing with default language + scope.WriteLock(Constants.Locks.Languages); + var saveEventArgs = new SaveEventArgs(language); if (scope.Events.DispatchCancelable(SavingLanguage, this, saveEventArgs)) { @@ -386,6 +389,9 @@ namespace Umbraco.Core.Services.Implement { using (var scope = ScopeProvider.CreateScope()) { + // write-lock languages to guard against race conds when dealing with default language + scope.WriteLock(Constants.Locks.Languages); + var deleteEventArgs = new DeleteEventArgs(language); if (scope.Events.DispatchCancelable(DeletingLanguage, this, deleteEventArgs)) { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index f584621fc0..02bad84a31 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -355,6 +355,7 @@ + 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 afb5333ded..a46671fb56 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 @@ -122,9 +122,7 @@ } function toggleMandatory() { - if(!vm.language.isDefault) { - vm.language.isMandatory = !vm.language.isMandatory; - } + vm.language.isMandatory = !vm.language.isMandatory; } function toggleDefault() { @@ -136,7 +134,6 @@ vm.language.isDefault = !vm.language.isDefault; if(vm.language.isDefault) { - vm.language.isMandatory = true; vm.showDefaultLanguageInfo = true; } else { vm.showDefaultLanguageInfo = false; 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 6aaf915960..265fe0fd2b 100644 --- a/src/Umbraco.Web.UI.Client/src/views/languages/edit.html +++ b/src/Umbraco.Web.UI.Client/src/views/languages/edit.html @@ -47,7 +47,7 @@ -
+
diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index 60337ba3b1..fea307abb4 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -1708,7 +1708,7 @@ To manage your website, simply open the Umbraco back office and start adding con Mandatory Properties on this language has to be filled out before the node can be published. Default language - An Umbraco site can only have one default langugae set. + An Umbraco site can only have one default language set. Switching default language may result in default content missing. diff --git a/src/Umbraco.Web/Editors/LanguageController.cs b/src/Umbraco.Web/Editors/LanguageController.cs index 96019da702..8df19027a4 100644 --- a/src/Umbraco.Web/Editors/LanguageController.cs +++ b/src/Umbraco.Web/Editors/LanguageController.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Net; @@ -8,8 +7,6 @@ using System.Web.Http; using AutoMapper; using Umbraco.Core; using Umbraco.Core.Models; -using Umbraco.Core.Persistence; -using Umbraco.Web.Models; using Umbraco.Web.Mvc; using Umbraco.Web.WebApi; using Umbraco.Web.WebApi.Filters; @@ -56,27 +53,7 @@ namespace Umbraco.Web.Editors if (lang == null) throw new HttpResponseException(Request.CreateResponse(HttpStatusCode.NotFound)); - var model = Mapper.Map(lang); - - //if there's only one language, by default it is the default - var allLangs = Services.LocalizationService.GetAllLanguages().OrderBy(x => x.Id).ToList(); - if (!lang.IsDefaultVariantLanguage) - { - if (allLangs.Count == 1) - { - model.IsDefaultVariantLanguage = true; - model.Mandatory = true; - } - else if (allLangs.All(x => !x.IsDefaultVariantLanguage)) - { - //if no language has the default flag, then the defaul language is the one with the lowest id - model.IsDefaultVariantLanguage = allLangs[0].Id == lang.Id; - model.Mandatory = allLangs[0].Id == lang.Id; - } - } - - - return model; + return Mapper.Map(lang); } /// @@ -90,11 +67,10 @@ namespace Umbraco.Web.Editors var language = Services.LocalizationService.GetLanguageById(id); if (language == null) return NotFound(); - var totalLangs = Services.LocalizationService.GetAllLanguages().Count(); - - if (language.IsDefaultVariantLanguage || totalLangs == 1) + // the service would not let us do it, but test here nevertheless + if (language.IsDefaultVariantLanguage) { - var message = $"Language '{language.IsoCode}' is currently set to 'default' or it is the only installed language and can not be deleted."; + var message = $"Language '{language.IsoCode}' is currently set to 'default' and can not be deleted."; throw new HttpResponseException(Request.CreateNotificationValidationErrorResponse(message)); } @@ -113,16 +89,17 @@ namespace Umbraco.Web.Editors if (!ModelState.IsValid) throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); - var found = Services.LocalizationService.GetLanguageByIsoCode(language.IsoCode); + // this is prone to race conds but the service will not let us proceed anyways + var existing = Services.LocalizationService.GetLanguageByIsoCode(language.IsoCode); - if (found != null && language.Id != found.Id) + if (existing != null && language.Id != existing.Id) { - //someone is trying to create a language that alraedy exist + //someone is trying to create a language that already exist ModelState.AddModelError("IsoCode", "The language " + language.IsoCode + " already exists"); throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); } - if (found == null) + if (existing == null) { CultureInfo culture; try @@ -146,11 +123,20 @@ namespace Umbraco.Web.Editors return Mapper.Map(newLang); } - found.Mandatory = language.Mandatory; - found.IsDefaultVariantLanguage = language.IsDefaultVariantLanguage; - Services.LocalizationService.Save(found); - return Mapper.Map(found); - } - + existing.Mandatory = language.Mandatory; + + // note that the service will prevent the default language from being "un-defaulted" + // but does not hurt to test here - though the UI should prevent it too + if (existing.IsDefaultVariantLanguage && !language.IsDefaultVariantLanguage) + { + ModelState.AddModelError("IsDefaultVariantLanguage", "Cannot un-default the default language."); + throw new HttpResponseException(Request.CreateValidationErrorResponse(ModelState)); + } + + existing.IsDefaultVariantLanguage = language.IsDefaultVariantLanguage; + + Services.LocalizationService.Save(existing); + return Mapper.Map(existing); + } } }