HHH-12146 is about being able to enable/disable caching at various levels in an entity hierarchy. E.g., given a hierarchy such as `Person` and `Company` both extending `LegalEntity`, this would allow users to say that only `Company` should be cached but not `Person` nor any other `LegalEntity` subclass. The underlying approach here is to still define region and access-strategy information per-hierarchy - users will simply be able to opt out of (or into) caching particular subclasses. In my initial attempt I simply allowed both `@Cache` and `@Cacheable` to appear anywhere in the hierarchy. However, allowing `@Cache` (as currently defined) implies that users should be able to define different regions and/or access strategies for various subclasses within the hierarchy. Stepping back, I thought a better solution would be to continue to support `@Cache` only at the root level and define this new feature in terms of `@Cacheable` at the various levels. This has a few implications that I wanted to discuss. The main thing is that this means that applications using just `@Cache` to define caching would still only be able to declare caching for the entire hierarchy. But I think that is ok because that was the legacy behavior, and so nothing is really changing there. If we find `@Cache` on the root we'd assume an implicit `@Cacheable(true)`. I think some examples will help explain... Current behavior @Inheritance(...) @Cache(...) abstract class LegalEntity { ... } class Person extends LegalEntity { ... } class Company extends LegalEntity { ... } In the current behavior both `@Cache` and `@Cacheable` are only valid on the root as seen above. Placing them on any subclass results in an error. Note too that we could have used `@Cacheable` here instead of `@Cache` in which case the default `@Cache` values would be applied. It was also legal to use both together. In fact, a portable application would use `@Cacheable` with or without `@Cache`. Proposed behavior @Inheritance(...) @Cache(...) @Cacheable(false) abstract class LegalEntity { ... } class Person extends LegalEntity { ... } @Cacheable(true) class Company extends LegalEntity { ... } Here we have the root disabling caching (assuming `SharedCacheMode.ENABLE_SELECTIVE`). `Person` inherits that setting. `Company` however overrides that to enable caching. We still have `@Cache` attached to the root to define the specifics of caching (region, access strategy). But as noted earlier, we could have left off `@Cache` and accepted the defaults. I also propose that we continue to accept `@Cache` (without explicit `@Cacheable`) as implying `@Cacheable(true)` although `SharedCacheMode` plays a part in that too. Anyway, I wanted to make sure everyone agrees with this. _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Its not any different than `#get( LegalEntity.class, key )` in the old config when key refers to a Person and Person is *not* in the cache simply because it has not been loaded/saved via this SF. On Tue, Dec 12, 2017 at 12:33 PM Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: > Conceptually it sounds useful but I'm wondering about this being safe > to do in various more tricky mapping scenarios. > > For example consider this case: > > @Inheritance(...) > @Cache(...) > @Cacheable(true) > class LegalEntity { > ... > } > > @Cacheable(false) > class Person extends LegalEntity { > ... > } > > @Cacheable(true) > class Company extends LegalEntity { > ... > } > > [N.B. the parent class is no longer abstract] > > Now imagine we have to implement a polymorphic load: `load(key, > LegalEntity.class)` > > Would you be able to use the 2LC safely in all possible inheritance > mappings? > > I'm particularly curious about the possibility of a Person being > stored, but since we won't have data about the Person in the cache > we'd materialize the load as a LegalEntity. > > Incidentally while writing this example I realize that such a mapping > could trigger issues even with existing caching options. > > Thanks, > Sanne > > > > On 12 December 2017 at 14:49, Steve Ebersole <steve(a)hibernate.org&gt; wrote: > > HHH-12146 is about being able to enable/disable caching at various > levels > > in an entity hierarchy. E.g., given a hierarchy such as `Person` and > > `Company` both extending `LegalEntity`, this would allow users to say > that > > only `Company` should be cached but not `Person` nor any other > > `LegalEntity` subclass. > > > > The underlying approach here is to still define region and > access-strategy > > information per-hierarchy - users will simply be able to opt out of (or > > into) caching particular subclasses. In my initial attempt I simply > > allowed both `@Cache` and `@Cacheable` to appear anywhere in the > > hierarchy. However, allowing `@Cache` (as currently defined) implies > that > > users should be able to define different regions and/or access > strategies > > for various subclasses within the hierarchy. Stepping back, I thought a > > better solution would be to continue to support `@Cache` only at the > root > > level and define this new feature in terms of `@Cacheable` at the > various > > levels. This has a few implications that I wanted to discuss. > > > > The main thing is that this means that applications using just `@Cache` > to > > define caching would still only be able to declare caching for the > entire > > hierarchy. But I think that is ok because that was the legacy behavior, > > and so nothing is really changing there. If we find `@Cache` on the > root > > we'd assume an implicit `@Cacheable(true)`. I think some examples will > > help explain... > > > > > > Current behavior > > > > @Inheritance(...) > > @Cache(...) > > abstract class LegalEntity { > > ... > > } > > > > class Person extends LegalEntity { > > ... > > } > > > > class Company extends LegalEntity { > > ... > > } > > > > In the current behavior both `@Cache` and `@Cacheable` are only valid > on > > the root as seen above. Placing them on any subclass results in an > error. > > Note too that we could have used `@Cacheable` here instead of `@Cache` > in > > which case the default `@Cache` values would be applied. It was also > legal > > to use both together. In fact, a portable application would use > > `@Cacheable` with or without `@Cache`. > > > > > > Proposed behavior > > > > @Inheritance(...) > > @Cache(...) > > @Cacheable(false) > > abstract class LegalEntity { > > ... > > } > > > > class Person extends LegalEntity { > > ... > > } > > > > @Cacheable(true) > > class Company extends LegalEntity { > > ... > > } > > > > Here we have the root disabling caching (assuming > > `SharedCacheMode.ENABLE_SELECTIVE`). `Person` inherits that setting. > > `Company` however overrides that to enable caching. We still have > > `@Cache` attached to the root to define the specifics of caching > (region, > > access strategy). But as noted earlier, we could have left off `@Cache` > > and accepted the defaults. > > > > I also propose that we continue to accept `@Cache` (without explicit > > `@Cacheable`) as implying `@Cacheable(true)` although `SharedCacheMode` > > plays a part in that too. > > > > Anyway, I wanted to make sure everyone agrees with this. > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >

We discussed this further in chat. For the sake of others reading, the summary: I got confused about the risks of having multiple cache regions for a single hierarchy; the proposal is *not* to have multiple regions yet be able to exclude specific types from the (shared) region of a type hierarchy. Sounds great. On 12 December 2017 at 18:45, Steve Ebersole <steve(a)hibernate.org&gt; wrote: > And btw, this *has* to happen. JPA requires it and the 2.2 TCK tests for > it. So there is no "keep allowing caching at the root-level" option here > > > On Tue, Dec 12, 2017 at 12:44 PM Steve Ebersole <steve(a)hibernate.org&gt; wrote: >> >> Its not any different than `#get( LegalEntity.class, key )` in the old >> config when key refers to a Person and Person is *not* in the cache simply >> because it has not been loaded/saved via this SF. >> >> On Tue, Dec 12, 2017 at 12:33 PM Sanne Grinovero <sanne(a)hibernate.org&gt; >> wrote: >>> >>> Conceptually it sounds useful but I'm wondering about this being safe >>> to do in various more tricky mapping scenarios. >>> >>> For example consider this case: >>> >>> @Inheritance(...) >>> @Cache(...) >>> @Cacheable(true) >>> class LegalEntity { >>> ... >>> } >>> >>> @Cacheable(false) >>> class Person extends LegalEntity { >>> ... >>> } >>> >>> @Cacheable(true) >>> class Company extends LegalEntity { >>> ... >>> } >>> >>> [N.B. the parent class is no longer abstract] >>> >>> Now imagine we have to implement a polymorphic load: `load(key, >>> LegalEntity.class)` >>> >>> Would you be able to use the 2LC safely in all possible inheritance >>> mappings? >>> >>> I'm particularly curious about the possibility of a Person being >>> stored, but since we won't have data about the Person in the cache >>> we'd materialize the load as a LegalEntity. >>> >>> Incidentally while writing this example I realize that such a mapping >>> could trigger issues even with existing caching options. >>> >>> Thanks, >>> Sanne >>> >>> >>> >>> On 12 December 2017 at 14:49, Steve Ebersole <steve(a)hibernate.org&gt; wrote: >>> > HHH-12146 is about being able to enable/disable caching at various >>> > levels >>> > in an entity hierarchy. E.g., given a hierarchy such as `Person` and >>> > `Company` both extending `LegalEntity`, this would allow users to say >>> > that >>> > only `Company` should be cached but not `Person` nor any other >>> > `LegalEntity` subclass. >>> > >>> > The underlying approach here is to still define region and >>> > access-strategy >>> > information per-hierarchy - users will simply be able to opt out of (or >>> > into) caching particular subclasses. In my initial attempt I simply >>> > allowed both `@Cache` and `@Cacheable` to appear anywhere in the >>> > hierarchy. However, allowing `@Cache` (as currently defined) implies >>> > that >>> > users should be able to define different regions and/or access >>> > strategies >>> > for various subclasses within the hierarchy. Stepping back, I thought >>> > a >>> > better solution would be to continue to support `@Cache` only at the >>> > root >>> > level and define this new feature in terms of `@Cacheable` at the >>> > various >>> > levels. This has a few implications that I wanted to discuss. >>> > >>> > The main thing is that this means that applications using just `@Cache` >>> > to >>> > define caching would still only be able to declare caching for the >>> > entire >>> > hierarchy. But I think that is ok because that was the legacy >>> > behavior, >>> > and so nothing is really changing there. If we find `@Cache` on the >>> > root >>> > we'd assume an implicit `@Cacheable(true)`. I think some examples will >>> > help explain... >>> > >>> > >>> > Current behavior >>> > >>> > @Inheritance(...) >>> > @Cache(...) >>> > abstract class LegalEntity { >>> > ... >>> > } >>> > >>> > class Person extends LegalEntity { >>> > ... >>> > } >>> > >>> > class Company extends LegalEntity { >>> > ... >>> > } >>> > >>> > In the current behavior both `@Cache` and `@Cacheable` are only valid >>> > on >>> > the root as seen above. Placing them on any subclass results in an >>> > error. >>> > Note too that we could have used `@Cacheable` here instead of `@Cache` >>> > in >>> > which case the default `@Cache` values would be applied. It was also >>> > legal >>> > to use both together. In fact, a portable application would use >>> > `@Cacheable` with or without `@Cache`. >>> > >>> > >>> > Proposed behavior >>> > >>> > @Inheritance(...) >>> > @Cache(...) >>> > @Cacheable(false) >>> > abstract class LegalEntity { >>> > ... >>> > } >>> > >>> > class Person extends LegalEntity { >>> > ... >>> > } >>> > >>> > @Cacheable(true) >>> > class Company extends LegalEntity { >>> > ... >>> > } >>> > >>> > Here we have the root disabling caching (assuming >>> > `SharedCacheMode.ENABLE_SELECTIVE`). `Person` inherits that setting. >>> > `Company` however overrides that to enable caching. We still have >>> > `@Cache` attached to the root to define the specifics of caching >>> > (region, >>> > access strategy). But as noted earlier, we could have left off >>> > `@Cache` >>> > and accepted the defaults. >>> > >>> > I also propose that we continue to accept `@Cache` (without explicit >>> > `@Cacheable`) as implying `@Cacheable(true)` although `SharedCacheMode` >>> > plays a part in that too. >>> > >>> > Anyway, I wanted to make sure everyone agrees with this. >>> > _______________________________________________ >>> > hibernate-dev mailing list >>> > hibernate-dev(a)lists.jboss.org >>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

Vlad, these changes cause some failures in the userguide tests[1]. This is not completely unexpected since they seem to do caching-specific assertions. However, for the life of me I cannot figure out how to run these locally. The problem is the annotation processor for generating the "JPA static metamodel" classes. They either do not get generated, or at the very least the build is not adding them to the project in a way that IntelliJ import picks them up. [1] http://ci.hibernate.org/job/hibernate-orm-5.3-steve/28/testReport/ On Tue, Dec 12, 2017 at 9:27 PM Steve Ebersole <steve(a)hibernate.org&gt; wrote: > I have just pushed this work to the 5.3 branch. Tomorrow I will start > working on getting this all integrated with master (and branching 5.2 > off). At that point we are pretty much ready for that first Beta, so let > me know if anyone notices anything awry - this is a pretty big change. > > On Tue, Dec 12, 2017 at 7:18 PM Sanne Grinovero <sanne(a)hibernate.org&gt; > wrote: > >> We discussed this further in chat. For the sake of others reading, the >> summary: I got confused about the risks of having multiple cache >> regions for a single hierarchy; the proposal is *not* to have multiple >> regions yet be able to exclude specific types from the (shared) region >> of a type hierarchy. >> >> Sounds great. >> >> On 12 December 2017 at 18:45, Steve Ebersole <steve(a)hibernate.org&gt; >> wrote: >> > And btw, this *has* to happen. JPA requires it and the 2.2 TCK tests >> for >> > it. So there is no "keep allowing caching at the root-level" option >> here >> > >> > >> > On Tue, Dec 12, 2017 at 12:44 PM Steve Ebersole <steve(a)hibernate.org&gt; >> wrote: >> >> >> >> Its not any different than `#get( LegalEntity.class, key )` in the old >> >> config when key refers to a Person and Person is *not* in the cache >> simply >> >> because it has not been loaded/saved via this SF. >> >> >> >> On Tue, Dec 12, 2017 at 12:33 PM Sanne Grinovero <sanne(a)hibernate.org >> > >> >> wrote: >> >>> >> >>> Conceptually it sounds useful but I'm wondering about this being safe >> >>> to do in various more tricky mapping scenarios. >> >>> >> >>> For example consider this case: >> >>> >> >>> @Inheritance(...) >> >>> @Cache(...) >> >>> @Cacheable(true) >> >>> class LegalEntity { >> >>> ... >> >>> } >> >>> >> >>> @Cacheable(false) >> >>> class Person extends LegalEntity { >> >>> ... >> >>> } >> >>> >> >>> @Cacheable(true) >> >>> class Company extends LegalEntity { >> >>> ... >> >>> } >> >>> >> >>> [N.B. the parent class is no longer abstract] >> >>> >> >>> Now imagine we have to implement a polymorphic load: `load(key, >> >>> LegalEntity.class)` >> >>> >> >>> Would you be able to use the 2LC safely in all possible inheritance >> >>> mappings? >> >>> >> >>> I'm particularly curious about the possibility of a Person being >> >>> stored, but since we won't have data about the Person in the cache >> >>> we'd materialize the load as a LegalEntity. >> >>> >> >>> Incidentally while writing this example I realize that such a mapping >> >>> could trigger issues even with existing caching options. >> >>> >> >>> Thanks, >> >>> Sanne >> >>> >> >>> >> >>> >> >>> On 12 December 2017 at 14:49, Steve Ebersole <steve(a)hibernate.org&gt; >> wrote: >> >>> > HHH-12146 is about being able to enable/disable caching at various >> >>> > levels >> >>> > in an entity hierarchy. E.g., given a hierarchy such as `Person` >> and >> >>> > `Company` both extending `LegalEntity`, this would allow users to >> say >> >>> > that >> >>> > only `Company` should be cached but not `Person` nor any other >> >>> > `LegalEntity` subclass. >> >>> > >> >>> > The underlying approach here is to still define region and >> >>> > access-strategy >> >>> > information per-hierarchy - users will simply be able to opt out >> of (or >> >>> > into) caching particular subclasses. In my initial attempt I >> simply >> >>> > allowed both `@Cache` and `@Cacheable` to appear anywhere in the >> >>> > hierarchy. However, allowing `@Cache` (as currently defined) >> implies >> >>> > that >> >>> > users should be able to define different regions and/or access >> >>> > strategies >> >>> > for various subclasses within the hierarchy. Stepping back, I >> thought >> >>> > a >> >>> > better solution would be to continue to support `@Cache` only at >> the >> >>> > root >> >>> > level and define this new feature in terms of `@Cacheable` at the >> >>> > various >> >>> > levels. This has a few implications that I wanted to discuss. >> >>> > >> >>> > The main thing is that this means that applications using just >> `@Cache` >> >>> > to >> >>> > define caching would still only be able to declare caching for the >> >>> > entire >> >>> > hierarchy. But I think that is ok because that was the legacy >> >>> > behavior, >> >>> > and so nothing is really changing there. If we find `@Cache` on >> the >> >>> > root >> >>> > we'd assume an implicit `@Cacheable(true)`. I think some examples >> will >> >>> > help explain... >> >>> > >> >>> > >> >>> > Current behavior >> >>> > >> >>> > @Inheritance(...) >> >>> > @Cache(...) >> >>> > abstract class LegalEntity { >> >>> > ... >> >>> > } >> >>> > >> >>> > class Person extends LegalEntity { >> >>> > ... >> >>> > } >> >>> > >> >>> > class Company extends LegalEntity { >> >>> > ... >> >>> > } >> >>> > >> >>> > In the current behavior both `@Cache` and `@Cacheable` are only >> valid >> >>> > on >> >>> > the root as seen above. Placing them on any subclass results in an >> >>> > error. >> >>> > Note too that we could have used `@Cacheable` here instead of >> `@Cache` >> >>> > in >> >>> > which case the default `@Cache` values would be applied. It was >> also >> >>> > legal >> >>> > to use both together. In fact, a portable application would use >> >>> > `@Cacheable` with or without `@Cache`. >> >>> > >> >>> > >> >>> > Proposed behavior >> >>> > >> >>> > @Inheritance(...) >> >>> > @Cache(...) >> >>> > @Cacheable(false) >> >>> > abstract class LegalEntity { >> >>> > ... >> >>> > } >> >>> > >> >>> > class Person extends LegalEntity { >> >>> > ... >> >>> > } >> >>> > >> >>> > @Cacheable(true) >> >>> > class Company extends LegalEntity { >> >>> > ... >> >>> > } >> >>> > >> >>> > Here we have the root disabling caching (assuming >> >>> > `SharedCacheMode.ENABLE_SELECTIVE`). `Person` inherits that >> setting. >> >>> > `Company` however overrides that to enable caching. We still have >> >>> > `@Cache` attached to the root to define the specifics of caching >> >>> > (region, >> >>> > access strategy). But as noted earlier, we could have left off >> >>> > `@Cache` >> >>> > and accepted the defaults. >> >>> > >> >>> > I also propose that we continue to accept `@Cache` (without >> explicit >> >>> > `@Cacheable`) as implying `@Cacheable(true)` although >> `SharedCacheMode` >> >>> > plays a part in that too. >> >>> > >> >>> > Anyway, I wanted to make sure everyone agrees with this. >> >>> > _______________________________________________ >> >>> > hibernate-dev mailing list >> >>> > hibernate-dev(a)lists.jboss.org >> >>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >