diff --git a/application/src/test/java/org/thingsboard/server/controller/DeviceControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/DeviceControllerTest.java index cf46d0f7fd..c036f70e4a 100644 --- a/application/src/test/java/org/thingsboard/server/controller/DeviceControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/DeviceControllerTest.java @@ -1503,11 +1503,12 @@ public class DeviceControllerTest extends AbstractControllerTest { Assert.assertTrue(deviceBulkImportResult.getErrorsList().isEmpty()); Device updatedDevice = doGet("/api/device/" + savedDevice.getId().getId(), Device.class); + savedDevice.setVersion(updatedDevice.getVersion()); Assert.assertEquals(savedDevice, updatedDevice); DeviceCredentials updatedCredentials = doGet("/api/device/" + savedDevice.getId().getId() + "/credentials", DeviceCredentials.class); - + savedCredentials.setVersion(updatedCredentials.getVersion()); Assert.assertEquals(savedCredentials, updatedCredentials); } diff --git a/application/src/test/java/org/thingsboard/server/controller/RuleChainControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/RuleChainControllerTest.java index 0aca783e67..3d85c4cc12 100644 --- a/application/src/test/java/org/thingsboard/server/controller/RuleChainControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/RuleChainControllerTest.java @@ -360,7 +360,7 @@ public class RuleChainControllerTest extends AbstractControllerTest { @Test public void testSaveRuleChainWithOutdatedVersion() throws Exception { - RuleChain ruleChain = createRuleChain("Rule chain with invalid nodes"); + RuleChain ruleChain = createRuleChain("My rule chain"); RuleChainMetaData ruleChainMetaData = new RuleChainMetaData(); ruleChainMetaData.setRuleChainId(ruleChain.getId()); @@ -392,11 +392,11 @@ public class RuleChainControllerTest extends AbstractControllerTest { .andExpect(status().isConflict()); ruleChainMetaData.setVersion(3L); - doPost("/api/ruleChain/metadata", ruleChainMetaData) - .andExpect(status().isOk()); - ruleChain.setVersion(3L); - doPost("/api/ruleChain", ruleChain) - .andExpect(status().isOk()); + ruleChainMetaData = doPost("/api/ruleChain/metadata", ruleChainMetaData, RuleChainMetaData.class); + assertThat(ruleChainMetaData.getVersion()).isEqualTo(4); + ruleChain.setVersion(4L); + ruleChain = doPost("/api/ruleChain", ruleChain, RuleChain.class); + assertThat(ruleChain.getVersion()).isEqualTo(5); } private RuleChain createRuleChain(String name) { diff --git a/dao/src/main/java/org/thingsboard/server/dao/rule/BaseRuleChainService.java b/dao/src/main/java/org/thingsboard/server/dao/rule/BaseRuleChainService.java index 68bb185e5f..902bf0f6a7 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/rule/BaseRuleChainService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/rule/BaseRuleChainService.java @@ -168,7 +168,7 @@ public class BaseRuleChainService extends AbstractEntityService implements RuleC return saveRuleChainMetaData(tenantId, ruleChainMetaData, ruleNodeUpdater, true); } - + @Transactional @Override public RuleChainUpdateResult saveRuleChainMetaData(TenantId tenantId, RuleChainMetaData ruleChainMetaData, Function ruleNodeUpdater, boolean publishSaveEvent) { Validator.validateId(ruleChainMetaData.getRuleChainId(), "Incorrect rule chain id."); @@ -237,7 +237,6 @@ public class BaseRuleChainService extends AbstractEntityService implements RuleC if ((ruleChain.getFirstRuleNodeId() != null && !ruleChain.getFirstRuleNodeId().equals(firstRuleNodeId)) || (ruleChain.getFirstRuleNodeId() == null && firstRuleNodeId != null)) { ruleChain.setFirstRuleNodeId(firstRuleNodeId); - ruleChainDao.save(tenantId, ruleChain); } if (ruleChainMetaData.getConnections() != null) { for (NodeConnectionInfo nodeConnection : ruleChainMetaData.getConnections()) { @@ -286,6 +285,7 @@ public class BaseRuleChainService extends AbstractEntityService implements RuleC if (!relations.isEmpty()) { relationService.saveRelations(tenantId, relations); } + ruleChain = ruleChainDao.save(tenantId, ruleChain); if (publishSaveEvent) { eventPublisher.publishEvent(SaveEntityEvent.builder().tenantId(tenantId).entity(ruleChain).entityId(ruleChain.getId()).build()); } diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/JpaAbstractDao.java b/dao/src/main/java/org/thingsboard/server/dao/sql/JpaAbstractDao.java index a7d49e244b..f044629364 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/JpaAbstractDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/JpaAbstractDao.java @@ -97,20 +97,28 @@ public abstract class JpaAbstractDao, D> if (versionedEntity.getVersion() == null) { HasVersion existingEntity = entityManager.find(versionedEntity.getClass(), entity.getUuid()); if (existingEntity != null) { - versionedEntity.setVersion(existingEntity.getVersion()); // manually resetting the version to latest to allow force overwrite of the entity + /* + * manually resetting the version to latest to allow force overwrite of the entity + * */ + versionedEntity.setVersion(existingEntity.getVersion()); } else { return doSave(entity, true, flush); } } - entity = entityManager.merge(entity); + versionedEntity = entityManager.merge(versionedEntity); + /* + * by default, Hibernate doesn't issue an update query and thus version increment + * if the entity was not modified. to bypass this and always increment the version, we do it manually + * */ + versionedEntity.setVersion(versionedEntity.getVersion() + 1); /* - * flushing so that the query is executed right away and the version is incremented, - * then removing the entity from the persistence context so that it is not affected + * flushing and then removing the entity from the persistence context so that it is not affected * by next flushes (e.g. when a transaction is committed) to avoid double version increment * */ entityManager.flush(); - entityManager.detach(entity); + entityManager.detach(versionedEntity); flushed = true; + entity = (E) versionedEntity; } else { entity = entityManager.merge(entity); } @@ -161,6 +169,7 @@ public abstract class JpaAbstractDao, D> log.debug("Remove request: {}", id); } + @Override @Transactional public void removeAllByIds(Collection ids) { JpaRepository repository = getRepository(); diff --git a/dao/src/test/java/org/thingsboard/server/dao/sql/device/JpaDeviceDaoTest.java b/dao/src/test/java/org/thingsboard/server/dao/sql/device/JpaDeviceDaoTest.java index 644ce40bb8..90510cf2b7 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/sql/device/JpaDeviceDaoTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/sql/device/JpaDeviceDaoTest.java @@ -45,6 +45,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -105,7 +106,37 @@ public class JpaDeviceDaoTest extends AbstractJpaDaoTest { @Test public void testSaveDeviceName0x00_thenSomeDatabaseException() { Device device = getDevice(tenantId1, customerId1, "\u0000"); - assertThatThrownBy(() -> deviceIds.add(deviceDao.save(TenantId.fromUUID(tenantId1), device).getUuidId())); + assertThatThrownBy(() -> deviceIds.add(saveDevice(tenantId1, device).getUuidId())); + } + + @Test + public void testSaveDevice_versionIncrement() { + Device device = getDevice(tenantId1, customerId1, "1ewfewf2"); + device = saveDevice(tenantId1, device); + deviceIds.add(device.getUuidId()); + assertThat(device.getVersion()).isEqualTo(1); + + device.setName(device.getName() + "x"); + device = saveDevice(tenantId1, device); + assertThat(device.getVersion()).isEqualTo(2); + + device.setName(device.getName() + "x"); + device = saveDevice(tenantId1, device); + assertThat(device.getVersion()).isEqualTo(3); + } + + @Test + public void testSaveDevice_versionIncrement_noChanges() { + Device device = getDevice(tenantId1, customerId1, "1ewfewf2"); + device = saveDevice(tenantId1, device); + deviceIds.add(device.getUuidId()); + assertThat(device.getVersion()).isEqualTo(1); + + device = saveDevice(tenantId1, device); + assertThat(device.getVersion()).isEqualTo(2); + + device = saveDevice(tenantId1, device); + assertThat(device.getVersion()).isEqualTo(3); } @Test @@ -126,7 +157,7 @@ public class JpaDeviceDaoTest extends AbstractJpaDaoTest { UUID customerId = Uuids.timeBased(); // send to method getDevice() number = 40, because make random name is bad and name "SEARCH_TEXT_40" don't used Device device = getDevice(tenantId, customerId, 40); - deviceIds.add(deviceDao.save(TenantId.fromUUID(tenantId), device).getUuidId()); + deviceIds.add(saveDevice(tenantId, device).getUuidId()); UUID uuid = device.getId().getId(); Device entity = deviceDao.findById(TenantId.fromUUID(tenantId), uuid); @@ -156,8 +187,8 @@ public class JpaDeviceDaoTest extends AbstractJpaDaoTest { private List createDevices(UUID tenantId1, UUID tenantId2, UUID customerId1, UUID customerId2, int count) { List savedDevicesUUID = new ArrayList<>(); for (int i = 0; i < count / 2; i++) { - savedDevicesUUID.add(deviceDao.save(TenantId.fromUUID(tenantId1), getDevice(tenantId1, customerId1, i)).getUuidId()); - savedDevicesUUID.add(deviceDao.save(TenantId.fromUUID(tenantId2), getDevice(tenantId2, customerId2, i + count / 2)).getUuidId()); + savedDevicesUUID.add(saveDevice(tenantId1, getDevice(tenantId1, customerId1, i)).getUuidId()); + savedDevicesUUID.add(saveDevice(tenantId2, getDevice(tenantId2, customerId2, i + count / 2)).getUuidId()); } return savedDevicesUUID; } @@ -175,4 +206,9 @@ public class JpaDeviceDaoTest extends AbstractJpaDaoTest { device.setDeviceProfileId(savedDeviceProfile.getId()); return device; } + + private Device saveDevice(UUID tenantId, Device device) { + return deviceDao.save(TenantId.fromUUID(tenantId), device); + } + }