From 9fc6929a77374bbf9a4d9c7f65c1e1b52843269a Mon Sep 17 00:00:00 2001 From: Andrii Landiak Date: Mon, 15 Jul 2024 15:44:34 +0300 Subject: [PATCH 1/3] Use cacheEnabled to disable caching in case cache.specs field 'maxSize' is 0 --- .../server/cache/CacheSpecsMap.java | 2 +- .../cache/RedisTbTransactionalCache.java | 29 ++++++- .../server/dao/service/AlarmServiceTest.java | 87 +++++++++---------- 3 files changed, 70 insertions(+), 48 deletions(-) diff --git a/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java b/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java index 583a871fe1..abca4624e6 100644 --- a/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java +++ b/common/cache/src/main/java/org/thingsboard/server/cache/CacheSpecsMap.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.cache; +import jakarta.annotation.PostConstruct; import lombok.Data; import lombok.Getter; import org.springframework.beans.factory.annotation.Value; @@ -22,7 +23,6 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; import org.thingsboard.server.common.data.CacheConstants; -import jakarta.annotation.PostConstruct; import java.util.Map; @Configuration diff --git a/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java b/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java index abfbb398c9..c130318b8b 100644 --- a/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java +++ b/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java @@ -29,7 +29,6 @@ import org.springframework.data.redis.core.types.Expiration; import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.thingsboard.server.common.data.FstStatsService; -import redis.clients.jedis.Connection; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; import redis.clients.jedis.util.JedisClusterCRC16; @@ -57,6 +56,7 @@ public abstract class RedisTbTransactionalCache valueSerializer; private final Expiration evictExpiration; private final Expiration cacheTtl; + private final boolean cacheEnabled; public RedisTbTransactionalCache(String cacheName, CacheSpecsMap cacheSpecsMap, @@ -73,10 +73,19 @@ public abstract class RedisTbTransactionalCache Expiration.from(t, TimeUnit.MINUTES)) .orElseGet(Expiration::persistent); + this.cacheEnabled = Optional.ofNullable(cacheSpecsMap) + .map(CacheSpecsMap::getSpecs) + .map(x -> x.get(cacheName)) + .map(CacheSpecs::getMaxSize) + .map(size -> size > 0) + .orElse(false); } @Override public TbCacheValueWrapper get(K key) { + if (!cacheEnabled) { + return null; + } try (var connection = connectionFactory.getConnection()) { byte[] rawKey = getRawKey(key); byte[] rawValue = connection.get(rawKey); @@ -98,6 +107,9 @@ public abstract class RedisTbTransactionalCache keys) { + if (!cacheEnabled) { + return; + } //Redis expects at least 1 key to delete. Otherwise - ERR wrong number of arguments for 'del' command if (keys.isEmpty()) { return; @@ -130,6 +151,9 @@ public abstract class RedisTbTransactionalCache alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(childId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -257,8 +256,8 @@ public class AlarmServiceTest extends AbstractServiceTest { // Check parent relation alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(parentId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -272,8 +271,8 @@ public class AlarmServiceTest extends AbstractServiceTest { // Check child relation alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(childId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -284,8 +283,8 @@ public class AlarmServiceTest extends AbstractServiceTest { // Check parent relation alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(parentId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -298,8 +297,8 @@ public class AlarmServiceTest extends AbstractServiceTest { alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(childId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.ACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.ACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -310,8 +309,8 @@ public class AlarmServiceTest extends AbstractServiceTest { // Check not existing relation alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(childId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.UNACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -323,8 +322,8 @@ public class AlarmServiceTest extends AbstractServiceTest { alarms = alarmService.findAlarmsV2(tenantId, AlarmQueryV2.builder() .affectedEntityId(childId) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL)) - .statusList(Arrays.asList(AlarmSearchStatus.CLEARED, AlarmSearchStatus.ACK)).pageLink( + .severityList(List.of(AlarmSeverity.CRITICAL)) + .statusList(List.of(AlarmSearchStatus.CLEARED, AlarmSearchStatus.ACK)).pageLink( new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -334,7 +333,7 @@ public class AlarmServiceTest extends AbstractServiceTest { } @Test - public void testFindAssignedAlarm() throws ExecutionException, InterruptedException { + public void testFindAssignedAlarm() { AssetId parentId = new AssetId(Uuids.timeBased()); AssetId childId = new AssetId(Uuids.timeBased()); @@ -368,7 +367,6 @@ public class AlarmServiceTest extends AbstractServiceTest { PageData alarms = alarmService.findAlarms(tenantId, AlarmQuery.builder() .assigneeId(tenantUser.getId()) - .fetchOriginator(true) .pageLink(new TimePageLink(1, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) ).build()); @@ -405,7 +403,7 @@ public class AlarmServiceTest extends AbstractServiceTest { } @Test - public void testFindCustomerAlarm() throws ExecutionException, InterruptedException { + public void testFindCustomerAlarm() { Customer customer = new Customer(); customer.setTitle("TestCustomer"); customer.setTenantId(tenantId); @@ -451,10 +449,10 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); - PageData tenantAlarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Arrays.asList(tenantDevice.getId(), customerDevice.getId())); + PageData tenantAlarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), List.of(tenantDevice.getId(), customerDevice.getId())); Assert.assertEquals(2, tenantAlarms.getData().size()); PageData customerAlarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(customerDevice.getId())); @@ -473,7 +471,7 @@ public class AlarmServiceTest extends AbstractServiceTest { } @Test - public void testFindPropagatedCustomerAssetAlarm() throws ExecutionException, InterruptedException { + public void testFindPropagatedCustomerAssetAlarm() { Customer customer = new Customer(); customer.setTitle("TestCustomer"); customer.setTenantId(tenantId); @@ -525,8 +523,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); //TEST that propagated alarms are visible on the asset level. PageData customerAlarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(customerAsset.getId())); @@ -576,7 +574,7 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); pageLink.setStatusList(Collections.singletonList(AlarmSearchStatus.ACTIVE)); //TEST that propagated alarms are visible on the asset level. @@ -599,7 +597,7 @@ public class AlarmServiceTest extends AbstractServiceTest { } @Test - public void testFindHighestAlarmSeverity() throws ExecutionException, InterruptedException { + public void testFindHighestAlarmSeverity() { Customer customer = new Customer(); customer.setTitle("TestCustomer"); customer.setTenantId(tenantId); @@ -679,8 +677,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(false); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); PageData alarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(childId)); @@ -695,8 +693,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(false); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); alarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(childId)); Assert.assertNotNull(alarms.getData()); @@ -722,8 +720,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); alarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(childId)); Assert.assertNotNull(alarms.getData()); @@ -738,8 +736,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); alarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(parentId)); Assert.assertNotNull(alarms.getData()); @@ -748,7 +746,6 @@ public class AlarmServiceTest extends AbstractServiceTest { PageData alarmsInfoData = alarmService.findAlarms(tenantId, AlarmQuery.builder() .affectedEntityId(childId) - .fetchOriginator(true) .status(AlarmStatus.ACTIVE_UNACK).pageLink( new TimePageLink(10, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) @@ -759,7 +756,6 @@ public class AlarmServiceTest extends AbstractServiceTest { alarmsInfoData = alarmService.findAlarms(tenantId, AlarmQuery.builder() .affectedEntityId(parentId) - .fetchOriginator(true) .status(AlarmStatus.ACTIVE_UNACK).pageLink( new TimePageLink(10, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) @@ -770,7 +766,6 @@ public class AlarmServiceTest extends AbstractServiceTest { alarmsInfoData = alarmService.findAlarms(tenantId, AlarmQuery.builder() .affectedEntityId(parentId2) - .fetchOriginator(true) .status(AlarmStatus.ACTIVE_UNACK).pageLink( new TimePageLink(10, 0, "", new SortOrder("createdTime", SortOrder.Direction.DESC), 0L, System.currentTimeMillis()) @@ -786,8 +781,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); alarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(parentId)); Assert.assertNotNull(alarms.getData()); @@ -803,8 +798,8 @@ public class AlarmServiceTest extends AbstractServiceTest { pageLink.setStartTs(0L); pageLink.setEndTs(System.currentTimeMillis()); pageLink.setSearchPropagatedAlarms(true); - pageLink.setSeverityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); - pageLink.setStatusList(Arrays.asList(AlarmSearchStatus.ACTIVE)); + pageLink.setSeverityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)); + pageLink.setStatusList(List.of(AlarmSearchStatus.ACTIVE)); alarms = alarmService.findAlarmDataByQueryForEntities(tenantId, toQuery(pageLink), Collections.singletonList(childId)); Assert.assertNotNull(alarms.getData()); @@ -813,7 +808,7 @@ public class AlarmServiceTest extends AbstractServiceTest { } @Test - public void testCountAlarmsUsingAlarmDataQuery() throws ExecutionException, InterruptedException { + public void testCountAlarmsUsingAlarmDataQuery() { AssetId childId = new AssetId(Uuids.timeBased()); long ts = System.currentTimeMillis(); @@ -829,7 +824,7 @@ public class AlarmServiceTest extends AbstractServiceTest { .startTs(0L) .endTs(System.currentTimeMillis()) .searchPropagatedAlarms(false) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)) + .severityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)) .statusList(List.of(AlarmSearchStatus.ACTIVE)) .build(); @@ -841,7 +836,7 @@ public class AlarmServiceTest extends AbstractServiceTest { .startTs(0L) .endTs(System.currentTimeMillis()) .searchPropagatedAlarms(true) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)) + .severityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)) .statusList(List.of(AlarmSearchStatus.ACTIVE)) .build(); @@ -866,7 +861,7 @@ public class AlarmServiceTest extends AbstractServiceTest { .startTs(0L) .endTs(System.currentTimeMillis()) .searchPropagatedAlarms(true) - .severityList(Arrays.asList(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)) + .severityList(List.of(AlarmSeverity.CRITICAL, AlarmSeverity.WARNING)) .statusList(List.of(AlarmSearchStatus.ACTIVE, AlarmSearchStatus.CLEARED)) .build(); @@ -939,6 +934,6 @@ public class AlarmServiceTest extends AbstractServiceTest { ).build()); Assert.assertNotNull(alarms.getData()); Assert.assertEquals(0, alarms.getData().size()); - } + } From 1720803eba5d1cf3f53926e9484f9f015cba5993 Mon Sep 17 00:00:00 2001 From: Andrii Landiak Date: Tue, 16 Jul 2024 12:46:05 +0300 Subject: [PATCH 2/3] Improvement after review: reuse getAndPutInTransaction, write test --- .../cache/RedisTbTransactionalCache.java | 10 +++ .../server/cache/TbTransactionalCache.java | 34 ++----- .../server/cache/CacheSpecsMapTest.java | 3 +- .../cache/RedisTbTransactionalCacheTest.java | 89 +++++++++++++++++++ 4 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java diff --git a/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java b/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java index c130318b8b..8e7bc86d53 100644 --- a/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java +++ b/common/cache/src/main/java/org/thingsboard/server/cache/RedisTbTransactionalCache.java @@ -39,6 +39,8 @@ import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import java.util.function.Supplier; @Slf4j public abstract class RedisTbTransactionalCache implements TbTransactionalCache { @@ -177,6 +179,14 @@ public abstract class RedisTbTransactionalCache(this, connection); } + @Override + public R getAndPutInTransaction(K key, Supplier dbCall, Function cacheValueToResult, Function dbValueToCacheValue, boolean cacheNullValue) { + if (!cacheEnabled) { + return dbCall.get(); + } + return TbTransactionalCache.super.getAndPutInTransaction(key, dbCall, cacheValueToResult, dbValueToCacheValue, cacheNullValue); + } + private RedisConnection getConnection(byte[] rawKey) { if (!connectionFactory.isRedisClusterAware()) { return connectionFactory.getConnection(); diff --git a/common/cache/src/main/java/org/thingsboard/server/cache/TbTransactionalCache.java b/common/cache/src/main/java/org/thingsboard/server/cache/TbTransactionalCache.java index be0b38b65e..4d57736cc1 100644 --- a/common/cache/src/main/java/org/thingsboard/server/cache/TbTransactionalCache.java +++ b/common/cache/src/main/java/org/thingsboard/server/cache/TbTransactionalCache.java @@ -60,15 +60,20 @@ public interface TbTransactionalCache dbCall, boolean cacheNullValue) { + return getAndPutInTransaction(key, dbCall, Function.identity(), Function.identity(), cacheNullValue); + } + + default R getAndPutInTransaction(K key, Supplier dbCall, Function cacheValueToResult, Function dbValueToCacheValue, boolean cacheNullValue) { TbCacheValueWrapper cacheValueWrapper = get(key); if (cacheValueWrapper != null) { - return cacheValueWrapper.get(); + V cacheValue = cacheValueWrapper.get(); + return cacheValue != null ? cacheValueToResult.apply(cacheValue) : null; } var cacheTransaction = newTransactionForKey(key); try { - V dbValue = dbCall.get(); + R dbValue = dbCall.get(); if (dbValue != null || cacheNullValue) { - cacheTransaction.putIfAbsent(key, dbValue); + cacheTransaction.putIfAbsent(key, dbValueToCacheValue.apply(dbValue)); cacheTransaction.commit(); return dbValue; } else { @@ -94,27 +99,4 @@ public interface TbTransactionalCache R getAndPutInTransaction(K key, Supplier dbCall, Function cacheValueToResult, Function dbValueToCacheValue, boolean cacheNullValue) { - TbCacheValueWrapper cacheValueWrapper = get(key); - if (cacheValueWrapper != null) { - var cacheValue = cacheValueWrapper.get(); - return cacheValue == null ? null : cacheValueToResult.apply(cacheValue); - } - var cacheTransaction = newTransactionForKey(key); - try { - R dbValue = dbCall.get(); - if (dbValue != null || cacheNullValue) { - cacheTransaction.putIfAbsent(key, dbValueToCacheValue.apply(dbValue)); - cacheTransaction.commit(); - return dbValue; - } else { - cacheTransaction.rollback(); - return null; - } - } catch (Throwable e) { - cacheTransaction.rollback(); - throw e; - } - } - } diff --git a/common/cache/src/test/java/org/thingsboard/server/cache/CacheSpecsMapTest.java b/common/cache/src/test/java/org/thingsboard/server/cache/CacheSpecsMapTest.java index aec5e01f39..eb83cc2821 100644 --- a/common/cache/src/test/java/org/thingsboard/server/cache/CacheSpecsMapTest.java +++ b/common/cache/src/test/java/org/thingsboard/server/cache/CacheSpecsMapTest.java @@ -62,4 +62,5 @@ public class CacheSpecsMapTest { public void givenCacheConfig_whenCacheManagerReady_thenVerifyNonExistedCaches() { assertThat(cacheManager.getCache("rainbows_and_unicorns")).isNull(); } -} \ No newline at end of file + +} diff --git a/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java b/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java new file mode 100644 index 0000000000..914472098c --- /dev/null +++ b/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java @@ -0,0 +1,89 @@ +/** + * Copyright © 2016-2024 The Thingsboard Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.dao.cache; + +import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.data.redis.connection.RedisConnection; +import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.thingsboard.server.cache.CacheSpecsMap; +import org.thingsboard.server.cache.RedisSslCredentials; +import org.thingsboard.server.cache.TBRedisCacheConfiguration; +import org.thingsboard.server.common.data.id.DeviceId; +import org.thingsboard.server.common.data.relation.RelationTypeGroup; +import org.thingsboard.server.dao.relation.RelationCacheKey; +import org.thingsboard.server.dao.relation.RelationRedisCache; + +import java.util.List; +import java.util.UUID; + +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {RelationRedisCache.class, CacheSpecsMap.class, TBRedisCacheConfiguration.class}) +@TestPropertySource(properties = { + "cache.type=redis", + "cache.specs.relations.timeToLiveInMinutes=1440", + "cache.specs.relations.maxSize=0", +}) +@Slf4j +public class RedisTbTransactionalCacheTest { + + @MockBean + private RelationRedisCache relationRedisCache; + @MockBean + private RedisConnectionFactory connectionFactory; + @MockBean + private RedisConnection redisConnection; + @MockBean + private RedisSslCredentials redisSslCredentials; + + + @BeforeEach + public void setup() { + when(connectionFactory.getConnection()).thenReturn(redisConnection); + } + + @Test + public void testEvictNoOpWhenCacheDisabled() { + relationRedisCache.evict(List.of(createRelationCacheKey())); + + verify(connectionFactory, never()).getConnection(); + verifyNoInteractions(redisConnection); + } + + @Test + public void testEvictOrPutNoOpWhenCacheDisabled() { + relationRedisCache.evictOrPut(createRelationCacheKey(), null); + + verify(connectionFactory, never()).getConnection(); + verifyNoInteractions(redisConnection); + } + + private RelationCacheKey createRelationCacheKey() { + return new RelationCacheKey(new DeviceId(UUID.randomUUID()), new DeviceId(UUID.randomUUID()), null, RelationTypeGroup.COMMON); + } + +} \ No newline at end of file From fca1eef2c1871c6c084e1d6ede3c2dc4843af3f8 Mon Sep 17 00:00:00 2001 From: Andrii Landiak Date: Tue, 16 Jul 2024 14:54:12 +0300 Subject: [PATCH 3/3] Improve RedisTbTransactionalCacheTest --- .../cache/RedisTbTransactionalCacheTest.java | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java b/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java index 914472098c..12b6f5d7d1 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/cache/RedisTbTransactionalCacheTest.java @@ -16,7 +16,6 @@ package org.thingsboard.server.dao.cache; import lombok.extern.slf4j.Slf4j; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.boot.test.mock.mockito.MockBean; @@ -60,23 +59,17 @@ public class RedisTbTransactionalCacheTest { @MockBean private RedisSslCredentials redisSslCredentials; - - @BeforeEach - public void setup() { + @Test + public void testNoOpWhenCacheDisabled() { when(connectionFactory.getConnection()).thenReturn(redisConnection); - } - @Test - public void testEvictNoOpWhenCacheDisabled() { + relationRedisCache.put(createRelationCacheKey(), null); + relationRedisCache.putIfAbsent(createRelationCacheKey(), null); + relationRedisCache.evict(createRelationCacheKey()); relationRedisCache.evict(List.of(createRelationCacheKey())); - - verify(connectionFactory, never()).getConnection(); - verifyNoInteractions(redisConnection); - } - - @Test - public void testEvictOrPutNoOpWhenCacheDisabled() { - relationRedisCache.evictOrPut(createRelationCacheKey(), null); + relationRedisCache.getAndPutInTransaction(createRelationCacheKey(), null, false); + relationRedisCache.getAndPutInTransaction(createRelationCacheKey(), null, null, null, false); + relationRedisCache.getOrFetchFromDB(createRelationCacheKey(), null, false, false); verify(connectionFactory, never()).getConnection(); verifyNoInteractions(redisConnection); @@ -86,4 +79,4 @@ public class RedisTbTransactionalCacheTest { return new RelationCacheKey(new DeviceId(UUID.randomUUID()), new DeviceId(UUID.randomUUID()), null, RelationTypeGroup.COMMON); } -} \ No newline at end of file +}