chore: Remove deprecated Action and Page classes (#30524)

These classes are deprecated and have been replaced with `NewAction` and
`NewPage`.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## 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.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Shrikant Sharat Kandula 2024-01-24 05:54:05 +05:30 committed by GitHub
parent 2cf98ffbb1
commit a0563b71e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 23 additions and 185 deletions

View File

@ -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);
}

View File

@ -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<ActionCollectionR
@Override
public void generateAndSetPolicies(NewPage page, ActionCollection actionCollection) {
Set<Policy> documentPolicies =
policyGenerator.getAllChildPolicies(page.getPolicies(), Page.class, Action.class);
policyGenerator.getAllChildPolicies(page.getPolicies(), NewPage.class, NewAction.class);
actionCollection.setPolicies(documentPolicies);
}

View File

@ -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<ApplicationRepository,
List<Mono<Void>> list = new ArrayList<>();
Map<String, Policy> pagePolicyMap = policySolution.generateInheritedPoliciesFromSourcePolicies(
applicationPolicyMap, Application.class, Page.class);
Map<String, Policy> actionPolicyMap =
policySolution.generateInheritedPoliciesFromSourcePolicies(pagePolicyMap, Page.class, Action.class);
applicationPolicyMap, Application.class, NewPage.class);
Map<String, Policy> actionPolicyMap = policySolution.generateInheritedPoliciesFromSourcePolicies(
pagePolicyMap, NewPage.class, NewAction.class);
Map<String, Policy> themePolicyMap = policySolution.generateInheritedPoliciesFromSourcePolicies(
applicationPolicyMap, Application.class, Theme.class);

View File

@ -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<Property> dynamicBindingPathList;
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@JsonView(Views.Public.class)
Boolean isValid;
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@JsonView(Views.Public.class)
Set<String> 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<String> 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;
}
}

View File

@ -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<Layout> layouts;
}

View File

@ -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<NewActionRepository, New
throw new AppsmithException(AppsmithError.INTERNAL_SERVER_ERROR, "No page found to copy policies from.");
}
Set<Policy> documentPolicies =
policyGenerator.getAllChildPolicies(page.getPolicies(), Page.class, Action.class);
policyGenerator.getAllChildPolicies(page.getPolicies(), NewPage.class, NewAction.class);
action.setPolicies(documentPolicies);
}

View File

@ -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<Ne
.is(null);
criteria.add(deletedCriterion);
}
layoutsIdKey = layoutsKey + "." + fieldName(QLayout.layout.id);
layoutsIdKey = layoutsKey + "." + FieldName.ID;
Criteria layoutCriterion = where(layoutsIdKey).is(layoutId);
criteria.add(layoutCriterion);

View File

@ -21,7 +21,6 @@ import com.appsmith.server.domains.GitApplicationMetadata;
import com.appsmith.server.domains.Layout;
import com.appsmith.server.domains.NewAction;
import com.appsmith.server.domains.NewPage;
import com.appsmith.server.domains.Page;
import com.appsmith.server.domains.QNewAction;
import com.appsmith.server.domains.Theme;
import com.appsmith.server.domains.User;
@ -518,7 +517,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE {
public void generateAndSetPagePolicies(Application application, PageDTO page) {
Set<Policy> documentPolicies =
policyGenerator.getAllChildPolicies(application.getPolicies(), Application.class, Page.class);
policyGenerator.getAllChildPolicies(application.getPolicies(), Application.class, NewPage.class);
page.setPolicies(documentPolicies);
}

View File

@ -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();
}