fix: Empty mono handling in theme repository

appsmithorg · Jun 19, 2024 · a2e0212 · a2e0212
1 parent 4b29951
commit a2e0212
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 52 deletions.
diff --git a/...-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java b/...-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java
@@ -815,14 +815,14 @@ public Mono<Application> findByBranchNameAndDefaultApplicationId(
             List<String> projectionFieldNames,
             AclPermission aclPermission) {
         if (!StringUtils.hasLength(branchName)) {
-            return UserPermissionUtils.updateAclWithUserContext(aclPermission).flatMap(permission -> repository
+            return UserPermissionUtils.updateAclWithUserContext(aclPermission).then(Mono.defer(() -> repository
                     .queryBuilder()
                     .byId(defaultApplicationId)
                     .fields(projectionFieldNames)
-                    .permission(permission)
+                    .permission(aclPermission)
                     .one()
                     .switchIfEmpty(Mono.error(new AppsmithException(
-                            AppsmithError.NO_RESOURCE_FOUND, FieldName.APPLICATION, defaultApplicationId))));
+                            AppsmithError.NO_RESOURCE_FOUND, FieldName.APPLICATION, defaultApplicationId)))));
         }
         return repository
                 .getApplicationByGitBranchAndDefaultApplicationId(

diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/PermissionAspect.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/PermissionAspect.java
@@ -1,7 +1,6 @@
 package com.appsmith.server.aspect;
 
 import com.appsmith.server.acl.AclPermission;
-import com.appsmith.server.repositories.ce.params.QueryAllParams;
 import lombok.extern.slf4j.Slf4j;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.annotation.Around;
@@ -32,12 +31,11 @@ public Object handlePermission(ProceedingJoinPoint joinPoint) throws Throwable {
 
         AclPermission permissionWithoutUserContext = Arrays.stream(joinPoint.getArgs())
                 .filter(arg -> arg instanceof AclPermission
-                        || (arg instanceof Optional && ((Optional<?>) arg).orElse(null) instanceof AclPermission)
-                        || arg instanceof QueryAllParams<?>)
+                        // TODO (Abhijeet): This is a temporary fix to avoid Optional<AclPermission> in the repository
+                        // methods.
+                        || (arg instanceof Optional && ((Optional<?>) arg).orElse(null) instanceof AclPermission))
                 .map(arg -> {
-                    if (arg instanceof QueryAllParams<?>) {
-                        return ((QueryAllParams<?>) arg).getPermission();
-                    } else if (arg instanceof AclPermission) {
+                    if (arg instanceof AclPermission) {
                         return (AclPermission) arg;
                     }
                     return (AclPermission) ((Optional<?>) arg).orElse(null);

diff --git a/...appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java b/...appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java
@@ -594,13 +594,14 @@ public Mono<String> findRootApplicationIdFromNewPage(String branchName, String d
             if (!StringUtils.hasLength(defaultPageId)) {
                 return Mono.error(new AppsmithException(INVALID_PARAMETER, FieldName.PAGE_ID, defaultPageId));
             }
-            getPageMono = UserPermissionUtils.updateAclWithUserContext(pagePermission.getReadPermission())
-                    .flatMap(permission -> repository
+            AclPermission permission = pagePermission.getReadPermission();
+            getPageMono = UserPermissionUtils.updateAclWithUserContext(permission)
+                    .then(Mono.defer(() -> repository
                             .queryBuilder()
                             .byId(defaultPageId)
                             .fields(FieldName.APPLICATION_ID, FieldName.DEFAULT_RESOURCES)
                             .permission(permission)
-                            .one());
+                            .one()));
         } else {
             getPageMono = repository.findPageByBranchNameAndDefaultPageId(
                     branchName, defaultPageId, pagePermission.getReadPermission());

diff --git a/...server/src/main/java/com/appsmith/server/repositories/ce/CustomThemeRepositoryCEImpl.java b/...server/src/main/java/com/appsmith/server/repositories/ce/CustomThemeRepositoryCEImpl.java
@@ -2,9 +2,8 @@
 
 import com.appsmith.server.acl.AclPermission;
 import com.appsmith.server.domains.Theme;
-import com.appsmith.server.domains.User;
 import com.appsmith.server.helpers.CollectionUtils;
-import com.appsmith.server.helpers.ReactiveContextUtils;
+import com.appsmith.server.helpers.UserPermissionUtils;
 import com.appsmith.server.helpers.ce.bridge.Bridge;
 import com.appsmith.server.helpers.ce.bridge.BridgeQuery;
 import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl;
@@ -30,51 +29,29 @@ public Flux<Theme> getApplicationThemes(String applicationId, AclPermission aclP
 
     @Override
     public Flux<Theme> getSystemThemes() {
-        Mono<User> currentUserMono = ReactiveContextUtils.getCurrentUser();
-        return currentUserMono.flatMapMany(user -> {
-            if (user == null) {
-                return Flux.empty();
-            }
-            AclPermission permission = AclPermission.READ_THEMES;
-            permission.setUser(user);
-            return queryBuilder()
-                    .criteria(Bridge.isTrue(Theme.Fields.isSystemTheme))
-                    .permission(permission)
-                    .all();
-        });
+        AclPermission permission = AclPermission.READ_THEMES;
+        return UserPermissionUtils.updateAclWithUserContext(permission).thenMany(Flux.defer(() -> queryBuilder()
+                .criteria(Bridge.isTrue(Theme.Fields.isSystemTheme))
+                .permission(permission)
+                .all()));
     }
 
     @Override
     public Mono<Theme> getSystemThemeByName(String themeName) {
-        Mono<User> currentUserMono = ReactiveContextUtils.getCurrentUser();
-        return currentUserMono.flatMap(user -> {
-            if (user == null) {
-                return Mono.empty();
-            }
-            AclPermission permission = AclPermission.READ_THEMES;
-            permission.setUser(user);
-            return queryBuilder()
-                    .criteria(
-                            Bridge.equalIgnoreCase(Theme.Fields.name, themeName).isTrue(Theme.Fields.isSystemTheme))
-                    .permission(permission)
-                    .one();
-        });
+        AclPermission permission = AclPermission.READ_THEMES;
+        return UserPermissionUtils.updateAclWithUserContext(permission).then(Mono.defer(() -> queryBuilder()
+                .criteria(Bridge.equalIgnoreCase(Theme.Fields.name, themeName).isTrue(Theme.Fields.isSystemTheme))
+                .permission(permission)
+                .one()));
     }
 
     private Mono<Boolean> archiveThemeByCriteria(BridgeQuery<Theme> criteria) {
-        Mono<User> currentUserMono = ReactiveContextUtils.getCurrentUser();
-        return currentUserMono.flatMap(user -> {
-            if (user == null) {
-                return Mono.empty();
-            }
-            AclPermission permission = AclPermission.MANAGE_THEMES;
-            permission.setUser(user);
-            return queryBuilder()
-                    .criteria(criteria)
-                    .permission(permission)
-                    .updateAll(Bridge.update().set(Theme.Fields.deletedAt, Instant.now()))
-                    .map(count -> count > 0);
-        });
+        AclPermission permission = AclPermission.MANAGE_THEMES;
+        return UserPermissionUtils.updateAclWithUserContext(permission).then(Mono.defer(() -> queryBuilder()
+                .criteria(criteria)
+                .permission(permission)
+                .updateAll(Bridge.update().set(Theme.Fields.deletedAt, Instant.now()))
+                .map(count -> count > 0)));
     }
 
     @Override