Support adding new model instances to already-released model projects.

This basically involves using both user and service clients.
Note that the service client needs at least read access to the private (collab) space (need to add model-curators group to the collab team with "viewer" role)

validation_service_api/validation_service/db.py

@@ -9,6 +9,7 @@
 from fairgraph.openminds.core import Model, ModelVersion, SoftwareVersion
 from fairgraph.openminds.computation import ValidationTest, ValidationTestVersion
 from . import settings
+from .auth import get_kg_client_for_service_account
 
 RETRY_INTERVAL = 60  # seconds
 
@@ -21,13 +22,13 @@ def _check_service_status():
         )
 
 
-def _get_model_by_id_or_alias(model_id, kg_client, scope):
+def _get_model_by_id_or_alias(model_id, kg_client, scope, use_cache=False):
     try:
         model_id = UUID(model_id)
         get_model = Model.from_uuid
     except ValueError:
         get_model = Model.from_alias
-    model_project = get_model(str(model_id), kg_client, scope=scope)
+    model_project = get_model(str(model_id), kg_client, scope=scope, use_cache=use_cache)
     if not model_project:
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,
@@ -45,21 +46,24 @@ def _get_model_instance_by_id(instance_id, kg_client, scope):
             detail=f"Model instance with ID '{instance_id}' not found.",
         )
 
-    model_project = Model.list(kg_client, scope=scope, space=model_instance.space,
-                               has_versions=model_instance)
+    model_project = Model.list(kg_client, scope=scope, has_versions=model_instance, use_cache=False)
     if not model_project:
         # we could get an empty response if the model_project has just been
         # updated and the KG is not consistent, so we wait and try again
         sleep(RETRY_INTERVAL)
-        model_project = Model.list(kg_client, scope=scope, space=model_instance.space,
-                                   has_versions=model_instance)
+        model_project = Model.list(kg_client, scope=scope, has_versions=model_instance, use_cache=False)
         if not model_project:
-            # in case of a dangling model instance, where the parent model_project
-            # has been deleted but the instance wasn't
-            raise HTTPException(
-                status_code=status.HTTP_404_NOT_FOUND,
-                detail=f"Model instance with ID '{instance_id}' no longer exists.",
-            )
+            # try the service client
+            kg_service_client = get_kg_client_for_service_account()
+            model_project = Model.list(kg_service_client, scope="in progress", has_versions=model_instance, use_cache=False)
+            if not model_project:
+                raise Exception("foo")
+                # in case of a dangling model instance, where the parent model_project
+                # has been deleted but the instance wasn't
+                raise HTTPException(
+                    status_code=status.HTTP_404_NOT_FOUND,
+                    detail=f"Model instance with ID '{instance_id}' no longer exists.",
+                )
     model_project = model_project[0]
     return model_instance, model_project.uuid
 

validation_service_api/validation_service/resources/models.py

@@ -5,7 +5,7 @@
 from fairgraph.utility import as_list
 import fairgraph.openminds.core as omcore
 import fairgraph.openminds.computation as omcmp
-from fairgraph.errors import ResolutionFailure
+from fairgraph.errors import ResolutionFailure, AuthorizationError, ResourceExistsError
 
 from fastapi import APIRouter, Depends, Query, Path, HTTPException, status
 from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials
@@ -26,6 +26,7 @@
     ModelInstance,
     ModelInstancePatch,
     project_id_from_space,
+    space_from_project_id,
     special_spaces
 )
 from ..queries import build_model_project_filters, model_alias_exists, expand_combinations
@@ -393,7 +394,9 @@ async def create_model(
             )
         assert space_name == kg_space
 
-    model_project.save(kg_user_client, space=kg_space, recursive=True)
+    # todo: we might want to save the tree explicitly rather than recursively,
+    #       to have more control over when we ignore duplicates
+    model_project.save(kg_user_client, space=kg_space, recursive=True, ignore_duplicates=True)
     model_project.scope = "in progress"
     return ScientificModel.from_kg_object(model_project, kg_user_client)
 
@@ -629,9 +632,15 @@ async def create_model_instance(
             detail=f"Another model instance with the same name already exists.",
         )
     # otherwise save to KG
-    model_instance_kg.save(kg_user_client, space=model_project.space, recursive=True)
+    target_space = space_from_project_id(collab_id)
+    model_instance_kg.save(kg_user_client, space=target_space, recursive=True)
     model_project.has_versions = as_list(model_project.has_versions) + [model_instance_kg]
-    model_project.save(kg_user_client, recursive=False)
+    try:
+        model_project.save(kg_user_client, recursive=False)
+    except AuthorizationError:
+        # if the model project has already been published we may not be able
+        # to save with user client, so use service client
+        model_project.save(kg_service_client, recursive=False)
     return ModelInstance.from_kg_object(model_instance_kg, kg_user_client, model_project.uuid, scope="any")
 
 
@@ -708,18 +717,18 @@ async def delete_model_instance_by_id(
     _check_service_status()
     user = User(token, allow_anonymous=False)
     kg_user_client = get_kg_client_for_user_account(token)
+    kg_service_client = get_kg_client_for_service_account()
     model_instance_kg, model_id = _get_model_instance_by_id(model_instance_id, kg_user_client, scope="any")
-    model_project = _get_model_by_id_or_alias(model_id, kg_user_client, scope="any")
-    collab_id = model_project.space[len("collab-"):]
+    collab_id = project_id_from_space(model_instance_kg.space)
     if not (
         await user.can_edit_collab(collab_id)
         or await user.is_admin()
     ):
         raise HTTPException(
             status_code=status.HTTP_403_FORBIDDEN,
-            detail=f"Access to this model is restricted to members of Collab #{model_project.collab_id}",
+            detail=f"Access to this model is restricted to members of Collab #{model_instance_kg.space}",
         )
-    await _delete_model_instance(model_instance_id, model_project, kg_user_client)
+    await _delete_model_instance(model_instance_id, model_id, kg_user_client, kg_service_client)
 
 
 @router.delete("/models/{model_id}/instances/{model_instance_id}", status_code=status.HTTP_200_OK)
@@ -730,21 +739,27 @@ async def delete_model_instance(
     # todo: handle non-existent UUID
     user = User(token, allow_anonymous=False)
     kg_user_client = get_kg_client_for_user_account(token)
+    kg_service_client = get_kg_client_for_service_account()
     model_instance_kg, model_id = _get_model_instance_by_id(model_instance_id, kg_user_client, scope="any")
-    model_project = _get_model_by_id_or_alias(model_id, kg_user_client, scope="any")
-    collab_id = model_project.space[len("collab-"):]
+    collab_id = project_id_from_space(model_instance_kg.space)
     if not (
         await user.can_edit_collab(collab_id)
         or await user.is_admin()
     ):
         raise HTTPException(
             status_code=status.HTTP_403_FORBIDDEN,
-            detail=f"Access to this model is restricted to members of Collab #{model_project.collab_id}",
+            detail=f"Access to this model is restricted to members of Collab #{model_instance_kg.space}",
         )
-    await _delete_model_instance(model_instance_id, model_project, kg_user_client)
+    await _delete_model_instance(model_instance_id, model_id, kg_user_client, kg_service_client)
 
 
-async def _delete_model_instance(model_instance_id, model_project, kg_user_client):
+async def _delete_model_instance(model_instance_id, model_id, kg_user_client, kg_service_client):
+    try:
+        model_project = _get_model_by_id_or_alias(model_id, kg_user_client, scope="in progress", use_cache=False)
+        kg_client_for_model = kg_user_client
+    except HTTPException:
+        model_project = _get_model_by_id_or_alias(model_id, kg_service_client, scope="in progress", use_cache=False)
+        kg_client_for_model = kg_service_client
     model_instances = as_list(model_project.has_versions)
     n_start = len(model_instances)
     for model_instance in model_instances[:]:
@@ -757,4 +772,4 @@ async def _delete_model_instance(model_instance_id, model_project, kg_user_clien
     if n_start > 0:
         assert len(model_instances) == n_start - 1
     model_project.has_versions = model_instances
-    model_project.save(kg_user_client, recursive=False)
+    model_project.save(kg_client_for_model, recursive=False)

validation_service_api/validation_service/resources/tests.py

@@ -261,7 +261,7 @@ async def create_test(test: ValidationTest, token: HTTPAuthorizationCredentials
         assert space_name == kg_space
 
     try:
-        test_definition.save(kg_user_client, recursive=True, space=kg_space)
+        test_definition.save(kg_user_client, recursive=True, space=kg_space, ignore_duplicates=True)
     except AuthenticationError as err:
         user_info = await user.get_user_info()
         raise HTTPException(

validation_service_api/validation_service/tests/fixtures.py

@@ -17,6 +17,14 @@
     credentials = os.environ["VF_TEST_TOKEN"]
 )
 AUTH_HEADER = {"Authorization": f"{token.scheme} {token.credentials}"}
+if "VF_ADMIN_TOKEN" in os.environ:
+    admin_token = HTTPAuthorizationCredentials(
+        scheme="Bearer",
+        credentials = os.environ["VF_ADMIN_TOKEN"]
+    )
+    ADMIN_AUTH_HEADER = {"Authorization": f"{admin_token.scheme} {admin_token.credentials}"}
+else:
+    ADMIN_AUTH_HEADER = None
 #TEST_SPACE = "myspace"
 #TEST_PROJECT = "myspace"
 TEST_PROJECT = "validation-framework-testing"

validation_service_api/validation_service/tests/test_models.py

@@ -9,7 +9,10 @@
 import pytest
 
 from ..data_models import BrainRegion, Species
-from .fixtures import _build_sample_model, private_model, released_model, client, token, AUTH_HEADER, TEST_PROJECT
+from .fixtures import (
+    _build_sample_model, private_model, released_model, client, token,
+    AUTH_HEADER, ADMIN_AUTH_HEADER, TEST_PROJECT
+)
 
 
 def check_model(model):
@@ -654,6 +657,53 @@ def test_delete_model_instance(caplog):
     assert response.status_code == 200
 
 
+def test_add_instance_to_published_model(caplog):
+    # We retrieve a model in the "model" space that has been released
+    # A normal user should be able to add an instance to this model
+    # but should not be able to change any other metadata.
+    published_model_uuid = "e919d175-b831-4295-b84c-9b00c944f0e3"  # Olfactory bulb network model 2003
+    response = client.get(f"/models/{published_model_uuid}")
+    assert response.status_code == 200
+    published_model = response.json()
+    assert len(published_model["instances"]) == 3  # three published versions
+    assert published_model["project_id"] == "model"
+    now = datetime.now(timezone.utc)
+    new_instance = {
+        "version": f"test-{now.strftime('%Y-%m-%dT%H:%M:%S.%f')}",
+        "description": "This is fake data for testing",
+        "code_format": "application/vnd.neuron-simulator+hoc",
+        "source": "http://example.com/fake_olfactory_bulb_model_for_testing",
+        "license": "The MIT license",
+        "project_id": TEST_PROJECT
+    }
+    response = client.post(f"/models/{published_model_uuid}/instances/", json=new_instance, headers=AUTH_HEADER)
+    assert response.status_code == 201
+    new_instance_uuid = response.json()["id"]
+
+    # get the model project again without authentication - should still have 3 (published) instances
+    response = client.get(f"/models/{published_model_uuid}")
+    assert response.status_code == 200
+    published_model_again = response.json()
+    assert len(published_model_again["instances"]) == 3  # three published versions
+
+    # get the model project again with an admin account - should now have 4 instances - 3 published plus one unpublished
+    # note that this doesn't work with a normal user account, which can't get in-progress info from "model" space
+    response = client.get(f"/models/{published_model_uuid}", headers=ADMIN_AUTH_HEADER)
+    assert response.status_code == 200
+    published_model_again = response.json()
+    assert len(published_model_again["instances"]) == 4
+
+    # cleanup: delete model instance again
+    response = client.delete(f"/models/{published_model_uuid}/instances/{new_instance_uuid}", headers=AUTH_HEADER)
+    assert response.status_code == 200
+
+    # get the model project one last time with admin account - should be back to the three published instances
+    response = client.get(f"/models/{published_model_uuid}", headers=ADMIN_AUTH_HEADER)
+    assert response.status_code == 200
+    published_model_again = response.json()
+    assert len(published_model_again["instances"]) == 3
+
+
 def test_hhnb_models(caplog):
     # list of models used by the HHNB app, to check
     hhnb_models = [