search

godotengine / godot-asset-library

PHP frontend for Godot Engine's asset library
https://godotengine.org/asset-library
MIT License
285 stars 85 forks source link

Implement Swagger/OpenAPI definition for client generation #255

Open fenix-hub opened 2 years ago

fenix-hub commented 2 years ago

This PR adds the Swagger/OpenAPI definition for the AssetLib APIs in order to test them through SwaggerUI and generate clients with different programming languages.

fenix-hub commented 2 years ago

For now only CORE APIs have been mapped, next commits will implement the rest of them.

fenix-hub commented 2 years ago

Added the rest of exposed APIs. Some descriptions/summaries are missing (related to parameters, request body and responses). It would be nice if developers/maintainers could add details to make the definition complete. image The definition is being tested. I'm generating a go client for my project Gibson directly using Swagger Editor.

The only known issue (already discussed in Discord) is related to the modify_date parameter in the AssetDetails object. Since it is described as a string with format: date-time most client generators will treat it as a timestamp (relative to the programming language) most of the time using Time classes which adopt by default a format compliant to both ISO8601 and RFC3339 (2020-12-09T16:09:53+00:00), but not all of them are able to parse an RFC3339 only dates (2020-12-09 16:09:53+00:00) like the Asset Library currently uses, resulting in a parsing exception for most of the clients.

In this case the options are: a) define the modify_date parameter as a simple string b) update the library with a replacement of ` character with aT` character before sending the response, in order to not create conflicts with how the timestamp is stored in the database

Frontrider commented 1 year ago

How does the authentication work?

fenix-hub commented 1 year ago

The /login endpoint will return an Authentication and Authorization token. However the usage is not standardized and to authenticate requests it must be passed in the body of requests that require authentication.

Frontrider commented 1 year ago

I see. Doesn't the /asset's post method need a token field then for now? :)

Frontrider commented 1 year ago

I guess I start carpet bombing with questions until I can publish/update a plugin :)

Is download_provider in AssetSummary an enum containing github, gitlab,bitbucket, gogs,cgit and custom? Same with category, and godot_version might also be one.

fenix-hub commented 1 year ago

I'll make sure to update the specs with the "token" parameter in all the requests that require it. However I'm unsure about the use of enums in OpenAPI specs, since some client/server generators do not handle language-specific enums in the same way, and since the are just Strings on the frontend but actually a tinyint on the backend adding or removing one won't require to update the specs too.

Frontrider commented 1 year ago

However I'm unsure about the use of enums in OpenAPI specs, since some client/server generators do not handle language-specific enums in the same way, and since the are just Strings on the frontend but actually a tinyint on the backend adding or removing one won't require to update the specs too.

On the frontend they are a fixed set of strings. An enum is how it shows up when I look at it from my side, that int behind the curtain is not relevant.

From a consumer perspective it is a breeding ground for annoying errors. If not then just more code to try to validate a fixed set of strings based on cross referencing the website, that will break in annoying ways. Instead of just knowing what it wants me to hand over, and building around it.



The official doc specifically states what enums are, and it fits for what the api is trying to do with it. The only way to change it is to have more api endpoints to retrieve them which is overkill for now. https://swagger.io/docs/specification/data-models/enums/

If a client does not understand enums that match the specification then that is not the api's fault.

Frontrider commented 1 year ago

Good lord. Do I have to send both category AND category id? Only one of them is not sufficient, while they seem to mean the same thing. Yeah, that is needed an enum 100% I will never get the ids (that is probably the int you're talking about, the enum's ordinal) right any other way without a dedicated endpoint telling them to me.

fenix-hub commented 1 year ago

Good lord. Do I have to send both category AND category id? Only one of them is not sufficient, while they seem to mean the same thing.

At least for POST requests, it doesn't seem so. https://github.com/godotengine/godot-asset-library/blob/aa7f9b57396563c78d5e38acc38ab90d521256ca/API.md

For GET requests, category == category_id. https://github.com/godotengine/godot-asset-library/blob/aa7f9b57396563c78d5e38acc38ab90d521256ca/API.md#get-asset

Where is it failing for you just by providing the category_id ?

Frontrider commented 1 year ago

I understand it now then. Remove the category name from places where sending it is invalid. I'm not sure how it could be documented then. That is the bit I messed up, because you gave me both fields for an update/creation. Okay.

The other enums work out now.

The best I can think of is to type out the ids in the description but that is also a bit out there. Still better than nothing.

Frontrider commented 1 year ago

Well, I feel like I should apologize to that beautiful person who will have to look at my 5 "useless" edits on my addon, but It works now. Everything but the categories are coming from the api definition, and validated according to whatever is in that.

https://plugins.gradle.org/plugin/io.github.frontrider.godle-publish