Suggestions

[yvdh]

Hello,

First of all: great work!
I have been trying to generate proper C# servers using the EPCIS 2.0 open-api files for some time with several tools (NSWAG, OpenAPIGen, Smartbear tooling, ...), without success (lot's of inconsistent classes, manual touchup required, etc.. ).
So for me this project is a godsend as we want to replace an aging EPCIS 1.1 backend with a much more accessible EPCIS 2.0 eco-system.

I got the project running, and dove into the code after reading up on the minimal API system, I usually stick to traditional controllers.
I am willing to help with this project, and I have the following suggestions:

• the minimal API's are quite straightforward, but as they currently stand it is not very 'swagger friendly'.
  - Especially the use of an extra delegate layer ('DelegateFactory') makes discovery of endpoint parameters impossible. I suggest moving the code from this delegate factory to a middleware (I have the code for this, let me know).
  - Adding 'Produces<>' on the endpoints allows for typing of the response, so that is covered.
• Possibly I have broken something, but EpcisErrors (e.g. capture with id 1 not found) seam to lead to http 500 without any description. Probably my fault ..
• Authorization seems not very usable as it stands now (any user is ok, but he can only access his own events, is this correct?). I have added role based authorization with an 'option pattern' and class binding 'APISetting' with 'APIUsers' (as opposed to reading the configuration keys directly). This config class is also injected ... Roles are 'role.capture and role.query, and they are tied to policies 'policy.capture and policy.query'. I also like to put these in const string classes to avoid any typo's. These also work for the swagger UI ...

Again, great work!

Let me know what you think.
Yves

[louisaxel-ambroise]

Hi @yvdh,

Thank you very much for the feedback, it's much appreciated! I'll try to answer to all the points:

the minimal API's are quite straightforward, but as they currently stand it is not very 'swagger friendly'.

I totally agree on this point.
IMO given the complexity of the current model I don't think having the API definition generated is feasible. Currently the input parameter and response classes (e.g. CaptureDocumentRequest or ListCapturesResult) do not match the JSON schema as it is transformed through custom binders/formatters (to support both v1.2 and v2.0 of the spec as well as XML/JSON formats) so it'll probably be very difficult to have a valid generated schema.
A naive approach could be to include a copy of the openapi definition from GS1 and include it in the static files. After all a correct implementation should be compliant with this definition, and it would avoid to make a lot of changes in the endpoints.
I'm open for any suggestion as I agree that it would be nice to provide a swagger documentation of the API, but I think it's not at top priority at the moment.

Possibly I have broken something, but EpcisErrors (e.g. capture with id 1 not found) seam to lead to http 500 without any description. Probably my fault ..

It looks like this is working in the main branch, I just tried in the sandbox a proper error is returned. That's probably something that you broke with your changes, but it should be quite easy to fix!

Authorization seems not very usable as it stands now (any user is ok, but he can only access his own events, is this correct?). I have added role based authorization with an 'option pattern' and class binding 'APISetting' with 'APIUsers' (as opposed to reading the configuration keys directly). This config class is also injected ... Roles are 'role.capture and role.query, and they are tied to policies 'policy.capture and policy.query'. I also like to put these in const string classes to avoid any typo's. These also work for the swagger UI ...

Authorization was really a headache for me on this project for a while. I decided to go with the simplest way possible and let everybody to implement it in it's own way, as some might go with certificates, other with OAuth/JWT or event AD, etc. So currently it's working like you describe, a hash of the username/password is stored in the request table and only events captures with the same authorization values will be returned in the queries.
I'd be really happy to see any improvement on that side, I would only want to keep that it's not bound to a specific authorization type. Your proposal looks promising, so if you want to open a PR I'll be happy to have a look at it.

Thanks again for your interest and your contribution!

Louis-Axel