CacheManager.getProperties() exact Specification and Tests

cruftex commented 8 years ago

From: https://github.com/jsr107/jsr107tck/issues/102

A cache manager has properties. The relevant passages from the specification:

CacheManager

  /**
   * Get the {@link Properties} that were used to create this
   * {@link CacheManager}.
   * <p>
   * Implementations are not required to re-configure the
   * {@link CacheManager} should modifications to the returned
   * {@link Properties} be made.
   *
   * @return the Properties used to create the {@link CacheManager}
   */
  Properties getProperties();

CachingProvider

  /**
   * Requests a {@link CacheManager} configured according to the implementation
   * specific {@link URI} be made available that uses the provided
   * {@link ClassLoader} for loading underlying classes.
   * <p>
   * Multiple calls to this method with the same {@link URI} and
   * {@link ClassLoader} must return the same {@link CacheManager} instance,
   * except if a previously returned {@link CacheManager} has been closed.
   * <p>
   * Properties are used in construction of a {@link CacheManager} and do not form
   * part of the identity of the CacheManager. i.e. if a second call is made to
   * with the same {@link URI} and {@link ClassLoader} but different properties,
   * the {@link CacheManager} created in the first call is returned.
   *
   * @param uri         an implementation specific URI for the
   *                    {@link CacheManager} (null means use
   *                    {@link #getDefaultURI()})
   * @param classLoader the {@link ClassLoader}  to use for the
   *                    {@link CacheManager} (null means use
   *                    {@link #getDefaultClassLoader()})
   * @param properties  the {@link Properties} for the {@link CachingProvider}
   *                    to create the {@link CacheManager} (null means no
   *                    implementation specific Properties are required)
   * @throws CacheException    when a {@link CacheManager} for the
   *                           specified arguments could not be produced
   * @throws SecurityException when the operation could not be performed
   *                           due to the current security settings
   */
  CacheManager getCacheManager(URI uri, ClassLoader classLoader,
                               Properties properties);

  /**
   * Obtains the default {@link Properties} for the {@link CachingProvider}.
   * <p>
   * Use this method to obtain suitable {@link Properties} for the
   * {@link CachingProvider}.
   *
   * @return the default {@link Properties} for the {@link CachingProvider}
   */
  Properties getDefaultProperties();

Current State

No implementation actually uses the content in the properties for its configuration.

Applications can set arbitrary property values and that get passed through transparently by the caching provider.

The RICachingManager always constructs a new Properties instance and takes over the specified properties at CachingProvider level as default properties:

this.properties = properties == null ? new Properties() : new Properties(properties);

Hazelcast 3.4.2 and Coherence V12.1.3 do the same. Others simple use the identical property instance.

=> The current behavior is not really specified. Implementations are not consistent.

Possible Clarifications

a) Properties are only interpreted by the implementation. Everything non-null is implementation specific. The behavior of mutating properties obtained from CacheManager.getProperties() is undefined.

b) Property values are passed through transparently. Applications can read and write properties in the instance obtained by CacheManager.getProperties(). This has only visible effect within one cache manager, meaning, it must be ensured that there is one properties object per cache manager.

gregrluck commented 7 years ago

The intention was definitely option a) which I have clarified in the CachingProvider Javadoc.

vbekiaris commented 7 years ago

In current javadoc of CachingProvider.getCacheManager(URI, ClassLoader, Properties) it is mentioned that:

Properties are caching implementation specific.

Javadoc of CacheManager.getProperties reads:

Get the {@link Properties} that were used to create this {@link CacheManager}.

According to the discussion in https://github.com/jsr107/jsr107tck/issues/102#issuecomment-267604441 (regarding TCK test CachingTest.getCacheManager_nonNullProperties ):

The Spec doesn't say that properties have to be preserved and how properties are interpreted is implementation specific. A legal implementation may refuse operation if there are any properties. If there is the assumption that properties are preserved and an implementation ignores properties which are not in its namespace, then we need to put that in the Spec first. In jsr107/jsr107spec#368 Greg opted for a. That means everything non null is implementation specific, so we cannot test anything here.

It seems that further clarification is required to indicate properties must be preserved, otherwise the related TCK test does not make sense.

jsr107 / jsr107spec