Merge remote-tracking branch 'origin' into tauquir/scenario-tests

OpenMined · Oct 8, 2024 · 2bd24b9 · 2bd24b9
2 parents 38ff6d4 + 99e6659
commit 2bd24b9
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 39 deletions.
diff --git a/packages/syft/src/syft/service/output/output_service.py b/packages/syft/src/syft/service/output/output_service.py
@@ -315,23 +315,6 @@ def get_by_output_policy_id(
     def get(self, context: AuthedServiceContext, id: UID) -> ExecutionOutput:
         return self.stash.get_by_uid(context.credentials, id).unwrap()
 
-    # @service_method(
-    #     path="output.set_permission",
-    #     name="set_permission",
-    #     roles=GUEST_ROLE_LEVEL,
-    # )
-    # def set_permission(
-    #     self, context: AuthedServiceContext, uid, credentials
-    # ) -> ExecutionOutput:
-    #     exec_output = self.get(context, uid)
-    #     permissions = [
-    #         ActionObjectREAD(uid=_id.id, credentials=credentials)
-    #         for _id in exec_output.output_id_list
-    #     ]
-    #     return context.server.services.action.stash.add_permissions(
-    #         permissions
-    #     ).unwrap()
-
     @service_method(path="output.get_all", name="get_all", roles=GUEST_ROLE_LEVEL)
     def get_all(self, context: AuthedServiceContext) -> list[ExecutionOutput]:
         return self.stash.get_all(context.credentials).unwrap()

diff --git a/packages/syft/src/syft/service/sync/diff_state.py b/packages/syft/src/syft/service/sync/diff_state.py
@@ -1602,15 +1602,18 @@ def from_batch_decision(
                             )
                         ]
                     }
-                    new_permissions_high_side = {
-                        diff.obj_type: [
-                            ActionObjectPermission(
-                                uid=diff.object_id,
-                                permission=ActionPermission.READ,
-                                credentials=share_to_user,
-                            )
-                        ]
-                    }
+                    if diff.obj_type in [Job, SyftLog, Request] or issubclass(
+                        diff.obj_type, ActionObject
+                    ):
+                        new_permissions_high_side = {
+                            diff.obj_type: [
+                                ActionObjectPermission(
+                                    uid=diff.object_id,
+                                    permission=ActionPermission.READ,
+                                    credentials=share_to_user,
+                                )
+                            ]
+                        }
 
         # storage permissions
         new_storage_permissions = []

diff --git a/packages/syft/src/syft/service/sync/sync_service.py b/packages/syft/src/syft/service/sync/sync_service.py
@@ -25,6 +25,8 @@
 from ..code.user_code import UserCodeStatusCollection
 from ..context import AuthedServiceContext
 from ..job.job_stash import Job
+from ..log.log import SyftLog
+from ..request.request import Request
 from ..response import SyftSuccess
 from ..service import AbstractService
 from ..service import TYPE_TO_SERVICE
@@ -199,6 +201,29 @@ def sync_items(
             for permission in permission_list:
                 permissions_dict[permission.uid].append(permission)
 
+        item_ids = [item.id.id for item in items]
+
+        # If we just want to add permissions without having an object
+        # This should happen only for the high side when we sync results but
+        # we need to add permissions for the DS to properly show the status of the requests
+        for obj_type, permission_list in permissions.items():
+            for permission in permission_list:
+                if permission.uid in item_ids:
+                    continue
+                if obj_type not in [Job, SyftLog, Request] and not issubclass(
+                    obj_type, ActionObject
+                ):
+                    raise SyftException(
+                        public_message="Permission for object type not supported!"
+                    )
+                if issubclass(obj_type, ActionObject):
+                    store = context.server.services.action.stash
+                else:
+                    service = context.server.get_service(TYPE_TO_SERVICE[obj_type])
+                    store = service.stash  # type: ignore[assignment]
+                if permission.permission == ActionPermission.READ:
+                    store.add_permission(permission)
+
         storage_permissions_dict = defaultdict(list)
         for storage_permission in storage_permissions:
             storage_permissions_dict[storage_permission.uid].append(storage_permission)
@@ -219,19 +244,6 @@ def sync_items(
                     context, item, new_storage_permissions
                 )
 
-        # If we just want to add permissions without having an object
-        # This should happen only for the high side when we sync results but
-        # we need to add permissions for the DS to properly show the status of the requests
-        for obj_type, permission_list in permissions.items():
-            if issubclass(obj_type, ActionObject):
-                store = context.server.services.action.stash
-            else:
-                service = context.server.get_service(TYPE_TO_SERVICE[obj_type])
-                store = service.stash  # type: ignore[assignment]
-            for permission in permission_list:
-                if permission.permission == ActionPermission.READ:
-                    store.add_permission(permission)
-
         # NOTE include_items=False to avoid snapshotting the database
         # Snapshotting is disabled to avoid mongo size limit and performance issues
         new_state = self.build_current_state(