fix(*): new serialization mechanism

[garrappachc]

Using class-transformer to serialize objects does not allow us to use async methods, thus rendering us incapable of resolving sub-documents properly. This PR attemps to approach object serialization another way, using proper services, some meta-types and injector.

Model changes:

• Game.slot.player is a Player instance,
• Game.gameServer is a GameServer instance,
• QueueSlot.playerId is gone

[c43721]

This is kinda why I hate JS. I can't really just automatically assume serialization fields with async or can I just "return and forget"

While this approach looks nice, you're basically just using classes. Why not just use classes instead of creating an entirely abstract interface + paying for the injection costs?

Something like return this.toJSON() in the classes? And if you need more information than just do something like return {...this.toJSON(), someField: someService.toJSON()

[garrappachc]

It's not a limitation of JS, it's class-transformer's one. The issue is reported, but it seems any kind of development is on hold for now, sadly.

When it comes to your second question: yes, we could go for a full object-oriented approach, but it seems like pissing into the wind with NestJS' architecture. We would have to workaround the whole injection system, it looks... nasty?

export class Game implements Serializable<GameDto> {
  // ...
  async serialize(): Promise<GameDto> {
    const playersService = app.get(PlayersService); // this line here seems wrong
    return {
      // ...
      players: Promise.all(this.players.map(async player => await (await playersService.getById(player)).serialize()),
      // ...
    };
  }
}

[garrappachc]

I mean, I'm not entirely against this approach, I just think it's a little bit more iffy...

[c43721]

everything about serialization is stupid.

[garrappachc]

everything about serialization is stupid.

That's a strong statement if I've ever seen one...

[garrappachc]

On the other hand, app.get() is a dependency injection as well, so I guess there's nothing wrong with the second approach...

[garrappachc]

@c43721 take a look now, does it look better to you?

[c43721]

I actually like this newer approach. That's quite smart.

[c43721]

Technically we can use intercepters and return instance of like some class (Serializable in our case) and do the .serialize() in an intercepter if we don't want to call it ourselves.

[garrappachc]

That would require traversing deep through object to find instances of Serializable... Efficiency where?

[c43721]

js

efficiency

lol

(it would be at most like 20ms additional time. You're using express- you don't care about performance. If you did, we'd be using fastify and not nest)

[codecov]

Codecov Report

Merging #1660 (fe2b9ae) into master (3af3fe4) will increase coverage by 0.17%.
The diff coverage is 97.26%.

@@            Coverage Diff             @@
##           master    #1660      +/-   ##
==========================================
+ Coverage   94.82%   95.00%   +0.17%     
==========================================
  Files         221      219       -2     
  Lines        4388     4420      +32     
  Branches      436      445       +9     
==========================================
+ Hits         4161     4199      +38     
+ Misses        218      212       -6     
  Partials        9        9              

Impacted Files	Coverage Δ
...ns/discord/notifications/player-profile-updated.ts	100.00% <ø> (ø)
src/queue/queue.module.ts	100.00% <ø> (ø)
src/players/models/player-skill.ts	75.00% <57.14%> (-25.00%)	⬇️
src/players/models/player-ban.ts	86.36% <71.42%> (-8.09%)	⬇️
src/game-servers/models/game-server.ts	100.00% <100.00%> (ø)
...ers/games-with-substitution-requests.controller.ts	100.00% <100.00%> (ø)
src/games/controllers/games.controller.ts	96.55% <100.00%> (-0.06%)	⬇️
src/games/gateways/games.gateway.ts	100.00% <100.00%> (ø)
src/games/models/game.ts	100.00% <100.00%> (ø)
src/games/services/games.service.ts	83.45% <100.00%> (ø)
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3af3fe4...fe2b9ae. Read the comment docs.

[garrappachc]

@c43721 ready for review. The tests are a mess, don't look at them.

[c43721]

some nits, comments, and other things I found