Fix Legacy OpenAPI specification generation #1293
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Using oapi-codegen against the full OpenAPI specification and go-swagger against Legacy APIs, I've found a few specification violations in regards to the Legacy APIs, which I have fixed in this PR:
isCircular, defined as boolean, but is defined with enum stringvalues (
"true"and"false"), and a default string value./v1/packages/{packageId}/assetsincorrectly defines parameterpackageIdaspackageID.filterTypedefined as a query but is actually part of the path.However, there are some things that are not fixed in this PR that are in violation of OpenAPI, which I can not fix as I need to know the underlying server implementation or original specification:
/v1/trades/{tradeId} or {tradeStatusType}/v1/localization-table/tables/{tableId} or {assetId}/v1/localization-table/tables/{tableId}/entry-countparameterentryFormathasdefault value of type int and not a string.
Additionally, For the separated legacy API specifications (Not part of the OpenAPI spec, but the standalone specifications):
While out of the scope of the PR, I would like to mention that these provide broken contracts:
/v2/users/{userId}/inventorydefines parameterassetTypeas enum int, but it is actually a named string (pointed out by @juliaoverflow), https://inventory.roblox.com/v2/users/5541653843/inventory/2 compared to https://inventory.roblox.com/v2/users/5541653843/inventory?assetTypes=2 and https://inventory.roblox.com/v2/users/5541653843/inventory?assetTypes=HatModel names for Legacy APIs are also inconsistent and a little difficult to name, for example, a model could be named as such:
Roblox.{API}.Api.[Request|Response].{Model}[Request|Response]ModelRoblox.{API}.Api.Models.{Model}[Request|Response]Roblox.Web.WebAPI.Api{Model}[Request|Response]ModelRoblox.Web.[Request|Response]s.{Source}.{Model}[Request|Response]Making the legacy models have naming similar to OpenCloud (eg.
CreatorStoreProductoverRoblox.AccountInformation.Api.Models.BirthdateResponse) would be helpful, but not a requirement.The full OpenAPI specification is unsuitable for the legacy APIs, as each legacy API is it's own service under a specific host, and makes code generation difficult, since it is combined with the OpenCloud API (eg. an OpenCloud API and a Legacy API are both defined as functions with no separation inbetween).
While working on manually implementing these endpoints myself, I noticed that the official server does not gurantee the API contract for certain legacy APIs. Unfortunately, I can not recall which endpoints specifically, and I could be incorrect.
Checks
By submitting your pull request for review, you agree to the following: