add documentation for new modules

api/dao/storage.py

@@ -9,20 +9,37 @@ log = logging.getLogger('scitran.api')
 
 
 class ListStorage(object):
+    """
+    This class provides access to sublists of a mongodb collections elements (called containers).
+    """
 
     def __init__(self, coll_name, list_name, use_oid = False):
         self.coll_name = coll_name
         self.list_name = list_name
         self.use_oid = use_oid
-        self.container = None
+        # the collection is not loaded when the class is instantiated
+        # this allows to instantiate the class when the db is not available
+        # dbc is initialized using the load_collection method
         self.dbc = None
 
 
     def load_collection(self, db):
-        self.dbc = db.get_collection(self.coll_name)
+        """
+        Initialize the mongodb collection.
+        """
+        if self.dbc is None:
+            self.dbc = db.get_collection(self.coll_name)
         return self.dbc
 
     def get_container(self, _id):
+        """
+        Load a container from the _id.
+
+        This method is usually used to to check permission properties of the container.
+        e.g. list of users that can access the container
+
+        For simplicity we load its full content.
+        """
         if self.dbc is None:
             raise RuntimeError('collection not initialized before calling get_container')
         if self.use_oid:
@@ -33,6 +50,10 @@ class ListStorage(object):
         return self.container
 
     def apply_change(self, action, _id, elem_match=None, payload=None):
+        """
+        Generic method to apply an operation.
+        The request is dispatched to the corresponding private methods.
+        """
         if self.use_oid:
             _id = bson.objectid.ObjectId(_id)
         if action == 'GET':
@@ -43,7 +64,7 @@ class ListStorage(object):
             return self._update_el(_id, elem_match, payload)
         if action == 'POST':
             return self._create_el(_id, payload)
-        raise ValueError('action should be one of POST, PUT, DELETE')
+        raise ValueError('action should be one of GET, POST, PUT, DELETE')
 
     def _create_el(self, _id, payload):
         log.debug('payload {}'.format(payload))
@@ -117,6 +138,9 @@ class StringListStorage(ListStorage):
         self.key_name = key_name
 
     def apply_change(self, action, _id, elem_match=None, payload=None):
+        """
+        This method "flattens" the query parameter and the payload to handle string lists
+        """
         if elem_match is not None:
             elem_match = elem_match[self.key_name]
         if payload is not None:

api/files.py

@@ -14,6 +14,16 @@ log = logging.getLogger('scitran.api')
 
 
 class FileRequest(object):
+    """This class provides and interface for file uploads.
+    To perform an upload the client of the class should follow these steps:
+
+    1) initialize the request
+    2) save a temporary file
+    3) check identical
+    4) move the temporary file to its destination
+
+    The operations could be safely interleaved with other actions like permission checks or database updates.
+    """
 
     def __init__(self, client_addr, filename, body, received_md5, metadata, tags, flavor):
         self.client_addr = client_addr
@@ -84,6 +94,9 @@ class FileRequest(object):
 
     @classmethod
     def from_handler(cls, handler, filename=None):
+        """
+        Convenient method to initialize an upload request from the FileListHandler receiving it.
+        """
         tags = []
         metadata = {}
         if handler.request.content_type == 'multipart/form-data':

api/listhandler.py

@@ -15,6 +15,17 @@ log = logging.getLogger('scitran.api')
 
 
 class ListHandler(base.RequestHandler):
+    """
+    This class handle operations on a generic sublist of a container like tags, group roles, user permissions, etc.
+
+    The pattern used is:
+    1) initialize request
+    2) exec request
+    3) check and return result
+
+    Specific behaviors (permissions checking logic for authenticated and not superuser users, storage interaction)
+    are specified in the routes defined in api.py
+    """
 
     def __init__(self, request=None, response=None):
         super(ListHandler, self).__init__(request, response)
@@ -23,12 +34,11 @@ class ListHandler(base.RequestHandler):
     def get(self, *args, **kwargs):
         container, perm_checker, storage = self._initialize_request(kwargs)
         _id = container["_id"]
-        list_name = storage.list_name
 
         result = perm_checker(storage.apply_change)('GET', _id, elem_match=kwargs)
 
         if result is None or result.get(list_name) is None or len(result[list_name]) == 0:
-            self.abort(404, 'Element not found in list {} of collection {} {}'.format(list_name, storage.coll_name, _id))
+            self.abort(404, 'Element not found in list {} of collection {} {}'.format(storage.list_name, storage.coll_name, _id))
         return result[list_name][0]
 
     def post(self, *args, **kwargs):
@@ -66,6 +76,12 @@ class ListHandler(base.RequestHandler):
             self.abort(404, 'Element not removed from list {} in collection {} {}'.format(storage.list_name, storage.coll_name, _id))
 
     def _initialize_request(self, kwargs):
+        """
+        This method loads:
+        1) the container that will be modified
+        2) the storage class that will handle the database actions
+        3) the permission checker decorator that will be used
+        """
         if self._initialized:
             return self._initialized
         perm_checker = kwargs.pop('permchecker')
@@ -87,6 +103,9 @@ class ListHandler(base.RequestHandler):
         return self._initialized
 
 class FileListHandler(ListHandler):
+    """
+    This class implements a more specific logic for list of files as the api needs to interact with the filesystem.
+    """
 
     def __init__(self, request=None, response=None):
         super(FileListHandler, self).__init__(request, response)

api/permchecker.py

 # @author:  Renzo Frigato
+"""
+Purpose of this module is to define all the permissions checker decorators.
 
+This decorators are currently supported only by the ListHandler and FileListHandler classes.
+"""
 import logging
 
 from users import INTEGER_ROLES
@@ -14,12 +18,19 @@ def _get_access(uid, container):
     else:
         return -1
 
-
 def always_ok(apply_change):
+    """
+    This decorator leaves the original method unchanged.
+    It is used as permissions checker when the request is a superuser_request
+    """
     return apply_change
 
-
 def default_sublist(handler, container):
+    """
+    This is the default permissions checker generator.
+    The resulting permissions checker modifies the apply_change method by checking the user permissions
+    on the container before actually executing this method.
+    """
     access = _get_access(handler.uid, container)
     def g(apply_change):
         def f(method, _id, elem_match = None, payload = None):
@@ -39,13 +50,17 @@ def default_sublist(handler, container):
         return f
     return g
 
-
 def group_roles_sublist(handler, container):
+    """
+    This is the customized permissions checker for group roles operations.
+    """
     access = _get_access(handler.uid, container)
     def g(apply_change):
         def f(method, _id, elem_match = None, payload = None):
             if method == 'GET' and elem_match.get('_id') == handler.uid:
                 return apply_change(method, _id, elem_match, payload)
+            elif method == 'PUT' and elem_match.get('_id') == handler.uid:
+                handler.abort(403, 'user not authorized to modify its own permissions')
             elif access >= INTEGER_ROLES['admin']:
                 return apply_change(method, _id, elem_match, payload)
             else:
@@ -54,6 +69,9 @@ def group_roles_sublist(handler, container):
     return g
 
 def public_request(handler, container):
+    """
+    For public requests we allow only GET operations on containers marked as public.
+    """
     def g(apply_change):
         def f(method, _id, elem_match = None, payload = None):
             if method == 'GET' and container.get('public', False):
@@ -61,4 +79,4 @@ def public_request(handler, container):
             else:
                 handler.abort(403, 'not authorized to perform a {} operation on this container'.format(method))
         return f
-    return g
\ No newline at end of file
+    return g