inovex / scrumlr.io

Webapp for collaborative online retrospectives
https://scrumlr.io
MIT License
263 stars 58 forks source link

Evaluate benefits of using generics for realtime #3415

Open bitionaire opened 1 year ago

mateo-ivc commented 6 months ago

Using generics instead of interfaces as message data

What will change: realtime/boards.go:

{
 "type": "NOTES_UPDATED",
  "data": [
    {
      "author": "77a83f9e-24ac-423e-b640-ec66be080baa",
      "id": "90cb33cf-16e5-42f5-b6da-8c7b7b338914",
      "position": {
        "column": "404ac4c5-c972-446f-b562-adf3dfc2bc11",
        "rank": 0,
        "stack": null
      },
      "text": "test"
    }
  ]
}

Translates to this struct:

type BoardEventNotesUpdated struct {
    author   uuid.UUID
    id       uuid.UUID
    position struct {
        column uuid.UUID
        rank   int
        stack  bool
    }
    text string
}

type BoardEvent[T BoardEventNotesUpdated | EventTypeB | ...] struct {
    eventType BoardEventType
    data      T
}

Or use the already existing dto's

realtime/boards.go BroadcastToBoard needs to be a generic method. This leads to making the broker also generic since parameterized methods are not allowed.

type DataType interface {
  dto.NoteCreateRequest | dto.NoteUpdateRequest | ...
}

func (b *Broker[T]) BroadcastToBoard(boardID uuid.UUID, msg BoardEventTest[T]) error {
  logger.Get().Debugw("broadcasting to board", "board", boardID, "msg", msg.Type)
  return b.con.Publish(boardsSubject(boardID), msg)
}

services/...:

// realtime/bords.go
func BroadcastToBoard(boardID uuid.UUID, b *Broker, msg BoardEvent) error {
    logger.Get().Debugw("broadcasting to board", "board", boardID, "msg", msg.Type)
    return b.con.Publish(boardsSubject(boardID), msg)
}

// services/notes.go
func (s *NoteService) UpdatedNotes(board uuid.UUID, notes []database.Note) {
    eventNotes := make([]dto.Note, len(notes))
    for index, note := range notes {
        eventNotes[index] = *new(dto.Note).From(note)
    }

    err := realtime.BroadcastToBoard(board, s.realtime, realtime.BoardEvent{
        Type: realtime.BoardEventNotesUpdated,
        Data: eventNotes,
    })
    if err != nil {
        logger.Get().Errorw("unable to broadcast updated notes", "err", err)
    }
}

How would we benifit:

What problems would we encounter:

Conclusion:

Using generics for the realtime messages would be a nice addition, since it simplifies the error handling and parsing. The problem here is that go doesn’t allow parameterized methods. To fix this issue we would have to make the Broker generic

But this also forces all other structures that have a realtime.broker field to be generic (all services)

Another solution would be to make the BroadcastToBoard method a function.

But do we really need generic realtime messages? The broker only sends messages, but never receives any, which means that if implemented correctly there is no way to mess up a message and type safety isn't necessary

The only thing that would change is that the data is not passed as type interface{} when sending, but it forces to parse the data into a structure and then pass it on. For most messages we already do that and in some cases like delete it would lead to more boilerplate code because instead of just passing the ID we would have to parse the ID into a structure first and then send it, which seems like an unnecessary step.

This leads me to the conclusion that we don’t need generics for our realtime messages. This only makes our code more complex leading to the same result.