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

fix: Input validations on page update API (#32692)

Browse Source 
[Slack thread with
details](https://theappsmith.slack.com/archives/C03RPDB936Z/p1713161688607509).
Not including details here.

Sanity passes on EE.

/ok-to-test tags="@tag.Sanity"<!-- This is an auto-generated comment:
Cypress test results -->
> [!WARNING]
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/8704837594>
> Commit: fb1724a0e93f21e25dd09f821b101438a8bee52f
> Cypress dashboard url: <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=8704837594&attempt=1"
target="_blank">Click here!</a>
> It seems like **no tests ran** 😔. We are not able to recognize it,
please check workflow <a
href="https://github.com/appsmithorg/appsmith/actions/runs/8704837594"
target="_blank">here.</a>

<!-- end of auto-generated comment: Cypress test results  -->



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **New Features**
- Enhanced page update capabilities with new fields for page name, icon,
and custom slug.
- **Improvements**
- Improved text handling support for non-Latin characters and
punctuation in page names.
- **Tests**
	- Added test cases for new page creation scenarios and page updates.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Shrikant Sharat Kandula 2024-04-18 16:14:07 +05:30 committed by GitHub
parent 02c1d9e7b4
commit dcada4522d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 96 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/PageControllerCE.java
View File

@ -8,6 +8,7 @@ import com.appsmith.server.dtos.ApplicationPagesDTO;
import com.appsmith.server.dtos.CRUDPageResourceDTO;
import com.appsmith.server.dtos.CRUDPageResponseDTO;
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.dtos.PageUpdateDTO;
import com.appsmith.server.dtos.ResponseDTO;
import com.appsmith.server.newpages.base.NewPageService;
import com.appsmith.server.services.ApplicationPageService;
@ -166,7 +167,7 @@ public class PageControllerCE {
@PutMapping("/{defaultPageId}")
public Mono<ResponseDTO<PageDTO>> updatePage(
@PathVariable String defaultPageId,
@RequestBody PageDTO resource,
@RequestBody @Valid PageUpdateDTO resource,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
log.debug("Going to update page with id: {}, branchName: {}", defaultPageId, branchName);
return newPageService

18
app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageUpdateDTO.java Normal file
View File

@ -0,0 +1,18 @@
package com.appsmith.server.dtos;
import jakarta.validation.constraints.Pattern;
import jakarta.validation.constraints.Size;
public record PageUpdateDTO(
@Pattern(regexp = "[^/\\\\:]+", message = "/, \\, : not allowed in page names") @Size(max = 30) String name,
String icon,
@Pattern(regexp = "[-\\w]+") String customSlug) {
public PageDTO toPageDTO() {
final PageDTO page = new PageDTO();
page.setName(name);
page.setIcon(icon);
page.setCustomSlug(customSlug);
return page;
}
}

17
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java
View File

@ -15,14 +15,14 @@ public class TextUtils {
* NON_LATIN regex matches any letter that is not a ASCII i.e. A-Za-z0-9, `-` and `_`
* It'll match the unicode letters.
* */
private static final Pattern NON_LATIN = Pattern.compile("[^\\w_-]");
private static final Pattern NON_LATIN = Pattern.compile("[^\\w-]");
/*
* The SEPARATORS pattern matches those characters which cane be replaced with `-`
* This includes Punctuation characters, `&`, space and not `-` itself. Details on `Punct`
* http://www.gnu.org/software/grep/manual/html_node/Character-Classes-and-Bracket-Expressions.html
* */
private static final Pattern SEPARATORS = Pattern.compile("[\\s\\p{Punct}&&[^-]]");
/**
* The SEPARATORS pattern matches those characters which will be replaced with `-`. This includes spaces and
* punctuation characters.
* <a href="https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/regex/Pattern.html#posix">Details on {@code Punct}</a>.
*/
private static final Pattern SEPARATORS = Pattern.compile("[\\s\\p{Punct}]+");
/**
* Creates URL safe text aka slug from the input text. It supports english locale only.
@ -36,7 +36,8 @@ public class TextUtils {
if (inputText == null) {
return "";
}
// remove all the separators with a `-`
// Replace all spaces and punctuation with a `-`.
String noseparators = SEPARATORS.matcher(inputText).replaceAll("-");
String normalized = Normalizer.normalize(noseparators, Normalizer.Form.NFD);

3
app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java
View File

@ -6,6 +6,7 @@ import com.appsmith.server.domains.Layout;
import com.appsmith.server.domains.NewPage;
import com.appsmith.server.dtos.ApplicationPagesDTO;
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.dtos.PageUpdateDTO;
import com.appsmith.server.services.CrudService;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@ -62,7 +63,7 @@ public interface NewPageServiceCE extends CrudService<NewPage, String> {
Mono<PageDTO> updatePage(String pageId, PageDTO page);
Mono<PageDTO> updatePageByDefaultPageIdAndBranch(String defaultPageId, PageDTO page, String branchName);
Mono<PageDTO> updatePageByDefaultPageIdAndBranch(String defaultPageId, PageUpdateDTO page, String branchName);
Mono<NewPage> save(NewPage page);

6
app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java
View File

@ -13,6 +13,7 @@ import com.appsmith.server.domains.NewPage;
import com.appsmith.server.dtos.ApplicationPagesDTO;
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.dtos.PageNameIdDTO;
import com.appsmith.server.dtos.PageUpdateDTO;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.ResponseUtils;
@ -527,10 +528,11 @@ public class NewPageServiceCEImpl extends BaseService<NewPageRepository, NewPage
}
@Override
public Mono<PageDTO> updatePageByDefaultPageIdAndBranch(String defaultPageId, PageDTO page, String branchName) {
public Mono<PageDTO> updatePageByDefaultPageIdAndBranch(
String defaultPageId, PageUpdateDTO page, String branchName) {
return repository
.findPageByBranchNameAndDefaultPageId(branchName, defaultPageId, pagePermission.getEditPermission())
.flatMap(newPage -> updatePage(newPage.getId(), page))
.flatMap(newPage -> updatePage(newPage.getId(), page.toPageDTO()))
.map(responseUtils::updatePageDTOWithDefaultResources);
}

61
app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/PageControllerTest.java Normal file
View File

@ -0,0 +1,61 @@
package com.appsmith.server.controllers;
import com.appsmith.server.configurations.SecurityTestConfig;
import com.appsmith.server.newpages.base.NewPageService;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.context.annotation.Import;
import org.springframework.http.MediaType;
import org.springframework.security.test.context.support.WithMockUser;
import org.springframework.test.web.reactive.server.WebTestClient;
import org.springframework.web.reactive.function.BodyInserters;
import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.verifyNoInteractions;
@SpringBootTest
@AutoConfigureWebTestClient
@Import({SecurityTestConfig.class})
public class PageControllerTest {
@Autowired
private WebTestClient client;
@MockBean
private NewPageService newPageService;
@Test
@WithMockUser
void noBody() {
client.put().uri("/api/v1/pages/abcdef").exchange().expectStatus().isBadRequest();
}
@ParameterizedTest
@ValueSource(
strings = {
"../mal", "..\\mal", "/mal", "C:\\mal", "mal/", "/mal/", "\\mal", "mal\\", "\\mal\\", ":mal", ":mal/",
":mal\\"
})
@WithMockUser
void invalidName(String name) {
client.put()
.uri("/api/v1/pages/abcdef")
.contentType(MediaType.APPLICATION_JSON)
.body(BodyInserters.fromValue(Map.of("name", name)))
.exchange()
.expectStatus()
.isBadRequest()
.expectBody()
.jsonPath("$.errorDisplay")
.value(s -> assertThat((String) s).contains("Validation Failure"));
verifyNoInteractions(newPageService);
}
}