qaware / collection-cacheable-for-spring

Spring cache extension for putting a whole collection of entities as single cache items
Apache License 2.0
60 stars 9 forks source link

Did not find zero or one Collection argument. #7

Closed YAGAMIL closed 3 years ago

YAGAMIL commented 3 years ago
    @CollectionCacheable(value = CacheConstant.CACHE_ACCOUNT)
    @Override
    public Map<Long, LoginAccountDO> queryLoginAccounts() {
        return lambdaQuery().gt(LoginAccountDO::getConnectedStatus, OFFLINE).list().stream()
                .collect(Collectors.toMap(LoginAccountDO::getMpAccountId, e -> e));
    }

    @CollectionCacheable(value = CacheConstant.CACHE_ACCOUNT)
    @Override
    public Map<Long, LoginAccountDO> queryLoginByAccounts(Set<Long> idSet) {
        //noinspection unchecked
        return idSet.size() > 0 ? lambdaQuery().in(LoginAccountDO::getMpAccountId, idSet)
                .orderByDesc(LoginAccountDO::getLoginTime).list().stream()
                .collect(Collectors.toMap(LoginAccountDO::getMpAccountId, e -> e)) :
                Collections.emptyMap();
    }

    @Cacheable(value = CacheConstant.CACHE_ACCOUNT, key = "#mpAccountId", unless = "#result == null ")
    @Override
    public Optional<LoginAccountDO> queryLoginByAccount(Long mpAccountId) {
        return lambdaQuery().eq(LoginAccountDO::getMpAccountId, mpAccountId)
                .gt(LoginAccountDO::getConnectedStatus, OFFLINE).oneOpt();
    }

Isn't I useful, why do you report such an exception?

neiser commented 3 years ago

@YAGAMIL Can you post a fully working example which reproduces the exception? Best would be to create a minimal Spring Boot app showing only your issue. Then I can investigate.

YAGAMIL commented 3 years ago

``> @YAGAMIL Can you post a fully working example which reproduces the exception? Best would be to create a minimal Spring Boot app showing only your issue. Then I can investigate.

I have viewed your source code, I found the problem, your judgment does not match your case, the code is here

    private static void validateMethodArguments(Method method) {
        Class<?>[] parameterTypes = method.getParameterTypes();
        if (parameterTypes.length != 1 || !parameterTypes[0].equals(Collection.class)) {
            throw new IllegalStateException(String.format(
                    MESSAGE_INVALID_COLLECTION_CACHEABLE_ANNOTATION_CONFIGURATION +
                            " Did not find zero or one Collection argument.",
                    method.toString()));
        }
    }

Judgment conditions you should modify parameterTypes.length != 1 ||!Collection.class.isAssignableFrom(parameterTypes[0]) The parameters given by your case are set, but actually determine the direct judgment class Equals instead of whether to inherit from Collection.class

neiser commented 3 years ago

Aha, I see. As far as I remember, it's required that the method signature ONLY requires Collection as the type, as we need to call that method with a modified list of items in case we have cache hits. So allowing any subclasses of Collection is not going to work. See de.qaware.tools.collectioncacheableforspring.CollectionCacheableCacheInterceptor#injectCollectionArgument

The message is incorrect though, as it should read "Did not find exactly one Collection argument". I fixed that on master in cd2bf108f63178ffd9fd18c551bc45701ff63b47 (will be released once there are more relevant changes to be released)

I've also noticed that the README still used Set as an example method signature, which doesn't work. I've fixed this now as well.

I consider this issue closed then.