Give priority to users connected to collaboration server (aka no websocket feature) #1093
Conversation
We need a new endpoint in the y-provider server allowing the backend to retrieve the number of active connections on a document and if a session key exists.
When a document is updated, users not connected to the collaboration server can override work made by other people connected to the collaboration server. To avoid this, the priority is given to user connected to the collaboration server. If the websocket property in the request payload is missing or set to False, the backend fetch the collaboration server to now if the user can save or not. If users are already connected, the user can't save. Also, only one user without websocket can save a connect, the first user saving acquire a lock and all other users can't save. To implement this behavior, we need to track all users, connected and not, so a session is created for every user in the ForceSessionMiddleware.
lunika
force-pushed
the
feat/no-websocket-v2
branch
from
7110c60 to
e1409ae
Compare
| @@ -470,6 +471,7 @@ class Base(Configuration): | |||
| SESSION_COOKIE_AGE = values.PositiveIntegerValue( | |||
| default=60 * 60 * 12, environ_name="SESSION_COOKIE_AGE", environ_prefix=None | |||
| ) | |||
| SESSION_COOKIE_NAME = "docs_sessionid" | |||
There was a problem hiding this comment.
note: every user will be disconnected on next deployment
| f"Response: {response.text}" | ||
| ) | ||
|
|
||
| return response.json() |
There was a problem hiding this comment.
Please add a comment (or a dataclass) to easily know the json is something like {"count": int, "exists": bool}?
I'm even wondering if the return of this method should not be directly return count, exists
| def _compute_no_websocket_cache_key(self, document_id): | ||
| """Compute the cache key for the no websocket cache.""" | ||
| return f"docs:no-websocket:{document_id}" |
There was a problem hiding this comment.
This is called only once, are you sure you need a dedicated method instead of an f-string where used?
(also this method should be static)
| self.request.session.session_key, | ||
| ) | ||
| except requests.HTTPError as e: | ||
| capture_exception(e) |
There was a problem hiding this comment.
| capture_exception(e) | |
| logger.exception("Failed to call collaboration server: ", e) |
No?
| if self._can_user_edit_document(serializer.instance.id, set_cache=True): | ||
| return super().perform_update(serializer) |
There was a problem hiding this comment.
You may want to raise a specific error content to display a proper message to the user
|
|
||
| def perform_update(self, serializer): | ||
| """Check rules about collaboration.""" | ||
| if serializer.validated_data.get("websocket"): |
There was a problem hiding this comment.
| if serializer.validated_data.get("websocket"): | |
| if serializer.validated_data.get("websocket", False): |
Just to be explicit with the default behavior (I know None is falsy, but...)
| """Force session creation for unauthenticated users.""" | ||
|
|
||
| if not request.user.is_authenticated and request.session.session_key is None: | ||
| request.session.save() |
There was a problem hiding this comment.
Why save instead of create?
Is there a test somewhere to check this middleware behavior?
Purpose
Some users have no access to the websocket protocol and can't collaborate to a document. This lead to inconsistent document content because they will override the content on every save.
With this PR we implement a new behavior to prevent the loss of data.
Priority is given to users connected to the collaboration server, while users are connected to it, users without websocket can't save their content, no matter if a user without websocket came first on the document.
Also there is a priority between user without websocket. The first user saving a document acquire a lock and this lock remains active while the user is working on the document. After 2 minutes of inactivity, the user release the lock and an other user without a websocket can save its work.
Proposal