better types for MessageParser

emmanueltouzery commented 3 years ago

comment on top of the MessageParser interface:

/// A MessageParser allows hotwire to parse & display messages related to
/// a certain protocol, for instance HTTP. The message parser deals with
/// parsing packets as well as displaying them.
///
/// The first step is to take TSharkPackets and build a StreamData from it.
/// The StreamData contains among other things a list of messages, which
/// the parser builds from the packets. Conceptually the MessageParser
/// trait "should" have two associated types: one for a StreamGlobals type
/// specific for the parser, and one for a MessageData type specific
/// for the parser.
/// This was attempted in the 'better_types' branch, but it complicated
/// many things, because then it was not possible to iterate over parsers
/// or over messages from different parsers.
///
/// Instead we now have a "workaround" where MessageData and StreamGlobals
/// are enums and basically each parser puts and expects to find its own
/// type. Hopefully a better solution can be found in the future.

In the better_types branch, I have things like:

pub trait MessageParser {
    type MessageType;
    type StreamGlobalsType;

    fn initial_globals(&self) -> Self::StreamGlobalsType;

    fn add_to_stream(
        &self,
        stream: &mut StreamData<Self::MessageType, Self::StreamGlobalsType>,
        new_packet: TSharkPacket,
    ) -> Result<(), String>;

While in mainline:

    fn initial_globals(&self) -> StreamGlobals;

    fn add_to_stream(
        &self,
        stream: &mut StreamData,
        new_packet: TSharkPacket,
    ) -> Result<(), String>;

// ...

pub enum StreamGlobals {
    Postgres(PostgresStreamGlobals),
    Http2(Http2StreamGlobals),
    Http(HttpStreamGlobals),
    None,
}

pub enum MessageData {
    Http(HttpMessageData),
    Postgres(PostgresMessageData),
}

However that forced me to have in win.rs in that branch a message_parser_visitor macro to be able to work on multiple message parsers. And also things like:

    http_streams: HashMap<u32, StreamData<HttpMessageData, ()>>,
    http2_streams: HashMap<u32, StreamData<HttpMessageData, ()>>,
    postgres_streams: HashMap<u32, StreamData<PostgresMessageData, PostgresStreamGlobals>>,

    // ...

    http_details_component_emitters:
        Vec<Box<dyn Fn(mpsc::Sender<BgFunc>, PathBuf, HttpMessageData)>>,
    http2_details_component_emitters:
        Vec<Box<dyn Fn(mpsc::Sender<BgFunc>, PathBuf, HttpMessageData)>>,
    postgres_details_component_emitters:
        Vec<Box<dyn Fn(mpsc::Sender<BgFunc>, PathBuf, PostgresMessageData)>>,

While in mainline I have a single field in both cases. Note otherwise that the better_types branch is waaaay behind mainline in other aspects of the code.

I'd like to see that we could have both the advantages of the better types of the branch and the ergonomics of mainline... (besides the fact that in mainline the enums must be "casted" to the right type)

emmanueltouzery commented 2 years ago

I've also done some work on the better_types2 branch, but again I think it's not going to pan out sadly. Complexity and still some conversions etc.

emmanueltouzery / hotwire

better types for MessageParser #1