Fixed a couple of occurrences where scopes was auto-completed while modified db state (#14947)

* Fixed a couple of occurrences where scopes was auto-complated while actually modified the state of the database.

* Added temp ctor to fix boot issue
This commit is contained in:
Bjarke Berg
2023-10-10 11:51:47 +02:00
committed by GitHub
parent ccd54bfb0e
commit 41f8f03c2c
17 changed files with 145 additions and 41 deletions

View File

@@ -372,7 +372,7 @@ public class ContentService : RepositoryService, IContentService
public IContent CreateAndSave(string name, int parentId, string contentTypeAlias, int userId = Constants.Security.SuperUserId)
{
// TODO: what about culture?
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
// locking the content tree secures content types too
scope.WriteLock(Constants.Locks.ContentTree);
@@ -395,6 +395,8 @@ public class ContentService : RepositoryService, IContentService
Save(content, userId);
scope.Complete();
return content;
}
}
@@ -416,7 +418,7 @@ public class ContentService : RepositoryService, IContentService
throw new ArgumentNullException(nameof(parent));
}
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
// locking the content tree secures content types too
scope.WriteLock(Constants.Locks.ContentTree);
@@ -431,6 +433,7 @@ public class ContentService : RepositoryService, IContentService
Save(content, userId);
scope.Complete();
return content;
}
}
@@ -508,10 +511,11 @@ public class ContentService : RepositoryService, IContentService
/// <inheritdoc />
public void PersistContentSchedule(IContent content, ContentScheduleCollection contentSchedule)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.ContentTree);
_documentRepository.PersistContentSchedule(content, contentSchedule);
scope.Complete();
}
}
@@ -2978,7 +2982,7 @@ public class ContentService : RepositoryService, IContentService
public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.ContentTree);
@@ -2991,6 +2995,8 @@ public class ContentService : RepositoryService, IContentService
scope.Notifications.Publish(new ContentTreeChangeNotification(root, TreeChangeTypes.RefreshAll, EventMessagesFactory.Get()));
}
scope.Complete();
return report;
}
}

View File

@@ -322,7 +322,6 @@ public abstract class ContentTypeServiceBase<TRepository, TItem> : ContentTypeSe
}
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
scope.ReadLock(ReadLockIds);
return Repository.GetMany(ids.ToArray());

View File

@@ -68,7 +68,7 @@ internal class ContentVersionService : IContentVersionService
/// <inheritdoc />
public void SetPreventCleanup(int versionId, bool preventCleanup, int userId = Constants.Security.SuperUserId)
{
using (ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.ContentTree);
_documentVersionRepository.SetPreventCleanup(versionId, preventCleanup);
@@ -87,6 +87,7 @@ internal class ContentVersionService : IContentVersionService
var message = $"set preventCleanup = '{preventCleanup}' for version '{versionId}'";
Audit(auditType, userId, version.ContentId, message, $"{version.VersionDate}");
scope.Complete();
}
}
@@ -120,7 +121,7 @@ internal class ContentVersionService : IContentVersionService
*
* tl;dr lots of scopes to enable other connections to use the DB whilst we work.
*/
using (ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
IReadOnlyCollection<ContentVersionMeta>? allHistoricVersions =
_documentVersionRepository.GetDocumentVersionsEligibleForCleanup();
@@ -154,6 +155,8 @@ internal class ContentVersionService : IContentVersionService
versionsToDelete.Add(version);
}
scope.Complete();
}
if (!versionsToDelete.Any())
@@ -169,7 +172,7 @@ internal class ContentVersionService : IContentVersionService
foreach (IEnumerable<ContentVersionMeta> group in versionsToDelete.InGroupsOf(Constants.Sql.MaxParameterCount))
{
using (ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.ContentTree);
var groupEnumerated = group.ToList();
@@ -182,12 +185,16 @@ internal class ContentVersionService : IContentVersionService
scope.Notifications.Publish(
new ContentDeletedVersionsNotification(version.ContentId, messages, version.VersionId));
}
scope.Complete();
}
}
using (_scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
Audit(AuditType.Delete, Constants.Security.SuperUserId, -1, $"Removed {versionsToDelete.Count} ContentVersion(s) according to cleanup policy");
scope.Complete();
}
return versionsToDelete;

View File

@@ -608,7 +608,7 @@ namespace Umbraco.Cms.Core.Services.Implement
public IReadOnlyDictionary<Udi, IEnumerable<string>> GetReferences(int id)
{
using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete:true);
using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
return _dataTypeRepository.FindUsages(id);
}

View File

@@ -33,7 +33,7 @@ public class DefaultContentVersionCleanupPolicy : IContentVersionCleanupPolicy
var theRest = new List<ContentVersionMeta>();
using (_scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
var policyOverrides = _documentVersionRepository.GetCleanupPolicies()?
.ToDictionary(x => x.ContentTypeId);
@@ -77,6 +77,8 @@ public class DefaultContentVersionCleanupPolicy : IContentVersionCleanupPolicy
yield return version;
}
}
scope.Complete();
}
}

View File

@@ -1197,7 +1197,7 @@ namespace Umbraco.Cms.Core.Services
public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.MediaTree);
@@ -1210,6 +1210,7 @@ namespace Umbraco.Cms.Core.Services
scope.Notifications.Publish(new MediaTreeChangeNotification(root, TreeChangeTypes.RefreshAll, EventMessagesFactory.Get()));
}
scope.Complete();
return report;
}
}

View File

@@ -42,8 +42,10 @@ public class TwoFactorLoginService : ITwoFactorLoginService
/// <inheritdoc />
public async Task DeleteUserLoginsAsync(Guid userOrMemberKey)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
await _twoFactorLoginRepository.DeleteUserLoginsAsync(userOrMemberKey);
scope.Complete();
}
/// <inheritdoc />
@@ -138,8 +140,12 @@ public class TwoFactorLoginService : ITwoFactorLoginService
/// <inheritdoc />
public async Task<bool> DisableAsync(Guid userOrMemberKey, string providerName)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
return await _twoFactorLoginRepository.DeleteUserLoginsAsync(userOrMemberKey, providerName);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
var result = await _twoFactorLoginRepository.DeleteUserLoginsAsync(userOrMemberKey, providerName);
scope.Complete();
return result;
}
/// <inheritdoc />
@@ -156,9 +162,10 @@ public class TwoFactorLoginService : ITwoFactorLoginService
/// <inheritdoc />
public Task SaveAsync(TwoFactorLogin twoFactorLogin)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
_twoFactorLoginRepository.Save(twoFactorLogin);
scope.Complete();
return Task.CompletedTask;
}

View File

@@ -106,7 +106,7 @@ public class ScheduledPublishing : RecurringHostedServiceBase
// but then what should be its "scope"? could we attach it to scopes?
// - and we should definitively *not* have to flush it here (should be auto)
using UmbracoContextReference contextReference = _umbracoContextFactory.EnsureUmbracoContext();
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
/* We used to assume that there will never be two instances running concurrently where (IsMainDom && ServerRole == SchedulingPublisher)
* However this is possible during an azure deployment slot swap for the SchedulingPublisher instance when trying to achieve zero downtime deployments.
@@ -125,6 +125,8 @@ public class ScheduledPublishing : RecurringHostedServiceBase
grouped.Count(),
grouped.Key);
}
scope.Complete();
}
finally
{

View File

@@ -1,6 +1,8 @@
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Infrastructure.Scoping;
using IScope = Umbraco.Cms.Infrastructure.Scoping.IScope;
namespace Umbraco.Cms.Core.Logging.Viewer;
@@ -10,6 +12,21 @@ public class LogViewerConfig : ILogViewerConfig
private readonly ILogViewerQueryRepository _logViewerQueryRepository;
private readonly IScopeProvider _scopeProvider;
[Obsolete("Use non-obsolete ctor. This will be removed in Umbraco 14.")]
public LogViewerConfig(ILogViewerQueryRepository logViewerQueryRepository, Umbraco.Cms.Core.Scoping.IScopeProvider scopeProvider)
: this(logViewerQueryRepository, StaticServiceProvider.Instance.GetRequiredService<IScopeProvider>())
{
}
//Temp ctor used by MSDI (Greedy)
[Obsolete("Use non-obsolete ctor. This will be removed in Umbraco 14.")]
public LogViewerConfig(ILogViewerQueryRepository logViewerQueryRepository, Umbraco.Cms.Core.Scoping.IScopeProvider coreScopeProvider, IScopeProvider scopeProvider)
: this(logViewerQueryRepository, scopeProvider)
{
}
public LogViewerConfig(ILogViewerQueryRepository logViewerQueryRepository, IScopeProvider scopeProvider)
{
_logViewerQueryRepository = logViewerQueryRepository;
@@ -26,9 +43,10 @@ public class LogViewerConfig : ILogViewerConfig
public IReadOnlyList<SavedLogSearch> AddSavedSearch(string name, string query)
{
using IScope scope = _scopeProvider.CreateScope(autoComplete: true);
using IScope scope = _scopeProvider.CreateScope();
_logViewerQueryRepository.Save(new LogViewerQuery(name, query));
scope.Complete();
return GetSavedSearches();
}
@@ -37,7 +55,7 @@ public class LogViewerConfig : ILogViewerConfig
public IReadOnlyList<SavedLogSearch> DeleteSavedSearch(string name)
{
using IScope scope = _scopeProvider.CreateScope(autoComplete: true);
using IScope scope = _scopeProvider.CreateScope();
ILogViewerQuery? item = _logViewerQueryRepository.GetByName(name);
if (item is not null)
@@ -46,6 +64,8 @@ public class LogViewerConfig : ILogViewerConfig
}
// Return the updated object - so we can instantly reset the entire array from the API response
return GetSavedSearches();
IReadOnlyList<SavedLogSearch> result = GetSavedSearches();
scope.Complete();
return result;
}
}

View File

@@ -96,7 +96,7 @@ public class MemberUserStore : UmbracoUserStore<MemberIdentityUser, UmbracoIdent
throw new ArgumentNullException(nameof(user));
}
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
// create member
IMember memberEntity = _memberService.CreateMember(
@@ -150,6 +150,7 @@ public class MemberUserStore : UmbracoUserStore<MemberIdentityUser, UmbracoIdent
x.Value)));
}
scope.Complete();
return Task.FromResult(IdentityResult.Success);
}
catch (Exception ex)
@@ -179,7 +180,7 @@ public class MemberUserStore : UmbracoUserStore<MemberIdentityUser, UmbracoIdent
throw new InvalidOperationException("The user id must be an integer to work with the Umbraco");
}
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
IMember? found = _memberService.GetById(asInt);
if (found != null)
@@ -220,6 +221,7 @@ public class MemberUserStore : UmbracoUserStore<MemberIdentityUser, UmbracoIdent
}
}
scope.Complete();
return Task.FromResult(IdentityResult.Success);
}
catch (Exception ex)

View File

@@ -102,7 +102,7 @@ public class UmbProfileController : SurfaceController
private async Task<IdentityResult> UpdateMemberAsync(ProfileModel model, MemberIdentityUser currentMember)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
currentMember.Email = model.Email;
currentMember.Name = model.Name;
@@ -140,6 +140,7 @@ public class UmbProfileController : SurfaceController
_memberService.Save(member);
scope.Complete();
return saveResult;
}
}

View File

@@ -118,7 +118,7 @@ public class UmbRegisterController : SurfaceController
/// <returns>Result of registration operation.</returns>
private async Task<IdentityResult> RegisterMemberAsync(RegisterModel model)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();
// U4-10762 Server error with "Register Member" snippet (Cannot save member with empty name)
// If name field is empty, add the email address instead.
@@ -160,6 +160,8 @@ public class UmbRegisterController : SurfaceController
}
}
scope.Complete();
return identityResult;
}
}

View File

@@ -7,6 +7,7 @@ using System.Reflection;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Moq;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
@@ -14,6 +15,8 @@ using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Diagnostics;
using Umbraco.Cms.Core.DistributedLocking;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Hosting;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Logging;
@@ -24,6 +27,8 @@ using Umbraco.Cms.Core.Runtime;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.SqlSyntax;
using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Cms.Infrastructure.Serialization;
using Umbraco.Cms.Tests.Common.TestHelpers;
using Umbraco.Extensions;
@@ -76,6 +81,61 @@ public abstract class TestHelperBase
public IShortStringHelper ShortStringHelper { get; } =
new DefaultShortStringHelper(new DefaultShortStringHelperConfig());
public IScopeProvider ScopeProvider
{
get
{
var loggerFactory = NullLoggerFactory.Instance;
var fileSystems = new FileSystems(
loggerFactory,
Mock.Of<IIOHelper>(),
Mock.Of<IOptions<GlobalSettings>>(),
Mock.Of<IHostingEnvironment>());
var mediaFileManager = new MediaFileManager(
Mock.Of<IFileSystem>(),
Mock.Of<IMediaPathScheme>(),
loggerFactory.CreateLogger<MediaFileManager>(),
Mock.Of<IShortStringHelper>(),
Mock.Of<IServiceProvider>(),
Options.Create(new ContentSettings()));
var databaseFactory = new Mock<IUmbracoDatabaseFactory>();
var database = new Mock<IUmbracoDatabase>();
var sqlContext = new Mock<ISqlContext>();
var lockingMechanism = new Mock<IDistributedLockingMechanism>();
lockingMechanism.Setup(x => x.ReadLock(It.IsAny<int>(), It.IsAny<TimeSpan?>()))
.Returns(Mock.Of<IDistributedLock>());
lockingMechanism.Setup(x => x.WriteLock(It.IsAny<int>(), It.IsAny<TimeSpan?>()))
.Returns(Mock.Of<IDistributedLock>());
var lockingMechanismFactory = new Mock<IDistributedLockingMechanismFactory>();
lockingMechanismFactory.Setup(x => x.DistributedLockingMechanism)
.Returns(lockingMechanism.Object);
// Setup mock of database factory to return mock of database.
databaseFactory.Setup(x => x.CreateDatabase()).Returns(database.Object);
databaseFactory.Setup(x => x.SqlContext).Returns(sqlContext.Object);
// Setup mock of database to return mock of sql SqlContext
database.Setup(x => x.SqlContext).Returns(sqlContext.Object);
var syntaxProviderMock = new Mock<ISqlSyntaxProvider>();
// Setup mock of ISqlContext to return syntaxProviderMock
sqlContext.Setup(x => x.SqlSyntax).Returns(syntaxProviderMock.Object);
return new ScopeProvider(
new AmbientScopeStack(),
new AmbientScopeContextStack(),
lockingMechanismFactory.Object,
databaseFactory.Object,
fileSystems,
new TestOptionsMonitor<CoreDebugSettings>(new CoreDebugSettings()),
mediaFileManager,
loggerFactory,
Mock.Of<IEventAggregator>());
}
}
public IJsonSerializer JsonSerializer { get; } = new JsonNetSerializer();

View File

@@ -31,6 +31,7 @@ using Umbraco.Cms.Core.Net;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Runtime;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Infrastructure.Mail;
@@ -42,6 +43,7 @@ using Umbraco.Cms.Tests.Common.Testing;
using Umbraco.Extensions;
using File = System.IO.File;
using IHostingEnvironment = Umbraco.Cms.Core.Hosting.IHostingEnvironment;
using IScopeProvider = Umbraco.Cms.Infrastructure.Scoping.IScopeProvider;
namespace Umbraco.Cms.Tests.UnitTests.TestHelpers;
@@ -58,6 +60,8 @@ public static class TestHelper
/// <value>The assembly directory.</value>
public static string WorkingDirectory => s_testHelperInternal.WorkingDirectory;
public static IScopeProvider ScopeProvider => s_testHelperInternal.ScopeProvider;
public static ICoreScopeProvider CoreScopeProvider => s_testHelperInternal.ScopeProvider;
public static IShortStringHelper ShortStringHelper => s_testHelperInternal.ShortStringHelper;
public static IJsonSerializer JsonSerializer => s_testHelperInternal.JsonSerializer;

View File

@@ -48,7 +48,7 @@ public class LogviewerTests
File.Copy(exampleLogfilePath, _newLogfilePath, true);
var logger = Mock.Of<ILogger<SerilogJsonLogViewer>>();
var logViewerConfig = new LogViewerConfig(LogViewerQueryRepository, Mock.Of<IScopeProvider>());
var logViewerConfig = new LogViewerConfig(LogViewerQueryRepository, TestHelper.ScopeProvider);
var logLevelLoader = Mock.Of<ILogLevelLoader>();
_logViewer =
new SerilogJsonLogViewer(logger, logViewerConfig, loggingConfiguration, logLevelLoader, Log.Logger);

View File

@@ -17,6 +17,7 @@ using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Cms.Tests.Common;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Builders.Extensions;
using Umbraco.Cms.Tests.UnitTests.TestHelpers;
using Umbraco.Cms.Web.Common.Security;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security;
@@ -33,7 +34,7 @@ public class MemberManagerTests
public MemberManager CreateSut()
{
var scopeProvider = new Mock<IScopeProvider>().Object;
var scopeProvider = TestHelper.ScopeProvider;
_mockMemberService = new Mock<IMemberService>();
var mapDefinitions = new List<IMapDefinition>

View File

@@ -14,6 +14,7 @@ using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Tests.UnitTests.TestHelpers;
using Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper;
using IScopeProvider = Umbraco.Cms.Infrastructure.Scoping.IScopeProvider;
@@ -27,23 +28,12 @@ public class MemberUserStoreTests
public MemberUserStore CreateSut()
{
_mockMemberService = new Mock<IMemberService>();
var mockScope = new Mock<IScope>();
var mockScopeProvider = new Mock<IScopeProvider>();
mockScopeProvider
.Setup(x => x.CreateScope(
It.IsAny<IsolationLevel>(),
It.IsAny<RepositoryCacheMode>(),
It.IsAny<IEventDispatcher>(),
It.IsAny<IScopedNotificationPublisher>(),
It.IsAny<bool?>(),
It.IsAny<bool>(),
It.IsAny<bool>()))
.Returns(mockScope.Object);
var mockScopeProvider = TestHelper.ScopeProvider;
return new MemberUserStore(
_mockMemberService.Object,
new UmbracoMapper(new MapDefinitionCollection(() => new List<IMapDefinition>()), mockScopeProvider.Object, NullLogger<UmbracoMapper>.Instance),
mockScopeProvider.Object,
new UmbracoMapper(new MapDefinitionCollection(() => new List<IMapDefinition>()), mockScopeProvider, NullLogger<UmbracoMapper>.Instance),
mockScopeProvider,
new IdentityErrorDescriber(),
Mock.Of<IPublishedSnapshotAccessor>(),
Mock.Of<IExternalLoginWithKeyService>(),