From 8e568d6056890137ebe53e6c75cc1e00a5490842 Mon Sep 17 00:00:00 2001 From: Nayan <83352306+nayan-rafiq@users.noreply.github.com> Date: Mon, 1 Nov 2021 14:18:21 +0600 Subject: [PATCH] feat: [Feature] Add slug for applications and pages (#8860) Generates and stores a slug from application name and page names when they are created or updated. Also adds a migration to set slug to existing applications and pages. --- .../appsmith/server/domains/Application.java | 2 + .../com/appsmith/server/dtos/PageDTO.java | 2 + .../appsmith/server/helpers/TextUtils.java | 45 +++++++++++++++++++ .../server/migrations/DatabaseChangelog.java | 40 +++++++++++++++++ .../services/ApplicationServiceImpl.java | 12 ++++- .../server/services/NewPageServiceImpl.java | 10 +++-- .../server/helpers/TextUtilsTest.java | 30 +++++++++++++ .../services/ApplicationServiceTest.java | 3 ++ .../server/services/PageServiceTest.java | 8 +++- 9 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java create mode 100644 app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/TextUtilsTest.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java index 9483ce56c4..45764849e2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java @@ -59,6 +59,8 @@ public class Application extends BaseDomain { String icon; + private String slug; + @JsonIgnore AppLayout unpublishedAppLayout; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java index 389b5c31a0..6e87d3fdc5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java @@ -25,6 +25,8 @@ public class PageDTO { String name; + String slug; + @Transient String applicationId; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java new file mode 100644 index 0000000000..2eb36663b8 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java @@ -0,0 +1,45 @@ +package com.appsmith.server.helpers; + +import lombok.extern.slf4j.Slf4j; + +import java.text.Normalizer; +import java.util.Locale; +import java.util.regex.Pattern; + +@Slf4j +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_-]"); + + /* + * 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}&&[^-]]"); + + /** + * Creates URL safe text aka slug from the input text. It supports english locale only. + * See the test cases for sample conversions + * For other languages, it'll return empty. + * @param inputText String that'll be converted + * @return String, empty if failed due to encoding exception + */ + public static String makeSlug(String inputText) { + if(inputText == null) { + return ""; + } + // remove all the separators with a `-` + String noseparators = SEPARATORS.matcher(inputText).replaceAll("-"); + String normalized = Normalizer.normalize(noseparators, Normalizer.Form.NFD); + + // remove any non ascii letter with empty + String slug = NON_LATIN.matcher(normalized).replaceAll(""); + // convert to lower case, remove multiple consecutive `-` with single `-` + // if we've only `-` left and nothing else, replace it with empty string + return slug.toLowerCase(Locale.ENGLISH).replaceAll("-{2,}","-").replaceAll("^-|-$",""); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index 3f7adc85cf..fed3011da9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -35,6 +35,7 @@ import com.appsmith.server.domains.PluginType; import com.appsmith.server.domains.QApplication; import com.appsmith.server.domains.QConfig; import com.appsmith.server.domains.QNewAction; +import com.appsmith.server.domains.QNewPage; import com.appsmith.server.domains.QOrganization; import com.appsmith.server.domains.QPlugin; import com.appsmith.server.domains.Role; @@ -46,6 +47,7 @@ import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.dtos.DslActionDTO; import com.appsmith.server.dtos.OrganizationPluginStatus; import com.appsmith.server.dtos.PageDTO; +import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.services.OrganizationService; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.cloudyrock.mongock.ChangeLog; @@ -3580,4 +3582,42 @@ public class DatabaseChangelog { // Now that the actions have completed the migrations, update the plugin to use the new UI form. mongockTemplate.save(s3Plugin); } + + @ChangeSet(order = "094", id = "set-slug-to-application-and-page", author = "") + public void setSlugToApplicationAndPage(MongockTemplate mongockTemplate) { + // update applications + List applications = mongockTemplate.findAll(Application.class); + for(Application application : applications) { + mongockTemplate.updateFirst( + query(where(fieldName(QApplication.application.id)).is(application.getId()) + .and(fieldName(QApplication.application.deleted)).is(false)), + new Update().set(fieldName(QApplication.application.slug), TextUtils.makeSlug(application.getName())), + Application.class + ); + } + + // update pages + List pages = mongockTemplate.findAll(NewPage.class); + for(NewPage page : pages) { + Update update = new Update(); + if(page.getUnpublishedPage() != null) { + String fieldName = String.format("%s.%s", + fieldName(QNewPage.newPage.unpublishedPage), fieldName(QNewPage.newPage.unpublishedPage.slug) + ); + update = update.set(fieldName, TextUtils.makeSlug(page.getUnpublishedPage().getName())); + } + if(page.getPublishedPage() != null) { + String fieldName = String.format("%s.%s", + fieldName(QNewPage.newPage.publishedPage), fieldName(QNewPage.newPage.publishedPage.slug) + ); + update = update.set(fieldName, TextUtils.makeSlug(page.getPublishedPage().getName())); + } + mongockTemplate.updateFirst( + query(where(fieldName(QNewPage.newPage.id)).is(page.getId()) + .and(fieldName(QNewPage.newPage.deleted)).is(false)), + update, + NewPage.class + ); + } + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java index 6007573855..1c2190359a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java @@ -19,6 +19,7 @@ import com.appsmith.server.dtos.ApplicationAccessDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.PolicyUtils; +import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.CommentThreadRepository; import com.jcraft.jsch.JSch; @@ -146,6 +147,9 @@ public class ApplicationServiceImpl extends BaseService save(Application application) { + if(!StringUtils.isEmpty(application.getName())) { + application.setSlug(TextUtils.makeSlug(application.getName())); + } return repository.save(application) .flatMap(this::setTransientFields); } @@ -156,13 +160,17 @@ public class ApplicationServiceImpl extends BaseService createDefault(Application object) { - return super.create(object); + public Mono createDefault(Application application) { + application.setSlug(TextUtils.makeSlug(application.getName())); + return super.create(application); } @Override public Mono update(String id, Application application) { application.setIsPublic(null); + if(!StringUtils.isEmpty(application.getName())) { + application.setSlug(TextUtils.makeSlug(application.getName())); + } return repository.updateById(id, application, AclPermission.MANAGE_APPLICATIONS) .onErrorResume(error -> { if (error instanceof DuplicateKeyException) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java index d7e58f6cee..458c4fde2a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java @@ -11,6 +11,7 @@ import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.dtos.PageNameIdDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.repositories.NewPageRepository; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONObject; @@ -21,6 +22,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.mongodb.core.ReactiveMongoTemplate; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; @@ -128,8 +130,8 @@ public class NewPageServiceImpl extends BaseService createDefault(PageDTO object) { NewPage newPage = new NewPage(); newPage.setUnpublishedPage(object); - newPage.setApplicationId(object.getApplicationId()); + newPage.getUnpublishedPage().setSlug(TextUtils.makeSlug(object.getName())); newPage.setPolicies(object.getPolicies()); if (newPage.getGitSyncId() == null) { newPage.setGitSyncId(newPage.getApplicationId() + "_" + Instant.now().toString()); @@ -379,13 +381,13 @@ public class NewPageServiceImpl extends BaseService updatePage(String id, PageDTO page) { - NewPage newPage = new NewPage(); - newPage.setUnpublishedPage(page); - return repository.findById(id, AclPermission.MANAGE_PAGES) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, id))) .flatMap(dbPage -> { copyNewFieldValuesIntoOldObject(page, dbPage.getUnpublishedPage()); + if(!StringUtils.isEmpty(page.getName())) { + dbPage.getUnpublishedPage().setSlug(TextUtils.makeSlug(page.getName())); + } return this.update(id, dbPage); }) .flatMap(savedPage -> applicationService.saveLastEditInformation(savedPage.getApplicationId()) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/TextUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/TextUtilsTest.java new file mode 100644 index 0000000000..1e6a1a4950 --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/TextUtilsTest.java @@ -0,0 +1,30 @@ +package com.appsmith.server.helpers; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TextUtilsTest { + @Test + public void makeSlug() { + assertThat(TextUtils.makeSlug("Hello Darkness My Old Friend")).isEqualTo("hello-darkness-my-old-friend"); + assertThat(TextUtils.makeSlug("Page1")).isEqualTo("page1"); + assertThat(TextUtils.makeSlug("TestPage123")).isEqualTo("testpage123"); + assertThat(TextUtils.makeSlug("Page 1")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug(" Page 1 ")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug("Page 1")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug("_page 1")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug("!page 1")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug("page 1!")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug("page__1")).isEqualTo("page-1"); + assertThat(TextUtils.makeSlug("Hello (new)")).isEqualTo("hello-new"); + assertThat(TextUtils.makeSlug("")).isEqualTo(""); + assertThat(TextUtils.makeSlug(null)).isEqualTo(""); + // text is hindi + assertThat(TextUtils.makeSlug("परीक्षण पृष्ठ")).isEqualTo(""); + // text is in spanish + assertThat(TextUtils.makeSlug("página de prueba")).isEqualTo("pagina-de-prueba"); + // text in chinese + assertThat(TextUtils.makeSlug("测试页")).isEqualTo(""); + } +} \ No newline at end of file diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java index 0109e85546..e14dc6f5ce 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java @@ -30,6 +30,7 @@ import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.MockPluginExecutor; import com.appsmith.server.helpers.PluginExecutorHelper; import com.appsmith.server.helpers.PolicyUtils; +import com.appsmith.server.helpers.TextUtils; import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.NewPageRepository; import com.appsmith.server.repositories.PluginRepository; @@ -173,6 +174,7 @@ public class ApplicationServiceTest { .create(applicationMono) .assertNext(application -> { assertThat(application).isNotNull(); + assertThat(application.getSlug()).isEqualTo(TextUtils.makeSlug(application.getName())); assertThat(application.isAppIsExample()).isFalse(); assertThat(application.getId()).isNotNull(); assertThat(application.getName().equals("ApplicationServiceTest TestApp")); @@ -325,6 +327,7 @@ public class ApplicationServiceTest { assertThat(t.getId()).isNotNull(); assertThat(t.getPolicies()).isNotEmpty(); assertThat(t.getName()).isEqualTo("NewValidUpdateApplication-Test"); + assertThat(t.getSlug()).isEqualTo(TextUtils.makeSlug(t.getName())); }) .verifyComplete(); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index 34a250b6fa..6c23b7cada 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -19,6 +19,7 @@ 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.helpers.TextUtils; import com.appsmith.server.repositories.PluginRepository; import lombok.extern.slf4j.Slf4j; import net.minidev.json.JSONArray; @@ -165,7 +166,9 @@ public class PageServiceTest { .assertNext(page -> { assertThat(page).isNotNull(); assertThat(page.getId()).isNotNull(); - assertThat("PageServiceTest TestApp".equals(page.getName())); + + assertThat(page.getName()).isEqualTo("PageServiceTest TestApp"); + assertThat(page.getSlug()).isEqualTo(TextUtils.makeSlug(page.getName())); assertThat(page.getPolicies()).isNotEmpty(); assertThat(page.getPolicies()).containsOnly(managePagePolicy, readPagePolicy); @@ -243,7 +246,8 @@ public class PageServiceTest { .assertNext(page -> { assertThat(page).isNotNull(); assertThat(page.getId()).isNotNull(); - assertThat("New Page Name".equals(page.getName())); + assertThat(page.getName()).isEqualTo("New Page Name"); + assertThat(page.getSlug()).isEqualTo(TextUtils.makeSlug(page.getName())); // Check for the policy object not getting overwritten during update assertThat(page.getPolicies()).isNotEmpty();