Merge pull request #14775 from AndriiLandiak/ota-data-clean-up

Added OTA package data cleanup

application/src/test/java/org/thingsboard/server/service/housekeeper/HousekeeperServiceTest.java

@@ -23,8 +23,8 @@ import org.junit.Test;
 import org.mockito.ArgumentMatcher;
 import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.test.mock.mockito.SpyBean;
 import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
 import org.testcontainers.shaded.org.apache.commons.lang3.RandomStringUtils;
 import org.thingsboard.common.util.JacksonUtil;
 import org.thingsboard.rule.engine.metadata.TbGetAttributesNode;
@@ -127,10 +127,12 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 })
 public class HousekeeperServiceTest extends AbstractControllerTest {
 
-    @SpyBean
+    @MockitoSpyBean
     private HousekeeperService housekeeperService;
-    @SpyBean
+    @MockitoSpyBean
     private HousekeeperReprocessingService housekeeperReprocessingService;
+    @MockitoSpyBean
+    private TsHistoryDeletionTaskProcessor tsHistoryDeletionTaskProcessor;
     @Autowired
     private EventService eventService;
     @Autowired
@@ -153,8 +155,6 @@ public class HousekeeperServiceTest extends AbstractControllerTest {
     private CustomerService customerService;
     @Autowired
     private DashboardService dashboardService;
-    @SpyBean
-    private TsHistoryDeletionTaskProcessor tsHistoryDeletionTaskProcessor;
 
     private TenantId tenantId;
 

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/AlarmsDeletionHousekeeperTask.java

@@ -23,6 +23,7 @@ import lombok.ToString;
 import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.id.TenantId;
 
+import java.io.Serial;
 import java.util.List;
 import java.util.UUID;
 
@@ -32,6 +33,9 @@ import java.util.UUID;
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class AlarmsDeletionHousekeeperTask extends HousekeeperTask {
 
+    @Serial
+    private static final long serialVersionUID = 9214680001573764374L;
+
     private List<UUID> alarms;
 
     public AlarmsDeletionHousekeeperTask(TenantId tenantId, EntityId entityId) {

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/AlarmsUnassignHousekeeperTask.java

@@ -24,6 +24,7 @@ import org.thingsboard.server.common.data.User;
 import org.thingsboard.server.common.data.id.TenantId;
 import org.thingsboard.server.common.data.id.UserId;
 
+import java.io.Serial;
 import java.util.List;
 import java.util.UUID;
 
@@ -33,6 +34,9 @@ import java.util.UUID;
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class AlarmsUnassignHousekeeperTask extends HousekeeperTask {
 
+    @Serial
+    private static final long serialVersionUID = 9156667024462937756L;
+
     private String userTitle;
     private List<UUID> alarms;
 

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/EntitiesDeletionHousekeeperTask.java

@@ -23,6 +23,7 @@ import lombok.ToString;
 import org.thingsboard.server.common.data.EntityType;
 import org.thingsboard.server.common.data.id.TenantId;
 
+import java.io.Serial;
 import java.util.List;
 import java.util.UUID;
 
@@ -32,6 +33,9 @@ import java.util.UUID;
 @NoArgsConstructor
 public class EntitiesDeletionHousekeeperTask extends HousekeeperTask {
 
+    @Serial
+    private static final long serialVersionUID = 9009068831061529286L;
+
     private EntityType entityType;
     private List<UUID> entities;
 

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/HousekeeperTask.java

@@ -29,6 +29,7 @@ import org.thingsboard.server.common.data.User;
 import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.id.TenantId;
 
+import java.io.Serial;
 import java.io.Serializable;
 
 @JsonIgnoreProperties(ignoreUnknown = true)
@@ -45,6 +46,9 @@ import java.io.Serializable;
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class HousekeeperTask implements Serializable {
 
+    @Serial
+    private static final long serialVersionUID = -2585974110832225152L;
+
     private TenantId tenantId;
     private EntityId entityId;
     private HousekeeperTaskType taskType;

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/LatestTsDeletionHousekeeperTask.java

@@ -23,12 +23,17 @@ import lombok.ToString;
 import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.id.TenantId;
 
+import java.io.Serial;
+
 @Data
 @ToString(callSuper = true)
 @EqualsAndHashCode(callSuper = true)
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class LatestTsDeletionHousekeeperTask extends HousekeeperTask {
 
+    @Serial
+    private static final long serialVersionUID = 5193191938513490138L;
+
     private String key;
 
     public LatestTsDeletionHousekeeperTask(TenantId tenantId, EntityId entityId, String key) {

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/TenantEntitiesDeletionHousekeeperTask.java

@@ -23,12 +23,17 @@ import lombok.ToString;
 import org.thingsboard.server.common.data.EntityType;
 import org.thingsboard.server.common.data.id.TenantId;
 
+import java.io.Serial;
+
 @Data
 @ToString(callSuper = true)
 @EqualsAndHashCode(callSuper = true)
 @NoArgsConstructor
 public class TenantEntitiesDeletionHousekeeperTask extends HousekeeperTask {
 
+    @Serial
+    private static final long serialVersionUID = -8033108795318393447L;
+
     private EntityType entityType;
 
     public TenantEntitiesDeletionHousekeeperTask(TenantId tenantId, EntityType entityType) {

common/data/src/main/java/org/thingsboard/server/common/data/housekeeper/TsHistoryDeletionHousekeeperTask.java

@@ -23,12 +23,17 @@ import lombok.ToString;
 import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.id.TenantId;
 
+import java.io.Serial;
+
 @Data
 @ToString(callSuper = true)
 @EqualsAndHashCode(callSuper = true)
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class TsHistoryDeletionHousekeeperTask extends HousekeeperTask {
 
+    @Serial
+    private static final long serialVersionUID = 4573851542705079043L;
+
     private String key;
 
     public TsHistoryDeletionHousekeeperTask(TenantId tenantId, EntityId entityId, String key) {

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

@@ -196,7 +196,9 @@ public class BaseOtaPackageService extends AbstractCachedEntityService<OtaPackag
         log.trace("Executing deleteOtaPackage [{}]", otaPackageId);
         validateId(otaPackageId, id -> INCORRECT_OTA_PACKAGE_ID + id);
         try {
+            Long oid = getDataOidById(tenantId, otaPackageId);
             otaPackageDao.removeById(tenantId, otaPackageId.getId());
+            unlinkDataIfPresent(tenantId, otaPackageId, oid);
             publishEvictEvent(new OtaPackageCacheEvictEvent(otaPackageId));
             eventPublisher.publishEvent(DeleteEntityEvent.builder().tenantId(tenantId).entityId(otaPackageId).build());
         } catch (Exception t) {
@@ -215,6 +217,30 @@ public class BaseOtaPackageService extends AbstractCachedEntityService<OtaPackag
         }
     }
 
+    private Long getDataOidById(TenantId tenantId, OtaPackageId otaPackageId) {
+        try {
+            log.trace("Executing getDataOidById tenantId [{}], otaPackageId [{}]", tenantId, otaPackageId);
+            return otaPackageDao.getDataOidById(otaPackageId.getId());
+        } catch (Exception e) {
+            log.warn("[{}][{}] Failed to retrieve OID before deletion", tenantId, otaPackageId, e);
+        }
+        return null;
+    }
+
+    private void unlinkDataIfPresent(TenantId tenantId, OtaPackageId otaPackageId, Long oid) {
+        if (oid == null) {
+            return;
+        }
+        try {
+            Integer result = otaPackageDao.unlinkLargeObject(oid);
+            if (result != 1) {
+                log.warn("[{}][{}] Failed to delete large object (OID: {}). Result code: {}", tenantId, otaPackageId, oid, result);
+            }
+        } catch (Exception e) {
+            log.warn("[{}][{}] Failed to delete large object (OID: {})", tenantId, otaPackageId, oid, e);
+        }
+    }
+
     @Override
     public void deleteEntity(TenantId tenantId, EntityId id, boolean force) {
         deleteOtaPackage(tenantId, (OtaPackageId) id);
@@ -225,6 +251,11 @@ public class BaseOtaPackageService extends AbstractCachedEntityService<OtaPackag
         return otaPackageDao.sumDataSizeByTenantId(tenantId);
     }
 
+    @Override
+    public void deleteByTenantId(TenantId tenantId) {
+        deleteOtaPackagesByTenantId(tenantId);
+    }
+
     @Override
     public void deleteOtaPackagesByTenantId(TenantId tenantId) {
         log.trace("Executing deleteOtaPackagesByTenantId, tenantId [{}]", tenantId);
@@ -232,24 +263,17 @@ public class BaseOtaPackageService extends AbstractCachedEntityService<OtaPackag
         tenantOtaPackageRemover.removeEntities(tenantId, tenantId);
     }
 
-    @Override
+    private final PaginatedRemover<TenantId, OtaPackageInfo> tenantOtaPackageRemover = new PaginatedRemover<>() {
-    public void deleteByTenantId(TenantId tenantId) {
+        @Override
-        deleteOtaPackagesByTenantId(tenantId);
+        protected PageData<OtaPackageInfo> findEntities(TenantId tenantId, TenantId id, PageLink pageLink) {
-    }
+            return otaPackageInfoDao.findOtaPackageInfoByTenantId(id, pageLink);
-
+        }
-    private PaginatedRemover<TenantId, OtaPackageInfo> tenantOtaPackageRemover =
-            new PaginatedRemover<>() {
-
-                @Override
-                protected PageData<OtaPackageInfo> findEntities(TenantId tenantId, TenantId id, PageLink pageLink) {
-                    return otaPackageInfoDao.findOtaPackageInfoByTenantId(id, pageLink);
-                }
 
-                @Override
+        @Override
-                protected void removeEntity(TenantId tenantId, OtaPackageInfo entity) {
+        protected void removeEntity(TenantId tenantId, OtaPackageInfo entity) {
-                    deleteOtaPackage(tenantId, entity.getId());
+            deleteOtaPackage(tenantId, entity.getId());
-                }
+        }
-            };
+    };
 
     @Override
     public Optional<HasId<?>> findEntity(TenantId tenantId, EntityId entityId) {

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

@@ -18,15 +18,20 @@ package org.thingsboard.server.dao.ota;
 import org.thingsboard.server.common.data.OtaPackage;
 import org.thingsboard.server.common.data.id.OtaPackageId;
 import org.thingsboard.server.common.data.id.TenantId;
-import org.thingsboard.server.common.data.ota.OtaPackageType;
 import org.thingsboard.server.dao.Dao;
 import org.thingsboard.server.dao.ExportableEntityDao;
 import org.thingsboard.server.dao.TenantEntityWithDataDao;
 
+import java.util.UUID;
+
 public interface OtaPackageDao extends Dao<OtaPackage>, TenantEntityWithDataDao, ExportableEntityDao<OtaPackageId, OtaPackage> {
 
     Long sumDataSizeByTenantId(TenantId tenantId);
 
     OtaPackage findOtaPackageByTenantIdAndTitleAndVersion(TenantId tenantId, String title, String version);
 
+    Long getDataOidById(UUID id);
+
+    Integer unlinkLargeObject(Long dataOid);
+
 }

dao/src/main/java/org/thingsboard/server/dao/sql/ota/JpaOtaPackageDao.java

@@ -77,6 +77,16 @@ public class JpaOtaPackageDao extends JpaAbstractDao<OtaPackageEntity, OtaPackag
         return DaoUtil.getData(otaPackageRepository.findByTenantIdAndExternalId(tenantId, externalId));
     }
 
+    @Override
+    public Long getDataOidById(UUID id) {
+        return otaPackageRepository.getDataOidById(id);
+    }
+
+    @Override
+    public Integer unlinkLargeObject(Long dataOid) {
+        return otaPackageRepository.unlinkLargeObject(dataOid);
+    }
+
     @Override
     public OtaPackageId getExternalIdByInternal(OtaPackageId internalId) {
         return DaoUtil.toEntityId(otaPackageRepository.getExternalIdById(internalId.getId()), OtaPackageId::new);

dao/src/main/java/org/thingsboard/server/dao/sql/ota/OtaPackageRepository.java

@@ -20,7 +20,7 @@ import org.springframework.data.domain.Pageable;
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Query;
 import org.springframework.data.repository.query.Param;
-import org.thingsboard.server.common.data.ota.OtaPackageType;
+import org.springframework.transaction.annotation.Transactional;
 import org.thingsboard.server.dao.ExportableEntityRepository;
 import org.thingsboard.server.dao.model.sql.OtaPackageEntity;
 
@@ -41,4 +41,12 @@ public interface OtaPackageRepository extends JpaRepository<OtaPackageEntity, UU
     @Query("SELECT r.id FROM OtaPackageEntity r WHERE r.tenantId = :tenantId")
     Page<UUID> findIdsByTenantId(@Param("tenantId") UUID tenantId, Pageable pageable);
 
+    // The 'data' column is of type OID (PostgreSQL large object reference), so it returns the OID as Long
+    @Query(value = "SELECT data FROM ota_package WHERE id = :id AND data IS NOT NULL", nativeQuery = true)
+    Long getDataOidById(@Param("id") UUID id);
+
+    @Transactional
+    @Query(value = "SELECT lo_unlink(:oid)", nativeQuery = true)
+    Integer unlinkLargeObject(@Param("oid") Long oid);
+
 }

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

@@ -37,13 +37,13 @@ import org.thingsboard.server.common.data.tenant.profile.DefaultTenantProfileCon
 import org.thingsboard.server.dao.device.DeviceProfileService;
 import org.thingsboard.server.dao.device.DeviceService;
 import org.thingsboard.server.dao.exception.DataValidationException;
+import org.thingsboard.server.dao.ota.OtaPackageDao;
 import org.thingsboard.server.dao.ota.OtaPackageService;
 import org.thingsboard.server.dao.tenant.TbTenantProfileCache;
 import org.thingsboard.server.dao.tenant.TenantProfileService;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -77,6 +77,8 @@ public class OtaPackageServiceTest extends AbstractServiceTest {
     @Autowired
     TenantProfileService tenantProfileService;
     @Autowired
+    OtaPackageDao otaPackageDao;
+    @Autowired
     TbTenantProfileCache tenantProfileCache;
 
     @Before
@@ -118,10 +120,8 @@ public class OtaPackageServiceTest extends AbstractServiceTest {
         Assert.assertEquals(1, otaPackageService.sumDataSizeByTenantId(tenantId));
 
         int maxSumDataSize = 8;
-        List<OtaPackage> packages = new ArrayList<>(maxSumDataSize);
-
         for (int i = 2; i <= maxSumDataSize; i++) {
-            packages.add(createAndSaveFirmware(tenantId, "0." + i));
+            createAndSaveFirmware(tenantId, "0." + i);
             Assert.assertEquals(i, otaPackageService.sumDataSizeByTenantId(tenantId));
         }
 
@@ -533,6 +533,39 @@ public class OtaPackageServiceTest extends AbstractServiceTest {
         Assert.assertNull(foundFirmware);
     }
 
+    @Test
+    public void testDeleteOtaPackageWithoutData() {
+        OtaPackageInfo firmwareInfo = new OtaPackageInfo();
+        firmwareInfo.setTenantId(tenantId);
+        firmwareInfo.setDeviceProfileId(deviceProfileId);
+        firmwareInfo.setType(FIRMWARE);
+        firmwareInfo.setTitle(TITLE);
+        firmwareInfo.setVersion(VERSION);
+        OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo, false);
+
+        Assert.assertNotNull(savedFirmwareInfo);
+        Assert.assertNotNull(savedFirmwareInfo.getId());
+
+        // Should not throw NPE when deleting package without data (OID is null)
+        otaPackageService.deleteOtaPackage(tenantId, savedFirmwareInfo.getId());
+
+        OtaPackageInfo foundFirmware = otaPackageService.findOtaPackageInfoById(tenantId, savedFirmwareInfo.getId());
+        Assert.assertNull(foundFirmware);
+    }
+
+    @Test
+    public void testDeleteOtaPackageUnlinksLargeObject() {
+        OtaPackage savedFirmware = createAndSaveFirmware(tenantId, VERSION);
+
+        Long oid = otaPackageDao.getDataOidById(savedFirmware.getId().getId());
+        Assert.assertNotNull(oid);
+
+        otaPackageService.deleteOtaPackage(tenantId, savedFirmware.getId());
+
+        // Verify the large object was unlinked - PostgreSQL throws an exception when the object doesn't exist
+        assertThatThrownBy(() -> otaPackageDao.unlinkLargeObject(oid)).hasMessageContaining("large object " + oid + " does not exist");
+    }
+
     @Test
     public void testFindTenantFirmwaresByTenantId() {
         List<OtaPackageInfo> firmwares = new ArrayList<>();
@@ -567,8 +600,8 @@ public class OtaPackageServiceTest extends AbstractServiceTest {
             }
         } while (pageData.hasNext());
 
-        Collections.sort(firmwares, idComparator);
+        firmwares.sort(idComparator);
-        Collections.sort(loadedFirmwares, idComparator);
+        loadedFirmwares.sort(idComparator);
 
         assertThat(firmwares).isEqualTo(loadedFirmwares);
 
@@ -622,8 +655,8 @@ public class OtaPackageServiceTest extends AbstractServiceTest {
             }
         } while (pageData.hasNext());
 
-        Collections.sort(firmwares, idComparator);
+        firmwares.sort(idComparator);
-        Collections.sort(loadedFirmwares, idComparator);
+        loadedFirmwares.sort(idComparator);
 
         assertThat(firmwares).isEqualTo(loadedFirmwares);
 
@@ -726,4 +759,5 @@ public class OtaPackageServiceTest extends AbstractServiceTest {
         firmware.setDataSize(DATA_SIZE);
         return firmware;
     }
+
 }