Url validation for Ota Package and tests

application/src/main/java/org/thingsboard/server/controller/OtaPackageController.java

@@ -34,10 +34,10 @@ import org.thingsboard.server.common.data.OtaPackage;
 import org.thingsboard.server.common.data.OtaPackageInfo;
 import org.thingsboard.server.common.data.audit.ActionType;
 import org.thingsboard.server.common.data.exception.ThingsboardException;
-import org.thingsboard.server.common.data.ota.ChecksumAlgorithm;
-import org.thingsboard.server.common.data.ota.OtaPackageType;
 import org.thingsboard.server.common.data.id.DeviceProfileId;
 import org.thingsboard.server.common.data.id.OtaPackageId;
+import org.thingsboard.server.common.data.ota.ChecksumAlgorithm;
+import org.thingsboard.server.common.data.ota.OtaPackageType;
 import org.thingsboard.server.common.data.page.PageData;
 import org.thingsboard.server.common.data.page.PageLink;
 import org.thingsboard.server.queue.util.TbCoreComponent;
@@ -109,12 +109,12 @@ public class OtaPackageController extends BaseController {
     @PreAuthorize("hasAnyAuthority('TENANT_ADMIN')")
     @RequestMapping(value = "/otaPackage", method = RequestMethod.POST)
     @ResponseBody
-    public OtaPackageInfo saveOtaPackageInfo(@RequestBody OtaPackageInfo otaPackageInfo) throws ThingsboardException {
+    public OtaPackageInfo saveOtaPackageInfo(@RequestBody OtaPackageInfo otaPackageInfo, @RequestParam boolean isUrl) throws ThingsboardException {
         boolean created = otaPackageInfo.getId() == null;
         try {
             otaPackageInfo.setTenantId(getTenantId());
             checkEntity(otaPackageInfo.getId(), otaPackageInfo, Resource.OTA_PACKAGE);
-            OtaPackageInfo savedOtaPackageInfo = otaPackageService.saveOtaPackageInfo(otaPackageInfo);
+            OtaPackageInfo savedOtaPackageInfo = otaPackageService.saveOtaPackageInfo(otaPackageInfo, isUrl);
             logEntityAction(savedOtaPackageInfo.getId(), savedOtaPackageInfo,
                     null, created ? ActionType.ADDED : ActionType.UPDATED, null);
             return savedOtaPackageInfo;

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

@@ -98,7 +98,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
 
-        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
         Assert.assertNotNull(savedFirmwareInfo);
         Assert.assertNotNull(savedFirmwareInfo.getId());
@@ -109,7 +109,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
 
         savedFirmwareInfo.setAdditionalInfo(JacksonUtil.newObjectNode());
 
-        save(savedFirmwareInfo);
+        save(savedFirmwareInfo, false);
 
         OtaPackageInfo foundFirmwareInfo = doGet("/api/otaPackage/info/" + savedFirmwareInfo.getId().getId().toString(), OtaPackageInfo.class);
         Assert.assertEquals(foundFirmwareInfo.getTitle(), savedFirmwareInfo.getTitle());
@@ -123,7 +123,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
 
-        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
         Assert.assertNotNull(savedFirmwareInfo);
         Assert.assertNotNull(savedFirmwareInfo.getId());
@@ -134,7 +134,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
 
         savedFirmwareInfo.setAdditionalInfo(JacksonUtil.newObjectNode());
 
-        save(savedFirmwareInfo);
+        save(savedFirmwareInfo, false);
 
         OtaPackageInfo foundFirmwareInfo = doGet("/api/otaPackage/info/" + savedFirmwareInfo.getId().getId().toString(), OtaPackageInfo.class);
         Assert.assertEquals(foundFirmwareInfo.getTitle(), savedFirmwareInfo.getTitle());
@@ -157,10 +157,10 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
 
-        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
         loginDifferentTenant();
-        doPost("/api/otaPackage", savedFirmwareInfo, OtaPackageInfo.class, status().isForbidden());
+        doPost("/api/otaPackage?isUrl=false", savedFirmwareInfo, OtaPackageInfo.class, status().isForbidden());
         deleteDifferentTenant();
     }
 
@@ -172,7 +172,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
 
-        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
         OtaPackageInfo foundFirmware = doGet("/api/otaPackage/info/" + savedFirmwareInfo.getId().getId().toString(), OtaPackageInfo.class);
         Assert.assertNotNull(foundFirmware);
@@ -187,7 +187,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
 
-        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
         MockMultipartFile testData = new MockMultipartFile("file", FILE_NAME, CONTENT_TYPE, DATA.array());
 
@@ -207,7 +207,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
 
-        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
         doDelete("/api/otaPackage/" + savedFirmwareInfo.getId().getId().toString())
                 .andExpect(status().isOk());
@@ -226,7 +226,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
             firmwareInfo.setTitle(TITLE);
             firmwareInfo.setVersion(VERSION + i);
 
-            OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+            OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
             if (i > 100) {
                 MockMultipartFile testData = new MockMultipartFile("file", FILE_NAME, CONTENT_TYPE, DATA.array());
@@ -269,7 +269,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
             firmwareInfo.setTitle(TITLE);
             firmwareInfo.setVersion(VERSION + i);
 
-            OtaPackageInfo savedFirmwareInfo = save(firmwareInfo);
+            OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false);
 
             if (i > 100) {
                 MockMultipartFile testData = new MockMultipartFile("file", FILE_NAME, CONTENT_TYPE, DATA.array());
@@ -316,9 +316,8 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes
         Assert.assertEquals(allOtaPackages, allLoadedOtaPackages);
     }
 
-
+    private OtaPackageInfo save(OtaPackageInfo firmwareInfo, boolean isUrl) throws Exception {
-    private OtaPackageInfo save(OtaPackageInfo firmwareInfo) throws Exception {
+        return doPost("/api/otaPackage?isUrl=" + isUrl, firmwareInfo, OtaPackageInfo.class);
-        return doPost("/api/otaPackage", firmwareInfo, OtaPackageInfo.class);
     }
 
     protected OtaPackageInfo savaData(String urlTemplate, MockMultipartFile content, String... params) throws Exception {

common/dao-api/src/main/java/org/thingsboard/server/dao/ota/OtaPackageService.java

@@ -18,11 +18,11 @@ package org.thingsboard.server.dao.ota;
 import com.google.common.util.concurrent.ListenableFuture;
 import org.thingsboard.server.common.data.OtaPackage;
 import org.thingsboard.server.common.data.OtaPackageInfo;
-import org.thingsboard.server.common.data.ota.ChecksumAlgorithm;
-import org.thingsboard.server.common.data.ota.OtaPackageType;
 import org.thingsboard.server.common.data.id.DeviceProfileId;
 import org.thingsboard.server.common.data.id.OtaPackageId;
 import org.thingsboard.server.common.data.id.TenantId;
+import org.thingsboard.server.common.data.ota.ChecksumAlgorithm;
+import org.thingsboard.server.common.data.ota.OtaPackageType;
 import org.thingsboard.server.common.data.page.PageData;
 import org.thingsboard.server.common.data.page.PageLink;
 
@@ -30,7 +30,7 @@ import java.nio.ByteBuffer;
 
 public interface OtaPackageService {
 
-    OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo);
+    OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo, boolean isUrl);
 
     OtaPackage saveOtaPackage(OtaPackage otaPackage);
 

dao/src/main/java/org/thingsboard/server/dao/ota/BaseOtaPackageService.java

@@ -77,8 +77,11 @@ public class BaseOtaPackageService implements OtaPackageService {
     private TbTenantProfileCache tenantProfileCache;
 
     @Override
-    public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo) {
+    public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo, boolean isUrl) {
         log.trace("Executing saveOtaPackageInfo [{}]", otaPackageInfo);
+        if(isUrl && (StringUtils.isEmpty(otaPackageInfo.getUrl()) || otaPackageInfo.getUrl().trim().length() == 0)) {
+            throw new DataValidationException("Ota package URL should be specified!");
+        }
         otaPackageInfoValidator.validate(otaPackageInfo, OtaPackageInfo::getTenantId);
         try {
             OtaPackageId otaPackageId = otaPackageInfo.getId();
@@ -277,7 +280,9 @@ public class BaseOtaPackageService implements OtaPackageService {
                     throw new DataValidationException("Wrong otaPackage file!");
                 }
             } else {
-                //TODO: validate url
+                if(otaPackage.getData() != null) {
+                    throw new DataValidationException("File can't be saved if URL present!");
+                }
             }
         }
 
@@ -336,6 +341,9 @@ public class BaseOtaPackageService implements OtaPackageService {
         if (otaPackageOld.getDataSize() != null && !otaPackageOld.getDataSize().equals(otaPackage.getDataSize())) {
             throw new DataValidationException("Updating otaPackage data size is prohibited!");
         }
+        if(otaPackageOld.getUrl() != null && !otaPackageOld.getUrl().equals(otaPackage.getUrl())) {
+            throw new DataValidationException("Updating otaPackage URL is prohibited!");
+        }
     }
 
     private void validateImpl(OtaPackageInfo otaPackageInfo) {

dao/src/test/java/org/thingsboard/server/dao/service/BaseOtaPackageServiceTest.java

@@ -163,7 +163,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmware.setVersion(VERSION);
         firmware.setUrl(URL);
         firmware.setDataSize(0L);
-        OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware);
+        OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware, true);
 
         Assert.assertNotNull(savedFirmware);
         Assert.assertNotNull(savedFirmware.getId());
@@ -174,7 +174,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         Assert.assertEquals(firmware.getContentType(), savedFirmware.getContentType());
 
         savedFirmware.setAdditionalInfo(JacksonUtil.newObjectNode());
-        otaPackageService.saveOtaPackageInfo(savedFirmware);
+        otaPackageService.saveOtaPackageInfo(savedFirmware, true);
 
         OtaPackage foundFirmware = otaPackageService.findOtaPackageById(tenantId, savedFirmware.getId());
         Assert.assertEquals(foundFirmware.getTitle(), savedFirmware.getTitle());
@@ -190,7 +190,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmwareInfo.setType(FIRMWARE);
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
-        OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo);
+        OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo, false);
 
         Assert.assertNotNull(savedFirmwareInfo);
         Assert.assertNotNull(savedFirmwareInfo.getId());
@@ -216,7 +216,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
 
         savedFirmwareInfo = otaPackageService.findOtaPackageInfoById(tenantId, savedFirmwareInfo.getId());
         savedFirmwareInfo.setAdditionalInfo(JacksonUtil.newObjectNode());
-        otaPackageService.saveOtaPackageInfo(savedFirmwareInfo);
+        otaPackageService.saveOtaPackageInfo(savedFirmwareInfo, false);
 
         OtaPackage foundFirmware = otaPackageService.findOtaPackageById(tenantId, firmware.getId());
         firmware.setAdditionalInfo(JacksonUtil.newObjectNode());
@@ -399,7 +399,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmwareInfo.setType(FIRMWARE);
         firmwareInfo.setTitle(TITLE);
         firmwareInfo.setVersion(VERSION);
-        otaPackageService.saveOtaPackageInfo(firmwareInfo);
+        otaPackageService.saveOtaPackageInfo(firmwareInfo, false);
 
         OtaPackageInfo newFirmwareInfo = new OtaPackageInfo();
         newFirmwareInfo.setTenantId(tenantId);
@@ -410,7 +410,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
 
         thrown.expect(DataValidationException.class);
         thrown.expectMessage("OtaPackage with such title and version already exists!");
-        otaPackageService.saveOtaPackageInfo(newFirmwareInfo);
+        otaPackageService.saveOtaPackageInfo(newFirmwareInfo, false);
     }
 
     @Test
@@ -506,7 +506,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmware.setType(FIRMWARE);
         firmware.setTitle(TITLE);
         firmware.setVersion(VERSION);
-        OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware);
+        OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware, false);
 
         OtaPackageInfo foundFirmware = otaPackageService.findOtaPackageInfoById(tenantId, savedFirmware.getId());
         Assert.assertNotNull(foundFirmware);
@@ -543,7 +543,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmwareWithUrl.setUrl(URL);
         firmwareWithUrl.setDataSize(0L);
 
-        OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl);
+        OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl, true);
         savedFwWithUrl.setHasData(true);
 
         firmwares.add(savedFwWithUrl);
@@ -588,7 +588,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         firmwareWithUrl.setUrl(URL);
         firmwareWithUrl.setDataSize(0L);
 
-        OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl);
+        OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl, true);
         savedFwWithUrl.setHasData(true);
 
         firmwares.add(savedFwWithUrl);
@@ -627,6 +627,40 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest {
         Assert.assertTrue(pageData.getData().isEmpty());
     }
 
+    @Test
+    public void testSaveOtaPackageInfoWithBlankAndEmptyUrl() {
+        OtaPackageInfo firmwareInfo = new OtaPackageInfo();
+        firmwareInfo.setDeviceProfileId(deviceProfileId);
+        firmwareInfo.setType(FIRMWARE);
+        firmwareInfo.setTitle(TITLE);
+        firmwareInfo.setVersion(VERSION);
+        firmwareInfo.setUrl("   ");
+        thrown.expect(DataValidationException.class);
+        thrown.expectMessage("Ota package URL should be specified!");
+        otaPackageService.saveOtaPackageInfo(firmwareInfo, true);
+        firmwareInfo.setUrl("");
+        otaPackageService.saveOtaPackageInfo(firmwareInfo, true);
+    }
+
+    @Test
+    public void testSaveOtaPackageUrlCantBeUpdated() {
+        OtaPackageInfo firmwareInfo = new OtaPackageInfo();
+        firmwareInfo.setDeviceProfileId(deviceProfileId);
+        firmwareInfo.setType(FIRMWARE);
+        firmwareInfo.setTitle(TITLE);
+        firmwareInfo.setVersion(VERSION);
+        firmwareInfo.setUrl(URL);
+        firmwareInfo.setTenantId(tenantId);
+
+        OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo, true);
+
+        thrown.expect(DataValidationException.class);
+        thrown.expectMessage("Updating otaPackage URL is prohibited!");
+
+        savedFirmwareInfo.setUrl("https://newurl.com");
+        otaPackageService.saveOtaPackageInfo(savedFirmwareInfo, true);
+    }
+
     private OtaPackage createFirmware(TenantId tenantId, String version) {
         OtaPackage firmware = new OtaPackage();
         firmware.setTenantId(tenantId);

rest-client/src/main/java/org/thingsboard/rest/client/RestClient.java

@@ -2967,8 +2967,10 @@ public class RestClient implements ClientHttpRequestInterceptor, Closeable {
         ).getBody();
     }
 
-    public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo) {
+    public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo, boolean isUrl) {
-        return restTemplate.postForEntity(baseURL + "/api/otaPackage", otaPackageInfo, OtaPackageInfo.class).getBody();
+        Map<String, String> params = new HashMap<>();
+        params.put("isUrl", Boolean.toString(isUrl));
+        return restTemplate.postForEntity(baseURL + "/api/otaPackage?isUrl={isUrl}", otaPackageInfo, OtaPackageInfo.class, params).getBody();
     }
 
     public OtaPackageInfo saveOtaPackageData(OtaPackageId otaPackageId, String checkSum, ChecksumAlgorithm checksumAlgorithm, MultipartFile file) throws Exception {