Skip to content

Fixed check method args check for @Cacheable/@CachePut/@CacheInvalidate #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GoodforGod wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed check method args check for @Cacheable/@CachePut/@CacheInvalidate #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions ...ocessor/src/main/java/ru/tinkoff/kora/cache/annotation/processor/CacheOperationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,22 @@ private static CacheOperation getOperation(ExecutableElement method,
for (String parameter : parameters) {
if (method.getParameters().stream().noneMatch(p -> p.getSimpleName().contentEquals(parameter))) {
throw new ProcessingErrorException(new ProcessingError(Diagnostic.Kind.ERROR,
"Unknown method parameter is declared: " + parameter, method));
"Unknown method cache parameter is declared: " + parameter, method));
}
}
}

if (parameters.isEmpty() && type != CacheOperation.Type.EVICT_ALL) {
throw new ProcessingErrorException(new ProcessingError(Diagnostic.Kind.ERROR,
"@%s method must declare any method parameters for cache key"
.formatted(annotation.getAnnotationType().asElement().getSimpleName().toString()), method));
}

for (List<String> arguments : cacheKeyArguments) {
if (!arguments.equals(parameters)) {
throw new ProcessingErrorException(new ProcessingError(Diagnostic.Kind.ERROR,
annotation.getClass() + " parameters mismatch for different annotations for: " + origin, method));
"@%s parameters mismatch for different annotations for: %s"
.formatted(annotation.getAnnotationType().asElement().getSimpleName().toString(), origin), method));
}
}

Expand Down Expand Up @@ -178,13 +185,13 @@ private static CacheOperation getOperation(ExecutableElement method,
} else {
if (parameters.size() > 9) {
throw new ProcessingErrorException("@%s doesn't support more than 9 method arguments for Cache Key"
.formatted(annotation.getAnnotationType().asElement().getSimpleName()), method);
.formatted(annotation.getAnnotationType().asElement().getSimpleName().toString()), method);
}

if(parameters.isEmpty() && (type == CacheOperation.Type.GET || type == CacheOperation.Type.EVICT)) {
if (parameters.isEmpty()) {
throw new ProcessingErrorException(
"@%s requires minimum 1 Cache Key method argument, but got 0".formatted(annotation.getAnnotationType().asElement().getSimpleName().toString()),
method);
"@%s requires minimum 1 Cache Key method argument, but got 0"
.formatted(annotation.getAnnotationType().asElement().getSimpleName().toString()), method);
}

var mapperType = getKeyMapper(cacheKeyMirror, parameterResult, env);
Expand Down
1 change: 1 addition & 0 deletions .../src/main/java/ru/tinkoff/kora/cache/annotation/processor/aop/CacheableAopKoraAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public ApplyResult apply(ExecutableElement method, String superCall, AspectConte

final CacheOperation operation = CacheOperationUtils.getCacheOperation(method, env, aspectContext);
final CodeBlock body;

if (MethodUtils.isMono(method)) {
if (MethodUtils.isMonoVoid(method)) {
throw new ProcessingErrorException("@Cacheable can't be applied for type Void", method);
Expand Down
88 changes: 86 additions & 2 deletions ...c/test/java/ru/tinkoff/kora/cache/annotation/processor/CacheAnnotationProcessorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.*;

class CacheAnnotationProcessorTests extends AbstractAnnotationProcessorTest {

Expand Down Expand Up @@ -106,4 +105,89 @@ interface MyCache extends ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, S
""");
compileResult.assertSuccess();
}

@Test
public void testCacheableNoParametersFails() {
compile(List.of(new CacheAnnotationProcessor(), new AopAnnotationProcessor()), """
public interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache extends ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>{}

class CacheableSync {

@ru.tinkoff.kora.cache.annotation.Cacheable(OuterType.MyCache.class)
public String getValue() { return "1"; }
}
}
""");
assertTrue(compileResult.isFailed());
}

@Test
public void testCacheableNoParametersNonStringFails() {
compile(List.of(new CacheAnnotationProcessor(), new AopAnnotationProcessor()), """
public interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache extends ru.tinkoff.kora.cache.caffeine.CaffeineCache<Integer, String>{}

class CacheableSync {

@ru.tinkoff.kora.cache.annotation.Cacheable(OuterType.MyCache.class)
public String getValue() { return "1"; }
}
}
""");
assertTrue(compileResult.isFailed());
}

@Test
public void testCachePutNoParametersFails() {
compile(List.of(new CacheAnnotationProcessor(), new AopAnnotationProcessor()), """
public interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache extends ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>{}

class CacheableSync {

@ru.tinkoff.kora.cache.annotation.CachePut(OuterType.MyCache.class)
public String getValue() { return "1"; }
}
}
""");
assertTrue(compileResult.isFailed());
}

@Test
public void testCacheEvictNoParametersFails() {
compile(List.of(new CacheAnnotationProcessor(), new AopAnnotationProcessor()), """
public interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache extends ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>{}

class CacheableSync {

@ru.tinkoff.kora.cache.annotation.CacheInvalidate(OuterType.MyCache.class)
public String getValue() { return "1"; }
}
}
""");
assertTrue(compileResult.isFailed());
}

@Test
public void testCacheEvictAllNoParametersSuccess() {
compile(List.of(new CacheAnnotationProcessor(), new AopAnnotationProcessor()), """
public interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache extends ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>{}

class CacheableSync {

@ru.tinkoff.kora.cache.annotation.CacheInvalidate(value = OuterType.MyCache.class, invalidateAll = true)
public String getValue() { return "1"; }
}
}
""");
compileResult.assertSuccess();
}
}
14 changes: 12 additions & 2 deletions ...l-processor/src/main/kotlin/ru/tinkoff/kora/cache/symbol/processor/CacheOperationUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import com.squareup.kotlinpoet.ksp.toTypeName
import jakarta.annotation.Nullable
import ru.tinkoff.kora.aop.symbol.processor.KoraAspect
import ru.tinkoff.kora.ksp.common.AnnotationUtils.findAnnotations
import ru.tinkoff.kora.ksp.common.FunctionUtils.isCompletionStage
import ru.tinkoff.kora.ksp.common.FunctionUtils.isFlow
import ru.tinkoff.kora.ksp.common.FunctionUtils.isFlux
import ru.tinkoff.kora.ksp.common.FunctionUtils.isCompletionStage
import ru.tinkoff.kora.ksp.common.FunctionUtils.isFuture
import ru.tinkoff.kora.ksp.common.FunctionUtils.isMono
import ru.tinkoff.kora.ksp.common.FunctionUtils.isPublisher
Expand Down Expand Up @@ -155,14 +155,24 @@ class CacheOperationUtils {
if (parameter != parameters) {
throw ProcessingErrorException(
ProcessingError(
"${annotation.javaClass} parameters mismatch for different annotations for $origin",
"@${annotation.shortName.asString()} parameters mismatch for different annotations for $origin",
method,
Diagnostic.Kind.ERROR
)
)
}
}

if (parameters.isEmpty() && type != CacheOperation.Type.EVICT_ALL) {
throw ProcessingErrorException(
ProcessingError(
"@${annotation.shortName.asString()} method must declare any method parameters for cache key",
method,
Diagnostic.Kind.ERROR,
)
)
}

val cacheImpl = annotation.arguments.filter { a -> a.name!!.asString() == "value" }
.map { a -> a.value as KSType }
.first()
Expand Down
94 changes: 88 additions & 6 deletions ...essor/src/test/kotlin/ru/tinkoff/kora/cache/symbol/processor/CacheSymbolProcessorTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ru.tinkoff.kora.cache.symbol.processor

import org.junit.jupiter.api.Assertions.assertThrows
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import ru.tinkoff.kora.aop.symbol.processor.AopSymbolProcessorProvider
Expand Down Expand Up @@ -110,12 +111,93 @@ class CacheSymbolProcessorTests : AbstractSymbolProcessorTest() {

@Test
fun testInnerTypeCache() {
compile0("""
interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache : ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>
}
""".trimIndent()
compile0(
"""
interface OuterType {
@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache : ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>
}
""".trimIndent()
)
compileResult.assertSuccess()
}

@Test
fun testCacheableNoParametersFails() {
compile0(
"""
interface OuterType {

@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache : ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>

open class CacheableSync {

@ru.tinkoff.kora.cache.annotation.Cacheable(MyCache::class)
open fun getValue(): String = "1"
}
}
""".trimIndent()
)
assertTrue(compileResult.isFailed())
}

@Test
fun testCachePutNoParametersFails() {
compile0(
"""
interface OuterType {

@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache : ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>

open class CacheableSync {

@ru.tinkoff.kora.cache.annotation.CachePut(MyCache::class)
open fun getValue(): String = "1"
}
}
""".trimIndent()
)
assertTrue(compileResult.isFailed())
}

@Test
fun testCacheEvictNoParametersFails() {
compile0(
"""
interface OuterType {

@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache : ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>

open class CacheableSync {

@ru.tinkoff.kora.cache.annotation.CacheInvalidate(MyCache::class)
open fun getValue(): String = "1"
}
}
""".trimIndent()
)
assertTrue(compileResult.isFailed())
}

@Test
fun testCacheEvictAllNoParametersFails() {
compile0(
"""
interface OuterType {

@ru.tinkoff.kora.cache.annotation.Cache("test")
interface MyCache : ru.tinkoff.kora.cache.caffeine.CaffeineCache<String, String>

open class CacheableSync {

@ru.tinkoff.kora.cache.annotation.CacheInvalidate(value = MyCache::class, invalidateAll = true)
open fun getValue(): String = "1"
}
}
""".trimIndent()
)
compileResult.assertSuccess()
}
Expand Down