[hibernate-dev] HHH-12146 - subclass-specific caching

[Steve Ebersole]

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 at hibernate.org> 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 at hibernate.org>
> 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 at hibernate.org> 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 at hibernate.org>
>> 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 at 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 at hibernate.org>
>> 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 at lists.jboss.org
>> >>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>