Skip to content

Commit

Permalink
Fixed a couple of occurrences where scopes was auto-completed while m…
Browse files Browse the repository at this point in the history
…odified 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

(cherry picked from commit 41f8f03)
  • Loading branch information
bergmania committed Nov 15, 2023
1 parent 9a143c0 commit 04e59e9
Show file tree
Hide file tree
Showing 17 changed files with 146 additions and 42 deletions.
14 changes: 10 additions & 4 deletions src/Umbraco.Core/Services/ContentService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ public IContent Create(string name, IContent? parent, string contentTypeAlias, i
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);
Expand All @@ -395,6 +395,8 @@ public IContent CreateAndSave(string name, int parentId, string contentTypeAlias

Save(content, userId);

scope.Complete();

return content;
}
}
Expand All @@ -416,7 +418,7 @@ public IContent CreateAndSave(string name, IContent parent, string contentTypeAl
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);
Expand All @@ -431,6 +433,7 @@ public IContent CreateAndSave(string name, IContent parent, string contentTypeAl

Save(content, userId);

scope.Complete();
return content;
}
}
Expand Down Expand Up @@ -508,10 +511,11 @@ public ContentScheduleCollection GetContentScheduleByContentId(int contentId)
/// <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();
}
}

Expand Down Expand Up @@ -2960,7 +2964,7 @@ private OperationResult Sort(ICoreScope scope, IContent[] itemsA, int userId, Ev

public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.ContentTree);

Expand All @@ -2973,6 +2977,8 @@ public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportO
scope.Notifications.Publish(new ContentTreeChangeNotification(root, TreeChangeTypes.RefreshAll, EventMessagesFactory.Get()));
}

scope.Complete();

return report;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ public IEnumerable<TItem> GetAll(IEnumerable<Guid>? ids)
}

using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))

{
scope.ReadLock(ReadLockIds);
return Repository.GetMany(ids.ToArray());
Expand Down
15 changes: 11 additions & 4 deletions src/Umbraco.Core/Services/ContentVersionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public IReadOnlyCollection<ContentVersionMeta> PerformContentVersionCleanup(Date
/// <inheritdoc />
public void SetPreventCleanup(int versionId, bool preventCleanup, int userId = -1)
{
using (ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.ContentTree);
_documentVersionRepository.SetPreventCleanup(versionId, preventCleanup);
Expand All @@ -87,6 +87,7 @@ public void SetPreventCleanup(int versionId, bool preventCleanup, int userId = -
var message = $"set preventCleanup = '{preventCleanup}' for version '{versionId}'";

Audit(auditType, userId, version.ContentId, message, $"{version.VersionDate}");
scope.Complete();
}
}

Expand Down Expand Up @@ -120,7 +121,7 @@ private IReadOnlyCollection<ContentVersionMeta> CleanupDocumentVersions(DateTime
*
* 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();
Expand Down Expand Up @@ -149,6 +150,8 @@ private IReadOnlyCollection<ContentVersionMeta> CleanupDocumentVersions(DateTime

versionsToDelete.Add(version);
}

scope.Complete();
}

if (!versionsToDelete.Any())
Expand All @@ -161,7 +164,7 @@ private IReadOnlyCollection<ContentVersionMeta> CleanupDocumentVersions(DateTime

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();
Expand All @@ -174,12 +177,16 @@ private IReadOnlyCollection<ContentVersionMeta> CleanupDocumentVersions(DateTime
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;
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Services/DataTypeService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ public void Delete(IDataType dataType, int userId = Constants.Security.SuperUser

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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public IEnumerable<ContentVersionMeta> Apply(DateTime asAtDate, IEnumerable<Cont

var theRest = new List<ContentVersionMeta>();

using (_scopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = _scopeProvider.CreateCoreScope())
{
var policyOverrides = _documentVersionRepository.GetCleanupPolicies()?
.ToDictionary(x => x.ContentTypeId);
Expand Down Expand Up @@ -77,6 +77,8 @@ public IEnumerable<ContentVersionMeta> Apply(DateTime asAtDate, IEnumerable<Cont
yield return version;
}
}

scope.Complete();
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/Umbraco.Core/Services/MediaService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,7 @@ public bool Sort(IEnumerable<IMedia> items, int userId = Constants.Security.Supe

public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
scope.WriteLock(Constants.Locks.MediaTree);

Expand All @@ -1210,6 +1210,7 @@ public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportO
scope.Notifications.Publish(new MediaTreeChangeNotification(root, TreeChangeTypes.RefreshAll, EventMessagesFactory.Get()));
}

scope.Complete();
return report;
}
}
Expand Down
15 changes: 11 additions & 4 deletions src/Umbraco.Core/Services/TwoFactorLoginService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ public TwoFactorLoginService(
/// <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 />
Expand Down Expand Up @@ -155,8 +157,12 @@ public async Task<bool> IsTwoFactorEnabledAsync(Guid userOrMemberKey) =>
/// <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 />
Expand All @@ -173,9 +179,10 @@ public bool ValidateTwoFactorSetup(string providerName, string secret, string co
/// <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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public override Task PerformExecuteAsync(object? state)
// 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.
Expand All @@ -113,6 +113,8 @@ public override Task PerformExecuteAsync(object? state)
grouped.Count(),
grouped.Key);
}

scope.Complete();
}
finally
{
Expand Down
30 changes: 25 additions & 5 deletions src/Umbraco.Infrastructure/Logging/Viewer/LogViewerConfig.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using Umbraco.Cms.Core.Models;
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;
Expand All @@ -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;
Expand All @@ -26,22 +43,25 @@ public LogViewerConfig(ILogViewerQueryRepository logViewerQueryRepository, IScop

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();
}

public IReadOnlyList<SavedLogSearch>? DeleteSavedSearch(string? name, string? query)
{
using IScope scope = _scopeProvider.CreateScope(autoComplete: true);
using IScope scope = _scopeProvider.CreateScope();
ILogViewerQuery? item = name is null ? null : _logViewerQueryRepository.GetByName(name);
if (item is not null)
{
_logViewerQueryRepository.Delete(item);
}

// 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;
}
}
6 changes: 4 additions & 2 deletions src/Umbraco.Infrastructure/Security/MemberUserStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public override Task<IdentityResult> CreateAsync(
throw new ArgumentNullException(nameof(user));
}

using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
using ICoreScope scope = _scopeProvider.CreateCoreScope();

// create member
IMember memberEntity = _memberService.CreateMember(
Expand Down Expand Up @@ -150,6 +150,7 @@ public override Task<IdentityResult> CreateAsync(
x.Value)));
}

scope.Complete();
return Task.FromResult(IdentityResult.Success);
}
catch (Exception ex)
Expand Down Expand Up @@ -179,7 +180,7 @@ public override Task<IdentityResult> UpdateAsync(
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)
Expand Down Expand Up @@ -220,6 +221,7 @@ public override Task<IdentityResult> UpdateAsync(
}
}

scope.Complete();
return Task.FromResult(IdentityResult.Success);
}
catch (Exception ex)
Expand Down
3 changes: 2 additions & 1 deletion src/Umbraco.Web.Website/Controllers/UmbProfileController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ private void AddErrors(IdentityResult result)

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;
Expand Down Expand Up @@ -140,6 +140,7 @@ private async Task<IdentityResult> UpdateMemberAsync(ProfileModel model, MemberI

_memberService.Save(member);

scope.Complete();
return saveResult;
}
}
4 changes: 3 additions & 1 deletion src/Umbraco.Web.Website/Controllers/UmbRegisterController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private void AddErrors(IdentityResult result)
/// <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.
Expand Down Expand Up @@ -160,6 +160,8 @@ private async Task<IdentityResult> RegisterMemberAsync(RegisterModel model)
}
}

scope.Complete();

return identityResult;
}
}
Loading

0 comments on commit 04e59e9

Please sign in to comment.