#408 Репозиторий хранения данных WITS0 #7

Merged
on.nemtina merged 9 commits from WitsData into master 2024-12-12 16:11:56 +05:00
Collaborator
No description provided.
rs.efremov added 4 commits 2024-12-06 11:41:50 +05:00
on.nemtina requested changes 2024-12-10 13:58:22 +05:00
@ -0,0 +28,4 @@
/// <param name="token"></param>
/// <returns></returns>
[HttpGet("datesRange")]
public async Task<ActionResult<DatesRangeDto>> GetDatesRangeAsync([FromQuery] int discriminatorId, CancellationToken token)
Owner

discriminatorId лучше добавить в роут

discriminatorId лучше добавить в роут
@ -0,0 +44,4 @@
/// <param name="token"></param>
/// <returns></returns>
[HttpGet("part")]
public async Task<ActionResult<IEnumerable<WitsDataDto>>> GetPart([FromQuery] int discriminatorId, [FromQuery] DateTimeOffset dateBegin, [FromQuery] int take, CancellationToken token)
Owner

discriminatorId лучше добавить в роут

discriminatorId лучше добавить в роут
@ -0,0 +61,4 @@
/// <param name="token"></param>
/// <returns></returns>
[HttpGet("graph")]
public async Task<ActionResult<IEnumerable<WitsDataDto>>> GetValuesForGraph([FromQuery] int discriminatorId,
Owner

discriminatorId лучше добавить в роут

discriminatorId лучше добавить в роут
@ -0,0 +7,4 @@
private const string BaseRoute = "/api/witsData";
[Get($"{BaseRoute}/graph")]
Task<IApiResponse<IEnumerable<WitsDataDto>>> GetValuesForGraph([Query] int discriminatorId, [Query] DateTimeOffset dateFrom, [Query] DateTimeOffset dateTo, [Query] int approxPointsCount, CancellationToken token);
Owner

discriminatorId лучше добавить в роут

discriminatorId лучше добавить в роут
@ -0,0 +47,4 @@
var take = 1;
//act
var response = await witsDataClient.GetPart(discriminatorId, dateBegin, take, new CancellationToken());
Owner

Лучше в тестах писать не new CancellationToken(), а CancellationToken.None (по согласованию с Никитой)

Лучше в тестах писать не new CancellationToken(), а CancellationToken.None (по согласованию с Никитой)
@ -0,0 +109,4 @@
.ToString("dd.MM.yyyy-HH:mm:ss");
Assert.Equal(expectedDateFrom, actualDateFrom);
var expectedDateTo = dtos
Owner

Здесь 2 раза сравнивается минимум с From

Здесь 2 раза сравнивается минимум с From
@ -0,0 +136,4 @@
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.NotNull(response.Content);
Assert.NotEmpty(response.Content);
Assert.Equal(take, response.Content.Count());
Owner

Здесь желательно проверить не только количество элементов, но и содержание (id, discriminatorId или что-то в этом роде): вдруг вернулся не тот элемент?

Здесь желательно проверить не только количество элементов, но и содержание (id, discriminatorId или что-то в этом роде): вдруг вернулся не тот элемент?
@ -0,0 +161,4 @@
}
[Fact]
public async Task InsertRange_returns_BadRequest()
Owner

AddRange_returns_BadRequest

AddRange_returns_BadRequest
@ -0,0 +193,4 @@
{
var dtos = new List<WitsDataDto>();
var timestamped = DateTimeOffset.UtcNow;
for (var i = 0; i < countToCreate && i < 100; i++)
Owner

i < countToCreate && i < 100
А это так и нужно?

i < countToCreate && i < 100 А это так и нужно?
Author
Collaborator

По сути нет, это на всякий случай - вдруг кому то когда нибудь придёт идея написать в этом контроллере тест и передать в качестве count >100. Хотя, наверное, лучше тогда человеку сразу увидеть ошибку, чем гадать - почему он говорит создать 150 записей, а создаётся 100 ))

По сути нет, это на всякий случай - вдруг кому то когда нибудь придёт идея написать в этом контроллере тест и передать в качестве count >100. Хотя, наверное, лучше тогда человеку сразу увидеть ошибку, чем гадать - почему он говорит создать 150 записей, а создаётся 100 ))
@ -0,0 +205,4 @@
{
RecordId = i + 1,
ItemId = i + 1,
Value = new Random().Next(1, 100)
Owner

Лучше new Random() вынести на уровень выше, чтобы в цикле не создавался каждый раз новый экземпляр

Лучше new Random() вынести на уровень выше, чтобы в цикле не создавался каждый раз новый экземпляр
@ -0,0 +27,4 @@
Min = group.Min(e => e.Timestamp),
Max = group.Max(e => e.Timestamp),
});
var values = await query.FirstOrDefaultAsync(token);
Owner

Тут values может быть null. И если будет null, то null нужно и вернуть. Соответственно, в нужных методах контроллера также необходимо будет поправить возвращаемый тип: вместо Ok возвращать NoContent.

PS: У меня тоже есть такой же метод GetDatesRangeAsync и Никита писал к этому методу вот такие замечания

Тут values может быть null. И если будет null, то null нужно и вернуть. Соответственно, в нужных методах контроллера также необходимо будет поправить возвращаемый тип: вместо Ok возвращать NoContent. PS: У меня тоже есть такой же метод GetDatesRangeAsync и Никита писал к этому методу вот такие замечания
Author
Collaborator

Сделал так: image
Мы же договаривались только для Post методов использовать Task<IActionResult> и ProducesResponseType

Сделал так: ![image](/attachments/35d115c7-e986-4cff-9b21-22c9e8df6877) <br> Мы же договаривались только для Post методов использовать `Task<IActionResult>` и `ProducesResponseType`
@ -0,0 +58,4 @@
var universalDateTo = dateTo.ToUniversalTime();
query = query
.Where(e => e.DiscriminatorId == discriminatorId && e.Timestamp >= universalDateFrom && e.Timestamp <= universalDateTo)
Owner

Тут для удобства чтения лучше разделиnь Where на 2 части:
.Where(e => e.DiscriminatorId == discriminatorId)
.Where(e => e.Timestamp >= universalDateFrom && e.Timestamp <= universalDateTo)

Тут для удобства чтения лучше разделиnь Where на 2 части: .Where(e => e.DiscriminatorId == discriminatorId) .Where(e => e.Timestamp >= universalDateFrom && e.Timestamp <= universalDateTo)
@ -0,0 +67,4 @@
var entities = await query
.Take((int)(2.5 * approxPointsCount))
.ToListAsync(token);
Owner

ToArrayAsync(token)

ToArrayAsync(token)
@ -0,0 +16,4 @@
/// <param name="take">количество записей</param>
/// <param name="token"></param>
/// <returns></returns>
Task<ActionResult<IEnumerable<TDto>>> GetPart(int idDiscriminator, DateTimeOffset dateBegin, int take = 24 * 60 * 60, CancellationToken token = default);
Owner

default у CancellationToken лучше убрать

default у CancellationToken лучше убрать
Author
Collaborator

Тогда и у take надо убирать
Иначе порядок придётся у них менять

Тогда и у take надо убирать Иначе порядок придётся у них менять
@ -0,0 +2,4 @@
namespace Persistence.Models.Configurations;
public class WitsInfo
Owner

Тут нужны комментарии, без комментариев не очень понятно

Тут нужны комментарии, без комментариев не очень понятно
@ -0,0 +18,4 @@
private const int multiplier = 1000;
private const string witsConfigPath = "Persistence.Services.Config.WitsConfig.json";
public WitsDataService(IParameterRepository witsDataRepository, IConfiguration configuration)
Owner

IConfiguration configuration можно удалить, он нигде не используется

IConfiguration configuration можно удалить, он нигде не используется
@ -0,0 +98,4 @@
var result = new List<WitsDataDto>();
foreach (var dto in dtos)
{
var witsDataDto = result.FirstOrDefault(e => e.DiscriminatorId == dto.DiscriminatorId && e.Timestamped == dto.Timestamp);
Owner

Тут тоже лучше разделить на 2 подзапроса:
result.Where(e => e.DiscriminatorId == dto.DiscriminatorId)
.Where(e => e.Timestamped == dto.Timestamp)
.FirstOrDefault();

Тут тоже лучше разделить на 2 подзапроса: result.Where(e => e.DiscriminatorId == dto.DiscriminatorId) .Where(e => e.Timestamped == dto.Timestamp) .FirstOrDefault();
@ -0,0 +117,4 @@
Value = ConvertValue(recordId, itemId, dto.Value)
};
witsDataDto.Values.Append(witsValueDto);
Owner
  1. Тут внутрь witsDataDto ничего не добавляется потому что Append возвращает новую последовательность, поэтому писать нужно так:
    witsDataDto.Values = witsDataDto.Values.Append(witsValueDto)

  2. Не совсем понятный код: цикл по dto, потом поиск еще не добавленных элементов внутрь result и Append.
    Это лучше бы отрефакторить. Ниже приложила вариант того, как можно читаемость (по моему мнению) улучшить.

  3. И еще параметр take. Желательно иметь значение по умолчанию, а то если пользователь нечаянно null отправит, то запрос ничего не выдаст

image

1. Тут внутрь witsDataDto ничего не добавляется потому что Append возвращает новую последовательность, поэтому писать нужно так: witsDataDto.Values = witsDataDto.Values.Append(witsValueDto) 2. Не совсем понятный код: цикл по dto, потом поиск еще не добавленных элементов внутрь result и Append. Это лучше бы отрефакторить. Ниже приложила вариант того, как можно читаемость (по моему мнению) улучшить. 3. И еще параметр take. Желательно иметь значение по умолчанию, а то если пользователь нечаянно null отправит, то запрос ничего не выдаст ![image](/attachments/0dc037f6-7cac-40a5-a8cf-2ca8c60acd5c)
rs.efremov added 1 commit 2024-12-10 17:10:30 +05:00
rs.efremov added 1 commit 2024-12-10 17:15:15 +05:00
rs.efremov added 2 commits 2024-12-11 09:37:33 +05:00
rs.efremov added 1 commit 2024-12-11 14:31:56 +05:00
on.nemtina merged commit f2ab363b6a into master 2024-12-12 16:11:56 +05:00
on.nemtina deleted branch WitsData 2024-12-12 16:11:56 +05:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: on.nemtina/persistence#7
No description provided.