From b90a14ce00841e297abf286855a9c258aa4e55f8 Mon Sep 17 00:00:00 2001 From: Nilesh Sarupriya Date: Thu, 20 Apr 2023 11:34:08 +0530 Subject: [PATCH] chore: change workspace update from save to update object (#22549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > Change the update workspace flow to update using the MongoTemplate `updateFirst` rather than Repository `save` Fixes # (issue) > if no issue exists, please create an issue and ask the maintainers about this first Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video ## Type of change - Chore (housekeeping or task changes that don't impact user perception) ## How Has This Been Tested? > `testWorkspaceUpdate_checkAdditionalFieldsArePresentAfterUpdate` ### Test Plan > Add Testsmith test cases links that relate to this PR ### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) ## Checklist: ### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag ### QA activity: - [ ] Test plan has been approved by relevant developers - [ ] Test plan has been peer reviewed by QA - [ ] Cypress test cases have been added and approved by either SDET or manual QA - [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA - [ ] Added Test Plan Approved label after reveiwing all Cypress test --------- Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com> Co-authored-by: Aishwarya UR --- .../services/ce/WorkspaceServiceCEImpl.java | 23 +++++++++- .../server/services/WorkspaceServiceTest.java | 42 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java index 330212e7ae..30153c972e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java @@ -9,6 +9,7 @@ import com.appsmith.server.constants.Constraint; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Asset; import com.appsmith.server.domains.PermissionGroup; +import com.appsmith.server.domains.QWorkspace; import com.appsmith.server.domains.User; import com.appsmith.server.domains.Workspace; import com.appsmith.server.domains.WorkspacePlugin; @@ -32,12 +33,16 @@ import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserWorkspaceService; import com.appsmith.server.solutions.PermissionGroupPermission; import com.appsmith.server.solutions.WorkspacePermission; +import com.mongodb.DBObject; import lombok.extern.slf4j.Slf4j; import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Sort; import org.springframework.data.mongodb.core.ReactiveMongoTemplate; import org.springframework.data.mongodb.core.convert.MongoConverter; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; import org.springframework.http.codec.multipart.Part; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; @@ -69,6 +74,7 @@ import static com.appsmith.server.constants.PatternConstants.EMAIL_PATTERN; import static com.appsmith.server.constants.PatternConstants.WEBSITE_PATTERN; import static com.appsmith.server.helpers.PermissionUtils.collateAllPermissions; import static com.appsmith.server.helpers.TextUtils.generateDefaultRoleNameForResource; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.fieldName; import static java.lang.Boolean.TRUE; @Slf4j @@ -464,8 +470,21 @@ public class WorkspaceServiceCEImpl extends BaseService { + Query query = new Query(Criteria.where(fieldName(QWorkspace.workspace.id)).is(id)); + DBObject update = getDbObject(resource); + Update updateObj = new Update(); + Map updateMap = update.toMap(); + updateMap.forEach(updateObj::set); + return mongoTemplate.updateFirst(query, updateObj, resource.getClass()) + .flatMap(updateResult -> { + if (updateResult.getMatchedCount() == 0) { + return Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.WORKSPACE, id)); + } + return repository.findById(id) + .flatMap(analyticsService::sendUpdateEvent); + }); + })); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java index 8b689904d4..7d3ef11b9d 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java @@ -12,6 +12,7 @@ import com.appsmith.server.domains.Application; import com.appsmith.server.domains.Asset; import com.appsmith.server.domains.PermissionGroup; import com.appsmith.server.domains.Plugin; +import com.appsmith.server.domains.QWorkspace; import com.appsmith.server.domains.User; import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.InviteUsersDTO; @@ -28,6 +29,7 @@ import com.appsmith.server.repositories.PermissionGroupRepository; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; import com.appsmith.server.solutions.UserAndAccessManagementService; +import com.mongodb.client.result.UpdateResult; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -40,6 +42,10 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Criteria; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; import org.springframework.http.MediaType; import org.springframework.http.codec.multipart.FilePart; import org.springframework.security.test.context.support.WithMockUser; @@ -77,6 +83,7 @@ import static com.appsmith.server.constants.FieldName.ADMINISTRATOR; import static com.appsmith.server.constants.FieldName.DEVELOPER; import static com.appsmith.server.constants.FieldName.VIEWER; import static com.appsmith.server.helpers.TextUtils.generateDefaultRoleNameForResource; +import static com.appsmith.server.repositories.ce.BaseAppsmithRepositoryCEImpl.fieldName; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -129,6 +136,9 @@ public class WorkspaceServiceTest { @Autowired private PluginService pluginService; + @Autowired + MongoTemplate mongoTemplate; + @MockBean private PluginExecutorHelper pluginExecutorHelper; @@ -1625,4 +1635,36 @@ public class WorkspaceServiceTest { }) .verifyComplete(); } + + @Test + @WithUserDetails(value = "api_user") + void testWorkspaceUpdate_checkAdditionalFieldsArePresentAfterUpdate() { + String testName = "testWorkspaceUpdate"; + String additionalField = "testWorkspaceUpdate"; + Workspace workspace = new Workspace(); + workspace.setName(testName); + Workspace createdWorkspace = workspaceService.create(workspace).block(); + + Update updateAddAdditionalField = new Update().set(additionalField, true); + Query queryWorkspace = new Query(Criteria.where(fieldName(QWorkspace.workspace.id)).is(createdWorkspace.getId())); + UpdateResult updateResult = mongoTemplate.updateMulti(queryWorkspace, updateAddAdditionalField, Workspace.class); + + assertThat(updateResult.wasAcknowledged()).isTrue(); + assertThat(updateResult.getMatchedCount()).isEqualTo(1); + assertThat(updateResult.getModifiedCount()).isEqualTo(1); + + Criteria criteriaAdditionalField = new Criteria().andOperator(Criteria.where(fieldName(QWorkspace.workspace.id)).is(createdWorkspace.getId()), Criteria.where(additionalField).exists(true)); + Query queryWorkspaceWithAdditionalField = new Query(criteriaAdditionalField); + + long countWorkspaceWithAdditionalField = mongoTemplate.count(queryWorkspaceWithAdditionalField, Workspace.class); + assertThat(countWorkspaceWithAdditionalField).isEqualTo(1); + + Workspace updateWorkspace = new Workspace(); + updateWorkspace.setName(testName + " updated"); + Workspace updatedWorkspace = workspaceService.update(createdWorkspace.getId(), updateWorkspace).block(); + assertThat(updatedWorkspace.getName()).isEqualTo(testName + " updated"); + + long countWorkspaceWithAdditionalFieldAfterUpdate = mongoTemplate.count(queryWorkspaceWithAdditionalField, Workspace.class); + assertThat(countWorkspaceWithAdditionalFieldAfterUpdate).isEqualTo(1); + } }