Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Telegram message processing refactoring #145

Closed
Tracked by #102
ForNeVeR opened this issue Sep 26, 2021 · 0 comments · Fixed by #146
Closed
Tracked by #102

Telegram message processing refactoring #145

ForNeVeR opened this issue Sep 26, 2021 · 0 comments · Fixed by #146
Assignees

Comments

@ForNeVeR
Copy link
Member

After I've started working on #102, I can see that our architecture around the Telegram message processing is a bit messy, and it doesn't allow us to implement that feature in the right way.

Right now, Telegram message processing is done in multiple stages. Essentially, Emulsion.Telegram.Funogram module performs several synchronous transformations, and then passes a half-baked message (aka TelegramMessage) to the Core actor.

The Core, in turn, adds its own stage (Funogram.MessageConverter.flatten) for whatever reason, and then passes the resulting message to the XMPP message system.

The new processing we are about to add in #102 is asynchronous (because it will go to the database which involves I/O), and currently there's no good place to add it: everything in the pipeline is synchronous.

I suggest we do the following:

  1. Make the processing stages more explicit (even if there're only two of them) – could help with testing.

  2. Move the processing from the Core actor to the Emulsion.Telegram.Funogram (so it's not the Core responsibility, and Core is, once again, just a simple message bus).

  3. Allow asynchronous stages in the Funogram pipeline.

    Funogram itself requires the message handler to be synchronous, but that shouldn't be a problem for us: we do want to block Funogram from receiving the next message until we're finished with the current one (to avoid races).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant