Skip to content
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.

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

Feature/duplicate confirm handling #331

Merged
merged 6 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 11 additions & 3 deletions src/Altinn.Broker.API/Controllers/FileController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Altinn.Broker.Core.Domain;
using Altinn.Broker.Core.Domain.Enums;
using Altinn.Broker.Core.Models;
using Altinn.Broker.Core.Repositories;
using Altinn.Broker.Enums;
using Altinn.Broker.Helpers;
using Altinn.Broker.Mappers;
Expand All @@ -20,6 +21,8 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;

using OneOf;

namespace Altinn.Broker.Controllers
{
[ApiController]
Expand All @@ -28,10 +31,12 @@ namespace Altinn.Broker.Controllers
public class FileController : Controller
{
private readonly ILogger<FileController> _logger;
private readonly IIdempotencyEventRepository _idempotencyEventRepository;

public FileController(ILogger<FileController> logger)
public FileController(ILogger<FileController> logger, IIdempotencyEventRepository idempotencyEventRepository)
{
_logger = logger;
_idempotencyEventRepository = idempotencyEventRepository;
}

/// <summary>
Expand Down Expand Up @@ -250,11 +255,14 @@ public async Task<ActionResult> ConfirmDownload(
{
LogContextHelpers.EnrichLogsWithToken(token);
_logger.LogInformation("Confirming download for file {fileId}", fileId.ToString());
var commandResult = await handler.Process(new ConfirmDownloadCommandRequest()
var requestData = new ConfirmDownloadCommandRequest()
{
FileId = fileId,
Token = token
}, cancellationToken);
};
var processingFunction = new Func<Task<OneOf<Task, Error>>>(() => handler.Process(requestData, cancellationToken));
var uniqueString = $"confirmDownload_{fileId}_{token.Consumer}";
var commandResult = await IdempotencyEventHelper.ProcessEvent(uniqueString, processingFunction, _idempotencyEventRepository, cancellationToken);
return commandResult.Match(
(_) => Ok(null),
Problem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

using Newtonsoft.Json;

using OneOf;

namespace Altinn.Broker.Webhooks.Controllers
{
[ApiController]
Expand All @@ -19,13 +21,13 @@ public class MalwareScanResultsController : Controller
{
private readonly IFileRepository _fileRepository;
private readonly IFileStatusRepository _fileStatusRepository;
private readonly IWebhookEventRepository _webhookEventRepository;
private readonly IIdempotencyEventRepository _idempotencyEventRepository;

public MalwareScanResultsController(IFileRepository fileRepository, IFileStatusRepository fileStatusRepository, IWebhookEventRepository webhookEventRepository)
public MalwareScanResultsController(IFileRepository fileRepository, IFileStatusRepository fileStatusRepository, IIdempotencyEventRepository idempotencyEventRepository)
{
_fileRepository = fileRepository;
_fileStatusRepository = fileStatusRepository;
_webhookEventRepository = webhookEventRepository;
_idempotencyEventRepository = idempotencyEventRepository;
}

[HttpPost]
Expand All @@ -51,7 +53,8 @@ public async Task<ActionResult> ProcessMalwareScanResult([FromServices] MalwareS
{
string jsonString = eventGridEvent.Data.ToString();
ScanResultData result = JsonConvert.DeserializeObject<ScanResultData>(jsonString);
var commandResult = await WebhookEventHelper.ProcessMalwareEvent(result, handler, _webhookEventRepository, cancellationToken);
var processFunction = new Func<Task<OneOf<Task, Error>>>(() => handler.Process(result, cancellationToken));
var commandResult = await IdempotencyEventHelper.ProcessEvent(result.ETag, processFunction, _idempotencyEventRepository, cancellationToken);
return commandResult.Match(
Ok,
Problem
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
using Altinn.Broker.Application;
using Altinn.Broker.Application.ConfirmDownloadCommand;
using Altinn.Broker.Core.Domain;
using Altinn.Broker.Core.Repositories;

using OneOf;

namespace Altinn.Broker.Helpers;

public class WebhookEventHelper
public class IdempotencyEventHelper
{
public static async Task<OneOf<Task, Error>> ProcessMalwareEvent(ScanResultData data, MalwareScanningResultHandler handler, IWebhookEventRepository webhookEventRepository, CancellationToken cancellationToken)
public static async Task<OneOf<Task, Error>> ProcessEvent(string uniqueString, Func<Task<OneOf<Task, Error>>> process, IIdempotencyEventRepository idempotencyEventRepository, CancellationToken cancellationToken)
{
try
{

// Create a new entry for that webhook id
await webhookEventRepository.AddWebhookEventAsync(data.ETag, cancellationToken);
await idempotencyEventRepository.AddIdempotencyEventAsync(uniqueString, cancellationToken);
try
{
// Call you method
return await handler.Process(data, cancellationToken);
return await process();
}
catch (Exception e)
Andreass2 marked this conversation as resolved.
Show resolved Hide resolved
{
// Delete the entry on error to make sure the next one isn't ignored
await webhookEventRepository.DeleteWebhookEventAsync(data.ETag, cancellationToken);
return Task.CompletedTask;
await idempotencyEventRepository.DeleteIdempotencyEventAsync(uniqueString, cancellationToken);
throw;
}
}
catch (Npgsql.PostgresException e)
Expand Down
2 changes: 1 addition & 1 deletion src/Altinn.Broker.API/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ static void BuildAndRun(string[] args)
app.MapControllers();

app.UseHangfireDashboard();
app.Services.GetService<IRecurringJobManager>().AddOrUpdate<MalwareScanningResultHandler>("Delete old webhook events", handler => handler.DeleteOldWebhookEvents(), Cron.Weekly());
app.Services.GetService<IRecurringJobManager>().AddOrUpdate<IdempotencyService>("Delete old impotency events", handler => handler.DeleteOldIdempotencyEvents(), Cron.Weekly());

app.Run();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ public async Task<OneOf<Task, Error>> Process(ConfirmDownloadCommandRequest requ
{
return Errors.FileNotPublished;
}
if (file.RecipientCurrentStatuses.First(recipientStatus => recipientStatus.Actor.ActorExternalId == request.Token.Consumer).Status == ActorFileStatus.DownloadConfirmed)
{
return Task.CompletedTask;
}

await _actorFileStatusRepository.InsertActorFileStatus(request.FileId, ActorFileStatus.DownloadConfirmed, request.Token.Consumer, cancellationToken);
await _eventBus.Publish(AltinnEventType.DownloadConfirmed, file.ResourceId, file.FileId.ToString(), cancellationToken);
Expand Down
1 change: 1 addition & 0 deletions src/Altinn.Broker.Application/Errors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ public static class Errors
public static Error UploadFailed = new Error(10, "Error occurred while uploading file. See /details for more information.", HttpStatusCode.InternalServerError);
public static Error ChecksumMismatch = new Error(11, "The checksum of uploaded file did not match the checksum specified in initialize call.", HttpStatusCode.BadRequest);
public static Error FileNotPublished = new Error(12, "A file can only be confirmed to be downloaded when it is published. See file status.", HttpStatusCode.BadRequest);
public static Error FileAlreadyConfirmed = new Error(13, "The file has already been confirmed to be downloaded.", HttpStatusCode.BadRequest);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Altinn.Broker.Application;
public class MalwareScanningResultHandler : IHandler<ScanResultData, Task>
{
private readonly IFileStatusRepository _fileStatusRepository;
private readonly IWebhookEventRepository _webhookEventRepository;
private readonly IIdempotencyEventRepository _idempotencyEventRepository;
private readonly IFileRepository _fileRepository;
private readonly IBackgroundJobClient _backgroundJobClient;
private readonly IEventBus _eventBus;
Expand All @@ -26,14 +26,14 @@ public MalwareScanningResultHandler(
IFileRepository fileRepository,
IBackgroundJobClient backgroundJobClient,
IEventBus eventBus,
IWebhookEventRepository webhookEventRepository,
IIdempotencyEventRepository idempotencyEventRepository,
ILogger<MalwareScanningResultHandler> logger)
{
_fileStatusRepository = fileStatusRepository;
_fileRepository = fileRepository;
_backgroundJobClient = backgroundJobClient;
_eventBus = eventBus;
_webhookEventRepository = webhookEventRepository;
_idempotencyEventRepository = idempotencyEventRepository;
_logger = logger;
}

Expand Down Expand Up @@ -64,11 +64,4 @@ public async Task<OneOf<Task, Error>> Process(ScanResultData data, CancellationT
return Task.CompletedTask;
}
}
//Tells hangfire to avoid retries on failure
[AutomaticRetry(Attempts = 0)]
public async Task DeleteOldWebhookEvents()
{
_logger.LogInformation("Deleting old webhook events");
await _webhookEventRepository.DeleteOldWebhookEvents();
}
}
7 changes: 0 additions & 7 deletions src/Altinn.Broker.Core/Domain/WebhookEventEntity.cs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace Altinn.Broker.Core.Repositories
{
public interface IIdempotencyEventRepository
{
Task AddIdempotencyEventAsync(string id, CancellationToken cancellationToken);
Task DeleteIdempotencyEventAsync(string id, CancellationToken cancellationToken);
Task DeleteOldIdempotencyEvents();
}
}

This file was deleted.

2 changes: 1 addition & 1 deletion src/Altinn.Broker.Integrations/DependencyInjection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static void AddIntegrations(this IServiceCollection services)
services.AddSingleton<IFileStore, BlobService>();
services.AddScoped<IResourceRepository, AltinnResourceRegistryRepository>();
services.AddScoped<IAuthorizationService, AltinnAuthorizationService>();
services.AddScoped<IWebhookEventRepository, WebhookEventRepository>();
services.AddScoped<IIdempotencyEventRepository, IdempotencyEventRepository>();
services.AddScoped<IEventBus, AltinnEventBus>();
}
}
28 changes: 28 additions & 0 deletions src/Altinn.Broker.Integrations/Hangfire/IdempotencyService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System.Threading.Tasks;

using Altinn.Broker.Core.Repositories;

using Hangfire;

using Microsoft.Extensions.Logging;

public class IdempotencyService
{
private readonly IIdempotencyEventRepository _idempotencyEventRepository;
private readonly ILogger<IdempotencyService> _logger;

public IdempotencyService(
IIdempotencyEventRepository idempotencyEventRepository,
ILogger<IdempotencyService> logger)
{
_idempotencyEventRepository = idempotencyEventRepository;
_logger = logger;
}

[AutomaticRetry(Attempts = 0)]
public async Task DeleteOldIdempotencyEvents()
{
_logger.LogInformation("Deleting old idempotency events");
await _idempotencyEventRepository.DeleteOldIdempotencyEvents();
}
}
2 changes: 1 addition & 1 deletion src/Altinn.Broker.Persistence/DependencyInjection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ public static void AddPersistence(this IServiceCollection services)
services.AddSingleton<IFileStatusRepository, FileStatusRepository>();
services.AddSingleton<IActorFileStatusRepository, ActorFileStatusRepository>();
services.AddSingleton<IResourceOwnerRepository, ResourceOwnerRepository>();
services.AddSingleton<IWebhookEventRepository, WebhookEventRepository>();
services.AddSingleton<IIdempotencyEventRepository, IdempotencyEventRepository>();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
CREATE TABLE broker.idempotency_event (
idempotency_event_id_pk character varying(80) PRIMARY KEY,
created timestamp without time zone NOT NULL
);
Andreass2 marked this conversation as resolved.
Show resolved Hide resolved

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
ALTER TABLE broker.resource_owner
ADD resource_group_name uuid NOT NULL;
ADD resource_group_name uuid NOT NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
using Altinn.Broker.Core.Repositories;

using Npgsql;

namespace Altinn.Broker.Persistence.Repositories;

public class IdempotencyEventRepository : IIdempotencyEventRepository
{
private DatabaseConnectionProvider _connectionProvider;

public IdempotencyEventRepository(DatabaseConnectionProvider connectionProvider)
{
_connectionProvider = connectionProvider;
}


public async Task AddIdempotencyEventAsync(string IdempotencyEventId, CancellationToken cancellationToken)
{
NpgsqlCommand command = await _connectionProvider.CreateCommand(
"INSERT INTO broker.idempotency_event (idempotency_event_id_pk, created)" +
"VALUES (@idempotency_event_id_pk, @created) ");
command.Parameters.AddWithValue("@idempotency_event_id_pk", IdempotencyEventId);
command.Parameters.AddWithValue("@created", DateTime.UtcNow);

await command.ExecuteNonQueryAsync(cancellationToken);
}
public async Task DeleteIdempotencyEventAsync(string IdempotencyEventId, CancellationToken cancellationToken)
{
NpgsqlCommand command = await _connectionProvider.CreateCommand(
"DELETE FROM broker.idempotency_event " +
"WHERE idempotency_event_id_pk = @idempotency_event_id_pk");
command.Parameters.AddWithValue("@idempotency_event_id_pk", IdempotencyEventId);

await command.ExecuteNonQueryAsync(cancellationToken);
}
public async Task DeleteOldIdempotencyEvents()
{
NpgsqlCommand command = await _connectionProvider.CreateCommand(
"DELETE FROM broker.idempotency_event " +
"WHERE created < @created");

command.Parameters.AddWithValue("@created", DateTime.UtcNow.AddDays(-1));

await command.ExecuteNonQueryAsync();
}
}

This file was deleted.

2 changes: 2 additions & 0 deletions start.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
docker compose up -d
dotnet watch --project ./src/Altinn.Broker.API/Altinn.Broker.API.csproj
Loading