From a5013ebd6483d30c2f8c8766ee75936e59cb87df Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Wed, 20 Nov 2024 13:20:06 +0530 Subject: [PATCH] fix: Instance admin not updating when email added via env variable (#37568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using feature flagged function to evict cache to ensure correct cache line gets evicted depending on the flag ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes https://github.com/appsmithorg/appsmith/issues/33741 ## Automation /test all ### :mag: Cypress test results > [!CAUTION] > 🔴 🔴 🔴 Some tests have failed. > Workflow run: > Commit: d4aed1f340e907c588f156d83c32e67c1ab4da18 > Cypress dashboard. > Tags: @tag.All > Spec: > The following are new failures, please fix them before merging the PR:
    >
  1. cypress/e2e/Regression/ClientSide/FormLogin/EnableFormLogin_spec.js >
  2. cypress/e2e/Regression/ClientSide/Github/EnableGithub_spec.ts
> List of identified flaky tests. >
Tue, 19 Nov 2024 15:04:20 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Enhanced migration functionalities for database schema updates and permission management. - Added methods for managing SSL settings for MSSQL datasources and updating Oracle plugin configurations. - Improved query performance with the creation of new indexes. - **Bug Fixes** - Updated permission management for super users and system themes, ensuring proper access. - **Chores** - Minor formatting adjustments and import updates for code clarity. --- .../appsmith/server/migrations/DatabaseChangelog2.java | 8 +++----- .../server/migrations/MigrationHelperMethods.java | 8 ++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java index e64c407760..24acdcdda6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java @@ -28,6 +28,7 @@ import com.appsmith.server.dtos.Permission; import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.migrations.solutions.UpdateSuperUserMigrationHelper; import com.appsmith.server.repositories.CacheableRepositoryHelper; +import com.appsmith.server.repositories.PermissionGroupRepository; import com.appsmith.server.solutions.PolicySolution; import com.github.cloudyrock.mongock.ChangeLog; import com.github.cloudyrock.mongock.ChangeSet; @@ -446,14 +447,11 @@ public class DatabaseChangelog2 { /** * Changing the order of this function to 10000 so that it always gets executed at the end. * This ensures that any permission changes for super users happen once all other migrations are completed - * - * @param mongoTemplate - * @param cacheableRepositoryHelper */ @ChangeSet(order = "10000", id = "update-super-users", author = "", runAlways = true) public void updateSuperUsers( MongoTemplate mongoTemplate, - CacheableRepositoryHelper cacheableRepositoryHelper, + PermissionGroupRepository permissionGroupRepository, PolicySolution policySolution, PolicyGenerator policyGenerator) { // Read the admin emails from the environment and update the super users accordingly @@ -499,7 +497,7 @@ public class DatabaseChangelog2 { Set oldSuperUsers = instanceAdminPG.getAssignedToUserIds(); Set updatedUserIds = findSymmetricDiff(oldSuperUsers, userIds); - evictPermissionCacheForUsers(updatedUserIds, mongoTemplate, cacheableRepositoryHelper); + evictPermissionCacheForUsers(updatedUserIds, mongoTemplate, permissionGroupRepository); Update update = new Update().set(PermissionGroup.Fields.assignedToUserIds, userIds); mongoTemplate.updateFirst(permissionGroupQuery, update, PermissionGroup.class); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/MigrationHelperMethods.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/MigrationHelperMethods.java index 277a3ac576..51abd5c67c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/MigrationHelperMethods.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/MigrationHelperMethods.java @@ -21,7 +21,7 @@ import com.appsmith.server.dtos.ApplicationJson; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.CollectionUtils; -import com.appsmith.server.repositories.CacheableRepositoryHelper; +import com.appsmith.server.repositories.PermissionGroupRepository; import com.fasterxml.jackson.databind.ObjectMapper; import net.minidev.json.JSONObject; import org.apache.commons.lang.StringUtils; @@ -211,7 +211,7 @@ public class MigrationHelperMethods { } public static void evictPermissionCacheForUsers( - Set userIds, MongoTemplate mongoTemplate, CacheableRepositoryHelper cacheableRepositoryHelper) { + Set userIds, MongoTemplate mongoTemplate, PermissionGroupRepository permissionGroupRepository) { if (userIds == null || userIds.isEmpty()) { // Nothing to do here. @@ -223,8 +223,8 @@ public class MigrationHelperMethods { User user = mongoTemplate.findOne(query, User.class); if (user != null) { // blocking call for cache eviction to ensure its subscribed immediately before proceeding further. - cacheableRepositoryHelper - .evictPermissionGroupsUser(user.getEmail(), user.getTenantId()) + permissionGroupRepository + .evictAllPermissionGroupCachesForUser(user.getEmail(), user.getTenantId()) .block(); } });