jsr107 / jsr107spec

JSR107 Cache Specification
Apache License 2.0
413 stars 164 forks source link

Scala and required CDI dependency #378

Open ben-manes opened 7 years ago

ben-manes commented 7 years ago

Unlike Java, Scala fully resolves all annotations and requires them to be on the classpath when loading a class. This means that Scala JCache users in a non-CDI environment, such as Guice, must include the provided scoped dependency. The pom.xml states that this dependency could be removed with CDI 1.1, which was released 4 years ago. This quirk of Scala surprised a user and, if the comment is accurate, it would be nice to remove the requirement.

<!--This is only needed if you are using a CDI based implementation of the annotations support.
    In CDI 1.1. we should be able to remove this dependency completely. -->
<dependency>
  <groupId>javax.enterprise</groupId>
  <artifactId>cdi-api</artifactId>
  <version>1.0-SP4</version>
  <scope>provided</scope>
  <optional>true</optional>
</dependency>
cruftex commented 7 years ago

I did a quick check. The dependency provides the javax.enterprise.util.Nonbinding annotation, which is used in the following JCache annotations: CacheRemove, CacheResult, CacheRemoveAll, CacheDefaults, CachePut, CacheRemove.

For example:

public @interface CacheDefaults {

  /**
   * The default name of the cache for the annotated class
   * <p>
   * If not specified defaults to:
   * package.name.ClassName.methodName(package.ParameterType,package.ParameterType)
   * <p>
   * Applicable for {@link CacheResult}, {@link CachePut}, {@link CacheRemove},
   * and {@link CacheRemoveAll}
   */
  @Nonbinding String cacheName() default "";
  . . . 

I don't think we can drop @Nonbinding to get rid of the dependency, especially in an MR which needs to be 100% API compatible.

ben-manes commented 7 years ago

Yes, that is the annotation causing the required dependency. I am unfamiliar with CDI, so to me the comment implied that the annotation may not be necessary anymore. However, we would need someone who uses CDI to clarify.

cruftex commented 7 years ago

The description is here: http://docs.jboss.org/cdi/spec/1.2/cdi-spec.html#performing_typesafe_resolution

I am unfamiliar with CDI, so to me comment implied that the annotation may not be necessary anymore. However, we would need someone who uses CDI to clarify.

Me too. However, it obviously has some intended semantic. We cannot change here any more after the JCache Spec was released.

cruftex commented 7 years ago

Tagged with 2.0 for now. Any comments whether we can rid of the dependency, at least for Version 2, are much appreciated.

ben-manes commented 7 years ago

I think the comment was alluding to adding bean-discovery-mode, set to none in the jar's beans.xml.

cruftex commented 5 years ago

Possible other solution:

Split the API jar into two jars and separate the annotations. I just tagged this with milestone 1.2, maybe its worth to talk about another MR.