From 51744a6727e1a9fdf500d383d63cc412e79d7728 Mon Sep 17 00:00:00 2001 From: Nayan Date: Wed, 8 Jun 2022 13:25:00 +0600 Subject: [PATCH] feat: Return recently used templates first (#13996) If user has recently used templates set, those should come first in the response of get templates API --- app/server/appsmith-server/pom.xml | 12 ++ .../ce/ApplicationTemplateControllerCE.java | 4 +- .../ce/ApplicationTemplateServiceCE.java | 4 +- .../ce/ApplicationTemplateServiceCEImpl.java | 51 ++++--- .../ApplicationTemplateServiceTest.java | 138 ++++++++++++++++++ 5 files changed, 185 insertions(+), 24 deletions(-) create mode 100644 app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationTemplateServiceTest.java diff --git a/app/server/appsmith-server/pom.xml b/app/server/appsmith-server/pom.xml index 541cc19ed2..328ad0c6f9 100644 --- a/app/server/appsmith-server/pom.xml +++ b/app/server/appsmith-server/pom.xml @@ -150,6 +150,18 @@ test + + + com.squareup.okhttp3 + okhttp + test + + + com.squareup.okhttp3 + mockwebserver + test + + diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationTemplateControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationTemplateControllerCE.java index e4a5b490ba..1a9b453316 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationTemplateControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationTemplateControllerCE.java @@ -27,7 +27,7 @@ public class ApplicationTemplateControllerCE { @GetMapping public Mono>> getAll() { - return applicationTemplateService.getActiveTemplates(null).collectList() + return applicationTemplateService.getActiveTemplates(null) .map(templates -> new ResponseDTO<>(HttpStatus.OK.value(), templates, null)); } @@ -58,7 +58,7 @@ public class ApplicationTemplateControllerCE { @GetMapping("recent") public Mono>> getRecentlyUsedTemplates() { - return applicationTemplateService.getRecentlyUsedTemplates().collectList() + return applicationTemplateService.getRecentlyUsedTemplates() .map(templates -> new ResponseDTO<>(HttpStatus.OK.value(), templates, null)); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCE.java index 914bcdb7c4..91581bd5b7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCE.java @@ -8,9 +8,9 @@ import reactor.core.publisher.Mono; import java.util.List; public interface ApplicationTemplateServiceCE { - Flux getActiveTemplates(List templateIds); + Mono> getActiveTemplates(List templateIds); Flux getSimilarTemplates(String templateId); - Flux getRecentlyUsedTemplates(); + Mono> getRecentlyUsedTemplates(); Mono getTemplateDetails(String templateId); Mono importApplicationFromTemplate(String templateId, String organizationId); Mono mergeTemplateWithApplication(String templateId, String applicationId, String organizationId, String branchName, List pagesToImport); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCEImpl.java index 1e84852a22..df12eb9972 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationTemplateServiceCEImpl.java @@ -1,10 +1,11 @@ package com.appsmith.server.services.ce; -import com.appsmith.server.configurations.CloudServicesConfig; import com.appsmith.external.constants.AnalyticsEvents; +import com.appsmith.server.configurations.CloudServicesConfig; import com.appsmith.server.converters.GsonISOStringToInstantConverter; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.ApplicationJson; +import com.appsmith.server.domains.UserData; import com.appsmith.server.dtos.ApplicationTemplate; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; @@ -28,8 +29,8 @@ import reactor.core.publisher.Mono; import java.lang.reflect.Type; import java.time.Instant; import java.util.Comparator; -import java.util.List; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.List; @@ -73,20 +74,20 @@ public class ApplicationTemplateServiceCEImpl implements ApplicationTemplateServ } @Override - public Flux getActiveTemplates(List templateIds) { + public Mono> getActiveTemplates(List templateIds) { final String baseUrl = cloudServicesConfig.getBaseUrl(); UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.newInstance() .queryParam("version", releaseNotesService.getReleasedVersion()); - if(!CollectionUtils.isEmpty(templateIds)) { + if (!CollectionUtils.isEmpty(templateIds)) { uriComponentsBuilder.queryParam("id", templateIds); } // uriComponents will build url in format: version=version&id=id1&id=id2&id=id3 UriComponents uriComponents = uriComponentsBuilder.build(); - Flux applicationTemplateFlux = WebClient + return WebClient .create(baseUrl + "/api/v1/app-templates?" + uriComponents.getQuery()) .get() .exchangeToFlux(clientResponse -> { @@ -97,17 +98,26 @@ public class ApplicationTemplateServiceCEImpl implements ApplicationTemplateServ } else { return clientResponse.createException().flatMapMany(Flux::error); } + }) + .collectList().zipWith(userDataService.getForCurrentUser()) + .map(objects -> { + List applicationTemplateList = objects.getT1(); + UserData userData = objects.getT2(); + List recentlyUsedTemplateIds = userData.getRecentlyUsedTemplateIds(); + if (!CollectionUtils.isEmpty(recentlyUsedTemplateIds)) { + applicationTemplateList.sort( + Comparator.comparingInt(o -> { + int index = recentlyUsedTemplateIds.indexOf(o.getId()); + if (index < 0) { + // template not in recent list, return a large value so that it's sorted out to the end + index = Integer.MAX_VALUE; + } + return index; + }) + ); + } + return applicationTemplateList; }); - - if(!CollectionUtils.isEmpty(templateIds)) { - // the items should be sorted based on the order of the templateIds when it's not empty - applicationTemplateFlux = applicationTemplateFlux.sort( - // sort based on index of id in templateIds list. - // index of first item will be less than index of next item - Comparator.comparingInt(o -> templateIds.indexOf(o.getId())) - ); - } - return applicationTemplateFlux; } @Override @@ -173,12 +183,13 @@ public class ApplicationTemplateServiceCEImpl implements ApplicationTemplateServ } @Override - public Flux getRecentlyUsedTemplates() { - return userDataService.getForCurrentUser().flatMapMany(userData -> { - if(!CollectionUtils.isEmpty(userData.getRecentlyUsedTemplateIds())) { - return getActiveTemplates(userData.getRecentlyUsedTemplateIds()); + public Mono> getRecentlyUsedTemplates() { + return userDataService.getForCurrentUser().flatMap(userData -> { + List templateIds = userData.getRecentlyUsedTemplateIds(); + if(!CollectionUtils.isEmpty(templateIds)) { + return getActiveTemplates(templateIds); } - return Flux.just(); + return Mono.empty(); }); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationTemplateServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationTemplateServiceTest.java new file mode 100644 index 0000000000..f23e6fc4ed --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationTemplateServiceTest.java @@ -0,0 +1,138 @@ +package com.appsmith.server.services; + +import com.appsmith.server.configurations.CloudServicesConfig; +import com.appsmith.server.domains.UserData; +import com.appsmith.server.dtos.ApplicationTemplate; +import com.appsmith.server.solutions.ImportExportApplicationService; +import com.appsmith.server.solutions.ReleaseNotesService; +import com.fasterxml.jackson.core.JsonProcessingException; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.io.IOException; +import java.util.List; + +import static com.appsmith.server.migrations.DatabaseChangelog.objectMapper; +import static org.assertj.core.api.Assertions.assertThat; + +/** + * This test is written based on the inspiration from the tutorial: https://www.baeldung.com/spring-mocking-webclient + */ +@RunWith(SpringJUnit4ClassRunner.class) +public class ApplicationTemplateServiceTest { + ApplicationTemplateService applicationTemplateService; + + @MockBean + private UserDataService userDataService; + @MockBean + private CloudServicesConfig cloudServicesConfig; + @MockBean + private ReleaseNotesService releaseNotesService; + @MockBean + private ImportExportApplicationService importExportApplicationService; + @MockBean + private AnalyticsService analyticsService; + + private static MockWebServer mockCloudServices; + + @BeforeClass + public static void setUp() throws IOException { + mockCloudServices = new MockWebServer(); + mockCloudServices.start(); + } + + @AfterClass + public static void tearDown() throws IOException { + mockCloudServices.shutdown(); + } + + @Before + public void initialize() { + String baseUrl = String.format("http://localhost:%s", mockCloudServices.getPort()); + + // mock the cloud services config so that it returns mock server url as cloud service base url + Mockito.when(cloudServicesConfig.getBaseUrl()).thenReturn(baseUrl); + + applicationTemplateService = new ApplicationTemplateServiceImpl( + cloudServicesConfig, releaseNotesService, importExportApplicationService, analyticsService, userDataService + ); + } + + private ApplicationTemplate create(String id, String title) { + ApplicationTemplate applicationTemplate = new ApplicationTemplate(); + applicationTemplate.setId(id); + applicationTemplate.setTitle(title); + return applicationTemplate; + } + + @Test + public void getActiveTemplates_WhenRecentlyUsedExists_RecentOnesComesFirst() throws JsonProcessingException { + ApplicationTemplate templateOne = create("id-one", "First template"); + ApplicationTemplate templateTwo = create("id-two", "Seonds template"); + ApplicationTemplate templateThree = create("id-three", "Third template"); + + // mock the server to return the above three templates + mockCloudServices + .enqueue(new MockResponse() + .setBody(objectMapper.writeValueAsString(List.of(templateOne, templateTwo, templateThree))) + .addHeader("Content-Type", "application/json")); + + // mock the user data to set second template as recently used + UserData mockUserData = new UserData(); + mockUserData.setRecentlyUsedTemplateIds(List.of("id-two")); + Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(mockUserData)); + + Mono> templateListMono = applicationTemplateService.getActiveTemplates(null); + + StepVerifier.create(templateListMono).assertNext(applicationTemplates -> { + assertThat(applicationTemplates.size()).isEqualTo(3); + assertThat(applicationTemplates.get(0).getId()).isEqualTo("id-two"); // second one should come first + }).verifyComplete(); + } + + @Test + public void getRecentlyUsedTemplates_WhenNoRecentTemplate_ReturnsEmpty() { + // mock the user data to that has no recent template + Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(new UserData())); + + StepVerifier.create(applicationTemplateService.getRecentlyUsedTemplates()) + .verifyComplete(); + } + + @Test + public void getRecentlyUsedTemplates_WhenRecentTemplatesExist_ReturnsTemplates() throws InterruptedException, JsonProcessingException { + // mock the user data to set recently used template ids + UserData mockUserData = new UserData(); + mockUserData.setRecentlyUsedTemplateIds(List.of("id-one", "id-two")); + Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(mockUserData)); + + // mock the server to return a template when it's called + mockCloudServices + .enqueue(new MockResponse() + .setBody(objectMapper.writeValueAsString(List.of(create("id-one", "First template")))) + .addHeader("Content-Type", "application/json")); + + // make sure we've received the response returned by the mockCloudServices + StepVerifier.create(applicationTemplateService.getRecentlyUsedTemplates()) + .assertNext(applicationTemplates -> assertThat(applicationTemplates.size()).isEqualTo(1)) + .verifyComplete(); + + // verify that mockCloudServices was called with the query param id i.e. id=id-one&id=id-two + RecordedRequest recordedRequest = mockCloudServices.takeRequest(); + List queryParameterValues = recordedRequest.getRequestUrl().queryParameterValues("id"); + assertThat(queryParameterValues).contains("id-one"); + assertThat(queryParameterValues).contains("id-two"); + assertThat(queryParameterValues.size()).isEqualTo(2); + } +} \ No newline at end of file