Merge branch 'develop' of github.com:tracim/tracim_backend into fix/simple_webdav_fix

tracim/__init__.py

@@ -29,6 +29,7 @@ from tracim.views.core_api.workspace_controller import WorkspaceController
 from tracim.views.contents_api.comment_controller import CommentController
 from tracim.views.errors import ErrorSchema
 from tracim.exceptions import NotAuthenticated
+from tracim.exceptions import InvalidId
 from tracim.exceptions import InsufficientUserProfile
 from tracim.exceptions import InsufficientUserRoleInWorkspace
 from tracim.exceptions import WorkspaceNotFoundInTracimRequest
@@ -90,6 +91,7 @@ def web(global_config, **local_settings):
     context.handle_exception(UserDoesNotExist, HTTPStatus.BAD_REQUEST)
     context.handle_exception(ContentNotFound, HTTPStatus.BAD_REQUEST)
     context.handle_exception(ContentTypeNotAllowed, HTTPStatus.BAD_REQUEST)
+    context.handle_exception(InvalidId, HTTPStatus.BAD_REQUEST)
     # Auth exception
     context.handle_exception(NotAuthenticated, HTTPStatus.UNAUTHORIZED)
     context.handle_exception(AuthenticationFailed, HTTPStatus.FORBIDDEN)

tracim/exceptions.py

@@ -121,6 +121,25 @@ class ContentNotFoundInTracimRequest(TracimException):
     pass
 
 
+class InvalidId(TracimException):
+    pass
+
+
+class InvalidContentId(InvalidId):
+    pass
+
+
+class InvalidCommentId(InvalidId):
+    pass
+
+
+class InvalidWorkspaceId(InvalidId):
+    pass
+
+
+class InvalidUserId(InvalidId):
+    pass
+
 class ContentNotFound(TracimException):
     pass
 
@@ -141,5 +160,5 @@ class EmptyLabelNotAllowed(EmptyValueNotAllowed):
     pass
 
 
-class EmptyRawContentNotAllowed(EmptyValueNotAllowed):
+class EmptyCommentContentNotAllowed(EmptyValueNotAllowed):
     pass

tracim/lib/core/content.py

@@ -24,7 +24,8 @@ from sqlalchemy.sql.elements import and_
 
 from tracim.lib.utils.utils import cmp_to_key
 from tracim.lib.core.notifications import NotifierFactory
-from tracim.exceptions import SameValueError, EmptyRawContentNotAllowed
+from tracim.exceptions import SameValueError
+from tracim.exceptions import EmptyCommentContentNotAllowed
 from tracim.exceptions import EmptyLabelNotAllowed
 from tracim.exceptions import ContentNotFound
 from tracim.exceptions import WorkspacesDoNotMatch
@@ -394,20 +395,28 @@ class ContentApi(object):
         return result
 
     def create(self, content_type: str, workspace: Workspace, parent: Content=None, label: str ='', filename: str = '', do_save=False, is_temporary: bool=False, do_notify=True) -> Content:
+        # TODO - G.M - 2018-07-16 - raise Exception instead of assert
         assert content_type in ContentType.allowed_types()
+        assert not (label and filename)
 
         if content_type == ContentType.Folder and not label:
             label = self.generate_folder_label(workspace, parent)
 
         content = Content()
-        if label:
-            content.label = label
-        elif filename:
-            # TODO - G.M - 2018-07-04 - File_name setting automatically
+
+        if filename:
+            # INFO - G.M - 2018-07-04 - File_name setting automatically
             # set label and file_extension
             content.file_name = label
+        elif label:
+            content.label = label
         else:
-            raise EmptyLabelNotAllowed()
+            if content_type == ContentType.Comment:
+                # INFO - G.M - 2018-07-16 - Default label for comments is
+                # empty string.
+                content.label = ''
+            else:
+                raise EmptyLabelNotAllowed('Content of this type should have a valid label')  # nopep8
 
         content.owner = self._user
         content.parent = parent
@@ -430,11 +439,11 @@ class ContentApi(object):
     def create_comment(self, workspace: Workspace=None, parent: Content=None, content:str ='', do_save=False) -> Content:
         assert parent and parent.type != ContentType.Folder
         if not content:
-            raise EmptyRawContentNotAllowed()
+            raise EmptyCommentContentNotAllowed()
         item = Content()
         item.owner = self._user
         item.parent = parent
-        if parent and not workspace:
+        if not workspace:
             workspace = item.parent.workspace
         item.workspace = workspace
         item.type = ContentType.Comment
@@ -446,7 +455,6 @@ class ContentApi(object):
             self.save(item, ActionDescription.COMMENT)
         return item
 
-
     def get_one_from_revision(self, content_id: int, content_type: str, workspace: Workspace=None, revision_id=None) -> Content:
         """
         This method is a hack to convert a node revision item into a node
@@ -483,6 +491,11 @@ class ContentApi(object):
         try:
             content = base_request.one()
         except NoResultFound as exc:
+            # TODO - G.M - 2018-07-16 - Add better support for all different
+            # error case who can happened here
+            # like content doesn't exist, wrong parent, wrong content_type, wrong workspace,
+            # wrong access to this workspace, wrong base filter according
+            # to content_status.
             raise ContentNotFound('Content "{}" not found in database'.format(content_id)) from exc  # nopep8
         return content
 

tracim/lib/utils/request.py

@@ -2,7 +2,12 @@
 from pyramid.request import Request
 from sqlalchemy.orm.exc import NoResultFound
 
-from tracim.exceptions import NotAuthenticated, ContentNotFound
+from tracim.exceptions import NotAuthenticated
+from tracim.exceptions import ContentNotFound
+from tracim.exceptions import InvalidUserId
+from tracim.exceptions import InvalidWorkspaceId
+from tracim.exceptions import InvalidContentId
+from tracim.exceptions import InvalidCommentId
 from tracim.exceptions import ContentNotFoundInTracimRequest
 from tracim.exceptions import WorkspaceNotFoundInTracimRequest
 from tracim.exceptions import UserNotFoundInTracimRequest
@@ -214,8 +219,9 @@ class TracimRequest(Request):
         comment_id = ''
         try:
             if 'comment_id' in request.matchdict:
-                if not request.matchdict['comment_id'].isdecimal():
-                    raise ContentNotFoundInTracimRequest('comment_id is not a correct integer')  # nopep8
+                comment_id_str = request.matchdict['content_id']
+                if not isinstance(comment_id_str, str) or not comment_id_str.isdecimal():  # nopep8
+                    raise InvalidCommentId('comment_id is not a correct integer')  # nopep8
                 comment_id = int(request.matchdict['comment_id'])
             if not comment_id:
                 raise ContentNotFoundInTracimRequest('No comment_id property found in request')  # nopep8
@@ -253,8 +259,9 @@ class TracimRequest(Request):
         content_id = ''
         try:
             if 'content_id' in request.matchdict:
-                if not request.matchdict['content_id'].isdecimal():
-                    raise ContentNotFoundInTracimRequest('content_id is not a correct integer')  # nopep8
+                content_id_str = request.matchdict['content_id']
+                if not isinstance(content_id_str, str) or not content_id_str.isdecimal():  # nopep8
+                    raise InvalidContentId('content_id is not a correct integer')  # nopep8
                 content_id = int(request.matchdict['content_id'])
             if not content_id:
                 raise ContentNotFoundInTracimRequest('No content_id property found in request')  # nopep8
@@ -286,8 +293,9 @@ class TracimRequest(Request):
         try:
             login = None
             if 'user_id' in request.matchdict:
-                if not request.matchdict['user_id'].isdecimal():
-                    raise UserNotFoundInTracimRequest('user_id is not a correct integer')  # nopep8
+                user_id_str = request.matchdict['user_id']
+                if not isinstance(user_id_str, str) or not user_id_str.isdecimal():
+                    raise InvalidUserId('user_id is not a correct integer')  # nopep8
                 login = int(request.matchdict['user_id'])
             if not login:
                 raise UserNotFoundInTracimRequest('You request a candidate user but the context not permit to found one')  # nopep8
@@ -331,8 +339,9 @@ class TracimRequest(Request):
         workspace_id = ''
         try:
             if 'workspace_id' in request.matchdict:
-                if not request.matchdict['workspace_id'].isdecimal():
-                    raise WorkspaceNotFoundInTracimRequest('workspace_id is not a correct integer')  # nopep8
+                workspace_id_str = request.matchdict['workspace_id']
+                if not isinstance(workspace_id_str, str) or not workspace_id_str.isdecimal():  # nopep8
+                    raise InvalidWorkspaceId('workspace_id is not a correct integer')  # nopep8
                 workspace_id = int(request.matchdict['workspace_id'])
             if not workspace_id:
                 raise WorkspaceNotFoundInTracimRequest('No workspace_id property found in request')  # nopep8
@@ -368,7 +377,7 @@ class TracimRequest(Request):
                     if workspace_id.isdecimal():
                         workspace_id = int(workspace_id)
                     else:
-                        raise WorkspaceNotFoundInTracimRequest('workspace_id is not a correct integer')  # nopep8
+                        raise InvalidWorkspaceId('workspace_id is not a correct integer')  # nopep8
             if not workspace_id:
                 raise WorkspaceNotFoundInTracimRequest('No new_workspace_id property found in body')  # nopep8
             wapi = WorkspaceApi(

tracim/models/context_models.py

@@ -338,10 +338,6 @@ class ContentInContext(object):
         return self.content.content_id
 
     @property
-    def id(self) -> int:
-        return self.content_id
-
-    @property
     def parent_id(self) -> int:
         """
         Return parent_id of the content
@@ -445,10 +441,6 @@ class RevisionInContext(object):
         return self.revision.content_id
 
     @property
-    def id(self) -> int:
-        return self.content_id
-
-    @property
     def parent_id(self) -> int:
         """
         Return parent_id of the content

tracim/tests/functional/test_comments.py

@@ -113,6 +113,7 @@ class TestCommentsEndpoint(FunctionalTest):
             params=params,
             status=400
         )
+
     def test_api__delete_content_comment__ok_200__user_is_owner_and_workspace_manager(self) -> None:  # nopep8
         """
         delete comment (user is workspace_manager and owner)

tracim/tests/library/test_content_api.py

@@ -506,7 +506,7 @@ class TestContentApi(DefaultTest):
             session=self.session,
             config=self.app_config,
         )
-        c = api.create(ContentType.Folder, workspace, None, 'parent', True)
+        c = api.create(ContentType.Folder, workspace, None, 'parent', '', True)
         with new_revision(
             session=self.session,
             tm=transaction.manager,
@@ -546,7 +546,7 @@ class TestContentApi(DefaultTest):
             session=self.session,
             config=self.app_config,
         )
-        c = api.create(ContentType.Folder, workspace, None, 'parent', True)
+        c = api.create(ContentType.Folder, workspace, None, 'parent', '', True)
         with new_revision(
             session=self.session,
             tm=transaction.manager,
@@ -656,6 +656,7 @@ class TestContentApi(DefaultTest):
             workspace,
             None,
             'folder a',
+            '',
             True
         )
         with self.session.no_autoflush:
@@ -692,6 +693,7 @@ class TestContentApi(DefaultTest):
             workspace2,
             None,
             'folder b',
+            '',
             True
         )
 
@@ -775,6 +777,7 @@ class TestContentApi(DefaultTest):
             workspace,
             None,
             'folder a',
+            '',
             True
         )
         with self.session.no_autoflush:
@@ -811,6 +814,7 @@ class TestContentApi(DefaultTest):
             workspace2,
             None,
             'folder b',
+            '',
             True
         )
         api2.copy(
@@ -891,6 +895,7 @@ class TestContentApi(DefaultTest):
             workspace,
             None,
             'folder a',
+            '',
             True
         )
         with self.session.no_autoflush:
@@ -2008,9 +2013,9 @@ class TestContentApi(DefaultTest):
             config=self.app_config,
         )
         a = api.create(ContentType.Folder, workspace, None,
-                       'this is randomized folder', True)
+                       'this is randomized folder', '', True)
         p = api.create(ContentType.Page, workspace, a,
-                       'this is randomized label content', True)
+                       'this is randomized label content', '', True)
 
         with new_revision(
             session=self.session,
@@ -2064,9 +2069,9 @@ class TestContentApi(DefaultTest):
             config=self.app_config,
         )
         a = api.create(ContentType.Folder, workspace, None,
-                       'this is randomized folder', True)
+                       'this is randomized folder', '', True)
         p = api.create(ContentType.Page, workspace, a,
-                       'this is dummy label content', True)
+                       'this is dummy label content', '', True)
 
         with new_revision(
             tm=transaction.manager,

tracim/views/contents_api/comment_controller.py

@@ -19,10 +19,7 @@ from tracim.views.core_api.schemas import CommentsPathSchema
 from tracim.views.core_api.schemas import SetCommentSchema
 from tracim.views.core_api.schemas import WorkspaceAndContentIdPathSchema
 from tracim.views.core_api.schemas import NoContentSchema
-from tracim.exceptions import WorkspaceNotFound, EmptyRawContentNotAllowed
-from tracim.exceptions import InsufficientUserRoleInWorkspace
-from tracim.exceptions import NotAuthenticated
-from tracim.exceptions import AuthenticationFailed
+from tracim.exceptions import EmptyCommentContentNotAllowed
 from tracim.models.contents import ContentTypeLegacy as ContentType
 from tracim.models.revision_protection import new_revision
 from tracim.models.data import UserRoleInWorkspace
@@ -59,7 +56,7 @@ class CommentController(Controller):
         ]
 
     @hapic.with_api_doc(tags=[COMMENT_ENDPOINTS_TAG])
-    @hapic.handle_exception(EmptyRawContentNotAllowed, HTTPStatus.BAD_REQUEST)
+    @hapic.handle_exception(EmptyCommentContentNotAllowed, HTTPStatus.BAD_REQUEST)  # nopep8
     @require_workspace_role(UserRoleInWorkspace.CONTRIBUTOR)
     @hapic.input_path(WorkspaceAndContentIdPathSchema())
     @hapic.input_body(SetCommentSchema())

tracim/views/core_api/schemas.py

@@ -2,6 +2,7 @@
 import marshmallow
 from marshmallow import post_load
 from marshmallow.validate import OneOf
+from marshmallow.validate import Range
 
 from tracim.lib.utils.utils import DATETIME_FORMAT
 from tracim.models.auth import Profile
@@ -79,15 +80,30 @@ class UserSchema(UserDigestSchema):
 
 
 class UserIdPathSchema(marshmallow.Schema):
-    user_id = marshmallow.fields.Int(example=3, required=True)
+    user_id = marshmallow.fields.Int(
+        example=3,
+        required=True,
+        description='id of a valid user',
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
 
 
 class WorkspaceIdPathSchema(marshmallow.Schema):
-    workspace_id = marshmallow.fields.Int(example=4, required=True)
+    workspace_id = marshmallow.fields.Int(
+        example=4,
+        required=True,
+        description='id of a valid workspace',
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
 
 
 class ContentIdPathSchema(marshmallow.Schema):
-    content_id = marshmallow.fields.Int(example=6, required=True)
+    content_id = marshmallow.fields.Int(
+        example=6,
+        required=True,
+        description='id of a valid content',
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
 
 
 class WorkspaceAndContentIdPathSchema(
@@ -102,8 +118,9 @@ class WorkspaceAndContentIdPathSchema(
 class CommentsPathSchema(WorkspaceAndContentIdPathSchema):
     comment_id = marshmallow.fields.Int(
         example=6,
-        description='id of a comment related to content content_id',
-        required=True
+        description='id of a valid comment related to content content_id',
+        required=True,
+        validate=Range(min=1, error="Value must be greater than 0"),
     )
     @post_load
     def make_path_object(self, data):
@@ -118,19 +135,22 @@ class FilterContentQuerySchema(marshmallow.Schema):
                     ' If not set, then return all contents.'
                     ' If set to 0, then return root contents.'
                     ' If set to another value, return all contents'
-                    ' directly included in the folder parent_id'
+                    ' directly included in the folder parent_id',
+        validate=Range(min=0, error="Value must be positive or 0"),
     )
     show_archived = marshmallow.fields.Int(
         example=0,
         default=0,
         description='if set to 1, then show archived contents.'
-                    ' Default is 0 - hide archived content'
+                    ' Default is 0 - hide archived content',
+        validate=Range(min=0, max=1, error="Value must be 0 or 1"),
     )
     show_deleted = marshmallow.fields.Int(
         example=0,
         default=0,
         description='if set to 1, then show deleted contents.'
-                    ' Default is 0 - hide deleted content'
+                    ' Default is 0 - hide deleted content',
+        validate=Range(min=0, max=1, error="Value must be 0 or 1"),
     )
     show_active = marshmallow.fields.Int(
         example=1,
@@ -140,7 +160,8 @@ class FilterContentQuerySchema(marshmallow.Schema):
                     ' Note: active content are content '
                     'that is neither archived nor deleted. '
                     'The reason for this parameter to exist is for example '
-                    'to allow to show only archived documents'
+                    'to allow to show only archived documents',
+        validate=Range(min=0, max=1, error="Value must be 0 or 1"),
     )
 
     @post_load
@@ -204,7 +225,10 @@ class WorkspaceMenuEntrySchema(marshmallow.Schema):
 
 
 class WorkspaceDigestSchema(marshmallow.Schema):
-    workspace_id = marshmallow.fields.Int(example=4)
+    workspace_id = marshmallow.fields.Int(
+        example=4,
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
     slug = marshmallow.fields.String(example='intranet')
     label = marshmallow.fields.String(example='Intranet')
     sidebar_entries = marshmallow.fields.Nested(
@@ -228,8 +252,14 @@ class WorkspaceMemberSchema(marshmallow.Schema):
         example='contributor',
         validate=OneOf(UserRoleInWorkspace.get_all_role_slug())
     )
-    user_id = marshmallow.fields.Int(example=3)
-    workspace_id = marshmallow.fields.Int(example=4)
+    user_id = marshmallow.fields.Int(
+        example=3,
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
+    workspace_id = marshmallow.fields.Int(
+        example=4,
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
     user = marshmallow.fields.Nested(
         UserSchema(only=('public_name', 'avatar_url'))
     )
@@ -318,11 +348,13 @@ class ContentMoveSchema(marshmallow.Schema):
         description='id of the new parent content id.',
         allow_none=True,
         required=True,
+        validate=Range(min=0, error="Value must be positive or 0"),
     )
     new_workspace_id = marshmallow.fields.Int(
         example=2,
         description='id of the new workspace id.',
-        required=True
+        required=True,
+        validate=Range(min=1, error="Value must be greater than 0"),
     )
 
     @post_load
@@ -346,15 +378,20 @@ class ContentCreationSchema(marshmallow.Schema):
 
 
 class ContentDigestSchema(marshmallow.Schema):
-    content_id = marshmallow.fields.Int(example=6)
+    content_id = marshmallow.fields.Int(
+        example=6,
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
     slug = marshmallow.fields.Str(example='intervention-report-12')
     parent_id = marshmallow.fields.Int(
         example=34,
         allow_none=True,
-        default=None
+        default=None,
+        validate=Range(min=0, error="Value must be positive or 0"),
     )
     workspace_id = marshmallow.fields.Int(
         example=19,
+        validate=Range(min=1, error="Value must be greater than 0"),
     )
     label = marshmallow.fields.Str(example='Intervention Report 12')
     content_type = marshmallow.fields.Str(
@@ -421,8 +458,16 @@ class TextBasedContentSchema(ContentSchema, TextBasedDataAbstractSchema):
 
 
 class RevisionSchema(ContentDigestSchema):
-    comment_ids = marshmallow.fields.List(marshmallow.fields.Int(example=4))
-    revision_id = marshmallow.fields.Int(example=12)
+    comment_ids = marshmallow.fields.List(
+        marshmallow.fields.Int(
+            example=4,
+            validate=Range(min=1, error="Value must be greater than 0"),
+        )
+    )
+    revision_id = marshmallow.fields.Int(
+        example=12,
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
     created = marshmallow.fields.DateTime(
         format=DATETIME_FORMAT,
         description='Content creation date',
@@ -435,8 +480,14 @@ class TextBasedRevisionSchema(RevisionSchema, TextBasedDataAbstractSchema):
 
 
 class CommentSchema(marshmallow.Schema):
-    content_id = marshmallow.fields.Int(example=6)
-    parent_id = marshmallow.fields.Int(example=34)
+    content_id = marshmallow.fields.Int(
+        example=6,
+        validate=Range(min=1, error="Value must be greater than 0"),
+    )
+    parent_id = marshmallow.fields.Int(
+        example=34,
+        validate=Range(min=0, error="Value must be positive or 0"),
+    )
     raw_content = marshmallow.fields.String(
         example='<p>This is just an html comment !</p>'
     )