Conversation
Не судите строго, я pycharman
С каждой строчкой я все дальше от бога
Секрет Гудини
Сделал исправления Пожалуйста, не ругайтесь на C#-инвалида (на меня)
Добавил недостающее summary
Скорее всего где-то накосячил, не ругайтесь на меня
Потихоньку делаю
Написал всякой фигни, понял, что не работает, теперь переделываю
Посмотрел не свой вариант и увидел, что у меня Minio, а не Localstack
Пока не готово, но уже что-то
| IConfiguration configuration, | ||
| ILogger<SnsPublisherService> logger) | ||
| { | ||
| private readonly string? _topicArn = configuration["SNS:TopicArn"] ?? "arn:aws:sns:us-east-1:000000000000:employee-events"; |
There was a problem hiding this comment.
Не надо хардкодить параметры работы бекенда. У любого безопасника, который будет прогонять статический анализатор по твоему коду, произойдет кровоизлияние в мозг от этой строки.
Пароли и апи ключи же ты не хардкодишь, а чем уникальный глобальный идентификатор aws-ного ресурса хуже?
Лучше выкинуть исключение, что требуемых параметров в конфигурации не нашлось
There was a problem hiding this comment.
Я стесняюсь спросить, а что делать людям, которые не на винде будут поднимать твои сервисы?
Ладно, пофигу, оставляй так. Но это одна из причин, почему я советовал пользоваться каким-нибудь IaC-ом типа CloudFormation
There was a problem hiding this comment.
А как же описание проекта...
| app.MapPost("/api/sns/notification", async (HttpRequest request, IStorageService storage, ILogger<Program> logger) => | ||
| { | ||
| using var reader = new StreamReader(request.Body); | ||
| var body = await reader.ReadToEndAsync(); | ||
|
|
||
| using var doc = JsonDocument.Parse(body); | ||
| var type = doc.RootElement.GetProperty("Type").GetString(); | ||
|
|
||
| if (type == "SubscriptionConfirmation") | ||
| { | ||
| var subscribeUrl = doc.RootElement.GetProperty("SubscribeURL").GetString(); | ||
| logger.LogInformation("������������� ��������: {Url}", subscribeUrl); | ||
|
|
||
| using var client = new HttpClient(); | ||
| await client.GetAsync(subscribeUrl); | ||
|
|
||
| return Results.Ok(new { message = "Subscription confirmed" }); | ||
| } | ||
|
|
||
| if (type == "Notification") | ||
| { | ||
| var messageJson = doc.RootElement.GetProperty("Message").GetString(); | ||
| var employee = JsonSerializer.Deserialize<Employee>(messageJson, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); | ||
|
|
||
| if (employee != null) | ||
| { | ||
| var fileName = $"employee_{employee.Id}.json"; | ||
| var content = JsonSerializer.SerializeToUtf8Bytes(employee); | ||
| await storage.SaveFileAsync(bucketName, fileName, content); | ||
| logger.LogInformation("��������� {Id} �������� � MinIO", employee.Id); | ||
| } | ||
| } | ||
|
|
||
| return Results.Ok(); | ||
| }); |
There was a problem hiding this comment.
Метод на 34 строки - это нечто противоположное концепции minimal api. Предлагаю либо вынести этот метод в отдельную службу, либо сделать классический контроллер
There was a problem hiding this comment.
Тесты уже выглядят гораздо лучше, но главного теста я так и не вижу - чтобы дергался эндпоинт гейтвея с определенным id, а потом проверялось наличие файла с этим генерацией по этому же id в s3 хранилище
|
Спасибо большое, обязательно зайду на лекцию завтра, чтобы все исправить нормально и больше не мозолить вам глаза своим PR. С Пасхой! |
Вроде, я поправил все, что нужно было, все сохраняется, swagger добавил для fileservice, сделал end to end тест, и изменил фикстуру. Вроде, даже праймари конструкторы везде... Надеюсь, я не зря не спал эту ночь и еще лекцию прослушал, и ваш код посмотрел... Здоровья всем нам
| /// <param name="generator">Генератор сотрудников.</param> | ||
| /// <param name="cache">Кэш Redis.</param> | ||
| /// <param name="snsPublisher">Публикатор SNS.</param> | ||
| /// <param name="logger">Логгер.</param> |
There was a problem hiding this comment.
Ну вот это ты, конечно, зря снес
| private static readonly JsonSerializerOptions _jsonOptions = new() { PropertyNameCaseInsensitive = true }; | ||
|
|
||
| private readonly string _topicArn = configuration["SNS:TopicArn"] | ||
| ?? "arn:aws:sns:us-east-1:000000000000:employee-events"; |
There was a problem hiding this comment.
Повторяю, не надо хардкодить арны
| .WithEndpoint(port: 9000, targetPort: 9000, name: "api", scheme: "http") | ||
| .WithEndpoint(port: 9001, targetPort: 9001, name: "console", scheme: "http"); | ||
| .WithEndpoint(port: 9000, targetPort: 9000, scheme: "http", name: "minio-api") | ||
| .WithEndpoint(port: 9001, targetPort: 9001, scheme: "http", name: "minio-console"); |
There was a problem hiding this comment.
Для минио есть нугет-пакет с интеграцией под аспаер
| var localstack = builder.AddContainer("localstack", "localstack/localstack:3.8.0") | ||
| .WithEndpoint(port: 4566, targetPort: 4566, name: "localstack", scheme: "http") | ||
| .WithEndpoint(port: 4566, targetPort: 4566, scheme: "http", name: "localstack") | ||
| .WithEnvironment("SERVICES", "sns") | ||
| .WithEnvironment("AWS_ACCESS_KEY_ID", "test") | ||
| .WithEnvironment("AWS_SECRET_ACCESS_KEY", "test") | ||
| .WithEnvironment("AWS_DEFAULT_REGION", "us-east-1"); | ||
| .WithEnvironment("AWS_DEFAULT_REGION", "us-east-1") | ||
| .WithEnvironment("SKIP_SSL_CERT_DOWNLOAD", "1") | ||
| .WithEnvironment("LOCALSTACK_HOST", "localhost.localstack.cloud") | ||
| .WithLifetime(ContainerLifetime.Persistent); |
There was a problem hiding this comment.
Для локалстака есть нугет-пакет с интеграцией под аспаер
| /// <param name="minioClient">Клиент MinIO.</param> | ||
| /// <param name="logger">Логгер.</param> |
There was a problem hiding this comment.
Зачем сносить описания параметров? С ними все в порядке
| { | ||
| await minioClient.MakeBucketAsync(new MakeBucketArgs().WithBucket(bucket)); | ||
| Console.WriteLine($"����� {bucket} ������"); | ||
| logger.LogInformation("Bucket {BucketName} created successfully", bucket); |
There was a problem hiding this comment.
Как будто подобный код просится быть вынесенным в IStorageService в виде отдельного метода
There was a problem hiding this comment.
На старте этого приложения я бы вызвал инициализацию подписки на sns
| /// <summary> | ||
| /// Однократно инициализирует SNS топик и подписку в LocalStack. | ||
| /// </summary> | ||
| public async Task InitializeSnsOnce() |
There was a problem hiding this comment.
Чтобы, как минимум, в интеграционных тестах не заниматься этим
| return metadata != null ? Results.Ok(metadata) : Results.NotFound(); | ||
| }); | ||
|
|
||
| app.Run(); No newline at end of file |
Я устал, босс...


ФИО: Казаков Андрей
Номер группы: 6513
Номер лабораторной: 3
Номер варианта: 43
Краткое описание предметной области: Сотрудник компании
Краткое описание добавленных фич: Добавлен SNS брокер + MinIO хранилище + тесты