Skip to content

Conversation

@odesenfans
Copy link
Collaborator

@odesenfans odesenfans commented May 17, 2022

Propagated the use of the BasePendingMessage subclasses in all
functions related to message processing.

Split the message validation function, check_message, in two parts:

  • parse_message checks that the message is semantically valid,
    i.e. that all the required fields are present, are of the correct
    type and have sensible values.
  • verify_signature checks the signature of the message using
    the public key of the sender.

We now call parse_message as early as possible in the process.
This allows to simplify hypotheses about the presence/absence
of specific fields and allows to simplify the codebase.

One of the current issues is that the content of the message
can only be loaded conditionally (if the message has inline content).
Otherwise, we must load the content from the network. As this
operation goes beyond simple validation, we perform it later
in the flow, leading to a situation where the content of a message
is not guaranteed to be available at a given point depending
on the item_type field of the message.

Modified tests to use the raw message classes. Removed a few tests
that tested corner cases linked to incomplete message dictionaries.

@odesenfans odesenfans self-assigned this May 17, 2022
@odesenfans odesenfans added the work in progress Work in progress, major parts of the PR can change label May 17, 2022
@odesenfans odesenfans changed the base branch from master to dev May 17, 2022 17:53
@odesenfans odesenfans force-pushed the od-use-raw-message-in-pipeline branch 3 times, most recently from 1d356d6 to f759c1a Compare May 18, 2022 09:50
@odesenfans odesenfans changed the title [Messages] Use raw message model everywhere [Messages] Use pending message model everywhere May 18, 2022
@odesenfans
Copy link
Collaborator Author

To discuss with @hoh (not urgent): this PR as it stands is a good improvement already because it forces the usage of Pydantic models in the processing of messages. However, it doesn't implement the last mile, i.e. using Pydantic models to write to the DB. We still maintain dictionaries by hand towards the end of the job.

I plan to add new models to represent the documents in the messages collection, but that represents some additional work and I want to implement this in another PR. Ok for you?

@odesenfans odesenfans added help wanted Extra attention is needed and removed work in progress Work in progress, major parts of the PR can change labels May 20, 2022
@odesenfans odesenfans force-pushed the od-use-raw-message-in-pipeline branch from f759c1a to c7865bc Compare May 25, 2022 09:56
Propagated the use of the `BasePendingMessage` subclasses in all
functions related to message processing.

Split the message validation function, check_message, in two parts:
* `parse_message` checks that the message is semantically valid,
  i.e. that all the required fields are present, are of the correct
  type and have sensible values.
* `verify_signature` checks the signature of the message using
  the public key of the sender.

We now call parse_message as early as possible in the process.
This allows to simplify hypotheses about the presence/absence
of specific fields and allows to simplify the codebase.

One of the current issues is that the content of the message
can only be loaded conditionally (if the message has inline content).
Otherwise, we must load the content from the network. As this
operation goes beyond simple validation, we perform it later
in the flow, leading to a situation where the content of a message
is not guaranteed to be available at a given point depending
on the `item_type` field of the message.

Modified tests to use the raw message classes. Removed a few tests
that tested corner cases linked to incomplete message dictionaries.
@odesenfans odesenfans force-pushed the od-use-raw-message-in-pipeline branch from c7865bc to 6ff3830 Compare May 25, 2022 10:04
@odesenfans
Copy link
Collaborator Author

Superseded by #301, which is a full implementation.

@odesenfans odesenfans closed this Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant