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

Proposal for the HTTP API for Client-Server v2 #6

Closed
wants to merge 20 commits into from

Conversation


set_presence: "offline" // optional parameter to tell the server not to interpret this request as a client as coming online (and as a convenience method for overriding presence state in general)

presence: true/false (default true): do we want to show presence updates?
Copy link
Contributor

Choose a reason for hiding this comment

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

You mention presence, but what about typing? What about other ephemeral details we might one day invent?

ara4n added 2 commits January 21, 2015 17:41
…ence), better and more accurate examples for /sync and friends, rename 'chunk' to be 'batch', specify mx:// URLs and more...
@ara4n
Copy link
Member

ara4n commented Jan 21, 2015

All of leo's concerns above should now have been addressed (albeit mainly because the comments referred to the prior incarnation of the presence semantics).

The core functionality here (Sync, Scrollback, Context APIs and Actions) should now be complete and good for implementing as alpha. Any other concerns, please voice them here asap.

@erikjohnston
Copy link
Member

Sync

// 'format' gives the desired shape of the response
// federation = include the federation layer as well as the raw content of events.
// events = the plain events
format: "federation",

I think these choices of values are going to be confusing. The format for events that get sent to the client (for the most part) is a subset of keys that are sent over federation. Due to all the signing that goes on I always think of the federation event format to be canonical, while the (current) c-s format is simply a stripped down version.

My suggestions are "full" and "stripped"/"partial"/"collapsed" or something like that. At the very least can we change the value "events" to something else so that its clear that the stripped down versions of events are not the canonical format?

Scrollback

The scrollback API doesn't seem to have a way of indicating the various states it could be in when returning less than the limit:

  • There are no more events in the room (i.e., we've paginated back to the start).
  • The server needs to talk to other servers to get more events (or is in the process of doing so).
  • The server has tried to find more events, but no other server has returned anything more. (We could still potentially retry again later.)

@erikjohnston
Copy link
Member

Otherwise, looks OK.

@kegsay
Copy link
Member

kegsay commented Jan 22, 2015

Can we s/filter_id/filter_token/g please? Or is ID chosen for a particular reason?

@ara4n
Copy link
Member

ara4n commented Jan 23, 2015

On 22/01/2015 16:49, Kegsay wrote:

Can we |s/filter_id/filter_token/g| please? Or is ID chosen for a
particular reason?

filter_id not filter_token as the token doesn't change between uses of
the filter
unlike batch_tokens which keep changing and are fairly ephemeral
batch_tokens represent a token in a stream
whereas a filter_id is just, well, the ID you use to refer to a
particular filter
just like a room_id or user_id.

@kegsay
Copy link
Member

kegsay commented Jan 23, 2015

Okay, that seems reasonable. We do reserve the right for home servers to expire filter ids, but it should be infrequent enough to not be confusing.

To represent events as "{event}" and not "event_id: {event}", given these events do not appear in any event map (since they aren't tied to a room and hence are never prone to duplication in events/state).
// N.B. This only takes effect when paginating, and is ignored for streaming data, and can only be specified once per filter.
//
// the sort order of messages in the room, *only honoured during an initial sync*. default: "timeline,asc". may appear multiple times
// subsequent calls to /sync will always return event updates in timeline order (thanks to causality)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should say "return event updates in the order they arrive at the server" rather than "timeline order". Since timeline order is different from the arrival ordering.

Copy link
Member

Choose a reason for hiding this comment

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

done

@kegsay
Copy link
Member

kegsay commented Jan 26, 2015

From earlier on #matrix-dev:

hmm, just had a thought on filters. For the /sync API, what I really want is to say "limit=10 for this filter ID, but return results from this filter ID" which is awkward since we don't currently support that. I want the web client to get all events (for display in the room dialog), but I want to apply limit=10 for displayable events.

in terms of the API changes, it would just be something like filter_limit=$filter_id I guess, so it's not the end of the world, but I need to work through an exact example of what that would look like. I guess all filter_limit is doing is serving as the filter to determine what counts as "10" when you say limit=10, but return results by filtering on filter=

This concept needs to be added to the HTTP spec. The use case being the web client: I want to display the 10 most recent displayable events, but I want to receive all the different event types evar so I can populate the state events dialog, which currently you can't do.

@kegsay
Copy link
Member

kegsay commented Jan 26, 2015

I guess this could be done as two /sync requests, one for displayable events limit 10, and another for the current state (limit 0?). The main downside to what I suggested above is that there is an indeterminate amount of events between the most recent 10 displayable events and "now" potentially.

@ara4n
Copy link
Member

ara4n commented Jan 26, 2015

Surely the problem here is that we want may want total state for the room (which can't be paginated currently), but only 10 visible events. limit=10 would never apply to state anyway, so how about having two filter params - one specifically for state, and the other specifically for general timeline events?

@kegsay
Copy link
Member

kegsay commented Jan 27, 2015

That could work. I could then say state_filter=* and msg_filter=[m.room.message/topic/name/member] and be done with it, and we preserve the original proposed behaviour by specifying the same filter for both.

@ara4n
Copy link
Member

ara4n commented Jan 27, 2015

in spec-3 we defined non-state events to be called messages. instead i suggest events_filter or timeline_filter or even graph_filter?

On 27 Jan 2015, at 09:18, Kegsay [email protected] wrote:

That could work. I could then say state_filter=* and msg_filter=[m.room.message/topic/name/member] and be done with it, and we preserve the original proposed behaviour by specifying the same filter for both.


Reply to this email directly or view it on GitHub.

@NegativeMjark
Copy link
Contributor Author

Should control over what state is filtered and what events are filtered be part of the filter itself? Rather than having two filters. Especially if we are using the filter to control what format the events are in given that the event_map contains both events from the state and from the recent events.

@kegsay
Copy link
Member

kegsay commented Jan 27, 2015

events_filter or timeline_filter or even graph_filter?

Given these are events which appear in the events key for a room, I like events_filter.

Should control over what state is filtered and what events are filtered be part of the filter itself?

My gut reaction originally said "no, the filter just controls which events are obtained and how each event is represented (eg which keys), not room related stuff". However, there is already a precedent for modifying global/room stuff via public_user_data: true, and co. Therefore, I agree with Mark and think that this should probably form part of the filter. If it is part of the filter, should we be mimicking the structure of the /sync response more to make the relation clearer? Something like:

room: {
  state: {
    types: ["something"],
    not_senders: ["@foo:bar"]
  },
  events: {
    types: ["something"],
    not_senders: ["@foo:bar"]
  }
}

Which would end up with rather satisfyingly symmetric filter request keys/sync response keys:

Creating filter:
{
  room: {
    state: {
      types: ["something"],
      not_senders: ["@foo:bar"]
    },
    events: {
      types: ["something"],
      not_senders: ["@foo:bar"]
    }
  },
  public_user_data: true,
  private_user_data: true
}

Sync response:
{
  next_batch: "foo",
  rooms: [
    {
      state: [ event_ids ],
      events: [ event_ids ],
      ...
    }
  ],
  public_user_data: [
    { event }
  ],
  private_user_data: [
    { event }
  ]
}

Do we want public/private user data to just be bools at this point? If they are, which filter is applied when I say public_user_data: true? The state one? It's not clear. We also need clear guidelines on what would be public_user_data (e.g. an event without a room_id).

@NegativeMjark
Copy link
Contributor Author

Should we be specifying separate lists for public/private user data as well rather than true/false?
Creating filter:

{
 room: {
   state: {
     types: ["something"],
     not_senders: ["@foo:bar"]
   },
   events: {
     types: ["something"],
     not_senders: ["@foo:bar"]
   }
 },
 public_user_data: {
   types: ["something"],
   not_senders: ["@foo:bar"]
 },
 private_user_data: {
   types: ["something"],
   not_senders: ["@foo:bar"]
 }
}

@kegsay
Copy link
Member

kegsay commented Jan 27, 2015

That looks good to me. This way you know exactly what you're filtering on for each key in /sync. As for determining the "pool" of events that you're filtering on, the room ones are fairly obvious (timeline order for events and presence of state_key for state), but the other 2 are more interesting. Presumably, the absence of a room_id means that it is public_user_data, but where does typing fit into this?

We need to work out the semantics for private_user_data too. Given it's user only, and given it's just a generic store, it is probably Good Enough to just say "hey, if you PUT stuff to your private userdata then you can filter on it if you know which events you're working with". I would typically use this to store app-specific config info in my own event type namespace e.g. org.matrix.console.android.* and then use that as my filter on private_user_data.

@erikjohnston
Copy link
Member

Presumably, the absence of a room_id means that it is public_user_data, but where does typing fit into this?

Can we please have a better way of differentiating types of events than this? An absence of a room_id key really only means that its not tied to a room; for example I want to people able to send down server status messages which won't have a room_id.

@ara4n
Copy link
Member

ara4n commented Jan 27, 2015

More than happy with mjarkegan's suggestion above; my bad for not proposing it in the first place.
Not sure why Erik is worried about distinguishing user_data from graph and state events given we surely just look at where in the /sync datastructure they're returned... (i.e. are they in a user_data key, or a state key, or an events key?)

@NegativeMjark
Copy link
Contributor Author

I think Erik is complaining that we have lost the ability for servers to send messages to a client outside of a room.

@kegsay
Copy link
Member

kegsay commented Jan 28, 2015

So the outstanding problems on this are:

  • Where do m.typing events go?
  • Where do server-generated events not scoped to a room go?

The general API mentions that there should be a prefix to the event type to indicate state/message/ephemeral event. We currently determine the different kinds of events based on the keys e.g. state vs events vs public_user_data, so it may be worth adding in different sections for these kinds of events? E.g.

{
 room: {
   state: {
     types: ["something"],
     not_senders: ["@foo:bar"]
   },
   events: {
     types: ["something"],
     not_senders: ["@foo:bar"]
   },
   ephemeral: {
     types: ["something"],
     not_senders: ["@foo:bar"]
   }
 },
 public_user_data: {
   types: ["something"],
   not_senders: ["@foo:bar"]
 },
 private_user_data: {
   types: ["something"],
   not_senders: ["@foo:bar"]
 },
 server_data: {
   types: ["something"],
   not_senders: ["@foo:bar"]
 }
}

This covers all the different kinds of events which exist (or are planned to exist in the future):

  • Room state events like m.room.name : room.state
  • Room events (state and message) like m.room.message: room.events
  • Room EDUs like m.typing: room.ephemeral
  • Private user data like push/app specific configs: private_user_data
  • Public user data like m.profile and m.presence: public_user_data
  • Server-generated events (not room scoped): server_data
  • Server-generated events (room-scoped), with modified sender as per general API: room.events

This would negate the need to prefix event types since you can tell what kind of event it is based on the key it is inside of. For sending those event types, you hit different CS endpoints e.g. /user/{userId}/public/{eventType} for public_user_data, /room/{roomId}/state/{eventType}/{stateKey} for room.state, and so on.

@ara4n
Copy link
Member

ara4n commented Jan 28, 2015

On 28/01/2015 15:41, Kegsay wrote:

So the outstanding problems on this are:
The general API mentions that there should be a prefix to the event type
to indicate state/message/ephemeral event. We currently determine the
different kinds of events based on the keys e.g. |state| vs |events| vs
|public_user_data|, so it may be worth adding in different sections for
these kinds of events?

wfm

@ara4n
Copy link
Member

ara4n commented Feb 7, 2015

kegan: the last commit doesn't seem entirely complete - don't we need ephemeral blocks on the response for /sync too? And do we have a way to distinguish ephemeral non-room data from non-ephemeral?

@kegsay
Copy link
Member

kegsay commented Feb 19, 2015

don't we need ephemeral blocks on the response for /sync too?

What's the use case?

do we have a way to distinguish ephemeral non-room data from non-ephemeral?

When would we want ephemeral non-room data?

It's a lot easier to add these fields in at a later date, but very difficult to remove them if they turn out to be duds, hence prompting for use cases.

@richvdh
Copy link
Member

richvdh commented Jun 16, 2016

At this point the comments in the PR are more valuable than the actual change itself. The change isn't going to land in its current form, so we'll close this PR.

Ideal next steps from here (with sufficient round tuits) would be:

  1. attempt to distill useful information from the conversation so far into a document in the google-docs drafts
  2. argue further there
  3. create a PR which updates the spec, rather than adds to /drafts

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.

7 participants