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

Client server v2 #2

Closed
wants to merge 3 commits into from
Closed

Client server v2 #2

wants to merge 3 commits into from

Conversation

NegativeMjark
Copy link
Contributor

Version 2 of the client server API.

@NegativeMjark
Copy link
Contributor Author

From IRC:
<Arathorn> actually: ignore [10:39] <Arathorn> w.r.t. the strawman: my biggest concerns were whether the proposed HL APIs were too arbitrarily seperated
<Arathorn> i misread it
<Arathorn> actually, no, i didn't
<Arathorn> i'm basically confused at the proposal of both initialsync, scrollback and pagination
<Arathorn> why isn't scrollback a degenerate form of pagination (or vice versa)?
<kegan> there may be scope to merge some of these together; I purposefully am not trying to design the http apis at this stage, but rather what a client actually wants to do
<Arathorn> another issue i had was whether 'filters' are the right words. i'd prefer the language of subscribing to particular kinds of events, rather than 'filtering the global stream'
<kegan> if it turns out they can be merged, then cool
<kegan> but when I went through, I thought it would be easier to be clear that one was for "I just opened the room" and another for "I am infinite scrolling"
<Arathorn> hm, okay. i think it would be a real shame to end up with actual overlapping HTTP APIs which can be used to do the same thing, in terms of confusing the user on how to do things.

@ara4n
Copy link
Member

ara4n commented Dec 15, 2014

my only other concern is the amount of stuff it doesn't address. i'm particularly concerned that this helps us get further with the application services API (i.e. subscribing to events on behalf of virtual users, and possibly expose virtual rooms(?))
the whole "Pagination can take a while for backfill" thing feels like it should be addressed too - albeit by just letting backfill information trickle in down the eventstream if it's not available locally during the initial sync.
although i guess that's effectively what "FIX: Add flag to say server_local vs backfill_yes_please? Given the client is best suited to say how long they are willing to wait." is trying to say.

then and now, with the filter tokens specified. If it finds > 30 events for a given room, it returns a brand new page
for that room. If it finds < 30 events, it returns those events. Any new rooms are also returned. Returns a new stream token.
- GET /eventStream with new stream token. Blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the server gets old events for a room as part of resolving a net split. Does it just force a resync of the client? How does it do so if the client has already done an initialSync. Does it just expire the old stream tokens?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of order events in general are not covered by this proposal.

@NegativeMjark
Copy link
Contributor Author

Would it make sense to have a clear concept of a "session of actively using the client", i.e. hitting the event stream, using the API? It might make timing out presence and typing notifications due to inactivity simpler.

Currently this seems to be wrapped up in the "stream token" concept.

@NegativeMjark
Copy link
Contributor Author

Do have any way of identifying individual devices for presence and push notifications?

@erikjohnston
Copy link
Member

This is a good start.

Inline Comments

API Changes Summary

New API: Scrollback API: ...

How does this relate to room initial sync?

New API: Pagination Overview API: ...

I'm not sure how this is going to work given the fact that we might receive out of order events? I think this may possible, but will require a lot of thought.

Resolves issues

  • There are race conditions when getting events via room initial sync and from the event stream. FIX: optional streaming token param allows intelligent suppression
  • You can't tell if an event you PUT is the same event when it comes down the event stream. FIX: optional streaming token param allows intelligent suppression

I don't understand how the "fixes" solve these problems.

  • How do you obtain partial room state / handle large room state? FIX: You specify via the event filter token.

Do we also want to be able to "paginate" on room members?

  • How do you sensibly do incremental updates? FIX: You give it a streaming token to return incremental updates.

Again, I don't follow what the solution here is.

Issues not addressed

  • Can't set power levels incrementally.

The current API enforces atomic updates of power levels, which is quite a nice property.

  • Requesting context (read: events around) around an arbitrary event (which may have been 6 months ago)

From a federation/server point of view, its easy and reliable to get events before a given event. Its harder to reliably get events after.

  • Handle rejection of events after the fact. e.g. HS later finds out that it shouldn't have accepted an event. TODO: Clarifiy if 'rejection' === redaction.

Rejection can be either "I should not have shown you that event and its no longer part of the history of a room", or it may be included as part of history in its redacted form.

  • Distinguish between room EDUs (e.g. typing) and PDUs

It would be nice if we separated all types EDUs from PDUs.

  • Event timestamps (ISO8601?)

I really really want us to use timestamps. Either you have a date library or you don't. If you don't, then timestamps are easier to work with. If you do, then changing timestamps into ISO8601 is easy. I am of the opinion that first and foremost we should make the JSON easy to parse before we make it easy for humans to read.

Meta APIs

Generating an event filtering token

Do we also want to be able to specify negative filters? e.g. don't give me topic events.

What is the lifecycle of these tokens? When do they die?

Should we have some predefined default tokens?

Action APIs

Joining a room

We probably also want an API that allows us to specify a room ID and list of hosts to join via.

Syncing (aka I want live data)

NB: Does NOT provide any sort of 'catchup' service. This keeps the API simpler, and prevents potential attacks where people are dumb/maliciously request from ancient streaming tokens which then return 100000s of events, slowing down the HS. Alternatively, we could expire streaming tokens after a given time (but that doesn't help if 10000s of events come down really quickly). The general idea is to block all forms of historical data behind max events limits.

I don't think this is such a huge concern necessarily, since the home server can return less than the clients have requested. Home servers will probably have an internal limit (e.g. 100 or 1000) to make sure that clients can't do exactly this.

Examples

#2 Less buggy SYWEB recreation

  • Login. POST streaming token (users[""], rooms[""]). Store stream token.
  • POST event filter token (["m.room.message", "m.room.topic", "m.room.name", "m.room.member"]). Store as Message Event filter token.
  • POST event filter token (["m.room.name", "m.room.topic", "m.room.member"]). Store as Current State Event filter token.

These look like they could/should be combined into one.

Unaddressed topics

  • Multiple device support
  • Server level events - the server may want to inform their local clients about something (e.g., upcoming server maintenance, server status, "this room has been banned from this server", etc.)
  • Profiles
  • How do clients handle gaps in their room history?

@NegativeMjark
Copy link
Contributor Author

There doesn't seem to be any mention of the requirements. We are told that a "Less buggy SYWEB recreation" does "GET /initialSync max=1", but we have no idea what client-side requirement that is supposed to fulfil.

I think it would be worthwhile to specify the requirements for each of the hypothetical clients in more detail.

@ara4n
Copy link
Member

ara4n commented Dec 24, 2014

We also have lost improvements to the public room API (e.g. paginating or searching it)

@kegsay
Copy link
Member

kegsay commented Jan 5, 2015

Closing to avoid confusion.

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

Successfully merging this pull request may close these issues.

4 participants