From c6535a128e3d44aae2506ebc6d8c0b3552385e00 Mon Sep 17 00:00:00 2001 From: nickAS21 Date: Wed, 24 Aug 2022 13:18:51 +0300 Subject: [PATCH] fix_bug: Entity delete with delete relations, Dashboard --- .../server/controller/AbstractWebTest.java | 59 ++++++++++++- .../BaseDashboardControllerTest.java | 88 ++++++++++--------- .../server/dao/tenant/TenantServiceImpl.java | 1 - 3 files changed, 105 insertions(+), 43 deletions(-) diff --git a/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java b/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java index 462b40135a..a145acb929 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java @@ -27,6 +27,7 @@ import io.jsonwebtoken.Jwt; import io.jsonwebtoken.Jwts; import lombok.extern.slf4j.Slf4j; import org.hamcrest.Matcher; +import org.hibernate.exception.ConstraintViolationException; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -34,6 +35,7 @@ import org.junit.Rule; import org.junit.rules.TestRule; import org.junit.rules.TestWatcher; import org.junit.runner.Description; +import org.mockito.BDDMockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; @@ -67,6 +69,7 @@ import org.thingsboard.server.common.data.device.profile.ProtoTransportPayloadCo import org.thingsboard.server.common.data.device.profile.TransportPayloadTypeConfiguration; import org.thingsboard.server.common.data.edge.Edge; import org.thingsboard.server.common.data.id.CustomerId; +import org.thingsboard.server.common.data.id.EntityId; import org.thingsboard.server.common.data.id.HasId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UUIDBased; @@ -74,20 +77,25 @@ import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.page.TimePageLink; +import org.thingsboard.server.common.data.relation.EntityRelation; import org.thingsboard.server.common.data.security.Authority; import org.thingsboard.server.config.ThingsboardSecurityConfiguration; +import org.thingsboard.server.dao.Dao; import org.thingsboard.server.dao.tenant.TenantProfileService; import org.thingsboard.server.service.mail.TestMailService; import org.thingsboard.server.service.security.auth.jwt.RefreshTokenRequest; import org.thingsboard.server.service.security.auth.rest.LoginRequest; import java.io.IOException; +import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -656,7 +664,11 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { } protected T readResponse(ResultActions result, TypeReference type) throws Exception { - byte[] content = result.andReturn().getResponse().getContentAsByteArray(); + return readResponse(result.andReturn(), type); + } + + protected T readResponse(MvcResult result, TypeReference type) throws Exception { + byte[] content = result.getResponse().getContentAsByteArray(); return mapper.readerFor(type).readValue(content); } @@ -699,4 +711,49 @@ public abstract class AbstractWebTest extends AbstractInMemoryStorageTest { return Futures.allAsList(futures); } + protected void tesEntityDaoWithRelationsOk(EntityId entityIdFrom, EntityId entityTo, String urlDelete) throws Exception { + createEntityRelation(entityIdFrom, entityTo, "TEST_TYPE"); + assertThat(getRelationsTo(entityTo)).hasSize(1); + + doDelete(urlDelete).andExpect(status().isOk()); + + assertThat(getRelationsTo(entityTo)).hasSize(0); + } + + protected void tesEntityDaoWithRelationsTransactionalException(Dao dao, EntityId entityIdFrom, EntityId entityTo, + String urlDelete) throws Exception { + entityDaoRemoveByIdWithException (dao); + createEntityRelation(entityIdFrom, entityTo, "TEST_TRANSACTIONAL_TYPE"); + assertThat(getRelationsTo(entityTo)).hasSize(1); + + doDelete(urlDelete) + .andExpect(status().isInternalServerError()); + + assertThat(getRelationsTo(entityTo)).hasSize(1); + } + + protected void entityDaoRemoveByIdWithException (Dao dao) { + BDDMockito.willThrow(new ConstraintViolationException("mock message", new SQLException(), "MOCK_CONSTRAINT")) + .given(dao).removeById(any(), any()); + } + + protected void afterTestEntityDaoRemoveByIdWithException (Dao dao) { + BDDMockito.willCallRealMethod().given(dao).removeById(any(), any()); + } + + protected void createEntityRelation(EntityId entityIdFrom, EntityId entityIdTo, String typeRelation) throws Exception { + EntityRelation relation = new EntityRelation(entityIdFrom, entityIdTo, typeRelation); + doPost("/api/relation", relation); + } + + protected List getRelationsTo(EntityId entityId) throws Exception { + String url = String.format("/api/relations?toId=%s&toType=%s", entityId.getId(), entityId.getEntityType().name()); + MvcResult mvcResult = doGet(url).andReturn(); + + switch (mvcResult.getResponse().getStatus()) { + case 200: return readResponse(mvcResult, new TypeReference<>() {}); + case 404: return Collections.emptyList(); + } + throw new AssertionError("Unexpected status " + mvcResult.getResponse().getStatus()); + } } diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseDashboardControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseDashboardControllerTest.java index f5478845a9..ba275e46f5 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseDashboardControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseDashboardControllerTest.java @@ -22,6 +22,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.thingsboard.server.common.data.Customer; import org.thingsboard.server.common.data.Dashboard; import org.thingsboard.server.common.data.DashboardInfo; @@ -31,9 +32,11 @@ import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.audit.ActionType; import org.thingsboard.server.common.data.edge.Edge; import org.thingsboard.server.common.data.id.CustomerId; +import org.thingsboard.server.common.data.id.DashboardId; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.Authority; +import org.thingsboard.server.dao.dashboard.DashboardDao; import org.thingsboard.server.dao.exception.DataValidationException; import java.util.ArrayList; @@ -50,6 +53,9 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest private Tenant savedTenant; private User tenantAdmin; + @SpyBean + private DashboardDao dashboardDao; + @Before public void beforeTest() throws Exception { loginSysAdmin(); @@ -73,18 +79,18 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest public void afterTest() throws Exception { loginSysAdmin(); + afterTestEntityDaoRemoveByIdWithException (dashboardDao); + doDelete("/api/tenant/" + savedTenant.getId().getId().toString()) .andExpect(status().isOk()); } @Test public void testSaveDashboard() throws Exception { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Mockito.reset(tbClusterService, auditLogService); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + String title = "My dashboard"; + Dashboard savedDashboard = createDashboard(title); testNotifyEntityOneTimeMsgToEdgeServiceNever(savedDashboard, savedDashboard.getId(), savedDashboard.getId(), savedTenant.getId(), tenantAdmin.getCustomerId(), tenantAdmin.getId(), tenantAdmin.getEmail(), ActionType.ADDED); @@ -93,7 +99,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest Assert.assertNotNull(savedDashboard.getId()); Assert.assertTrue(savedDashboard.getCreatedTime() > 0); Assert.assertEquals(savedTenant.getId(), savedDashboard.getTenantId()); - Assert.assertEquals(dashboard.getTitle(), savedDashboard.getTitle()); + Assert.assertEquals(title, savedDashboard.getTitle()); savedDashboard.setTitle("My new dashboard"); @@ -120,7 +126,6 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest .andExpect(status().isBadRequest()) .andExpect(statusReason(containsString(msgError))); - dashboard.setTenantId(savedTenant.getId()); testNotifyEntityEqualsOneTimeServiceNeverError(dashboard, savedTenant.getId(), tenantAdmin.getId(), tenantAdmin.getEmail(), ActionType.ADDED, new DataValidationException(msgError)); Mockito.reset(tbClusterService, auditLogService); @@ -128,9 +133,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest @Test public void testUpdateDashboardFromDifferentTenant() throws Exception { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); loginDifferentTenant(); @@ -145,9 +148,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest @Test public void testFindDashboardById() throws Exception { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); Dashboard foundDashboard = doGet("/api/dashboard/" + savedDashboard.getId().getId().toString(), Dashboard.class); Assert.assertNotNull(foundDashboard); Assert.assertEquals(savedDashboard, foundDashboard); @@ -155,9 +156,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest @Test public void testDeleteDashboard() throws Exception { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); Mockito.reset(tbClusterService, auditLogService); @@ -190,9 +189,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest @Test public void testAssignUnassignDashboardToCustomer() throws Exception { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); Customer customer = new Customer(); customer.setTitle("My customer"); @@ -230,9 +227,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest @Test public void testAssignDashboardToNonExistentCustomer() throws Exception { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); String customerIdStr = Uuids.timeBased().toString(); doPost("/api/customer/" + customerIdStr @@ -268,9 +263,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest login(tenantAdmin.getEmail(), "testPassword1"); - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); doPost("/api/customer/" + savedCustomer.getId().getId().toString() + "/dashboard/" + savedDashboard.getId().getId().toString()) @@ -300,9 +293,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest int cntEntity = 173; for (int i = 0; i < cntEntity; i++) { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("Dashboard" + i); - dashboards.add(new DashboardInfo(doPost("/api/dashboard", dashboard, Dashboard.class))); + dashboards.add(new DashboardInfo(createDashboard("Dashboard" + i))); } testNotifyManyEntityManyTimeMsgToEdgeServiceNever(new Dashboard(), new Dashboard(), @@ -334,24 +325,19 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest List dashboardsTitle1 = new ArrayList<>(); int cntEntity = 134; for (int i = 0; i < cntEntity; i++) { - Dashboard dashboard = new Dashboard(); String suffix = StringUtils.randomAlphanumeric((int) (Math.random() * 15)); String title = title1 + suffix; title = i % 2 == 0 ? title.toLowerCase() : title.toUpperCase(); - dashboard.setTitle(title); - dashboardsTitle1.add(new DashboardInfo(doPost("/api/dashboard", dashboard, Dashboard.class))); - } + dashboardsTitle1.add(new DashboardInfo(createDashboard(title))); + } String title2 = "Dashboard title 2"; List dashboardsTitle2 = new ArrayList<>(); for (int i = 0; i < 112; i++) { - Dashboard dashboard = new Dashboard(); String suffix = StringUtils.randomAlphanumeric((int) (Math.random() * 15)); String title = title2 + suffix; title = i % 2 == 0 ? title.toLowerCase() : title.toUpperCase(); - dashboard.setTitle(title); - dashboardsTitle2.add(new DashboardInfo(doPost("/api/dashboard", dashboard, Dashboard.class))); - } + dashboardsTitle2.add(new DashboardInfo(createDashboard(title))); } List loadedDashboardsTitle1 = new ArrayList<>(); PageLink pageLink = new PageLink(15, 0, title1); @@ -431,9 +417,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest int cntEntity = 173; List dashboards = new ArrayList<>(); for (int i = 0; i < cntEntity; i++) { - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("Dashboard" + i); - dashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard dashboard = createDashboard("Dashboard" + i); dashboards.add(new DashboardInfo(doPost("/api/customer/" + customerId.getId().toString() + "/dashboard/" + dashboard.getId().getId().toString(), Dashboard.class))); } @@ -466,9 +450,7 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest Edge edge = constructEdge("My edge", "default"); Edge savedEdge = doPost("/api/edge", edge, Edge.class); - Dashboard dashboard = new Dashboard(); - dashboard.setTitle("My dashboard"); - Dashboard savedDashboard = doPost("/api/dashboard", dashboard, Dashboard.class); + Dashboard savedDashboard = createDashboard(); Mockito.reset(tbClusterService, auditLogService); @@ -495,4 +477,28 @@ public abstract class BaseDashboardControllerTest extends AbstractControllerTest Assert.assertEquals(0, pageData.getData().size()); } + @Test + public void testDeleteDashboardWithRelationsOk() throws Exception { + DashboardId dashboardId = createDashboard().getId(); + tesEntityDaoWithRelationsOk(savedTenant.getId(), dashboardId, "/api/dashboard/" + dashboardId); + } + + @Test + public void testDeleteDashboardWithRelationsTransactionalException() throws Exception { + DashboardId dashboardId = createDashboard().getId(); + tesEntityDaoWithRelationsTransactionalException(dashboardDao, savedTenant.getId(), dashboardId, "/api/dashboard/" + dashboardId); + } + + private Dashboard createDashboard(String title) { + Dashboard dashboard = new Dashboard(); + dashboard.setTitle(title); + return doPost("/api/dashboard", dashboard, Dashboard.class); + } + + private Dashboard createDashboard() { + String title = "My dashboard"; + Dashboard dashboard = new Dashboard(); + dashboard.setTitle(title); + return doPost("/api/dashboard", dashboard, Dashboard.class); + } } diff --git a/dao/src/main/java/org/thingsboard/server/dao/tenant/TenantServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/tenant/TenantServiceImpl.java index 49738e13a5..866150e679 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/tenant/TenantServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/tenant/TenantServiceImpl.java @@ -171,7 +171,6 @@ public class TenantServiceImpl extends AbstractCachedEntityService