From 1aa3e7537f7d2ecc9e3531e0f2bd20b8a0b1945b Mon Sep 17 00:00:00 2001 From: zonia3000 Date: Thu, 15 Dec 2022 17:23:11 +0100 Subject: [PATCH] ENG-4280 Allowed null custom UI for widgets having a default UI --- .../services/widgettype/WidgetService.java | 33 +++-- .../widgettype/WidgetServiceTest.java | 130 ++++++++++++++++-- .../WidgetControllerIntegrationTest.java | 64 ++++++++- 3 files changed, 195 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetService.java b/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetService.java index 3e9d1d28d..d684e0e60 100644 --- a/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetService.java +++ b/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetService.java @@ -229,31 +229,34 @@ public WidgetDto updateWidget(String widgetCode, WidgetRequest widgetUpdateReque try { if (null == this.getGroupManager().getGroup(widgetUpdateRequest.getGroup())) { BeanPropertyBindingResult bindingResult = new BeanPropertyBindingResult(type, "widget"); - bindingResult.reject(WidgetValidator.ERRCODE_WIDGET_GROUP_INVALID, new String[]{widgetUpdateRequest.getGroup()}, "widgettype.group.invalid"); + bindingResult.reject(WidgetValidator.ERRCODE_WIDGET_GROUP_INVALID, + new String[]{widgetUpdateRequest.getGroup()}, "widgettype.group.invalid"); throw new ValidationGenericException(bindingResult); } this.processWidgetType(type, widgetUpdateRequest); String customUi = this.extractCustomUi(type, widgetUpdateRequest); + GuiFragment guiFragment = this.getGuiFragmentManager().getUniqueGuiFragmentByWidgetType(widgetCode); if (type.isUserType() && StringUtils.isBlank(customUi) - && !type.isLogic() - && !WidgetType.existsJsp(this.srvCtx, widgetCode, type.getPluginCode())) { + && !type.isLogic() + && !(WidgetType.existsJsp(this.srvCtx, widgetCode, type.getPluginCode()) + || (guiFragment != null && !(StringUtils.isBlank(guiFragment.getDefaultGui()))))) { BeanPropertyBindingResult bindingResult = new BeanPropertyBindingResult(type, "widget"); - bindingResult.reject(WidgetValidator.ERRCODE_NOT_BLANK, new String[]{type.getCode()}, "widgettype.customUi.notBlank"); + bindingResult.reject(WidgetValidator.ERRCODE_NOT_BLANK, new String[]{type.getCode()}, + "widgettype.customUi.notBlank"); throw new ValidationGenericException(bindingResult); } widgetDto = dtoBuilder.convert(type); - this.getWidgetManager().updateWidgetType(widgetCode, type.getTitles(), type.getConfig(), type.getMainGroup(), - type.getConfigUi(), type.getBundleId(), type.isReadonlyPageWidgetConfig(), type.getWidgetCategory(), - type.getIcon()); - if (!StringUtils.isEmpty(widgetCode)) { - GuiFragment guiFragment = this.getGuiFragmentManager().getUniqueGuiFragmentByWidgetType(widgetCode); - if (null == guiFragment) { - this.createAndAddFragment(type, customUi); - } else { - guiFragment.setGui(customUi); - this.getGuiFragmentManager().updateGuiFragment(guiFragment); - } + this.getWidgetManager() + .updateWidgetType(widgetCode, type.getTitles(), type.getConfig(), type.getMainGroup(), + type.getConfigUi(), type.getBundleId(), type.isReadonlyPageWidgetConfig(), + type.getWidgetCategory(), + type.getIcon()); + if (null == guiFragment) { + this.createAndAddFragment(type, customUi); + } else { + guiFragment.setGui(customUi); + this.getGuiFragmentManager().updateGuiFragment(guiFragment); } this.addFragments(widgetDto); } catch (ValidationGenericException vge) { diff --git a/src/test/java/org/entando/entando/aps/system/services/widgettype/WidgetServiceTest.java b/src/test/java/org/entando/entando/aps/system/services/widgettype/WidgetServiceTest.java index e688c0867..b190f41fa 100644 --- a/src/test/java/org/entando/entando/aps/system/services/widgettype/WidgetServiceTest.java +++ b/src/test/java/org/entando/entando/aps/system/services/widgettype/WidgetServiceTest.java @@ -13,6 +13,21 @@ */ package org.entando.entando.aps.system.services.widgettype; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import com.agiletec.aps.system.services.group.Group; import com.agiletec.aps.system.services.group.IGroupManager; import com.agiletec.aps.system.services.page.IPage; @@ -22,6 +37,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import javax.servlet.ServletContext; import org.entando.entando.aps.system.init.IComponentManager; import org.entando.entando.aps.system.services.assertionhelper.WidgetAssertionHelper; import org.entando.entando.aps.system.services.guifragment.GuiFragment; @@ -33,6 +58,7 @@ import org.entando.entando.aps.system.services.widgettype.model.WidgetDtoBuilder; import org.entando.entando.ent.exception.EntException; import org.entando.entando.web.common.assembler.PagedMetadataMapper; +import org.entando.entando.web.common.exceptions.ValidationGenericException; import org.entando.entando.web.common.model.Filter; import org.entando.entando.web.common.model.FilterOperator; import org.entando.entando.web.common.model.PagedMetadata; @@ -49,17 +75,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; - -import java.util.*; -import java.util.stream.Collectors; -import java.util.stream.IntStream; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; +import org.springframework.util.FileSystemUtils; @ExtendWith(MockitoExtension.class) class WidgetServiceTest { @@ -93,6 +109,9 @@ class WidgetServiceTest { @Mock private PagedMetadataMapper pagedMetadataMapper; + @Mock + private ServletContext srvCtx; + @InjectMocks private WidgetService widgetService; @@ -467,8 +486,97 @@ void shouldNotThrowExceptionIfWidgetItsBroken() throws Exception { assertThat(result.getBody()).hasSize(0); } + @Test + void updateWidgetWithDefaultUiAndNoCustomUi() throws Exception { + + WidgetRequest widgetRequest = getWidgetUpdateRequest1(); + widgetRequest.setCustomUi(null); + + WidgetType type = getWidget1(); + type.setLocked(false); + + when(widgetManager.getWidgetType(WIDGET_1_CODE)).thenReturn(type); + when(groupManager.getGroup(widgetRequest.getGroup())).thenReturn(mock(Group.class)); + GuiFragment mockedFragment = mock(GuiFragment.class); + when(mockedFragment.getDefaultGui()).thenReturn("default-gui"); + when(guiFragmentManager.getUniqueGuiFragmentByWidgetType(any())).thenReturn(mockedFragment); + + widgetService.updateWidget(WIDGET_1_CODE, widgetRequest); + + verify(widgetManager).updateWidgetType(eq(WIDGET_1_CODE), any(), any(), anyString(), anyString(), + anyString(), anyBoolean(), anyString(), anyString()); + verify(guiFragmentManager).updateGuiFragment(argThat(fragment -> fragment.getGui() == null)); + } + + @Test + void updateWidgetWithExistingJspAndNoCustomUi() throws Exception { + + String jspRelativePath = WidgetType.getJspPath(WIDGET_1_CODE, null).substring(1); + Path tmpDir = Files.createTempDirectory(null); + Path jspPath = tmpDir.resolve(jspRelativePath); + jspPath.getParent().toFile().mkdirs(); + jspPath.toFile().createNewFile(); + Mockito.when(srvCtx.getRealPath("/")).thenReturn(tmpDir.toAbsolutePath().toString()); + + try { + WidgetRequest widgetRequest = getWidgetUpdateRequest1(); + widgetRequest.setCustomUi(null); + + WidgetType type = getWidget1(); + type.setLocked(false); + + when(widgetManager.getWidgetType(WIDGET_1_CODE)).thenReturn(type); + when(groupManager.getGroup(widgetRequest.getGroup())).thenReturn(mock(Group.class)); + + widgetService.updateWidget(WIDGET_1_CODE, widgetRequest); + + verify(widgetManager).updateWidgetType(eq(WIDGET_1_CODE), any(), any(), anyString(), anyString(), + anyString(), anyBoolean(), anyString(), anyString()); + verify(guiFragmentManager, never()).updateGuiFragment(any()); + } finally { + FileSystemUtils.deleteRecursively(tmpDir.toFile()); + } + } + + @Test + void updateWidgetWithoutDefaultUiAndCustomUi() throws Exception { + + WidgetRequest widgetRequest = getWidgetUpdateRequest1(); + widgetRequest.setCustomUi(null); + + WidgetType type = getWidget1(); + type.setLocked(false); + when(widgetManager.getWidgetType(WIDGET_1_CODE)).thenReturn(type); + when(groupManager.getGroup(widgetRequest.getGroup())).thenReturn(mock(Group.class)); + GuiFragment mockedFragment = mock(GuiFragment.class); + when(guiFragmentManager.getUniqueGuiFragmentByWidgetType(any())).thenReturn(mockedFragment); + + ValidationGenericException ex = assertThrows(ValidationGenericException.class, + () -> widgetService.updateWidget(WIDGET_1_CODE, widgetRequest)); + + assertEquals(1, ex.getBindingResult().getAllErrors().size()); + assertEquals("widgettype.customUi.notBlank", ex.getBindingResult().getAllErrors().get(0).getDefaultMessage()); + } + + @Test + void updateWidgetWithoutGuiFragmentAndCustomUi() throws Exception { + WidgetRequest widgetRequest = getWidgetUpdateRequest1(); + widgetRequest.setCustomUi(null); + + WidgetType type = getWidget1(); + type.setLocked(false); + + when(widgetManager.getWidgetType(WIDGET_1_CODE)).thenReturn(type); + when(groupManager.getGroup(widgetRequest.getGroup())).thenReturn(mock(Group.class)); + + ValidationGenericException ex = assertThrows(ValidationGenericException.class, + () -> widgetService.updateWidget(WIDGET_1_CODE, widgetRequest)); + + assertEquals(1, ex.getBindingResult().getAllErrors().size()); + assertEquals("widgettype.customUi.notBlank", ex.getBindingResult().getAllErrors().get(0).getDefaultMessage()); + } /** * init mock for a multipaged request diff --git a/src/test/java/org/entando/entando/web/widget/WidgetControllerIntegrationTest.java b/src/test/java/org/entando/entando/web/widget/WidgetControllerIntegrationTest.java index 3e9c50736..345822535 100644 --- a/src/test/java/org/entando/entando/web/widget/WidgetControllerIntegrationTest.java +++ b/src/test/java/org/entando/entando/web/widget/WidgetControllerIntegrationTest.java @@ -19,6 +19,7 @@ import com.agiletec.aps.system.services.user.UserDetails; import com.fasterxml.jackson.databind.ObjectMapper; import java.util.List; +import org.entando.entando.aps.system.services.guifragment.GuiFragment; import org.entando.entando.aps.system.services.guifragment.IGuiFragmentManager; import org.entando.entando.aps.system.services.widgettype.IWidgetTypeManager; import org.entando.entando.aps.system.services.widgettype.WidgetType; @@ -42,6 +43,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; @@ -913,12 +915,62 @@ void testComponentExistenceAnalysis() throws Exception { new ContextOfControllerTests(mockMvc, mapper) ); } - + + /** + * Non-logical widgets having a default UI can be saved without custom UI. + * When the custom UI is null in the database, the API populates it using the value of default UI. + */ + @Test + void testUpdateWidgetWithNullCustomUiShouldReturnDefaultUi() throws Exception { + UserDetails user = new OAuth2TestUtils.UserBuilder("jack_bauer", "0x24").grantedToRoleAdmin().build(); + String accessToken = mockOAuthInterceptor(user); + + String widgetTypeCode = "login_form"; + + GuiFragment fragment = guiFragmentManager.getGuiFragment(widgetTypeCode); + assertNull(fragment.getGui()); + assertNotNull(fragment.getDefaultGui()); + + WidgetType type = widgetTypeManager.getWidgetType(widgetTypeCode); + + Map titles = new HashMap<>(); + titles.put("it", type.getTitles().getProperty("it")); + titles.put("en", type.getTitles().getProperty("en")); + + WidgetRequest request = new WidgetRequest(); + request.setCode(type.getCode()); + request.setTitles(titles); + request.setGroup(Group.FREE_GROUP_NAME); + request.setWidgetCategory(type.getWidgetCategory()); + request.setIcon(type.getIcon()); + + // Set a custom UI different from the default UI + request.setCustomUi("new-custom-ui"); + executeWidgetPut(request, widgetTypeCode, accessToken, status().isOk()); + + // Verify that it has been updated + executeWidgetGet(widgetTypeCode, accessToken, status().isOk()) + .andExpect(jsonPath("$.payload.guiFragments[0].defaultUi", is(fragment.getDefaultGui()))) + .andExpect(jsonPath("$.payload.guiFragments[0].customUi", is("new-custom-ui"))); + + // Set the custom UI to null + request.setCustomUi(null); + executeWidgetPut(request, widgetTypeCode, accessToken, status().isOk()); + + // Verify that customUI is equal to defaultUI + executeWidgetGet(widgetTypeCode, accessToken, status().isOk()) + .andExpect(jsonPath("$.payload.guiFragments[0].defaultUi", is(fragment.getDefaultGui()))) + .andExpect(jsonPath("$.payload.guiFragments[0].customUi", is(fragment.getDefaultGui()))); + + fragment = guiFragmentManager.getGuiFragment(widgetTypeCode); + assertNull(fragment.getGui()); + } + private ResultActions executeWidgetGet(String widgetTypeCode, String accessToken, ResultMatcher expected) throws Exception { ResultActions result = mockMvc .perform(get("/widgets/{code}", new Object[]{widgetTypeCode}) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)); - result.andExpect(expected); + result.andDo(resultPrint()).andExpect(expected); return result; } @@ -926,7 +978,7 @@ private ResultActions executeWidgetUsage(String widgetTypeCode, String accessTok ResultActions result = mockMvc .perform(get("/widgets/{code}/usage", new Object[]{widgetTypeCode}) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)); - result.andExpect(expected); + result.andDo(resultPrint()).andExpect(expected); return result; } @@ -936,7 +988,7 @@ private ResultActions executeWidgetPost(WidgetRequest request, String accessToke .content(mapper.writeValueAsString(request)) .contentType(MediaType.APPLICATION_JSON_VALUE) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)); - result.andExpect(expected); + result.andDo(resultPrint()).andExpect(expected); return result; } @@ -946,7 +998,7 @@ private ResultActions executeWidgetPut(WidgetRequest request, String widgetTypeC .content(mapper.writeValueAsString(request)) .contentType(MediaType.APPLICATION_JSON_VALUE) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)); - result.andExpect(expected); + result.andDo(resultPrint()).andExpect(expected); return result; } @@ -954,7 +1006,7 @@ private ResultActions executeWidgetDelete(String widgetTypeCode, String accessTo ResultActions result = mockMvc.perform( delete("/widgets/{code}", new Object[]{widgetTypeCode}) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)); - result.andExpect(expected); + result.andDo(resultPrint()).andExpect(expected); return result; }