@CacheKey depending on order of properties

[BiggA94]

Describe the bug
A function that invalidates a cache with an @CacheKey annotation is only working as expected, if the parameters of the function are in the correct order.

There are two problems here that I can see.

1. The cache invalidator seems to not ignore the values without a @CacheKey annotation for the Key generation
2. The cache invalidator depends on the order of all @CacheKey parameters

Expected behavior
All invalidation functions of the TestClass in the Reproducer should actually invalidate the cache for the cached function.
This should not depend on order of parameters, neither if the are non-@CacheKey parameters nor if there are only @CacheKey parameters (or @CacheKey completely missing).

Actual behavior
testChangedOrder() and testChangedOrderWithNonCacheKey() are not working because the cache is not invalidated.

To Reproduce
I have the following Class inside my Application:

import io.quarkus.cache.CacheInvalidate;
import io.quarkus.cache.CacheKey;
import io.quarkus.cache.CacheResult;

import javax.inject.Singleton;

@Singleton
public class TestClass {
    public static int callCounter = 0;

    @CacheInvalidateAll(cacheName = "test")
    public void invalidateTestCache() {
        // noop
    }

    @CacheResult(cacheName = "test")
    public String cachedFunction(String randomId, String randomId2) {
        System.out.println("cachedFunction called");
        return randomId + randomId2 + ++callCounter;
    }

    @CacheInvalidate(cacheName = "test")
    public void calculateSomethingRandom(String randomId, String randomId2) {
        // do something randomly
        System.out.println(randomId);
        System.out.println(randomId2);
    }

    @CacheInvalidate(cacheName = "test")
    public void calculateSomeOtherRandom(@CacheKey String randomId, @CacheKey String randomId2, String param1) {
        // do something randomly
        System.out.println(param1);
        System.out.println(randomId);
        System.out.println(randomId2);
    }

    @CacheInvalidate(cacheName = "test")
    public void calculateSomeOtherRandomChangedOrder(@CacheKey String randomId2, @CacheKey String randomId, String param1) {
        // do something randomly
        System.out.println(param1);
        System.out.println(randomId);
        System.out.println(randomId2);
    }

    @CacheInvalidate(cacheName = "test")
    public void calculateSomeOtherRandomWithOtherOrderAndNonCacheKey(String param1, @CacheKey String randomId, @CacheKey String randomId2) {
        // do something randomly
        System.out.println(param1);
        System.out.println(randomId);
        System.out.println(randomId2);
    }
}

And the following test:

import io.quarkus.test.junit.QuarkusTest;
import org.junit.jupiter.api.Test;

import javax.inject.Inject;

import static org.junit.jupiter.api.Assertions.assertEquals;

@QuarkusTest
public class TempTest {

    @Inject
    public TestClass testClass;

    @BeforeEach
    public void reset() {
        TestClass.callCounter = 0;
        testClass.invalidateTestCache();
    }

    @Test
    public void testCache() {
        assertEquals("testtest21", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest21", testClass.cachedFunction("test", "test2"));

        testClass.calculateSomethingRandom("test", "test2");
        assertEquals("testtest22", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest22", testClass.cachedFunction("test", "test2"));

        testClass.calculateSomeOtherRandom("test", "test2", "randomString");
        assertEquals("testtest23", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest23", testClass.cachedFunction("test", "test2"));
    }

    @Test
    public void testChangedOrder() {
        assertEquals("testtest21", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest21", testClass.cachedFunction("test", "test2"));

        testClass.calculateSomeOtherRandomChangedOrder("test2", "test", "randomString");
        assertEquals("testtest22", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest22", testClass.cachedFunction("test", "test2"));
    }

    @Test
    public void testChangedOrderWithNonCacheKey() {
        assertEquals("testtest21", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest21", testClass.cachedFunction("test", "test2"));

        testClass.calculateSomeOtherRandomWithOtherOrderAndNonCacheKey("randomString", "test", "test2");
        assertEquals("testtest22", testClass.cachedFunction("test", "test2"));
        assertEquals("testtest22", testClass.cachedFunction("test", "test2"));
    }

}

Screenshots
(If applicable, add screenshots to help explain your problem.)

Environment (please complete the following information):

• Output of java -version:

openjdk 11.0.2 2019-01-15
OpenJDK Runtime Environment 18.9 (build 11.0.2+9)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.2+9, mixed mode)

• Quarkus version or git rev: 1.5.2.Final
• Build tool (ie. output of mvnw --version or gradlew --version):

Java version: 11.0.2, vendor: Oracle Corporation, runtime: ...
Default locale: de_DE, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

[geoand]

cc @gwenneg

[gsmet]

So I checked the code and that's a normal behavior considering how it's done. Only the parameter positions are taken into account and they are iterated in order. We don't consider the parameter names at all.

Given we consider compiling with -parameters mandatory with Quarkus, that might be doable but I'm not entirely sure it's worth the added complexity.

In any case, we should probably document it properly.

[geoand]

We should definitely document this

[gwenneg]

Hi @BiggA94 and sorry for the late answer. As explained by @gsmet, the current cache key building logic is based on the parameters positions only and not on their names. But this should not cause testChangedOrderWithNonCacheKey to fail so I suspect there's something fishy going on in the cache extension code. I'll check that right away.

[gwenneg]

As I suspected, there's a bug in the cache key building logic which should be fixed with #11046.

Thanks for opening this issue @BiggA94!