lahirujayathilake commented on code in PR #44:
URL:
https://github.com/apache/airavata-data-catalog/pull/44#discussion_r1951079677
##########
data-catalog-api/server/service/src/main/java/org/apache/airavata/datacatalog/api/service/impl/SharingServiceImpl.java:
##########
@@ -0,0 +1,153 @@
+package org.apache.airavata.datacatalog.api.service.impl;
+
+import io.grpc.Status;
+import io.grpc.stub.StreamObserver;
+import org.apache.airavata.datacatalog.api.GrantPermissionRequest;
+import org.apache.airavata.datacatalog.api.GrantPermissionResponse;
+import org.apache.airavata.datacatalog.api.GrantPermissionToGroupRequest;
+import org.apache.airavata.datacatalog.api.Permission;
+import org.apache.airavata.datacatalog.api.RevokePermissionFromGroupRequest;
+import org.apache.airavata.datacatalog.api.RevokePermissionRequest;
+import org.apache.airavata.datacatalog.api.RevokePermissionResponse;
+import org.apache.airavata.datacatalog.api.SearchGroupsResponse;
+import org.apache.airavata.datacatalog.api.SearchRequest;
+import org.apache.airavata.datacatalog.api.SearchUsersResponse;
+import org.apache.airavata.datacatalog.api.SharingServiceGrpc;
+import org.apache.airavata.datacatalog.api.sharing.SharingManager;
+import org.apache.airavata.datacatalog.api.sharing.exception.SharingException;
+import org.lognet.springboot.grpc.GRpcService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+/**
+ * In the same Spring Boot application as DataCatalogServiceImpl,
+ * mark it with @GRpcService to expose all RPC methods of SharingService
+ * (such as SearchUsers, SearchGroups, Grant/Revoke Permission, etc.)
externally.
+ */
+@GRpcService
+public class SharingServiceImpl extends
SharingServiceGrpc.SharingServiceImplBase {
+
+ @Autowired
+ private SharingManager sharingManager;
+
+ @Override
+ public void searchUsers(SearchRequest request,
StreamObserver<SearchUsersResponse> responseObserver) {
+ try {
+ // get keyword and tenant ID
+ String searchQuery = request.getQuery();
+ String tenantId = request.getTenantId();
+
+
+ var users = sharingManager.searchUsers(searchQuery, tenantId);
+
+
+ SearchUsersResponse response = SearchUsersResponse.newBuilder()
+ .addAllUsers(users)
+ .build();
+
+ responseObserver.onNext(response);
+ responseObserver.onCompleted();
+ } catch (SharingException e) {
+ responseObserver.onError(Status.INTERNAL
+ .withDescription("searchUsers failed: " + e.getMessage())
Review Comment:
better to include the tenantId
##########
data-catalog-api/server/core/src/main/java/org/apache/airavata/datacatalog/api/sharing/SharingManager.java:
##########
@@ -120,4 +122,21 @@ void revokePermissionFromGroup(GroupInfo groupInfo,
DataProduct dataProduct, Per
* @param permission
*/
void revokePublicAccess(DataProduct dataProduct, Permission permission)
throws SharingException;
+ /**
+ *
+ *
+ * @param searchQuery
Review Comment:
please include a description about this method
##########
data-catalog-api/server/simple-sharing/src/main/java/org/apache/airavata/datacatalog/api/sharing/SimpleSharingManagerImpl.java:
##########
@@ -289,4 +290,102 @@ private DataProductEntity resolveDataProduct(DataProduct
dataProduct) throws Sha
});
return dataProductEntity;
}
+
+ @Override
+ public List<UserInfo> searchUsers(String searchQuery, String tenantId)
throws SharingException {
+ try {
+ // get tenant
+ Optional<SimpleTenantEntity> tenantOptional =
simpleTenantRepository.findByExternalId(tenantId);
+
+ if (tenantOptional.isEmpty()) {
+ throw new SharingException("Tenant not found for ID: " +
tenantId);
+ }
+
+ SimpleTenantEntity tenant = tenantOptional.get();
+
+ // if searchQuery is null，return all users
+ if (searchQuery == null || searchQuery.trim().isEmpty()) {
+ // use EntityManager to search
+ var queryAll = entityManager.createQuery(
+ "SELECT u FROM SimpleUserEntity u WHERE u.simpleTenant
= :tenant",
+ SimpleUserEntity.class
+ );
+ queryAll.setParameter("tenant", tenant);
+ List<SimpleUserEntity> allUserEntities =
queryAll.getResultList();
+
+ // convert to UserInfo list, return
+ return allUserEntities.stream()
+ .map(u -> UserInfo.newBuilder()
+ .setUserId(u.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build())
+ .toList();
+ }
+
+ // find exact match
+ Optional<SimpleUserEntity> exactMatch =
simpleUserRepository.findByExternalIdAndSimpleTenant_ExternalId(searchQuery,
tenant.getExternalId());
+
+
+ List<UserInfo> users = exactMatch.map(user ->
List.of(UserInfo.newBuilder()
+ .setUserId(user.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build()))
+ .orElse(List.of());
+
+
+ return users;
+ } catch (Exception e) {
+ throw new SharingException("Failed to search users with query: " +
searchQuery, e);
+ }
+ }
+ @Override
+ public List<GroupInfo> searchGroups(String searchQuery, String tenantId)
throws SharingException {
+ try {
+
+ Optional<SimpleTenantEntity> tenantOptional =
simpleTenantRepository.findByExternalId(tenantId);
+
+ if (tenantOptional.isEmpty()) {
+ throw new SharingException("Tenant not found for ID: " +
tenantId);
+ }
+
+ SimpleTenantEntity tenant = tenantOptional.get();
+
+
+ if (searchQuery == null || searchQuery.trim().isEmpty()) {
+ var queryAll = entityManager.createQuery(
+ "SELECT g FROM SimpleGroupEntity g WHERE
g.simpleTenant = :tenant",
+ SimpleGroupEntity.class
+ );
+ queryAll.setParameter("tenant", tenant);
+ List<SimpleGroupEntity> allGroups = queryAll.getResultList();
+
+ return allGroups.stream()
+ .map(g -> GroupInfo.newBuilder()
+ .setGroupId(g.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build())
+ .toList();
+ }
+
+ Optional<SimpleGroupEntity> exactMatch =
simpleGroupRepository.findByExternalIdAndSimpleTenant(searchQuery, tenant);
+
+
+ List<GroupInfo> groups = exactMatch.map(group ->
List.of(GroupInfo.newBuilder()
+ .setGroupId(group.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build()))
+ .orElse(List.of());
+
+
Review Comment:
remove these lines
##########
data-catalog-api/server/simple-sharing/src/main/java/org/apache/airavata/datacatalog/api/sharing/SimpleSharingManagerImpl.java:
##########
@@ -289,4 +290,102 @@ private DataProductEntity resolveDataProduct(DataProduct
dataProduct) throws Sha
});
return dataProductEntity;
}
+
+ @Override
+ public List<UserInfo> searchUsers(String searchQuery, String tenantId)
throws SharingException {
+ try {
+ // get tenant
+ Optional<SimpleTenantEntity> tenantOptional =
simpleTenantRepository.findByExternalId(tenantId);
+
+ if (tenantOptional.isEmpty()) {
+ throw new SharingException("Tenant not found for ID: " +
tenantId);
+ }
+
+ SimpleTenantEntity tenant = tenantOptional.get();
+
+ // if searchQuery is null，return all users
+ if (searchQuery == null || searchQuery.trim().isEmpty()) {
Review Comment:
you can use the Stringutils.isNotBlank method
##########
data-catalog-api/server/service/src/main/java/org/apache/airavata/datacatalog/api/service/impl/SharingServiceImpl.java:
##########
Review Comment:
it's better to log the errors
##########
data-catalog-api/server/core/src/main/java/org/apache/airavata/datacatalog/api/sharing/SharingManager.java:
##########
@@ -120,4 +122,21 @@ void revokePermissionFromGroup(GroupInfo groupInfo,
DataProduct dataProduct, Per
* @param permission
*/
void revokePublicAccess(DataProduct dataProduct, Permission permission)
throws SharingException;
+ /**
+ *
+ *
+ * @param searchQuery
Review Comment:
and for the `searchGroups` too
##########
data-catalog-api/server/simple-sharing/src/main/java/org/apache/airavata/datacatalog/api/sharing/SimpleSharingManagerImpl.java:
##########
@@ -289,4 +290,102 @@ private DataProductEntity resolveDataProduct(DataProduct
dataProduct) throws Sha
});
return dataProductEntity;
}
+
+ @Override
+ public List<UserInfo> searchUsers(String searchQuery, String tenantId)
throws SharingException {
+ try {
+ // get tenant
+ Optional<SimpleTenantEntity> tenantOptional =
simpleTenantRepository.findByExternalId(tenantId);
+
+ if (tenantOptional.isEmpty()) {
+ throw new SharingException("Tenant not found for ID: " +
tenantId);
+ }
+
+ SimpleTenantEntity tenant = tenantOptional.get();
+
+ // if searchQuery is null，return all users
+ if (searchQuery == null || searchQuery.trim().isEmpty()) {
+ // use EntityManager to search
+ var queryAll = entityManager.createQuery(
+ "SELECT u FROM SimpleUserEntity u WHERE u.simpleTenant
= :tenant",
+ SimpleUserEntity.class
+ );
+ queryAll.setParameter("tenant", tenant);
+ List<SimpleUserEntity> allUserEntities =
queryAll.getResultList();
+
+ // convert to UserInfo list, return
+ return allUserEntities.stream()
+ .map(u -> UserInfo.newBuilder()
+ .setUserId(u.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build())
+ .toList();
+ }
+
+ // find exact match
+ Optional<SimpleUserEntity> exactMatch =
simpleUserRepository.findByExternalIdAndSimpleTenant_ExternalId(searchQuery,
tenant.getExternalId());
+
+
+ List<UserInfo> users = exactMatch.map(user ->
List.of(UserInfo.newBuilder()
+ .setUserId(user.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build()))
+ .orElse(List.of());
+
+
+ return users;
+ } catch (Exception e) {
+ throw new SharingException("Failed to search users with query: " +
searchQuery, e);
+ }
+ }
+ @Override
+ public List<GroupInfo> searchGroups(String searchQuery, String tenantId)
throws SharingException {
+ try {
+
+ Optional<SimpleTenantEntity> tenantOptional =
simpleTenantRepository.findByExternalId(tenantId);
+
+ if (tenantOptional.isEmpty()) {
+ throw new SharingException("Tenant not found for ID: " +
tenantId);
+ }
+
+ SimpleTenantEntity tenant = tenantOptional.get();
+
+
+ if (searchQuery == null || searchQuery.trim().isEmpty()) {
+ var queryAll = entityManager.createQuery(
+ "SELECT g FROM SimpleGroupEntity g WHERE
g.simpleTenant = :tenant",
+ SimpleGroupEntity.class
+ );
+ queryAll.setParameter("tenant", tenant);
+ List<SimpleGroupEntity> allGroups = queryAll.getResultList();
+
+ return allGroups.stream()
+ .map(g -> GroupInfo.newBuilder()
+ .setGroupId(g.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build())
+ .toList();
+ }
+
+ Optional<SimpleGroupEntity> exactMatch =
simpleGroupRepository.findByExternalIdAndSimpleTenant(searchQuery, tenant);
+
+
+ List<GroupInfo> groups = exactMatch.map(group ->
List.of(GroupInfo.newBuilder()
+ .setGroupId(group.getExternalId())
+ .setTenantId(tenant.getExternalId())
+ .build()))
+ .orElse(List.of());
+
+
+
+ return groups;
+ } catch (Exception e) {
+ throw new SharingException("Failed to search groups with query: "
+ searchQuery, e);
Review Comment:
better log this
--
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]
