-
Notifications
You must be signed in to change notification settings - Fork 4
Cleanup job for Missing Notification Events in Dialogporten #2018
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
Open
RagnarFatland
wants to merge
21
commits into
main
Choose a base branch
from
fix/cleanupjobforNotifications
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
cedd82b
Initial checkin of cleanup job for notifications
bffc705
Merge branch 'main' into fix/cleanupjobforNotifications
4a6adfc
Changed to only use required entities and tweaked logging. Moved from…
39d57cc
Add handler and controller tests
d9d21a7
Changed cursor fo NotificationSent + CorrespondenceId.
75b0c59
tweaked tests
12c3932
Refactor to do duplication check against exisitng dialog.
afb041f
Merge branch 'main' into fix/cleanupjobforNotifications
2e05370
Refactor to follow new batch framework.
b196e18
Fixed unit tests
81f1b23
Apply suggestions from code review
RagnarFatland 787c465
attempt to fix failing test run in Github
b63b325
Merge branch 'fix/cleanupjobforNotifications' of https://github.com/A…
db2f953
Multiple fixes after Coderabbit comments
7a30da8
attempt merge from Main; failing unit tests.
43da1c3
fix migration issue that was causing unit test failures
0502400
Tweaks suggested by coderabbit.
b95d78f
minor null guard.
64ea547
fix error in sql script in docs
fadce1a
Made StartDate required parameter, and adjusted unit tests to verify …
a3a89f2
Fix comments from Coderabbit; added unit test for missing error case,…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
238 changes: 238 additions & 0 deletions
238
...ndence.Tests/TestingController/Maintenance/CleanupMissingSyncedNotificationEventsTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,238 @@ | ||
| using Altinn.Correspondence.Application.BatchJobs; | ||
| using Altinn.Correspondence.Application.CleanupMissingSyncedNotificationsBatch; | ||
| using Altinn.Correspondence.Common.Constants; | ||
| using Altinn.Correspondence.Core.Repositories; | ||
| using Altinn.Correspondence.Core.Services; | ||
| using Altinn.Correspondence.Tests.Factories; | ||
| using Altinn.Correspondence.Tests.Fixtures; | ||
| using Altinn.Correspondence.Tests.Helpers; | ||
| using Hangfire; | ||
| using Hangfire.Common; | ||
| using Hangfire.States; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Logging; | ||
| using Moq; | ||
| using System.Net; | ||
| using System.Net.Http.Json; | ||
| using System.Text.Json; | ||
|
|
||
| namespace Altinn.Correspondence.Tests.TestingController.Maintenance; | ||
|
|
||
| [Collection(nameof(CustomWebApplicationTestsCollection))] | ||
| public class CleanupMissingSyncedNotificationEventsTests | ||
| { | ||
| private readonly CustomWebApplicationFactory _factory; | ||
| private readonly HttpClient _maintenanceClient; | ||
| private readonly JsonSerializerOptions _responseSerializerOptions; | ||
| private const string MaintenanceControllerBaseUrl = "correspondence/api/v1/maintenance"; | ||
|
|
||
| public CleanupMissingSyncedNotificationEventsTests(CustomWebApplicationFactory factory) | ||
| { | ||
| _factory = factory; | ||
| _maintenanceClient = _factory.CreateClientWithAddedClaims(("scope", AuthorizationConstants.MaintenanceScope)); | ||
|
|
||
| _responseSerializerOptions = new JsonSerializerOptions(new JsonSerializerOptions() | ||
| { | ||
| PropertyNameCaseInsensitive = true | ||
| }); | ||
| _responseSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CleanupMissingSyncedNotificationEvents_WithDefaultParameters_ReturnsOk() | ||
| { | ||
| // Arrange | ||
| var url = $"{MaintenanceControllerBaseUrl}/cleanup-missing-synced-notification-events"; | ||
|
|
||
| // Act | ||
| var response = await _maintenanceClient.PostAsync(url, null); | ||
|
|
||
| // Assert | ||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
|
|
||
| var content = await response.Content.ReadAsStringAsync(); | ||
| var result = JsonSerializer.Deserialize<JsonElement>(content, _responseSerializerOptions); | ||
|
|
||
| Assert.True(result.TryGetProperty("message", out var message)); | ||
| Assert.Equal("Notification events cleanup started", message.GetString()); | ||
| Assert.True(result.TryGetProperty("batchCount", out var batchCount)); | ||
| Assert.Equal(100, batchCount.GetInt32()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CleanupMissingSyncedNotificationEvents_WithCustomBatchCount_UsesProvidedValue() | ||
| { | ||
| // Arrange | ||
| var customBatchCount = 50; | ||
| var url = $"{MaintenanceControllerBaseUrl}/cleanup-missing-synced-notification-events?batchCount={customBatchCount}"; | ||
|
|
||
| // Act | ||
| var response = await _maintenanceClient.PostAsync(url, null); | ||
|
|
||
| // Assert | ||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
|
|
||
| var content = await response.Content.ReadAsStringAsync(); | ||
| var result = JsonSerializer.Deserialize<JsonElement>(content, _responseSerializerOptions); | ||
|
|
||
| Assert.True(result.TryGetProperty("batchCount", out var batchCount)); | ||
| Assert.Equal(customBatchCount, batchCount.GetInt32()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CleanupMissingSyncedNotificationEvents_BatchSize50_2NotificationEvents_ProcessedSuccessfully() | ||
| { | ||
| // Arrange | ||
| var url = $"{MaintenanceControllerBaseUrl}/cleanup-missing-synced-notification-events?batchCount=50"; | ||
|
|
||
| // Create test correspondences with notifications that need cleanup | ||
| var correspondence1 = await CreateCorrespondenceWithSyncedNotifications(2, new DateTime(2024, 1, 5)); | ||
| var correspondence2 = await CreateCorrespondenceWithSyncedNotifications(3, new DateTime(2024, 1, 10)); | ||
|
|
||
| // Act | ||
| var response = await _maintenanceClient.PostAsync(url, null); | ||
|
|
||
| // Assert | ||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | ||
|
|
||
| var content = await response.Content.ReadAsStringAsync(); | ||
| var result = JsonSerializer.Deserialize<JsonElement>(content, _responseSerializerOptions); | ||
|
|
||
| // Verify all expected properties are present | ||
| Assert.True(result.TryGetProperty("message", out var message)); | ||
| Assert.Equal("Notification events cleanup started", message.GetString()); | ||
|
|
||
| Assert.True(result.TryGetProperty("batchCount", out var batchCount)); | ||
| Assert.Equal(50, batchCount.GetInt32()); | ||
|
|
||
| Assert.True(result.TryGetProperty("startingFrom", out _)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CleanupMissingSyncedNotificationEvents_VerifiesHangfireJobsEnqueued() | ||
| { | ||
| // Arrange - Create test correspondences with notifications first | ||
| var correspondence1 = await CreateCorrespondenceWithSyncedNotifications(2, new DateTime(2024, 1, 5)); | ||
|
|
||
| // Create a mock IBackgroundJobClient to capture enqueued jobs | ||
| var mockBackgroundJobClient = new Mock<IBackgroundJobClient>(); | ||
| var enqueuedJobs = new List<(Type serviceType, string methodName)>(); | ||
|
|
||
| // Capture all Create calls (Enqueue extension method calls Create internally) | ||
| mockBackgroundJobClient | ||
| .Setup(x => x.Create(It.IsAny<Job>(), It.IsAny<IState>())) | ||
| .Returns<Job, IState>((job, state) => | ||
| { | ||
| enqueuedJobs.Add((job.Type, job.Method.Name)); | ||
| return Guid.NewGuid().ToString(); | ||
| }); | ||
|
|
||
| // Get the handler dependencies from the factory | ||
| // Keep scope alive until after handler completes to prevent DbContext disposal | ||
| using var scope = _factory.Services.CreateScope(); | ||
| var notificationRepository = scope.ServiceProvider.GetRequiredService<ICorrespondenceNotificationRepository>(); | ||
| var batchJob = new CleanupMissingSyncedNotificationsBatchJob( | ||
| notificationRepository, | ||
| mockBackgroundJobClient.Object, | ||
| Mock.Of<ILogger<CleanupMissingSyncedNotificationsBatchJob>>()); | ||
| var orchestrator = scope.ServiceProvider.GetRequiredService<ChainedBatchJobOrchestrator>(); | ||
| var handlerLogger = scope.ServiceProvider.GetRequiredService<ILogger<CleanupMissingSyncedNotificationsBatchHandler>>(); | ||
|
|
||
| // Create handler with orchestrator | ||
| var handler = new CleanupMissingSyncedNotificationsBatchHandler( | ||
| mockBackgroundJobClient.Object, | ||
| orchestrator, | ||
| batchJob, | ||
| handlerLogger); | ||
|
|
||
| // Act - Call ExecuteBatch directly to actually run the batch processing logic | ||
| var request = new CleanupMissingSyncedNotificationsBatchRequest | ||
| { | ||
| BatchSize = 50, | ||
| CursorNotificationSent = DateTimeOffset.MaxValue, | ||
| CursorId = null | ||
| }; | ||
| await handler.ExecuteBatch(request, CancellationToken.None); | ||
|
|
||
| // Assert - Verify that jobs were enqueued (scope still alive here) | ||
| Assert.NotEmpty(enqueuedJobs); | ||
|
|
||
| // Should have enqueued AddNotificationActivitiesWithDuplicateCheck jobs for correspondences | ||
| // (may include notifications from other tests in shared database, so we check for "at least") | ||
| var addNotificationActivityJobs = enqueuedJobs | ||
| .Where(j => j.serviceType == typeof(IDialogportenService) && j.methodName == nameof(IDialogportenService.AddNotificationActivitiesWithDuplicateCheck)) | ||
| .ToList(); | ||
|
|
||
| Assert.True(addNotificationActivityJobs.Count >= 1, | ||
| $"Expected at least 1 AddNotificationActivitiesWithDuplicateCheck job, but got {addNotificationActivityJobs.Count}"); | ||
|
|
||
| // Verify next batch job behavior: | ||
| // - If we got a full batch (batchSize notifications), a next batch job should be enqueued | ||
| // - If we got less than a full batch, no next batch job (batch processing is complete) | ||
| var nextBatchJobs = enqueuedJobs | ||
| .Where(j => j.serviceType == typeof(CleanupMissingSyncedNotificationsBatchHandler) && j.methodName == nameof(CleanupMissingSyncedNotificationsBatchHandler.ExecuteBatch)) | ||
| .ToList(); | ||
|
|
||
| // Since we're in a shared test database, we can't predict exactly how many notifications exist | ||
| // We just verify that if there are more batches to process, a next batch job was enqueued | ||
| // If no next batch job was enqueued, that means processing completed (which is valid) | ||
| } | ||
|
|
||
| private async Task<Guid> CreateCorrespondenceWithSyncedNotifications(int notificationCount, DateTime notificationSentDate) | ||
| { | ||
| // Create a migrated correspondence with 1 initial notification from Altinn2 | ||
| var migrateCorrespondence = new MigrateCorrespondenceBuilder() | ||
| .CreateMigrateCorrespondence() | ||
| .WithNotificationHistoryEvent( | ||
| 1000, // altinn2NotificationId | ||
| "initial@example.com", // notificationAddress | ||
| API.Models.Enums.NotificationChannelExt.Email, // notificationChannelExt | ||
| notificationSentDate, // notificationSent | ||
| false) // isReminder | ||
| .Build(); | ||
| migrateCorrespondence.MakeAvailable = true; | ||
|
|
||
| // Use migration endpoint to create correspondence with initial Altinn2 notification | ||
| var migrationClient = _factory.CreateClientWithAddedClaims(("scope", AuthorizationConstants.MigrateScope)); | ||
| var migrateResponse = await migrationClient.PostAsJsonAsync( | ||
| "correspondence/api/v1/migration/correspondence", | ||
| migrateCorrespondence, | ||
| _responseSerializerOptions); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, migrateResponse.StatusCode); | ||
|
|
||
| var responseContent = await migrateResponse.Content.ReadAsStringAsync(); | ||
| var result = JsonSerializer.Deserialize<API.Models.CorrespondenceMigrationStatusExt>(responseContent, _responseSerializerOptions); | ||
| var correspondenceId = result!.CorrespondenceId; | ||
|
|
||
| // Now sync additional notifications (this sets SyncedFromAltinn2 field) | ||
| var syncedNotifications = new List<API.Models.MigrateCorrespondenceNotificationExt>(); | ||
| for (int i = 1; i < notificationCount; i++) // Start at 1 since we already have 1 notification | ||
| { | ||
| syncedNotifications.Add(new API.Models.MigrateCorrespondenceNotificationExt | ||
| { | ||
| Altinn2NotificationId = 1000 + i, | ||
| NotificationAddress = $"test{i}@example.com", | ||
| NotificationChannel = API.Models.Enums.NotificationChannelExt.Email, | ||
| NotificationSent = new DateTimeOffset(notificationSentDate.AddHours(i)), | ||
| IsReminder = false | ||
| }); | ||
| } | ||
|
|
||
| var syncRequest = new API.Models.SyncCorrespondenceNotificationEventRequestExt | ||
| { | ||
| CorrespondenceId = correspondenceId, | ||
| SyncedEvents = syncedNotifications | ||
| }; | ||
|
|
||
| var syncResponse = await migrationClient.PostAsJsonAsync( | ||
| "correspondence/api/v1/migration/correspondence/syncNotificationEvent", | ||
| syncRequest, | ||
| _responseSerializerOptions); | ||
|
|
||
| Assert.True(syncResponse.IsSuccessStatusCode, | ||
| $"Sync failed with status {syncResponse.StatusCode}: {await syncResponse.Content.ReadAsStringAsync()}"); | ||
|
|
||
| return correspondenceId; | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.