From a0563b71e12116fd755d265c7e74abb7b2ef9b8f Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 24 Jan 2024 05:54:05 +0530 Subject: [PATCH] chore: Remove deprecated `Action` and `Page` classes (#30524) These classes are deprecated and have been replaced with `NewAction` and `NewPage`. ## Summary by CodeRabbit - **Refactor** - Updated permissions handling to support new page and action classes. - Replaced deprecated page and action classes with their new counterparts in various service implementations and policy generation methods. - **Tests** - Adjusted test cases to align with the updated permissions and entity references. --- .../appsmith/server/acl/AclPermission.java | 33 ++--- .../base/ActionCollectionServiceCEImpl.java | 4 +- .../base/ApplicationServiceCEImpl.java | 10 +- .../com/appsmith/server/domains/Action.java | 117 ------------------ .../com/appsmith/server/domains/Page.java | 25 ---- .../base/NewActionServiceCEImpl.java | 4 +- .../ce/CustomNewPageRepositoryCEImpl.java | 3 +- .../ce/ApplicationPageServiceCEImpl.java | 3 +- .../server/acl/AclPermissionTest.java | 9 +- 9 files changed, 23 insertions(+), 185 deletions(-) delete mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java delete mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Page.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java index 4716fdead0..6316cfad15 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/acl/AclPermission.java @@ -2,13 +2,11 @@ package com.appsmith.server.acl; import com.appsmith.external.models.BaseDomain; import com.appsmith.external.models.Datasource; -import com.appsmith.server.domains.Action; import com.appsmith.server.domains.ActionCollection; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.Config; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; -import com.appsmith.server.domains.Page; import com.appsmith.server.domains.PermissionGroup; import com.appsmith.server.domains.Tenant; import com.appsmith.server.domains.Theme; @@ -87,16 +85,16 @@ public enum AclPermission { APPLICATION_CREATE_PAGES("create:pages", Application.class), - MANAGE_PAGES("manage:pages", Page.class), - READ_PAGES("read:pages", Page.class), - DELETE_PAGES("delete:pages", Page.class), + MANAGE_PAGES("manage:pages", NewPage.class), + READ_PAGES("read:pages", NewPage.class), + DELETE_PAGES("delete:pages", NewPage.class), - PAGE_CREATE_PAGE_ACTIONS("create:pageActions", Page.class), + PAGE_CREATE_PAGE_ACTIONS("create:pageActions", NewPage.class), - MANAGE_ACTIONS("manage:actions", Action.class), - READ_ACTIONS("read:actions", Action.class), - EXECUTE_ACTIONS("execute:actions", Action.class), - DELETE_ACTIONS("delete:actions", Action.class), + MANAGE_ACTIONS("manage:actions", NewAction.class), + READ_ACTIONS("read:actions", NewAction.class), + EXECUTE_ACTIONS("execute:actions", NewAction.class), + DELETE_ACTIONS("delete:actions", NewAction.class), MANAGE_DATASOURCES("manage:datasources", Datasource.class), READ_DATASOURCES("read:datasources", Datasource.class), @@ -142,19 +140,14 @@ public enum AclPermission { return null; } - public static boolean isPermissionForEntity(AclPermission aclPermission, Class clazz) { - Class entityClass = clazz; + public static boolean isPermissionForEntity(AclPermission aclPermission, Class clazz) { + Class entityClass = clazz; /* - * Action class has been deprecated, and we have started using NewAction class instead. - * Page class has been deprecated, and we have started using NewPage class instead. * NewAction and ActionCollection are similar entities w.r.t. AclPermissions. - * Hence, whenever we want to check for any Permission w.r.t. NewAction or Action Collection, we use Action, and - * whenever we want to check for any Permission w.r.t. NewPage, we use Page. + * Hence, whenever we want to check for any Permission w.r.t. ActionCollection, we use NewAction. */ - if (entityClass.equals(NewAction.class) || entityClass.equals(ActionCollection.class)) { - entityClass = Action.class; - } else if (entityClass.equals(NewPage.class)) { - entityClass = Page.class; + if (entityClass.equals(ActionCollection.class)) { + entityClass = NewAction.class; } return aclPermission.getEntity().equals(entityClass); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java index dc3b49ac45..cf54c2866e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java @@ -10,11 +10,9 @@ import com.appsmith.server.acl.PolicyGenerator; import com.appsmith.server.applications.base.ApplicationService; import com.appsmith.server.constants.FieldName; import com.appsmith.server.defaultresources.DefaultResourcesService; -import com.appsmith.server.domains.Action; import com.appsmith.server.domains.ActionCollection; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; -import com.appsmith.server.domains.Page; import com.appsmith.server.dtos.ActionCollectionDTO; import com.appsmith.server.dtos.ActionCollectionViewDTO; import com.appsmith.server.exceptions.AppsmithError; @@ -119,7 +117,7 @@ public class ActionCollectionServiceCEImpl extends BaseService documentPolicies = - policyGenerator.getAllChildPolicies(page.getPolicies(), Page.class, Action.class); + policyGenerator.getAllChildPolicies(page.getPolicies(), NewPage.class, NewAction.class); actionCollection.setPolicies(documentPolicies); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java index 349becb4e0..83120b506f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java @@ -7,14 +7,14 @@ import com.appsmith.server.acl.AclPermission; import com.appsmith.server.constants.ApplicationConstants; import com.appsmith.server.constants.Assets; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.domains.Action; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.ApplicationDetail; import com.appsmith.server.domains.ApplicationMode; import com.appsmith.server.domains.Asset; import com.appsmith.server.domains.GitApplicationMetadata; import com.appsmith.server.domains.GitAuth; -import com.appsmith.server.domains.Page; +import com.appsmith.server.domains.NewAction; +import com.appsmith.server.domains.NewPage; import com.appsmith.server.domains.QApplication; import com.appsmith.server.domains.Theme; import com.appsmith.server.domains.UserData; @@ -677,9 +677,9 @@ public class ApplicationServiceCEImpl extends BaseService> list = new ArrayList<>(); Map pagePolicyMap = policySolution.generateInheritedPoliciesFromSourcePolicies( - applicationPolicyMap, Application.class, Page.class); - Map actionPolicyMap = - policySolution.generateInheritedPoliciesFromSourcePolicies(pagePolicyMap, Page.class, Action.class); + applicationPolicyMap, Application.class, NewPage.class); + Map actionPolicyMap = policySolution.generateInheritedPoliciesFromSourcePolicies( + pagePolicyMap, NewPage.class, NewAction.class); Map themePolicyMap = policySolution.generateInheritedPoliciesFromSourcePolicies( applicationPolicyMap, Application.class, Theme.class); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java deleted file mode 100644 index 44c197ad4f..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java +++ /dev/null @@ -1,117 +0,0 @@ -package com.appsmith.server.domains; - -import com.appsmith.external.models.ActionConfiguration; -import com.appsmith.external.models.BaseDomain; -import com.appsmith.external.models.Datasource; -import com.appsmith.external.models.Documentation; -import com.appsmith.external.models.PluginType; -import com.appsmith.external.models.Property; -import com.appsmith.external.views.Views; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonView; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; -import lombok.ToString; -import org.springframework.data.annotation.Transient; -import org.springframework.data.mongodb.core.mapping.Document; - -import java.util.List; -import java.util.Set; - -@Getter -@Setter -@ToString -@NoArgsConstructor -@Document -@Deprecated -public class Action extends BaseDomain { - - @JsonView(Views.Public.class) - String name; - - @JsonView(Views.Public.class) - Datasource datasource; - - // Organizations migrated to workspaces, kept the field as depricated to support the old migration - @Deprecated - @JsonView(Views.Public.class) - String organizationId; - - @JsonView(Views.Public.class) - String workspaceId; - - @JsonView(Views.Public.class) - String pageId; - - @JsonView(Views.Public.class) - String collectionId; - - @JsonView(Views.Public.class) - ActionConfiguration actionConfiguration; - - @JsonView(Views.Public.class) - PluginType pluginType; - - @JsonView(Views.Public.class) - Boolean executeOnLoad; - - /* - * This is a list of fields specified by the client to signify which fields have dynamic bindings in them. - * TODO: The server can use this field to simplify our Mustache substitutions in the future - */ - @JsonView(Views.Public.class) - List dynamicBindingPathList; - - @JsonProperty(access = JsonProperty.Access.READ_ONLY) - @JsonView(Views.Public.class) - Boolean isValid; - - @JsonProperty(access = JsonProperty.Access.READ_ONLY) - @JsonView(Views.Public.class) - Set invalids; - - // This is a list of keys that the client whose values the client needs to send during action execution. - // These are the Mustache keys that the server will replace before invoking the API - @JsonProperty(access = JsonProperty.Access.READ_ONLY) - @JsonView(Views.Public.class) - Set jsonPathKeys; - - @JsonView(Views.Internal.class) - String cacheResponse; - - @JsonView(Views.Public.class) - String templateId; // If action is created via a template, store the id here. - - @JsonView(Views.Public.class) - String providerId; // If action is created via a template, store the template's provider id here. - - @Transient - @JsonView(Views.Public.class) - String pluginId; - - @JsonView(Views.Internal.class) - Boolean userSetOnLoad = false; - - @JsonView(Views.Public.class) - Boolean confirmBeforeExecute = false; - - @JsonView(Views.Public.class) - Documentation documentation; - - /** - * If the Datasource is null, create one and set the autoGenerated flag to true. This is required because spring-data - * cannot add the createdAt and updatedAt properties for null embedded objects. At this juncture, we couldn't find - * a way to disable the auditing for nested objects. - * - * @return - */ - @JsonView(Views.Public.class) - public Datasource getDatasource() { - if (this.datasource == null) { - this.datasource = new Datasource(); - this.datasource.setIsAutoGenerated(true); - } - return datasource; - } -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Page.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Page.java deleted file mode 100644 index 833fcd3306..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Page.java +++ /dev/null @@ -1,25 +0,0 @@ -package com.appsmith.server.domains; - -import com.appsmith.external.models.BaseDomain; -import jakarta.validation.constraints.NotNull; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; -import lombok.ToString; -import org.springframework.data.mongodb.core.mapping.Document; - -import java.util.List; - -@Getter -@Setter -@ToString -@NoArgsConstructor -@Document -@Deprecated -public class Page extends BaseDomain { - String name; - - @NotNull String applicationId; - - List layouts; -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java index 7057acec1d..18cf8bfee4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java @@ -22,14 +22,12 @@ import com.appsmith.server.applications.base.ApplicationService; import com.appsmith.server.constants.FieldName; import com.appsmith.server.datasources.base.DatasourceService; import com.appsmith.server.defaultresources.DefaultResourcesService; -import com.appsmith.server.domains.Action; import com.appsmith.server.domains.ActionCollection; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.ApplicationMode; import com.appsmith.server.domains.DatasourceContext; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; -import com.appsmith.server.domains.Page; import com.appsmith.server.domains.Plugin; import com.appsmith.server.dtos.ActionViewDTO; import com.appsmith.server.dtos.ImportActionCollectionResultDTO; @@ -266,7 +264,7 @@ public class NewActionServiceCEImpl extends BaseService documentPolicies = - policyGenerator.getAllChildPolicies(page.getPolicies(), Page.class, Action.class); + policyGenerator.getAllChildPolicies(page.getPolicies(), NewPage.class, NewAction.class); action.setPolicies(documentPolicies); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java index f0cb49c011..651f2cecd8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java @@ -4,7 +4,6 @@ import com.appsmith.external.models.QBranchAwareDomain; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.NewPage; -import com.appsmith.server.domains.QLayout; import com.appsmith.server.domains.QNewPage; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; @@ -96,7 +95,7 @@ public class CustomNewPageRepositoryCEImpl extends BaseAppsmithRepositoryImpl documentPolicies = - policyGenerator.getAllChildPolicies(application.getPolicies(), Application.class, Page.class); + policyGenerator.getAllChildPolicies(application.getPolicies(), Application.class, NewPage.class); page.setPolicies(documentPolicies); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/acl/AclPermissionTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/acl/AclPermissionTest.java index 56a6db44c2..9a405297ec 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/acl/AclPermissionTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/acl/AclPermissionTest.java @@ -1,11 +1,9 @@ package com.appsmith.server.acl; -import com.appsmith.server.domains.Action; import com.appsmith.server.domains.ActionCollection; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; -import com.appsmith.server.domains.Page; import com.appsmith.server.domains.Theme; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -23,18 +21,13 @@ class AclPermissionTest { assertThat(AclPermission.isPermissionForEntity(AclPermission.READ_APPLICATIONS, Theme.class)) .isFalse(); - // Assert that Action related Permission should return True, when checked against Action, NewAction and Action + // Assert that NewAction related Permission should return True, when checked against NewAction and Action // Collection. - assertThat(AclPermission.isPermissionForEntity(AclPermission.MANAGE_ACTIONS, Action.class)) - .isTrue(); assertThat(AclPermission.isPermissionForEntity(AclPermission.MANAGE_ACTIONS, NewAction.class)) .isTrue(); assertThat(AclPermission.isPermissionForEntity(AclPermission.MANAGE_ACTIONS, ActionCollection.class)) .isTrue(); - // Assert that Page related Permission should return True, when checked against Page and NewPage. - assertThat(AclPermission.isPermissionForEntity(AclPermission.MANAGE_PAGES, Page.class)) - .isTrue(); assertThat(AclPermission.isPermissionForEntity(AclPermission.MANAGE_PAGES, NewPage.class)) .isTrue(); }