1
0
mirror of https://github.com/bitwarden/server.git synced 2025-01-06 19:28:08 +01:00

PM-10563: Paging simplification by page number and size in database

This commit is contained in:
Maciej Zieniuk 2024-10-07 13:03:46 +01:00
parent aa7a62d586
commit 5b3a09dbe8
No known key found for this signature in database
GPG Key ID: 9CACE59F1272ACD9
11 changed files with 163 additions and 123 deletions

View File

@ -1,13 +1,11 @@
#nullable enable
using System.Text.Json;
using Bit.Api.Models.Response;
using Bit.Api.NotificationCenter.Models;
using Bit.Api.NotificationCenter.Models.Request;
using Bit.Api.NotificationCenter.Models.Response;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Commands.Interfaces;
using Bit.Core.NotificationCenter.Models.Filter;
using Bit.Core.NotificationCenter.Queries.Interfaces;
using Bit.Core.Utilities;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
@ -21,7 +19,7 @@ public class NotificationsController : Controller
private readonly IMarkNotificationDeletedCommand _markNotificationDeletedCommand;
private readonly IMarkNotificationReadCommand _markNotificationReadCommand;
private const int RowsPerPage = 10;
private const int DefaultPageSize = 10;
public NotificationsController(
IGetNotificationStatusDetailsForUserQuery getNotificationStatusDetailsForUserQuery,
@ -37,42 +35,28 @@ public class NotificationsController : Controller
public async Task<ListResponseModel<NotificationResponseModel>> List(
[FromQuery] NotificationFilterRequestModel filter)
{
var continuationToken = ParseContinuationToken(filter.ContinuationToken);
var pageOptions = new PageOptions
{
ContinuationToken = filter.ContinuationToken,
PageSize = DefaultPageSize
};
var notificationStatusFilter = new NotificationStatusFilter
{
Read = filter.ReadStatusFilter,
Deleted = filter.DeletedStatusFilter
};
var notificationStatusDetails =
await _getNotificationStatusDetailsForUserQuery.GetByUserIdStatusFilterAsync(notificationStatusFilter);
var notificationStatusDetailsPagedResult =
await _getNotificationStatusDetailsForUserQuery.GetByUserIdStatusFilterAsync(notificationStatusFilter,
pageOptions);
if (continuationToken != null)
{
// Priority and CreationDate are always in descending order
notificationStatusDetails = notificationStatusDetails
.Where(n => n.Priority < continuationToken.Priority ||
(n.Priority == continuationToken.Priority && n.CreationDate < continuationToken.Date));
}
var pagedNotificationStatusDetails = notificationStatusDetails
.Take(RowsPerPage)
.ToList();
var responses = pagedNotificationStatusDetails
var responses = notificationStatusDetailsPagedResult.Data
.Select(n => new NotificationResponseModel(n))
.ToList();
var nextContinuationToken = pagedNotificationStatusDetails.Count == RowsPerPage
? new NotificationContinuationToken
{
Priority = pagedNotificationStatusDetails.Last().Priority,
Date = pagedNotificationStatusDetails.Last().CreationDate
}
: null;
return new ListResponseModel<NotificationResponseModel>(responses,
CreateContinuationToken(nextContinuationToken));
notificationStatusDetailsPagedResult.ContinuationToken);
}
[HttpPatch("{id}/delete")]
@ -86,28 +70,4 @@ public class NotificationsController : Controller
{
await _markNotificationReadCommand.MarkReadAsync(id);
}
private static NotificationContinuationToken? ParseContinuationToken(string? continuationToken)
{
if (continuationToken == null)
{
return null;
}
var decodedContinuationToken = CoreHelpers.Base64UrlDecodeString(continuationToken);
return JsonSerializer.Deserialize<NotificationContinuationToken>(decodedContinuationToken,
JsonHelpers.CamelCase);
}
private static string? CreateContinuationToken(NotificationContinuationToken? notificationContinuationToken)
{
if (notificationContinuationToken == null)
{
return null;
}
var serializedContinuationToken = JsonSerializer.Serialize(notificationContinuationToken,
JsonHelpers.CamelCase);
return CoreHelpers.Base64UrlEncodeString(serializedContinuationToken);
}
}

View File

@ -1,11 +0,0 @@
#nullable enable
using Bit.Core.NotificationCenter.Enums;
namespace Bit.Api.NotificationCenter.Models;
public class NotificationContinuationToken
{
public Priority Priority { get; set; }
public DateTime Date { get; set; }
}

View File

@ -1,6 +1,7 @@
#nullable enable
using Bit.Core.Context;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
using Bit.Core.NotificationCenter.Queries.Interfaces;
@ -21,8 +22,8 @@ public class GetNotificationStatusDetailsForUserQuery : IGetNotificationStatusDe
_notificationRepository = notificationRepository;
}
public async Task<IEnumerable<NotificationStatusDetails>> GetByUserIdStatusFilterAsync(
NotificationStatusFilter statusFilter)
public async Task<PagedResult<NotificationStatusDetails>> GetByUserIdStatusFilterAsync(
NotificationStatusFilter statusFilter, PageOptions pageOptions)
{
if (!_currentContext.UserId.HasValue)
{
@ -33,6 +34,6 @@ public class GetNotificationStatusDetailsForUserQuery : IGetNotificationStatusDe
// Note: only returns the user's notifications - no authorization check needed
return await _notificationRepository.GetByUserIdAndStatusAsync(_currentContext.UserId.Value, clientType,
statusFilter);
statusFilter, pageOptions);
}
}

View File

@ -1,4 +1,5 @@
#nullable enable
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
@ -6,5 +7,6 @@ namespace Bit.Core.NotificationCenter.Queries.Interfaces;
public interface IGetNotificationStatusDetailsForUserQuery
{
Task<IEnumerable<NotificationStatusDetails>> GetByUserIdStatusFilterAsync(NotificationStatusFilter statusFilter);
Task<PagedResult<NotificationStatusDetails>> GetByUserIdStatusFilterAsync(NotificationStatusFilter statusFilter,
PageOptions pageOptions);
}

View File

@ -1,5 +1,6 @@
#nullable enable
using Bit.Core.Enums;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Entities;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
@ -22,10 +23,13 @@ public interface INotificationRepository : IRepository<Notification, Guid>
/// If both <see cref="NotificationStatusFilter.Read"/> and <see cref="NotificationStatusFilter.Deleted"/>
/// are not set, includes notifications without a status.
/// </param>
/// <param name="pageOptions">
/// Pagination options.
/// </param>
/// <returns>
/// Ordered by priority (highest to lowest) and creation date (descending).
/// Paged results ordered by priority (descending, highest to lowest) and creation date (descending).
/// Includes all fields from <see cref="Notification"/> and <see cref="NotificationStatus"/>
/// </returns>
Task<IEnumerable<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId, ClientType clientType,
NotificationStatusFilter? statusFilter);
Task<PagedResult<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId, ClientType clientType,
NotificationStatusFilter? statusFilter, PageOptions pageOptions);
}

View File

@ -1,6 +1,7 @@
#nullable enable
using System.Data;
using Bit.Core.Enums;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Entities;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
@ -24,16 +25,33 @@ public class NotificationRepository : Repository<Notification, Guid>, INotificat
{
}
public async Task<IEnumerable<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId,
ClientType clientType, NotificationStatusFilter? statusFilter)
public async Task<PagedResult<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId,
ClientType clientType, NotificationStatusFilter? statusFilter, PageOptions pageOptions)
{
await using var connection = new SqlConnection(ConnectionString);
if (!int.TryParse(pageOptions.ContinuationToken, out var pageNumber))
{
pageNumber = 1;
}
var results = await connection.QueryAsync<NotificationStatusDetails>(
"[dbo].[Notification_ReadByUserIdAndStatus]",
new { UserId = userId, ClientType = clientType, statusFilter?.Read, statusFilter?.Deleted },
new
{
UserId = userId,
ClientType = clientType,
statusFilter?.Read,
statusFilter?.Deleted,
PageNumber = pageNumber,
pageOptions.PageSize
},
commandType: CommandType.StoredProcedure);
return results.ToList();
return new PagedResult<NotificationStatusDetails>
{
Data = results.ToList(),
ContinuationToken = (pageNumber + 1).ToString()
};
}
}

View File

@ -1,6 +1,7 @@
#nullable enable
using AutoMapper;
using Bit.Core.Enums;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
using Bit.Core.NotificationCenter.Repositories;
@ -36,12 +37,17 @@ public class NotificationRepository : Repository<Core.NotificationCenter.Entitie
return Mapper.Map<List<Core.NotificationCenter.Entities.Notification>>(notifications);
}
public async Task<IEnumerable<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId,
ClientType clientType, NotificationStatusFilter? statusFilter)
public async Task<PagedResult<NotificationStatusDetails>> GetByUserIdAndStatusAsync(Guid userId,
ClientType clientType, NotificationStatusFilter? statusFilter, PageOptions pageOptions)
{
await using var scope = ServiceScopeFactory.CreateAsyncScope();
var dbContext = GetDatabaseContext(scope);
if (!int.TryParse(pageOptions.ContinuationToken, out var pageNumber))
{
pageNumber = 1;
}
var notificationStatusDetailsViewQuery = new NotificationStatusDetailsViewQuery(userId, clientType);
var query = notificationStatusDetailsViewQuery.Run(dbContext);
@ -55,9 +61,17 @@ public class NotificationRepository : Repository<Core.NotificationCenter.Entitie
select n;
}
return await query
var results = await query
.OrderByDescending(n => n.Priority)
.ThenByDescending(n => n.CreationDate)
.Skip(pageOptions.PageSize * (pageNumber - 1))
.Take(pageOptions.PageSize)
.ToListAsync();
return new PagedResult<NotificationStatusDetails>
{
Data = results,
ContinuationToken = (pageNumber + 1).ToString()
};
}
}

View File

@ -2,7 +2,9 @@ CREATE PROCEDURE [dbo].[Notification_ReadByUserIdAndStatus]
@UserId UNIQUEIDENTIFIER,
@ClientType TINYINT,
@Read BIT,
@Deleted BIT
@Deleted BIT,
@PageNumber INT = 1,
@PageSize INT = 10
AS
BEGIN
SET NOCOUNT ON
@ -30,4 +32,6 @@ BEGIN
(@Deleted = 0 AND n.[DeletedDate] IS NULL),
1, 0) = 1))))
ORDER BY [Priority] DESC, n.[CreationDate] DESC
OFFSET @PageSize * (@PageNumber - 1) ROWS
FETCH NEXT @PageSize ROWS ONLY
END

View File

@ -1,13 +1,12 @@
#nullable enable
using System.Text.Json;
using Bit.Api.NotificationCenter.Controllers;
using Bit.Api.NotificationCenter.Models.Request;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Commands.Interfaces;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
using Bit.Core.NotificationCenter.Queries.Interfaces;
using Bit.Core.Test.NotificationCenter.AutoFixture;
using Bit.Core.Utilities;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute;
@ -40,8 +39,8 @@ public class NotificationsControllerTest
.ToList();
sutProvider.GetDependency<IGetNotificationStatusDetailsForUserQuery>()
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>())
.Returns(notificationStatusDetailsList);
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>(), Arg.Any<PageOptions>())
.Returns(new PagedResult<NotificationStatusDetails> { Data = notificationStatusDetailsList });
var expectedNotificationStatusDetailsMap = notificationStatusDetailsList
.Take(10)
@ -74,13 +73,15 @@ public class NotificationsControllerTest
await sutProvider.GetDependency<IGetNotificationStatusDetailsForUserQuery>()
.Received(1)
.GetByUserIdStatusFilterAsync(Arg.Is<NotificationStatusFilter>(filter =>
filter.Read == readStatusFilter && filter.Deleted == deletedStatusFilter));
filter.Read == readStatusFilter && filter.Deleted == deletedStatusFilter),
Arg.Is<PageOptions>(pageOptions =>
pageOptions.ContinuationToken == null && pageOptions.PageSize == 10));
}
[Theory]
[BitAutoData]
[NotificationStatusDetailsListCustomize(19)]
public async Task List_PagingNoContinuationToken_ReturnedFirst10MatchingNotifications(
public async Task List_PagingRequestNoContinuationToken_ReturnedFirst10MatchingNotifications(
SutProvider<NotificationsController> sutProvider,
IEnumerable<NotificationStatusDetails> notificationStatusDetailsEnumerable)
{
@ -90,8 +91,9 @@ public class NotificationsControllerTest
.ToList();
sutProvider.GetDependency<IGetNotificationStatusDetailsForUserQuery>()
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>())
.Returns(notificationStatusDetailsList);
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>(), Arg.Any<PageOptions>())
.Returns(new PagedResult<NotificationStatusDetails>
{ Data = notificationStatusDetailsList.Take(10).ToList(), ContinuationToken = "2" });
var expectedNotificationStatusDetailsMap = notificationStatusDetailsList
.Take(10)
@ -115,22 +117,19 @@ public class NotificationsControllerTest
Assert.Equal(expectedNotificationStatusDetails.ReadDate, notificationResponseModel.ReadDate);
Assert.Equal(expectedNotificationStatusDetails.DeletedDate, notificationResponseModel.DeletedDate);
});
Assert.Equal("2", listResponse.ContinuationToken);
var expectedContinuationToken = new Dictionary<string, object>
{
{ "priority", notificationStatusDetailsList[9].Priority },
{ "date", notificationStatusDetailsList[9].CreationDate }
};
var expectedJsonContinuationToken = JsonSerializer.Serialize(expectedContinuationToken);
var expectedBase64EncodedJsonContinuationToken =
CoreHelpers.Base64UrlEncodeString(expectedJsonContinuationToken);
Assert.Equal(expectedBase64EncodedJsonContinuationToken, listResponse.ContinuationToken);
await sutProvider.GetDependency<IGetNotificationStatusDetailsForUserQuery>()
.Received(1)
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>(),
Arg.Is<PageOptions>(pageOptions =>
pageOptions.ContinuationToken == null && pageOptions.PageSize == 10));
}
[Theory]
[BitAutoData]
[NotificationStatusDetailsListCustomize(19)]
public async Task List_PagingUsingContinuationToken_ReturnedLast9MatchingNotifications(
public async Task List_PagingRequestUsingContinuationToken_ReturnedLast9MatchingNotifications(
SutProvider<NotificationsController> sutProvider,
IEnumerable<NotificationStatusDetails> notificationStatusDetailsEnumerable)
{
@ -140,25 +139,15 @@ public class NotificationsControllerTest
.ToList();
sutProvider.GetDependency<IGetNotificationStatusDetailsForUserQuery>()
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>())
.Returns(notificationStatusDetailsList);
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>(), Arg.Any<PageOptions>())
.Returns(new PagedResult<NotificationStatusDetails>
{ Data = notificationStatusDetailsList.Skip(10).ToList() });
var expectedNotificationStatusDetailsMap = notificationStatusDetailsList
.Skip(10)
.ToDictionary(n => n.Id);
var continuationToken = new Dictionary<string, object>
{
{ "priority", notificationStatusDetailsList[9].Priority },
{ "date", notificationStatusDetailsList[9].CreationDate }
};
var jsonContinuationToken = JsonSerializer.Serialize(continuationToken);
var base64EncodedJsonContinuationToken = CoreHelpers.Base64UrlEncodeString(jsonContinuationToken);
var listResponse = await sutProvider.Sut.List(new NotificationFilterRequestModel
{
ContinuationToken = base64EncodedJsonContinuationToken
});
var listResponse = await sutProvider.Sut.List(new NotificationFilterRequestModel { ContinuationToken = "2" });
Assert.Equal("list", listResponse.Object);
Assert.Equal(9, listResponse.Data.Count());
@ -177,6 +166,12 @@ public class NotificationsControllerTest
Assert.Equal(expectedNotificationStatusDetails.DeletedDate, notificationResponseModel.DeletedDate);
});
Assert.Null(listResponse.ContinuationToken);
await sutProvider.GetDependency<IGetNotificationStatusDetailsForUserQuery>()
.Received(1)
.GetByUserIdStatusFilterAsync(Arg.Any<NotificationStatusFilter>(),
Arg.Is<PageOptions>(pageOptions =>
pageOptions.ContinuationToken == "2" && pageOptions.PageSize == 10));
}
[Theory]

View File

@ -2,6 +2,7 @@
using Bit.Core.Context;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.NotificationCenter.Models.Data;
using Bit.Core.NotificationCenter.Models.Filter;
using Bit.Core.NotificationCenter.Queries;
@ -19,37 +20,49 @@ namespace Bit.Core.Test.NotificationCenter.Queries;
public class GetNotificationStatusDetailsForUserQueryTest
{
private static void Setup(SutProvider<GetNotificationStatusDetailsForUserQuery> sutProvider,
List<NotificationStatusDetails> notificationsStatusDetails, NotificationStatusFilter statusFilter, Guid? userId)
List<NotificationStatusDetails> notificationsStatusDetails, NotificationStatusFilter statusFilter, Guid? userId,
PageOptions pageOptions, string? continuationToken)
{
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(userId);
sutProvider.GetDependency<INotificationRepository>().GetByUserIdAndStatusAsync(
userId.GetValueOrDefault(Guid.NewGuid()), Arg.Any<ClientType>(), statusFilter)
.Returns(notificationsStatusDetails);
sutProvider.GetDependency<INotificationRepository>()
.GetByUserIdAndStatusAsync(userId.GetValueOrDefault(Guid.NewGuid()), Arg.Any<ClientType>(), statusFilter,
pageOptions)
.Returns(new PagedResult<NotificationStatusDetails>
{
Data = notificationsStatusDetails,
ContinuationToken = continuationToken
});
}
[Theory]
[BitAutoData]
public async Task GetByUserIdStatusFilterAsync_NotLoggedIn_NotFoundException(
SutProvider<GetNotificationStatusDetailsForUserQuery> sutProvider,
List<NotificationStatusDetails> notificationsStatusDetails, NotificationStatusFilter notificationStatusFilter)
List<NotificationStatusDetails> notificationsStatusDetails, NotificationStatusFilter notificationStatusFilter,
PageOptions pageOptions, string? continuationToken)
{
Setup(sutProvider, notificationsStatusDetails, notificationStatusFilter, userId: null);
Setup(sutProvider, notificationsStatusDetails, notificationStatusFilter, userId: null, pageOptions,
continuationToken);
await Assert.ThrowsAsync<NotFoundException>(() =>
sutProvider.Sut.GetByUserIdStatusFilterAsync(notificationStatusFilter));
sutProvider.Sut.GetByUserIdStatusFilterAsync(notificationStatusFilter, pageOptions));
}
[Theory]
[BitAutoData]
public async Task GetByUserIdStatusFilterAsync_NotificationsFound_Returned(
SutProvider<GetNotificationStatusDetailsForUserQuery> sutProvider,
List<NotificationStatusDetails> notificationsStatusDetails, NotificationStatusFilter notificationStatusFilter)
List<NotificationStatusDetails> notificationsStatusDetails, NotificationStatusFilter notificationStatusFilter,
PageOptions pageOptions, string? continuationToken)
{
Setup(sutProvider, notificationsStatusDetails, notificationStatusFilter, Guid.NewGuid());
Setup(sutProvider, notificationsStatusDetails, notificationStatusFilter, Guid.NewGuid(), pageOptions,
continuationToken);
var actualNotificationsStatusDetails =
await sutProvider.Sut.GetByUserIdStatusFilterAsync(notificationStatusFilter);
var actualNotificationsStatusDetailsPagedResult =
await sutProvider.Sut.GetByUserIdStatusFilterAsync(notificationStatusFilter, pageOptions);
Assert.Equal(notificationsStatusDetails, actualNotificationsStatusDetails);
Assert.NotNull(actualNotificationsStatusDetailsPagedResult);
Assert.Equal(notificationsStatusDetails, actualNotificationsStatusDetailsPagedResult.Data);
Assert.Equal(continuationToken, actualNotificationsStatusDetailsPagedResult.ContinuationToken);
}
}

View File

@ -0,0 +1,40 @@
-- Stored Procedure Notification_ReadByUserIdAndStatus
CREATE OR ALTER PROCEDURE [dbo].[Notification_ReadByUserIdAndStatus]
@UserId UNIQUEIDENTIFIER,
@ClientType TINYINT,
@Read BIT,
@Deleted BIT,
@PageNumber INT = 1,
@PageSize INT = 10
AS
BEGIN
SET NOCOUNT ON
SELECT n.*
FROM [dbo].[NotificationStatusDetailsView] n
LEFT JOIN [dbo].[OrganizationUserView] ou ON n.[OrganizationId] = ou.[OrganizationId]
AND ou.[UserId] = @UserId
WHERE (n.[NotificationStatusUserId] IS NULL OR n.[NotificationStatusUserId] = @UserId)
AND [ClientType] IN (0, CASE WHEN @ClientType != 0 THEN @ClientType END)
AND ([Global] = 1
OR (n.[UserId] = @UserId
AND (n.[OrganizationId] IS NULL
OR ou.[OrganizationId] IS NOT NULL))
OR (n.[UserId] IS NULL
AND ou.[OrganizationId] IS NOT NULL))
AND ((@Read IS NULL AND @Deleted IS NULL)
OR (n.[NotificationStatusUserId] IS NOT NULL
AND ((@Read IS NULL
OR IIF((@Read = 1 AND n.[ReadDate] IS NOT NULL) OR
(@Read = 0 AND n.[ReadDate] IS NULL),
1, 0) = 1)
OR (@Deleted IS NULL
OR IIF((@Deleted = 1 AND n.[DeletedDate] IS NOT NULL) OR
(@Deleted = 0 AND n.[DeletedDate] IS NULL),
1, 0) = 1))))
ORDER BY [Priority] DESC, n.[CreationDate] DESC
OFFSET @PageSize * (@PageNumber - 1) ROWS
FETCH NEXT @PageSize ROWS ONLY
END
GO