Required Properties

[cnguy]

Hey!

Thank you for this project as usual. I would like to talk about adding required model properties to two cases:

1. Return Models (e.g. summoner-v4.SummonerDTO)
2. Models that are parameters (e.g. tournament-v4.TournamentCodeParameters)

https://docs.swagger.io/spec.html

The reasoning is that it allows one to generate types using sw2dts, for example.

For the first case,
Do you think it would be wise to trust that all Riot League of Legend's model DTO's to always have the properties (required properties as defined by Swagger).

So for example, SummonerDTO

These fields should always exist, and so we can define them in the SwaggerSpec like so:

  summoner-v4.SummonerDTO:
    type: object
    title: SummonerDTO
    properties:
      profileIconId:
        type: integer
        format: int32
        x-type: int
        description: 'ID of the summoner icon associated with the summoner.'
      name:
        type: string
        x-type: string
        description: 'Summoner name.'
      puuid:
        type: string
        x-type: string
        description: 'Encrypted PUUID. Exact length of 78 characters.'
      summonerLevel:
        type: integer
        format: int64
        x-type: long
        description: 'Summoner level associated with the summoner.'
      revisionDate:
        type: integer
        format: int64
        x-type: long
        description: 'Date summoner was last modified specified as epoch milliseconds. The following events will update this timestamp: profile icon change, playing the tutorial or advanced tutorial, finishing a game, summoner name change'
      id:
        type: string
        x-type: string
        description: 'Encrypted summoner ID. Max length 63 characters.'
      accountId:
        type: string
        x-type: string
        description: 'Encrypted account ID. Max length 56 characters.'
    description: 'represents a summoner'
    required:
      - profileIconId
      - name
      - puuid
      - summonerLevel
      - revisionDate
      - id
      - accountId

The second case is a bit weird and really only applies to the models that are used as parameters, such as tournament-v4.TournamentCodeParameters.

In this case, I noticed that Riot doesn't really have a column to mention that something is optional, and instead places it in the description text. And so in my code, I just have something that checks for "optional" basically.

I've already coded the stuffs required to handle both cases, but it basically makes the above assumptions and is therefore hacky. Is it something safe enough to add? :O If not, I can just maintain it in a fork and keep testing.

[MingweiSamuel]

Thanks for the issue!

Starting with 2., definitely makes sense to include the required/optional information from the api reference description, and this whole project is built on many quirks/hacks so that is 100% OK.

---

For 1., I know there are some times when the API doesn't return certain fields. Off the top of my head I can think of LeaguePositionDTO.miniSeries which is null if the player isn't in a series, and also seems LeaguePositionDTO.leagueName, .leagueId, and .tier aren't given by the paginated ranked positions endpoint. So I don't think we should make this change in general.
(mstssk/sw2dts#17)

[cnguy]

Makes sense, thanks! Up to you whether you want to close this, not much we can do about optional dto responses

[MingweiSamuel]

I'm taking a second look at this, I intend on actually manually going through to figure out which fields may be null.

[MingweiSamuel]

Non-required fields: (to be updated as I find more)

DTOs

champion-mastery-v4

None.

champion-v3

None.

league-v4 / league-exp-v4:

• MiniSeries is always optional:

league-v4.LeaguePositionDTO.miniSeries
league-v4.LeagueEntry.miniSeries
league-exp-v4.LeaguePositionDTO.miniSeries

Others TODO

lol-status-v3

Unfortunately really hard to test... leave all as optional? TODO
None -- seems to return empty lists, hopefully.

match-v4

Should probably just whitelist things

ParticipantStatsDto

Everything in ParticipantStatsDto? ("node" is from which gamemode?)
Gamemode specific considerations?

// Summoners rift 400 / 420
match-v4.ParticipantStatsDto.nodeNeutralizeAssist
match-v4.ParticipantStatsDto.nodeCapture
match-v4.ParticipantStatsDto.nodeNeutralize
match-v4.ParticipantStatsDto.nodeCaptureAssist

match-v4.ParticipantStatsDto.altarsCaptured
match-v4.ParticipantStatsDto.altarsNeutralized

match-v4.ParticipantStatsDto.teamObjective
match-v4.ParticipantTimelineDto.csDiffPerMinDeltas // !
match-v4.ParticipantTimelineDto.xpDiffPerMinDeltas
match-v4.ParticipantTimelineDto.damageTakenDiffPerMinDeltas

// Additionally, aram 450
match-v4.ParticipantStatsDto.firstInhibitorKill
match-v4.ParticipantStatsDto.firstInhibitorAssist

// Really old games (nearing the 2 year availability deadline)
match-v4.ParticipantStatsDto.perk#var#

Participant

From new runes reforged
Consider old games?

match-v4.Participant.runes
match-v4.Participant.masteries
match-v4.Participant.highestAchievedSeasonTier // !

Make perks optional??? For old matches???

(these are all needed for a new match 2019-10-29)

TeamStatsDTO

Really feel like some of these should be optional, but they all seem to be there (0).

spectator-v4

TODO

summoner-v4

None.

third-party-code-v4

N/A (None)

tournament

ignoring (for now)

Methods

champion-mastery-v4

getChampionMastery (for specific champion) returns 404 if the summoner has never played the champion (or if the champion ID is not a real champion, including negative numbers).

champion-v3

None

league-exp-v4

None (returns 400 on bad requests, empty array on pages too large, 400 on pages 0 and less; 1-indexed)

league-v4

getLeagueById returns 404 if uuid doesn't exist

status-v3

None

match-v4

getMatch, getMatchTimeline returns 404 if match id isn't a match (or is too old, including negative values)
getMatchlist returns 404 is the champion hasn't been played (or doesn't exist), or if the queue hasn't been played (or doesn't exist)

spectator-v4

getCurrentGameInfoBySummoner returns 404 if player not in game

summoner-v4

getBySummonerName

Can return 404 if name doesn't exist. Don't bother with null for other encrypted ID cases - would be 400 decryption error.

third-party-code-v4

Singular method can return 404 (null) if summoner has never set their code. (And maybe other cases)

[MingweiSamuel]

I've implemented a hardcoded list of optional/required fields that now gets included in the created schema. DTO's now have the required list of field names. There may be some missing/wrong fields though as it will need to be manually maintained.