Fix/get templates (#870) (#871)

- Fixes issue introduced with #866: template studies did not open. GET project/{project_uuid} was only returning template projects
- sets all as default in GET projects/
- Fixed fake_db fixture and doc

Cherry-picked squashed PR #870  to master

Author: pcrespov

api/specs/webserver/v0/openapi-projects.yaml

@@ -16,6 +16,7 @@ paths:
           in: query
           schema:
             type: string
+            default: 'all'
             enum: [template, user, all]
           description: if true only templates otherwise only users
         - name: start

services/web/server/src/simcore_service_webserver/projects/projects_api.py

@@ -40,14 +40,27 @@ def validate_project(app: web.Application, project: Dict):
     validate_instance(project, project_schema) # TODO: handl
 
 
-async def get_project_for_user(request: web.Request, project_uuid, user_id) -> Dict:
+async def get_project_for_user(request: web.Request, project_uuid, user_id, *, include_templates=False) -> Dict:
+    """ Returns a project accessible to user
+
+    :raises web.HTTPNotFound: if no match found
+    :return: schema-compliant project data
+    :rtype: Dict
+    """
     await check_permission(request, "project.read")
 
     try:
         db = request.config_dict[APP_PROJECT_DBAPI]
-        project = await db.get_user_project(user_id, project_uuid)
+
+        project = None
+        if include_templates:
+            project = await db.get_template_project(project_uuid)
+
+        if not project:
+            project = await db.get_user_project(user_id, project_uuid)
 
         # TODO: how to handle when database has an invalid project schema???
+        # Notice that db model does not include a check on project schema.
         validate_project(request.app, project)
         return project
 

services/web/server/src/simcore_service_webserver/projects/projects_db.py

@@ -2,10 +2,8 @@
 
     - Adds a layer to the postgres API with a focus on the projects data
     - Shall be used as entry point for all the queries to the database regarding projects
-"""
 
-#TODO: move here class ProjectDB:
-# TODO: should implement similar model as services/web/server/src/simcore_service_webserver/login/storage.py
+"""
 
 import logging
 import uuid as uuidlib
@@ -26,8 +24,8 @@
 from ..utils import format_datetime, now_str
 from .projects_exceptions import (ProjectInvalidRightsError,
                                   ProjectNotFoundError)
-from .projects_models import ProjectType, projects, user_to_projects
 from .projects_fakes import Fake
+from .projects_models import ProjectType, projects, user_to_projects
 
 log = logging.getLogger(__name__)
 
@@ -41,7 +39,6 @@ def _convert_to_db_names(project_data: Dict) -> Dict:
         converted_args[ChangeCase.camel_to_snake(key)] = value
     return converted_args
 
-
 def _convert_to_schema_names(project_db_data: Mapping) -> Dict:
     converted_args = {}
     for key, value in project_db_data.items():
@@ -54,7 +51,6 @@ def _convert_to_schema_names(project_db_data: Mapping) -> Dict:
     return converted_args
 
 
-
 # TODO: test all function return schema-compatible data
 # TODO: is user_id str or int?
 # TODO: systemaic user_id, project
@@ -190,6 +186,7 @@ async def load_user_projects(self, user_id: str) -> List[Dict]:
 
     async def load_template_projects(self) -> List[Dict]:
         log.info("Loading template projects")
+        # TODO: eliminate this and use mock to replace get_user_project instead
         projects_list = [prj.data for prj in Fake.projects.values() if prj.template]
 
         async with self.engine.acquire() as conn:
@@ -202,46 +199,54 @@ async def load_template_projects(self) -> List[Dict]:
                 projects_list.append(_convert_to_schema_names(result_dict))
         return projects_list
 
-    async def get_template_project(self, project_uuid: str) -> Dict:
-        prj = Fake.projects.get(project_uuid)
-        if prj and prj.template:
-            return prj.data
-
-        template_prj = None
-        async with self.engine.acquire() as conn:
-            query = select([projects]).where(
-                and_(projects.c.type == ProjectType.TEMPLATE, projects.c.uuid == project_uuid)
-            )
-            result = await conn.execute(query)
-            row = await result.first()
-            template_prj = _convert_to_schema_names(row) if row else None
-
-        return template_prj
-
     async def get_user_project(self, user_id: str, project_uuid: str) -> Dict:
-        """
+        """ Returns all projects *owned* by the user
+
+            - A project is owned with it is mapped in user_to_projects list
+            - prj_owner field is not
+            - Notice that a user can have access to a template but he might not onw it
 
         :raises ProjectNotFoundError: project is not assigned to user
         :return: schema-compliant project
         :rtype: Dict
         """
+        # TODO: eliminate this and use mock to replace get_user_project instead
         prj = Fake.projects.get(project_uuid)
         if prj and not prj.template:
             return Fake.projects[project_uuid].data
 
-        log.info("Getting project %s for user %s", project_uuid, user_id)
         async with self.engine.acquire() as conn:
             joint_table = user_to_projects.join(projects)
-            query = select([projects]).\
-                select_from(joint_table).\
-                    where(and_(projects.c.uuid == project_uuid, user_to_projects.c.user_id == user_id))
+            query = select([projects]).select_from(joint_table).where(
+                and_(projects.c.uuid == project_uuid,
+                     user_to_projects.c.user_id == user_id)
+            )
             result = await conn.execute(query)
             row = await result.first()
+
+            # FIXME: prefer None to raise an exception. Read https://stackoverflow.com/questions/1313812/raise-exception-vs-return-none-in-functions?answertab=votes#tab-top
             if not row:
                 raise ProjectNotFoundError(project_uuid)
-            result_dict = {key:value for key,value in row.items()}
-            log.debug("found project: %s", result_dict)
-            return _convert_to_schema_names(result_dict)
+            return _convert_to_schema_names(row)
+
+    async def get_template_project(self, project_uuid: str) -> Dict:
+        # TODO: eliminate this and use mock to replace get_user_project instead
+        prj = Fake.projects.get(project_uuid)
+        if prj and prj.template:
+            return prj.data
+
+        template_prj = None
+        async with self.engine.acquire() as conn:
+            query = select([projects]).where(
+                and_(projects.c.type == ProjectType.TEMPLATE,
+                     projects.c.uuid == project_uuid)
+            )
+            result = await conn.execute(query)
+            row = await result.first()
+            if row:
+                template_prj = _convert_to_schema_names(row)
+
+        return template_prj
 
     async def update_user_project(self, project_data: Dict, user_id: str, project_uuid: str):
         """ updates a project from a user
@@ -317,7 +322,6 @@ async def delete_user_project(self, user_id: int, project_uuid: str):
                         where(projects.c.id == row[projects.c.id])
                     await conn.execute(query)
 
-
     async def make_unique_project_uuid(self) -> str:
         """ Generates a project identifier still not used in database
 
@@ -340,7 +344,6 @@ async def make_unique_project_uuid(self) -> str:
                     break
         return project_uuid
 
-
     async def _get_user_email(self, user_id):
         async with self.engine.acquire() as conn:
             result = await conn.execute(

services/web/server/src/simcore_service_webserver/projects/projects_handlers.py

@@ -71,7 +71,7 @@ async def list_projects(request: web.Request):
     # TODO: implement all query parameters as
     # in https://www.ibm.com/support/knowledgecenter/en/SSCRJU_3.2.0/com.ibm.swg.im.infosphere.streams.rest.api.doc/doc/restapis-queryparms-list.html
     user_id = request[RQT_USERID_KEY]
-    ptype = request.query.get('type', 'user')
+    ptype = request.query.get('type', 'all') # TODO: get default for oaspecs
     db = request.config_dict[APP_PROJECT_DBAPI]
 
     projects_list = []
@@ -103,12 +103,18 @@ async def list_projects(request: web.Request):
 
 @login_required
 async def get_project(request: web.Request):
+    """ Returns all projects accessible to a user (not necesarly owned)
+
+    """
     # TODO: temporary hidden until get_handlers_from_namespace refactor to seek marked functions instead!
     from .projects_api import get_project_for_user
 
+    project_uuid = request.match_info.get("project_id")
+
     project = await get_project_for_user(request,
-        project_uuid=request.match_info.get("project_id"),
-        user_id=request[RQT_USERID_KEY]
+        project_uuid=project_uuid,
+        user_id=request[RQT_USERID_KEY],
+        include_templates=True
     )
 
     return {

services/web/server/src/simcore_service_webserver/studies_access.py

@@ -156,12 +156,12 @@ async def access_study(request: web.Request) -> web.Response:
 
     # FIXME: if identified user, then he can access not only to template but also his own projects!
     if study_id not in SHARABLE_TEMPLATE_STUDY_IDS:
-        raise web.HTTPNotFound(reason="Requested study is not shared ['%s']" % study_id)
+        raise web.HTTPNotFound(reason="This study was not shared [{}]".format(study_id))
 
     # TODO: should copy **any** type of project is sharable -> get_sharable_project
     template_project = await get_template_project(request.app, study_id)
     if not template_project:
-        raise RuntimeError("Unable to load study %s" % study_id)
+        raise web.HTTPNotFound(reason="Invalid study [{}]".format(study_id))
 
     user = None
     is_anonymous_user = await is_anonymous(request)
@@ -174,12 +174,12 @@ async def access_study(request: web.Request) -> web.Response:
     if not user:
         raise RuntimeError("Unable to start user session")
 
+    msg_tail = "study {} to {} account ...".format(template_project.get('name'), user.get("email"))
+    log.debug("Copying %s ...", msg_tail)
 
-    log.debug("Copying study %s to %s account ...", template_project['name'], user["email"])
     copied_project_id = await copy_study_to_account(request, template_project, user)
 
-    log.debug("Coped study %s to %s account as %s",
-        template_project['name'], user["email"], copied_project_id)
+    log.debug("Copied %s as %s", msg_tail, copied_project_id)
 
 
     try:

services/web/server/tests/data/fake-project.json

@@ -1,6 +1,6 @@
 {
   "uuid": "template-uuid-6257-a462-d7bf73b76c0c",
-  "name": "ex",
+  "name": "fake-project-name",
   "description": "anim sint pariatur do dolore",
   "prjOwner": "dolore ad do consectetur",
   "creationDate": "1865-11-30T04:00:14.000Z",

services/web/server/tests/unit/test_template_projects.py

@@ -33,11 +33,12 @@ async def project_specs(loop, project_schema_file: Path) -> Dict:
 def fake_db():
     Fake.reset()
     Fake.load_template_projects()
+    yield Fake
+    Fake.reset()
 
 async def test_validate_templates(loop, project_specs: Dict, fake_db):
-    for pid, project in Fake.projects.items():
+    for pid, project in fake_db.projects.items():
         try:
             validate_instance(project.data, project_specs)
         except ValidationError:
             pytest.fail("validation of project {} failed".format(pid))
-

services/web/server/tests/unit/with_postgres/test_access_to_studies.py

@@ -111,7 +111,7 @@ async def logged_user(client): #, role: UserRole):
 
 async def _get_user_projects(client):
     url = client.app.router["list_projects"].url_for()
-    resp = await client.get(url.with_query(start=0, count=3))
+    resp = await client.get(url.with_query(start=0, count=3, type="user"))
     payload = await resp.json()
     assert resp.status == 200, payload
 
@@ -157,7 +157,6 @@ async def test_access_study_anonymously(client, qx_client_outdir):
     }
 
     async with NewProject(params, client.app, force_uuid=True) as template_project:
-
         url_path = "/study/%s" % SHARED_STUDY_UUID
         resp = await client.get(url_path)
         content = await resp.text()

services/web/server/tests/unit/with_postgres/test_projects.py

@@ -63,7 +63,7 @@ def client(loop, aiohttp_client, aiohttp_unused_port, app_cfg, postgres_service)
 
     # teardown here ...
 
-@pytest.fixture
+@pytest.fixture()
 async def logged_user(client, user_role: UserRole):
     """ adds a user in db and logs in with client
 
@@ -74,7 +74,9 @@ async def logged_user(client, user_role: UserRole):
         {"role": user_role.name},
         check_if_succeeds = user_role!=UserRole.ANONYMOUS
     ) as user:
+        print("-----> logged in user", user_role)
         yield user
+        print("<----- logged out user", user_role)
 
 @pytest.fixture
 async def user_project(client, fake_project, logged_user):
@@ -83,17 +85,26 @@ async def user_project(client, fake_project, logged_user):
         client.app,
         user_id=logged_user["id"]
     ) as project:
+        print("-----> added project", project["name"])
         yield project
+        print("<----- removed project", project["name"])
 
 
 @pytest.fixture
 async def template_project(client, fake_project):
+    project_data = deepcopy(fake_project)
+    project_data["name"] = "Fake template"
+    project_data["uuid"] = "d4d0eca3-d210-4db6-84f9-63670b07176b"
+
     async with NewProject(
-        fake_project,
+        project_data,
         client.app,
-        user_id=None
-    ) as template_prj:
-        yield template_prj
+        user_id=None,
+        clear_all=True
+    ) as template_project:
+        print("-----> added template project", template_project["name"])
+        yield template_project
+        print("<----- removed template project", template_project["name"])
 
 def assert_replaced(current_project, update_data):
     def _extract(dikt, keys):
@@ -115,20 +126,35 @@ def _extract(dikt, keys):
     (UserRole.USER, web.HTTPOk),
     (UserRole.TESTER, web.HTTPOk),
 ])
-async def test_list_projects(client, logged_user, user_project, expected):
+async def test_list_projects(client, logged_user, user_project, template_project, expected):
     # GET /v0/projects
     url = client.app.router["list_projects"].url_for()
     assert str(url) == API_PREFIX + "/projects"
 
     resp = await client.get(url)
     data, errors = await assert_status(resp, expected)
 
-    #TODO: GET /v0/projects?type=user
+    if not errors:
+        assert len(data) == 2
+        assert data[0] == template_project
+        assert data[1] == user_project
 
+    #GET /v0/projects?type=user
+    resp = await client.get(url.with_query(type='user'))
+    data, errors = await assert_status(resp, expected)
     if not errors:
         assert len(data) == 1
         assert data[0] == user_project
 
+    #GET /v0/projects?type=template
+    resp = await client.get(url.with_query(type='template'))
+    data, errors = await assert_status(resp, expected)
+    if not errors:
+        assert len(data) == 1
+        assert data[0] == template_project
+
+
+
 @pytest.mark.skip("TODO")
 async def test_list_templates_only(client, logged_user, user_project, expected):
     #TODO: GET /v0/projects?type=template
@@ -141,8 +167,10 @@ async def test_list_templates_only(client, logged_user, user_project, expected):
     (UserRole.USER, web.HTTPOk),
     (UserRole.TESTER, web.HTTPOk),
 ])
-async def test_get_project(client, logged_user, user_project, expected):
+async def test_get_project(client, logged_user, user_project, template_project, expected):
     # GET /v0/projects/{project_id}
+
+    # with a project owned by user
     url = client.app.router["get_project"].url_for(project_id=user_project["uuid"])
 
     resp = await client.get(url)
@@ -151,6 +179,15 @@ async def test_get_project(client, logged_user, user_project, expected):
     if not error:
         assert data == user_project
 
+    # with a template
+    url = client.app.router["get_project"].url_for(project_id=template_project["uuid"])
+
+    resp = await client.get(url)
+    data, error = await assert_status(resp, expected)
+
+    if not error:
+        assert data == template_project
+
 
 # POST --------
 @pytest.mark.parametrize("user_role,expected", [