Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Google sheet shared drive support added behind a flag #37776

Merged
merged 6 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public enum FeatureFlagEnum {
APP_NAVIGATION_LOGO_UPLOAD,
release_embed_hide_share_settings_enabled,
rollout_datasource_test_rate_limit_enabled,
release_google_sheets_shared_drive_support_enabled,

// Deprecated CE flags over here
release_git_autocommit_feature_enabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,49 @@ default Mono<ActionExecutionResult> executeParameterizedWithMetrics(
.tap(Micrometer.observation(observationRegistry));
}

// Following methods of executeParameterizedWithFlags, executeParameterizedWithMetricsAndFlags, triggerWithFlags are
// added
// to support feature flags in the plugin modules. Current implementation of featureFlagService is only available in
// server module
// and not available in any of the plugin modules due to dependencies on SessionUserService, TenantService etc.
// Hence, these methods are added to support feature flags in the plugin modules.
// Ideal solution would be to move featureFlagService and its dependencies to the shared interface module
// But this is a bigger change and will be done in future. Current change of passing flags was done to resolve
// release blocker
// https://github.com/appsmithorg/appsmith/issues/37714
// Once thorogh testing of shared drive support is done, we can remove this tech debt of passing feature flags like
// this.
default Mono<ActionExecutionResult> executeParameterizedWithFlags(
C connection,
ExecuteActionDTO executeActionDTO,
DatasourceConfiguration datasourceConfiguration,
ActionConfiguration actionConfiguration,
Map<String, Boolean> featureFlagMap) {
return this.executeParameterized(connection, executeActionDTO, datasourceConfiguration, actionConfiguration);
}

default Mono<ActionExecutionResult> executeParameterizedWithMetricsAndFlags(
C connection,
ExecuteActionDTO executeActionDTO,
DatasourceConfiguration datasourceConfiguration,
ActionConfiguration actionConfiguration,
ObservationRegistry observationRegistry,
Map<String, Boolean> featureFlagMap) {
return this.executeParameterizedWithFlags(
connection, executeActionDTO, datasourceConfiguration, actionConfiguration, featureFlagMap)
.tag("plugin", this.getClass().getName())
.name(ACTION_EXECUTION_PLUGIN_EXECUTION)
.tap(Micrometer.observation(observationRegistry));
}

default Mono<TriggerResultDTO> triggerWithFlags(
C connection,
DatasourceConfiguration datasourceConfiguration,
TriggerRequestDTO request,
Map<String, Boolean> featureFlagMap) {
return this.trigger(connection, datasourceConfiguration, request);
}

/**
* This function is responsible for preparing the action and datasource configurations to be ready for execution.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ default Mono<Object> executePrerequisites(MethodConfig methodConfig, OAuth2 oaut
return Mono.just(true);
}

WebClient.RequestHeadersSpec<?> getExecutionClient(WebClient webClient, MethodConfig methodConfig);
default WebClient.RequestHeadersSpec<?> getExecutionClient(WebClient webClient, MethodConfig methodConfig) {
return null;
}
sneha122 marked this conversation as resolved.
Show resolved Hide resolved

default JsonNode transformExecutionResponse(
JsonNode response, MethodConfig methodConfig, Set<String> userAuthorizedSheetIds) {
Expand All @@ -102,4 +104,9 @@ default Map<DataType, DataType> getDataTypeConversionMap() {
conversionMap.put(DataType.FLOAT, DataType.DOUBLE);
return conversionMap;
}

default WebClient.RequestHeadersSpec<?> getExecutionClientWithFlags(
WebClient webClient, MethodConfig methodConfig, Map<String, Boolean> featureFlagMap) {
return getExecutionClient(webClient, methodConfig);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.external.config;

import com.appsmith.external.enums.FeatureFlagEnum;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import com.external.constants.ErrorMessages;
import com.external.enums.GoogleSheetMethodEnum;
Expand All @@ -24,8 +25,9 @@
* API reference: https://developers.google.com/sheets/api/guides/migration#list_spreadsheets_for_the_authenticated_user
*/
public class FileListMethod implements ExecutionMethod, TriggerMethod {

ObjectMapper objectMapper;
private final String SHARED_DRIVE_PARAMS =
"&includeItemsFromAllDrives=true&supportsAllDrives=true&corpora=allDrives";

public FileListMethod(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
Expand All @@ -37,10 +39,16 @@ public boolean validateExecutionMethodRequest(MethodConfig methodConfig) {
}

@Override
public WebClient.RequestHeadersSpec<?> getExecutionClient(WebClient webClient, MethodConfig methodConfig) {
public WebClient.RequestHeadersSpec<?> getExecutionClientWithFlags(
WebClient webClient, MethodConfig methodConfig, Map<String, Boolean> featureFlagMap) {
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
// Flags are needed here for google sheets integration to support shared drive behind a flag
// Once thoroughly tested, this flag can be removed
Boolean isSharedDriveSupportEnabled = featureFlagMap.getOrDefault(
FeatureFlagEnum.release_google_sheets_shared_drive_support_enabled.toString(), Boolean.FALSE);
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
UriComponentsBuilder uriBuilder = getBaseUriBuilder(
this.BASE_DRIVE_API_URL,
"?includeItemsFromAllDrives=true&supportsAllDrives=true&orderBy=name&q=mimeType%3D'application%2Fvnd.google-apps.spreadsheet'%20and%20trashed%3Dfalse",
"?orderBy=name&q=mimeType%3D'application%2Fvnd.google-apps.spreadsheet'%20and%20trashed%3Dfalse"
+ (isSharedDriveSupportEnabled.equals(Boolean.TRUE) ? SHARED_DRIVE_PARAMS : ""),
true);

return webClient
Expand Down Expand Up @@ -74,8 +82,9 @@ public boolean validateTriggerMethodRequest(MethodConfig methodConfig) {
}

@Override
public WebClient.RequestHeadersSpec<?> getTriggerClient(WebClient webClient, MethodConfig methodConfig) {
return this.getExecutionClient(webClient, methodConfig);
public WebClient.RequestHeadersSpec<?> getTriggerClientWithFlags(
WebClient webClient, MethodConfig methodConfig, Map<String, Boolean> featureFlagMap) {
return this.getExecutionClientWithFlags(webClient, methodConfig, featureFlagMap);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.databind.JsonNode;
import org.springframework.web.reactive.function.client.WebClient;

import java.util.Map;
import java.util.Set;

/**
Expand All @@ -21,7 +22,17 @@ public interface TriggerMethod {
/**
* Returns with the specification required to hit that particular trigger request
*/
WebClient.RequestHeadersSpec<?> getTriggerClient(WebClient webClient, MethodConfig methodConfig);
default WebClient.RequestHeadersSpec<?> getTriggerClient(WebClient webClient, MethodConfig methodConfig) {
abhvsn marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

/**
* Returns with the specification required to hit that particular trigger request
*/
default WebClient.RequestHeadersSpec<?> getTriggerClientWithFlags(
WebClient webClient, MethodConfig methodConfig, Map<String, Boolean> featureFlagMap) {
return getTriggerClient(webClient, methodConfig);
}

/**
* Transforms the response from the end point into an Appsmith friendly structure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@ public Mono<ActionExecutionResult> executeParameterized(
ExecuteActionDTO executeActionDTO,
DatasourceConfiguration datasourceConfiguration,
ActionConfiguration actionConfiguration) {
return executeParameterizedWithFlags(
connection, executeActionDTO, datasourceConfiguration, actionConfiguration, null);
}

@Override
public Mono<TriggerResultDTO> trigger(
Void connection, DatasourceConfiguration datasourceConfiguration, TriggerRequestDTO request) {
return triggerWithFlags(connection, datasourceConfiguration, request, null);
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public Mono<ActionExecutionResult> executeParameterizedWithFlags(
Void connection,
ExecuteActionDTO executeActionDTO,
DatasourceConfiguration datasourceConfiguration,
ActionConfiguration actionConfiguration,
Map<String, Boolean> featureFlagMap) {

log.debug(Thread.currentThread().getName() + ": executeParameterized() called for GoogleSheets plugin.");
boolean smartJsonSubstitution;
Expand Down Expand Up @@ -133,13 +150,14 @@ public Mono<ActionExecutionResult> executeParameterized(

prepareConfigurationsForExecution(executeActionDTO, actionConfiguration, datasourceConfiguration);

return this.executeCommon(connection, datasourceConfiguration, actionConfiguration);
return this.executeCommon(connection, datasourceConfiguration, actionConfiguration, featureFlagMap);
}

public Mono<ActionExecutionResult> executeCommon(
Void connection,
DatasourceConfiguration datasourceConfiguration,
ActionConfiguration actionConfiguration) {
ActionConfiguration actionConfiguration,
Map<String, Boolean> featureFlagMap) {

log.debug(Thread.currentThread().getName() + ": executeCommon() called for GoogleSheets plugin.");
// Initializing object for error condition
Expand Down Expand Up @@ -185,7 +203,7 @@ public Mono<ActionExecutionResult> executeCommon(
// method
.flatMap(res -> {
return executionMethod
.getExecutionClient(client, methodConfig)
.getExecutionClientWithFlags(client, methodConfig, featureFlagMap)
.headers(headers -> headers.set(
"Authorization",
"Bearer "
Expand Down Expand Up @@ -319,8 +337,11 @@ public Object substituteValueInInput(
}

@Override
public Mono<TriggerResultDTO> trigger(
Void connection, DatasourceConfiguration datasourceConfiguration, TriggerRequestDTO request) {
public Mono<TriggerResultDTO> triggerWithFlags(
Void connection,
DatasourceConfiguration datasourceConfiguration,
TriggerRequestDTO request,
Map<String, Boolean> featureFlagMap) {
log.debug(Thread.currentThread().getName() + ": trigger() called for GoogleSheets plugin.");
final TriggerMethod triggerMethod = GoogleSheetsMethodStrategy.getTriggerMethod(request, objectMapper);
MethodConfig methodConfig = new MethodConfig(request);
Expand All @@ -343,7 +364,7 @@ public Mono<TriggerResultDTO> trigger(
validateAndGetUserAuthorizedSheetIds(datasourceConfiguration, methodConfig);

return triggerMethod
.getTriggerClient(client, methodConfig)
.getTriggerClientWithFlags(client, methodConfig, featureFlagMap)
.headers(headers -> headers.set(
"Authorization",
"Bearer " + oauth2.getAuthenticationResponse().getToken()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.appsmith.server.helpers.PluginExecutorHelper;
import com.appsmith.server.repositories.PluginRepository;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.services.FeatureFlagService;
import com.appsmith.server.services.TenantService;
import com.appsmith.server.solutions.DatasourceTriggerSolution;
import org.apache.commons.lang3.StringUtils;
Expand All @@ -28,18 +29,21 @@ public class PluginTriggerSolutionCEImpl implements PluginTriggerSolutionCE {
private final PluginRepository pluginRepository;
private final ConfigService configService;
private final TenantService tenantService;
private final FeatureFlagService featureFlagService;

public PluginTriggerSolutionCEImpl(
DatasourceTriggerSolution datasourceTriggerSolution,
PluginExecutorHelper pluginExecutorHelper,
PluginRepository pluginRepository,
ConfigService configService,
TenantService tenantService) {
TenantService tenantService,
FeatureFlagService featureFlagService) {
this.datasourceTriggerSolution = datasourceTriggerSolution;
this.pluginExecutorHelper = pluginExecutorHelper;
this.pluginRepository = pluginRepository;
this.configService = configService;
this.tenantService = tenantService;
this.featureFlagService = featureFlagService;
}

/**
Expand Down Expand Up @@ -74,6 +78,11 @@ public Mono<TriggerResultDTO> trigger(
Mono<PluginExecutor> pluginExecutorMono =
pluginMono.flatMap(plugin -> pluginExecutorHelper.getPluginExecutor(Mono.just(plugin)));

// Flags are needed here for google sheets integration to support shared drive behind a flag
// Once thoroughly tested, this flag can be removed
abhvsn marked this conversation as resolved.
Show resolved Hide resolved
Map<String, Boolean> featureFlagMap =
featureFlagService.getCachedTenantFeatureFlags().getFeatures();

/*
* Since there is no datasource provided, we are passing the Datasource Context connection and datasourceConfiguration as null.
* We will leave the execution to respective plugin executor.
Expand All @@ -83,8 +92,8 @@ public Mono<TriggerResultDTO> trigger(
PluginExecutor pluginExecutor = pair.getT2();
setHeadersToTriggerRequest(plugin, httpHeaders, triggerRequestDTO);
return setTenantAndInstanceId(triggerRequestDTO)
.flatMap(updatedTriggerRequestDTO ->
((PluginExecutor<Object>) pluginExecutor).trigger(null, null, updatedTriggerRequestDTO));
.flatMap(updatedTriggerRequestDTO -> ((PluginExecutor<Object>) pluginExecutor)
.triggerWithFlags(null, null, updatedTriggerRequestDTO, featureFlagMap));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.appsmith.server.helpers.PluginExecutorHelper;
import com.appsmith.server.repositories.PluginRepository;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.services.FeatureFlagService;
import com.appsmith.server.services.TenantService;
import com.appsmith.server.solutions.DatasourceTriggerSolution;
import org.springframework.stereotype.Component;
Expand All @@ -14,7 +15,14 @@ public PluginTriggerSolutionImpl(
PluginExecutorHelper pluginExecutorHelper,
PluginRepository pluginRepository,
ConfigService configService,
TenantService tenantService) {
super(datasourceTriggerSolution, pluginExecutorHelper, pluginRepository, configService, tenantService);
TenantService tenantService,
FeatureFlagService featureFlagService) {
super(
datasourceTriggerSolution,
pluginExecutorHelper,
pluginRepository,
configService,
tenantService,
featureFlagService);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.appsmith.server.services.AuthenticationValidator;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.services.DatasourceContextService;
import com.appsmith.server.services.FeatureFlagService;
import com.appsmith.server.services.SessionUserService;
import com.appsmith.server.services.TenantService;
import com.appsmith.server.solutions.ce.ActionExecutionSolutionCEImpl;
Expand Down Expand Up @@ -42,7 +43,8 @@ public ActionExecutionSolutionImpl(
ConfigService configService,
TenantService tenantService,
CommonConfig commonConfig,
ActionExecutionSolutionHelper actionExecutionSolutionHelper) {
ActionExecutionSolutionHelper actionExecutionSolutionHelper,
FeatureFlagService featureFlagService) {
super(
newActionService,
actionPermission,
Expand All @@ -63,6 +65,7 @@ public ActionExecutionSolutionImpl(
configService,
tenantService,
commonConfig,
actionExecutionSolutionHelper);
actionExecutionSolutionHelper,
featureFlagService);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.appsmith.server.services.AuthenticationValidator;
import com.appsmith.server.services.ConfigService;
import com.appsmith.server.services.DatasourceContextService;
import com.appsmith.server.services.FeatureFlagService;
import com.appsmith.server.services.TenantService;
import com.appsmith.server.solutions.ce.DatasourceTriggerSolutionCEImpl;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -28,7 +29,8 @@ public DatasourceTriggerSolutionImpl(
DatasourcePermission datasourcePermission,
EnvironmentPermission environmentPermission,
ConfigService configService,
TenantService tenantService) {
TenantService tenantService,
FeatureFlagService featureFlagService) {
super(
datasourceService,
datasourceStorageService,
Expand All @@ -40,6 +42,7 @@ public DatasourceTriggerSolutionImpl(
datasourcePermission,
environmentPermission,
configService,
tenantService);
tenantService,
featureFlagService);
}
}
Loading
Loading