Skip to content
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.

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

Allow @Cacheable method to return java.util.Optional variant of cached value [SPR-14230] #18804

Closed
spring-issuemaster opened this issue Apr 28, 2016 · 8 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Acacio Antonio Andruczewicz opened SPR-14230 and commented

class MyServiceImpl implements MyService {

    private static final String CACHE_NAME = "itemCache";      

    @Override
    @Cacheable(CACHE_NAME)
    public Optional<Item> findById(Long id) {
        // access the repository to retrieve the item
    }

    @Override
    @CachePut(cacheNames = CACHE_NAME, key = "#item.id")
    public Item insertItem(Item item) {
        ...
    }

}

In the above example, a ClassCastException is thrown because insertItem puts an Item instance in the cache, and findById expects an Optional that may contain an Item instance. It would be nice if Spring cache abstraction supported Java 8 Optional.


Affects: 4.2.5

Reference URL: http://stackoverflow.com/questions/36844270/handling-java-8-optional-with-spring-cache

Issue Links:

  • #13892 Provide means to configure multithreaded access for cache abstraction
  • #14212 Using Spring beans as a part of key in cache annotations (@Cacheable etc.) doesn't work
  • #19419 Allow @Cacheable method to return java.util.Optional variant of cached value with @Cacheable(sync=true)
  • #18913 Allow @CachePut to specify the object to store using SpEL

Referenced from: commits 240f254, fdb31cd

0 votes, 5 watchers

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Juergen Hoeller commented

We're internally always caching the actual target value now, automatically adapting to java.util.Optional declarations when receiving or returning an instance. This will be available in 4.3 RC2.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Acacio Antonio Andruczewicz commented

It will be very useful. Thank you.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 17, 2016

Martin Soubeyrand commented

Is the support of OptionalDouble, OptionalInt and OptionalLong planned?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 17, 2016

Juergen Hoeller commented

Across the framework, we generally only support java.util.Optional itself, expecting it to be used with primitive wrapper types if necessary.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 10, 2016

Patras Vlad Sebastian commented

Note this issue breaks implementations using java 8 Optional with caches that do not support null (ex. Guava cache).
Before the change an empty Optional would be stored as an empty Optional, after the change it is unwrapped to a null and the cache put will fail.

For those with this issue one solution would be to proxy the cache implementation and wrap back the null to an Optional (and vice-versa). But this is essentially a hack and it would be better if Spring could make this behavior configurable.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 10, 2016

Juergen Hoeller commented

I don't see it as a good idea to store java.util.Optional in an actual cache: Neither is it serializable nor is it meant to be a persistent representation to begin with. The general Java opinion is that Optional is only meant to be used in parameter and return type signatures.

GuavaCacheManager and other Spring cache adapters have an allowNullValues flag, even on by default. This automatically converts null user values to an internal null holder object. Have you intentionally turned this off, or are you using a custom cache adapter?

While we could support an allowNullValues variant which stores an Optional in the target cache, I don't see the benefit... I could imagine a non-Spring client to have an easier time checking for a null representation, but even then, Optional is not an ideal representation.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 10, 2016

Patras Vlad Sebastian commented

I agree Optional should not be stored in a cache and we should have implemented our proxy with a marker object like the spring is doing.
In the end we configured the cache manager to allow null values (and replace nulls with NullValue), instead of proxying the guava cache.
Thanks Juergen.

The only downside is we were setting allowNullValues to false because we wanted to enforce use of Optional in services instead of returning null. Now we do not know whether the value is NullValue because the service returned null or it was unwrapped from Optional.absent(). But enforcing this through the cache wasn't a good idea anyway.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Oct 26, 2016

Craig commented

Note that this issue still occurs if sync=true, see #19419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.