ruma / homeserver

A Matrix homeserver written in Rust.
https://www.ruma.io/
1.08k stars 41 forks source link

Add /room/:room_id/state #135

Closed mujx closed 7 years ago

mujx commented 7 years ago

This PR is based on #127 which should be addressed first, so don't mind the other commits.

Diesel currently lacks support for the query so raw SQL was used.

I created a quick custom struct (lacks proper serialization) to represent the response of the endpoint because currently we don't have anything that represents the examples of the spec. There are some extra or renamed fields from the ones in ruma-events. Should we create another type for these on ruma-events? Customize the existing ones? or use something like my example?

farodin91 commented 7 years ago

this could help for your sql query https://github.com/farodin91/ruma/blob/5c72f1ea3904194815926cc4f3e2ffc77222573b/src/models/room.rs#L343

farodin91 commented 7 years ago

Rebase this branch to the master not to initial_state.

mujx commented 7 years ago

I use initial_state to create state events and facilitate testing.

Also the endpoint returns all the latest state events (one of everything), so your query won't work.

farodin91 commented 7 years ago

Okay

mujx commented 7 years ago

Here is the response format I was referring to.

We should probably start targeting the HEAD of the spec and modify ruma-events accordingly. Here are the updated event structs.

jimmycuadra commented 7 years ago

127 is merged now so this can be rebased on master.

jimmycuadra commented 7 years ago

I'm confused on the response type here. I don't see the "depth" field in either the r0.2.0 or unstable versions of the spec. In any case, I'd like to wait until r0.3.0 is published to change anything in ruma-events.

mujx commented 7 years ago

There isn't a depth field, my mistake. It was mostly a proof of concept so we can decide how to handle the updated event types.

The changes are really small though; the user_id is renamed to sender and a timestamp was added.

Should we implement this endpoint now with v0.2.0 events or wait until we update ruma-events with the current HEAD of the spec? According to the changelog the timestamp is backwards compatible but it doesn't mention something about sender.

The reason I'm asking is to ensure ruma is compatible with riot, since we are getting closer to the first release.

jimmycuadra commented 7 years ago

In general I want to drive Ruma's implementation on what the spec says rather than what Riot currently does. If the spec is wrong, we should file bugs against it. Matthew gave a very rough estimate of the end of January for r0.3.0. It sounds like that will be a blocker for finishing up the first Ruma release.

jimmycuadra commented 7 years ago

Is this still a work in progress or is it ready for review?

mujx commented 7 years ago

I have considered this blocked because the v0.2.0 version of the spec references fields from the next version, but that was probably a mistake and it doesn't really affect the endpoint. I will try to finish this with the current events.

farodin91 commented 7 years ago

Do you want to do any changes?

mujx commented 7 years ago

I also included the m.room.member event.