From 1720803eba5d1cf3f53926e9484f9f015cba5993 Mon Sep 17 00:00:00 2001 From: Andrii Landiak Date: Tue, 16 Jul 2024 12:46:05 +0300 Subject: [PATCH] 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