#127 Хранение технологических сообщений #4
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "TechMessages"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -6,1 +6,4 @@
namespace Persistence.API.Controllers;
/// <summary>
/// <20>אבמעא ס גנולוםם<D79D>לט האםם<D79D>לט
Кодировка
@ -45,4 +79,0 @@
public async Task<ActionResult<int>> Save(Guid setpointKey, object newValue, CancellationToken token)
{
// ToDo: вычитка idUser
await setpointRepository.Save(setpointKey, newValue, 0, token);
Здесь userId вместо 0 можно уже использовать
iduser вынесен в параметры запроса
@ -35,0 +56,4 @@
/// <param name="token"></param>
/// <returns></returns>
[HttpGet("log")]
public async Task<ActionResult<Dictionary<Guid, IEnumerable<SetpointLogDto>>>> GetLog([FromQuery] IEnumerable<Guid> setpointKeys, CancellationToken token)
Лучше писать так. Так читабельнее будет выглядеть. Я у себя тоже этот момент поправлю.
@ -43,0 +73,4 @@
[HttpPost]
public async Task<ActionResult<int>> Save(Guid setpointKey, object newValue, CancellationToken token)
{
// ToDo: вычитка idUser
var userId = User.GetUserId();
Вынесено в параметр
@ -0,0 +42,4 @@
/// <param name="token"></param>
/// <returns></returns>
[HttpGet("statistics/{autoDrillingSystem}")]
public async Task<ActionResult<int>> GetStatistics([FromRoute] string? autoDrillingSystem, int? importantId, CancellationToken token)
Возвращаемый тип Dictionary<string, int>
@ -0,0 +44,4 @@
[HttpGet("statistics/{autoDrillingSystem}")]
public async Task<ActionResult<int>> GetStatistics([FromRoute] string? autoDrillingSystem, int? importantId, CancellationToken token)
{
var result = await techMessagesRepository.GetStatistics(importantId, autoDrillingSystem, token);
Лучше все переменные и свойства importantId переименовать на categoryId или на categoryImportantId. Нужно, чтобы слово "категория" была, а то не очень понятно.
@ -0,0 +71,4 @@
[HttpPost]
public async Task<ActionResult<int>> InsertRange([FromBody] IEnumerable<TechMessageDto> dtos, CancellationToken token)
{
var result = await techMessagesRepository.InsertRange(dtos, token);
Здесь нужно вычислить пользователя, чтобы передать ключ пользователя далее внутрь метода InsertRange репозитория (для вставки в базу, так как у TechMessage есть UserId)
@ -0,0 +83,4 @@
[HttpGet("categories")]
public ActionResult<Dictionary<int, string>> GetImportantCategories()
{
var result = new Dictionary<int, string>()
Желательно вынести в константу из метода контроллера.
Желательно эту константу хранить где-то на уровне репозитория (спросить у Никиты)
@ -19,2 +20,3 @@
[HttpGet]
/// <summary>
/// Получить список объектов, удовлетворяющий диапазону дат
Тут тоже кодировка полетела
@ -29,3 +40,4 @@
[HttpGet("datesRange")]
public async Task<IActionResult> GetDatesRange(CancellationToken token)
{
var result = await this.timeSeriesDataRepository.GetDatesRange(token);
можно без this
@ -0,0 +21,4 @@
Task<IApiResponse<IEnumerable<string>>> GetSystems(CancellationToken token);
[Get($"{BaseRoute}/statistics/" + "{autoDrillingSystem}")]
Task<IApiResponse<int>> GetStatistics(string? autoDrillingSystem, int? importantId, CancellationToken token);
Здесь тоже нужно поправить тип возвращаемых данных
$"{BaseRoute}/statistics/" + "{autoDrillingSystem}" -> можно написать слитно, а то так хуже читается
не актуально, autoDrillingSystem ныне приходит не с роута
@ -0,0 +159,4 @@
var imortantId = 0;
var autoDrillingSystem = nameof(TechMessageDto.System);
var dtos = await InsertRange();
var filteredDtos = dtos.Where(e => e.CategoryId == imortantId && e.System == e.System);
Не очень понятно, что такое e.System == e.System, оно же всегда будет true давать?
@ -0,0 +30,4 @@
.SortBy(request.SortSettings)
.Skip(request.Skip)
.Take(request.Take)
.ToListAsync();
По код-стандарту договорились, что материализация будет происходить в массив. Поэтому лучше ToArrayAsync(token). Должен быть еще токен отмены.
Плюс желательно сортировку по умолчанию иметь, если вдруг request.SortSettings будет пустым
@ -0,0 +35,4 @@
{
Skip = request.Skip,
Take = request.Take,
Count = entities.Count,
Count должен рассчитываться до Skip и Take, - то есть Count показывает сколько всего элементов существует
@ -0,0 +45,4 @@
public async Task<Dictionary<string, int>> GetStatistics(int? importantId, string? autoDrillingSystem, CancellationToken token)
{
var query = GetQueryReadOnly();
var count = await query
Название переменной желательно поправить. Можно назвать result.
@ -0,0 +47,4 @@
var query = GetQueryReadOnly();
var count = await query
.Where(e => importantId == null || e.CategoryId == importantId)
.Where(e => autoDrillingSystem == null || e.System.Name == autoDrillingSystem)
Вообще, думаю, что во всех местах, где есть поиск или фильтрация по имени, необходимо обе сравниваемые переменные приводить к одному регистру и убирать пробелы.
@ -0,0 +56,4 @@
public async Task<IEnumerable<string>> GetSystems(CancellationToken token)
{
var entities = await GetSystems();
Нужно токен передать внутрь приватного метода
@ -0,0 +59,4 @@
var entities = await GetSystems();
var systems = entities.Select(e => e.Name);
return systems ?? [];
Здесь можно не писать
?? []
, по идее в systems не может быть null, там может быть только пустая коллекция@ -0,0 +69,4 @@
foreach (var dto in dtos)
{
var entity = dto.Adapt<TechMessage>();
var systems = await GetSystems();
Здесь тоже нужно токен передать
@ -0,0 +70,4 @@
{
var entity = dto.Adapt<TechMessage>();
var systems = await GetSystems();
var systemId = systems.FirstOrDefault(e => e.Name == dto.System)?.SystemId
Здесь точно e.Name == dto.System необходимо приводить к одному регистру и убирать пробелы.
Пользователь может передать dto.System, например, с пробелом. Тогда поиск по системам не найдет необходимого элемента и создаст новый с таким же названием и с пробелом.
Также при создании системы CreateSystem лучше предварительно убирать пробелы.
@ -0,0 +88,4 @@
{
var systems = await memoryCache.GetOrCreateAsync(SystemCacheKey, async f =>
{
f.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(60);
60 минут - лучше в отдельное поле вынести
@ -0,0 +91,4 @@
f.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(60);
var query = db.Set<ADSystem>();
var entities = await query.ToListAsync();
ToArrayAsync и туда внутрь еще токен передать....
@ -0,0 +97,4 @@
return dtos;
});
return systems ?? [];
Здесь тоже можно убрать
?? []
, так как внутри systems не может быть null.Чтобы гарантированно это обозначить, можно написать так: return systems!
@ -0,0 +103,4 @@
{
memoryCache.Remove(SystemCacheKey);
var systemId = Guid.NewGuid();
Здесь можно дополнительную переменную не создавать
@ -0,0 +3,4 @@
/// <summary>
/// Модель системы автобурения
/// </summary>
public class ADSystemDto
Сокращения не желательны.
Предлагаю название DrillingSystem (без auto, так как системы необязательно должны быть автоматизированые)
@ -0,0 +38,4 @@
/// <param name="autoDrillingSystem">Система автобурения</param>
/// <param name="token"></param>
/// <returns></returns>
Task<Dictionary<string, int>> GetStatistics(int? importantId, string? autoDrillingSystem, CancellationToken token);
Аргументы лучше местами поменять, чтобы их порядок быть от более крупной сущности к более мелкой (Плюс тогда сигнатура будет больше похожа на сигнатуру веб-апи).
В запросе от пользователя также лучше сразу предусмотреть возможность множественного выбора систем и категорий. Прим.:
IEnumerable<int>? CategoriesIds
Возвращаемый Dictionary<string, int> не особо понятно, что именно возвращает.
Предлагаю возвращать:
@ -52,0 +106,4 @@
{
await setpointRepository.Save(setpointKey, newValue, idUser, token);
return Ok();
Вроде бы договорились CreatedAtAction возвращать?
А еще метод репозитория setpointRepository.Save ничего не возвращает, хотя в других подлобных методах возвращается int.
Вот, например:
var result = await techMessagesRepository.InsertRange(dtos, token);
return CreatedAtAction(nameof(InsertRange), result);
Еще название самого метода Save путает. По коду видно, что это именно вставка, а не просто сохранение...Но, может, так надо...
setpointRepository.Save ничего не возвращает т.к. количество сохраненных записей всегда будет == 1
Для единообразия можно сделать "Add" "AddRange"
@ -134,0 +144,4 @@
//assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.NotNull(response.Content);
Assert.Equal(DateTimeOffset.MinValue, response.Content?.From);
Тут по идее на null уже можно не проверять response.Content?.From, потому что выше есть проверка Assert.NotNull(response.Content). Если студия ругается, то можно так написать: response.Content!.From
А еще, думаю, нужно в качестве теста заинсертить туда несколько записей, и ,зная, какие у них максимальные и минимальные даты - сверять их с тем, что вернет GetDatesRangeAsyncю.
Ниже приложила скрин подобного тестового метода, только в ChangeLogTestController
Сверять максимальные и минимальные даты необъодимо будет в методе GetDatesRange_AfterSave_returns_success
@ -134,0 +161,4 @@
//assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.NotNull(response.Content);
Assert.NotNull(response.Content?.From);
Assert.NotNull(response.Content?.From);
Тут также выше уже была проверка на null: Assert.NotNull(response.Content);
Поэтому ниже можно не писать знак вопроса: response.Content?.From.
А если не писать знак вопроса, то response.Content.From тоже не нужно проверять на null,потому что From не nullable.
Тут тоже желательно сравнить From и To с каким-то конкретными значениями, которые ты ожиадешь
@ -0,0 +144,4 @@
public async Task GetStatistics_returns_success()
{
//arrange
memoryCache.Remove(SystemCacheKey);
Для тестов, которые что-то возвращают, необходимо предварительно записывать данные. И сравнивать уже получаемые данные с теми, что туда записывались.
Например, в данном методе нужно было предварительно что-то записать в базу, на основании чего бы рассчиталась статистика, а потом этот расчет уже сравнить с тем, что ты ожидаешь получить.
Сейчас здесь просто проверка на то, что метод вернул 200 ответ и что пришла пустая коллекция. Это верно, потому что в базе ничего нет. Но не факт, что тест бы прошел, если бы в базе что-то было.
У каждого returns_success теста есть аналог с AfterSave - в котором, как раз таки, проверяются сохраненные данные. Так сделано для тестирования работоспособности отдельных методов вне зависимости от метода сохранения.
@ -0,0 +185,4 @@
public async Task GetDatesRange_returns_success()
{
//act
var response = await techMessagesClient.GetDatesRangeAsync(new CancellationToken());
Перед проверкой необходимо в базу что-то записывать и уже потом проверять сам метод контроллера.
Сейчас же здесь просто проверка на то, что метод вернул 200 ответ и что пришла пустая коллекция. Это верно, потому что в базе ничего нет. Но не факт, что тест бы прошел, если бы в базе что-то было.
У каждого returns_success теста есть аналог с AfterSave - в котором, как раз таки, проверяются сохраненные данные. Так сделано для тестирования работоспособности отдельных методов вне зависимости от метода сохранения.
@ -46,0 +47,4 @@
{
var query = GetQueryReadOnly();
var entities = await query
.Where(e => e.Created > dateBegin)
Тут нужно у Никиты спросить, строго или не строгое неравенство нужно. Я почему-то думала, что нужно >= писать....Но лучше у Никиты спросить)
Чтобы такие вопросы не возникали мы называем переменные для фильтрации:
Конкретно в этом случае нужен ≥