Lock file when edited by collabora

[basilelegal]

If a file is edited using collabora we need to lock the upload of a new file for the same content.

Create 2 new endpoint:

• ẁopi/files//lock
• wopi/files//unlock

[inkhey]

It's unclear what a lock mean it this case. Locking mecanism can be really tricky depending on what you are expecting.

[lebouquetin]

@inkhey in which case the lock is tricky? I think the lock here means that another user can't update/upgrade the file until the lock is released.

[inkhey]

@lebouquetin I'm not sure how we should implement this lock in order to allow tracim to always work correctly as multiprocess app and with different databases engine.

By the way, i'm not sure, we should use this kind of mecanism for libreoffice. There is another way to deal with this: send last modified time or revision_id as entry and if this doesn't match properly, we say "something as changed" and we can have some kind of merge mecanism. That can be tricky for ui but that have also some avantages.

Up to now, i thought that wopi support only lock mecanism, but it seems that libreoffice online doesn't really support lock mecanism by itself. it support last modified date check and merge : https://github.com/LibreOffice/online/blob/master/wsd/reference.md#detecting-external-document-change

[inkhey]

just added support for libreoffice "file_as_changed" spec in collabora_branch: 8a3b2fa4de79c20ed186c9e4757be674263c8274

[tracim]

@inkhey "file as changed" is "file has changed".

[tracim]

We have discussed about implementing a lock for tracim upload if file is being edited online.

I do not understand the database related problem.

The case we must manage is: do not allow to upload a new revision of a file if file is currently being edited.

[inkhey]

just try a poc on https://github.com/tracim/tracim/tree/fix/2073__lock_mecanism_for_new_revisions

It does:

• block only upload new file endpoint (doesn't block any other change, or upload via webdav)
• add add/remove lock tracim endpoint.

It does not:

• lock wopi endpoint : we do accept any upload through this endpoint now. There is no lock token verification here at this point, which may be tricky as we need to rely on wopi protocol itself.
• handle specific lock/unlock endpoint of wopi (collabora/libreoffice doesn't support this but office does)
• handle case a lock stay on file for a very long time (we should think about deprecate lock after some time).

diff --git a/backend/tracim_backend/__init__.py b/backend/tracim_backend/__init__.py
index 8ae25e9ac..b4be2ca4b 100644
--- a/backend/tracim_backend/__init__.py
+++ b/backend/tracim_backend/__init__.py
@@ -60,6 +60,7 @@ from tracim_backend.views.contents_api.folder_controller import FolderController
 from tracim_backend.views.contents_api.html_document_controller import HTMLDocumentController
 from tracim_backend.views.contents_api.threads_controller import ThreadController
 from tracim_backend.views.core_api.account_controller import AccountController
+from tracim_backend.views.core_api.content_lock_controller import ContentLockController
 from tracim_backend.views.core_api.reset_password_controller import ResetPasswordController
 from tracim_backend.views.core_api.session_controller import SessionController
 from tracim_backend.views.core_api.system_controller import SystemController
@@ -193,6 +194,7 @@ def web(global_config: OrderedDict, **local_settings) -> Router:
     thread_controller = ThreadController()
     file_controller = FileController()
     folder_controller = FolderController()
+    content_lock_controller = ContentLockController()
     configurator.include(session_controller.bind, route_prefix=BASE_API_V2)
     configurator.include(system_controller.bind, route_prefix=BASE_API_V2)
     configurator.include(user_controller.bind, route_prefix=BASE_API_V2)
@@ -204,7 +206,7 @@ def web(global_config: OrderedDict, **local_settings) -> Router:
     configurator.include(thread_controller.bind, route_prefix=BASE_API_V2)
     configurator.include(file_controller.bind, route_prefix=BASE_API_V2)
     configurator.include(folder_controller.bind, route_prefix=BASE_API_V2)
-
+    configurator.include(content_lock_controller.bind, route_prefix=BASE_API_V2)
     # INFO - G.M - 2019-08-08 - import app here instead of top of file,
     # to make thing easier later
     # when app will be load dynamycally.
diff --git a/backend/tracim_backend/error.py b/backend/tracim_backend/error.py
index 7a6e220d5..ced579e78 100644
--- a/backend/tracim_backend/error.py
+++ b/backend/tracim_backend/error.py
@@ -19,6 +19,7 @@ class ErrorCode(IntEnum):
     CONTENT_TYPE_NOT_EXIST = 1006
     CONTENT_SHARE_NOT_FOUND = 1007
     UPLOAD_PERMISSION_NOT_FOUND = 1008
+    CONTENT_LOCK_NOT_FOUND = 1009
     # Preview Based
     UNAVAILABLE_PREVIEW_TYPE = 1011
     PAGE_OF_PREVIEW_NOT_FOUND = 1012
@@ -78,6 +79,7 @@ class ErrorCode(IntEnum):
     USER_ROLE_ALREADY_EXIST = 3008
     CONFLICTING_MOVE_IN_ITSELF = 3009
     CONFLICTING_MOVE_IN_CHILD = 3010
+    CONTENT_LOCKED = 3011

     # Auth Error
     AUTHENTICATION_FAILED = 4001
diff --git a/backend/tracim_backend/exceptions.py b/backend/tracim_backend/exceptions.py
index 623314f97..9f2225704 100644
--- a/backend/tracim_backend/exceptions.py
+++ b/backend/tracim_backend/exceptions.py
@@ -150,6 +150,14 @@ class ContentShareNotFound(NotFound):
     error_code = ErrorCode.CONTENT_SHARE_NOT_FOUND

+class ContentLockNotFound(NotFound):
+    error_code = ErrorCode.CONTENT_LOCK_NOT_FOUND
+
+
+class ContentLocked(TracimException):
+    error_code = ErrorCode.CONTENT_LOCKED
+
+
 class WorkspaceNotFoundInTracimRequest(NotFound):
     error_code = ErrorCode.WORKSPACE_NOT_IN_TRACIM_REQUEST

diff --git a/backend/tracim_backend/lib/core/upload_content_lock.py b/backend/tracim_backend/lib/core/upload_content_lock.py
new file mode 100644
index 000000000..f14fa67d9
--- /dev/null
+++ b/backend/tracim_backend/lib/core/upload_content_lock.py
@@ -0,0 +1,74 @@
+import typing
+import uuid
+
+from sqlalchemy.orm import Session
+from sqlalchemy.orm.exc import NoResultFound
+
+from tracim_backend import CFG
+from tracim_backend.exceptions import ContentLockNotFound
+from tracim_backend.models.auth import User
+from tracim_backend.models.content_lock import UploadContentLock
+
+
+class UploadContentLockLib(object):
+    def __init__(
+        self,
+        current_user: typing.Optional[User],
+        session: Session,
+        config: CFG,
+        show_deleted: bool = False,
+    ) -> None:
+        self._session = session
+        self._user = current_user
+        self._config = config
+        self.show_deleted = show_deleted
+
+    def _base_query(self):
+        query = self._session.query(UploadContentLock)
+        if not self.show_deleted:
+            query = query.filter(UploadContentLock.enabled == True)  # noqa: E711
+        return query
+
+    def get_lock_by_token(self, lock_token: str, content_id: int) -> UploadContentLock:
+        try:
+            return (
+                self._base_query()
+                .filter(UploadContentLock.lock_token == lock_token)
+                .filter(UploadContentLock.content_id == content_id)
+                .one()
+            )
+        except NoResultFound as exc:
+            raise ContentLockNotFound(
+                'Content Lock with token "{}" not found in database'.format(lock_token)
+            ) from exc
+
+    def get_content_locks(self, content_id: int) -> UploadContentLock:
+        return self._base_query().filter(UploadContentLock.content_id == content_id).all()
+
+    def can_upload_content(self, content_id: int, lock_token: typing.Optional[str]):
+        if lock_token:
+            try:
+                self.get_lock_by_token(lock_token, content_id)
+                return True
+            except ContentLockNotFound:
+                return False
+        else:
+            return len(self.get_content_locks(content_id)) == 0
+
+    def add_lock(
+        self, content_id: int, lock_token: typing.Optional[str] = None
+    ) -> UploadContentLock:
+        if not lock_token:
+            lock_token = str(uuid.uuid4().hex)
+        return UploadContentLock(
+            lock_token=lock_token, content_id=content_id, user_id=self._user.user_id
+        )
+
+    def remove_lock(self, lock_token: str, content_id: int) -> UploadContentLock:
+        lock = self.get_lock_by_token(lock_token, content_id)
+        lock.enabled = False
+        return lock
+
+    def save(self, upload_content_lock: UploadContentLock) -> UploadContentLock:
+        self._session.add(upload_content_lock)
+        return upload_content_lock
diff --git a/backend/tracim_backend/lib/utils/authorization.py b/backend/tracim_backend/lib/utils/authorization.py
index 933b9cdd6..ad32d21fe 100644
--- a/backend/tracim_backend/lib/utils/authorization.py
+++ b/backend/tracim_backend/lib/utils/authorization.py
@@ -8,12 +8,14 @@ from zope.interface import implementer

 from tracim_backend.app_models.contents import ContentTypeList
 from tracim_backend.app_models.contents import content_type_list
+from tracim_backend.exceptions import ContentLocked
 from tracim_backend.exceptions import ContentTypeNotAllowed
 from tracim_backend.exceptions import InsufficientUserProfile
 from tracim_backend.exceptions import InsufficientUserRoleInWorkspace
 from tracim_backend.exceptions import TracimException
 from tracim_backend.exceptions import UserGivenIsNotTheSameAsAuthenticated
 from tracim_backend.exceptions import UserIsNotContentOwner
+from tracim_backend.lib.core.upload_content_lock import UploadContentLockLib
 from tracim_backend.lib.utils.request import TracimContext
 from tracim_backend.models.auth import Group
 from tracim_backend.models.roles import WorkspaceRoles
@@ -175,6 +177,24 @@ class ContentTypeChecker(AuthorizationChecker):
         raise ContentTypeNotAllowed()

+class ContentNotLocked(AuthorizationChecker):
+    """
+    Check if content is locked or not
+    """
+
+    def check(self, tracim_context: TracimContext):
+        locklib = UploadContentLockLib(
+            config=tracim_context.app_config,
+            current_user=tracim_context.current_user,
+            session=tracim_context.dbsession,
+        )
+        if locklib.can_upload_content(
+            lock_token=None, content_id=tracim_context.current_content.content_id
+        ):
+            return True
+        raise ContentLocked()
+
+
 class ContentTypeCreationChecker(AuthorizationChecker):
     """
     Check if user can create content of this type
@@ -285,12 +305,14 @@ can_move_content = AndAuthorizationChecker(
     is_content_manager, CandidateWorkspaceRoleChecker(WorkspaceRoles.CONTENT_MANAGER.level)
 )
 can_create_content = ContentTypeCreationChecker(content_type_list)
+content_not_locked = ContentNotLocked()
 # comments
 is_comment_owner = CommentOwnerChecker()
 can_delete_comment = OrAuthorizationChecker(
     AndAuthorizationChecker(is_contributor, is_comment_owner), is_workspace_manager
 )

+
 ###
 # Authorization decorators for views

diff --git a/backend/tracim_backend/models/content_lock.py b/backend/tracim_backend/models/content_lock.py
new file mode 100644
index 000000000..bf0099327
--- /dev/null
+++ b/backend/tracim_backend/models/content_lock.py
@@ -0,0 +1,20 @@
+from datetime import datetime
+
+from sqlalchemy import Boolean
+from sqlalchemy import Column
+from sqlalchemy import DateTime
+from sqlalchemy import ForeignKey
+from sqlalchemy import Integer
+from sqlalchemy import Unicode
+
+from tracim_backend.models.meta import DeclarativeBase
+
+
+class UploadContentLock(DeclarativeBase):
+    __tablename__ = "upload_content_lock"
+
+    lock_token = Column(Unicode(256), primary_key=True, nullable=False)
+    user_id = Column(Integer, ForeignKey("users.user_id"), nullable=False, default=None)
+    content_id = Column(Integer, ForeignKey("content.id"), nullable=False, default=None)
+    created = Column(DateTime, default=datetime.utcnow)
+    enabled = Column(Boolean, default=True)
diff --git a/backend/tracim_backend/models/context_models.py b/backend/tracim_backend/models/context_models.py
index c03f3371c..e6865eb03 100644
--- a/backend/tracim_backend/models/context_models.py
+++ b/backend/tracim_backend/models/context_models.py
@@ -218,6 +218,11 @@ class WorkspaceAndContentPath(object):
         self.workspace_id = workspace_id

+class LockTokenBody(object):
+    def __init__(self, lock_token: str) -> None:
+        self.lock_token = lock_token
+
+
 class WorkspaceAndContentRevisionPath(object):
     """
     Paths params with workspace id and content_id model
diff --git a/backend/tracim_backend/models/setup_models.py b/backend/tracim_backend/models/setup_models.py
index 50e7b64df..2efca0d1f 100644
--- a/backend/tracim_backend/models/setup_models.py
+++ b/backend/tracim_backend/models/setup_models.py
@@ -18,6 +18,7 @@ from tracim_backend.lib.utils.utils import sliced_dict
 from tracim_backend.models.auth import Group  # noqa: F401
 from tracim_backend.models.auth import Permission  # noqa: F401
 from tracim_backend.models.auth import User  # noqa: F401
+from tracim_backend.models.content_lock import UploadContentLock  # noqa: F401
 from tracim_backend.models.data import Content  # noqa: F401
 from tracim_backend.models.data import ContentRevisionRO  # noqa: F401
 from tracim_backend.models.meta import DeclarativeBase  # noqa: F401
diff --git a/backend/tracim_backend/views/contents_api/file_controller.py b/backend/tracim_backend/views/contents_api/file_controller.py
index 17b7975f5..313428353 100644
--- a/backend/tracim_backend/views/contents_api/file_controller.py
+++ b/backend/tracim_backend/views/contents_api/file_controller.py
@@ -10,6 +10,7 @@ from tracim_backend.app_models.contents import FILE_TYPE
 from tracim_backend.app_models.contents import content_type_list
 from tracim_backend.config import CFG
 from tracim_backend.exceptions import ContentFilenameAlreadyUsedInFolder
+from tracim_backend.exceptions import ContentLocked
 from tracim_backend.exceptions import ContentNotFound
 from tracim_backend.exceptions import ContentStatusException
 from tracim_backend.exceptions import EmptyLabelNotAllowed
@@ -25,6 +26,7 @@ from tracim_backend.lib.core.content import ContentApi
 from tracim_backend.lib.utils.authorization import ContentTypeChecker
 from tracim_backend.lib.utils.authorization import ContentTypeCreationChecker
 from tracim_backend.lib.utils.authorization import check_right
+from tracim_backend.lib.utils.authorization import content_not_locked
 from tracim_backend.lib.utils.authorization import is_contributor
 from tracim_backend.lib.utils.authorization import is_reader
 from tracim_backend.lib.utils.request import TracimRequest
@@ -128,7 +130,9 @@ class FileController(Controller):
     @hapic.with_api_doc(tags=[SWAGGER_TAG__CONTENT_FILE_ENDPOINTS])
     @check_right(is_contributor)
     @check_right(is_file_content)
+    @check_right(content_not_locked)
     @hapic.handle_exception(ContentFilenameAlreadyUsedInFolder, HTTPStatus.BAD_REQUEST)
+    @hapic.handle_exception(ContentLocked, HTTPStatus.BAD_REQUEST)
     @hapic.input_path(FilePathSchema())
     @hapic.input_files(SimpleFileSchema())
     @hapic.output_body(NoContentSchema(), default_http_code=HTTPStatus.NO_CONTENT)
diff --git a/backend/tracim_backend/views/core_api/content_lock_controller.py b/backend/tracim_backend/views/core_api/content_lock_controller.py
new file mode 100644
index 000000000..cfa0780d2
--- /dev/null
+++ b/backend/tracim_backend/views/core_api/content_lock_controller.py
@@ -0,0 +1,73 @@
+from http import HTTPStatus
+
+from pyramid.config import Configurator
+
+from tracim_backend.config import CFG
+from tracim_backend.extensions import hapic
+from tracim_backend.lib.core.upload_content_lock import UploadContentLockLib
+from tracim_backend.lib.utils.authorization import check_right
+from tracim_backend.lib.utils.authorization import is_contributor
+from tracim_backend.lib.utils.request import TracimRequest
+from tracim_backend.lib.utils.utils import generate_documentation_swagger_tag
+from tracim_backend.views.controllers import Controller
+from tracim_backend.views.core_api.schemas import LockTokenBodySchema
+from tracim_backend.views.core_api.schemas import NoContentSchema
+from tracim_backend.views.core_api.schemas import WorkspaceAndContentIdPathSchema
+from tracim_backend.views.swagger_generic_section import SWAGGER_TAG__CONTENT_ENDPOINTS
+
+SWAGGER_TAG__CONTENT_LOCK_SECTION = "Lock"
+SWAGGER_TAG__CONTENT_LOCK_ENDPOINTS = generate_documentation_swagger_tag(
+    SWAGGER_TAG__CONTENT_ENDPOINTS, SWAGGER_TAG__CONTENT_LOCK_SECTION
+)
+
+
+class ContentLockController(Controller):
+    @hapic.with_api_doc(tags=[SWAGGER_TAG__CONTENT_LOCK_ENDPOINTS])
+    @check_right(is_contributor)
+    @hapic.input_path(WorkspaceAndContentIdPathSchema())
+    @hapic.output_body(LockTokenBodySchema())
+    def lock_content(self, context, request: TracimRequest, hapic_data=None):
+        """
+        Lock content
+        """
+        app_config = request.registry.settings["CFG"]  # type: CFG
+        locklib = UploadContentLockLib(
+            current_user=request.current_user, session=request.dbsession, config=app_config
+        )
+        lock = locklib.add_lock(content_id=hapic_data.path.content_id)
+        locklib.save(lock)
+        return lock
+
+    @hapic.with_api_doc(tags=[SWAGGER_TAG__CONTENT_LOCK_ENDPOINTS])
+    @check_right(is_contributor)
+    @hapic.input_path(WorkspaceAndContentIdPathSchema())
+    @hapic.input_body(LockTokenBodySchema())
+    @hapic.output_body(NoContentSchema(), default_http_code=HTTPStatus.NO_CONTENT)
+    def unlock_content(self, context, request: TracimRequest, hapic_data=None):
+        """
+        Unlock content
+        """
+        app_config = request.registry.settings["CFG"]  # type: CFG
+        locklib = UploadContentLockLib(
+            current_user=request.current_user, session=request.dbsession, config=app_config
+        )
+        lock = locklib.remove_lock(
+            lock_token=hapic_data.body.lock_token, content_id=hapic_data.path.content_id
+        )
+        locklib.save(lock)
+
+    def bind(self, configurator: Configurator):
+        # lock content
+        configurator.add_route(
+            "lock_content",
+            "/workspaces/{workspace_id}/contents/{content_id}/lock",
+            request_method="PUT",
+        )
+        configurator.add_view(self.lock_content, route_name="lock_content")
+        # unlock content
+        configurator.add_route(
+            "unlock_content",
+            "/workspaces/{workspace_id}/contents/{content_id}/unlock",
+            request_method="PUT",
+        )
+        configurator.add_view(self.unlock_content, route_name="unlock_content")
diff --git a/backend/tracim_backend/views/core_api/schemas.py b/backend/tracim_backend/views/core_api/schemas.py
index c222424a9..5933647ac 100644
--- a/backend/tracim_backend/views/core_api/schemas.py
+++ b/backend/tracim_backend/views/core_api/schemas.py
@@ -42,6 +42,7 @@ from tracim_backend.models.context_models import FileQuery
 from tracim_backend.models.context_models import FileRevisionPath
 from tracim_backend.models.context_models import FolderContentUpdate
 from tracim_backend.models.context_models import KnownMemberQuery
+from tracim_backend.models.context_models import LockTokenBody
 from tracim_backend.models.context_models import LoginCredentials
 from tracim_backend.models.context_models import MoveParams
 from tracim_backend.models.context_models import PageQuery
@@ -366,6 +367,14 @@ class WorkspaceAndContentIdPathSchema(WorkspaceIdPathSchema, ContentIdPathSchema
         return WorkspaceAndContentPath(**data)

+class LockTokenBodySchema(marshmallow.Schema):
+    lock_token = marshmallow.fields.String()
+
+    @post_load
+    def make_body_object(self, data: typing.Dict[str, typing.Any]) -> object:
+        return LockTokenBody(**data)
+
+
 class FilenamePathSchema(marshmallow.Schema):
     filename = StrippedString("filename.ext")

[inkhey]

according to https://wopi.readthedocs.io/projects/wopirest/en/latest/concepts.html#term-lock: some change are needed if we want our lock mecanism https://github.com/tracim/tracim/tree/fix/2073__lock_mecanism_for_new_revisions to be somehow compatible with WOPI lock :

• lock should be valid 30min.
• lock should NOT be associated to one user.