Handle Device service transactional methods exceptions (fix exception handling for devices with same name)

dao/src/main/java/org/thingsboard/server/dao/device/DeviceServiceImpl.java

@@ -30,7 +30,6 @@ import org.springframework.cache.annotation.Cacheable;
 import org.springframework.cache.annotation.Caching;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.stereotype.Service;
-import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import org.thingsboard.common.util.JacksonUtil;
@@ -82,6 +81,7 @@ import org.thingsboard.server.dao.service.DataValidator;
 import org.thingsboard.server.dao.service.PaginatedRemover;
 import org.thingsboard.server.dao.tenant.TbTenantProfileCache;
 import org.thingsboard.server.dao.tenant.TenantDao;
+import org.thingsboard.server.dao.tx.TransactionHandler;
 
 import javax.annotation.Nullable;
 import java.util.ArrayList;
@@ -141,6 +141,9 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
     @Autowired
     private OtaPackageService otaPackageService;
 
+    @Autowired
+    private TransactionHandler transactionHandler;
+
     @Override
     public DeviceInfo findDeviceInfoById(TenantId tenantId, DeviceId deviceId) {
         log.trace("Executing findDeviceInfoById [{}]", deviceId);
@@ -184,10 +187,13 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
             @CacheEvict(cacheNames = DEVICE_CACHE, key = "{#device.tenantId, #device.name}"),
             @CacheEvict(cacheNames = DEVICE_CACHE, key = "{#device.tenantId, #device.id}")
     })
-    @Transactional
     @Override
     public Device saveDeviceWithAccessToken(Device device, String accessToken) {
-        return doSaveDevice(device, accessToken, true);
+        try {
+            return transactionHandler.runInTransaction(() -> doSaveDevice(device, accessToken, true));
+        } catch (Exception t) {
+            throw handleDeviceSaveException(device, t);
+        }
     }
 
     @Caching(evict= {
@@ -212,9 +218,11 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
             @CacheEvict(cacheNames = DEVICE_CACHE, key = "{#device.tenantId, #device.name}"),
             @CacheEvict(cacheNames = DEVICE_CACHE, key = "{#device.tenantId, #device.id}")
     })
-    @Transactional
     @Override
-    public Device saveDeviceWithCredentials(Device device, DeviceCredentials deviceCredentials) {
+    public Device saveDeviceWithCredentials(Device toSave, DeviceCredentials deviceCredentials) {
+        try {
+            return transactionHandler.runInTransaction(() -> {
+                Device device = toSave;
                 if (device.getId() == null) {
                     Device deviceWithName = this.findDeviceByTenantIdAndName(device.getTenantId(), device.getName());
                     device = deviceWithName == null ? device : deviceWithName.updateDevice(device);
@@ -232,6 +240,10 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
                     }
                 }
                 return savedDevice;
+            });
+        } catch (Exception t) {
+            throw handleDeviceSaveException(toSave, t);
+        }
     }
 
     private Device doSaveDevice(Device device, String accessToken, boolean doValidate) {
@@ -270,15 +282,7 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
             device.setDeviceData(syncDeviceData(deviceProfile, device.getDeviceData()));
             return deviceDao.save(device.getTenantId(), device);
         } catch (Exception t) {
-            ConstraintViolationException e = extractConstraintViolationException(t).orElse(null);
+            throw handleDeviceSaveException(device, t);
-            if (e != null && e.getConstraintName() != null && e.getConstraintName().equalsIgnoreCase("device_name_unq_key")) {
-                // remove device from cache in case null value cached in the distributed redis.
-                removeDeviceFromCacheByName(device.getTenantId(), device.getName());
-                removeDeviceFromCacheById(device.getTenantId(), device.getId());
-                throw new DataValidationException("Device with such name already exists!");
-            } else {
-                throw t;
-            }
         }
     }
 
@@ -364,17 +368,18 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
     }
 
     private void removeDeviceFromCacheByName(TenantId tenantId, String name) {
+        if (tenantId != null && !StringUtils.isEmpty(name)) {
             Cache cache = cacheManager.getCache(DEVICE_CACHE);
             cache.evict(Arrays.asList(tenantId, name));
         }
+    }
 
     private void removeDeviceFromCacheById(TenantId tenantId, DeviceId deviceId) {
-        if (deviceId == null) {
+        if (tenantId != null && deviceId != null) {
-            return;
-        }
             Cache cache = cacheManager.getCache(DEVICE_CACHE);
             cache.evict(Arrays.asList(tenantId, deviceId));
         }
+    }
 
     @Override
     public PageData<Device> findDevicesByTenantId(TenantId tenantId, PageLink pageLink) {
@@ -560,11 +565,11 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
                 }, MoreExecutors.directExecutor());
     }
 
-    @Transactional
     @Override
     public Device assignDeviceToTenant(TenantId tenantId, Device device) {
         log.trace("Executing assignDeviceToTenant [{}][{}]", tenantId, device);
-
+        try {
+            return transactionHandler.runInTransaction(() -> {
                 try {
                     List<EntityView> entityViews = entityViewService.findEntityViewsByTenantIdAndEntityIdAsync(device.getTenantId(), device.getId()).get();
                     if (!CollectionUtils.isEmpty(entityViews)) {
@@ -591,13 +596,18 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
                 removeDeviceFromCacheById(oldTenantId, device.getId());
 
                 return savedDevice;
+            });
+        } catch (Exception t) {
+            throw handleDeviceSaveException(device, t);
+        }
     }
 
     @Override
     @CacheEvict(cacheNames = DEVICE_CACHE, key = "{#profile.tenantId, #provisionRequest.deviceName}")
-    @Transactional
     public Device saveDevice(ProvisionRequest provisionRequest, DeviceProfile profile) {
         Device device = new Device();
+        try {
+            return transactionHandler.runInTransaction(() -> {
                 device.setName(provisionRequest.getDeviceName());
                 device.setType(profile.getName());
                 device.setTenantId(profile.getTenantId());
@@ -638,6 +648,10 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
                 }
                 removeDeviceFromCacheById(savedDevice.getTenantId(), savedDevice.getId()); // eviction by name is described as annotation @CacheEvict above
                 return savedDevice;
+            });
+        } catch (Exception t) {
+            throw handleDeviceSaveException(device, t);
+        }
     }
 
     @Override
@@ -818,4 +832,20 @@ public class DeviceServiceImpl extends AbstractEntityService implements DeviceSe
             unassignDeviceFromCustomer(tenantId, new DeviceId(entity.getUuidId()));
         }
     };
+
+    private RuntimeException handleDeviceSaveException(Device device, Exception t)  {
+        ConstraintViolationException e = extractConstraintViolationException(t).orElse(null);
+        if (e != null && e.getConstraintName() != null && e.getConstraintName().equalsIgnoreCase("device_name_unq_key")) {
+            // remove device from cache in case null value cached in the distributed redis.
+            if (device != null) {
+                removeDeviceFromCacheByName(device.getTenantId(), device.getName());
+                removeDeviceFromCacheById(device.getTenantId(), device.getId());
+            }
+            return new DataValidationException("Device with such name already exists!");
+        } else if (t instanceof RuntimeException) {
+            return (RuntimeException)t;
+        } else {
+            return new RuntimeException("Failed to save device!", t);
+        }
+    }
 }

dao/src/main/java/org/thingsboard/server/dao/tx/TransactionHandler.java

@@ -0,0 +1,36 @@
+/**
+ * Copyright © 2016-2021 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.dao.tx;
+
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+
+import java.util.function.Supplier;
+
+@Service
+public class TransactionHandler {
+
+    @Transactional(propagation = Propagation.REQUIRED)
+    public <T> T runInTransaction(Supplier<T> supplier) {
+        return supplier.get();
+    }
+
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public <T> T runInNewTransaction(Supplier<T> supplier) {
+        return supplier.get();
+    }
+}