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

feat: Add idempotentId #1638

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6c94a09
Created IdempotentId with validation and db migration
Fargekritt Dec 16, 2024
5c9334d
Create Idempotency added
Fargekritt Dec 16, 2024
4041dd3
Added IdempotentId to get DialogDto
Fargekritt Dec 16, 2024
fac9a18
Search Idempotent added
Fargekritt Dec 16, 2024
c84485a
Added Validadtion on SearchDialog for idempotentId
Fargekritt Dec 16, 2024
def7f0a
Updated purgeDialog to handle idempotentId.
Fargekritt Dec 27, 2024
3661f12
Cleanup
Fargekritt Dec 27, 2024
04dec21
Created more tests
Fargekritt Dec 27, 2024
510b0e9
Merge branch 'main' into feat/add-idempotency-field
Fargekritt Dec 27, 2024
7650083
Updated GQL
Fargekritt Dec 27, 2024
13e370b
Updated test
Fargekritt Dec 27, 2024
0a7e7f7
Swagge summary updated
Fargekritt Dec 30, 2024
84169fc
Updated summary
Fargekritt Jan 6, 2025
5ccd0d5
Updated swagger
Fargekritt Jan 9, 2025
bd3e2f9
Renamed IdempotentId to IdempotentKey.
Fargekritt Jan 13, 2025
3a2422b
Query improvement
Fargekritt Jan 13, 2025
78f0d20
Clean up
Fargekritt Jan 13, 2025
3d9c6db
Cleaning
Fargekritt Jan 13, 2025
f9fe516
PR changes
Fargekritt Jan 14, 2025
4eee9ec
Merge remote-tracking branch 'origin/main' into feat/add-idempotency-…
Fargekritt Jan 14, 2025
a92f270
Merge remote-tracking branch 'origin/main' into feat/add-idempotency-…
Fargekritt Jan 14, 2025
4635d21
clean up
Fargekritt Jan 14, 2025
3524d7e
Made e2e test more robust
Fargekritt Jan 14, 2025
2b61954
fix typo
Fargekritt Jan 14, 2025
346a694
Merge branch 'main' into feat/add-idempotency-field
Fargekritt Jan 14, 2025
3e66899
PR changes
Fargekritt Jan 16, 2025
9f846c7
Added proper filter on index
Fargekritt Jan 17, 2025
d4b32ce
Removed include org
Fargekritt Jan 17, 2025
9fb5399
Merge branch 'main' into feat/add-idempotency-field
Fargekritt Jan 17, 2025
a5c0ab5
Handle creating with idempotentKey optimistically and catch the excep…
Fargekritt Jan 24, 2025
72e11e5
Removed IdempotentId from SearchDialogQuery.cs
Fargekritt Jan 24, 2025
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
2 changes: 1 addition & 1 deletion docs/schema/V1/schema.verified.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -406,4 +406,4 @@ scalar DateTime @specifiedBy(url: "https:\/\/www.graphql-scalars.com\/date-time"

scalar URL @specifiedBy(url: "https:\/\/tools.ietf.org\/html\/rfc3986")

scalar UUID @specifiedBy(url: "https:\/\/tools.ietf.org\/html\/rfc4122")
scalar UUID @specifiedBy(url: "https:\/\/tools.ietf.org\/html\/rfc4122")
27 changes: 27 additions & 0 deletions docs/schema/V1/swagger.verified.json
Original file line number Diff line number Diff line change
Expand Up @@ -2335,6 +2335,11 @@
"nullable": true,
"type": "string"
},
"idempotentKey": {
"description": "A self-defined Id may be provided to support idempotent creation of dialogs.",
"nullable": true,
"type": "string"
},
Fargekritt marked this conversation as resolved.
Show resolved Hide resolved
"party": {
"description": "The party code representing the organization or person that the dialog belongs to in URN format.",
"example": "urn:altinn:person:identifier-no:01125512345\nurn:altinn:organization:identifier-no:912345678",
Expand Down Expand Up @@ -3418,6 +3423,11 @@
"format": "guid",
"type": "string"
},
"idempotentKey": {
"description": "A self-defined Id may be provided to support idempotent creation of dialogs.\n ",
"nullable": true,
"type": "string"
},
"org": {
"description": "The service owner code representing the organization (service owner) related to this dialog.",
"example": "ske",
Expand Down Expand Up @@ -4069,6 +4079,11 @@
"format": "guid",
"type": "string"
},
"idempotentKey": {
"description": "A self-defined Id may be provided to support idempotent creation of dialogs.\n ",
"nullable": true,
"type": "string"
},
"latestActivity": {
"description": "The latest entry in the dialog\u0027s activity log.",
"nullable": true,
Expand Down Expand Up @@ -5581,6 +5596,15 @@
"description": "Performs a search for dialogs, returning a paginated list of dialogs. For more information see the documentation (link TBD).\n\n* All date parameters must contain explicit time zone. Example: 2023-10-27T10:00:00Z or 2023-10-27T10:00:00\u002B01:00\n* See \u0022continuationToken\u0022 in the response for how to get the next page of results.\n* hasNextPage will be set to true if there are more items to get.",
"operationId": "V1ServiceOwnerDialogsSearch_SearchDialog",
"parameters": [
{
"description": "Filter by IdempotentKey ",
"in": "query",
"name": "idempotentKey",
"schema": {
"nullable": true,
"type": "string"
}
},
Fargekritt marked this conversation as resolved.
Show resolved Hide resolved
{
"description": "Filter by one or more service resources",
"explode": true,
Expand Down Expand Up @@ -5893,6 +5917,9 @@
"403": {
"description": "Unauthorized to create a dialog for the given serviceResource (not owned by authenticated organization or has additional scope requirements defined in policy)."
},
"409": {
"description": "Dialog with IdempotentKey 01941821-ffca-73a1-9335-435a882be014 has already been created."
},
"422": {
"content": {
"application/problem\u002Bjson": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using FluentValidation.Results;

namespace Digdir.Domain.Dialogporten.Application.Common.ReturnTypes;

public sealed record Conflict(IEnumerable<ConflictError> Conflicts)
{
public Conflict(ConflictError error) : this([error]) { }
public List<ValidationFailure> ToValidationResults() => Conflicts
.Select(x => new ValidationFailure(x.PropertyName, x.ErrorMessage))
.ToList();
}

public sealed record ConflictError(string PropertyName, string ErrorMessage);
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public interface IDialogDbContext
DbSet<LabelAssignmentLog> LabelAssignmentLogs { get; }
DbSet<ResourcePolicyInformation> ResourcePolicyInformation { get; }


/// <summary>
/// Validate a property on the <typeparamref name="TEntity"/> using a lambda
/// expression to specify the predicate only when the property is modified.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Digdir.Domain.Dialogporten.Domain.Parties;
using Digdir.Library.Entity.Abstractions.Features.Identifiable;
using MediatR;
using Microsoft.EntityFrameworkCore;
using OneOf;
using OneOf.Types;

Expand All @@ -24,7 +25,7 @@ public sealed class CreateDialogCommand : CreateDialogDto, IRequest<CreateDialog
public sealed record CreateDialogSuccess(Guid DialogId, Guid Revision);

[GenerateOneOf]
public sealed partial class CreateDialogResult : OneOfBase<CreateDialogSuccess, DomainError, ValidationError, Forbidden>;
public sealed partial class CreateDialogResult : OneOfBase<CreateDialogSuccess, DomainError, ValidationError, Forbidden, Conflict>;

internal sealed class CreateDialogCommandHandler : IRequestHandler<CreateDialogCommand, CreateDialogResult>
{
Expand Down Expand Up @@ -76,6 +77,23 @@ public async Task<CreateDialogResult> Handle(CreateDialogCommand request, Cancel
dialog.Org = serviceResourceInformation.OwnOrgShortName;
}

if (request.IdempotentKey is not null && !string.IsNullOrEmpty(dialog.Org))
elsand marked this conversation as resolved.
Show resolved Hide resolved
{
var dialogId = await _db.Dialogs
.Where(x => x.IdempotentKey == request.IdempotentKey && x.Org == dialog.Org)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: The order of the checks should probably match that of the composite index definition. However, as we're using a simple comparison, the order of the checks here should be handled by the optimizer in PostgreSQL such that it doesn't make any difference. For a inequality check (>, <, BETWEEN), the order would be significant, so I think it's generally best to always match the order.

.Select(x => x.Id)
.FirstOrDefaultAsync(cancellationToken);

if (dialogId != default)
{
return new Conflict(
new ConflictError(
nameof(request.IdempotentKey),
$"'{request.IdempotentKey}' already exists with DialogId '{dialogId}'"
));
}
}

CreateDialogEndUserContext(request, dialog);
await EnsureNoExistingUserDefinedIds(dialog, cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public CreateDialogCommandValidator(
.WithMessage($"'{{PropertyName}}' must be greater than or equal to '{nameof(CreateDialogCommand.CreatedAt)}'.")
.When(x => x.CreatedAt != default && x.UpdatedAt != default);

RuleFor(x => x.IdempotentKey)
.MaximumLength(36);

RuleFor(x => x.ServiceResource)
.NotNull()
.IsValidUri()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public class CreateDialogDto
/// <example>01913cd5-784f-7d3b-abef-4c77b1f0972d</example>
public Guid? Id { get; set; }

/// <summary>
/// A self-defined Id may be provided to support idempotent creation of dialogs.
/// </summary>
public string? IdempotentKey { get; set; }

/// <summary>
/// The service identifier for the service that the dialog is related to in URN-format.
/// This corresponds to a resource in the Altinn Resource Registry, which the authenticated organization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public async Task<PurgeDialogResult> Handle(PurgeDialogCommand request, Cancella
{
return new Forbidden($"User cannot modify resource type {dialog.ServiceResourceType}.");
}

_db.Dialogs.HardRemove(dialog);
var saveResult = await _unitOfWork
.EnableConcurrencyCheck(dialog, request.IfMatchDialogRevision)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public sealed class DialogDto
/// <example>01913cd5-784f-7d3b-abef-4c77b1f0972d</example>
public Guid Id { get; set; }

/// <summary>
/// A self-defined Id may be provided to support idempotent creation of dialogs.
/// </summary>
public string? IdempotentKey { get; set; }

/// <summary>
/// The unique identifier for the revision in UUIDv4 format.
/// </summary>
Expand Down Expand Up @@ -270,7 +275,6 @@ public sealed class DialogSeenLogDto
public bool IsCurrentEndUser { get; set; }
}


public sealed class ContentDto
{
/// <summary>
Expand Down Expand Up @@ -372,7 +376,6 @@ public sealed class DialogActivityDto
public List<LocalizationDto> Description { get; set; } = [];
}


public sealed class DialogApiActionDto
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ public class DialogDtoBase
/// <example>01913cd5-784f-7d3b-abef-4c77b1f0972d</example>
public Guid Id { get; set; }

/// <summary>
/// A self-defined Id may be provided to support idempotent creation of dialogs.
/// </summary>
public string? IdempotentKey { get; set; }

/// <summary>
/// The service owner code representing the organization (service owner) related to this dialog.
/// </summary>
Expand Down Expand Up @@ -158,7 +163,6 @@ public sealed class DialogSeenLogDto
public bool IsCurrentEndUser { get; set; }
}


public sealed class DialogActivityDto
{
/// <summary>
Expand Down Expand Up @@ -198,4 +202,3 @@ public sealed class DialogActivityDto
/// </summary>
public List<LocalizationDto> Description { get; set; } = [];
}

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ public sealed class SearchDialogQuery : SortablePaginationParameter<SearchDialog
{
private string? _searchLanguageCode;

/// <summary>
/// Filter by IdempotentKey
/// </summary>
public string? IdempotentKey { get; set; }
/// <summary>
/// Filter by one or more service resources
/// </summary>
Expand Down Expand Up @@ -169,6 +173,7 @@ public async Task<SearchDialogResult> Handle(SearchDialogQuery request, Cancella
var paginatedList = await dialogQuery
.Include(x => x.Content)
.ThenInclude(x => x.Value.Localizations)
.WhereIf(request.IdempotentKey is not null, x => x.IdempotentKey != null && x.IdempotentKey == request.IdempotentKey)
Fargekritt marked this conversation as resolved.
Show resolved Hide resolved
.WhereIf(!request.ServiceResource.IsNullOrEmpty(),
x => request.ServiceResource!.Contains(x.ServiceResource))
.WhereIf(!request.Party.IsNullOrEmpty(), x => request.Party!.Contains(x.Party))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public SearchDialogQueryValidator()
.MinimumLength(3)
.When(x => x.Search is not null);

RuleFor(x => x.IdempotentKey)
.MaximumLength(36)
.WithMessage("'{{PropertyName}}' can't be longer than 36 characters.");
Fargekritt marked this conversation as resolved.
Show resolved Hide resolved

RuleFor(x => x.SearchLanguageCode)
.Must(x => x is null || Localization.IsValidCultureCode(x))
.WithMessage(searchQuery =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public sealed class DialogEntity :
{
public Guid Id { get; set; }
public Guid Revision { get; set; }
public string? IdempotentKey { get; set; }
public DateTimeOffset CreatedAt { get; set; }
public DateTimeOffset UpdatedAt { get; set; }
public bool Deleted { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Digdir.Domain.Dialogporten.GraphQL.EndUser.Common;
using Digdir.Domain.Dialogporten.GraphQL.EndUser.MutationTypes;
using DialogStatus = Digdir.Domain.Dialogporten.GraphQL.EndUser.Common.DialogStatus;
Fargekritt marked this conversation as resolved.
Show resolved Hide resolved

namespace Digdir.Domain.Dialogporten.GraphQL.EndUser.DialogById;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,7 @@ public void Configure(EntityTypeBuilder<DialogEntity> builder)
builder.HasIndex(x => x.ServiceResource);
builder.HasIndex(x => x.Party);
builder.HasIndex(x => x.Process);
builder.Property(x => x.IdempotentKey).HasMaxLength(36);
builder.HasIndex(x => new { x.Org, x.IdempotentKey }).IsUnique().HasFilter(null);
Fargekritt marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
// Explicitly configure the Actor entity so that it will register as TPH in the database
modelBuilder.Entity<Actor>();


modelBuilder
.RemovePluralizingTableNameConvention()
.AddAuditableEntities()
Expand Down
Loading
Loading