asterisk / asterisk-feature-requests

A place to submit feature and improvement requests for the Asterisk project. Contains no code.
2 stars 1 forks source link

[improvement]: Upgrade ARI Swagger version #15

Open jp1987 opened 1 year ago

jp1987 commented 1 year ago

ARI is still using Swagger v1.1, which is deprecated.

crazybolillo commented 4 months ago

I am willing to work in the issue and submit a patch but I would first like to know if this is something the maintainers of Asterisk would merge?

jcolp commented 4 months ago

That depends on how it is done. If we maintain backwards compatibility for the existing way then most likely yes.However before actually starting work how it is used today has to be explored and understood, and then ideally a proposal written of how it would work going forward so it can be discussed before work begins.

crazybolillo commented 4 months ago

Well I can think of mainly two uses:

  1. Live test the API using ari.asterisk.org and pointing it to your Asterisk server. This is a method shown on books and on conference videos I have seen on Youtube.
  2. Generate an HTTP client implementation based on the specs. This is where I hit a problem with Asterisk's using Swagger 1.1, as the tool I wanted to use did not support said version.

Technically since there is no code involved and its just a JSON file, I really can't think of something we could consider breaking, at least in the level of changing the API of some code or diaplan function. However these are two disruptions I can think of:

  1. ari.asterisk.org stops working with the new v2 json file. This is probably the worst disruption possible since people coming from youtube videos or a book trying to live test the API will not be able to do so anymore.
  2. Developers need to update their code gen tools. This depends on the tool they are using, considering that OpenAPI v2 is not deprecated yet, I think there is a big chance there will be no disruption.

Worst case scenario we can leave everything and simply add a new v2 file. ARI does not have constant changes so keeping them up to date probably won't be an issue, however if possible I am a bigger fan of deleting the old and leaving the new.

crazybolillo commented 4 months ago

I missed the part of how it would work after the changes, ideally it will work exactly the same, just with a newer spec. I will have to see if that is actually the case.

jcolp commented 4 months ago

The JSON definitions are also used to produce templated C code in Asterisk for the ARI server side implementation. Which means the old one can't just be removed, because that would break the ability to modify ARI in Asterisk itself.

crazybolillo commented 4 months ago

Oh dangit, TIL haha. I will test and also take that into consideration as well.

crazybolillo commented 4 months ago

I will attach the converted swagger.json file. I used swagger-codegen for the conversion.

File: swagger.json

Sadly it seems more destructive than I thought:

  1. The Swagger UI hosted on ari.asterisk.org only supports 1.1 and 1.2 specs (at least from the Javascript I read), so it is unable to load this new file.
  2. The script used in the Makefile for the ari-stubs target seems to rely on each resource (channel, bridge, etc...) having its own json file. In OpenAPI v2 they are all in a single file.

So it seems like Sangoma would need to update the Swagger UI version on they host. The script used for ari-stubs could be updated or we can probably find some third-party tool that does all the conversion. I could not find references in code to resources.json to generate C code.

Finally, here is a screenshot of what could be, looks pretty, although the auto-generated file still needs to have its tags fixed.

Screenshot from 2024-08-01 09-43-05