From aff02b5c9430e00207071c4108fcc27dd196c79e Mon Sep 17 00:00:00 2001 From: Matthew Davies Date: Sun, 6 Jun 2021 16:36:08 +0100 Subject: [PATCH 1/4] Modified signature of CreatOrder --- .../BookingSystem.AspNetCore/Stores/OrderStore.cs | 6 ++++-- .../Stores/OrderStore.cs | 6 ++++-- .../StoreBookingEngine/StoreBookingEngine.cs | 2 +- .../StoreBookingEngine/Stores/OrderStore.cs | 15 ++++++++++++--- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/Examples/BookingSystem.AspNetCore/Stores/OrderStore.cs b/Examples/BookingSystem.AspNetCore/Stores/OrderStore.cs index 07d8b333..3d581c35 100644 --- a/Examples/BookingSystem.AspNetCore/Stores/OrderStore.cs +++ b/Examples/BookingSystem.AspNetCore/Stores/OrderStore.cs @@ -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 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"); @@ -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) diff --git a/Examples/BookingSystem.AspNetFramework/Stores/OrderStore.cs b/Examples/BookingSystem.AspNetFramework/Stores/OrderStore.cs index 07d8b333..3d581c35 100644 --- a/Examples/BookingSystem.AspNetFramework/Stores/OrderStore.cs +++ b/Examples/BookingSystem.AspNetFramework/Stores/OrderStore.cs @@ -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 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"); @@ -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) diff --git a/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs b/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs index 0a9a6b36..02f31d34 100644 --- a/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs +++ b/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs @@ -723,7 +723,7 @@ public async override Task ProcessFlowRequest(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); diff --git a/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs b/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs index 08f1acc8..389063d3 100644 --- a/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs +++ b/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs @@ -16,6 +16,15 @@ public enum DeleteOrderResult OrderDidNotExist } + /// + /// Result of creating (or attempting to create) an Order in a store + /// + public enum CreateOrderResult + { + OrderSuccessfullyCreated, + OrderAlreadyExists + } + public interface IOrderStore { void SetConfiguration(OrderIdTemplate orderIdTemplate, SingleIdTemplate sellerIdTemplate); @@ -38,7 +47,7 @@ public interface IOrderStore ValueTask 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 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); @@ -80,8 +89,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 CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, TStateContext stateContext, TDatabaseTransaction dbTransaction); + public ValueTask CreateOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction) { return CreateOrder(responseOrder, flowContext, (TStateContext)stateContext, (TDatabaseTransaction)dbTransaction); } From ab5bb2e8e407eebe26eec64ebc7432d0a5c527b1 Mon Sep 17 00:00:00 2001 From: Matthew Davies Date: Sun, 6 Jun 2021 16:53:38 +0100 Subject: [PATCH 2/4] Add GetIdempotentOrderResponse to IOrderStore --- .../StoreBookingEngine/Stores/OrderStore.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs b/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs index 389063d3..89325706 100644 --- a/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs +++ b/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs @@ -51,6 +51,7 @@ public interface IOrderStore 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 GetIdempotentOrderResponse(OrderIdComponents orderId, string requestHash); } public interface IStateContext @@ -112,5 +113,10 @@ public ValueTask UpdateOrderProposal(OrderProposal responseOrderProposal, StoreB { return UpdateOrderProposal(responseOrderProposal, flowContext, (TStateContext)stateContext, (TDatabaseTransaction)dbTransaction); } + + public async virtual ValueTask GetIdempotentOrderResponse(OrderIdComponents orderIdComponents, string requestHash) + { + return ""; + } } } From 4292b2dcfb5c87925219ac07a3393bf81e3f8509 Mon Sep 17 00:00:00 2001 From: Matthew Davies Date: Sun, 6 Jun 2021 16:57:43 +0100 Subject: [PATCH 3/4] Add CompleteOrder to IOrderStore --- .../StoreBookingEngine/Stores/OrderStore.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs b/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs index 89325706..de1df780 100644 --- a/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs +++ b/OpenActive.Server.NET/StoreBookingEngine/Stores/OrderStore.cs @@ -52,6 +52,7 @@ public interface IOrderStore ValueTask<(Guid, OrderProposalStatus)> CreateOrderProposal(OrderProposal responseOrderProposal, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction); ValueTask UpdateOrderProposal(OrderProposal responseOrderProposal, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction); ValueTask GetIdempotentOrderResponse(OrderIdComponents orderId, string requestHash); + ValueTask CompleteOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction, string serialisedResponseOrder, string requestHash); } public interface IStateContext @@ -118,5 +119,9 @@ public async virtual ValueTask GetIdempotentOrderResponse(OrderIdCompone { return ""; } + + public async ValueTask CompleteOrder(Order responseOrder, StoreBookingFlowContext flowContext, IStateContext stateContext, IDatabaseTransaction dbTransaction, string serialisedResponseOrder, string requestHash) + { + } } } From 0ccf8642b55011793fb0d48213fe13ec22f33174 Mon Sep 17 00:00:00 2001 From: Matthew Davies Date: Tue, 8 Jun 2021 11:52:10 +0100 Subject: [PATCH 4/4] Change return type of ProcessFlowRequest --- .../CustomBookingEngine.cs | 28 +++++++++++++------ .../StoreBookingEngine/StoreBookingEngine.cs | 15 ++++++---- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/OpenActive.Server.NET/CustomBookingEngine/CustomBookingEngine.cs b/OpenActive.Server.NET/CustomBookingEngine/CustomBookingEngine.cs index 9bb9f638..5fbbbb1a 100644 --- a/OpenActive.Server.NET/CustomBookingEngine/CustomBookingEngine.cs +++ b/OpenActive.Server.NET/CustomBookingEngine/CustomBookingEngine.cs @@ -335,11 +335,11 @@ public async Task GetOrderStatus(string clientId, Uri sellerId, } else { - return ResponseContent.OpenBookingResponse(OpenActiveSerializer.Serialize(result), HttpStatusCode.OK); + return ResponseContent.OpenBookingResponse(result.Response, result.HttpStatusCode); } } - protected abstract Task ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerId, ILegalEntity seller); + protected abstract Task ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerId, ILegalEntity seller); protected bool IsOpportunityTypeRecognised(string opportunityTypeString) @@ -384,8 +384,7 @@ private async Task ProcessCheckpoint(string clientId, Uri selle var (orderId, sellerIdComponents, seller) = await ConstructIdsFromRequest(clientId, sellerId, uuid, orderType); var orderResponse = await ProcessFlowRequest(ValidateFlowRequest(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); } public async Task ProcessOrderCreationB(string clientId, Uri sellerId, string uuid, string orderJson) { @@ -400,7 +399,7 @@ public async Task ProcessOrderCreationB(string clientId, Uri se var response = order.OrderProposalVersion != null ? await ProcessOrderCreationFromOrderProposal(orderId, settings.OrderIdTemplate, seller, sellerIdComponents, order) : await ProcessFlowRequest(ValidateFlowRequest(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 ProcessOrderProposalCreationP(string clientId, Uri sellerId, string uuid, string orderJson) @@ -413,7 +412,8 @@ public async Task 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(orderId, sellerIdComponents, seller, FlowStage.P, order), order)), HttpStatusCode.OK); + var response = await ProcessFlowRequest(ValidateFlowRequest(orderId, sellerIdComponents, seller, FlowStage.P, order), order); + return ResponseContent.OpenBookingResponse(response.Response, response.HttpStatusCode); } private SellerIdComponents GetSellerIdComponentsFromApiKey(Uri sellerId) @@ -760,9 +760,21 @@ async Task IBookingEngine.TriggerTestAction(string actionJson) }; } - public abstract Task ProcessFlowRequest(BookingFlowContext request, TOrder order) where TOrder : Order, new(); + public abstract Task ProcessFlowRequest(BookingFlowContext request, TOrder order) where TOrder : Order, new(); - public abstract Task ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order); + public abstract Task 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; + } + } } diff --git a/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs b/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs index 02f31d34..df728562 100644 --- a/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs +++ b/OpenActive.Server.NET/StoreBookingEngine/StoreBookingEngine.cs @@ -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; @@ -342,7 +343,7 @@ private static void CheckOrderIntegrity(Order requestOrder, Order responseOrder) } } - protected async override Task ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerIdComponents, ILegalEntity seller) + protected async override Task ProcessGetOrderStatus(OrderIdComponents orderId, SellerIdComponents sellerIdComponents, ILegalEntity seller) { // Get Order without OrderItems expanded var order = await storeBookingEngineSettings.OrderStore.GetOrderStatus(orderId, sellerIdComponents, seller); @@ -367,10 +368,10 @@ protected async override Task 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 ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order) + public async override Task ProcessOrderCreationFromOrderProposal(OrderIdComponents orderId, OrderIdTemplate orderIdTemplate, ILegalEntity seller, SellerIdComponents sellerId, Order order) { if (!await storeBookingEngineSettings.OrderStore.CreateOrderFromOrderProposal(orderId, sellerId, order.OrderProposalVersion, order)) { @@ -528,7 +529,7 @@ public async override Task ProcessOrderCreationFromOrderProposal(OrderIdC return context; } - public async override Task ProcessFlowRequest(BookingFlowContext request, TOrder order) + public async override Task ProcessFlowRequest(BookingFlowContext request, TOrder order) { var context = AugmentContextFromOrder(request, order); @@ -550,6 +551,7 @@ public async override Task ProcessFlowRequest(BookingFlowContext Payment = context.Payment, OrderedItem = orderItemContexts.Select(x => x.ResponseOrderItem).ToList() }; + HttpStatusCode responseCode = HttpStatusCode.OK; // Add totals to the resulting Order OrderCalculations.AugmentOrderWithTotals( @@ -701,6 +703,9 @@ public async override Task ProcessFlowRequest(BookingFlowContext throw; } } + + if (responseOrderQuote.OrderedItem.Exists(x => x.Error?.Count > 0)) + responseCode = HttpStatusCode.Conflict; break; case Order responseOrder: @@ -783,7 +788,7 @@ public async override Task ProcessFlowRequest(BookingFlowContext throw new OpenBookingException(new UnexpectedOrderTypeError()); } - return responseGenericOrder; + return new ProcessFlowResult(OpenActiveSerializer.Serialize(responseGenericOrder), responseCode); } }