Skip to content
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0cfc3b4
EnableRetryOnFailure does not handle explicit transactions properly
Jun 19, 2026
deee1dc
Adjusted retry attempts down to 5 with exponential backoff
Jun 19, 2026
6897cb2
Added tests to verify all the retry behaviour for database transactions
Jun 19, 2026
66d22df
Complete re-factor to EnableRetryOnFailure
Jun 19, 2026
7e9e570
Minor fixes
Jun 19, 2026
e7db6a8
Limit retry count when running tests
Jun 19, 2026
3d622f4
Revert "Limit retry count when running tests"
Jun 19, 2026
7866d85
Remove PostgresTestcontainerFixture and rely on same postgres server …
Jun 19, 2026
4194b1f
Try adjusting pool size to make tests more reliable
Jun 19, 2026
764d394
Fix remaning old style transactions
Jun 19, 2026
4c7ae00
Wait longer for attachment upload to make tests less flaky
Jun 22, 2026
3d685f2
Pass in cancellationToken
Jun 22, 2026
4d61e00
Do not upload to blob inside retry
Jun 22, 2026
84c3350
Fix OpsGenie
Jun 22, 2026
f8a55a0
Deferring saving changes to avoid undue database load, and also re-fa…
Jun 25, 2026
32888b2
Add tests
Jun 25, 2026
edd2a3a
Added tests for uniqueness violation
Jun 25, 2026
fc3f35b
Fixed upload case
Jun 25, 2026
ae715fc
Re-wrote to be explicit about SaveChanges wherever needed
Jun 25, 2026
92241da
Merge branch 'main' into feat/deferred-save-changes
Ceredron Jun 26, 2026
64c20bb
Fix build
Jun 26, 2026
6187bd6
Try to make tests less flaky
Jun 26, 2026
af9b7ab
Fix test
Jun 26, 2026
90b0569
Fixes from comments
Jun 26, 2026
fba8a1a
Loosen up a bit on the polling
Jun 26, 2026
0092f36
Merge branch 'main' into feat/deferred-save-changes
Aug 4, 2026
c01357f
Merge from main
Aug 10, 2026
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
using Altinn.Correspondence.Core.Services;
using Altinn.Correspondence.Integrations.Dialogporten;
using Altinn.Correspondence.Integrations.Dialogporten.Models;
using Altinn.Correspondence.Persistence;
using Altinn.Correspondence.Tests.Helpers;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Moq;
Expand All @@ -21,6 +24,8 @@ namespace Altinn.Correspondence.Tests.Dialogporten;

public class DialogportenServiceTests
{
private static ApplicationDbContext CreateTestDbContext() => TestDbContextFactory.Create();

private static (DialogportenService service, Func<string> getLastRequestBody) CreateServiceWithMockedDialogPost(CorrespondenceEntity correspondence)
{
var capturedRequestBody = string.Empty;
Expand Down Expand Up @@ -76,6 +81,7 @@ private static (DialogportenService service, Func<string> getLastRequestBody) Cr
mockLogger.Object,
mockIdem.Object,
mockResourceRegistryService.Object,
CreateTestDbContext(),
mockPartyUrnHelper.Object);
return (service, () => capturedRequestBody);
}
Expand Down Expand Up @@ -141,6 +147,7 @@ private static (DialogportenService service, Mock<ICorrespondenceForwardingEvent
mockLogger.Object,
mockIdem.Object,
mockResourceRegistryService.Object,
CreateTestDbContext(),
mockPartyUrnHelper.Object);
return (service, mockCorrespondenceForwardingEventRepository, mockAltinnRegisterService, () => capturedRequestBody);
}
Expand Down Expand Up @@ -174,6 +181,7 @@ private static (DialogportenService service, Mock<ICorrespondenceRepository> rep
var mockAltinnRegisterService = new Mock<IAltinnRegisterService>();
var mockPartyUrnHelper = new Mock<PartyUrnHelper>(mockAltinnRegisterService.Object, Mock.Of<ILogger<PartyUrnHelper>>());
var options = Options.Create(new GeneralSettings { CorrespondenceBaseUrl = "https://correspondence.example" });
var dbContext = TestDbContextFactory.Create();

var service = new DialogportenService(
httpClient,
Expand All @@ -184,6 +192,7 @@ private static (DialogportenService service, Mock<ICorrespondenceRepository> rep
Mock.Of<ILogger<DialogportenService>>(),
Mock.Of<IIdempotencyKeyRepository>(),
Mock.Of<IResourceRegistryService>(),
dbContext,
mockPartyUrnHelper.Object);

return (service, mockRepo);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using Altinn.Correspondence.Persistence;
using Microsoft.EntityFrameworkCore;
using Npgsql;

namespace Altinn.Correspondence.Tests.Fixtures;

/// <summary>
/// Simulates a concurrent idempotency insert losing the race on the batched flush inside
/// <see cref="Altinn.Correspondence.Application.Helpers.DatabaseTransactionHelper.ExecuteAsync"/>.
/// </summary>
public sealed class UniqueViolationOnDeferredSaveDbContext(
DbContextOptions<ApplicationDbContext> options,
int uniqueViolationOnSaveAttempt = 1)
: TestApplicationDbContext(options)
{
private int _saveAttempts;

public override async Task<int> SaveChangesAsync(CancellationToken cancellationToken = default)
{
_saveAttempts++;
if (_saveAttempts == uniqueViolationOnSaveAttempt)
{
throw new DbUpdateException(
"duplicate key",
new PostgresException(
"duplicate key value violates unique constraint",
"ERROR",
"ERROR",
"23505"));
}

return await base.SaveChangesAsync(cancellationToken);
}
}
4 changes: 2 additions & 2 deletions Test/Altinn.Correspondence.Tests/Helpers/AttachmentHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ public static string CalculateChecksum(byte[] data)
return BitConverter.ToString(hash).Replace("-", "").ToLower();
}
}
public static async Task<AttachmentOverviewExt> WaitForAttachmentStatusUpdate(HttpClient client, JsonSerializerOptions responseSerializerOptions, Guid attachmentId, AttachmentStatusExt expectedStatus, int maxRetries = 5, int delayMs = 900)
public static async Task<AttachmentOverviewExt> WaitForAttachmentStatusUpdate(HttpClient client, JsonSerializerOptions responseSerializerOptions, Guid attachmentId, AttachmentStatusExt expectedStatus, int maxRetries = 10, int delayMs = 1000)
{
await Task.Delay(500);
await Task.Delay(1000);
for (int i = 0; i < maxRetries; i++)
{
var attachment = await client.GetFromJsonAsync<AttachmentOverviewExt>($"correspondence/api/v1/attachment/{attachmentId}", responseSerializerOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public static MultipartFormDataContent CorrespondenceToFormData(BaseCorresponden
return formData;
}

public static async Task<CorrespondenceOverviewExt> WaitForCorrespondenceStatusUpdate(HttpClient client, JsonSerializerOptions responseSerializerOptions, Guid correspondenceId, CorrespondenceStatusExt expectedStatus, int maxRetries = 5, int delayMs = 900)
public static async Task<CorrespondenceOverviewExt> WaitForCorrespondenceStatusUpdate(HttpClient client, JsonSerializerOptions responseSerializerOptions, Guid correspondenceId, CorrespondenceStatusExt expectedStatus, int maxRetries = 10, int delayMs = 1000)
{
await Task.Delay(200);
for (int i = 0; i < maxRetries; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ protected override void ConfigureWebHost(
.AddJsonFile("appsettings.Development.json")
.AddInMemoryCollection(new Dictionary<string, string?>
{
["GeneralSettings:MalwareScanBypassWhiteList"] = TestConstants.ResourceWhitelistedForMalwareScanBypass
["GeneralSettings:MalwareScanBypassWhiteList"] = TestConstants.ResourceWhitelistedForMalwareScanBypass,
["DatabaseOptions:ConnectionString"] = "Host=localhost:5432;Username=postgres;Password=postgres;Database=correspondence;Maximum Pool Size=50;Timeout=30"
})
.Build());

Expand Down Expand Up @@ -79,7 +80,6 @@ protected override void ConfigureWebHost(
services.AddHangfireServer(options =>
{
options.SchedulePollingInterval = TimeSpan.FromSeconds(1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C 4 \
  'WorkerCount|QueuePollInterval|SchedulePollingInterval|WaitFor|WaitUntil|Timeout|TimeSpan\.From' \
  Test/Altinn.Correspondence.Tests/Helpers \
  Test/Altinn.Correspondence.Tests/TestingHandler \
  Test/Altinn.Correspondence.Tests/TestingController

Repository: Altinn/altinn-correspondence

Length of output: 50385


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate CustomWebApplicationFactory =="
fd -a 'CustomWebApplicationFactory\.cs$' .

echo "== Outline helper files =="
ast-grep outline Test/Altinn.Correspondence.Tests/Helpers/CustomWebApplicationFactory.cs --view expanded || true

echo "== Read CustomWebApplicationFactory =="
cat -n Test/Altinn.Correspondence.Tests/Helpers/CustomWebApplicationFactory.cs

echo "== Search WorkerCount usage globally =="
rg -n 'WorkerCount|UseSimpleAssemblyName|UseSqlStorage|QueuePoll|SchedulePollingInterval|GetDefaultWorkerCount|DefaultWorker' .

Repository: Altinn/altinn-correspondence

Length of output: 18915


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Dependency injection paths =="
cat -n src/Altinn.Correspondence.Integrations/Hangfire/DependencyInjection.cs | sed -n '1,120p'

echo "== Search for deterministic wait helpers across tests/helpers =="
rg -n 'WaitFor|WaitUntil|Task\.Delay|Thread\.Sleep|Timeout|Assert\.Wait|Poll|PollUntil|Stopwatch|StopAsync|RunAsync|AddHangfireServer|Hangfire' \
  Test/Altinn.Correspondence.Tests \
  Test -g '*.cs' \
  --glob '!**/obj/**' --glob '!**/bin/**' --max-count 200

Repository: Altinn/altinn-correspondence

Length of output: 50384


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== CorrespondenceHelper poll implementation =="
cat -n Test/Altinn.Correspondence.Tests/Helpers/CorrespondenceHelper.cs | sed -n '1,150p'

echo "== AttachmentHelper poll implementation =="
cat -n Test/Altinn.Correspondence.Tests/Helpers/AttachmentHelper.cs | sed -n '1,160p'

echo "== Precise WorkerCount occurrences across cs files =="
rg -n 'WorkerCount|BackgroundJobServerOptions|AddHangfireServer\(' . -g '*.cs' --glob '!**/obj/**' --glob '!**/bin/**'

Repository: Altinn/altinn-correspondence

Length of output: 18822


Set a test-only Hangfire worker count.

CustomWebApplicationFactory no longer sets WorkerCount, so the test server now uses Hangfire’s default. This test assembly already uses explicit worker counts where concurrency/ordering is relevant, so add an explicit test-only value for this configuration too.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Test/Altinn.Correspondence.Tests/Helpers/CustomWebApplicationFactory.cs` at
line 82, Update the Hangfire configuration in CustomWebApplicationFactory to
explicitly set a test-only WorkerCount alongside SchedulePollingInterval, using
the established worker-count value or convention already used by this test
assembly.

options.WorkerCount = 5;
options.Queues = new[] { HangfireQueues.Default, HangfireQueues.LiveMigration, HangfireQueues.Migration };
options.ServerTimeout = TimeSpan.FromSeconds(30);
options.ShutdownTimeout = TimeSpan.FromSeconds(5);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Altinn.Correspondence.Persistence;
using Altinn.Correspondence.Persistence.Helpers;
using Altinn.Correspondence.Tests.Fixtures;
using Altinn.Correspondence.Tests.Fixtures;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage;
using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure;
Expand All @@ -17,6 +18,9 @@ public static class TestDbContextFactory

public static TestApplicationDbContext Create() => new(Options.Value);

public static UniqueViolationOnDeferredSaveDbContext CreateUniqueViolationOnDeferredSave(int onAttempt = 1)
=> new(Options.Value, onAttempt);

public static TestApplicationDbContext Create(int maxRetryCount)
{
var options = new DbContextOptionsBuilder<ApplicationDbContext>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1326,6 +1326,32 @@ public async Task InitializeCorrespondence_WithDuplicateIdempotentKey_ReturnsCon
Assert.Contains(CorrespondenceErrors.DuplicateInitCorrespondenceRequest.Message, errorContent);
}

[Fact]
public async Task InitializeCorrespondence_WithDuplicateIdempotentKey_SkipsValidation()
{
var idempotentKey = Guid.NewGuid();
var validCorrespondence = new CorrespondenceBuilder()
.CreateCorrespondence()
.WithIdempotentKey(idempotentKey)
.Build();

var firstResponse = await _senderClient.PostAsJsonAsync("correspondence/api/v1/correspondence", validCorrespondence);
Assert.Equal(HttpStatusCode.OK, firstResponse.StatusCode);

var invalidIfValidated = new CorrespondenceBuilder()
.CreateCorrespondence()
.WithIdempotentKey(idempotentKey)
.WithExternalReferencesDialogId("not-a-guid")
.Build();

var duplicateResponse = await _senderClient.PostAsJsonAsync("correspondence/api/v1/correspondence", invalidIfValidated);

Assert.Equal(HttpStatusCode.Conflict, duplicateResponse.StatusCode);
var errorContent = await duplicateResponse.Content.ReadAsStringAsync();
Assert.Contains(CorrespondenceErrors.DuplicateInitCorrespondenceRequest.Message, errorContent);
Assert.DoesNotContain(CorrespondenceErrors.InvalidCorrespondenceDialogId.Message, errorContent);
}

[Fact]
public async Task InitializeCorrespondence_WithDifferentContentAndSameIdempotentKey_ShouldReturnConflict()
{
Expand Down Expand Up @@ -2555,10 +2581,20 @@ public async Task WhenDialogCreationFails_PublishCorrespondenceHandlerIsStillSch
Statuses = new List<CorrespondenceStatusEntity>()
});

var idempotencyKeyRepositoryMock = new Mock<IIdempotencyKeyRepository>();
idempotencyKeyRepositoryMock
.Setup(x => x.GetByIdAsync(It.IsAny<Guid>(), It.IsAny<CancellationToken>()))
.ReturnsAsync((IdempotencyKeyEntity?)null);
idempotencyKeyRepositoryMock
.Setup(x => x.CreateAsync(It.IsAny<IdempotencyKeyEntity>(), It.IsAny<CancellationToken>()))
.ReturnsAsync((IdempotencyKeyEntity entity, CancellationToken _) => entity);

var helper = new HangfireScheduleHelper(
scheduleClientMock.Object,
hybridCacheWrapperMock.Object,
scheduleRepositoryMock.Object,
idempotencyKeyRepositoryMock.Object,
TestDbContextFactory.Create(),
new Mock<ILogger<HangfireScheduleHelper>>().Object);

await helper.SchedulePublishAfterDialogCreated(correspondenceId, CancellationToken.None);
Expand Down Expand Up @@ -2586,12 +2622,15 @@ s is AwaitingState &&
var correspondenceRepositoryMock = new Mock<ICorrespondenceRepository>();
var correspondenceStatusRepositoryMock = new Mock<ICorrespondenceStatusRepository>();
var altinnRegisterServiceMock = new Mock<IAltinnRegisterService>();
var idempotencyKeyRepositoryMock = new Mock<IIdempotencyKeyRepository>();
var publishIdempotencyKeyRepositoryMock = new Mock<IIdempotencyKeyRepository>();

publishClientMock
.Setup(x => x.Create(It.IsAny<Job>(), It.IsAny<IState>()))
.Returns(() => Guid.NewGuid().ToString());
idempotencyKeyRepositoryMock
publishIdempotencyKeyRepositoryMock
.Setup(x => x.GetByIdAsync(It.IsAny<Guid>(), It.IsAny<CancellationToken>()))
.ReturnsAsync((IdempotencyKeyEntity?)null);
publishIdempotencyKeyRepositoryMock
.Setup(x => x.CreateAsync(It.IsAny<IdempotencyKeyEntity>(), It.IsAny<CancellationToken>()))
.ReturnsAsync((IdempotencyKeyEntity key, CancellationToken _) => key);

Expand Down Expand Up @@ -2636,7 +2675,7 @@ s is AwaitingState &&
correspondenceStatusRepositoryMock.Object,
new Mock<IContactReservationRegistryService>().Object,
publishClientMock.Object,
idempotencyKeyRepositoryMock.Object,
publishIdempotencyKeyRepositoryMock.Object,
TestDbContextFactory.Create());

await publishHandler.Process(correspondenceId, null, CancellationToken.None);
Expand Down
Loading
Loading