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

fix: fix action objects missing plugin id and plugin type info in database (#13263)

Browse Source 
* add plugin id and type info to action object if found missing
* this fix is currently added to the read and update action flows
This commit is contained in:
Sumit Kumar 2022-04-26 16:51:30 +05:30 committed by GitHub
parent b88cdacc7d
commit 7e9eb58f03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 168 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java
View File

@ -99,4 +99,5 @@ public interface NewActionServiceCE extends CrudService<NewAction, String> {
Mono<String> findBranchedIdByBranchNameAndDefaultActionId(String branchName, String defaultActionId, AclPermission permission);
public Mono<NewAction> sanitizeAction(NewAction action);
}

116
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java
View File

@ -120,6 +120,8 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
public static final String NATIVE_QUERY_PATH = "formToNativeQuery";
public static final String NATIVE_QUERY_PATH_DATA = NATIVE_QUERY_PATH + "." + DATA;
public static final String NATIVE_QUERY_PATH_STATUS = NATIVE_QUERY_PATH + "." + STATUS;
public static final PluginType JS_PLUGIN_TYPE = PluginType.JS;
public static final String JS_PLUGIN_PACKAGE_NAME = "js-plugin";
private final NewActionRepository repository;
private final DatasourceService datasourceService;
@ -371,10 +373,10 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
action.setInvalids(invalids);
action.setPluginName(plugin.getName());
newAction.setUnpublishedAction(action);
newAction.setPluginType(plugin.getType());
newAction.setPluginId(plugin.getId());
return newAction;
}).map(this::extractAndSetJsonPathKeys)
})
.flatMap(this::sanitizeAction)
.map(this::extractAndSetJsonPathKeys)
.map(updatedAction -> {
// In case of external datasource (not embedded) instead of storing the entire datasource
// again inside the action, instead replace it with just the datasource ID. This is so that
@ -1100,7 +1102,8 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
@Override
public Flux<NewAction> findUnpublishedOnLoadActionsExplicitSetByUserInPage(String pageId) {
return repository
.findUnpublishedActionsByPageIdAndExecuteOnLoadSetByUserTrue(pageId, MANAGE_ACTIONS);
.findUnpublishedActionsByPageIdAndExecuteOnLoadSetByUserTrue(pageId, MANAGE_ACTIONS)
.flatMap(this::sanitizeAction);
}
/**
@ -1113,27 +1116,32 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
@Override
public Flux<NewAction> findUnpublishedActionsInPageByNames(Set<String> names, String pageId) {
return repository
.findUnpublishedActionsByNameInAndPageId(names, pageId, MANAGE_ACTIONS);
.findUnpublishedActionsByNameInAndPageId(names, pageId, MANAGE_ACTIONS)
.flatMap(this::sanitizeAction);
}
@Override
public Mono<NewAction> findById(String id) {
return repository.findById(id);
return repository.findById(id)
.flatMap(this::sanitizeAction);
}
@Override
public Mono<NewAction> findById(String id, AclPermission aclPermission) {
return repository.findById(id, aclPermission);
return repository.findById(id, aclPermission)
.flatMap(this::sanitizeAction);
}
@Override
public Flux<NewAction> findByPageId(String pageId, AclPermission permission) {
return repository.findByPageId(pageId, permission);
return repository.findByPageId(pageId, permission)
.flatMap(this::sanitizeAction);
}
@Override
public Flux<NewAction> findByPageIdAndViewMode(String pageId, Boolean viewMode, AclPermission permission) {
return repository.findByPageIdAndViewMode(pageId, viewMode, permission);
return repository.findByPageIdAndViewMode(pageId, viewMode, permission)
.flatMap(this::sanitizeAction);
}
@Override
@ -1151,7 +1159,8 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
// every created action starts from an unpublishedAction state.
return Mono.just(action);
});
})
.flatMap(this::sanitizeAction);
}
@Override
@ -1320,9 +1329,11 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
return applicationService
.findById(params.getFirst(FieldName.APPLICATION_ID), READ_APPLICATIONS)
.flatMapMany(application -> repository.findByApplicationIdAndViewMode(application.getId(), false, READ_ACTIONS))
.flatMap(this::sanitizeAction)
.flatMap(this::setTransientFieldsInUnpublishedAction);
}
return repository.findAllActionsByNameAndPageIdsAndViewMode(name, pageIds, false, READ_ACTIONS, sort)
.flatMap(this::sanitizeAction)
.flatMap(this::setTransientFieldsInUnpublishedAction);
}
@ -1365,6 +1376,75 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
.filter(actionDTO -> !PluginType.JS.equals(actionDTO.getPluginType()));
}
/**
* This method is meant to be used to check for any missing or bad values in NewAction object and attempt to fix it.
*
* This method is added in response to certain cases where it was found that pluginId and pluginType keys
* were missing from the NewAction object in the database.Since it is currently not know what exactly causes
* these values to go missing, this check will serve as a workaround by fetching and setting pluginId and
* pluginType using the datasource object contained in the ActionDTO object.
* Ref: https://github.com/appsmithorg/appsmith/issues/11927
*
*/
public Mono<NewAction> sanitizeAction(NewAction action) {
Mono<NewAction> actionMono = Mono.just(action);
if (isPluginTypeOrPluginIdMissing(action)) {
actionMono = providePluginTypeAndIdToNewActionObjectUsingJSTypeOrDatasource(action);
}
return actionMono;
}
private boolean isPluginTypeOrPluginIdMissing(NewAction action) {
return action.getPluginId() == null || action.getPluginType() == null;
}
private Mono<NewAction> providePluginTypeAndIdToNewActionObjectUsingJSTypeOrDatasource(NewAction action) {
ActionDTO actionDTO = action.getUnpublishedAction();
if (actionDTO == null) {
return Mono.just(action);
}
/**
* if path:
* In case an action object is related to a JS Object then it must have a non-null collectionId.
*
* else path:
* Otherwise, check if the datasource object has the pluginId. If so, use this pluginId to fetch the correct
* pluginType.
*/
Datasource datasource = actionDTO.getDatasource();
if (actionDTO.getCollectionId() != null) {
return setPluginIdAndTypeForJSAction(action);
}
else if (datasource != null && datasource.getPluginId() != null) {
String pluginId = datasource.getPluginId();
action.setPluginId(pluginId);
return setPluginTypeFromId(action, pluginId);
}
return Mono.just(action);
}
private Mono<NewAction> setPluginTypeFromId(NewAction action, String pluginId) {
return pluginService.findById(pluginId)
.flatMap(plugin -> {
action.setPluginType(plugin.getType());
return Mono.just(action);
});
}
private Mono<NewAction> setPluginIdAndTypeForJSAction(NewAction action) {
action.setPluginType(JS_PLUGIN_TYPE);
return pluginService.findByPackageName(JS_PLUGIN_PACKAGE_NAME)
.flatMap(plugin -> {
action.setPluginId(plugin.getId());
return Mono.just(action);
});
}
// We can afford to make this call all the time since we already have all the info we need in context
private Mono<DatasourceContext> getRemoteDatasourceContext(Plugin plugin, Datasource datasource) {
final DatasourceContext datasourceContext = new DatasourceContext();
@ -1388,7 +1468,9 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
if (action.getGitSyncId() == null) {
action.setGitSyncId(action.getApplicationId() + "_" + Instant.now().toString());
}
return repository.save(action);
return sanitizeAction(action)
.flatMap(sanitizedAction -> repository.save(sanitizedAction));
}
@Override
@ -1396,12 +1478,17 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
actions.stream()
.filter(action -> action.getGitSyncId() == null)
.forEach(action -> action.setGitSyncId(action.getApplicationId() + "_" + Instant.now().toString()));
return repository.saveAll(actions);
return Flux.fromIterable(actions)
.flatMap(this::sanitizeAction)
.collectList()
.flatMapMany(actionList -> repository.saveAll(actionList));
}
@Override
public Flux<NewAction> findByPageId(String pageId) {
return repository.findByPageId(pageId);
return repository.findByPageId(pageId)
.flatMap(this::sanitizeAction);
}
/**
@ -1642,7 +1729,8 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
return repository.findByBranchNameAndDefaultActionId(branchName, defaultActionId, permission)
.switchIfEmpty(Mono.error(
new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.ACTION, defaultActionId + "," + branchName))
);
)
.flatMap(this::sanitizeAction);
}
public Mono<String> findBranchedIdByBranchNameAndDefaultActionId(String branchName, String defaultActionId, AclPermission permission) {

65
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceCEImplTest.java
View File

@ -1,8 +1,13 @@
package com.appsmith.server.services.ce;
import com.appsmith.external.models.ActionExecutionResult;
import com.appsmith.external.models.Datasource;
import com.appsmith.server.acl.PolicyGenerator;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.domains.NewAction;
import com.appsmith.server.domains.Plugin;
import com.appsmith.server.domains.PluginType;
import com.appsmith.server.dtos.ActionDTO;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.PluginExecutorHelper;
@ -23,6 +28,7 @@ import lombok.extern.slf4j.Slf4j;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.core.codec.ByteBufferDecoder;
import org.springframework.core.codec.StringDecoder;
@ -56,6 +62,12 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.spy;
@RunWith(SpringRunner.class)
@Slf4j
@ -215,4 +227,57 @@ public class NewActionServiceCEImplTest {
.verify();
}
@Test
public void testMissingPluginIdAndTypeFixForNonJSPluginType() {
/* Mock `findById` method of pluginService to return `testPlugin` */
Plugin testPlugin = new Plugin();
testPlugin.setId("testId");
testPlugin.setType(PluginType.DB);
Mockito.when(pluginService.findById(anyString()))
.thenReturn(Mono.just(testPlugin));
NewAction action = new NewAction();
action.setPluginId(null);
action.setPluginType(null);
ActionDTO actionDTO = new ActionDTO();
Datasource datasource = new Datasource();
/* Datasource has correct plugin id */
datasource.setPluginId(testPlugin.getId());
actionDTO.setDatasource(datasource);
action.setUnpublishedAction(actionDTO);
Mono<NewAction> updatedActionFlux = newActionService.sanitizeAction(action);
StepVerifier.create(updatedActionFlux)
.assertNext(updatedAction -> {
assertEquals("testId", updatedAction.getPluginId());
assertEquals(PluginType.DB, updatedAction.getPluginType());
})
.verifyComplete();
}
@Test
public void testMissingPluginIdAndTypeFixForJSPluginType() {
/* Mock `findByPackageName` method of pluginService to return `testPlugin` */
Plugin testPlugin = new Plugin();
testPlugin.setId("testId");
testPlugin.setType(PluginType.JS);
Mockito.when(pluginService.findByPackageName(anyString()))
.thenReturn(Mono.just(testPlugin));
NewAction action = new NewAction();
action.setPluginId(null);
action.setPluginType(null);
ActionDTO actionDTO = new ActionDTO();
/* Non-null collection id to indicate a JS action */
actionDTO.setCollectionId("testId");
action.setUnpublishedAction(actionDTO);
Mono<NewAction> updatedActionFlux = newActionService.sanitizeAction(action);
StepVerifier.create(updatedActionFlux)
.assertNext(updatedAction -> {
assertEquals("testId", updatedAction.getPluginId());
assertEquals(PluginType.JS, updatedAction.getPluginType());
})
.verifyComplete();
}
}