Models Generation

[Amerc0]

First the project is very clean and well structured.

The main thought is to auto generate using nswag/msbuild the Amazon Models using their json.

I would like to extract the models to a separate project or even solution and referenced by the API project and if needed to autogenerate them on changes from amazon.

[abuzuhri]

I was thinking on that but the problem there so many changes that @NSwag with keep over write it

If more on this idea we can do that

[abuzuhri]

@J-W-Chan @RenzoF @CrineTech @Xriuk @kevinvenclovas @TechyChap @ProNotion

Amazon keep changing the API , and the idea here is good to solve the problem , BUT we facing many case the developer ask to add Enum not exist in real amazon model and generated model has some minor problem .

I asking here for more opinion , do you think that will work ?

[Xriuk]

I think that the swagger model provided by Amazon is not enough on its own, some types are weirdly formatted (like the *Lists which could be just IEnumerable<T> instead of extending a list) and some classes are empty (like Timestamp which should be just DateTime), also it generates a lot of unneeded garbage code like validation (which is empty), ToString and GetHashCode overrides.
I think in order to reliably update the library each time the models change one should tweak the generation code in order to handle all these cases.

[RenzoF]

I like the idea of keeping it separate and I have been thinking for a while, the main issue is that some models on Amazon are wrong and they take a while to update them, where we need them ASAP.
I am not sure if the solution could be having a fork of the models and we patch them at the source level? then the auto-generate would kick in and update the models.
Regarding the List or IEnumerable we should be able to configure the generator to output how we want it.
On the priorities and I think before going this route, I would keep the way it is now which kind of works for now it would be more important to have some unit tests done before this change.
My 2 cents

[TechyChap]

We have experimented with the mustache templates stripping them down for our own projects. So it can definitely be done.

I agree that having these clearly separated is a good approach. A separate project can help provide that isolation.

[J-W-Chan]

In my opinion, even if we use Swagger CodeGen to generate the SDK, we must first understand the return values of the SP-API before we can use it. In fact, the SP-API is not modified frequently, so I think there is no problem with our current approach, and we should focus on how to promote more developers to participate in this open-source library.

[Xriuk]

Even if you were to maintain the models manually (which could be feasible as they don't change often, that's true) I think the current models would need a heavy refactoring with potential breaking changes in order to adapt them to the .NET world and clean them up.

[CrineTech]

I agree, auto generated classes are bloated of garbage. I also agree that may of the actual models needs heavy refactoring.

For the problem of API changes, in my opinion the best way is to keep actual models and add to the repository the plain descriptor file supplied by amazon, checking (to be sure) that the actual model have the same properties (with properly standardized names, enums and types for .NET). In this way, when a new version of a model of the SP api is relased we can simply perform a file diff on json files and apply the same changes to our models.

We can avoid most of the breaking changes to models by adding the new properties and methods alongside to the old ones, applying the "obsolete" attribute to the old ones and making the proper type conversion to adapt the value of the old property to the new one.

Since probably the same type conversion will be needed many times in different models, i think that the ideal solution (for types where a simple implicit conversion cannot be done) is to make a (not public) heler class containing the typeconverter definitions.

Ideally we should do this for all models, and write into a "backlog" file the changes to be done that cannod be done in this way. Once that this is done for all the models (and probably developers using this library have started to move to new properties and methods, due to the compiler warning about obsolete properties) we should release a major new versione where there are also the changes previously written in the backlog and obsolete properties / methods are removed.

If we want to follow this path, i think that @abuzuhri should add a (short) list of coding style rules to follow, in order to avoid different code styles on different API areas covered by different developers.