Validate and add REMS object to submission (merge commit)

Merge branch 'feature/rems-dac-policy' into 'main' * Change wording to internal_rems for metadata objects * Fix rems.http template logic slightly * Validate and add REMS object to submission Closes #532 and #649 See merge request https://gitlab.ci.csc.fi/sds-dev/sd-submit/metadata-submitter/-/merge_requests/932 Reviewed-by: Joonatan Mäkinen <[email protected]> Approved-by: Joonatan Mäkinen <[email protected]> Co-authored-by: lhang <[email protected]> Co-authored-by: Joonatan Mäkinen <[email protected]> Merged by Joonatan Mäkinen <[email protected]>
CSCfi · Dec 11, 2024 · 78a2a80 · 78a2a80
2 parents 62aaa82 + a189a9c
commit 78a2a80
Show file tree

Hide file tree

Showing 14 changed files with 223 additions and 58 deletions.
diff --git a/docs/openapi.yml b/docs/openapi.yml
@@ -2429,6 +2429,20 @@ components:
                     type: string
                     description: Type of submission
                     enum: ["XML", "CSV", "Form"]
+              rems:
+                type: object
+                description: Preserved REMS information for each dataset
+                additionalProperties: true
+                properties:
+                  url:
+                    type: string
+                    description: URL for application to apply access for dataset in REMS
+                  resourceId:
+                    type: string
+                    description: REMS id of the resource created. Created automatically during the publication process
+                  catalogueItemId:
+                    type: string
+                    description: REMS id of the catalogue created. Created automatically during the publication process
         drafts:
           type: array
           items:
@@ -3331,12 +3345,12 @@ components:
           description: ID of an organization in REMS
         workflowId:
           type: integer
-          description: ID of a Workflow, obtained from REMS
+          description: ID of a Workflow, obtained from REMS, and must belong to organizationId
         licenses:
           type: array
           items:
             type: integer
-            description: ID of a license, obtained from REMS, and must belong to organizationId
+            description: IDs of Licenses, obtained from REMS, and must belong to organizationId
     REMS:
       type: array
       description: Array of organizations with workflows and licenses from REMS database

diff --git a/metadata_backend/api/handlers/object.py b/metadata_backend/api/handlers/object.py
@@ -161,15 +161,13 @@ async def post_object(self, req: Request) -> Response:
 
         # ensure only one object with the same schema exists in the submission
         if is_single_instance:
-            schemas_in_submission = set()
             submission = {
                 "metadataObjects": await submission_op.get_submission_field_list(submission_id, "metadataObjects")
             }
             for _, schema in self.iter_submission_objects(submission):
-                if schema == schema_type and schema in schemas_in_submission:
+                if schema == schema_type:
                     reason = f"Submission of type {workflow.name} already has a '{schema}', and it can have only one."
                     raise web.HTTPBadRequest(reason=reason)
-                schemas_in_submission.add(schema)
 
         collection = f"draft-{schema_type}" if req.path.startswith(f"{API_PREFIX}/drafts") else schema_type
 

diff --git a/metadata_backend/api/handlers/publish.py b/metadata_backend/api/handlers/publish.py
@@ -419,8 +419,9 @@ async def _publish_rems(self, submission: dict[str, Any], obj_op: ObjectOperator
                             "infourl": self._make_discovery_url(object_data, submission["workflow"]),
                         }
                     },
-                    "metaxIdentifier": object_data.get("metaxIdentifier", ""),
                 }
+                if schema == "dataset":
+                    rems_ds.update({"metaxIdentifier": object_data.get("metaxIdentifier", "")})
                 rems_datasets.append(rems_ds)
 
         org_id = submission["rems"]["organizationId"]
@@ -440,16 +441,16 @@ async def _publish_rems(self, submission: dict[str, Any], obj_op: ObjectOperator
             # Add rems URL to metax dataset description
             rems_url = self.rems_handler.application_url(catalogue_id)
             if "metaxIdentifier" in ds:
-                new_description = ds["description"] + f"\n\nREMS DAC: {rems_url}"
+                new_description = ds["description"] + f"\n\nSD Apply's Application link: {rems_url}"
                 await self.metax_handler.update_draft_dataset_description(ds["metaxIdentifier"], new_description)
+
+            # Update metadata object with preserved rems info that might need later
             await obj_op.update_metadata_object(
                 ds["schema"],
                 ds["accession_id"],
                 {
-                    "rems": {
+                    "internal_rems": {
                         "url": rems_url,
-                        "workflowId": workflow_id,
-                        "organizationId": org_id,
                         "resourceId": resource_id,
                         "catalogueId": catalogue_id,
                     },
@@ -482,6 +483,9 @@ async def publish_submission(self, req: Request) -> Response:
         if "datacite" in workflow.required_schemas and "doiInfo" not in submission:
             raise web.HTTPBadRequest(reason=f"Submission '{submission_id}' must have the required DOI metadata.")
 
+        if "dac" in workflow.required_schemas and "rems" not in submission:
+            raise web.HTTPBadRequest(reason=f"Submission '{submission_id}' must have rems information.")
+
         schemas_in_submission = set()
         has_study = False
         for _, schema in self.iter_submission_objects(submission):
@@ -494,9 +498,7 @@ async def publish_submission(self, req: Request) -> Response:
                 reason = f"Submission of type {workflow.name} has more than one '{schema}', and it can have only one"
                 raise web.HTTPBadRequest(reason=reason)
             schemas_in_submission.add(schema)
-            # TO_DO: Can only be enabled after we have unified the DAC from EGA and REMS
-            # if "dac" in workflow.required_schemas and "rems" not in submission:
-            #     raise web.HTTPBadRequest(reason=f"Submission '{accession_id}' must have rems.")
+
         if workflow.name != "BigPicture" and not has_study:
             raise web.HTTPBadRequest(reason=f"Submission '{submission_id}' must have a study.")
 
@@ -513,7 +515,7 @@ async def publish_submission(self, req: Request) -> Response:
                     )
                     raise web.HTTPBadRequest(reason=reason)
 
-        # TO_DO: Can only be enabled after we have unified the DAC from EGA and REMS
+        # TO_DO: Need to include all required objects before publishing submission in unit and integration tests
         # missing_schemas = set()
         # for required_schema in workflow.required_schemas:
         #     if required_schema not in schemas_in_submission:
@@ -523,6 +525,7 @@ async def publish_submission(self, req: Request) -> Response:
         #     raise web.HTTPBadRequest(
         #         reason=f"{workflow.name} submission '{submission_id}' is missing '{required}' schema(s)."
         #     )
+
         # Create draft DOI and Metax records if not existing
         external_user_id = await self.get_user_external_id(req)
         async for accession_id, schema, object_data in self.iter_submission_objects_data(submission, obj_op):

diff --git a/metadata_backend/api/handlers/rems_proxy.py b/metadata_backend/api/handlers/rems_proxy.py
@@ -48,7 +48,7 @@ def _get_localized(language: str, _dict: dict[str, Any], fallback_language: str
 
     async def get_workflows_licenses_from_rems(self, request: web.Request) -> web.Response:
         """Get workflows and Policies for frontend."""
-        language = request.query.get("language", "en").split("_")[0]
+        language = request.query.get("language", "en")
         workflows = await self.rems_handler.get_workflows()
         licenses = await self.rems_handler.get_licenses()
 
@@ -68,7 +68,9 @@ def add_organization(organization_id: str, organization_name: str) -> None:
             org_id = workflow["organization"]["organization/id"]
             if org_id not in organizations:
                 add_organization(str(org_id), str(org_name))
-            organizations[org_id]["workflows"].append({"id": workflow["id"], "title": title})
+            organizations[org_id]["workflows"].append(
+                {"id": workflow["id"], "title": title, "licenses": workflow["licenses"]}
+            )
 
         for lic in licenses:
             title = self._get_localized(language, lic["localizations"])

diff --git a/metadata_backend/conf/workflows/2_sdsx.json b/metadata_backend/conf/workflows/2_sdsx.json
@@ -9,7 +9,7 @@
                 {
                     "name": "dac",
                     "required": true,
-                    "allowMultipleObjects": true
+                    "allowMultipleObjects": false
                 },
                 {
                     "name": "policy",
@@ -36,7 +36,7 @@
                 {
                     "name": "dataset",
                     "required": true,
-                    "allowMultipleObjects": true
+                    "allowMultipleObjects": false
                 }
             ]
         },

diff --git a/metadata_backend/helpers/schemas/submission.json b/metadata_backend/helpers/schemas/submission.json
@@ -1140,6 +1140,20 @@
                                 ]
                             }
                         }
+                    },
+                    "internal_rems": {
+                        "url": {
+                            "type": "string",
+                            "title": "URL for application to apply access for dataset in REMS"
+                        },
+                        "resourceId": {
+                            "type": "string",
+                            "title": "REMS id of the resource created. Created automatically during the publication process"
+                        },
+                        "catalogueItemId": {
+                            "type": "string",
+                            "title": "REMS id of the catalogue created. Created automatically during the publication process"
+                        }
                     }
                 }
             },
@@ -1208,14 +1222,6 @@
                         "type": "integer",
                         "title": "REMS id of a policy/license"
                     }
-                },
-                "resourceId": {
-                    "type": "string",
-                    "title": "REMS id of the resource created. Created automatically during the publication process"
-                },
-                "catalogueItemId": {
-                    "type": "string",
-                    "title": "REMS id of the catalogue created. Created automatically during the publication process"
                 }
             }
         },

diff --git a/metadata_backend/services/rems_service_handler.py b/metadata_backend/services/rems_service_handler.py
@@ -13,7 +13,7 @@
 """
 
 import time
-from typing import Any
+from typing import Any, Optional
 
 from aiohttp import ClientTimeout, web
 from aiohttp.client_exceptions import ClientConnectorError, InvalidURL
@@ -104,12 +104,15 @@ async def create_catalogue_item(
         result: str = created["id"]
         return result
 
-    async def item_ok(self, item_type: str, organization_id: str, item_id: int) -> bool:
+    async def item_ok(
+        self, item_type: str, organization_id: str, item_id: int, licenses: Optional[list[int]] = None
+    ) -> bool:
         """Check that item exists.
 
         :param item_type: 'workflow' or 'license'
         :param organization_id: REMS organization/id
         :param item_id: REMS item id
+        :param licenses: list of licence IDs
         :raises HTTPError
         :returns: True or raises
         """
@@ -127,6 +130,17 @@ async def item_ok(self, item_type: str, organization_id: str, item_id: int) -> b
                     reason=f"{capitalized_item_type} '{item_id}' doesn't belong to organization '{organization_id}'",
                     status=400,
                 )
+            if item_type == "workflow":
+                linked_licenses = item["licenses"]  # licenses that are pre-linked to workflow in REMS
+                for lic in linked_licenses:
+                    if lic["license/id"] not in licenses:
+                        raise self.make_exception(
+                            reason=f"Linked license '{lic['license/id']}' doesn't exist in licenses '{licenses}'",
+                            status=400,
+                        )
+
+                LOG.debug("Licenses are linked to workflow correctly.")
+
         except KeyError as exc:
             raise self.make_exception(
                 reason=f"{capitalized_item_type} '{item_id}' has unexpected structure.", status=400
@@ -138,7 +152,7 @@ async def item_ok(self, item_type: str, organization_id: str, item_id: int) -> b
         return True
 
     async def validate_workflow_licenses(self, organization_id: str, workflow_id: int, licenses: list[int]) -> bool:
-        """Check that workflow and policies exist.
+        """Check that workflow and licenses exist.
 
         :param organization_id: REMS organization/id
         :param workflow_id: REMS workflow id
@@ -159,13 +173,13 @@ async def validate_workflow_licenses(self, organization_id: str, workflow_id: in
         if not isinstance(licenses, list):
             raise self.make_exception(reason=f"Licenses '{licenses}' must be a list of integers.", status=400)
 
-        await self.item_ok("workflow", organization_id, workflow_id)
+        await self.item_ok("workflow", organization_id, workflow_id, licenses)
 
         for license_id in licenses:
             if not isinstance(license_id, int):
                 raise self.make_exception(reason=f"Workflow ID '{license_id}' must be an integer.", status=400)
-
             await self.item_ok("license", organization_id, license_id)
+
         LOG.debug("REMS All ok.")
         return True
 

diff --git a/tests/http/rems.http b/tests/http/rems.http
@@ -3,17 +3,19 @@
 
 
 @backend_url = http://localhost:5430
-# @auth_url = http://localhost:8000
-@rems_url = https://test-rems.sd.csc.fi/api
-# @rems_url = http://localhost:8003/api
-@rems_user_id = sd-submit-robot
-@rems_api_key = sd-submit-test-key
+@auth_url = http://localhost:8000
+# @rems_url = https://rems-dev.2.rahtiapp.fi/api
+@rems_url = http://localhost:8003/api
+@rems_url_2 = http://localhost:8003/
+
+@rems_user_id = owner
+@rems_api_key = 42
 
 @auth_email = [email protected]
 
 @resid = {{resid_req.response.body.id}}
-@wfid = {{dac_req.response.body.$[0].id}}
-@orgid = {{dac_req.response.body.$[0].organization.organization/id}}
+@wfid = {{dac_req.response.body.id}}
+@orgid = {{dac_req.response.body.organization.organization/id}}
 @application_id = {{catalogue_req.response.body.id}}
 
 ### Mock authentication
@@ -30,7 +32,6 @@ GET {{backend_url}}/v1/rems
     ?language=fi
 
 ### Get workflows
-# @name dac_req
 GET {{rems_url}}/workflows
     ?disabled=false
     &archived=false
@@ -53,7 +54,6 @@ x-rems-user-id: {{rems_user_id}}
 x-rems-api-key: {{rems_api_key}}
 
 ### Get license
-# @name dac_req
 GET {{rems_url}}/licenses/1
     ?disabled=false
     &archived=false
@@ -70,7 +70,7 @@ Content-Type: application/json
 {
   "resid": "test-doi-here",
   "organization": {
-    "organization/id": "another_org"
+    "organization/id": "CSC"
   },
   "licenses": [
     1
@@ -105,9 +105,8 @@ Content-Type: application/json
   "archived": false
 }
 
-
 ### Get application - only with mock api
 GET {{rems_url}}/application
-    ?items=2
+    ?items=1
 x-rems-user-id: {{rems_user_id}}
 x-rems-api-key: {{rems_api_key}}
diff --git a/tests/integration/mock_rems_api.py b/tests/integration/mock_rems_api.py
@@ -54,7 +54,31 @@
             "handlers": ["user_id_1", "user_id_2"],
             "forms": [1],
         },
-        "licenses": [],
+        "licenses": [
+            {
+                "license/id": 1,
+                "licensetype": "link",
+                "organization": {
+                    "organization/id": "CSC",
+                    "organization/short-name": organizations["CSC"]["short-name"],
+                    "organization/name": organizations["CSC"]["name"],
+                },
+                "localizations": {
+                    "fi": {
+                        "title": "Nimeä 4.0 Kansainvälinen (CC BY 4.0)",
+                        "textcontent": "https://creativecommons.org/licenses/by/4.0/deed.fi",
+                        "attachment-id": None,
+                    },
+                    "en": {
+                        "title": "Creative Commons Attribution 4.0 International (CC BY 4.0)",
+                        "textcontent": "https://creativecommons.org/licenses/by/4.0/",
+                        "attachment-id": None,
+                    },
+                },
+                "enabled": True,
+                "archived": False,
+            }
+        ],
         "enabled": True,
         "archived": False,
     },
@@ -231,16 +255,20 @@ async def get_license(request: web.Request) -> web.Response:
         raise web.HTTPNotFound(reason=f"License with id '{license_id}' was not found.")
 
 
+# TO_DO: Check with REMS if an application includes multiple catalogues would work
 async def get_application(request: web.Request) -> web.Response:
     """REMS application."""
+    application_catalogues = []
     try:
-        items = int(request.query["items"])
-        item = catalogue[items]
+        items = request.query["items"].split(",")
+        for i in items:
+            item = catalogue[int(i)]
+            application_catalogues.append(item)
     except (KeyError, TypeError) as e:
         LOG.exception(e)
         LOG.debug(request)
         return web.HTTPNotFound(reason="Catalogue item was not found")
-    return web.json_response(data=item)
+    return web.json_response(data=application_catalogues)
 
 
 async def post_resource(request: web.Request) -> web.Response:
@@ -321,7 +349,7 @@ async def init() -> web.Application:
     app.router.add_get("/api/licenses/{license_id}", get_license)
     app.router.add_post("/api/resources/create", post_resource)
     app.router.add_post("/api/catalogue-items/create", post_catalogue_item)
-    app.router.add_get("/application", get_application)
+    app.router.add_get("/api/application", get_application)
     return app