From 861b883d2949ae7a7bf05ae6025c819dfc7600f1 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 14 Apr 2023 12:03:25 +0200 Subject: [PATCH] Throw from IUserIdKeyResolver if id/key not found. (#14101) * Throw from IUserIdKeyResolver if id/key not found. This is too critical to not throw. * Fixed tests * Explicitly test that we can resolve super user key/ID from their counterparts --------- Co-authored-by: kjac --- .../Services/ContentEditingService.cs | 2 +- .../Services/DataTypeContainerService.cs | 4 +-- src/Umbraco.Core/Services/DataTypeService.cs | 20 ++++++------- .../Services/DictionaryItemService.cs | 6 ++-- src/Umbraco.Core/Services/FileService.cs | 8 ++--- .../Services/IUserIdKeyResolver.cs | 4 +-- src/Umbraco.Core/Services/LanguageService.cs | 4 +-- .../Services/LocalizationService.cs | 8 ++--- .../Services/MediaEditingService.cs | 2 +- src/Umbraco.Core/Services/RelationService.cs | 4 +-- src/Umbraco.Core/Services/TemplateService.cs | 6 ++-- .../Services/Implement/UserIdKeyResolver.cs | 30 +++++++++---------- .../Packaging/PackageDataInstallationTests.cs | 10 +++---- .../Services/DataTypeContainerServiceTests.cs | 7 +++-- .../Services/LocalizationServiceTests.cs | 11 +++---- .../Services/UserIdKeyResolverTests.cs | 24 +++++++++++---- 16 files changed, 82 insertions(+), 68 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentEditingService.cs b/src/Umbraco.Core/Services/ContentEditingService.cs index ba2bb04c16..aa95c2fb0b 100644 --- a/src/Umbraco.Core/Services/ContentEditingService.cs +++ b/src/Umbraco.Core/Services/ContentEditingService.cs @@ -152,5 +152,5 @@ internal sealed class ContentEditingService } } - private async Task GetUserIdAsync(Guid userKey) => await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + private async Task GetUserIdAsync(Guid userKey) => await _userIdKeyResolver.GetAsync(userKey); } diff --git a/src/Umbraco.Core/Services/DataTypeContainerService.cs b/src/Umbraco.Core/Services/DataTypeContainerService.cs index 7687c4830c..29ae1a1dee 100644 --- a/src/Umbraco.Core/Services/DataTypeContainerService.cs +++ b/src/Umbraco.Core/Services/DataTypeContainerService.cs @@ -138,7 +138,7 @@ internal sealed class DataTypeContainerService : RepositoryService, IDataTypeCon _dataTypeContainerRepository.Delete(container); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Delete, currentUserId, container.Id); scope.Complete(); @@ -172,7 +172,7 @@ internal sealed class DataTypeContainerService : RepositoryService, IDataTypeCon _dataTypeContainerRepository.Save(container); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(auditType, currentUserId, container.Id); scope.Complete(); diff --git a/src/Umbraco.Core/Services/DataTypeService.cs b/src/Umbraco.Core/Services/DataTypeService.cs index 664be7f41a..b5cd8e976d 100644 --- a/src/Umbraco.Core/Services/DataTypeService.cs +++ b/src/Umbraco.Core/Services/DataTypeService.cs @@ -109,7 +109,7 @@ namespace Umbraco.Cms.Core.Services.Implement Key = key }; - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = _dataTypeContainerService.CreateAsync(container, parentKey, currentUserKey).GetAwaiter().GetResult(); // mimic old service behavior @@ -175,7 +175,7 @@ namespace Umbraco.Cms.Core.Services.Implement { var isNew = container.Id == 0; Guid? parentKey = isNew && container.ParentId > 0 ? _dataTypeContainerRepository.Get(container.ParentId)?.Key : null; - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = isNew ? _dataTypeContainerService.CreateAsync(container, parentKey, currentUserKey).GetAwaiter().GetResult() @@ -205,7 +205,7 @@ namespace Umbraco.Cms.Core.Services.Implement return OperationResult.Attempt.NoOperation(evtMsgs); } - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = _dataTypeContainerService.DeleteAsync(container.Key, currentUserKey).GetAwaiter().GetResult(); // mimic old service behavior return result.Status switch @@ -235,7 +235,7 @@ namespace Umbraco.Cms.Core.Services.Implement } container.Name = name; - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = _dataTypeContainerService.UpdateAsync(container, currentUserKey).GetAwaiter().GetResult(); // mimic old service behavior return result.Status switch @@ -423,7 +423,7 @@ namespace Umbraco.Cms.Core.Services.Implement scope.Notifications.Publish(new DataTypeMovedNotification(moveEventInfo, eventMessages).WithStateFrom(movingDataTypeNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ??Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Move, currentUserId, toMove.Id); scope.Complete(); } @@ -452,7 +452,7 @@ namespace Umbraco.Cms.Core.Services.Implement containerKey = container.Key; } - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = CopyAsync(copying, containerKey, currentUserKey).GetAwaiter().GetResult(); // mimic old service behavior @@ -505,7 +505,7 @@ namespace Umbraco.Cms.Core.Services.Implement throw new InvalidOperationException("Name cannot be more than 255 characters in length."); } - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); SaveAsync( dataType, @@ -593,7 +593,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// Optional Id of the user issuing the deletion public void Delete(IDataType dataType, int userId = Constants.Security.SuperUserId) { - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); DeleteAsync(dataType.Key, currentUserKey).GetAwaiter().GetResult(); } @@ -651,7 +651,7 @@ namespace Umbraco.Cms.Core.Services.Implement scope.Notifications.Publish(new DataTypeDeletedNotification(dataType, eventMessages).WithStateFrom(deletingDataTypeNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Delete, currentUserId, dataType.Id); scope.Complete(); @@ -710,7 +710,7 @@ namespace Umbraco.Cms.Core.Services.Implement EventMessages eventMessages = EventMessagesFactory.Get(); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); dataType.CreatorId = currentUserId; using ICoreScope scope = ScopeProvider.CreateCoreScope(); diff --git a/src/Umbraco.Core/Services/DictionaryItemService.cs b/src/Umbraco.Core/Services/DictionaryItemService.cs index d873e6015e..00d6b35104 100644 --- a/src/Umbraco.Core/Services/DictionaryItemService.cs +++ b/src/Umbraco.Core/Services/DictionaryItemService.cs @@ -167,7 +167,7 @@ internal sealed class DictionaryItemService : RepositoryService, IDictionaryItem new DictionaryItemDeletedNotification(dictionaryItem, eventMessages) .WithStateFrom(deletingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Delete, "Delete DictionaryItem", currentUserId, dictionaryItem.Id, nameof(DictionaryItem)); scope.Complete(); @@ -229,7 +229,7 @@ internal sealed class DictionaryItemService : RepositoryService, IDictionaryItem scope.Notifications.Publish( new DictionaryItemMovedNotification(moveEventInfo, eventMessages).WithStateFrom(movingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Move, "Move DictionaryItem", currentUserId, dictionaryItem.Id, nameof(DictionaryItem)); scope.Complete(); @@ -281,7 +281,7 @@ internal sealed class DictionaryItemService : RepositoryService, IDictionaryItem scope.Notifications.Publish( new DictionaryItemSavedNotification(dictionaryItem, eventMessages).WithStateFrom(savingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(auditType, auditMessage, currentUserId, dictionaryItem.Id, nameof(DictionaryItem)); scope.Complete(); diff --git a/src/Umbraco.Core/Services/FileService.cs b/src/Umbraco.Core/Services/FileService.cs index 2e0561074c..01a0521050 100644 --- a/src/Umbraco.Core/Services/FileService.cs +++ b/src/Umbraco.Core/Services/FileService.cs @@ -383,7 +383,7 @@ public class FileService : RepositoryService, IFileService throw new InvalidOperationException("Name cannot be more than 255 characters in length."); } - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = _templateService.CreateForContentTypeAsync(contentTypeAlias, contentTypeName, currentUserKey).GetAwaiter().GetResult(); // mimic old service behavior @@ -418,7 +418,7 @@ public class FileService : RepositoryService, IFileService throw new ArgumentOutOfRangeException(nameof(name), "Name cannot be more than 255 characters in length."); } - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = _templateService.CreateAsync(name, alias, content, currentUserKey).GetAwaiter().GetResult(); return result.Result; } @@ -495,7 +495,7 @@ public class FileService : RepositoryService, IFileService "Name cannot be null, empty, contain only white-space characters or be more than 255 characters in length."); } - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); if (template.Id > 0) { _templateService.UpdateAsync(template, currentUserKey).GetAwaiter().GetResult(); @@ -548,7 +548,7 @@ public class FileService : RepositoryService, IFileService [Obsolete("Please use ITemplateService for template operations - will be removed in Umbraco 15")] public void DeleteTemplate(string alias, int userId = Constants.Security.SuperUserId) { - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); _templateService.DeleteAsync(alias, currentUserKey).GetAwaiter().GetResult(); } diff --git a/src/Umbraco.Core/Services/IUserIdKeyResolver.cs b/src/Umbraco.Core/Services/IUserIdKeyResolver.cs index de963567e3..30c775edb6 100644 --- a/src/Umbraco.Core/Services/IUserIdKeyResolver.cs +++ b/src/Umbraco.Core/Services/IUserIdKeyResolver.cs @@ -7,12 +7,12 @@ public interface IUserIdKeyResolver /// /// The key of the user. /// The id of the user, null if the user doesn't exist. - public Task GetAsync(Guid key); + public Task GetAsync(Guid key); /// /// Tries to resolve a user id to a user key without fetching the entire user. /// /// The id of the user. /// The key of the user, null if the user doesn't exist. - public Task GetAsync(int id); + public Task GetAsync(int id); } diff --git a/src/Umbraco.Core/Services/LanguageService.cs b/src/Umbraco.Core/Services/LanguageService.cs index 50762ce408..fbd3c84ff8 100644 --- a/src/Umbraco.Core/Services/LanguageService.cs +++ b/src/Umbraco.Core/Services/LanguageService.cs @@ -144,7 +144,7 @@ internal sealed class LanguageService : RepositoryService, ILanguageService scope.Notifications.Publish( new LanguageDeletedNotification(language, eventMessages).WithStateFrom(deletingLanguageNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Delete, "Delete Language", currentUserId, language.Id, UmbracoObjectTypes.Language.GetName()); scope.Complete(); return await Task.FromResult(Attempt.SucceedWithStatus(LanguageOperationStatus.Success, language)); @@ -197,7 +197,7 @@ internal sealed class LanguageService : RepositoryService, ILanguageService scope.Notifications.Publish( new LanguageSavedNotification(language, eventMessages).WithStateFrom(savingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(auditType, auditMessage, currentUserId, language.Id, UmbracoObjectTypes.Language.GetName()); scope.Complete(); diff --git a/src/Umbraco.Core/Services/LocalizationService.cs b/src/Umbraco.Core/Services/LocalizationService.cs index e799913411..e1fcfab220 100644 --- a/src/Umbraco.Core/Services/LocalizationService.cs +++ b/src/Umbraco.Core/Services/LocalizationService.cs @@ -221,7 +221,7 @@ internal class LocalizationService : RepositoryService, ILocalizationService [Obsolete("Please use IDictionaryItemService for dictionary item operations. Will be removed in V15.")] public void Save(IDictionaryItem dictionaryItem, int userId = Constants.Security.SuperUserId) { ; - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); if (dictionaryItem.Id > 0) { _dictionaryItemService.UpdateAsync(dictionaryItem, currentUserKey).GetAwaiter().GetResult(); @@ -241,7 +241,7 @@ internal class LocalizationService : RepositoryService, ILocalizationService [Obsolete("Please use IDictionaryItemService for dictionary item operations. Will be removed in V15.")] public void Delete(IDictionaryItem dictionaryItem, int userId = Constants.Security.SuperUserId) { - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); _dictionaryItemService.DeleteAsync(dictionaryItem.Key, currentUserKey).GetAwaiter().GetResult(); } @@ -321,7 +321,7 @@ internal class LocalizationService : RepositoryService, ILocalizationService [Obsolete("Please use ILanguageService for language operations. Will be removed in V15.")] public void Save(ILanguage language, int userId = Constants.Security.SuperUserId) { - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); Attempt result = language.Id > 0 ? _languageService.UpdateAsync(language, currentUserKey).GetAwaiter().GetResult() : _languageService.CreateAsync(language, currentUserKey).GetAwaiter().GetResult(); @@ -341,7 +341,7 @@ internal class LocalizationService : RepositoryService, ILocalizationService [Obsolete("Please use ILanguageService for language operations. Will be removed in V15.")] public void Delete(ILanguage language, int userId = Constants.Security.SuperUserId) { - Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult() ?? Constants.Security.SuperUserKey; + Guid currentUserKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult(); _languageService.DeleteAsync(language.IsoCode, currentUserKey).GetAwaiter().GetResult(); } diff --git a/src/Umbraco.Core/Services/MediaEditingService.cs b/src/Umbraco.Core/Services/MediaEditingService.cs index 67fda1c859..533e863e1c 100644 --- a/src/Umbraco.Core/Services/MediaEditingService.cs +++ b/src/Umbraco.Core/Services/MediaEditingService.cs @@ -109,5 +109,5 @@ internal sealed class MediaEditingService } } - private async Task GetUserIdAsync(Guid userKey) => await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + private async Task GetUserIdAsync(Guid userKey) => await _userIdKeyResolver.GetAsync(userKey); } diff --git a/src/Umbraco.Core/Services/RelationService.cs b/src/Umbraco.Core/Services/RelationService.cs index 200cbb37d6..7111015fc6 100644 --- a/src/Umbraco.Core/Services/RelationService.cs +++ b/src/Umbraco.Core/Services/RelationService.cs @@ -622,7 +622,7 @@ public class RelationService : RepositoryService, IRelationService } _relationTypeRepository.Save(relationType); - var currentUser = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUser = await _userIdKeyResolver.GetAsync(userKey); Audit(auditType, currentUser, relationType.Id, auditMessage); scope.Complete(); scope.Notifications.Publish( @@ -691,7 +691,7 @@ public class RelationService : RepositoryService, IRelationService } _relationTypeRepository.Delete(relationType); - var currentUser = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUser = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Delete, currentUser, relationType.Id, "Deleted relation type"); scope.Notifications.Publish(new RelationTypeDeletedNotification(relationType, eventMessages).WithStateFrom(deletingNotification)); scope.Complete(); diff --git a/src/Umbraco.Core/Services/TemplateService.cs b/src/Umbraco.Core/Services/TemplateService.cs index e412020ff3..fdf950567b 100644 --- a/src/Umbraco.Core/Services/TemplateService.cs +++ b/src/Umbraco.Core/Services/TemplateService.cs @@ -100,7 +100,7 @@ public class TemplateService : RepositoryService, ITemplateService scope.Notifications.Publish( new TemplateSavedNotification(template, eventMessages).WithStateFrom(savingEvent)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.New, currentUserId, template.Id, UmbracoObjectTypes.Template.GetName()); scope.Complete(); } @@ -239,7 +239,7 @@ public class TemplateService : RepositoryService, ITemplateService scope.Notifications.Publish( new TemplateSavedNotification(template, eventMessages).WithStateFrom(savingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(auditType, currentUserId, template.Id, UmbracoObjectTypes.Template.GetName()); scope.Complete(); return Attempt.SucceedWithStatus(TemplateOperationStatus.Success, template); @@ -386,7 +386,7 @@ public class TemplateService : RepositoryService, ITemplateService scope.Notifications.Publish( new TemplateDeletedNotification(template, eventMessages).WithStateFrom(deletingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey) ?? Constants.Security.SuperUserId; + var currentUserId = await _userIdKeyResolver.GetAsync(userKey); Audit(AuditType.Delete, currentUserId, template.Id, UmbracoObjectTypes.Template.GetName()); scope.Complete(); return Attempt.SucceedWithStatus(TemplateOperationStatus.Success, template); diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs b/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs index a4054808d9..b8a3c15553 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs @@ -13,17 +13,17 @@ namespace Umbraco.Cms.Infrastructure.Services.Implement; internal sealed class UserIdKeyResolver : IUserIdKeyResolver { private readonly IScopeProvider _scopeProvider; - private readonly ConcurrentDictionary _keyToId = new(); - private readonly ConcurrentDictionary _idToKey = new(); + private readonly ConcurrentDictionary _keyToId = new(); + private readonly ConcurrentDictionary _idToKey = new(); private readonly SemaphoreSlim _keytToIdLock = new(1, 1); private readonly SemaphoreSlim _idToKeyLock = new(1, 1); public UserIdKeyResolver(IScopeProvider scopeProvider) => _scopeProvider = scopeProvider; /// - public async Task GetAsync(Guid key) + public async Task GetAsync(Guid key) { - if (_keyToId.TryGetValue(key, out int? id)) + if (_keyToId.TryGetValue(key, out int id)) { return id; } @@ -33,7 +33,7 @@ internal sealed class UserIdKeyResolver : IUserIdKeyResolver await _keytToIdLock.WaitAsync(); try { - if (_keyToId.TryGetValue(key, out int? recheckedId)) + if (_keyToId.TryGetValue(key, out int recheckedId)) { // It was added while we were waiting, so we'll just return it return recheckedId; @@ -48,7 +48,9 @@ internal sealed class UserIdKeyResolver : IUserIdKeyResolver .From() .Where(x => x.Key == key); - int? fetchedId = await scope.Database.ExecuteScalarAsync(query); + int fetchedId = (await scope.Database.ExecuteScalarAsync(query)) + ?? throw new InvalidOperationException("No user found with the specified key"); + _keyToId[key] = fetchedId; return fetchedId; @@ -60,9 +62,9 @@ internal sealed class UserIdKeyResolver : IUserIdKeyResolver } /// - public async Task GetAsync(int id) + public async Task GetAsync(int id) { - if (_idToKey.TryGetValue(id, out Guid? key)) + if (_idToKey.TryGetValue(id, out Guid key)) { return key; } @@ -70,7 +72,7 @@ internal sealed class UserIdKeyResolver : IUserIdKeyResolver await _idToKeyLock.WaitAsync(); try { - if (_idToKey.TryGetValue(id, out Guid? recheckedKey)) + if (_idToKey.TryGetValue(id, out Guid recheckedKey)) { return recheckedKey; } @@ -84,13 +86,11 @@ internal sealed class UserIdKeyResolver : IUserIdKeyResolver .Where(x => x.Id == id); string? guidString = await scope.Database.ExecuteScalarAsync(query); - Guid? fetchedKey = guidString is null ? null : new Guid(guidString); + Guid fetchedKey = guidString is not null + ? new Guid(guidString) + : throw new InvalidOperationException("No user found with the specified id"); - // For ids we don't want to cache the null value, since unlike keys, it's pretty likely that we'll see collision - if (fetchedKey is not null) - { - _idToKey[id] = fetchedKey; - } + _idToKey[id] = fetchedKey; return fetchedKey; } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Packaging/PackageDataInstallationTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Packaging/PackageDataInstallationTests.cs index f93441ece7..c0a114b136 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Packaging/PackageDataInstallationTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Packaging/PackageDataInstallationTests.cs @@ -577,7 +577,7 @@ public class PackageDataInstallationTests : UmbracoIntegrationTestWithContent await AddLanguages(); // Act - PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), 0); + PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), Constants.Security.SuperUserId); // Assert await AssertDictionaryItem("Parent", expectedEnglishParentValue, "en-GB"); @@ -600,7 +600,7 @@ public class PackageDataInstallationTests : UmbracoIntegrationTestWithContent // Act var dictionaryItems = - PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), 0); + PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), Constants.Security.SuperUserId); // Assert Assert.That(await DictionaryItemService.ExistsAsync(parentKey), "DictionaryItem parentKey does not exist"); @@ -629,7 +629,7 @@ public class PackageDataInstallationTests : UmbracoIntegrationTestWithContent await AddExistingEnglishAndNorwegianParentDictionaryItem(expectedEnglishParentValue, expectedNorwegianParentValue); // Act - PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), 0); + PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), Constants.Security.SuperUserId); // Assert await AssertDictionaryItem("Parent", expectedEnglishParentValue, "en-GB"); @@ -654,7 +654,7 @@ public class PackageDataInstallationTests : UmbracoIntegrationTestWithContent await AddExistingEnglishParentDictionaryItem(expectedEnglishParentValue); // Act - PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), 0); + PackageDataInstallation.ImportDictionaryItems(dictionaryItemsElement.Elements("DictionaryItem"), Constants.Security.SuperUserId); // Assert await AssertDictionaryItem("Parent", expectedEnglishParentValue, "en-GB"); @@ -671,7 +671,7 @@ public class PackageDataInstallationTests : UmbracoIntegrationTestWithContent var languageItemsElement = newPackageXml.Elements("Languages").First(); // Act - var languages = PackageDataInstallation.ImportLanguages(languageItemsElement.Elements("Language"), 0); + var languages = PackageDataInstallation.ImportLanguages(languageItemsElement.Elements("Language"), Constants.Security.SuperUserId); var allLanguages = await LanguageService.GetAllAsync(); // Assert diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/DataTypeContainerServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/DataTypeContainerServiceTests.cs index 473974843d..dbb7012d89 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/DataTypeContainerServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/DataTypeContainerServiceTests.cs @@ -147,13 +147,14 @@ public class DataTypeContainerServiceTests : UmbracoIntegrationTest [Test] public async Task Can_Delete_Child_Container() { + Guid userKey = Constants.Security.SuperUserKey; EntityContainer root = new EntityContainer(Constants.ObjectTypes.DataType) { Name = "Root Container" }; - await DataTypeContainerService.CreateAsync(root, null, Constants.Security.SuperUserKey); + await DataTypeContainerService.CreateAsync(root, null, userKey); EntityContainer child = new EntityContainer(Constants.ObjectTypes.DataType) { Name = "Child Container" }; - await DataTypeContainerService.CreateAsync(child, null, root.Key); + await DataTypeContainerService.CreateAsync(child, root.Key, userKey); - var result = await DataTypeContainerService.DeleteAsync(child.Key, Constants.Security.SuperUserKey); + var result = await DataTypeContainerService.DeleteAsync(child.Key, userKey); Assert.IsTrue(result.Success); Assert.AreEqual(DataTypeContainerOperationStatus.Success, result.Status); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/LocalizationServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/LocalizationServiceTests.cs index 8c5e8c371f..26a8530358 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/LocalizationServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/LocalizationServiceTests.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using NUnit.Framework; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence; @@ -207,7 +208,7 @@ public class LocalizationServiceTests : UmbracoIntegrationTest var languageNbNo = new LanguageBuilder() .WithCultureInfo("nb-NO") .Build(); - LocalizationService.Save(languageNbNo, 0); + LocalizationService.Save(languageNbNo, Constants.Security.SuperUserId); Assert.That(languageNbNo.HasIdentity, Is.True); var languageId = languageNbNo.Id; @@ -225,7 +226,7 @@ public class LocalizationServiceTests : UmbracoIntegrationTest .WithCultureInfo("nb-NO") .WithFallbackLanguageIsoCode(languageDaDk.IsoCode) .Build(); - LocalizationService.Save(languageNbNo, 0); + LocalizationService.Save(languageNbNo, Constants.Security.SuperUserId); var languageId = languageDaDk.Id; LocalizationService.Delete(languageDaDk); @@ -443,8 +444,8 @@ public class LocalizationServiceTests : UmbracoIntegrationTest .WithCultureInfo("en-GB") .Build(); - LocalizationService.Save(languageDaDk, 0); - LocalizationService.Save(languageEnGb, 0); + LocalizationService.Save(languageDaDk, Constants.Security.SuperUserId); + LocalizationService.Save(languageEnGb, Constants.Security.SuperUserId); _danishLangId = languageDaDk.Id; _englishLangId = languageEnGb.Id; @@ -472,4 +473,4 @@ public class LocalizationServiceTests : UmbracoIntegrationTest _childItemGuidId = childItem.Key; _childItemIntId = childItem.Id; } -} \ No newline at end of file +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs index 7ad9b83b80..cfcc52fa8f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserIdKeyResolverTests.cs @@ -61,16 +61,28 @@ public class UserIdKeyResolverTests : UmbracoIntegrationTest } [Test] - public async Task Unknown_Key_Resolves_To_Null() + public async Task Can_Resolve_Super_User_Key_To_Id() { - var resolvedId = await UserIdKeyResolver.GetAsync(Guid.NewGuid()); - Assert.IsNull(resolvedId); + var resolvedId = await UserIdKeyResolver.GetAsync(Constants.Security.SuperUserKey); + Assert.AreEqual(Constants.Security.SuperUserId, resolvedId); } [Test] - public async Task Unknown_Id_Resolves_To_Null() + public async Task Can_Resolve_Super_User_Id_To_Key() { - var resolvedKey = await UserIdKeyResolver.GetAsync(1234567890); - Assert.IsNull(resolvedKey); + var resolvedKey = await UserIdKeyResolver.GetAsync(Constants.Security.SuperUserId); + Assert.AreEqual(Constants.Security.SuperUserKey, resolvedKey); + } + + [Test] + public async Task Unknown_Key_Throws() + { + Assert.ThrowsAsync(async () => await UserIdKeyResolver.GetAsync(Guid.NewGuid())); + } + + [Test] + public async Task Unknown_Id_Throws() + { + Assert.ThrowsAsync(async () => await UserIdKeyResolver.GetAsync(1234567890)); } }