From a31796a2166eaa08d9d641c1d5de53630482cd0f Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Mon, 8 Jun 2020 06:45:04 +0000 Subject: [PATCH] Removed the usage of get current organization id. Requires addition of organization id to a few api calls. --- .../appsmith/server/dtos/PluginOrgDTO.java | 2 + .../server/services/ActionServiceImpl.java | 41 +++---- .../services/ApplicationPageService.java | 2 - .../services/ApplicationPageServiceImpl.java | 15 --- .../services/DatasourceServiceImpl.java | 11 +- .../server/services/LayoutServiceImpl.java | 2 - .../server/services/PluginServiceImpl.java | 106 +++++++++--------- .../server/services/UserServiceImpl.java | 2 +- .../resources/application-local.properties | 2 +- 9 files changed, 81 insertions(+), 102 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PluginOrgDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PluginOrgDTO.java index 69be1b4c88..84de0d6ee4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PluginOrgDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PluginOrgDTO.java @@ -9,5 +9,7 @@ public class PluginOrgDTO { String pluginId; + String organizationId; + OrganizationPluginStatus status; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index 51b4e05edd..e160b9d32c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -521,35 +521,24 @@ public class ActionServiceImpl extends BaseService orgIdMono = sessionUserService - .getCurrentUser() - .map(user -> user.getCurrentOrganizationId()); - if (params.getFirst(FieldName.APPLICATION_ID) != null) { - return orgIdMono - .flatMapMany(orgId -> pageService - .findNamesByApplicationId(params.getFirst(FieldName.APPLICATION_ID)) - .switchIfEmpty(Mono.error(new AppsmithException( - AppsmithError.NO_RESOURCE_FOUND, "pages for application", params.getFirst(FieldName.APPLICATION_ID))) - ) - .map(applicationPagesDTO -> applicationPagesDTO.getPages()) - .flatMapMany(Flux::fromIterable) - .map(pageNameIdDTO -> { - Action example = new Action(); - example.setPageId(pageNameIdDTO.getId()); - example.setOrganizationId(orgId); - return example; - }) - .flatMap(example -> repository.findAll(Example.of(example), sort)) - ) + return pageService + .findNamesByApplicationId(params.getFirst(FieldName.APPLICATION_ID)) + .switchIfEmpty(Mono.error(new AppsmithException( + AppsmithError.NO_RESOURCE_FOUND, "pages for application", params.getFirst(FieldName.APPLICATION_ID))) + ) + .map(applicationPagesDTO -> applicationPagesDTO.getPages()) + .flatMapMany(Flux::fromIterable) + .map(pageNameIdDTO -> { + Action example = new Action(); + example.setPageId(pageNameIdDTO.getId()); + return example; + }) + .flatMap(example -> repository.findAll(Example.of(example), sort)) .flatMap(this::setTransientFieldsInAction); } - return orgIdMono - .flatMapMany(orgId -> { - actionExample.setOrganizationId(orgId); - return repository.findAll(Example.of(actionExample), sort); - }) - .flatMap(this::setTransientFieldsInAction); + return repository.findAll(Example.of(actionExample), sort) + .flatMap(this::setTransientFieldsInAction); } private ActionConfiguration updateActionConfigurationForPagination(ActionConfiguration actionConfiguration, diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java index dae53f0ecc..5d988bd642 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java @@ -9,8 +9,6 @@ public interface ApplicationPageService { Mono addPageToApplication(Mono applicationMono, Page page, Boolean isDefault); - Mono doesPageBelongToCurrentUserOrganization(Page page); - Mono getPage(String pageId, Boolean viewMode); Mono createApplication(Application application, String orgId); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index 29b6c80d7c..d410496371 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -117,21 +117,6 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { .flatMap(applicationService::save); } - public Mono doesPageBelongToCurrentUserOrganization(Page page) { - Mono userMono = sessionUserService.getCurrentUser(); - final String[] username = {null}; - - return userMono - .map(user -> { - username[0] = user.getEmail(); - return user; - }) - .flatMap(user -> applicationService.findByIdAndOrganizationId(page.getApplicationId(), user.getCurrentOrganizationId())) - .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.PAGE_DOESNT_BELONG_TO_USER_ORGANIZATION, page.getId(), username[0]))) - //If mono transmits, then application id belongs to the current user's organization. Return page. - .then(Mono.just(page)); - } - public Mono getPage(String pageId, Boolean viewMode) { return pageService.findById(pageId, READ_PAGES) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID))) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceServiceImpl.java index babdbf7b29..080c249c40 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/DatasourceServiceImpl.java @@ -243,12 +243,17 @@ public class DatasourceServiceImpl extends BaseService(); } + @Override public Flux get(MultiValueMap params) { + /** + * Note : Currently this API is ONLY used to fetch datasources for an organization. + */ + if (params.getFirst(FieldName.ORGANIZATION_ID) != null) { + return repository.findAllByOrganizationId(params.getFirst(FieldName.ORGANIZATION_ID), AclPermission.READ_DATASOURCES); + } - return sessionUserService - .getCurrentUser() - .flatMapMany(user -> repository.findAllByOrganizationId(user.getCurrentOrganizationId(), AclPermission.READ_DATASOURCES)); + return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORGANIZATION_ID)); } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java index 13dec2c450..666e64ef67 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java @@ -75,8 +75,6 @@ public class LayoutServiceImpl implements LayoutService { public Mono getLayout(String pageId, String layoutId, Boolean viewMode) { return pageService.findByIdAndLayoutsId(pageId, layoutId) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID + " or " + FieldName.LAYOUT_ID))) - .flatMap(applicationPageService::doesPageBelongToCurrentUserOrganization) - //The pageId given is correct and belongs to the current user's organization. .map(page -> { List layoutList = page.getLayouts(); //Because the findByIdAndLayoutsId call returned non-empty result, we are guaranteed to find the layoutId here. diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java index faca5ba8bb..3a806c7805 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java @@ -5,7 +5,6 @@ import com.appsmith.server.domains.Organization; import com.appsmith.server.domains.OrganizationPlugin; import com.appsmith.server.domains.Plugin; import com.appsmith.server.domains.PluginType; -import com.appsmith.server.domains.User; import com.appsmith.server.dtos.InstallPluginRedisDTO; import com.appsmith.server.dtos.OrganizationPluginStatus; import com.appsmith.server.dtos.PluginOrgDTO; @@ -84,36 +83,40 @@ public class PluginServiceImpl extends BaseService get(MultiValueMap params) { - return sessionUserService.getCurrentUser() - .flatMapMany(user -> { - log.debug("Going to filter plugin params for user: {}", user.getEmail()); - return organizationService.findById(user.getCurrentOrganizationId()) - .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()); - return Flux.fromIterable(new ArrayList<>()); - } + String organizationId = params.getFirst(FieldName.ORGANIZATION_ID); + if (organizationId == null) { + return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORGANIZATION_ID)); + } - List pluginIds = org.getPlugins() - .stream() - .map(obj -> obj.getPluginId()) - .collect(Collectors.toList()); - Query query = new Query(); - query.addCriteria(Criteria.where(FieldName.ID).in(pluginIds)); + // TODO : Think about the various scenarios where this plugin api is called and then decide on permissions. + Mono organizationMono = organizationService.findById(organizationId); - if (params.getFirst(FieldName.TYPE) != null) { - try { - PluginType pluginType = PluginType.valueOf(params.getFirst(FieldName.TYPE)); - query.addCriteria(Criteria.where(FieldName.TYPE).is(pluginType)); - } catch (IllegalArgumentException e) { - log.error("No plugins for type : {}", params.getFirst(FieldName.TYPE)); - List emptyPlugins = new ArrayList<>(); - return Flux.fromIterable(emptyPlugins); - } - } - return mongoTemplate.find(query, Plugin.class); - }); + return organizationMono + .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()); + return Flux.fromIterable(new ArrayList<>()); + } + + List pluginIds = org.getPlugins() + .stream() + .map(obj -> obj.getPluginId()) + .collect(Collectors.toList()); + Query query = new Query(); + query.addCriteria(Criteria.where(FieldName.ID).in(pluginIds)); + + if (params.getFirst(FieldName.TYPE) != null) { + try { + PluginType pluginType = PluginType.valueOf(params.getFirst(FieldName.TYPE)); + query.addCriteria(Criteria.where(FieldName.TYPE).is(pluginType)); + } catch (IllegalArgumentException e) { + log.error("No plugins for type : {}", params.getFirst(FieldName.TYPE)); + List emptyPlugins = new ArrayList<>(); + return Flux.fromIterable(emptyPlugins); + } + } + return mongoTemplate.find(query, Plugin.class); }); } @@ -137,9 +140,11 @@ public class PluginServiceImpl extends BaseService storeOrganizationPlugin(plugin, pluginOrgDTO.getStatus())) + return storeOrganizationPlugin(pluginOrgDTO, pluginOrgDTO.getStatus()) .switchIfEmpty(Mono.empty()); } @@ -148,47 +153,46 @@ public class PluginServiceImpl extends BaseService This is to find if the organization has the plugin installed - Mono userMono = sessionUserService.getCurrentUser(); - Mono organizationMono = userMono.flatMap(user -> - organizationService.findByIdAndPluginsPluginId(user.getCurrentOrganizationId(), pluginDTO.getPluginId())); + Mono organizationMono = organizationService.findByIdAndPluginsPluginId(pluginDTO.getOrganizationId(), + pluginDTO.getPluginId()); return organizationMono .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.PLUGIN_NOT_INSTALLED, pluginDTO.getPluginId()))) //In case the plugin is not found for the organization, the organizationMono 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 organization that can //be uninstalled. - .map(organization -> { + .flatMap(organization -> { List organizationPluginList = organization.getPlugins(); organizationPluginList.removeIf(listPlugin -> listPlugin.getPluginId().equals(pluginDTO.getPluginId())); organization.setPlugins(organizationPluginList); - return organization; - }) - .flatMap(organizationService::save); + return organizationService.save(organization); + }); } private Mono storeOrganizationPlugin(PluginOrgDTO pluginDTO, OrganizationPluginStatus status) { - //Find the organization using id and plugin id -> This is to find if the organization already has the plugin installed - Mono userMono = sessionUserService.getCurrentUser(); - Mono pluginInOrganizationMono = userMono.flatMap(user -> - organizationService.findByIdAndPluginsPluginId(user.getCurrentOrganizationId(), pluginDTO.getPluginId())); + Mono pluginInOrganizationMono = organizationService + .findByIdAndPluginsPluginId(pluginDTO.getOrganizationId(), pluginDTO.getPluginId()); //If plugin is already present for the organization, just return the organization, else install and return organization return pluginInOrganizationMono .switchIfEmpty(Mono.defer(() -> { - log.debug("Plugin {} not already installed. Running the switch if empty code block", pluginDTO.getPluginId()); + log.debug("Plugin {} not already installed. Installing now", pluginDTO.getPluginId()); //If the plugin is not found in the organization, its not installed already. Install now. return repository .findById(pluginDTO.getPluginId()) - .zipWith(userMono, (plugin, user) -> { + .map(plugin -> { log.debug("Before publishing to the redis queue"); //Publish the event to the pub/sub queue InstallPluginRedisDTO installPluginRedisDTO = new InstallPluginRedisDTO(); - installPluginRedisDTO.setOrganizationId(user.getCurrentOrganizationId()); + installPluginRedisDTO.setOrganizationId(pluginDTO.getOrganizationId()); installPluginRedisDTO.setPluginOrgDTO(pluginDTO); String jsonString; try { @@ -202,13 +206,12 @@ public class PluginServiceImpl extends BaseService organizationService.findById(user.getCurrentOrganizationId())) - .map(organization -> { + .then(organizationService.findById(pluginDTO.getOrganizationId())) + .flatMap(organization -> { List organizationPluginList = organization.getPlugins(); if (organizationPluginList == null) { - organizationPluginList = new ArrayList(); + organizationPluginList = new ArrayList<>(); } OrganizationPlugin organizationPlugin = new OrganizationPlugin(); @@ -219,9 +222,8 @@ public class PluginServiceImpl extends BaseService i * platform. This flow also ensures that a personal workspace name is created for the user. The new user is then * given admin permissions to the personal workspace. *

- * For new user invite flow, please {@link UserOrganizationService#inviteUserNew(InviteUserDTO, String)} + * For new user invite flow, please {@link UserService#inviteUser(InviteUserDTO, String)} * * @param user * @return diff --git a/app/server/appsmith-server/src/main/resources/application-local.properties b/app/server/appsmith-server/src/main/resources/application-local.properties index bcd870414e..cf6825e552 100644 --- a/app/server/appsmith-server/src/main/resources/application-local.properties +++ b/app/server/appsmith-server/src/main/resources/application-local.properties @@ -38,5 +38,5 @@ spring.mail.host=email-smtp.us-east-1.amazonaws.com spring.mail.port=587 spring.mail.username=AKIAVWHAAGIQOHPT4BZ7 spring.mail.password=BEE5W6i7YznAJ/YDOLbppovmOlRzxXElJ+uJtGhdCfjY -spring.mail.properties.mail.smtp.auth=true +spring.mail.properties.mail.smtp.auth=false spring.mail.properties.mail.smtp.starttls.enable=true \ No newline at end of file