ehcache / ehcache3

Ehcache 3.x line
http://www.ehcache.org
Apache License 2.0
2.02k stars 581 forks source link

NPE when not defining a CacheLoaderWriterConfiguration #433

Closed aurbroszniowski closed 9 years ago

aurbroszniowski commented 9 years ago

Take the following code:

    final Cache<Long, String> writeBehindCache = cacheManager.createCache("writeBehindCache",
        CacheConfigurationBuilder.newCacheConfigurationBuilder()
            .add(WriteBehindConfigurationBuilder.newWriteBehindConfiguration()) // <2>
            .buildConfig(Long.class, String.class));

Here the Default CacheLoaderWriter Configuration is missing, which is a problem since we need to define a CacheLoaderWriter when enabling writebehind.

An exception is correctly thrown during the cachemanager creation, however, the exception is a NPE, while it could be a more explicit one, to tell that the config is missing the CacheLoaderWriter definition.

...
Caused by: org.ehcache.exceptions.StateTransitionException: Cache 'cache0' creation in EhcacheManager failed.
    at org.ehcache.StatusTransitioner$Transition.failed(StatusTransitioner.java:224)
    at org.ehcache.EhcacheManager.init(EhcacheManager.java:408)
    at org.ehcache.CacheManagerBuilder.build(CacheManagerBuilder.java:50)
    at com.terracotta.qa.ehcache.configuration.HeapWriteBehindProvider.init(HeapWriteBehindProvider.java:85)
    at com.terracotta.qa.framework.BaseTest.test(BaseTest.java:186)
    ... 32 more
Caused by: java.lang.IllegalStateException: Cache 'cache0' creation in EhcacheManager failed.
    at org.ehcache.EhcacheManager.createCache(EhcacheManager.java:168)
    at org.ehcache.EhcacheManager.init(EhcacheManager.java:393)
    ... 35 more
Caused by: java.lang.NullPointerException
    at org.ehcache.loaderwriter.writebehind.AbstractWriteBehindQueue.<init>(AbstractWriteBehindQueue.java:93)
    at org.ehcache.loaderwriter.writebehind.LocalHeapWriteBehindQueue.<init>(LocalHeapWriteBehindQueue.java:38)
    at org.ehcache.loaderwriter.writebehind.AggregateWriteBehindQueue$WriteBehindQueueFactory.createQueue(AggregateWriteBehindQueue.java:147)
    at org.ehcache.loaderwriter.writebehind.AggregateWriteBehindQueue.<init>(AggregateWriteBehindQueue.java:43)
    at org.ehcache.loaderwriter.writebehind.AggregateWriteBehindQueue.<init>(AggregateWriteBehindQueue.java:48)
    at org.ehcache.loaderwriter.writebehind.WriteBehindDecoratorLoaderWriter.<init>(WriteBehindDecoratorLoaderWriter.java:45)
    at org.ehcache.loaderwriter.writebehind.WriteBehindDecoratorLoaderWriterProviderFactory$1.createWriteBehindDecoratorLoaderWriter(WriteBehindDecoratorLoaderWriterProviderFactory.java:53)
    at org.ehcache.loaderwriter.writebehind.WriteBehindDecoratorLoaderWriterProviderFactory$1.createWriteBehindDecoratorLoaderWriter(WriteBehindDecoratorLoaderWriterProviderFactory.java:36)
    at org.ehcache.EhcacheManager.createNewEhcache(EhcacheManager.java:257)
    at org.ehcache.EhcacheManager.createCache(EhcacheManager.java:149)
palmanojkumar commented 9 years ago

Different ways to fix this problem are:

  1. The obvious fix is at EhcacheManager#createNewEhcache() where if WriteBehindConfiguration is found and CacheLoaderWriter is null then we can throw appropriate exception message to user instead of NullPointerException.
  2. Other approach is validate all provided service configurations at CacheConfigurationBuilder level where each ServiceConfiguration should validate its own configuration and ensure availability or correctness of required ServiceConfiguration.

For Example:

  1. ResourcePoolsBuilder should validate whether configuration for heap, offheap and disk is appropriate.
  2. WriteBehindConfiguration should ensure that CacheLoaderWriterConfiguration is available.

Proposed change:

public interface ServiceConfiguration<T extends Service> {

  /**
   * Indicates which service this configuration works with.
   *
   * @return the service type
   */
  Class<T> getServiceType();

  void validate(List<ServiceConfiguration> serviceConfigurations) throws ValidationException;
}
palmanojkumar commented 9 years ago

approach 2, will lead to unavoidable use of concrete types of ServiceConfiguration in validation hence defaulting to approach#1 for implementation.