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

WFLY-11575 DefaultCacheContainer getCache(...) createIfAbsent variants throw NPE... #11988

Merged
merged 1 commit into from Jan 15, 2019

Conversation

pferraro
Copy link
Contributor

@pferraro pferraro commented Jan 8, 2019

…when called with false and cache does not exist

https://issues.jboss.org/browse/WFLY-11575

Supersedes #11985

…s throw NPE when called with false and cache does not exist
}

@Deprecated
@Override
public <K, V> Cache<K, V> getCache(String cacheName, String configurationTemplate, boolean createIfAbsent) {
return this.wrap(this.cm.<K, V>getCache(cacheName, configurationTemplate, createIfAbsent));
Cache<K, V> cache = this.cm.<K, V>getCache(cacheName, configurationTemplate, createIfAbsent);
return (cache != null) ? this.wrap(cache) : null;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to check this in the "wrap" method?
With this approach you are assuming implicit behaviour on the delegate manager.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCache(...) can only return null in the createIfAbsent variants.

Copy link

@blaghed blaghed Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But... aren't you inferring that knowledge by assuming how "cm" works?
Well, its your project, and this does fix the issue for me, so I'll not argue 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But... aren't you inferring that knowledge by assuming how "cm" works?

This behavior is explicitly defined in the EmbeddedCacheManager contract. As an implementor (via decoration) of that contract, this knowledge is to be expected.

@kabir kabir merged commit 60a657c into wildfly:master Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants