Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

chore: refactoring workspace creation and archive methods to facilitate environment creation in EE. (#20494)

Browse Source 
## Description

> In Enterprise edition, the environments will be created by default
when we create a new workspace.
However to facilitate those changes, and to keep code maintainable,
these snippets are neccesary.

> This PR has refactored workspace creation flow into separate methods
to
-  allow addition of resourceIds in `permissionGroup` permissions in EE.
-  added static method to reduce code repeatability

> This PR has changed some methods in workspaceServiceTest, those
methods were using same workspace object to create new workspaces when
called in a loop, however the workspace object used to create was
assigned id after first `workspaceService.create` call, the subsequent
create calls returned same workspace object, furthermore the linked
dependents were new each time causing problem.
As a fix, creating a new workspace object each time for create calls in
loop.

## TL;DR
> Refactor to facilitate the default environment creation PR in the EE 

Fixes https://github.com/appsmithorg/appsmith/issues/20790
> This issue is a facilitator PR for [default environments with
workspace PR](https://github.com/appsmithorg/appsmith-ee/pull/1027).

## Type of change
- Chore (housekeeping or task changes that don't impact user perception)

## How Has This Been Tested?
- Manual

## Checklist:
### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] 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
This commit is contained in:
Manish Kumar 2023-02-20 20:37:38 +05:30 committed by GitHub
parent 412179d1fc
commit 6282932c89
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 93 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PermissionUtils.java Normal file
View File

@ -0,0 +1,16 @@
package com.appsmith.server.helpers;
import com.appsmith.server.dtos.Permission;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
public class PermissionUtils {
public static Set<Permission> collateAllPermissions(Set<Permission>... permissionArgs) {
Set<Permission> permissions = new HashSet<>();
Arrays.stream(permissionArgs).peek(permissions::addAll).toList();
return permissions;
}
}

96
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java
View File

@ -46,6 +46,7 @@ import reactor.core.publisher.Mono;
import reactor.core.scheduler.Scheduler;
import jakarta.validation.Validator;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
@ -67,6 +68,7 @@ import static com.appsmith.server.constants.FieldName.WORKSPACE_DEVELOPER_DESCRI
import static com.appsmith.server.constants.FieldName.WORKSPACE_VIEWER_DESCRIPTION;
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 java.lang.Boolean.TRUE;
@Slf4j
@ -81,7 +83,7 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
private final AssetRepository assetRepository;
private final AssetService assetService;
private final ApplicationRepository applicationRepository;
private final PermissionGroupService permissionGroupService;
protected final PermissionGroupService permissionGroupService;
private final PolicyUtils policyUtils;
private final ModelMapper modelMapper;
private final WorkspacePermission workspacePermission;
@ -204,33 +206,41 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
}))
// Save the workspace in the db
.flatMap(repository::save)
// Generate the default permission groups & policy for the current user
.flatMap(createdWorkspace1 -> {
return Mono.zip(generateDefaultPermissionGroups(createdWorkspace1, user), Mono.just(createdWorkspace1))
.flatMap(tuple -> {
Set<PermissionGroup> permissionGroups = tuple.getT1();
Workspace createdWorkspace = tuple.getT2();
createdWorkspace.setDefaultPermissionGroups(
permissionGroups.stream()
.map(PermissionGroup::getId)
.collect(Collectors.toSet())
);
// Apply the permissions to the workspace
for (PermissionGroup permissionGroup : permissionGroups) {
Map<String, Policy> policyMap = policyUtils.generatePolicyFromPermissionGroupForObject(permissionGroup, createdWorkspace.getId());
createdWorkspace = policyUtils.addPoliciesToExistingObject(policyMap, createdWorkspace);
}
return repository.save(createdWorkspace);
});
})
.flatMap(createdWorkspace -> enrichWorkspaceWithDependents(createdWorkspace, user))
.flatMap(analyticsService::sendCreateEvent);
}
protected Mono<Workspace> enrichWorkspaceWithDependents(Workspace createdWorkspace, User user) {
// Generate the default permission groups & policy for the current user
return generateDefaultPermissionGroups(createdWorkspace, user)
.flatMap(permissionGroups -> enrichDependents(permissionGroups,createdWorkspace)
.then(addPoliciesAndSaveWorkspace(permissionGroups, createdWorkspace)));
}
/**
* returns the created workspace without any Operations
* See EE overrides for complete usage
* @param permissionGroups
* @param createdWorkspace
* @return Mono of the createdWorkspace
*/
protected Mono<Workspace> enrichDependents(Set<PermissionGroup> permissionGroups, Workspace createdWorkspace) {
return Mono.just(createdWorkspace);
}
protected Mono<Workspace> addPoliciesAndSaveWorkspace(Set<PermissionGroup> permissionGroups, Workspace createdWorkspace) {
createdWorkspace.setDefaultPermissionGroups(
permissionGroups.stream()
.map(PermissionGroup::getId)
.collect(Collectors.toSet()));
// Apply the permissions to the workspace
for (PermissionGroup permissionGroup : permissionGroups) {
Map<String, Policy> policyMap = policyUtils.generatePolicyFromPermissionGroupForObject(permissionGroup, createdWorkspace.getId());
createdWorkspace = policyUtils.addPoliciesToExistingObject(policyMap, createdWorkspace);
}
return repository.save(createdWorkspace);
}
protected Mono<Boolean> isCreateWorkspaceAllowed(Boolean isDefaultWorkspace) {
return Mono.just(TRUE);
}
@ -285,7 +295,8 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
.collect(Collectors.toSet());
}
Mono<Set<PermissionGroup>> generatePermissionsForDefaultPermissionGroups(Set<PermissionGroup> permissionGroups, Workspace workspace, User user) {
Mono<Set<PermissionGroup>> generatePermissionsForDefaultPermissionGroups(Set<PermissionGroup> permissionGroups,
Workspace workspace, User user) {
PermissionGroup adminPermissionGroup = permissionGroups.stream()
.filter(permissionGroup -> permissionGroup.getName().startsWith(ADMINISTRATOR))
.findFirst().get();
@ -316,11 +327,10 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
.collect(Collectors.toSet());
Set<Permission> permissions = new HashSet<>();
permissions.addAll(workspacePermissions);
permissions.addAll(assignPermissionGroupPermissions);
permissions.addAll(readPermissionGroupPermissions);
permissions.addAll(unassignPermissionGroupPermissions);
Set<Permission> permissions = collateAllPermissions(workspacePermissions,
assignPermissionGroupPermissions,
readPermissionGroupPermissions,
unassignPermissionGroupPermissions);
adminPermissionGroup.setPermissions(permissions);
// Assign the user creating the permission group to this permission group
@ -337,10 +347,9 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
assignPermissionGroupPermissions = Set.of(developerPermissionGroup, viewerPermissionGroup).stream()
.map(permissionGroup -> new Permission(permissionGroup.getId(), ASSIGN_PERMISSION_GROUPS))
.collect(Collectors.toSet());
permissions = new HashSet<>();
permissions.addAll(workspacePermissions);
permissions.addAll(assignPermissionGroupPermissions);
permissions.addAll(readPermissionGroupPermissions);
permissions = collateAllPermissions(workspacePermissions,
assignPermissionGroupPermissions,
readPermissionGroupPermissions);
developerPermissionGroup.setPermissions(permissions);
// App Viewer Permissions
@ -354,10 +363,10 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
assignPermissionGroupPermissions = Set.of(viewerPermissionGroup).stream()
.map(permissionGroup -> new Permission(permissionGroup.getId(), ASSIGN_PERMISSION_GROUPS))
.collect(Collectors.toSet());
permissions = new HashSet<>();
permissions.addAll(workspacePermissions);
permissions.addAll(assignPermissionGroupPermissions);
permissions.addAll(readPermissionGroupPermissions);
permissions = collateAllPermissions(workspacePermissions,
assignPermissionGroupPermissions,
readPermissionGroupPermissions);
viewerPermissionGroup.setPermissions(permissions);
Mono<Set<PermissionGroup>> savedPermissionGroupsMono = Flux.fromIterable(permissionGroups)
@ -385,15 +394,16 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
return Mono.zip(
savedPermissionGroupsMono,
cleanPermissionGroupCacheForCurrentUser
)
)
.map(tuple -> tuple.getT1());
}
private Mono<Set<PermissionGroup>> generateDefaultPermissionGroups(Workspace workspace, User user) {
protected Mono<Set<PermissionGroup>> generateDefaultPermissionGroups(Workspace workspace, User user) {
return generateDefaultPermissionGroupsWithoutPermissions(workspace)
// Generate the permissions per permission group
.flatMap(permissionGroups -> generatePermissionsForDefaultPermissionGroups(permissionGroups, workspace, user));
.flatMap(permissionGroups -> generatePermissionsForDefaultPermissionGroups(permissionGroups, workspace,
user));
}
/**
@ -531,7 +541,7 @@ public class WorkspaceServiceCEImpl extends BaseService<WorkspaceRepository, Wor
.collect(Collectors.toList());
});
return permissionGroupInfoDTOListMono;
return permissionGroupInfoDTOListMono;
}
@Override

52
app/server/appsmith-server/src/test/java/com/appsmith/server/services/WorkspaceServiceTest.java
View File

@ -529,15 +529,14 @@ public class WorkspaceServiceTest {
.users(Set.of("api_user"))
.build();
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> createWorkspace = workspaceService.create(workspace);
String[] validEmails = {"valid@email.com", "valid@email.co.in", "valid@email-assoc.co.in"};
for (String validEmail: validEmails) {
Mono<Workspace> updateWorkspace = createWorkspace
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> updateWorkspace = workspaceService.create(workspace)
.flatMap(t -> {
Workspace newWorkspace = new Workspace();
newWorkspace.setEmail(validEmail);
@ -563,15 +562,14 @@ public class WorkspaceServiceTest {
.users(Set.of("api_user"))
.build();
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> createWorkspace = workspaceService.create(workspace);
String[] invalidEmails = {"invalid@.com", "@invalid.com"};
for (String invalidEmail : invalidEmails) {
Mono<Workspace> updateWorkspace = createWorkspace
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> updateWorkspace = workspaceService.create(workspace)
.flatMap(t -> {
Workspace newWorkspace = new Workspace();
newWorkspace.setEmail(invalidEmail);
@ -595,19 +593,18 @@ public class WorkspaceServiceTest {
.users(Set.of("api_user"))
.build();
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> createWorkspace = workspaceService.create(workspace);
String[] validWebsites = {"https://www.valid.website.com", "http://www.valid.website.com",
"https://valid.website.com", "http://valid.website.com", "www.valid.website.com", "valid.website.com",
"valid-website.com", "valid.12345.com", "12345.com", "https://www.valid.website.com/",
"http://www.valid.website.com/", "https://valid.website.complete/", "http://valid.website.com/",
"www.valid.website.com/", "valid.website.com/", "valid-website.com/", "valid.12345.com/", "12345.com/"};
for (String validWebsite: validWebsites) {
Mono<Workspace> updateWorkspace = createWorkspace
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> updateWorkspace = workspaceService.create(workspace)
.flatMap(t -> {
Workspace newWorkspace = new Workspace();
newWorkspace.setWebsite(validWebsite);
@ -633,16 +630,15 @@ public class WorkspaceServiceTest {
.users(Set.of("api_user"))
.build();
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> createWorkspace = workspaceService.create(workspace);
String[] invalidWebsites = {"htp://www.invalid.website.com", "htp://invalid.website.com", "htp://www", "www",
"www."};
for (String invalidWebsite : invalidWebsites) {
Mono<Workspace> updateWorkspace = createWorkspace
Workspace workspace = new Workspace();
workspace.setName("Test Update Name");
workspace.setDomain("example.com");
workspace.setWebsite("https://example.com");
workspace.setSlug("test-update-name");
Mono<Workspace> updateWorkspace = workspaceService.create(workspace)
.flatMap(t -> {
Workspace newWorkspace = new Workspace();
newWorkspace.setWebsite(invalidWebsite);