Implement server changes for context sharing in conversations

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: standard8, Assigned: rhubscher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [loop-server 0.17.4])

(Reporter)

Description

4 years ago
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.
(Reporter)

Comment 1

4 years ago
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?
Flags: needinfo?(adam)

Comment 2

4 years ago
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.
Flags: needinfo?(adam)
(Reporter)

Comment 3

4 years ago
That sounds reasonable to me.
(Assignee)

Comment 4

4 years ago
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.
Assignee: nobody → rhubscher
Status: NEW → ASSIGNED
(Reporter)

Updated

4 years ago
Blocks: 1142522
(Reporter)

Updated

4 years ago
Blocks: 1142589

Comment 6

4 years ago
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.
Flags: needinfo?(adam)
(Assignee)

Comment 7

4 years ago
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.
(Reporter)

Comment 8

4 years ago
(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.
(Assignee)

Comment 9

4 years ago
> 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

4 years ago
(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.
Flags: needinfo?(adam)

Comment 11

4 years ago
(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.
This is in a master and the ship is about to sail.

https://github.com/mozilla-services/loop-server/commit/b28d3c00ec3ebc806d2111ca561d0608bcf42a8a
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [loop-server 0.17.4]
You need to log in before you can comment on or make changes to this bug.