Merge pull request #12763 from thingsboard/fix/created-entity-version

Fix invalid entity version after creation

application/src/test/java/org/thingsboard/server/controller/DeviceProfileControllerTest.java

@@ -27,6 +27,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Primary;
 import org.springframework.test.context.ContextConfiguration;
+import org.thingsboard.common.util.JacksonUtil;
 import org.thingsboard.server.common.data.Customer;
 import org.thingsboard.server.common.data.Dashboard;
 import org.thingsboard.server.common.data.Device;
@@ -60,6 +61,7 @@ import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 import static org.thingsboard.server.common.data.DataConstants.DEFAULT_DEVICE_TYPE;
@@ -79,6 +81,8 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Autowired
     private DeviceProfileDao deviceProfileDao;
 
+    static final String LWM2M_PROFILE_JSON = "{\"name\":\"lwm2m profile\",\"type\":\"DEFAULT\",\"image\":null,\"defaultQueueName\":null,\"transportType\":\"LWM2M\",\"provisionType\":\"DISABLED\",\"description\":\"\",\"profileData\":{\"configuration\":{\"type\":\"DEFAULT\"},\"transportConfiguration\":{\"observeAttr\":{\"observe\":[],\"attribute\":[],\"telemetry\":[\"/11_1.1/0/0\"],\"keyName\":{\"/11_1.1/0/0\":\"profileName\"},\"attributeLwm2m\":{}},\"bootstrap\":[{\"shortServerId\":123,\"bootstrapServerIs\":false,\"host\":\"0.0.0.0\",\"port\":5685,\"clientHoldOffTime\":1,\"serverPublicKey\":\"\",\"serverCertificate\":\"\",\"bootstrapServerAccountTimeout\":0,\"lifetime\":300,\"defaultMinPeriod\":1,\"notifIfDisabled\":true,\"binding\":\"U\",\"securityMode\":\"NO_SEC\"}],\"clientLwM2mSettings\":{\"clientOnlyObserveAfterConnect\":1,\"fwUpdateStrategy\":1,\"swUpdateStrategy\":1,\"powerMode\":\"DRX\",\"edrxCycle\":81000,\"psmActivityTimer\":10000,\"pagingTransmissionWindow\":10000,\"defaultObjectIDVer\":\"1.0\"},\"bootstrapServerUpdateEnable\":false,\"type\":\"LWM2M\"},\"alarms\":null,\"provisionConfiguration\":{\"type\":\"DISABLED\"}}}";
+
     static class Config {
         @Bean
         @Primary
@@ -119,7 +123,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
 
         Mockito.reset(tbClusterService, auditLogService);
 
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         Assert.assertNotNull(savedDeviceProfile);
         Assert.assertNotNull(savedDeviceProfile.getId());
         Assert.assertTrue(savedDeviceProfile.getCreatedTime() > 0);
@@ -135,7 +139,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
                 ActionType.ADDED);
 
         savedDeviceProfile.setName("New device profile");
-        doPost("/api/deviceProfile", savedDeviceProfile, DeviceProfile.class);
+        saveDeviceProfile(savedDeviceProfile);
         DeviceProfile foundDeviceProfile = doGet("/api/deviceProfile/" + savedDeviceProfile.getId().getId().toString(), DeviceProfile.class);
         Assert.assertEquals(savedDeviceProfile.getName(), foundDeviceProfile.getName());
 
@@ -162,7 +166,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testFindDeviceProfileById() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         DeviceProfile foundDeviceProfile = doGet("/api/deviceProfile/" + savedDeviceProfile.getId().getId().toString(), DeviceProfile.class);
         Assert.assertNotNull(foundDeviceProfile);
         Assert.assertEquals(savedDeviceProfile, foundDeviceProfile);
@@ -171,7 +175,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void whenGetDeviceProfileById_thenPermissionsAreChecked() throws Exception {
         DeviceProfile deviceProfile = createDeviceProfile("Device profile 1", null);
-        deviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        deviceProfile = saveDeviceProfile(deviceProfile);
 
         loginDifferentTenant();
 
@@ -183,7 +187,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testFindDeviceProfileInfoById() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         DeviceProfileInfo foundDeviceProfileInfo = doGet("/api/deviceProfileInfo/" + savedDeviceProfile.getId().getId().toString(), DeviceProfileInfo.class);
         Assert.assertNotNull(foundDeviceProfileInfo);
         Assert.assertEquals(savedDeviceProfile.getId(), foundDeviceProfileInfo.getId());
@@ -213,7 +217,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void whenGetDeviceProfileInfoById_thenPermissionsAreChecked() throws Exception {
         DeviceProfile deviceProfile = createDeviceProfile("Device profile 1", null);
-        deviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        deviceProfile = saveDeviceProfile(deviceProfile);
 
         loginDifferentTenant();
         doGet("/api/deviceProfileInfo/" + deviceProfile.getId())
@@ -235,7 +239,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testSetDefaultDeviceProfile() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile 1");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
 
         Mockito.reset(tbClusterService, auditLogService);
 
@@ -328,7 +332,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testChangeDeviceProfileTypeNull() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
 
         Mockito.reset(tbClusterService, auditLogService);
 
@@ -345,7 +349,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testChangeDeviceProfileTransportTypeWithExistingDevices() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         Device device = new Device();
         device.setName("Test device");
         device.setType("default");
@@ -367,7 +371,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testDeleteDeviceProfileWithExistingDevice() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
 
         Device device = new Device();
         device.setName("Test device");
@@ -419,7 +423,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     public void testSaveDeviceProfileWithFirmwareFromDifferentTenant() throws Exception {
         loginDifferentTenant();
         DeviceProfile differentProfile = createDeviceProfile("Different profile");
-        differentProfile = doPost("/api/deviceProfile", differentProfile, DeviceProfile.class);
+        differentProfile = saveDeviceProfile(differentProfile);
         SaveOtaPackageInfoRequest firmwareInfo = new SaveOtaPackageInfoRequest();
         firmwareInfo.setDeviceProfileId(differentProfile.getId());
         firmwareInfo.setType(FIRMWARE);
@@ -441,7 +445,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     public void testSaveDeviceProfileWithSoftwareFromDifferentTenant() throws Exception {
         loginDifferentTenant();
         DeviceProfile differentProfile = createDeviceProfile("Different profile");
-        differentProfile = doPost("/api/deviceProfile", differentProfile, DeviceProfile.class);
+        differentProfile = saveDeviceProfile(differentProfile);
         SaveOtaPackageInfoRequest softwareInfo = new SaveOtaPackageInfoRequest();
         softwareInfo.setDeviceProfileId(differentProfile.getId());
         softwareInfo.setType(SOFTWARE);
@@ -462,7 +466,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
     @Test
     public void testDeleteDeviceProfile() throws Exception {
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile");
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
 
         Mockito.reset(tbClusterService, auditLogService);
 
@@ -495,7 +499,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
         int cntEntity = 28;
         for (int i = 0; i < cntEntity; i++) {
             DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile" + i);
-            deviceProfiles.add(doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class));
+            deviceProfiles.add(saveDeviceProfile(deviceProfile));
         }
 
         testNotifyManyEntityManyTimeMsgToEdgeServiceEntityEqAny(new DeviceProfile(), new DeviceProfile(),
@@ -552,7 +556,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
 
         for (int i = 0; i < 28; i++) {
             DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile" + i);
-            deviceProfiles.add(doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class));
+            deviceProfiles.add(saveDeviceProfile(deviceProfile));
         }
 
         List<DeviceProfileInfo> loadedDeviceProfileInfos = new ArrayList<>();
@@ -961,7 +965,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
         JsonTransportPayloadConfiguration jsonTransportPayloadConfiguration = new JsonTransportPayloadConfiguration();
         MqttDeviceProfileTransportConfiguration mqttDeviceProfileTransportConfiguration = this.createMqttDeviceProfileTransportConfiguration(jsonTransportPayloadConfiguration, true);
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile", mqttDeviceProfileTransportConfiguration);
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         Assert.assertNotNull(savedDeviceProfile);
         Assert.assertEquals(savedDeviceProfile.getTransportType(), DeviceTransportType.MQTT);
         Assert.assertTrue(savedDeviceProfile.getProfileData().getTransportConfiguration() instanceof MqttDeviceProfileTransportConfiguration);
@@ -979,7 +983,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
                         "v1/devices/me/telemetry", "v1/devices/me/attributes", "v1/devices/me/subscribeattributes");
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile",
                 mqttDeviceProfileTransportConfiguration);
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         Assert.assertNotNull(savedDeviceProfile);
         Assert.assertEquals(savedDeviceProfile.getTransportType(), DeviceTransportType.MQTT);
         Assert.assertTrue(savedDeviceProfile.getProfileData().getTransportConfiguration() instanceof MqttDeviceProfileTransportConfiguration);
@@ -997,7 +1001,7 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
         ProtoTransportPayloadConfiguration protoTransportPayloadConfiguration = this.createProtoTransportPayloadConfiguration(schema, schema, null, null);
         MqttDeviceProfileTransportConfiguration mqttDeviceProfileTransportConfiguration = this.createMqttDeviceProfileTransportConfiguration(protoTransportPayloadConfiguration, false);
         DeviceProfile deviceProfile = this.createDeviceProfile("Device Profile", mqttDeviceProfileTransportConfiguration);
-        DeviceProfile savedDeviceProfile = doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
+        DeviceProfile savedDeviceProfile = saveDeviceProfile(deviceProfile);
         Assert.assertNotNull(savedDeviceProfile);
         DeviceProfile foundDeviceProfile = doGet("/api/deviceProfile/" + savedDeviceProfile.getId().getId().toString(), DeviceProfile.class);
         Assert.assertEquals(savedDeviceProfile, foundDeviceProfile);
@@ -1036,14 +1040,14 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
 
     @Test
     public void testDeleteDeviceProfileWithDeleteRelationsOk() throws Exception {
-        DeviceProfileId deviceProfileId = savedDeviceProfile("DeviceProfile for Test WithRelationsOk").getId();
+        DeviceProfileId deviceProfileId = saveDeviceProfile("DeviceProfile for Test WithRelationsOk").getId();
         testEntityDaoWithRelationsOk(savedTenant.getId(), deviceProfileId, "/api/deviceProfile/" + deviceProfileId);
     }
 
     @Ignore
     @Test
     public void testDeleteDeviceProfileExceptionWithRelationsTransactional() throws Exception {
-        DeviceProfileId deviceProfileId = savedDeviceProfile("DeviceProfile for Test WithRelations Transactional Exception").getId();
+        DeviceProfileId deviceProfileId = saveDeviceProfile("DeviceProfile for Test WithRelations Transactional Exception").getId();
         testEntityDaoWithRelationsTransactionalException(deviceProfileDao, savedTenant.getId(), deviceProfileId, "/api/deviceProfile/" + deviceProfileId);
     }
 
@@ -1103,8 +1107,36 @@ public class DeviceProfileControllerTest extends AbstractControllerTest {
         Assert.assertEquals(count, deviceProfileNames.size());
     }
 
-    private DeviceProfile savedDeviceProfile(String name) {
+    @Test
+    public void testSaveDeviceProfileWithOutdatedVersion() throws Exception {
+        DeviceProfile deviceProfile = JacksonUtil.fromString(LWM2M_PROFILE_JSON, DeviceProfile.class);
+        deviceProfile.setName("Device profile v1.0");
+        deviceProfile = saveDeviceProfile(deviceProfile);
+        assertThat(deviceProfile.getVersion()).isOne();
+
+        deviceProfile.setName("Device profile v2.0");
+        deviceProfile = saveDeviceProfile(deviceProfile);
+        assertThat(deviceProfile.getVersion()).isEqualTo(2);
+
+        deviceProfile.setName("Device profile v1.1");
+        deviceProfile.setVersion(1L);
+        String response = doPost("/api/deviceProfile", deviceProfile).andExpect(status().isConflict())
+                .andReturn().getResponse().getContentAsString();
+        assertThat(JacksonUtil.toJsonNode(response).get("message").asText())
+                .containsIgnoringCase("already changed by someone else");
+
+        deviceProfile.setVersion(null); // overriding entity
+        deviceProfile = saveDeviceProfile(deviceProfile);
+        assertThat(deviceProfile.getName()).isEqualTo("Device profile v1.1");
+        assertThat(deviceProfile.getVersion()).isEqualTo(3);
+    }
+
+    private DeviceProfile saveDeviceProfile(String name) {
         DeviceProfile deviceProfile = createDeviceProfile(name);
+        return saveDeviceProfile(deviceProfile);
+    }
+
+    private DeviceProfile saveDeviceProfile(DeviceProfile deviceProfile) {
         return doPost("/api/deviceProfile", deviceProfile, DeviceProfile.class);
     }
 

dao/src/main/java/org/thingsboard/server/dao/sql/JpaAbstractDao.java

@@ -88,10 +88,10 @@ public abstract class JpaAbstractDao<E extends BaseEntity<D>, D>
         boolean flushed = false;
         EntityManager entityManager = getEntityManager();
         if (isNew) {
+            entityManager.persist(entity);
             if (entity instanceof HasVersion versionedEntity) {
                 versionedEntity.setVersion(1L);
             }
-            entityManager.persist(entity);
         } else {
             if (entity instanceof HasVersion versionedEntity) {
                 if (versionedEntity.getVersion() == null) {
@@ -106,23 +106,25 @@ public abstract class JpaAbstractDao<E extends BaseEntity<D>, D>
                     }
                 }
                 versionedEntity = entityManager.merge(versionedEntity);
+                entity = (E) 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 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(versionedEntity);
-                flushed = true;
-                entity = (E) versionedEntity;
             } else {
                 entity = entityManager.merge(entity);
             }
         }
+        if (entity instanceof HasVersion versionedEntity) {
+            /*
+             * 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(versionedEntity);
+            flushed = true;
+        }
         if (flush && !flushed) {
             entityManager.flush();
         }