Skip to content

Commit

Permalink
fix: Added shared drive support behind a feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
“sneha122” committed Nov 27, 2024
1 parent f6c8748 commit 703dce1
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 42 deletions.
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;
}

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) {
// 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);
UriComponentsBuilder uriBuilder = getBaseUriBuilder(
this.BASE_DRIVE_API_URL,
"?includeItemsFromAllDrives=true&supportsAllDrives=true&corpora=allDrives&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) {
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 @@ -76,11 +76,12 @@ public static class GoogleSheetsPluginExecutor implements PluginExecutor<Void>,
new HashSet<>(Arrays.asList(FieldName.ROW_OBJECT, FieldName.ROW_OBJECTS));

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

log.debug(Thread.currentThread().getName() + ": executeParameterized() called for GoogleSheets plugin.");
boolean smartJsonSubstitution;
Expand Down Expand Up @@ -133,13 +134,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 +187,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 +321,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 +348,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 @@ -38,18 +38,6 @@
{
"label": "Read / Write / Delete | Selected google sheets",
"value": "https://www.googleapis.com/auth/drive.file"
},
{
"label": "Read / Write / Delete | All google sheets",
"value": "https://www.googleapis.com/auth/spreadsheets,https://www.googleapis.com/auth/drive"
},
{
"label": "Read / Write | All google sheets",
"value": "https://www.googleapis.com/auth/spreadsheets,https://www.googleapis.com/auth/drive.readonly"
},
{
"label": "Read | All google sheets",
"value": "https://www.googleapis.com/auth/spreadsheets.readonly,https://www.googleapis.com/auth/drive.readonly"
}
],
"initialValue": "https://www.googleapis.com/auth/drive.file",
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
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

0 comments on commit 703dce1

Please sign in to comment.