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

chore: Remove BaseController on PluginControllerCE (#32996)

Browse Source 
Remove the `BaseController`, not used anymore, and have the Plugin
controller only define the routes used by the client.

Unit tests and All tag Cypress passes on EE.

[Slack
conversation](https://theappsmith.slack.com/archives/C03RPDB936Z/p1714194492847219).

/ok-to-test tags="@tag.Sanity"

<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/8857896676>
> Commit: be346ec2892fa70727e3b686c251c914ef998615
> Cypress dashboard url: <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=8857896676&attempt=1"
target="_blank">Click here!</a>

<!-- end of auto-generated comment: Cypress test results  -->
This commit is contained in:
Shrikant Sharat Kandula 2024-04-28 17:30:46 +05:30 committed by GitHub
parent 947add2f20
commit e4b6363712
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 38 additions and 214 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/BaseController.java
View File

@ -1,101 +0,0 @@
package com.appsmith.server.controllers.ce;
import com.appsmith.external.models.BaseDomain;
import com.appsmith.external.views.Views;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.dtos.ResponseDTO;
import com.appsmith.server.services.CrudService;
import com.fasterxml.jackson.annotation.JsonView;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.server.ServerWebExchange;
import reactor.core.publisher.Mono;
import java.util.List;
@RequiredArgsConstructor
@Slf4j
public abstract class BaseController<S extends CrudService<T, ID>, T extends BaseDomain, ID> {
protected final S service;
@JsonView(Views.Public.class)
@PostMapping
@ResponseStatus(HttpStatus.CREATED)
public Mono<ResponseDTO<T>> create(
@Valid @RequestBody T resource,
@RequestHeader(name = "Origin", required = false) String originHeader,
ServerWebExchange exchange) {
log.debug(
"Going to create resource from base controller {}",
resource.getClass().getName());
return service.create(resource).map(created -> new ResponseDTO<>(HttpStatus.CREATED.value(), created, null));
}
/**
* TODO : Remove this function completely if this is not being used.
* If not, atleast remove it for :
* 1. Page
* 2. Datasources
*
* @param params
* @return
*/
@JsonView(Views.Public.class)
@GetMapping("")
public Mono<ResponseDTO<List<T>>> getAll(
@RequestParam MultiValueMap<String, String> params,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to get all resources from base controller {}", params);
MultiValueMap<String, String> modifiableParams = new LinkedMultiValueMap<>(params);
if (!StringUtils.isEmpty(branchName)) {
modifiableParams.add(FieldName.DEFAULT_RESOURCES + "." + FieldName.BRANCH_NAME, branchName);
}
return service.get(modifiableParams)
.collectList()
.map(resources -> new ResponseDTO<>(HttpStatus.OK.value(), resources, null));
}
@JsonView(Views.Public.class)
@GetMapping("/{id}")
public Mono<ResponseDTO<T>> getByIdAndBranchName(
@PathVariable ID id, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to get resource from base controller for id: {}", id);
return service.findByIdAndBranchName(id, branchName)
.map(resources -> new ResponseDTO<>(HttpStatus.OK.value(), resources, null));
}
@JsonView(Views.Public.class)
@PutMapping("/{id}")
public Mono<ResponseDTO<T>> update(
@PathVariable ID id,
@RequestBody T resource,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to update resource from base controller with id: {}", id);
return service.update(id, resource)
.map(updatedResource -> new ResponseDTO<>(HttpStatus.OK.value(), updatedResource, null));
}
@JsonView(Views.Public.class)
@DeleteMapping("/{id}")
public Mono<ResponseDTO<T>> delete(
@PathVariable ID id, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to delete resource from base controller with id: {}", id);
return service.archiveByIdAndBranchName(id, branchName)
.map(deletedResource -> new ResponseDTO<>(HttpStatus.OK.value(), deletedResource, null));
}
}

37
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/PluginControllerCE.java
View File

@ -6,15 +6,12 @@ import com.appsmith.external.views.Views;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.constants.Url;
import com.appsmith.server.domains.Plugin;
import com.appsmith.server.domains.Workspace;
import com.appsmith.server.dtos.PluginWorkspaceDTO;
import com.appsmith.server.dtos.ResponseDTO;
import com.appsmith.server.plugins.base.PluginService;
import com.appsmith.server.plugins.solutions.PluginTriggerSolution;
import com.fasterxml.jackson.annotation.JsonView;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.codec.multipart.FilePart;
@ -24,8 +21,8 @@ import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RequestPart;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.server.ServerWebExchange;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@ -33,31 +30,21 @@ import reactor.core.publisher.Mono;
import java.util.List;
@RequestMapping(Url.PLUGIN_URL)
@RequiredArgsConstructor
@Slf4j
public class PluginControllerCE extends BaseController<PluginService, Plugin, String> {
public class PluginControllerCE {
protected final PluginService service;
private final PluginTriggerSolution pluginTriggerSolution;
@Autowired
public PluginControllerCE(PluginService service, PluginTriggerSolution pluginTriggerSolution) {
super(service);
this.pluginTriggerSolution = pluginTriggerSolution;
}
@JsonView(Views.Public.class)
@PostMapping("/install")
@ResponseStatus(HttpStatus.CREATED)
public Mono<ResponseDTO<Workspace>> install(@Valid @RequestBody PluginWorkspaceDTO plugin) {
return service.installPlugin(plugin)
.map(workspace -> new ResponseDTO<>(HttpStatus.CREATED.value(), workspace, null));
}
@JsonView(Views.Public.class)
@PostMapping("/uninstall")
@ResponseStatus(HttpStatus.CREATED)
public Mono<ResponseDTO<Workspace>> uninstall(@Valid @RequestBody PluginWorkspaceDTO plugin) {
return service.uninstallPlugin(plugin)
.map(workspace -> new ResponseDTO<>(HttpStatus.CREATED.value(), workspace, null));
@GetMapping
public Mono<ResponseDTO<List<Plugin>>> getAll(@RequestParam String workspaceId) {
log.debug("Getting all plugins in workspace {}", workspaceId);
return service.getInWorkspace(workspaceId)
.collectList()
.map(resources -> new ResponseDTO<>(HttpStatus.OK.value(), resources, null));
}
@JsonView(Views.Public.class)

4
app/server/appsmith-server/src/main/java/com/appsmith/server/plugins/base/PluginServiceCE.java
View File

@ -15,6 +15,8 @@ import java.util.Set;
public interface PluginServiceCE extends CrudService<Plugin, String> {
Flux<Plugin> getInWorkspace(String workspaceId);
Flux<Plugin> getDefaultPlugins();
Flux<Plugin> getDefaultPluginIcons();
@ -23,8 +25,6 @@ public interface PluginServiceCE extends CrudService<Plugin, String> {
Flux<Workspace> installDefaultPlugins(List<Plugin> plugins);
Mono<Workspace> uninstallPlugin(PluginWorkspaceDTO plugin);
Mono<Plugin> findByName(String name);
Mono<Plugin> findByPackageName(String packageName);

82
app/server/appsmith-server/src/main/java/com/appsmith/server/plugins/base/PluginServiceCEImpl.java
View File

@ -11,8 +11,6 @@ import com.appsmith.server.dtos.PluginWorkspaceDTO;
import com.appsmith.server.dtos.WorkspacePluginStatus;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.ce.bridge.Bridge;
import com.appsmith.server.helpers.ce.bridge.BridgeQuery;
import com.appsmith.server.repositories.PluginRepository;
import com.appsmith.server.services.AnalyticsService;
import com.appsmith.server.services.BaseService;
@ -25,6 +23,7 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import jakarta.validation.Validator;
import lombok.Data;
import lombok.NonNull;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.io.FileUtils;
import org.pf4j.PluginManager;
@ -33,7 +32,6 @@ import org.springframework.core.io.Resource;
import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
import org.springframework.data.redis.core.ReactiveRedisTemplate;
import org.springframework.data.redis.listener.ChannelTopic;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StreamUtils;
import org.springframework.util.StringUtils;
import reactor.core.Exceptions;
@ -75,8 +73,6 @@ public class PluginServiceCEImpl extends BaseService<PluginRepository, Plugin, S
private static final String UQI_QUERY_EDITOR_BASE_FOLDER = "editor";
private static final String UQI_QUERY_EDITOR_ROOT_FILE = "root.json";
private static final String BASE_UQI_URL =
"https://raw.githubusercontent.com/appsmithorg/uqi-configurations/master/";
private static final String KEY_EDITOR = "editor";
private static final String KEY_CONFIG_PROPERTY = "configProperty";
@ -107,43 +103,24 @@ public class PluginServiceCEImpl extends BaseService<PluginRepository, Plugin, S
}
@Override
public Flux<Plugin> get(MultiValueMap<String, String> params) {
// Remove branch name as plugins are not shared across branches
params.remove(FieldName.DEFAULT_RESOURCES + "." + FieldName.BRANCH_NAME);
String workspaceId = params.getFirst(FieldName.WORKSPACE_ID);
if (workspaceId == null) {
return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.WORKSPACE_ID));
}
public Flux<Plugin> getInWorkspace(@NonNull String workspaceId) {
// TODO : Think about the various scenarios where this plugin api is called and then decide on permissions.
Mono<Workspace> workspaceMono = workspaceService.getById(workspaceId);
return workspaceMono
.flatMapMany(org -> {
log.debug("Fetching plugins by params: {} for org: {}", params, org.getName());
if (org.getPlugins() == null) {
log.debug("Null installed plugins found for org: {}. Return empty plugins", org.getName());
.flatMapMany(workspace -> {
if (workspace.getPlugins() == null) {
log.debug(
"Null installed plugins found for workspace: {}. Return empty plugins",
workspace.getName());
return Flux.empty();
}
List<String> pluginIds = org.getPlugins().stream()
Set<String> pluginIds = workspace.getPlugins().stream()
.map(WorkspacePlugin::getPluginId)
.collect(Collectors.toList());
final BridgeQuery<Plugin> criteria = Bridge.in(FieldName.ID, pluginIds);
.collect(Collectors.toUnmodifiableSet());
final String typeString = params.getFirst(FieldName.TYPE);
if (typeString != null) {
try {
PluginType.valueOf(typeString); // Check if the type is valid
criteria.equal(FieldName.TYPE, typeString);
} catch (IllegalArgumentException e) {
log.error("No plugins for type : {}", typeString);
return Flux.empty();
}
}
return repository.queryBuilder().criteria(criteria).all();
return repository.findAllById(pluginIds);
})
.flatMap(plugin ->
getTemplates(plugin).doOnSuccess(plugin::setTemplates).thenReturn(plugin));
@ -204,36 +181,6 @@ public class PluginServiceCEImpl extends BaseService<PluginRepository, Plugin, S
});
}
@Override
public Mono<Workspace> uninstallPlugin(PluginWorkspaceDTO pluginDTO) {
if (pluginDTO.getPluginId() == null) {
return Mono.error(new AppsmithException(AppsmithError.PLUGIN_ID_NOT_GIVEN));
}
if (pluginDTO.getWorkspaceId() == null) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.WORKSPACE_ID));
}
// Find the workspace using id and plugin id -> This is to find if the workspace has the plugin installed
Mono<Workspace> workspaceMono =
workspaceService.findByIdAndPluginsPluginId(pluginDTO.getWorkspaceId(), pluginDTO.getPluginId());
return workspaceMono
.switchIfEmpty(
Mono.error(new AppsmithException(AppsmithError.PLUGIN_NOT_INSTALLED, pluginDTO.getPluginId())))
// In case the plugin is not found for the workspace, the workspaceMono would not emit and the rest of
// the flow would stop
// i.e. the rest of the code flow would only happen when there is a plugin found for the workspace that
// can
// be uninstalled.
.flatMap(workspace -> {
Set<WorkspacePlugin> workspacePluginList = workspace.getPlugins();
workspacePluginList.removeIf(
listPlugin -> listPlugin.getPluginId().equals(pluginDTO.getPluginId()));
workspace.setPlugins(workspacePluginList);
return workspaceService.save(workspace);
});
}
private Mono<Workspace> storeWorkspacePlugin(PluginWorkspaceDTO pluginDTO, WorkspacePluginStatus status) {
Mono<Workspace> pluginInWorkspaceMono =
@ -521,14 +468,7 @@ public class PluginServiceCEImpl extends BaseService<PluginRepository, Plugin, S
InputStream getConfigInputStream(Plugin plugin, String fileName) throws IOException {
String resourcePath = UQI_QUERY_EDITOR_BASE_FOLDER + "/" + fileName;
// if (Set.of(
// "google-sheets-plugin",
// "mongo-plugin",
// "amazons3-plugin",
// "firestore-plugin"
// ).contains(plugin.getPackageName())) {
// return new URL(BASE_UQI_URL + plugin.getPackageName() + "/editor/" + fileName).openStream();
// }
return pluginManager
.getPlugin(plugin.getPackageName())
.getPluginClassLoader()

2
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/PluginRepositoryCE.java
View File

@ -12,8 +12,6 @@ public interface PluginRepositoryCE extends BaseRepository<Plugin, String>, Cust
Mono<Plugin> findByPackageName(String packageName);
Mono<Plugin> findById(String id);
Flux<Plugin> findByDefaultInstall(Boolean isDefaultInstall);
Flux<Plugin> findByType(PluginType pluginType);

24
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConsolidatedAPIServiceImpl.java
View File

@ -142,9 +142,9 @@ public class ConsolidatedAPIServiceImpl implements ConsolidatedAPIService {
return new ResponseDTO<>(HttpStatus.OK.value(), data, null);
}
<T> Mono<ResponseDTO<T>> getErrorResponseMono(Throwable error, Class<T> type) {
private <T> Mono<ResponseDTO<T>> getErrorResponseMono(Throwable error) {
if (error instanceof AppsmithException appsmithException) {
return Mono.just(new ResponseDTO<T>(
return Mono.just(new ResponseDTO<>(
appsmithException.getHttpStatus(),
new ErrorDTO(
appsmithException.getAppErrorCode(),
@ -157,6 +157,11 @@ public class ConsolidatedAPIServiceImpl implements ConsolidatedAPIService {
INTERNAL_SERVER_ERROR_STATUS, new ErrorDTO(INTERNAL_SERVER_ERROR_CODE, error.getMessage())));
}
@Deprecated(forRemoval = true)
<T> Mono<ResponseDTO<T>> getErrorResponseMono(Throwable error, Class<T> type) {
return getErrorResponseMono(error);
}
public static String getQualifiedSpanName(String spanName, ApplicationMode mode) {
return ApplicationMode.PUBLISHED.equals(mode)
? CONSOLIDATED_API_PREFIX + VIEW + spanName
@ -450,17 +455,12 @@ public class ConsolidatedAPIServiceImpl implements ConsolidatedAPIService {
.cache();
/* Get all plugins in workspace */
Mono<ResponseDTO<List>> listOfPluginsResponseDTOMonoCache = workspaceIdMonoCache
.flatMap(workspaceId -> {
MultiValueMap<String, String> params = new LinkedMultiValueMap<>();
if (!EMPTY_WORKSPACE_ID_ON_ERROR.equals(workspaceId)) {
params.add(WORKSPACE_ID, workspaceId);
}
return pluginService.get(params).collectList();
})
.map(res -> (List) res)
Mono<ResponseDTO<List<Plugin>>> listOfPluginsResponseDTOMonoCache = workspaceIdMonoCache
.flatMap(workspaceId -> EMPTY_WORKSPACE_ID_ON_ERROR.equals(workspaceId)
? Mono.empty()
: pluginService.getInWorkspace(workspaceId).collectList())
.map(this::getSuccessResponse)
.onErrorResume(error -> getErrorResponseMono(error, List.class))
.onErrorResume(this::getErrorResponseMono)
.name(getQualifiedSpanName(PLUGINS_SPAN, mode))
.tap(Micrometer.observation(observationRegistry))
.cache();

2
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ConsolidatedAPIServiceImplTest.java
View File

@ -439,7 +439,7 @@ public class ConsolidatedAPIServiceImplTest {
sampleAiPlugin.setName("sampleAiPlugin");
sampleAiPlugin.setId("sampleAiPluginId");
sampleAiPlugin.setPackageName(APPSMITH_AI_PLUGIN);
when(mockPluginService.get(any()))
when(mockPluginService.getInWorkspace(anyString()))
.thenReturn(Flux.just(samplePlugin, sampleRestApiPlugin, sampleGraphqlPlugin, sampleAiPlugin));
Datasource sampleDatasource = new Datasource();