From 6ec9fd75cbb9d603c33a5df4c76f7470642bbea8 Mon Sep 17 00:00:00 2001 From: James Wong Date: Mon, 1 Dec 2025 21:20:32 +0800 Subject: [PATCH] Refactor ClaimDevicesServiceImpl for readability, null-safety and cache handling improvements --- .../device/ClaimDevicesServiceImpl.java | 75 +++++++++++++------ 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/device/ClaimDevicesServiceImpl.java b/application/src/main/java/org/thingsboard/server/service/device/ClaimDevicesServiceImpl.java index 099103479f..850e457d46 100644 --- a/application/src/main/java/org/thingsboard/server/service/device/ClaimDevicesServiceImpl.java +++ b/application/src/main/java/org/thingsboard/server/service/device/ClaimDevicesServiceImpl.java @@ -83,14 +83,24 @@ public class ClaimDevicesServiceImpl implements ClaimDevicesService { @Value("${security.claim.duration}") private long systemDurationMs; + /** + * Registers claiming information for a device and optionally stores it in cache or attributes. + * + * @param tenantId tenant identifier + * @param deviceId device identifier + * @param secretKey optional secret key required to claim the device + * @param durationMs custom claiming duration in milliseconds; if non-positive, system default is used + * @return future completed once claiming info is stored or failed if claiming is not allowed + */ + @Override public ListenableFuture registerClaimingInfo(TenantId tenantId, DeviceId deviceId, String secretKey, long durationMs) { Device device = deviceService.findDeviceById(tenantId, deviceId); - Cache cache = cacheManager.getCache(CLAIM_DEVICES_CACHE); + Cache cache = getClaimDevicesCache(); List key = constructCacheKey(device.getId()); String deviceName = device.getName(); if (isAllowedClaimingByDefault) { - if (device.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) { + if (isDeviceUnassigned(device)) { persistInCache(secretKey, durationMs, cache, key); return Futures.immediateFuture(null); } @@ -102,7 +112,7 @@ public class ClaimDevicesServiceImpl implements ClaimDevicesService { if (list != null && !list.isEmpty()) { Optional claimingAllowedOptional = list.get(0).getBooleanValue(); if (claimingAllowedOptional.isPresent() && claimingAllowedOptional.get() - && device.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) { + && isDeviceUnassigned(device)) { persistInCache(secretKey, durationMs, cache, key); return null; } @@ -117,36 +127,40 @@ public class ClaimDevicesServiceImpl implements ClaimDevicesService { ClaimData claimDataFromCache = cache.get(key, ClaimData.class); if (claimDataFromCache != null) { return Futures.immediateFuture(new ClaimDataInfo(true, key, claimDataFromCache)); - } else { - ListenableFuture> claimDataAttrFuture = attributesService.find(device.getTenantId(), device.getId(), + } + + ListenableFuture> claimDataAttrFuture = + attributesService.find(device.getTenantId(), device.getId(), AttributeScope.SERVER_SCOPE, CLAIM_DATA_ATTRIBUTE_NAME); - return Futures.transform(claimDataAttrFuture, claimDataAttr -> { - if (claimDataAttr.isPresent()) { - ClaimData claimDataFromAttribute = JacksonUtil.fromString(claimDataAttr.get().getValueAsString(), ClaimData.class); - return new ClaimDataInfo(false, key, claimDataFromAttribute); - } - return null; - }, MoreExecutors.directExecutor()); - } + return Futures.transform(claimDataAttrFuture, claimDataAttr -> { + if (claimDataAttr.isPresent()) { + ClaimData claimDataFromAttribute = JacksonUtil.fromString( + claimDataAttr.get().getValueAsString(), ClaimData.class); + return new ClaimDataInfo(false, key, claimDataFromAttribute); + } + // No claim data present + return null; + }, MoreExecutors.directExecutor()); } + @Override public ListenableFuture claimDevice(Device device, CustomerId customerId, String secretKey) { - Cache cache = cacheManager.getCache(CLAIM_DEVICES_CACHE); + Cache cache = getClaimDevicesCache(); ListenableFuture claimDataFuture = getClaimData(cache, device); return Futures.transformAsync(claimDataFuture, claimData -> { if (claimData != null) { long currTs = System.currentTimeMillis(); - if (currTs > claimData.getData().getExpirationTime() || !secretKeyIsEmptyOrEqual(secretKey, claimData.getData().getSecretKey())) { + if (currTs > claimData.getData().getExpirationTime() || !isSecretKeyValid(secretKey, claimData.getData().getSecretKey())) { log.warn("The claiming timeout occurred or wrong 'secretKey' provided for the device [{}]", device.getName()); if (claimData.isFromCache()) { cache.evict(claimData.getKey()); } return Futures.immediateFuture(new ClaimResult(null, ClaimResponse.FAILURE)); } else { - if (device.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) { + if (isDeviceUnassigned(device)) { device.setCustomerId(customerId); Device savedDevice = deviceService.saveDevice(device); return Futures.transform(removeClaimingSavedData(cache, claimData, device), result -> new ClaimResult(savedDevice, ClaimResponse.SUCCESS), MoreExecutors.directExecutor()); @@ -155,7 +169,7 @@ public class ClaimDevicesServiceImpl implements ClaimDevicesService { } } else { log.warn("Failed to find the device's claiming message![{}]", device.getName()); - if (device.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) { + if (isDeviceUnassigned(device)) { return Futures.immediateFuture(new ClaimResult(null, ClaimResponse.FAILURE)); } else { return Futures.immediateFuture(new ClaimResult(null, ClaimResponse.CLAIMED)); @@ -164,13 +178,19 @@ public class ClaimDevicesServiceImpl implements ClaimDevicesService { }, MoreExecutors.directExecutor()); } - private boolean secretKeyIsEmptyOrEqual(String secretKeyA, String secretKeyB) { - return (StringUtils.isEmpty(secretKeyA) && StringUtils.isEmpty(secretKeyB)) || secretKeyA.equals(secretKeyB); + private boolean isSecretKeyValid(String providedSecretKey, String storedSecretKey) { + // Both empty or null → treat as valid + if (StringUtils.isEmpty(providedSecretKey) && StringUtils.isEmpty(storedSecretKey)) { + return true; + } + // Safe equality check + return java.util.Objects.equals(providedSecretKey, storedSecretKey); } + @Override public ListenableFuture reClaimDevice(TenantId tenantId, Device device) { - if (!device.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) { + if (isDeviceUnassigned(device)) { cacheEviction(device.getId()); Customer unassignedCustomer = customerService.findCustomerById(tenantId, device.getCustomerId()); device.setCustomerId(null); @@ -235,8 +255,21 @@ public class ClaimDevicesServiceImpl implements ClaimDevicesService { } private void cacheEviction(DeviceId deviceId) { - Cache cache = cacheManager.getCache(CLAIM_DEVICES_CACHE); + Cache cache = getClaimDevicesCache(); cache.evict(constructCacheKey(deviceId)); } + private boolean isDeviceUnassigned(Device device) { + return device.getCustomerId() != null + && device.getCustomerId().getId().equals(ModelConstants.NULL_UUID); + } + + private Cache getClaimDevicesCache() { + Cache cache = cacheManager.getCache(CLAIM_DEVICES_CACHE); + if (cache == null) { + throw new IllegalStateException("Claim devices cache is not configured"); + } + return cache; + } + }