lahirujayathilake commented on code in PR #48:
URL: 
https://github.com/apache/airavata-data-catalog/pull/48#discussion_r2014336522


##########
data-catalog-api/server/service/src/main/java/org/apache/airavata/datacatalog/api/query/impl/PostgresqlMetadataSchemaQueryWriterImpl.java:
##########
@@ -156,10 +156,13 @@ public SqlNode visit(SqlCall call) {
             return super.visit(call);
         }
     }
-
     @Override
     public String rewriteQuery(UserEntity userEntity, SqlNode sqlNode, 
Collection<MetadataSchemaEntity> metadataSchemas,
                                Map<String, String> tableAliases) {
+        List<String> groupIds = userEntity.getGroupIds();
+        if (groupIds == null) {

Review Comment:
   Can this be true? because you have an initialized groupIds variable in the 
user entity class



##########
data-catalog-api/server/custos-sharing/src/main/java/org/apache/airavata/datacatalog/api/sharing/SharingManagerImpl.java:
##########
@@ -133,7 +133,9 @@ public UserEntity resolveUser(UserInfo userInfo) throws 
SharingException {
         Optional<UserEntity> maybeUserEntity = 
userRepository.findByExternalIdAndTenant_ExternalId(userInfo.getUserId(),
                 userInfo.getTenantId());
         if (maybeUserEntity.isPresent()) {
-            return maybeUserEntity.get();
+            UserEntity userEntity = maybeUserEntity.get();
+            userEntity.setGroupIds(userInfo.getGroupIdsList());

Review Comment:
   I don't see a `setGroupIds` method in UserEntity



##########
data-catalog-api/server/service/src/main/java/org/apache/airavata/datacatalog/api/service/DataCatalogAPIService.java:
##########
@@ -336,4 +341,59 @@ private <T> boolean checkHasPermission(UserInfo userInfo, 
DataProduct dataProduc
         return false;
     }
 
+    @Override
+    public void grantPermissionToUser(GrantPermissionToUserRequest request,
+                                      
StreamObserver<GrantPermissionToUserResponse> responseObserver) {
+
+        try {
+
+            DataProduct dataProduct = 
dataCatalogService.getDataProduct(request.getDataProductId());
+
+            sharingManager.grantPermissionToUser(
+                    request.getTargetUser(),
+                    dataProduct,
+                    request.getPermission(),
+                    request.getUserInfo()
+            );
+            
responseObserver.onNext(GrantPermissionToUserResponse.getDefaultInstance());
+            responseObserver.onCompleted();
+        } catch (EntityNotFoundException e) {
+            responseObserver.onError(Status.NOT_FOUND
+                    .withDescription(e.getMessage()).asException());
+        } catch (Exception e) {
+            logger.error("Failed to grant permission to user", e);

Review Comment:
   log the errors, valid for `grantPermissionToGroup` method too



##########
data-catalog-api/server/service/src/main/java/org/apache/airavata/datacatalog/api/service/impl/DataCatalogServiceImpl.java:
##########
@@ -127,6 +127,8 @@ public MetadataSchemaQueryResult 
searchDataProducts(UserInfo userInfo, String sq
             throws MetadataSchemaSqlParseException, 
MetadataSchemaSqlValidateException {
         try {
             UserEntity userEntity = sharingManager.resolveUser(userInfo);
+            userEntity.getGroupIds().clear();
+            userEntity.getGroupIds().addAll( userInfo.getGroupIdsList() );

Review Comment:
   format these properly



##########
data-catalog-api/server/custos-sharing/src/main/java/org/apache/airavata/datacatalog/api/sharing/SharingManagerImpl.java:
##########


Review Comment:
   This project is not in the data-catalog-api pom



##########
data-catalog-api/server/simple-sharing/src/main/java/org/apache/airavata/datacatalog/api/sharing/SimpleSharingManagerImpl.java:
##########
@@ -80,13 +80,46 @@ public boolean userHasAccess(UserInfo userInfo, DataProduct 
dataProduct, Permiss
             throws SharingException {
         UserEntity user = resolveUser(userInfo);
         DataProductEntity dataProductEntity = resolveDataProduct(dataProduct);
-        Query query = entityManager.createNativeQuery("select 1 from " + 
getDataProductSharingView()
-                + " where user_id = :user_id and data_product_id = 
:data_product_id and permission_id in :permission_id");
-        query.setParameter("user_id", user.getUserId());
-        query.setParameter("data_product_id", 
dataProductEntity.getDataProductId());
-        query.setParameter("permission_id", 
Arrays.asList(permission.getNumber(), Permission.OWNER.getNumber()));
 
-        return query.getResultList().size() > 0;
+        Query userQuery = entityManager.createNativeQuery(
+                "SELECT 1 " +
+                        "  FROM " + getDataProductSharingView() +
+                        " WHERE user_id = :user_id " +
+                        "   AND data_product_id = :dp_id " +
+                        "   AND permission_id IN (:perm_ids_int)"
+        );
+        userQuery.setParameter("user_id", user.getUserId());
+        userQuery.setParameter("dp_id", dataProductEntity.getDataProductId());
+        userQuery.setParameter("perm_ids_int",
+                Arrays.asList(permission.getNumber(), 
Permission.OWNER.getNumber())
+        );
+
+        boolean userHasPerm = !userQuery.getResultList().isEmpty();
+        if (userHasPerm) {
+            return true;
+        }
+
+        if (userInfo.getGroupIdsCount() > 0) {
+            Query groupQuery = entityManager.createNativeQuery(
+                    "SELECT 1 " +
+                            "  FROM simple_group_sharing sgs " +
+                            "  JOIN simple_group g ON sgs.simple_group_id = 
g.simple_group_id " +
+                            " WHERE sgs.data_product_id = :dp_id " +
+                            "   AND sgs.permission_id IN (:perm_ids_str) " +
+                            "   AND g.external_id IN (:group_external_ids)"
+            );
+            groupQuery.setParameter("dp_id", 
dataProductEntity.getDataProductId());
+            groupQuery.setParameter("perm_ids_str",
+                    Arrays.asList(permission.name(), Permission.OWNER.name())
+            );
+            groupQuery.setParameter("group_external_ids", 
userInfo.getGroupIdsList());
+
+            boolean groupHasPerm = !groupQuery.getResultList().isEmpty();
+            if (groupHasPerm) {
+                return true;
+            }

Review Comment:
   simple use `return groupHasPerm;`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to