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

[Bug fix] A lax search for presence of binding during save page to match client algorithm to reduce page save error (#3698)

Browse Source 
* Lax mustache binding check added to match the client side check when client recognizes a field to have a dynamic binding. This would reduce/remove bad bindings from throwing a 400 during save page.

* Added a test to assert that update layout does not fail in case the binding is technically incorrect because part of the mustache's lie inside quotes. Since client has a lax way of finding a dynamic path, server also follows suite.
This commit is contained in:
Trisha Anand 2021-03-25 16:00:45 +05:30 committed by Arpit Mohan
parent e9f80f3312
commit bf6cdbe305
3 changed files with 85 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java vendored
View File

@ -41,6 +41,8 @@ public class MustacheHelper {
private static Pattern quoteQuestionPattern = Pattern.compile(regexQuotesTrimming);
// The final replacement string of ? for replacing '?' or "?"
private static String postQuoteTrimmingQuestionMark = "\\?";
private static String laxMustacheBindingRegex = "\\{\\{([\\s\\S]*?)\\}\\}";
private static Pattern laxMustacheBindingPattern = Pattern.compile(laxMustacheBindingRegex);
/**
@ -364,4 +366,8 @@ public class MustacheHelper {
return body;
}
public static Boolean laxIsBindingPresentInString(String input) {
return laxMustacheBindingPattern.matcher(input).find();
}
}

7
app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java
View File

@ -353,19 +353,20 @@ public class LayoutActionServiceImpl implements LayoutActionService {
}
// Only extract mustache keys from leaf nodes
if (isLeafNode) {
Set<String> mustacheKeysFromFields = MustacheHelper.extractMustacheKeysFromFields(parent);
// We found the path. But if the path does not have any mustache bindings, throw the error
if (mustacheKeysFromFields.isEmpty()) {
if (!MustacheHelper.laxIsBindingPresentInString((String) parent)) {
try {
String bindingAsString = objectMapper.writeValueAsString(parent);
throw new AppsmithException(AppsmithError.INVALID_DYNAMIC_BINDING_REFERENCE, widgetType,
widgetName, widgetId, fieldPath, pageId, layoutId, bindingAsString);
widgetName, widgetId, fieldPath, pageId, layoutId, bindingAsString);
} catch (JsonProcessingException e) {
throw new AppsmithException(AppsmithError.JSON_PROCESSING_ERROR, parent);
}
}
// Stricter extraction of dynamic bindings
Set<String> mustacheKeysFromFields = MustacheHelper.extractMustacheKeysFromFields(parent);
dynamicBindings.addAll(mustacheKeysFromFields);
}
}

76
app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java
View File

@ -452,7 +452,7 @@ public class LayoutServiceTest {
@Test
@WithUserDetails(value = "api_user")
public void testIncorrectDynamicBinding() {
public void testIncorrectDynamicBindingPathInDsl() {
Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor()));
PageDTO testPage = new PageDTO();
@ -525,6 +525,80 @@ public class LayoutServiceTest {
.verify();
}
@Test
@WithUserDetails(value = "api_user")
public void testIncorrectMustacheExpressionInBindingInDsl() {
Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor()));
PageDTO testPage = new PageDTO();
testPage.setName("testIncorrectMustacheExpressionInBinding Test Page");
Application app = new Application();
app.setName("newApplication-testIncorrectMustacheExpressionInBinding-Test");
PageDTO page = createPage(app, testPage).block();
String pageId = page.getId();
String layoutId = page.getLayouts().get(0).getId();
Mono<LayoutDTO> testMono = Mono.just(page)
.flatMap(page1 -> {
List<Mono<ActionDTO>> monos = new ArrayList<>();
ActionDTO action = new ActionDTO();
action.setName("aGetAction");
action.setActionConfiguration(new ActionConfiguration());
action.getActionConfiguration().setHttpMethod(HttpMethod.GET);
action.setPageId(page1.getId());
action.setDatasource(datasource);
monos.add(newActionService.createAction(action));
return Mono.zip(monos, objects -> page1);
})
.zipWhen(page1 -> {
Layout layout = new Layout();
JSONObject obj = new JSONObject(Map.of(
"key", "value"
));
layout.setDsl(obj);
return layoutService.createLayout(page1.getId(), layout);
})
.flatMap(tuple2 -> {
final PageDTO page1 = tuple2.getT1();
final Layout layout = tuple2.getT2();
Layout newLayout = new Layout();
JSONObject obj = new JSONObject(Map.of(
"widgetName", "testWidget",
"widgetId", "id",
"type", "test_type",
"key", "value-updated",
"dynamicGet", "a\"{{aGetAction\"/'\"'}}\"\""
));
JSONArray dynamicBindingsPathList = new JSONArray();
dynamicBindingsPathList.addAll(List.of(
new JSONObject(Map.of("key", "dynamicGet"))
));
obj.put("dynamicBindingPathList", dynamicBindingsPathList);
newLayout.setDsl(obj);
return layoutActionService.updateLayout(page1.getId(), layout.getId(), newLayout);
});
StepVerifier
.create(testMono)
.assertNext(layoutDTO -> {
// We have reached here means we didnt get a throwable. Thats good
assertThat(layoutDTO).isNotNull();
// Since this is still a bad mustache binding, we couldn't have extracted the action name
assertThat(layoutDTO.getLayoutOnLoadActions().size()).isEqualTo(0);
})
.verifyComplete();
}
@After
public void purgePages() {
newPageService.deleteAll();