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.

Sign up for GitHub

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

Add idempotency protection to B #145

Closed
wants to merge 4 commits into from
Closed

Add idempotency protection to B #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aff02b5
Modified signature of CreatOrder
matdavies Jun 6, 2021
ab5bb2e
Add GetIdempotentOrderResponse to IOrderStore
matdavies Jun 6, 2021
4292b2d
Add CompleteOrder to IOrderStore
matdavies Jun 6, 2021
0ccf864
Change return type of ProcessFlowRequest
matdavies Jun 8, 2021
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
6 changes: 4 additions & 2 deletions Examples/BookingSystem.AspNetCore/Stores/OrderStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public async override Task DeleteLease(OrderIdComponents orderId, SellerIdCompon
);
}

public async override ValueTask CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, OrderStateContext stateContext, OrderTransaction databaseTransaction)
public async override ValueTask<CreateOrderResult> CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, OrderStateContext stateContext, OrderTransaction databaseTransaction)
{
if (_appSettings.FeatureFlags.PaymentReconciliationDetailValidation && responseOrder.TotalPaymentDue.Price > 0 && ReconciliationMismatch(flowContext))
throw new OpenBookingException(new InvalidPaymentDetailsError(), "Payment reconciliation details do not match");
Expand Down Expand Up @@ -287,7 +287,9 @@ public async override ValueTask CreateOrder(Order responseOrder, StoreBookingFlo
null,
null);

if (!result) throw new OpenBookingException(new OrderAlreadyExistsError());
if (!result) return CreateOrderResult.OrderAlreadyExists;

return CreateOrderResult.OrderSuccessfullyCreated;
Comment on lines -290 to +292
Copy link
Contributor

@nickevansuk nickevansuk Aug 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Noting that this abstraction - the idea of moving the throwing of the OrderAlreadyExistsError into the StoreBookingEngine - is excellent.

It is however, a breaking change for users of the library.

Will break this out into a separate issue to capture it for future.

}

public async override ValueTask UpdateOrder(Order responseOrder, StoreBookingFlowContext flowContext, OrderStateContext stateContext, OrderTransaction databaseTransaction)
Expand Down
6 changes: 4 additions & 2 deletions Examples/BookingSystem.AspNetFramework/Stores/OrderStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public async override Task DeleteLease(OrderIdComponents orderId, SellerIdCompon
);
}

public async override ValueTask CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, OrderStateContext stateContext, OrderTransaction databaseTransaction)
public async override ValueTask<CreateOrderResult> CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, OrderStateContext stateContext, OrderTransaction databaseTransaction)
{
if (_appSettings.FeatureFlags.PaymentReconciliationDetailValidation && responseOrder.TotalPaymentDue.Price > 0 && ReconciliationMismatch(flowContext))
throw new OpenBookingException(new InvalidPaymentDetailsError(), "Payment reconciliation details do not match");
Expand Down Expand Up @@ -287,7 +287,9 @@ public async override ValueTask CreateOrder(Order responseOrder, StoreBookingFlo
null,
null);

if (!result) throw new OpenBookingException(new OrderAlreadyExistsError());
if (!result) return CreateOrderResult.OrderAlreadyExists;

return CreateOrderResult.OrderSuccessfullyCreated;
}

public async override ValueTask UpdateOrder(Order responseOrder, StoreBookingFlowContext flowContext, OrderStateContext stateContext, OrderTransaction databaseTransaction)
Expand Down
28 changes: 20 additions & 8 deletions OpenActive.Server.NET/CustomBookingEngine/CustomBookingEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,11 @@ public async Task<ResponseContent> GetOrderStatus(string clientId, Uri sellerId,
}
else
{
return ResponseContent.OpenBookingResponse(OpenActiveSerializer.Serialize(result), HttpStatusCode.OK);
return ResponseContent.OpenBookingResponse(result.Response, result.HttpStatusCode);
}
}

protected abstract Task<Order> ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerId, ILegalEntity seller);
protected abstract Task<ProcessFlowResult> ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerId, ILegalEntity seller);


protected bool IsOpportunityTypeRecognised(string opportunityTypeString)
Expand Down Expand Up @@ -384,8 +384,7 @@ private async Task<ResponseContent> ProcessCheckpoint(string clientId, Uri selle
var (orderId, sellerIdComponents, seller) = await ConstructIdsFromRequest(clientId, sellerId, uuid, orderType);
var orderResponse = await ProcessFlowRequest(ValidateFlowRequest<OrderQuote>(orderId, sellerIdComponents, seller, flowStage, orderQuote), orderQuote);
// Return a 409 status code if any OrderItem level errors exist
return ResponseContent.OpenBookingResponse(OpenActiveSerializer.Serialize(orderResponse),
orderResponse.OrderedItem.Exists(x => x.Error?.Count > 0) ? HttpStatusCode.Conflict : HttpStatusCode.OK);
return ResponseContent.OpenBookingResponse(orderResponse.Response, orderResponse.HttpStatusCode);
nickevansuk marked this conversation as resolved.
Show resolved Hide resolved
}
public async Task<ResponseContent> ProcessOrderCreationB(string clientId, Uri sellerId, string uuid, string orderJson)
{
Expand All @@ -400,7 +399,7 @@ public async Task<ResponseContent> ProcessOrderCreationB(string clientId, Uri se
var response = order.OrderProposalVersion != null ?
await ProcessOrderCreationFromOrderProposal(orderId, settings.OrderIdTemplate, seller, sellerIdComponents, order) :
await ProcessFlowRequest(ValidateFlowRequest<Order>(orderId, sellerIdComponents, seller, FlowStage.B, order), order);
return ResponseContent.OpenBookingResponse(OpenActiveSerializer.Serialize(response), HttpStatusCode.OK);
return ResponseContent.OpenBookingResponse(response.Response, response.HttpStatusCode);
}

public async Task<ResponseContent> ProcessOrderProposalCreationP(string clientId, Uri sellerId, string uuid, string orderJson)
Expand All @@ -413,7 +412,8 @@ public async Task<ResponseContent> ProcessOrderProposalCreationP(string clientId
throw new OpenBookingException(new UnexpectedOrderTypeError(), "OrderProposal is required for P");
}
var (orderId, sellerIdComponents, seller) = await ConstructIdsFromRequest(clientId, sellerId, uuid, OrderType.OrderProposal);
return ResponseContent.OpenBookingResponse(OpenActiveSerializer.Serialize(await ProcessFlowRequest(ValidateFlowRequest<OrderProposal>(orderId, sellerIdComponents, seller, FlowStage.P, order), order)), HttpStatusCode.OK);
var response = await ProcessFlowRequest(ValidateFlowRequest<OrderProposal>(orderId, sellerIdComponents, seller, FlowStage.P, order), order);
return ResponseContent.OpenBookingResponse(response.Response, response.HttpStatusCode);
}

private SellerIdComponents GetSellerIdComponentsFromApiKey(Uri sellerId)
Expand Down Expand Up @@ -760,9 +760,21 @@ async Task<ResponseContent> IBookingEngine.TriggerTestAction(string actionJson)
};
}

public abstract Task<TOrder> ProcessFlowRequest<TOrder>(BookingFlowContext request, TOrder order) where TOrder : Order, new();
public abstract Task<ProcessFlowResult> ProcessFlowRequest<TOrder>(BookingFlowContext request, TOrder order) where TOrder : Order, new();

public abstract Task<Order> ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order);
public abstract Task<ProcessFlowResult> ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order);

}

public class ProcessFlowResult
{
public string Response { get; }
public HttpStatusCode HttpStatusCode { get; }

public ProcessFlowResult(string response, HttpStatusCode httpStatusCode)
{
Response = response;
HttpStatusCode = httpStatusCode;
}
}
}
17 changes: 11 additions & 6 deletions OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections.Generic;
using System;
using System.Linq;
using System.Net;
using OpenActive.DatasetSite.NET;
using OpenActive.NET;
using OpenActive.Server.NET.OpenBookingHelper;
Expand Down Expand Up @@ -342,7 +343,7 @@ private static void CheckOrderIntegrity(Order requestOrder, Order responseOrder)
}
}

protected async override Task<Order> ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerIdComponents, ILegalEntity seller)
protected async override Task<ProcessFlowResult> ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerIdComponents, ILegalEntity seller)
{
// Get Order without OrderItems expanded
var order = await storeBookingEngineSettings.OrderStore.GetOrderStatus(orderId, sellerIdComponents, seller);
Expand All @@ -367,10 +368,10 @@ protected async override Task<Order> ProcessGetOrderStatus(OrderIdComponents ord

// TODO: Should other properties be extracted from the flowContext for consistency, or do we trust the internals not to create excessive props?
order.BookingService = flowContext.BookingService;
return order;
return new ProcessFlowResult(OpenActiveSerializer.Serialize(order), HttpStatusCode.OK);
}

public async override Task<Order> ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order)
public async override Task<ProcessFlowResult> ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order)
{
if (!await storeBookingEngineSettings.OrderStore.CreateOrderFromOrderProposal(orderId, sellerId, order.OrderProposalVersion, order))
{
Expand Down Expand Up @@ -528,7 +529,7 @@ public async override Task<Order> ProcessOrderCreationFromOrderProposal(OrderIdC
return context;
}

public async override Task<TOrder> ProcessFlowRequest<TOrder>(BookingFlowContext request, TOrder order)
public async override Task<ProcessFlowResult> ProcessFlowRequest<TOrder>(BookingFlowContext request, TOrder order)
{
var context = AugmentContextFromOrder(request, order);

Expand All @@ -550,6 +551,7 @@ public async override Task<TOrder> ProcessFlowRequest<TOrder>(BookingFlowContext
Payment = context.Payment,
OrderedItem = orderItemContexts.Select(x => x.ResponseOrderItem).ToList()
};
HttpStatusCode responseCode = HttpStatusCode.OK;

// Add totals to the resulting Order
OrderCalculations.AugmentOrderWithTotals(
Expand Down Expand Up @@ -701,6 +703,9 @@ public async override Task<TOrder> ProcessFlowRequest<TOrder>(BookingFlowContext
throw;
}
}

if (responseOrderQuote.OrderedItem.Exists(x => x.Error?.Count > 0))
responseCode = HttpStatusCode.Conflict;
break;

case Order responseOrder:
Expand All @@ -723,7 +728,7 @@ public async override Task<TOrder> ProcessFlowRequest<TOrder>(BookingFlowContext
{
// Create the parent Order
if (storeBookingEngineSettings.EnforceSyncWithinOrderTransactions)
storeBookingEngineSettings.OrderStore.CreateOrder(responseOrder, context, stateContext, dbTransaction).CheckSyncValueTaskWorked();
storeBookingEngineSettings.OrderStore.CreateOrder(responseOrder, context, stateContext, dbTransaction).CheckSyncValueTaskWorkedAndReturnResult();
else
await storeBookingEngineSettings.OrderStore.CreateOrder(responseOrder, context, stateContext, dbTransaction);

Expand Down Expand Up @@ -783,7 +788,7 @@ public async override Task<TOrder> ProcessFlowRequest<TOrder>(BookingFlowContext
throw new OpenBookingException(new UnexpectedOrderTypeError());
}

return responseGenericOrder;
return new ProcessFlowResult(OpenActiveSerializer.Serialize(responseGenericOrder), responseCode);
}
}

Expand Down
26 changes: 23 additions & 3 deletions OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ public enum DeleteOrderResult
OrderDidNotExist
}

/// <summary>
/// Result of creating (or attempting to create) an Order in a store
/// </summary>
public enum CreateOrderResult
{
OrderSuccessfullyCreated,
OrderAlreadyExists
}

public interface IOrderStore
{
void SetConfiguration(OrderIdTemplate orderIdTemplate, SingleIdTemplate<SellerIdComponents> sellerIdTemplate);
Expand All @@ -38,10 +47,12 @@ public interface IOrderStore

ValueTask<Lease> CreateLease(OrderQuote responseOrderQuote, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask UpdateLease(OrderQuote responseOrderQuote, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask<CreateOrderResult> CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask UpdateOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask<(Guid, OrderProposalStatus)> CreateOrderProposal(OrderProposal responseOrderProposal, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask UpdateOrderProposal(OrderProposal responseOrderProposal, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction);
ValueTask<string> GetIdempotentOrderResponse(OrderIdComponents orderId, string requestHash);
ValueTask CompleteOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction, string serialisedResponseOrder, string requestHash);
}

public interface IStateContext
Expand Down Expand Up @@ -80,8 +91,8 @@ public ValueTask UpdateLease(OrderQuote responseOrderQuote, StoreBookingFlowCont
return UpdateLease(responseOrderQuote, flowContext, (TStateContext)stateContext, (TDatabaseTransaction)dbTransaction);
}

public abstract ValueTask CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, TStateContext stateContext, TDatabaseTransaction dbTransaction);
public ValueTask CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction)
public abstract ValueTask<CreateOrderResult> CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, TStateContext stateContext, TDatabaseTransaction dbTransaction);
public ValueTask<CreateOrderResult> CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction)
{
return CreateOrder(responseOrder, flowContext, (TStateContext)stateContext, (TDatabaseTransaction)dbTransaction);
}
Expand All @@ -103,5 +114,14 @@ public ValueTask UpdateOrderProposal(OrderProposal responseOrderProposal, StoreB
{
return UpdateOrderProposal(responseOrderProposal, flowContext, (TStateContext)stateContext, (TDatabaseTransaction)dbTransaction);
}

public async virtual ValueTask<string> GetIdempotentOrderResponse(OrderIdComponents orderIdComponents, string requestHash)
{
return "";
}

public async ValueTask CompleteOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction, string serialisedResponseOrder, string requestHash)
{
}
}
}