From d8ae8282a02b2bb5f55eccbdd5b7bfeca1cc4a12 Mon Sep 17 00:00:00 2001 From: Igor Kulikov Date: Thu, 3 Sep 2020 18:54:08 +0300 Subject: [PATCH] Forbid change of device profile type or transport type when used by devices. --- .../server/controller/AbstractWebTest.java | 5 ++++ .../BaseDeviceProfileControllerTest.java | 24 ++++++++++++++++-- .../server/dao/device/DeviceDao.java | 1 + .../dao/device/DeviceProfileServiceImpl.java | 25 +++++++++++++++++-- .../dao/sql/device/DeviceRepository.java | 2 ++ .../server/dao/sql/device/JpaDeviceDao.java | 5 ++++ .../dao/service/AbstractServiceTest.java | 5 ++++ .../service/BaseDeviceProfileServiceTest.java | 23 ++++++++++++++++- 8 files changed, 85 insertions(+), 5 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 1fffd210ef..69c1b8875c 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AbstractWebTest.java @@ -62,9 +62,11 @@ import org.thingsboard.server.common.data.BaseData; import org.thingsboard.server.common.data.Customer; import org.thingsboard.server.common.data.DeviceProfile; import org.thingsboard.server.common.data.DeviceProfileType; +import org.thingsboard.server.common.data.DeviceTransportType; import org.thingsboard.server.common.data.Tenant; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.device.profile.DefaultDeviceProfileConfiguration; +import org.thingsboard.server.common.data.device.profile.DefaultDeviceProfileTransportConfiguration; import org.thingsboard.server.common.data.device.profile.DeviceProfileData; import org.thingsboard.server.common.data.id.HasId; import org.thingsboard.server.common.data.id.RuleChainId; @@ -315,10 +317,13 @@ public abstract class AbstractWebTest { DeviceProfile deviceProfile = new DeviceProfile(); deviceProfile.setName(name); deviceProfile.setType(DeviceProfileType.DEFAULT); + deviceProfile.setTransportType(DeviceTransportType.DEFAULT); deviceProfile.setDescription(name + " Test"); DeviceProfileData deviceProfileData = new DeviceProfileData(); DefaultDeviceProfileConfiguration configuration = new DefaultDeviceProfileConfiguration(); + DefaultDeviceProfileTransportConfiguration transportConfiguration = new DefaultDeviceProfileTransportConfiguration(); deviceProfileData.setConfiguration(configuration); + deviceProfileData.setTransportConfiguration(transportConfiguration); deviceProfile.setProfileData(deviceProfileData); deviceProfile.setDefault(false); deviceProfile.setDefaultRuleChainId(new RuleChainId(Uuids.timeBased())); diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseDeviceProfileControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseDeviceProfileControllerTest.java index f1a268996d..8d2c17c25a 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseDeviceProfileControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseDeviceProfileControllerTest.java @@ -25,6 +25,7 @@ import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.DeviceProfile; import org.thingsboard.server.common.data.DeviceProfileInfo; import org.thingsboard.server.common.data.DeviceProfileType; +import org.thingsboard.server.common.data.DeviceTransportType; import org.thingsboard.server.common.data.Tenant; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.page.PageData; @@ -154,13 +155,32 @@ public abstract class BaseDeviceProfileControllerTest extends AbstractController @Ignore @Test - public void testSaveSameDeviceProfileWithDifferentType() throws Exception { + public void testChangeDeviceProfileTypeWithExistingDevices() throws Exception { DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile"); DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class); + Device device = new Device(); + device.setName("Test device"); + device.setType("default"); + device.setDeviceProfileId(savedDeviceProfile.getId()); + doPost("/api/device", device, Device.class); //TODO uncomment once we have other device types; //savedDeviceProfile.setType(DeviceProfileType.LWM2M); doPost("/api/deviceProfile", savedDeviceProfile).andExpect(status().isBadRequest()) - .andExpect(statusReason(containsString("Changing type of device profile is prohibited"))); + .andExpect(statusReason(containsString("Can't change device profile type because devices referenced it"))); + } + + @Test + public void testChangeDeviceProfileTransportTypeWithExistingDevices() throws Exception { + DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile"); + DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class); + Device device = new Device(); + device.setName("Test device"); + device.setType("default"); + device.setDeviceProfileId(savedDeviceProfile.getId()); + doPost("/api/device", device, Device.class); + savedDeviceProfile.setTransportType(DeviceTransportType.MQTT); + doPost("/api/deviceProfile", savedDeviceProfile).andExpect(status().isBadRequest()) + .andExpect(statusReason(containsString("Can't change device profile transport type because devices referenced it"))); } @Test diff --git a/dao/src/main/java/org/thingsboard/server/dao/device/DeviceDao.java b/dao/src/main/java/org/thingsboard/server/dao/device/DeviceDao.java index ae87564e81..bd000f3181 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/device/DeviceDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/device/DeviceDao.java @@ -183,4 +183,5 @@ public interface DeviceDao extends Dao { */ ListenableFuture findDeviceByTenantIdAndIdAsync(TenantId tenantId, UUID id); + Long countDevicesByDeviceProfileId(TenantId tenantId, UUID deviceProfileId); } diff --git a/dao/src/main/java/org/thingsboard/server/dao/device/DeviceProfileServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/device/DeviceProfileServiceImpl.java index b6cc2d3e8a..424a7e4634 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/device/DeviceProfileServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/device/DeviceProfileServiceImpl.java @@ -58,6 +58,9 @@ public class DeviceProfileServiceImpl extends AbstractEntityService implements D @Autowired private DeviceProfileDao deviceProfileDao; + @Autowired + private DeviceDao deviceDao; + @Autowired private TenantDao tenantDao; @@ -241,6 +244,12 @@ public class DeviceProfileServiceImpl extends AbstractEntityService implements D if (StringUtils.isEmpty(deviceProfile.getName())) { throw new DataValidationException("Device profile name should be specified!"); } + if (deviceProfile.getType() == null) { + throw new DataValidationException("Device profile type should be specified!"); + } + if (deviceProfile.getTransportType() == null) { + throw new DataValidationException("Device profile transport type should be specified!"); + } if (deviceProfile.getTenantId() == null) { throw new DataValidationException("Device profile should be assigned to tenant!"); } else { @@ -262,8 +271,20 @@ public class DeviceProfileServiceImpl extends AbstractEntityService implements D DeviceProfile old = deviceProfileDao.findById(deviceProfile.getTenantId(), deviceProfile.getId().getId()); if (old == null) { throw new DataValidationException("Can't update non existing device profile!"); - } else if (!old.getType().equals(deviceProfile.getType())) { - throw new DataValidationException("Changing type of device profile is prohibited!"); + } + boolean profileTypeChanged = !old.getType().equals(deviceProfile.getType()); + boolean transportTypeChanged = !old.getTransportType().equals(deviceProfile.getTransportType()); + if (profileTypeChanged || transportTypeChanged) { + Long profileDeviceCount = deviceDao.countDevicesByDeviceProfileId(deviceProfile.getTenantId(), deviceProfile.getId().getId()); + if (profileDeviceCount > 0) { + String message = null; + if (profileTypeChanged) { + message = "Can't change device profile type because devices referenced it!"; + } else if (transportTypeChanged) { + message = "Can't change device profile transport type because devices referenced it!"; + } + throw new DataValidationException(message); + } } } }; diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/device/DeviceRepository.java b/dao/src/main/java/org/thingsboard/server/dao/sql/device/DeviceRepository.java index d74ffd2f43..ee7977a43c 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/device/DeviceRepository.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/device/DeviceRepository.java @@ -133,4 +133,6 @@ public interface DeviceRepository extends PagingAndSortingRepository return service.submit(() -> DaoUtil.getData(deviceRepository.findByTenantIdAndId(tenantId.getId(), id))); } + @Override + public Long countDevicesByDeviceProfileId(TenantId tenantId, UUID deviceProfileId) { + return deviceRepository.countByDeviceProfileId(deviceProfileId); + } + private List convertTenantDeviceTypesToDto(UUID tenantId, List types) { List list = Collections.emptyList(); if (types != null && !types.isEmpty()) { diff --git a/dao/src/test/java/org/thingsboard/server/dao/service/AbstractServiceTest.java b/dao/src/test/java/org/thingsboard/server/dao/service/AbstractServiceTest.java index 8d84d76087..a6d2d7c29f 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/service/AbstractServiceTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/service/AbstractServiceTest.java @@ -30,9 +30,11 @@ import org.springframework.test.context.support.AnnotationConfigContextLoader; import org.thingsboard.server.common.data.BaseData; import org.thingsboard.server.common.data.DeviceProfile; import org.thingsboard.server.common.data.DeviceProfileType; +import org.thingsboard.server.common.data.DeviceTransportType; import org.thingsboard.server.common.data.EntityType; import org.thingsboard.server.common.data.Event; import org.thingsboard.server.common.data.device.profile.DefaultDeviceProfileConfiguration; +import org.thingsboard.server.common.data.device.profile.DefaultDeviceProfileTransportConfiguration; import org.thingsboard.server.common.data.device.profile.DeviceProfileData; import org.thingsboard.server.common.data.id.EntityId; import org.thingsboard.server.common.data.id.HasId; @@ -197,10 +199,13 @@ public abstract class AbstractServiceTest { deviceProfile.setTenantId(tenantId); deviceProfile.setName(name); deviceProfile.setType(DeviceProfileType.DEFAULT); + deviceProfile.setTransportType(DeviceTransportType.DEFAULT); deviceProfile.setDescription(name + " Test"); DeviceProfileData deviceProfileData = new DeviceProfileData(); DefaultDeviceProfileConfiguration configuration = new DefaultDeviceProfileConfiguration(); + DefaultDeviceProfileTransportConfiguration transportConfiguration = new DefaultDeviceProfileTransportConfiguration(); deviceProfileData.setConfiguration(configuration); + deviceProfileData.setTransportConfiguration(transportConfiguration); deviceProfile.setProfileData(deviceProfileData); deviceProfile.setDefault(false); deviceProfile.setDefaultRuleChainId(new RuleChainId(Uuids.timeBased())); diff --git a/dao/src/test/java/org/thingsboard/server/dao/service/BaseDeviceProfileServiceTest.java b/dao/src/test/java/org/thingsboard/server/dao/service/BaseDeviceProfileServiceTest.java index 86099ef169..dfaa46e7c1 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/service/BaseDeviceProfileServiceTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/service/BaseDeviceProfileServiceTest.java @@ -24,6 +24,7 @@ import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.DeviceProfile; import org.thingsboard.server.common.data.DeviceProfileInfo; import org.thingsboard.server.common.data.DeviceProfileType; +import org.thingsboard.server.common.data.DeviceTransportType; import org.thingsboard.server.common.data.Tenant; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.page.PageData; @@ -148,14 +149,34 @@ public class BaseDeviceProfileServiceTest extends AbstractServiceTest { @Ignore @Test(expected = DataValidationException.class) - public void testSaveSameDeviceProfileWithDifferentType() { + public void testChangeDeviceProfileTypeWithExistingDevices() { DeviceProfile deviceProfile = this.createDeviceProfile(tenantId,"Device Profile"); DeviceProfile savedDeviceProfile = deviceProfileService.saveDeviceProfile(deviceProfile); + Device device = new Device(); + device.setTenantId(tenantId); + device.setName("Test device"); + device.setType("default"); + device.setDeviceProfileId(savedDeviceProfile.getId()); + deviceService.saveDevice(device); //TODO: once we have more profile types, we should test that we can not change profile type in runtime and uncomment the @Ignore. // savedDeviceProfile.setType(DeviceProfileType.LWM2M); deviceProfileService.saveDeviceProfile(savedDeviceProfile); } + @Test(expected = DataValidationException.class) + public void testChangeDeviceProfileTransportTypeWithExistingDevices() { + DeviceProfile deviceProfile = this.createDeviceProfile(tenantId,"Device Profile"); + DeviceProfile savedDeviceProfile = deviceProfileService.saveDeviceProfile(deviceProfile); + Device device = new Device(); + device.setTenantId(tenantId); + device.setName("Test device"); + device.setType("default"); + device.setDeviceProfileId(savedDeviceProfile.getId()); + deviceService.saveDevice(device); + savedDeviceProfile.setTransportType(DeviceTransportType.MQTT); + deviceProfileService.saveDeviceProfile(savedDeviceProfile); + } + @Test(expected = DataValidationException.class) public void testDeleteDeviceProfileWithExistingDevice() { DeviceProfile deviceProfile = this.createDeviceProfile(tenantId,"Device Profile");