Skip to content

Use audit service instead of repository directly in services #19357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: v17/improvement/audit-service-rework
Choose a base branch
from
+959 −197
Draft

Use audit service instead of repository directly in services #19357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 10 additions & 25 deletions src/Umbraco.Core/Services/AuditService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.Extensions.DependencyInjection;

Check notice on line 1 in src/Umbraco.Core/Services/AuditService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v17/improvement/audit-service-rework)

✅ Getting better: Missing Arguments Abstractions 

The average number of function arguments decreases from 4.95 to 4.85, threshold = 4.00. The functions in this file have too many arguments, indicating a lack of encapsulation or too many responsibilities in the same functions. Avoid adding more.
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
Expand All @@ -16,44 +16,27 @@
/// </summary>
public sealed class AuditService : RepositoryService, IAuditService
{
private readonly IUserIdKeyResolver _userIdKeyResolver;
private readonly IAuditRepository _auditRepository;
private readonly IEntityService _entityService;
private readonly IUserService _userService;

/// <summary>
/// Initializes a new instance of the <see cref="AuditService" /> class.
/// </summary>
[ActivatorUtilitiesConstructor]
public AuditService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IAuditRepository auditRepository,
IUserService userService,
IUserIdKeyResolver userIdKeyResolver,
IEntityService entityService)
: base(provider, loggerFactory, eventMessagesFactory)
{
_auditRepository = auditRepository;
_userService = userService;
_userIdKeyResolver = userIdKeyResolver;
_entityService = entityService;
}

Check notice on line 39 in src/Umbraco.Core/Services/AuditService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v17/improvement/audit-service-rework)

✅ No longer an issue: Constructor Over-Injection 

AuditService is no longer above the threshold for number of arguments. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
/// <summary>
/// Initializes a new instance of the <see cref="AuditService" /> class.
/// </summary>
[Obsolete("Use the non-obsolete constructor. Scheduled for removal in Umbraco 19.")]
public AuditService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IAuditRepository auditRepository,
IAuditEntryRepository auditEntryRepository,
IUserService userService,
IEntityService entityService)
: this(provider, loggerFactory, eventMessagesFactory, auditRepository, userService, entityService)
{
}

/// <inheritdoc />
public async Task<Attempt<AuditLogOperationStatus>> AddAsync(
AuditType type,
Expand All @@ -63,10 +46,12 @@
string? comment = null,
string? parameters = null)
{
var userId = userKey switch
int? userId = userKey switch
{
{ } when userKey == Constants.Security.UnknownUserKey => Constants.Security.UnknownUserId,
_ => (await _userService.GetAsync(userKey))?.Id,
_ => await _userIdKeyResolver.TryGetAsync(userKey) is { Success: true } userIdAttempt
? userIdAttempt.Result
: null,
};

if (userId is null)
Expand Down Expand Up @@ -245,8 +230,7 @@
ArgumentOutOfRangeException.ThrowIfNegative(skip);
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take);

IUser? user = await _userService.GetAsync(userKey);
if (user is null)
if (await _userIdKeyResolver.TryGetAsync(userKey) is not { Success: true } userIdAttempt)
{
return new PagedModel<IAuditItem>();
}
Expand All @@ -256,7 +240,7 @@
sinceDate.HasValue ? Query<IAuditItem>().Where(x => x.CreateDate >= sinceDate) : null;

PagedModel<IAuditItem> result = GetItemsByUserInner(
user.Id,
userIdAttempt.Result,
pageIndex,
pageSize,
orderDirection,
Expand Down Expand Up @@ -434,3 +418,4 @@
scope.Complete();
}
}

4 changes: 2 additions & 2 deletions src/Umbraco.Core/Services/ContentBlueprintContainerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ public ContentBlueprintContainerService(
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IDocumentBlueprintContainerRepository entityContainerRepository,
IAuditRepository auditRepository,
IAuditService auditService,
IEntityRepository entityRepository,
IUserIdKeyResolver userIdKeyResolver)
: base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditRepository, entityRepository, userIdKeyResolver)
: base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditService, entityRepository, userIdKeyResolver)
{
}

Expand Down
51 changes: 37 additions & 14 deletions src/Umbraco.Core/Services/ContentService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Diagnostics.CodeAnalysis;

Check notice on line 1 in src/Umbraco.Core/Services/ContentService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v17/improvement/audit-service-rework)

✅ Getting better: Overall Code Complexity 

The mean cyclomatic complexity decreases from 4.14 to 4.12, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.

Check notice on line 1 in src/Umbraco.Core/Services/ContentService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v17/improvement/audit-service-rework)

✅ Getting better: Primitive Obsession 

The ratio of primitive types in function arguments decreases from 46.52% to 46.18%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using System.Runtime.InteropServices;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -27,7 +27,7 @@
/// </summary>
public class ContentService : RepositoryService, IContentService
{
private readonly IAuditRepository _auditRepository;
private readonly IAuditService _auditService;
private readonly IContentTypeRepository _contentTypeRepository;
private readonly IDocumentBlueprintRepository _documentBlueprintRepository;
private readonly IDocumentRepository _documentRepository;
Expand All @@ -52,7 +52,7 @@
IEventMessagesFactory eventMessagesFactory,
IDocumentRepository documentRepository,
IEntityRepository entityRepository,
IAuditRepository auditRepository,
IAuditService auditService,
IContentTypeRepository contentTypeRepository,
IDocumentBlueprintRepository documentBlueprintRepository,
ILanguageRepository languageRepository,
Expand All @@ -68,7 +68,7 @@
{
_documentRepository = documentRepository;
_entityRepository = entityRepository;
_auditRepository = auditRepository;
_auditService = auditService;
_contentTypeRepository = contentTypeRepository;
_documentBlueprintRepository = documentBlueprintRepository;
_languageRepository = languageRepository;
Expand All @@ -87,8 +87,7 @@
_logger = loggerFactory.CreateLogger<ContentService>();
}

[Obsolete("Use non-obsolete constructor. Scheduled for removal in V17.")]

[Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")]
public ContentService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
Expand All @@ -104,14 +103,16 @@
ICultureImpactFactory cultureImpactFactory,
IUserIdKeyResolver userIdKeyResolver,
PropertyEditorCollection propertyEditorCollection,
IIdKeyMap idKeyMap)
IIdKeyMap idKeyMap,
IOptionsMonitor<ContentSettings> optionsMonitor,
IRelationService relationService)
: this(
provider,
loggerFactory,
eventMessagesFactory,
documentRepository,
entityRepository,
auditRepository,
StaticServiceProvider.Instance.GetRequiredService<IAuditService>(),
contentTypeRepository,
documentBlueprintRepository,
languageRepository,
Expand All @@ -121,33 +122,38 @@
userIdKeyResolver,
propertyEditorCollection,
idKeyMap,
StaticServiceProvider.Instance.GetRequiredService<IOptionsMonitor<ContentSettings>>(),
StaticServiceProvider.Instance.GetRequiredService<IRelationService>())
optionsMonitor,
relationService)
{
}
[Obsolete("Use non-obsolete constructor. Scheduled for removal in V17.")]

[Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")]
public ContentService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IDocumentRepository documentRepository,
IEntityRepository entityRepository,
IAuditRepository auditRepository,
IAuditService auditService,
IContentTypeRepository contentTypeRepository,
IDocumentBlueprintRepository documentBlueprintRepository,
ILanguageRepository languageRepository,
Lazy<IPropertyValidationService> propertyValidationService,
IShortStringHelper shortStringHelper,
ICultureImpactFactory cultureImpactFactory,
IUserIdKeyResolver userIdKeyResolver,
PropertyEditorCollection propertyEditorCollection)
PropertyEditorCollection propertyEditorCollection,
IIdKeyMap idKeyMap,
IOptionsMonitor<ContentSettings> optionsMonitor,
IRelationService relationService)
: this(
provider,
loggerFactory,
eventMessagesFactory,
documentRepository,
entityRepository,
auditRepository,
auditService,
contentTypeRepository,
documentBlueprintRepository,
languageRepository,
Expand All @@ -156,7 +162,9 @@
cultureImpactFactory,
userIdKeyResolver,
propertyEditorCollection,
StaticServiceProvider.Instance.GetRequiredService<IIdKeyMap>())
idKeyMap,
optionsMonitor,
relationService)
{
}

Expand Down Expand Up @@ -3082,7 +3090,22 @@
#region Private Methods

private void Audit(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) =>
_auditRepository.Save(new AuditItem(objectId, type, userId, UmbracoObjectTypes.Document.GetName(), message, parameters));
AuditAsync(type, userId, objectId, message, parameters).GetAwaiter().GetResult();

private async Task AuditAsync(AuditType type, int userId, int objectId, string? message = null, string? parameters = null)
{
Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt
? userKeyAttempt.Result
: Constants.Security.UnknownUserKey;

await _auditService.AddAsync(
type,
userKey,
objectId,
UmbracoObjectTypes.Document.GetName(),
message,
parameters);
}

private string GetLanguageDetailsForAuditEntry(IEnumerable<string> affectedCultures)
=> GetLanguageDetailsForAuditEntry(_languageRepository.GetMany(), affectedCultures);
Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Core/Services/ContentTypeContainerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ public ContentTypeContainerService(
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IDocumentTypeContainerRepository entityContainerRepository,
IAuditRepository auditRepository,
IAuditService auditService,
IEntityRepository entityRepository,
IUserIdKeyResolver userIdKeyResolver)
: base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditRepository, entityRepository, userIdKeyResolver)
: base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditService, entityRepository, userIdKeyResolver)
{
}

Expand Down
63 changes: 61 additions & 2 deletions src/Umbraco.Core/Services/ContentTypeService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
Expand All @@ -22,7 +24,7 @@ public ContentTypeService(
IEventMessagesFactory eventMessagesFactory,
IContentService contentService,
IContentTypeRepository repository,
IAuditRepository auditRepository,
IAuditService auditService,
IDocumentTypeContainerRepository entityContainerRepository,
IEntityRepository entityRepository,
IEventAggregator eventAggregator,
Expand All @@ -33,14 +35,71 @@ public ContentTypeService(
loggerFactory,
eventMessagesFactory,
repository,
auditRepository,
auditService,
entityContainerRepository,
entityRepository,
eventAggregator,
userIdKeyResolver,
contentTypeFilters) =>
ContentService = contentService;

[Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")]
public ContentTypeService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IContentService contentService,
IContentTypeRepository repository,
IAuditRepository auditRepository,
IDocumentTypeContainerRepository entityContainerRepository,
IEntityRepository entityRepository,
IEventAggregator eventAggregator,
IUserIdKeyResolver userIdKeyResolver,
ContentTypeFilterCollection contentTypeFilters)
: this(
provider,
loggerFactory,
eventMessagesFactory,
contentService,
repository,
StaticServiceProvider.Instance.GetRequiredService<IAuditService>(),
entityContainerRepository,
entityRepository,
eventAggregator,
userIdKeyResolver,
contentTypeFilters)
{
}

[Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")]
public ContentTypeService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IContentService contentService,
IContentTypeRepository repository,
IAuditRepository auditRepository,
IAuditService auditService,
IDocumentTypeContainerRepository entityContainerRepository,
IEntityRepository entityRepository,
IEventAggregator eventAggregator,
IUserIdKeyResolver userIdKeyResolver,
ContentTypeFilterCollection contentTypeFilters)
: this(
provider,
loggerFactory,
eventMessagesFactory,
contentService,
repository,
auditService,
entityContainerRepository,
entityRepository,
eventAggregator,
userIdKeyResolver,
contentTypeFilters)
{
}

protected override int[] ReadLockIds => ContentTypeLocks.ReadLockIds;

protected override int[] WriteLockIds => ContentTypeLocks.WriteLockIds;
Expand Down
Loading