:art: Enhance Response Schemas, reorganise common properties, and add operationIds to methods

[ff137]

Some response properties need to be updated to reflect that they can be nullable.

There is a ton of duplication throughout the spec, so this PR starts the initiative of defining common properties in a single location (renaming the previously defined properties.js module to commonProperties.js -- it was not being used yet and "properties" is too generic of a name).

Now if a field turns out to nullable, it can be edited in 1 place.

List of changes:

• RecordsResponse returns strings for: match_id, start_time, hero_id, and score. All of these should be integers, to be standardised with all other usages. So, :boom: breaking change RecordsResponse now returns integers instead of strings
• PlayerHeroesResponse returns hero_id: string, when consistency would require an int. Server response type is updated.
• TeamPlayersResponse returns account_id: string. This is now consistently an int.
• HeroStatsResponse didn't line up with the actual API response, so I updated the response params ✅
• OperationId function added to automatically generate names, added to all routes
• description updated to no longer reference "from 2018" API rates
• triaged previous commonProperties modules responses/properties and requests/queryParams
• convertSpec.js script added, to generate spec.json, which is now also tracked for version control.

Decided to refactor. routes/responses.js contained all of the response objects. Now it's triaged into routes/responses/:

responses/schemas/
├── hero
│   ├── HeroDurationsResponse.js
│   ├── HeroItemPopularityResponse.js
│   ├── HeroMatchupsResponse.js
│   ├── HeroObjectResponse.js
│   └── HeroStatsResponse.js
├── importResponses.js    <--- allows import of all these objects in one call
├── match
│   ├── MatchObjectResponse.js
│   ├── MatchResponse.js
│   ├── ParsedMatchesResponse.js
│   └── PublicMatchesResponse.js
├── miscellaneous
│   ├── BenchmarksResponse.js
│   ├── DistributionsResponse.js
│   ├── LeagueObjectResponse.js
│   ├── MetadataResponse.js
│   ├── RankingsResponse.js
│   ├── RecordsResponse.js
│   ├── ReplaysResponse.js
│   ├── ScenarioItemTimingsResponse.js
│   ├── ScenarioLaneRolesResponse.js
│   ├── ScenarioMiscResponse.js
│   ├── SchemaResponse.js
│   └── SearchResponse.js
├── player
│   ├── PlayerCountsResponse.js
│   ├── PlayerHeroesResponse.js
│   ├── PlayerMatchesResponse.js
│   ├── PlayerObjectResponse.js
│   ├── PlayerPeersResponse.js
│   ├── PlayerProsResponse.js
│   ├── PlayerRankingsResponse.js
│   ├── PlayerRatingsResponse.js
│   ├── PlayerRecentMatchesResponse.js
│   ├── PlayerTotalsResponse.js
│   ├── PlayerWardMapResponse.js
│   ├── PlayerWinLossResponse.js
│   ├── PlayerWordCloudResponse.js
│   ├── PlayersByRankResponse.js
│   └── PlayersResponse.js
└── team
    ├── TeamHeroesResponse.js
    ├── TeamMatchObjectResponse.js
    ├── TeamObjectResponse.js
    └── TeamPlayersResponse.js

(thank you GPT-4 for helping me auto-generate these file names)

Discussion for further refactoring is welcome

[ff137]

@howardchung thoughts on the refactoring so far? Good to carry on?

Next up I want to also break down params.js and commonProperties.js into subfolders/submodules for different categories, so they're not also one monolithic file. Probably worth doing same for the func-route defs in spec.js

[ff137]

Doing some more refactoring. We have responses/commonProperties and requests/commonProperties, so breaking that up a bit more for maintainability

[ff137]

@howardchung by the way, I notice there's some unused request query/parameters being defined:

  withAccountIdParam: {
    name: "with_account_id",
    in: "query",
    description: "Account IDs on the player's team (array)",
    required: false,
    schema: {
      type: "integer",
    },
  },
  againstAccountIdParam: {
    name: "against_account_id",
    in: "query",
    description: "Account IDs against the player's team (array)",
    required: false,
    schema: {
      type: "integer",
    },
  },
...
  minMmrParam: {
    name: "min_mmr",
    in: "query",
    description: "Minimum average MMR",
    required: false,
    schema: {
      type: "integer",
    },
  },
  maxMmrParam: {
    name: "max_mmr",
    in: "query",
    description: "Maximum average MMR",
    required: false,
    schema: {
      type: "integer",
    },
  },
  minTimeParam: {
    name: "min_time",
    in: "query",
    description: "Minimum start time (Unix time)",
    required: false,
    schema: {
      type: "integer",
    },
  },
  maxTimeParam: {
    name: "max_time",
    in: "query",
    description: "Maximum start time (Unix time)",
    required: false,
    schema: {
      type: "integer",
    },
  },
  matchOverviewParam: {
    name: "overview",
    in: "query",
    description: "Only fetch data required for match overview page",
    required: false,
    schema: {
      type: "integer",
    },
  },

But they are not used anywhere. Before I remove them, are these query params intended to be used anywhere? Maybe it's meant to be part of playerParamNames, which is used for "/players/{account_id}/..

Do we still want to implement these, or can I remove it?

[ff137]

Ok, pretty happy with it now, I think the schemas + params are much better organised, in folders requests and responses.

Still more improvements possible, like parameterising all the response properties. Currently only a few common params in responses.properties.commonProperties.

For example BenchmarksResponse defines the following pattern 7 times:

            type: "array",
            items: {
              type: "object",
              properties: {
                percentile: {
                  description: "percentile",
                  type: "number",
                },
                value: {
                  description: "value",
                  type: "number",
                },
              },
            },
          },

So it'll help a lot if everything is parameterised. It'll deduplicate and help spot inconsistencies.

Lastly it'll also help to move the route func definitions out from spec.js into their own modules as well. Eventually spec.js should be just a few hundred lines long. Currently it's 2800 lines long, and it was ~5300 lines when I started 😄

[ff137]

^ last change just made hero_id a required field in GET /heroes response. Helps with using it as a key in my local openapi client, otherwise it's Optional due not strictly being marked as required. Can definitely make a lot more fields required, but leaving that for later work

[howardchung]

Unused parameters--if they aren't in the API code then feel free to delete them

[ff137]

Unused parameters--if they aren't in the API code then feel free to delete them

Sorted, thanks.

Some of the unused queries make a lot of sense to add - like min/maxMmrParam, and min/maxDuration. Perhaps it was intended for match filtering? Is it worth making an issue/PR to start adding that option?

Also, let me know if this broke anything, here or discord. Anything I'm responsible for breaking I feel responsible to fix 😆

[howardchung]

I suspect it was meant to filter matches at some point (but only for pro/public game samples in SQL, not the cassandra-backed player match list)