1141105 - Implement server changes for context sharing in conversations

Status: RESOLVED FIXED

(Hello (Loop) :: Server, defect)

Description

[Mark Banner (:standard8)]

This bug is for the server side changes to implement the changes for context sharing in conversations. The server team may wish to use this for all implementation or as a meta bug.

Details of changes are here:

https://wiki.mozilla.org/Loop/Architecture/Context

Basic summary; For the following APIs:

- POST /rooms
- PATCH /rooms/{token}
- GET /rooms/{token}
- GET /rooms

the wiki page says to replace "roomName" with "context" which is used to store and access encrypted data.

Comment 1

[Mark Banner (:standard8)]

Adam, what are we going to do with respect to backwards-compatibility?

If we remove roomName, then the new clients won't have a way to get the roomName (without a separate request on an old API), and likewise old clients won't be able to access the roomName...

Should we have a period of overlap where both attributes are supported?

Comment 2

[Adam Roach [:abr]]

For the circumstances in which we have an FxA user on two different browsers, I was actually going to be okay with the roomName simply becoming unavailable for older clients. A one-cycle transition period probably makes sense, to give people six weeks or so to get their clients updated. Here's what I'd propose:

- When a 39 client finds a room with a roomName, it copies it into the encrypted context and leaves it on the room. Changes to the room name are written to both the roomName and the context fields.

- In 40, we change this behavior so that any context changes instead set roomName to something like "Upgrade Firefox to see room names."

- Sometime in the 41-42 timeframe, we update the server to stop using the roomName field altogether.

Comment 3

[Mark Banner (:standard8)]

That sounds reasonable to me.

Comment 4

[Rémy Hubscher (:natim)]

After thinking this through, it make sense to store the context as an encrypted file as it will probably grow in size in the future.

We will store the context in a S3 file that will be linked in the redis database and on the API side it will looks like what Adam explained.

We are working on it.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Thanks Rémy!

Comment 6

[:shell escalante]

Just checking - Adam says in the Fx41-42 timeframe to update the server to stop using the roomName field altogether.  For timing planning, do you mean when Fx41 goes to Central (May 11th) or when it releases (Sept 22)?

Asking as we're working on who to get to update the FxOS client and the timing.

Comment 7

[Rémy Hubscher (:natim)]

Please note that this change will add some delay in the room endpoint response time. (Around 300-500ms I guess but we will be able to evaluate this exact time with loadtest as soon as it gets deployed to stage.)

Tarek ask me to let you know to see if on the UX side you need to handle this extra delay.

Comment 8

[Mark Banner (:standard8)]

(In reply to Rémy Hubscher (:natim) from comment #7)
> Please note that this change will add some delay in the room endpoint
> response time. (Around 300-500ms I guess but we will be able to evaluate
> this exact time with loadtest as soon as it gets deployed to stage.)

Is this is going to affect the the time to "get all rooms"? If so, is it 300-500ms per room, or a one-off hit?

From the standalone perspective I'm not too worried I think. I'm more concerned about the effects on the UI - initially getting the list, updating it after a push notification, etc.

Comment 9

[Rémy Hubscher (:natim)]

> Is this is going to affect the the time to "get all rooms"? If so, is it 300-500ms per room, or a one-off hit?

Let's see with the loadtests.

Comment 10

[Adam Roach [:abr]]

(In reply to sescalante from comment #6)
> Just checking - Adam says in the Fx41-42 timeframe to update the server to
> stop using the roomName field altogether.  For timing planning, do you mean
> when Fx41 goes to Central (May 11th) or when it releases (Sept 22)?

When it releases.

> Asking as we're working on who to get to update the FxOS client and the
> timing.

If we need to hold longer, we can; but we do want to get rid of roomName sooner than later, as it introduces complications in the case that it differs from the encrypted context information.

Comment 11

[Adam Roach [:abr]]

(In reply to Mark Banner (:standard8) from comment #8)
> (In reply to Rémy Hubscher (:natim) from comment #7)
> > Please note that this change will add some delay in the room endpoint
> > response time. (Around 300-500ms I guess but we will be able to evaluate
> > this exact time with loadtest as soon as it gets deployed to stage.)
> 
> Is this is going to affect the the time to "get all rooms"? If so, is it
> 300-500ms per room, or a one-off hit?
> 
> From the standalone perspective I'm not too worried I think. I'm more
> concerned about the effects on the UI - initially getting the list, updating
> it after a push notification, etc.

Note that we're now going to be storing per-room information so that we don't completely lose everything during a lost-password password reset. If you also store the context information itself, you can display the cached information while waiting for a response from the server. While 500ms of displaying no information is pretty bad, 500ms of displaying slightly stale information is a lot less problematic.

Comment 12

[Alexis Metaireau (:alexis)]

This is in a master and the ship is about to sail.

https://github.com/mozilla-services/loop-server/commit/b28d3c00ec3ebc806d2111ca561d0608bcf42a8a