Merge with Polyjuice client?

[arjan]

cc'ing @uhoreg.. It seems this library has quite some overlap with https://gitlab.com/uhoreg/polyjuice_client

Polyjuice does more around managing the connection and the sync, while matrix-elixir-sdk is more focused on solely the HTTP requests. Given that the community is already quite small I don't see why there should be 2 Elixir Matrix client libraries.

Thoughts?

[niklaslong]

I'm all for consolidating our efforts if possible (and full disclosure, this has was already tentatively discussed a few months back). If I recall correctly @uhoreg was going to try depending on the sdk in Polyjuice to see how feasible a merge would be?

[uhoreg]

Yeah, we had discussed the idea of merging. I haven't had time yet to experiment with changing Polyjuice to use the functions defined in this library, though.

[arjan]

In terms of Client API coverage I think both libraries actually are quite on par; both of them are incomplete I think (understandably; there's a lot of endpoints to cover...)

Polyjuice also implements the decoding of the response, which is nice; while matrix-elixir-sdk just returns the JSON as-is.

Polyjuice, however, does not offer "one shot" API usage, it is always necessary to start a Client process, which can be cumbersome for one-off API requests. (However that is actually not that hard to implement due to the Polyjuice.Client.API protocol).

I'm not sure what the direction forward here is, if there is any. Personally I have started with using Polyjuice, so I'll be contributing there more in the future.

[niklaslong]

I think the first point of order is determining what the goals for either library are. I can speak only to the SDK at this point as I haven't studied Polyjuice. SDK-wise my initial goal was to build a set of modules that could be used to create Matrix clients in Elixir. This meant encryption functions (aka olm), wrapping the endpoints, and potentially some extra opt-in conveniences for syncing state, handling keys and so forth. The goal is to be as lightweight as possible and only abstract the main pain points for users.

Perhaps it would be fruitful to consider the SDK as a core library at this point, especially given the work done on Polyjuice. My suggestion would be to consolidate by having Polyjuice depend on the core and olm. Core would simply wrap the endpoints (current sdk), olm the encryption functionality and Polyjuice a stateful layer built on top. This would, I feel, be the most flexible solution for users. All of these libraries together would constitute the client SDK for Elixir. Arguably the transition should also be fairly simple as the sdk currently implements only minimal response parsing and this would be the junction between the two libraries. (Note: I suggest this design regardless of where the end code lives on the internet.)

@uhoreg it would be great to hear your thoughts on that last paragraph in particular.

I'd also like to ping @kowsheek and @fancycade as they both expressed interest in using the SDK in production—any thoughts you might have on this would be very much appreciated.

[uhoreg]

Polyjuice, however, does not offer "one shot" API usage, it is always necessary to start a Client process, which can be cumbersome for one-off API requests. (However that is actually not that hard to implement due to the Polyjuice.Client.API protocol).

@arjan Polyjuice has has Polyjuice.Client.LowLevel that can be used for this. It's just a simple struct -- it doesn't start a process or try to do anything fancy.

I think the first point of order is determining what the goals for either library are. I can speak only to the SDK at this point as I haven't studied Polyjuice. SDK-wise my initial goal was to build a set of modules that could be used to create Matrix clients in Elixir. This meant encryption functions (aka olm), wrapping the endpoints, and potentially some extra opt-in conveniences for syncing state, handling keys and so forth. The goal is to be as lightweight as possible and only abstract the main pain points for users.

Polyjuice client was intended to be a relatively high-level library, mostly aimed at bot development, but trying not to exclude others uses (e.g. it should be possible to write an IM client with it). It aims to abstract out as much as it makes sense to. I haven't made any attempt at supporting olm yet (since it's intended primarily for bot development, it wouldn't be unreasonable to use pantalaimon to handle encryption).

Perhaps it would be fruitful to consider the SDK as a core library at this point, especially given the work done on Polyjuice. My suggestion would be to consolidate by having Polyjuice depend on the core and olm. Core would simply wrap the endpoints (current sdk), olm the encryption functionality and Polyjuice a stateful layer built on top. This would, I feel, be the most flexible solution for users. All of these libraries together would constitue the client SDK for Elixir. Arguably the transition should also be fairly simple as the sdk currently implements only minimal response parsing and this would be the junction between the two libraries. (Note: I suggest this design regardless of where the end code lives on the internet.)

Yes, if we do merge, I think that would be the way to do it. My understanding of the current state of both projects is that SDK implements more endpoints than Polyjuice, but is lower-level, whereas Polyjuice seems to be higher-level but does not support as many endpoints. So I think it makes sense to try to convert the Polyjuice functions to use SDK's functions to call the endpoints, rather than its own endpoint handling.

[kowsheek]

Thanks for the ping @niklaslong. This is a great conversation and I agree with the direction of consolidation. In the words of great Joe, we should try to reduce the stuff we have.

As with many great libraries in the Elixir ecosystem, I think we should offer both high-level & low-level abstractions so this will be a good merger.

In the project I'm working on the design phase of, we are looking to use both a Matrix server & client. It would be great if we could consolidate further with @serra-allgood's library under one umbrella.

Looking forward to further discussion.

[uhoreg]

I've given some more thought about how we might possibly merge Polyjuice Client and Matrix SDK and jotted down some thoughts. I've started with a summary of how Matrix SDK does Matrix requests, then how Polyjuice Client does Matrix requests, and then some possibilities for merging.

Matrix SDK:

"https://matrix.org"
|> Request.send_room_event(access_token, room_id, event)
|> API.do_request()

alternatively:

Request.send_room_event("https://matrix.org", access_token, room_id, event)
|> API.do_request()

• call a function to create a MatrixSDK.API.Request struct, which contains the request details, including the homeserver URL and access token
• call API.do_request() on that struct
• result is the raw HTTP response (except for errors)

Polyjuice Client:

uses a Polyjuice.Client.API protocol, and structs that implement it are responsible for providing the homeserver URL, access token, and other information. So the equivalent in Polyjuice Client would be:

client = create_client_somehow()
Polyjuice.Client.Room.send_message(client, room_id, event)

Behind the scenes, Polyjuice.Client.Room.send_message:

• creates a Polyjuice.Client.Endpoint.PutRoomsSend struct, which encapsulates the function arguments, and implements the Polyjuice.Client.Endpoint.Proto protocol
• tells the client to call the Polyjuice.Client.Endpoint.PutRoomsSend, which
  • transforms the Polyjuice.Client.Endpoint.PutRoomsSend into a standardized Polyjuice.Client.Endpoint.HttpSpec (which is similar to MatrixSDK.API.Request, but without the base URL, and without the access token)
  • makes the HTTP call
  • parses the HTTP response according to how the endpoint struct implements the Polyjuice.Client.Endpoint.Proto protocol

Merging Polyjuice Client and Matrix SDK:

several possibilities

• could replace Polyjuice.Client.Endpoint.HttpSpec with MatrixSDK.API.Request

  • doing just this doesn't really buy us much
  • would require some changes to MatrixSDK.API.Request
    • make base URL, access token optional -- this can probably be done, if we create the MatrixSDK.API.Request ourselves, as we can just omit/ignore the base URL, and not add the access token header

• could replace Polyjuice.Client.Endpoint.* modules (e.g. Polyjuice.Client.Endpoint.PutroomsSend) with MatrixSDK.API.Request, and drop the Polyjuice.Client.Endpoint.Proto protocol

  • also, probably not much benefit?
  • again, would require some changes to MatrixSDK.API.Request
    • same as above, but we also need some way to specify a way to parse the result
  • we lose the ability to add other transports (e.g. in Polyjuice Client, we could make each endpoint have the ability to also generate a Polyjuice.Client.Endpoint.FooBarSpec, if someone defines a Matrix-over-foobar transport -- e.g. low-bandwidth Matrix: https://github.com/matrix-org/matrix-doc/pull/3079)

• could change Matrix SDK's Request functions to not take the matrix server and access token, and pass the MatrixSDK.API.Request to a function that does know the matrix server and access token. e.g.

  Request.send_room_event(room_id, event)
  |> API.do_request("https://matrix.org", access_token)
  

  at which point, Polyjuice Client could also do something like

  client = create_client_somehow()
  request = Request.send_room_event(room_id, event)
  Polyjuice.Client.do_request(client, request)
  

  • too weird?
  • seems like a lot of work and a pretty invasive change to Matrix SDK

• we could leave the libraries' request parts intact, and focus on collaborating on supporting functions (e.g. Polyjuice Client's message builder, Matrix SDK's authentication type builder)

  • we'll still have two libraries that people would have to choose from, but at least we can help each other out in some way

[niklaslong]

@kowsheek thanks for weighing in here!

As with many great libraries in the Elixir ecosystem, I think we should offer both high-level & low-level abstractions so this will be a good merger.

I agree with this point and I think it is worth attempting if all parties are on board.

It would be great if we could consolidate further with @serra-allgood's library under one umbrella.

I'm not convinced an umbrella is the ideal structure at this point given how these projects could grow. This is indeed something to keep in mind. If a merge, or at the very least a consolidation, is deemed possible, we can then discuss logistics.

@uhoreg, many thanks for the hard work on the above! 🙌

I have a few initial thoughts:

we lose the ability to add other transports (e.g. in Polyjuice Client, we could make each endpoint have the ability to also generate a Polyjuice.Client.Endpoint.FooBarSpec, if someone defines a Matrix-over-foobar transport -- e.g. low-bandwidth Matrix: matrix-org/matrix-doc#3079)

I think defining a custom transport is still a possibilty with the SDK, we would simply need to implement the behaviour for sending a request (including encoding and decoding)? This API could of course be adjusted and extended as necessary.

same as above, but we also need some way to specify a way to parse the result

Speaking of adjustements, this would work quite nicely with the decoupled transport: the SDK would benefit from having response parsing (it currently only has rudimentary error parsing implemented). Perhaps Polyjuice's parsing could provide some guidance for this?

[uhoreg]

I think defining a custom transport is still a possibilty with the SDK, we would simply need to implement the behaviour for sending a request (including encoding and decoding)? This API could of course be adjusted and extended as necessary.

While it would be possible to do that, I don't think it is a good way to do it. For example, the low-bandwidth makes some big changes to the request path. e.g. an HTTP request to the path /_matrix/client/r0/rooms/!7mqP7DYBUOmwAweF:localhost/send/m.room.message/$.AAABeH6obLU becomes a CoAP request to the path /9/!7mqP7DYBUOmwAweF:localhost/m.room.message/$.AAABeH6obLU (note that this isn't simply replacing a prefix in the path, but the /send that appears in the middle of the path also disappears). So the custom transport would need to parse the path and rewrite it, which would make it more complicated. It would be easier to just create the shortened CoAP path from the given parameters. If it was just a matter of throwing the exact same request data at a different transport, then I would agree that just creating a new module for the transport would be fine, but with needing to parse and mangle the path, I think it would be better to do that transformation closer to where we have access to the raw parameters.