Skip to content

Commit

Permalink
Fixed escaped layout being used for update (#3968)
Browse files Browse the repository at this point in the history 
* Fixed escaped layout being used for update

* Few more cases of unescaped layout usage
  • Loading branch information
@nidhi-nair
nidhi-nair committed Apr 13, 2021
1 parent 31290b5 commit a24eb90
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 40 deletions.
11 changes: 7 additions & 4 deletions ...ppsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,12 +320,12 @@ private Mono<PageDTO> clonePageGivenApplicationId(String pageId, String applicat
Mono<PageDTO> sourcePageMono = newPageService.findPageById(pageId, MANAGE_PAGES, false)
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, pageId)))
.flatMap(page -> Flux.fromIterable(page.getLayouts())
.map(layout -> layout.getDsl())
.map(dsl -> {
.map(layout -> {
Layout newLayout = new Layout();
String id = new ObjectId().toString();
newLayout.setId(id);
newLayout.setDsl(dsl);
newLayout.setMongoEscapedWidgetNames(layout.getMongoEscapedWidgetNames());
newLayout.setDsl(layout.getDsl());
return newLayout;
})
.collectList()
Expand Down Expand Up @@ -398,7 +398,10 @@ private Mono<PageDTO> clonePageGivenApplicationId(String pageId, String applicat
List<Layout> layouts = savedPage.getLayouts();

return Flux.fromIterable(layouts)
.flatMap(layout -> layoutActionService.updateLayout(savedPage.getId(), layout.getId(), layout))
.flatMap(layout -> {
layout.setDsl(layoutActionService.unescapeMongoSpecialCharacters(layout));
return layoutActionService.updateLayout(savedPage.getId(), layout.getId(), layout);
})
.collectList()
.thenReturn(savedPage);
})
Expand Down
18 changes: 14 additions & 4 deletions ...r/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ public Mono<ActionDTO> moveAction(ActionMoveDTO actionMoveDTO) {

// 2. Run updateLayout on the old page
return Flux.fromIterable(page.getLayouts())
.flatMap(layout -> updateLayout(oldPageId, layout.getId(), layout))
.flatMap(layout -> {
layout.setDsl(this.unescapeMongoSpecialCharacters(layout));
return updateLayout(page.getId(), layout.getId(), layout);
})
.collect(toSet());
})
// fetch the unpublished destination page
Expand All @@ -125,7 +128,10 @@ public Mono<ActionDTO> moveAction(ActionMoveDTO actionMoveDTO) {

// 3. Run updateLayout on the new page.
return Flux.fromIterable(page.getLayouts())
.flatMap(layout -> updateLayout(actionMoveDTO.getDestinationPageId(), layout.getId(), layout))
.flatMap(layout -> {
layout.setDsl(this.unescapeMongoSpecialCharacters(layout));
return updateLayout(page.getId(), layout.getId(), layout);
})
.collect(toSet());
})
// 4. Return the saved action.
Expand Down Expand Up @@ -270,7 +276,8 @@ private Mono<LayoutDTO> refactorName(String pageId, String layoutId, String oldN
List<Layout> layouts = page.getLayouts();
for (Layout layout : layouts) {
if (layout.getId().equals(layoutId)) {
return updateLayout(pageId, layout.getId(), layout);
layout.setDsl(this.unescapeMongoSpecialCharacters(layout));
return updateLayout(page.getId(), layout.getId(), layout);
}
}
return Mono.empty();
Expand Down Expand Up @@ -531,7 +538,10 @@ private Mono<String> updatePageLayoutsGivenAction(String pageId) {
return Mono.empty();
}
return Flux.fromIterable(page.getLayouts())
.flatMap(layout -> updateLayout(page.getId(), layout.getId(), layout));
.flatMap(layout -> {
layout.setDsl(this.unescapeMongoSpecialCharacters(layout));
return updateLayout(page.getId(), layout.getId(), layout);
});
})
.collectList()
.then(Mono.just(pageId));
Expand Down
1 change: 1 addition & 0 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public Mono<Layout> createLayout(String pageId, Layout layout) {
}
//Adding an Id to the layout to ensure that a layout can be referred to by its ID as well.
layout.setId(new ObjectId().toString());

layoutList.add(layout);
page.setLayouts(layoutList);
return page;
Expand Down
27 changes: 23 additions & 4 deletions app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.appsmith.server.repositories.PluginRepository;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONArray;
import net.minidev.json.JSONObject;
import org.junit.After;
import org.junit.Before;
Expand All @@ -55,6 +56,8 @@
import reactor.test.StepVerifier;

import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -146,13 +149,29 @@ public void setup() {
testApp = applicationPageService.createApplication(application, organization.getId()).block();

final String pageId = testApp.getPages().get(0).getId();
Layout layout = new Layout();

testPage = newPageService.findPageById(pageId, READ_PAGES, false).block();

Layout layout = testPage.getLayouts().get(0);
JSONObject dsl = new JSONObject(Map.of("text", "{{ query1.data }}"));

JSONObject dsl2 = new JSONObject();
dsl2.put("widgetName", "Table1");
dsl2.put("type", "TABLE_WIDGET");
Map<String, Object> primaryColumns = new HashMap<>();
JSONObject jsonObject = new JSONObject(Map.of("key", "value"));
primaryColumns.put("_id", "{{ query1.data }}");
primaryColumns.put("_class", jsonObject);
dsl2.put("primaryColumns", primaryColumns);
final ArrayList<Object> objects = new ArrayList<>();
JSONArray temp2 = new JSONArray();
temp2.addAll(List.of(new JSONObject(Map.of("key", "primaryColumns._id"))));
dsl2.put("dynamicBindingPathList", temp2);
objects.add(dsl2);
dsl.put("children", objects);

layout.setDsl(dsl);
layout.setPublishedDsl(dsl);
layoutService.createLayout(pageId, layout).block();

testPage = newPageService.findPageById(pageId, READ_PAGES, false).block();
}

Organization testOrg = organizationRepository.findByName("Another Test Organization", AclPermission.READ_ORGANIZATIONS).block();
Expand Down
18 changes: 18 additions & 0 deletions ...r/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -117,6 +118,22 @@ public void setup() {
temp.addAll(List.of(new JSONObject(Map.of("key", "testField"))));
dsl.put("dynamicBindingPathList", temp);
dsl.put("testField", "{{ query1.data }}");

JSONObject dsl2 = new JSONObject();
dsl2.put("widgetName", "Table1");
dsl2.put("type", "TABLE_WIDGET");
Map<String, Object> primaryColumns = new HashMap<>();
JSONObject jsonObject = new JSONObject(Map.of("key", "value"));
primaryColumns.put("_id", "{{ query1.data }}");
primaryColumns.put("_class", jsonObject);
dsl2.put("primaryColumns", primaryColumns);
final ArrayList<Object> objects = new ArrayList<>();
JSONArray temp2 = new JSONArray();
temp2.addAll(List.of(new JSONObject(Map.of("key", "primaryColumns._id"))));
dsl2.put("dynamicBindingPathList", temp2);
objects.add(dsl2);
dsl.put("children", objects);

layout.setDsl(dsl);
layout.setPublishedDsl(dsl);
layoutActionService.updateLayout(pageId, layout.getId(), layout).block();
Expand Down Expand Up @@ -267,6 +284,7 @@ public void refactorActionNameToDeletedName() {

ActionDTO firstAction = newActionService.createAction(action).block();

layout.setDsl(layoutActionService.unescapeMongoSpecialCharacters(layout));
LayoutDTO firstLayout = layoutActionService.updateLayout(testPage.getId(), layout.getId(), layout).block();

applicationPageService.publish(testPage.getApplicationId()).block();
Expand Down
90 changes: 62 additions & 28 deletions app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,20 @@
import com.appsmith.server.domains.Plugin;
import com.appsmith.server.domains.User;
import com.appsmith.server.dtos.ActionDTO;
import com.appsmith.server.dtos.LayoutDTO;
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.MockPluginExecutor;
import com.appsmith.server.helpers.PluginExecutorHelper;
import com.appsmith.server.repositories.PluginRepository;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONArray;
import net.minidev.json.JSONObject;
import net.minidev.json.parser.JSONParser;
import net.minidev.json.parser.ParseException;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -35,6 +38,8 @@
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -56,6 +61,9 @@ public class PageServiceTest {
@Autowired
UserService userService;

@Autowired
LayoutService layoutService;

@Autowired
OrganizationService organizationService;

Expand All @@ -77,6 +85,9 @@ public class PageServiceTest {
@MockBean
PluginExecutor pluginExecutor;

@Autowired
LayoutActionService layoutActionService;

Application application = null;

String applicationId = null;
Expand Down Expand Up @@ -252,13 +263,13 @@ public void clonePage() throws ParseException {
.users(Set.of("api_user"))
.build();

PageDTO testPage = new PageDTO();
testPage.setName("PageServiceTest CloneTest Source");
setupTestApplication();
testPage.setApplicationId(application.getId());
final String pageId = application.getPages().get(0).getId();

final PageDTO page = newPageService.findPageById(pageId, READ_PAGES, false).block();

ActionDTO action = new ActionDTO();
action.setName("Page Action");
action.setName("PageAction");
action.setActionConfiguration(new ActionConfiguration());
Datasource datasource = new Datasource();
datasource.setOrganizationId(orgId);
Expand All @@ -268,42 +279,65 @@ public void clonePage() throws ParseException {
action.setDatasource(datasource);
action.setExecuteOnLoad(true);

Mono<PageDTO> pageMono = applicationPageService.createPage(testPage)
.flatMap(page -> {
action.setPageId(page.getId());
return newActionService.createAction(action)
.thenReturn(page);
})
.flatMap(page -> applicationPageService.clonePage(page.getId()))
.cache();
assert page != null;
Layout layout = page.getLayouts().get(0);
JSONObject dsl = new JSONObject(Map.of("text", "{{ query1.data }}"));
dsl.put("widgetName", "firstWidget");

JSONObject dsl2 = new JSONObject();
dsl2.put("widgetName", "Table1");
dsl2.put("type", "TABLE_WIDGET");
Map<String, Object> primaryColumns = new HashMap<>();
JSONObject jsonObject = new JSONObject(Map.of("key", "value"));
primaryColumns.put("_id", "{{ PageAction.data }}");
primaryColumns.put("_class", jsonObject);
dsl2.put("primaryColumns", primaryColumns);
final ArrayList<Object> objects = new ArrayList<>();
JSONArray temp2 = new JSONArray();
temp2.addAll(List.of(new JSONObject(Map.of("key", "primaryColumns._id"))));
dsl2.put("dynamicBindingPathList", temp2);
objects.add(dsl2);
dsl.put("children", objects);

layout.setDsl(dsl);
layout.setPublishedDsl(dsl);

Mono<List<NewAction>> actionsMono = pageMono
// fetch the actions by new cloned page id.
.flatMapMany(page -> newActionService.findByPageId(page.getId(), READ_ACTIONS))
.collectList();
action.setPageId(page.getId());

final LayoutDTO layoutDTO = layoutActionService.updateLayout(page.getId(), layout.getId(), layout).block();

newActionService.createAction(action).block();

final Mono<PageDTO> pageMono = applicationPageService.clonePage(page.getId()).cache();

Mono<List<NewAction>> actionsMono =
pageMono
.flatMapMany(
page1 -> newActionService
.findByPageId(page1.getId(), READ_ACTIONS))
.collectList();

Object parsedJson = new JSONParser(JSONParser.MODE_PERMISSIVE).parse(FieldName.DEFAULT_PAGE_LAYOUT);
StepVerifier
.create(Mono.zip(pageMono, actionsMono))
.assertNext(tuple -> {
PageDTO page = tuple.getT1();
assertThat(page).isNotNull();
assertThat(page.getId()).isNotNull();
assertThat("PageServiceTest CloneTest Source Copy".equals(page.getName()));
PageDTO clonedPage = tuple.getT1();
assertThat(clonedPage).isNotNull();
assertThat(clonedPage.getId()).isNotNull();
Assert.assertEquals(page.getName() + " Copy", clonedPage.getName());

assertThat(page.getPolicies()).isNotEmpty();
assertThat(page.getPolicies()).containsOnly(managePagePolicy, readPagePolicy);
assertThat(clonedPage.getPolicies()).isNotEmpty();
assertThat(clonedPage.getPolicies()).containsOnly(managePagePolicy, readPagePolicy);

assertThat(page.getLayouts()).isNotEmpty();
assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(parsedJson);
assertThat(page.getLayouts().get(0).getWidgetNames()).isNotEmpty();
assertThat(page.getLayouts().get(0).getPublishedDsl()).isNullOrEmpty();
assertThat(clonedPage.getLayouts()).isNotEmpty();
assertThat(clonedPage.getLayouts().get(0).getDsl().get("widgetName")).isEqualTo("firstWidget");
assertThat(clonedPage.getLayouts().get(0).getWidgetNames()).isNotEmpty();
assertThat(clonedPage.getLayouts().get(0).getMongoEscapedWidgetNames()).isNotEmpty();
assertThat(clonedPage.getLayouts().get(0).getPublishedDsl()).isNullOrEmpty();

// Confirm that the page action got copied as well
List<NewAction> actions = tuple.getT2();
assertThat(actions.size()).isEqualTo(1);
assertThat(actions.get(0).getUnpublishedAction().getName()).isEqualTo("Page Action");
assertThat(actions.get(0).getUnpublishedAction().getName()).isEqualTo("PageAction");

// Confirm that executeOnLoad is cloned as well.
assertThat(Boolean.TRUE.equals(actions.get(0).getUnpublishedAction().getExecuteOnLoad()));
Expand Down

0 comments on commit a24eb90

Please sign in to comment.