From ea70386a7a8d9a4dd47c267ab7e777b41d66bd5f Mon Sep 17 00:00:00 2001 From: dpinkevych Date: Mon, 15 Jun 2026 13:18:06 +0300 Subject: [PATCH] fix alreadInstalledItems to searchByTenantAndItemId instead of only tenantId --- .../service/iot_hub/DefaultIotHubService.java | 56 +++++++++++++++--- .../iot_hub/DefaultIotHubServiceTest.java | 58 +++++++++++++++---- .../dao/iot_hub/IotHubInstalledItemDao.java | 3 + .../iot_hub/IotHubInstalledItemService.java | 3 + .../IotHubInstalledItemServiceImpl.java | 6 ++ .../IotHubInstalledItemRepository.java | 6 ++ .../iot_hub/JpaIotHubInstalledItemDao.java | 6 ++ 7 files changed, 119 insertions(+), 19 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/iot_hub/DefaultIotHubService.java b/application/src/main/java/org/thingsboard/server/service/iot_hub/DefaultIotHubService.java index 201e48001e..4ac2cd4244 100644 --- a/application/src/main/java/org/thingsboard/server/service/iot_hub/DefaultIotHubService.java +++ b/application/src/main/java/org/thingsboard/server/service/iot_hub/DefaultIotHubService.java @@ -713,7 +713,24 @@ public class DefaultIotHubService implements IotHubService { throw new IllegalArgumentException("Marketplace version not found: " + versionId); } - Set alreadyInstalledItemIds = new HashSet<>(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)); + // Null-safe: a malformed payload without an itemId must surface the friendly + // IllegalArgumentException thrown by addPlanEntry(root) below, not an NPE here. + String rootItemId = optText(rootVersion, FIELD_ITEM_ID); + JsonNode related = rootVersion.get(FIELD_RELATED_ITEMS); + + // The plan only ever touches the root and its (one-level-deep) related items, so we ask the + // DB whether just those item ids are already installed instead of loading every installed id + // for the tenant — a tenant may have thousands installed while a plan checks a handful. + Set candidateItemIds = new HashSet<>(); + addCandidateItemId(candidateItemIds, rootItemId); + if (related != null && related.isArray()) { + for (JsonNode relatedNode : related) { + addCandidateItemId(candidateItemIds, relatedNode.asText()); + } + } + Set alreadyInstalledItemIds = candidateItemIds.isEmpty() + ? Set.of() + : new HashSet<>(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(tenantId, candidateItemIds)); // LinkedHashMap preserves insertion order; we add the related items (deps) before the // root so a single forward iteration yields the correct install sequence (deps first, @@ -721,10 +738,6 @@ public class DefaultIotHubService implements IotHubService { // level deep, so there is no need to walk a related item's own related items. LinkedHashMap entries = new LinkedHashMap<>(); - // Null-safe: a malformed payload without an itemId must surface the friendly - // IllegalArgumentException thrown by addPlanEntry(root) below, not an NPE here. - String rootItemId = optText(rootVersion, FIELD_ITEM_ID); - JsonNode related = rootVersion.get(FIELD_RELATED_ITEMS); if (related != null && related.isArray()) { for (JsonNode relatedNode : related) { String relatedItemId = relatedNode.asText(); @@ -816,9 +829,22 @@ public class DefaultIotHubService implements IotHubService { } // The plan is resolved by a separate request, so by the time it is submitted an item may - // already have been installed (stale or replayed plan). Re-check against the current state - // so we never install the same item twice. - Set alreadyInstalledItemIds = new HashSet<>(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)); + // already have been installed (stale or replayed plan). Re-check the current state for only + // the items this plan would install, so we never install the same item twice. + Set candidateItemIds = new HashSet<>(); + for (InstallPlanEntry entry : plan.getEntries()) { + if (entry.getStatus() == InstallPlanEntry.Status.WILL_INSTALL) { + addCandidateItemId(candidateItemIds, entry.getItemId()); + } + } + // Mutable on purpose: as the cascade installs each item we add its id below, so a later + // duplicate entry for the same item in this plan is recognised as installed and skipped + // instead of installed twice. A client-supplied plan is not de-duplicated the way a freshly + // resolved one is, so the loop has to guard against duplicates itself. + Set alreadyInstalledItemIds = new HashSet<>(); + if (!candidateItemIds.isEmpty()) { + alreadyInstalledItemIds.addAll(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(tenantId, candidateItemIds)); + } InstallPlanResult result = new InstallPlanResult(); List resultEntries = new ArrayList<>(); @@ -847,6 +873,9 @@ public class DefaultIotHubService implements IotHubService { JsonNode entryData = entry.isRoot() ? data : null; IotHubInstalledItem installed = doInstallVersion(user, entry.getVersionId(), entryData, request); rollbackIds.add(installed.getId()); + // Record it as installed so a duplicate entry for the same item later in this + // plan is skipped rather than installed a second time. + addCandidateItemId(alreadyInstalledItemIds, entry.getItemId()); if (entry.isRoot()) { rootDescriptor = installed.getDescriptor(); } @@ -895,6 +924,17 @@ public class DefaultIotHubService implements IotHubService { return fullyRolledBack; } + private static void addCandidateItemId(Set candidates, String itemId) { + if (itemId == null || itemId.isEmpty()) { + return; + } + try { + candidates.add(UUID.fromString(itemId)); + } catch (IllegalArgumentException ignored) { + // A non-UUID item id can never match an installed record — leave it out of the lookup. + } + } + private static boolean isAlreadyInstalled(String itemId, Set alreadyInstalledItemIds) { if (itemId == null) { return false; diff --git a/application/src/test/java/org/thingsboard/server/service/iot_hub/DefaultIotHubServiceTest.java b/application/src/test/java/org/thingsboard/server/service/iot_hub/DefaultIotHubServiceTest.java index 9631129a81..d505509e4a 100644 --- a/application/src/test/java/org/thingsboard/server/service/iot_hub/DefaultIotHubServiceTest.java +++ b/application/src/test/java/org/thingsboard/server/service/iot_hub/DefaultIotHubServiceTest.java @@ -103,7 +103,7 @@ class DefaultIotHubServiceTest { String versionId = UUID.randomUUID().toString(); String itemId = UUID.randomUUID().toString(); when(iotHubRestClient.getVersionInfo(versionId)).thenReturn(version(versionId, itemId, "DASHBOARD", "Root", "1.0")); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); InstallPlan plan = service.resolveInstallPlan(user, versionId); @@ -125,7 +125,7 @@ class DefaultIotHubServiceTest { when(iotHubRestClient.getVersionInfo(versionId)).thenReturn(root); when(iotHubRestClient.getPublishedVersionByItemId(relatedItemId)) .thenReturn(version(UUID.randomUUID().toString(), relatedItemId, "WIDGET", "Dep", "2.0")); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); InstallPlan plan = service.resolveInstallPlan(user, versionId); @@ -146,7 +146,7 @@ class DefaultIotHubServiceTest { root.putArray("relatedItems").add(relatedItemId); when(iotHubRestClient.getVersionInfo(versionId)).thenReturn(root); when(iotHubRestClient.getPublishedVersionByItemId(relatedItemId)).thenReturn(null); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); InstallPlan plan = service.resolveInstallPlan(user, versionId); @@ -166,7 +166,7 @@ class DefaultIotHubServiceTest { root.putArray("relatedItems").add(relatedItemId); when(iotHubRestClient.getVersionInfo(versionId)).thenReturn(root); when(iotHubRestClient.getPublishedVersionByItemId(relatedItemId)).thenThrow(new RuntimeException("boom")); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); InstallPlan plan = service.resolveInstallPlan(user, versionId); @@ -181,7 +181,7 @@ class DefaultIotHubServiceTest { UUID rootItemId = UUID.randomUUID(); when(iotHubRestClient.getVersionInfo(versionId)) .thenReturn(version(versionId, rootItemId.toString(), "DASHBOARD", "Root", "1.0")); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of(rootItemId)); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of(rootItemId)); InstallPlan plan = service.resolveInstallPlan(user, versionId); @@ -204,7 +204,7 @@ class DefaultIotHubServiceTest { assertThat(result.isSuccess()).isFalse(); assertThat(result.getErrorMessage()).contains("empty"); - verify(iotHubInstalledItemService, never()).findInstalledItemIdsByTenantId(any()); + verify(iotHubInstalledItemService, never()).findInstalledItemIdsByTenantIdAndItemIdIn(any(), any()); } @Test @@ -213,7 +213,6 @@ class DefaultIotHubServiceTest { InstallPlanEntry missing = new InstallPlanEntry(); missing.setItemId(UUID.randomUUID().toString()); missing.setStatus(InstallPlanEntry.Status.MISSING); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); InstallPlanResult result = service.installPlan(user, new InstallPlan(null, List.of(missing)), null, null); @@ -221,6 +220,8 @@ class DefaultIotHubServiceTest { assertThat(result.getMissingItemIds()).containsExactly(missing.getItemId()); assertThat(result.getRootDescriptor()).isNull(); verify(iotHubRestClient, never()).getVersionInfo(anyString()); + // A plan with nothing to install never hits the DB to check what is already installed. + verify(iotHubInstalledItemService, never()).findInstalledItemIdsByTenantIdAndItemIdIn(any(), any()); } @Test @@ -233,7 +234,7 @@ class DefaultIotHubServiceTest { entry.setStatus(InstallPlanEntry.Status.WILL_INSTALL); entry.setRoot(true); // The item is reported as already installed at install time, even though the plan says WILL_INSTALL. - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of(itemId)); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of(itemId)); InstallPlanResult result = service.installPlan(user, new InstallPlan(null, List.of(entry)), null, null); @@ -243,6 +244,41 @@ class DefaultIotHubServiceTest { verify(iotHubRestClient, never()).getVersionInfo(anyString()); } + @Test + void installPlan_duplicateItemId_installsOnce() throws Exception { + mockTenant(); + HttpServletRequest request = mock(HttpServletRequest.class); + UUID itemId = UUID.randomUUID(); + String firstVersionId = UUID.randomUUID().toString(); + String secondVersionId = UUID.randomUUID().toString(); + // Two entries point at the same item — a client-supplied plan is not de-duplicated, so the + // cascade itself must not install the same item twice. + InstallPlanEntry first = new InstallPlanEntry(); + first.setItemId(itemId.toString()); + first.setVersionId(firstVersionId); + first.setStatus(InstallPlanEntry.Status.WILL_INSTALL); + first.setRoot(true); + InstallPlanEntry duplicate = new InstallPlanEntry(); + duplicate.setItemId(itemId.toString()); + duplicate.setVersionId(secondVersionId); + duplicate.setStatus(InstallPlanEntry.Status.WILL_INSTALL); + duplicate.setRoot(false); + + // Nothing is installed yet when the plan is submitted. + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); + doReturn(installedItem(UUID.randomUUID())).when(service).doInstallVersion(eq(user), eq(firstVersionId), any(), any()); + + InstallPlanResult result = service.installPlan(user, new InstallPlan(firstVersionId, List.of(first, duplicate)), null, request); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.getEntries()).hasSize(2); + assertThat(result.getEntries().get(0).getStatus()).isEqualTo(InstallPlanEntry.Status.WILL_INSTALL); + // The duplicate is recognised as freshly installed and skipped. + assertThat(result.getEntries().get(1).getStatus()).isEqualTo(InstallPlanEntry.Status.ALREADY_INSTALLED); + verify(service).doInstallVersion(eq(user), eq(firstVersionId), any(), any()); + verify(service, never()).doInstallVersion(eq(user), eq(secondVersionId), any(), any()); + } + @Test void installPlan_cascade_installsDependencyBeforeRoot_andRoutesDataToRootOnly() throws Exception { mockTenant(); @@ -253,7 +289,7 @@ class DefaultIotHubServiceTest { InstallPlanEntry dep = willInstall(depVersionId, false); InstallPlanEntry root = willInstall(rootVersionId, true); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); IotHubInstalledItem depItem = installedItem(UUID.randomUUID()); IotHubInstalledItem rootItem = installedItem(UUID.randomUUID()); DashboardInstalledItemDescriptor rootDescriptor = new DashboardInstalledItemDescriptor(); @@ -287,7 +323,7 @@ class DefaultIotHubServiceTest { InstallPlanEntry dep2 = willInstall(dep2VersionId, false); InstallPlanEntry root = willInstall(rootVersionId, true); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); IotHubInstalledItem dep1Item = installedItem(UUID.randomUUID()); IotHubInstalledItem dep2Item = installedItem(UUID.randomUUID()); doReturn(dep1Item).when(service).doInstallVersion(eq(user), eq(dep1VersionId), any(), any()); @@ -316,7 +352,7 @@ class DefaultIotHubServiceTest { InstallPlanEntry dep = willInstall(depVersionId, false); InstallPlanEntry root = willInstall(rootVersionId, true); - when(iotHubInstalledItemService.findInstalledItemIdsByTenantId(tenantId)).thenReturn(List.of()); + when(iotHubInstalledItemService.findInstalledItemIdsByTenantIdAndItemIdIn(eq(tenantId), any())).thenReturn(List.of()); IotHubInstalledItem depItem = installedItem(UUID.randomUUID()); doReturn(depItem).when(service).doInstallVersion(eq(user), eq(depVersionId), any(), any()); doThrow(new IllegalStateException("install failed")).when(service).doInstallVersion(eq(user), eq(rootVersionId), any(), any()); diff --git a/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemDao.java b/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemDao.java index 22692f6ee4..8e20a68eb0 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemDao.java @@ -22,6 +22,7 @@ import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.dao.Dao; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; @@ -35,6 +36,8 @@ public interface IotHubInstalledItemDao extends Dao { List findInstalledItemIdsByTenantId(TenantId tenantId); + List findInstalledItemIdsByTenantIdAndItemIdIn(TenantId tenantId, Collection itemIds); + long countByTenantId(TenantId tenantId, String itemType); Map findInstalledItemCounts(TenantId tenantId, String itemType); diff --git a/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemService.java b/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemService.java index 3fa15852d8..874e9bfcc5 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemService.java @@ -21,6 +21,7 @@ import org.thingsboard.server.common.data.iot_hub.IotHubInstalledItem; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.UUID; @@ -35,6 +36,8 @@ public interface IotHubInstalledItemService { List findInstalledItemIdsByTenantId(TenantId tenantId); + List findInstalledItemIdsByTenantIdAndItemIdIn(TenantId tenantId, Collection itemIds); + long countByTenantId(TenantId tenantId, String itemType); Map findInstalledItemCounts(TenantId tenantId, String itemType); diff --git a/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemServiceImpl.java index 5bb857ca2c..d89c21f548 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/iot_hub/IotHubInstalledItemServiceImpl.java @@ -24,6 +24,7 @@ import org.thingsboard.server.common.data.iot_hub.IotHubInstalledItem; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.UUID; @@ -56,6 +57,11 @@ class IotHubInstalledItemServiceImpl implements IotHubInstalledItemService { return iotHubInstalledItemDao.findInstalledItemIdsByTenantId(tenantId); } + @Override + public List findInstalledItemIdsByTenantIdAndItemIdIn(TenantId tenantId, Collection itemIds) { + return iotHubInstalledItemDao.findInstalledItemIdsByTenantIdAndItemIdIn(tenantId, itemIds); + } + @Override public long countByTenantId(TenantId tenantId, String itemType) { return iotHubInstalledItemDao.countByTenantId(tenantId, itemType); diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/IotHubInstalledItemRepository.java b/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/IotHubInstalledItemRepository.java index 8f06beaa39..393e40ea86 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/IotHubInstalledItemRepository.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/IotHubInstalledItemRepository.java @@ -24,6 +24,7 @@ import org.springframework.data.repository.query.Param; import org.springframework.transaction.annotation.Transactional; import org.thingsboard.server.dao.model.sql.IotHubInstalledItemEntity; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.Set; @@ -36,6 +37,11 @@ interface IotHubInstalledItemRepository extends JpaRepository findInstalledItemIdsByTenantId(@Param("tenantId") UUID tenantId); + @Query("SELECT DISTINCT item.itemId FROM IotHubInstalledItemEntity item " + + "WHERE item.tenantId = :tenantId AND item.itemId IN :itemIds") + List findInstalledItemIdsByTenantIdAndItemIdIn(@Param("tenantId") UUID tenantId, + @Param("itemIds") Collection itemIds); + @Query(""" SELECT item FROM IotHubInstalledItemEntity item WHERE item.tenantId = :tenantId diff --git a/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/JpaIotHubInstalledItemDao.java b/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/JpaIotHubInstalledItemDao.java index 9d61b13209..5cd305b5ad 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/JpaIotHubInstalledItemDao.java +++ b/dao/src/main/java/org/thingsboard/server/dao/sql/iot_hub/JpaIotHubInstalledItemDao.java @@ -33,6 +33,7 @@ import org.thingsboard.server.dao.model.sql.IotHubInstalledItemEntity; import org.thingsboard.server.dao.sql.JpaAbstractDao; import org.thingsboard.server.dao.util.SqlDao; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -68,6 +69,11 @@ class JpaIotHubInstalledItemDao extends JpaAbstractDao findInstalledItemIdsByTenantIdAndItemIdIn(TenantId tenantId, Collection itemIds) { + return repository.findInstalledItemIdsByTenantIdAndItemIdIn(tenantId.getId(), itemIds); + } + @Override public long countByTenantId(TenantId tenantId, String itemType) { return repository.countByTenantId(tenantId.getId(), itemType);