WIP: feat(apiEndpoint): add new endpoint to api; add sample matches (1)

[MarlonWelter]

This is related to #55

work done so far

• set up new api endpoint. It works on a toy example so far.
• add sample matches array containing 1 match

still missing

• The summonerId of one summoner is hard coded into the api method. It doesn't really make sense to use a summonerId here. It should just extend the participant stats for all participants in that match. It seems that is not really supported yet. Gonna look into that a bit deeper soon.
• I quickly added some code to solve an Circular reference error I received when parsing the extendedMatchResult with JSON.stringify. Not sure where exactly in that object the circular reference exists. Could need some looking into and if we can't remove it, the added code could be placed into a new method (just quick and dirty for now).

please check

• Is it alright where I placed the sampleMatches array.
• Not sure if axios used reasonably.

[lmachens]

The summonerId of one summoner is hard coded into the api method. It doesn't really make sense to use a summonerId here. It should just extend the participant stats for all participants in that match. It seems that is not really supported yet. Gonna look into that a bit deeper soon.

This is because of participant, participantIdentiy and maybe other fields like team. We could calculate extended stats for all participants, but this would only be required for this task.

[lmachens]

I quickly added some code to solve an Circular reference error I received when parsing the extendedMatchResult with JSON.stringify. Not sure where exactly in that object the circular reference exists. Could need some looking into and if we can't remove it, the added code could be placed into a new method (just quick and dirty for now).

I faced the same issue before. It would be great if we can fix it.

[lmachens]

What is the reason you created exampleMatches?
We could use the api to receive the matches.
Or do you want to modify these matches?

The api endpoint will be public, so it should make sense to call it.
If it doesn't make sense in public, I would prefer to setup a different solution to try out new trophies.

These are the use cases I see:
1: Create a new trophy and find out if it is obtainable. Select real matches as template and make it possible to modify them.
2: Modify extendedMatchResults and find out if existing and new trophies are obtainable.

Is this what you like to do @MarlonWelter ?

[MarlonWelter]

The summonerId of one summoner is hard coded into the api method. It doesn't really make sense to use a summonerId here. It should just extend the participant stats for all participants in that match. It seems that is not really supported yet. Gonna look into that a bit deeper soon.

This is because of participant, participantIdentiy and maybe other fields like team. We could calculate extended stats for all participants, but this would only be required for this task.

I disagree here. Actually we started a rework quite some time ago of the extendMatchResults method. One of the goals was to enable extendMatchResults without having to specify a summoner. Because right now we call the method 10 times for one match to calculate the trophies for everybody, right? That is a lot of wasted calculation because many of the same things are done 10 times that way.

Anyway, the refactoring was going quite far as I remember, but not quite finished. I remember however that the fields like "others", "team", "opponents" are now also referenced directly at each participant, so those fields being referenced directly from the match itself seems redundant. To me it feels unintuitive to reference participant, team, opponent, others etc directly from the match. Those fields should be referenced from each participant in the participants array.

I feel like just having a general method that calculates the trophy for all participant in a match without having to specify a certain participant would be good to have. If at some point someone new develops a new trophy and wants to test it he would also feel like it is quite inconvenient to have to specify a summoner and find a summonerId if you just want to test the trophy.

[MarlonWelter]

What is the reason you created exampleMatches?
We could use the api to receive the matches.
Or do you want to modify these matches?

The api endpoint will be public, so it should make sense to call it.
If it doesn't make sense in public, I would prefer to setup a different solution to try out new trophies.

These are the use cases I see:
1: Create a new trophy and find out if it is obtainable. Select real matches as template and make it possible to modify them.
2: Modify extendedMatchResults and find out if existing and new trophies are obtainable.

Is this what you like to do @MarlonWelter ?

Yes it should be public. Long term people can use it for the way I want to use it now. Check changes to extendMatchResults (usually through a new trophy) on a small set of real matches.

To me it doesn't matter if the matches are coded or requested through RIOT api. However, in this use-case the user should not worry to find matches himself, but just use a small predefined sample set. Since it will always be the same matches being returned, no riot api request is needed I think.

[MarlonWelter]

I quickly added some code to solve an Circular reference error I received when parsing the extendedMatchResult with JSON.stringify. Not sure where exactly in that object the circular reference exists. Could need some looking into and if we can't remove it, the added code could be placed into a new method (just quick and dirty for now).

I faced the same issue before. It would be great if we can fix it.

I think I understand what's causing the problem. There are actually many circular references.
An example of this is:
participant.team.participants[0] (or whatever index this participant is. There are other examples with teammates/opponents/others etc.).

I think the best way is to not remove the circular references, because they are actually very useful in those methods, but to use a JSON tool that can handle circular references. Usually this is done by giving every object an id and then replacing the object with its Id if it occurs more than once. That way the file is actually even shorter and can be fully recovered into an object in code later.

[lmachens]

I disagree here. Actually we started a rework quite some time ago of the extendMatchResults method. One of the goals was to enable extendMatchResults without having to specify a summoner. Because right now we call the method 10 times for one match to calculate the trophies for everybody, right? That is a lot of wasted calculation because many of the same things are done 10 times that way.

We do not have this use case right now. But I agree that it makes sense to make it configurable to allow calculating the extended stats for multiple participants.

Yes it should be public. Long term people can use it for the way I want to use it now. Check changes to extendMatchResults (usually through a new trophy) on a small set of real matches.

If it is part of the public api, you can not test changes or new trophies because it would require to update the api first.
Maybe a simple test site where you can load matches and modify them (in a textarea/JSON) and see which trophies would get achieved. But this is for devs only.
Let us have a talk about this to clearify the requirements.

I think the best way is to not remove the circular references, because they are actually very useful in those methods, but to use a JSON tool that can handle circular references. Usually this is done by giving every object an id and then replacing the object with its Id if it occurs more than once. That way the file is actually even shorter and can be fully recovered into an object in code later.

I agree that it doesn't make sense to have duplicate content and the references are useful. But we could work with IDs too and create helper functions.
If we want to return the extended matches from our api (which could make sense) we have to fix this issue.

[MarlonWelter]

This project is a json parser that can handle circular references (at least it claims so).

https://github.com/WebReflection/flatted

edit* Habs gerade mal getestet. Scheint auf den ersten Blick erstmal zu funktionieren.

[lmachens]

What happens with the references? I guess they won't be part of the JSON.
That means we can not access participant.team.participants[0] anymore.
If this is true, we should better find a solution without references or create helper functions to get the team for example.

[MarlonWelter]

The reference will be part of the JSON. you could revert the JSON back to an object and get the same object as you had before converting to a string.

[lmachens]

Can you showme an example how it will look like (your test results)?
Did they solve it like this?

{
  participants: [
   { summonerName: 'xyz' }
  },
  team: {
    participants: [
      '$ref.participants.0'
    ]
  }
}

[lmachens]

Can we close this?

[MarlonWelter]

No, I will implement the api endpoint for participant stats extension form this branch. There is already a lot of useful code in this branch.

[MarlonWelter]

Alright, now this branch has a runnable version of the new api endpoint that returns the participant stats for all participants for a match.

Two things are still missing:

• if the summoner name is set as a parameter, the last match of that summoner shall be extended.
• Once the extendMatchResults is refactored such that it doesn't need a summonerName (or similar) as input, that method shall be called here.

However, I think this branch could already be merged into the master. What do you think @lmachens ?

It can be tested with the following line: http://127.0.0.1:5009/?platformId=euw1&matchId=3867548933

[lmachens]

can you tell me more about the use case and where we will use this api?
in the app match end view we need the whole match and timeline anyway (for the heatmap/items/trophies).
the client can calculate the stats without another request.

I can only think about upcoming projects where we could use this. but as long as we didn't plan to use this api, I don't see why we should merge it. it feels like dead/unused code if this is true.

[lmachens]

use minus separated urls instead of camel case like sample-extended-match-results.
and if we want to have an endpoint which returns all summoner stats for a match, we should give it a better name.

[lmachens]

please remove these logs. we don't need them and we pay logs per line on our provider

[lmachens]

you can benefit from reduce method here.