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.

Sign up for GitHub

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

Jump to bottom

Fixed escaped layout being used for update #3968

Merged
merged 3 commits into from
Apr 13, 2021
Merged

Fixed escaped layout being used for update #3968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca61475
Fixed escaped layout being used for update
nidhi-nair Apr 12, 2021
0b65849
Fixed escaped layout being used for update
nidhi-nair Apr 12, 2021
b1396e9
Few more cases of unescaped layout usage
nidhi-nair Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 7 additions & 4 deletions ...ppsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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