euniversal / umei-api-specification

UMEI API specifications
Apache License 2.0
3 stars 0 forks source link

Required / optional fields #23

Closed cdmNSIDE closed 2 years ago

cdmNSIDE commented 2 years ago

Hello @narve, Small question for you: how do you plan to deal with required and optional fields in the API? For instance, when we create a new order the "periodFrom", "periodTo", "gridNodeId" and the "marketId" should be explicitly written as "required" fields. At the opposite, the "portfolioId" should be an optional field (because we need it only for sell orders). At the opposite, all those fields should be optional when we want to update or patch the order... In my opinion, the keyword "required" in the API could do the job... And we may also have to explicitly define an "OrderCreate" object and an "OrderUpdate" objects, both having different required properties. What do you thing? The idea behind is that the API will automatically return an error if a required field is missing.

I am not sure that using the keyword "nullable" is sufficient. For instance, "periodFrom" and "periodTo" are written as "nullable:true" in the current API. But that does not indicate if those fields are required or not. It only means that the market platform is able to manage a null value ..

narve commented 2 years ago

Required/optional fields are a recurring problem / discussion wrt REST and APIs.

My recommendation is: Let fields that are always mandatory, in all cases, both when submitting and storing, be required/non-nullable. All other fields should be specified as optional/nullable.

It is tempting to split orders into OrderCreate and OrderUpdate, so that we can specify which fields are required when creating vs when updating. Of course, "portfolioId" is only mandatory for sell orders, so we need to split into SellOrderCreate and BuyOrderCreate. We now have 4 APIs, and in most cases that would mean 4 classes, to specify an Order. Then it turns out that some participants should only be allowed to see "public" fields in other peoples orders. So we need one PublicOrder and one PrivateOrder. Then we need to keep all of these in sync, and map between them in both the server and the client code. Then it turns out that for FMO A, gridNodeId is not required for sellOrders because it can be deduced from the portfolioId, but in FMO B a portfolio can be active on multiple grid nodes and thus gridNodeId is not required. What do we do then? Furthermore, specifying price is only required when the price type is "Limit", but if you specify price type "Market" you don't need (and is not allowed to) specify price. What then? Solution: CreateLimitSellOrder, CreateMarketBuyOrder, UpdateMarketBuyOrder, etc.

narve commented 2 years ago

In general, the OpenAPI specification is not advanced enough to specify all the different cases. The only solution using OpenAPI is to have a multitude of different-but-similar schemas like above. However this does not scale and leads to a true spaghetti mess in most server- and client code

narve commented 2 years ago

To be even more general and philosophical, we can say that OpenAPI, and also most programming languages, are mixing/conflating two different things:

The second part here is different from the first and is not really supported neither by OpenAPI nor by most programming languages. It is usually implemented in imperative code:

if( side==Sell && portfolioId == null ) throw Error( ...)

In some programming languages, or using some libraries, this process can be described declaratively and automated, but that is not common.

So, rather then going a few meters down a road that either leads to a very messy API or requires a very specific set of tools/languages (or both), I recommend that we leave those fields as nullable and let the specifications be described by the different FMOs, in prose or in structured text, as suits them.

narve commented 2 years ago

However, related to this is my suggestion that we be rather explicit about error handling and error codes.

This entails both:

Further types can be defined by each FMO, this is unavoidable, but it is nice for the clients to have aligned error codes for known and obvious cases.

cdmNSIDE commented 2 years ago

Hello @narve , Thanks for all those very interesting comments. Yes indeed the API could quickly become a mess if we want to define all those CreateLimitSellOrder, CreateMarketBuyOrder, UpdateMarketBuyOrder and so on. Hopefully using the inheritance between those objects could simplify it a bit. The advantage is that the invalid inputs are automatically detected by the API. If we rather decide to exploit explicit error codes, we move the responsibility to check those errors to the FMOs.. which is fine for us I think ... So yes we can move in this second direction

narve commented 2 years ago

Ok, should we close this issue and possibly revisit it later on?