Server awareness of client's user store about comm HOT 2 CLOSED

We're getting to the point where we can finally get rid of all the code on the server that is constantly sending down userInfos. There is a lot of historical cruft to deal with here, so I wanted to take the time to outline the steps we'll need to go through in order to do this.

Step 1: making sure server updates clients when their userStore should change

This step is almost complete, but there was one oversight I missed during review.

In commitMembershipChangeset, we call updateUndirectedRelationships to update the DB with our new relationships. However, updating the DB is not enough - we also need to issue updateTypes.UPDATE_USER to make sure that clients get the update. Otherwise, they will only see the update if they do a full state sync.

I think we should extract these lines into a new function: https://github.com/Ashoat/squadcal/blob/2739065ff9445795a6e1df0f4a5e90db3c5fcdd0/server/src/updaters/relationship-updaters.js#L171-L186

We can then merge the updateTypes.UPDATE_USER with the other updateDatas in commitMembershipChangeset. Let's make sure that the updateTypes.UPDATE_USER come before the other updateDatas in the array. This doesn't currently affect anything, since right now we guarantee that a batch of updates created together will always be delivered together. But just in case we break this assumption in the future, we should make sure the updateTypes.UPDATE_USER updates come first.

Step 2: handle users who have left a thread

This is a continuation of step 1, but it's an edge case I realized we hadn't considered previously. This is probably the hardest step on this list.

Right now, when the server sends down a message or a calendar entry, it makes sure to send down the UserInfo of the author as well. This guarantees that if an author has left the thread before a new user has been added to it, the new user still gets the UserInfo object.

We don't currently consider this case in our userStore syncing logic. I think we will have to rethink some of the logic in thread-permission-updaters.js, unfortunately.

The solution I have in mind is to keep a membership row for a user, even after they leave a thread. We can use role = -1 to signify a former member, and we can edit the permissions column to support NULL values.

In every existing SQL query that touches memberships, we should make sure to only return rows where role >= 0. The only exception will be in thread-permission-updaters.js, where we will make sure to only look at role = -1 rows when determining relationship rows.

We'll need to update deleteMemberships to do an UPDATE instead of a DELETE FROM.

We will also need to update the create-relationships.js migration to look at any users who have sent a message or created a calendar entry for a given thread.

Step 3: update state sync

We have two kinds of state sync: incremental and full. Full state sync will still need userInfos, but can get it via a single fetchKnownUserInfos fetch instead of combining userInfos from multiple sources.

Incremental state sync will be more complicated. It might seem like we can get rid of the userInfos key. But there is an important quirk to how updateTypes.UPDATE_USER works:

https://github.com/Ashoat/squadcal/blob/2739065ff9445795a6e1df0f4a5e90db3c5fcdd0/lib/types/update-types.js#L244

In short, the UpdateInfo for updateTypes.UPDATE_USER does not include a UserInfo object. Instead, we rely on the UserInfo object being included in the userInfos that are returned from createUpdates and fetchUpdateInfosWithQuery.

That means that in order to avoid breaking old clients, we need to keep sending them a userInfos key in the incremental state sync.

For new clients, we could move the UserInfo objects to the actual updateTypes.UPDATE_USER updates, and then we could get rid of the userInfos key. But I think this probably isn't worth our time right now.

The easiest thing would be to update the incremental state sync so it only returns the userInfos that we get from the updatesResult here:

https://github.com/Ashoat/squadcal/blob/2739065ff9445795a6e1df0f4a5e90db3c5fcdd0/server/src/socket/socket.js#L513-L517

Step 4: update createUpdates and fetchUpdateInfosWithQuery

These two functions are the only places where the server constructs UpdateInfo objects to return to the client. createUpdates gets called when an update is first created, and makes sure to propagate any updates to clients with an active socket. For any non-active clients, a row is added to the MySQL table updates. The next time those clients connect, fetchUpdateInfosWithQuery looks at the MySQL table, creates UpdateInfos from the rows, and then returns them to the client.

Both of these functions go through updateInfosFromRawUpdateInfos, which handles constructing userInfos for FetchUpdatesResult. Right now we include any userInfos that are relevant to an update. We should update this code so we only include userInfos from updateTypes.UPDATE_USER.

Step 5: update MessagesServerSocketMessage

Just like createUpdates, createMessages is set up to immediately deliver messages to any clients with an active socket. Right now it includes the UserInfos of any message authors, but we don't need to do this going forward.

Actually, it looks like the client was never looking at the userInfos field of MessagesResultWithUserInfos anyways! I think we can safely remove this field entirely. No need to return an empty object, since it looks like no client has ever looked there.

Step 6: clean up actions/responders that currently deliver userInfos

We have several client/server interactions that currently deliver userInfos to the client. We don't need these anymore. The full list is covered by this conditional at the top of reduceUserInfos:

https://github.com/Ashoat/squadcal/blob/2739065ff9445795a6e1df0f4a5e90db3c5fcdd0/lib/reducers/user-reducer.js#L118-L123

searchUsersActionTypes is a special case. Current clients rely on this returning UserInfos, and it isn't handled by any updateTypes.UPDATE_USER. So we need to let the server continue returning UserInfos here. However, we can update new clients to no longer merge the UserInfos into the userStore, especially since this will confuse the state check mechanism.

joinThreadActionTypes is also a special case, because the server currently extracts the userInfos from the updatesResult and puts them in action.payload.userInfos. We should update the server so that it only includes the UserInfos from updatesResult here, but it will be easiest to leave the client alone.

For the other four, I don't think we need to return userInfos from the server anymore. Since old clients will continue expecting an object in action.payload.userInfos, I think it will be easiest to just return an empty object here.

For new clients, we should update reduceUserInfos so it no longer looks at these actions at all.

Step 7: clean up entry logic that currently delivers userInfos

This is kind of a continuation of step 6. reducerUserInfos currently handles calendar entry updates in a separate section:

https://github.com/Ashoat/squadcal/blob/2739065ff9445795a6e1df0f4a5e90db3c5fcdd0/lib/reducers/user-reducer.js#L172-L194

Similar to step 6, I don't think the server needs to return userInfos here anymore. I think once step 4 is complete, we will automatically be returning an empty object here anyways.

I think the only thing we need to do for these is to update the client code. New clients shouldn't need to look at these actions at all.

Step 8: createAccountDeletionUpdates in deleteAccount

This part should be pretty easy. Right now when a user deletes an account, we tell every single client. We should update this so we only tell clients whose users have a know_of row with the deleted account.

Step 9: clean up server fetchers

Finally!! We are almost done. The last step is to clean up all of the remaining server code. I think most of this will be in server/src/fetchers. Functions like fetchThreadInfos currently return a list of UserInfos, but they shouldn't need to anymore.

Any SQL query where you see u.username is probably an example of something that needs to be cleaned up. I think there are three files we need to fix up:

• server/src/fetchers/entry-fetchers.js
• server/src/fetchers/message-fetchers.js
• server/src/fetchers/thread-fetchers.js

from comm.

Note - I addressed step 1 in 28bb013

from comm.