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

[Steve Ebersole]

I just went ahead and did the same we do for the other modules.

On Tue, Dec 12, 2017 at 9:47 PM Steve Ebersole <steve at hibernate.org> wrote:

> 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
>>>
>>