<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0" /> <base href="https://hibernate.atlassian.net" />
<title>Message Title</title>
</head>
<body class="jira" style="color: #333; font-family: Arial, sans-serif; font-size: 14px; line-height: 1.429">
<table id="background-table" cellpadding="0" cellspacing="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; background-color: #f5f5f5; border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<!-- header here -->
<tr>
<td id="header-pattern-container" style="padding: 0px; border-collapse: collapse; padding: 10px 20px">
<table id="header-pattern" cellspacing="0" cellpadding="0" border="0" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td id="header-avatar-image-container" valign="top" style="padding: 0px; border-collapse: collapse; vertical-align: top; width: 32px; padding-right: 8px"> <img id="header-avatar-image" class="image_fix" src="https://secure.gravatar.com/avatar/57637a2eb871b34eba14e700c78c6a5d?d=mm&s=48" height="32" width="32" border="0" style="border-radius: 3px; vertical-align: top" />
</td>
<td id="header-text-container" valign="middle" style="padding: 0px; border-collapse: collapse; vertical-align: middle; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 1px"> <a class="user-hover" rel="sanne" id="email_sanne" href="https://hibernate.atlassian.net/secure/ViewProfile.jspa?name=sanne" style="color:#6c797f;; color: #3b73af; text-decoration: none">Sanne Grinovero</a> <strong>commented</strong> on <a href="https://hibernate.atlassian.net/browse/HHH-9780" style="color: #3b73af; text-decoration: none"><img src="cid:jira-generated-image-static-improvement-d57d1056-de21-474a-aa63-628faa82430c" height="16" width="16" border="0" align="absmiddle" alt="Improvement" /> HHH-9780</a>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td id="email-content-container" style="padding: 0px; border-collapse: collapse; padding: 0 20px">
<table id="email-content-table" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; border-spacing: 0; border-collapse: separate">
<tr>
<!-- there needs to be content in the cell for it to render in some clients -->
<td class="email-content-rounded-top mobile-expand" style="padding: 0px; border-collapse: collapse; color: #fff; padding: 0 15px 0 16px; height: 15px; background-color: #fff; border-left: 1px solid #ccc; border-top: 1px solid #ccc; border-right: 1px solid #ccc; border-bottom: 0; border-top-right-radius: 5px; border-top-left-radius: 5px; height: 10px; line-height: 10px; padding: 0 15px 0 16px; mso-line-height-rule: exactly">
</td>
</tr>
<tr>
<td class="email-content-main mobile-expand " style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff">
<table class="page-title-pattern" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td style="vertical-align: top;; padding: 0px; border-collapse: collapse; padding-right: 5px; font-size: 20px; line-height: 30px; mso-line-height-rule: exactly" class="page-title-pattern-header-container"> <span class="page-title-pattern-header" style="font-family: Arial, sans-serif; padding: 0; font-size: 20px; line-height: 30px; mso-text-raise: 2px; mso-line-height-rule: exactly; vertical-align: middle"> <a href="https://hibernate.atlassian.net/browse/HHH-9780" style="color: #3b73af; text-decoration: none">Re: Unique instance for both CacheKey and EntityKey contracts</a> </span>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td id="text-paragraph-pattern-top" class="email-content-main mobile-expand comment-top-pattern" style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff; border-bottom: none; padding-bottom: 0">
<table class="text-paragraph-pattern" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 2px">
<tr>
<td class="text-paragraph-pattern-container mobile-resize-text " style="padding: 0px; border-collapse: collapse; padding: 0 0 10px 0">
<p style="margin: 10px 0 0 0">We had an in depth design discussion on IRC:</p>
<div class="code panel" style="border-width: 1px;; border: 1px solid #ccc; background: #f5f5f5; font-size: 12px; line-height: 1.333; font-family: monospace; border: 1px solid #ccc; -moz-border-radius: 3px 3px 3px 3px; border-radius: 3px 3px 3px 3px; margin: 9px 0">
<div class="codeContent panelContent" style="padding: 9px 12px">
<pre class="code-java" style="margin: 10px 0 0 0; max-height: 30em; overflow: auto; white-space: pre-wrap; word-wrap: normal">[17:16] <sannegrinovero> sebersole: I can spend some time to aim at HHH-9780 <span class="code-keyword" style="color: #000091">this</span> weekend, but will need some pointers from you
[17:17] <sebersole> sannegrinovero: tbh i dont really know what you mean with your part of that :)
[17:17] <sebersole> also that seems directly at odds with the idea of leveraging the cache impls notion of a cache key
[17:18] <sebersole> bascially, there are 3 concepts here
[17:18] <sebersole> and i just dont see how we can model them all as the same class
[17:19] <sannegrinovero> maybe not, I just need to understand it better. You mentioned the 2lc implementors create their own, I didn't find that
[17:19] <sebersole> especially as delegated to the cache impl
[17:19] <sannegrinovero> you have a pointer?
[17:19] <sebersole> actually its something you mentioned :)
[17:19] <sebersole> so you tell me...
[17:20] <sebersole> lets take infinispan e.g...
[17:20] <sannegrinovero> yes I also remember that :) I'm not finding it anymore though, wondering <span class="code-keyword" style="color: #000091">if</span> that's just an outdated obeservation.
[17:20] <sebersole> ultimately I ask inifispan to store some data <span class="code-keyword" style="color: #000091">for</span> me
[17:20] <sebersole> i pass it (1) the data to cache
[17:20] <sebersole> and
[17:20] <sebersole> (2) my CacheKey
[17:21] <sebersole> what does infinispan <span class="code-keyword" style="color: #000091">do</span> there internally>
[17:21] <sebersole> ?
[17:21] <sebersole> does it directly key the entry using my CacheKey?
[17:21] <sebersole> surely not right?
[17:21] <sannegrinovero> looking at master today, it seems to just use that key instance w/o rewraps
[17:21] <sannegrinovero> (which is not what I remember)
[17:21] <sebersole> ok then
[17:21] <sebersole> well...
[17:22] <sebersole> master of what>
[17:22] <sebersole> hibernate-infinispan?
[17:22] <sebersole> or
[17:22] <sebersole> infinispan?
[17:22] <sannegrinovero> it also feels wrong actually because it means it can hardly optimize it <span class="code-keyword" style="color: #000091">for</span> its custom serialization strategies
[17:22] <sannegrinovero> hibernate-infinispan
[17:22] <sebersole> well more i mean what infnispan does after hibernate-infinispan hands it the info
[17:22] <sannegrinovero> so sebersole, proposal:
[17:23] <sebersole> i thought that was the issue
[17:23] <sannegrinovero> 1# we change the CacheKey to be an <span class="code-keyword" style="color: #000091">interface</span>
[17:23] <sebersole> ok...
[17:23] <sannegrinovero> 2# and then add a <span class="code-quote" style="color: #009100">"createCacheKey"</span> method on the 2lc provider SPI
[17:23] <sannegrinovero> so to allow the providers to factor custom keys as needed
[17:23] <sebersole> sure, thats what i suggested :)
[17:23] <sannegrinovero> (any custom optimisation there can wait)
[17:23] <sannegrinovero> ok great
[17:24] <sebersole> but
[17:24] <sebersole> we dont even need CacheKey <span class="code-keyword" style="color: #000091">interface</span>
[17:24] <sebersole> we never use those data values
[17:24] <sannegrinovero> that's right the 2lc <span class="code-keyword" style="color: #000091">interface</span> accepts <span class="code-quote" style="color: #009100">"<span class="code-object" style="color: #910091; color: #009100">Object</span>"</span> <span class="code-keyword" style="color: #000091">for</span> keys
[17:24] <sebersole> really createCacheKey can <span class="code-keyword" style="color: #000091">return</span> <span class="code-object" style="color: #910091">Object</span>
[17:24] <sebersole> right
[17:24] <sebersole> i mean
[17:25] <sebersole> might be a nice chance to improve that
[17:25] <sannegrinovero> but should we keep that? It seems nicer to change the SPI <span class="code-keyword" style="color: #000091">interface</span> to only accept the typesafe CacheKey
[17:25] <sebersole> 2 things i hate passing
[17:25] <sebersole> <span class="code-object" style="color: #910091">Object</span>
[17:25] <sebersole> <span class="code-object" style="color: #910091">String</span>
[17:25] <sannegrinovero> +1
[17:25] <sebersole> i am fine with that
[17:26] <sannegrinovero> I'd assume that once we offload the factory responsibility to the cache implementor, they should be able to make keys conforming to it w/o drawbacks.. right?
[17:26] <sebersole> alex may not be :)
[17:26] <sannegrinovero> I'll make <span class="code-keyword" style="color: #000091">this</span> change as PR and then ask <span class="code-keyword" style="color: #000091">for</span> feedback from Alex and Galder
[17:26] <sebersole> sounds perfect
[17:27] <sannegrinovero> ok, second part of the problem:
[17:27] <sebersole> but back to
[17:27] <sebersole> rightr
[17:27] <sebersole> and so here is one problem
[17:27] <sannegrinovero> the key to EntityEntry is a different beast entirely then, right?
[17:27] <sebersole> we removed the idea of some of the selectors from EntityKey
[17:28] <sebersole> well i assume you mean EntityKey, notEntityEntry
[17:28] <sannegrinovero> yes, EntityKey, the key to EntityEntry ;)
[17:29] <sebersole> so second level cache needs to segment things differently then PC
[17:29] <sebersole> it needs to account <span class="code-keyword" style="color: #000091">for</span> tenancy e.g.
[17:29] <sebersole> whereas the PC does not
[17:29] <sebersole> (the PC is inherently tied to *a* tenant)
[17:30] <sannegrinovero> good point
[17:31] <sannegrinovero> but what <span class="code-keyword" style="color: #000091">if</span> we were to add - again on the 2lc SPI - a method like "CacheKey convert(EntityKey, additional metadata like tenantId )
[17:31] <sannegrinovero> or vice-versa could be even more interesting
[17:31] <sannegrinovero> would there be some method in ORM to benefit from invoking <span class="code-keyword" style="color: #000091">this</span> conversion?
[17:32] <sebersole> well it assumes having the EntityKey
[17:32] <sannegrinovero> (some strategic method)
[17:32] <sebersole> or creating it <span class="code-keyword" style="color: #000091">if</span> we dont
[17:32] <sebersole> its <span class="code-keyword" style="color: #000091">this</span> later point that is concerning
[17:32] <sannegrinovero> well <span class="code-keyword" style="color: #000091">if</span> you don't, you'd invoke the other method we agreed on, the factory.
[17:32] <sebersole> oh
[17:32] <sebersole> you mean having multiple methods
[17:33] <sebersole> sure
[17:33] <sannegrinovero> yes, to add a conversion method as something on top of the factory
[17:33] <sannegrinovero> see there are <span class="code-keyword" style="color: #000091">for</span> sure cases in which one implementation could serve fine <span class="code-keyword" style="color: #000091">for</span> both use cases (say multi-tenancy is disabled, <span class="code-keyword" style="color: #000091">for</span> one)
[17:34] <sebersole> well depending on the declaration of CacheKey... the <span class="code-quote" style="color: #009100">"other way"</span> may not be needed
[17:34] <sannegrinovero> what I don't know, <span class="code-keyword" style="color: #000091">if</span> there are points in code in ORM in which you'd have one and need the other
[17:35] <sebersole> ohhhhh
[17:35] <sebersole> another oddity
[17:35] <sebersole> so CacheKey can be used to key entity data
[17:35] <sebersole> but
[17:35] <sebersole> it can also be used to cache collection data
[17:36] <sebersole> so its related to org.hibernate.engine.spi.CollectionKey as well
[17:36] <sebersole> CollectionKey is the EntityKey corollary in the PC
[17:36] <sannegrinovero> ah, contract-wise that gets ugly unless we seprate the notion of cache key into two different types too
[17:37] <sebersole> thats why its called <span class="code-quote" style="color: #009100">"entityOrRoleName"</span> in CacheKey
[17:37] <sebersole> which we could
[17:37] <sebersole> since the Regions are distinct
[17:37] <sebersole> CollectionRegion/CollectionRegionAccessStrategy
[17:37] <sebersole> versus
[17:37] <sannegrinovero> considering they're currently accepting <span class="code-quote" style="color: #009100">"<span class="code-object" style="color: #910091; color: #009100">Object</span>"</span> I guess it won't be too bad ;)
[17:37] <sebersole> EntityRegion/EntityRegionAccessStrategy
[17:38] <sebersole> we might have to parametize the access stratregy though <span class="code-keyword" style="color: #000091">for</span> that to work
[17:38] <sebersole> as I think they share get() etc methods
[17:38] <sebersole> yeah... org.hibernate.cache.spi.access.RegionAccessStrategy
[17:39] <sebersole> we'd need RegionAccessStrategy<T> - where <T> is <span class="code-quote" style="color: #009100">"key type"</span>
[17:39] <sannegrinovero> so each access strategy already is a different contract.. they could then accept the same type CacheKey but convert from/to the CollectionKey or EntityKey ?
[17:40] <sebersole> i think having the caches convert *to* CacheKey makes sense
[17:40] <sebersole> i actually dont think the other way makes sense
[17:40] <sebersole> imo
[17:40] <sebersole> again, assuming CacheKey exposes its state
[17:40] <sebersole> at least the ones we care about :)
[17:41] <sannegrinovero> ok, the biggest win is definitely from cache to EntityKey (as one would hope there are more cache hits than cache stores)
[17:41] <sannegrinovero> so my primary goal is to see <span class="code-keyword" style="color: #000091">if</span> we can avoid allocating an EntityKey after a cache hit
[17:42] <sebersole> are you thinking to wrapo the EntityKey?
[17:42] <sebersole> <span class="code-keyword" style="color: #000091">if</span> so...
[17:42] <sebersole> then here is what i think makes sense...
[17:42] <sebersole> 1) split CacheKey into 2 interfaces
[17:42] <sebersole> EntityCacheKey
[17:42] <sebersole> CollectionCacheKey
[17:43] <sebersole> they each define just one method
[17:43] <sebersole> EntityCacheKey#toEntityKey
[17:43] <sebersole> CollectionCacheKey#toCollectionKey
[17:43] <sannegrinovero> ah, sweet
[17:43] <sannegrinovero> then you could have some such implementations just <span class="code-keyword" style="color: #000091">return</span> <span class="code-quote" style="color: #009100">"<span class="code-keyword" style="color: #000091; color: #009100">this</span>"</span>, right?
[17:43] <sebersole> 2) RegionFactory provides the factory <span class="code-keyword" style="color: #000091">for</span> creating these cache keys
[17:44] <sebersole> right
[17:44] <sebersole> <span class="code-keyword" style="color: #000091">this</span> factory would allow:
[17:44] <sebersole> a) creation from simple values
[17:44] <sebersole> b) creation from EntityKey/CollectionKey +
[17:45] <sannegrinovero> nice, that all feels fitting well the needs
[17:45] <sebersole> our spi can provide a Helper <span class="code-keyword" style="color: #000091">for</span> these
[17:45] <sannegrinovero> I'm only wondering <span class="code-keyword" style="color: #000091">if</span> there is a practical use <span class="code-keyword" style="color: #000091">for</span> 2)b
[17:45] <sannegrinovero> you'd need to point me to the ORM code which could benefit from that, I don't have metrics <span class="code-keyword" style="color: #000091">for</span> <span class="code-keyword" style="color: #000091">this</span> <span class="code-keyword" style="color: #000091">case</span>
[17:46] <sebersole> it depends what you envision practicallty inside these cache key impls
[17:46] <sannegrinovero> although, <span class="code-keyword" style="color: #000091">this</span> could be done after an API freeze.. just making sure there is a practical use <span class="code-keyword" style="color: #000091">for</span> it.
[17:46] <sebersole> are they holding (wrapping) EntityKey/CollectionKey instances?
[17:46] <sebersole> its mainly the after action stuff
[17:47] <sebersole> so we are pushing flushed changes to the cache
[17:47] <sebersole> we'd have the PC keys
[17:48] <sannegrinovero> at best the 2lc could reuse the same instance, but the key type would need to satisfy the cache key contract too
[17:48] <sannegrinovero> the alternative is a wrap but I wonder <span class="code-keyword" style="color: #000091">if</span> that would still win us something
[17:49] <sebersole> not following
[17:49] <sannegrinovero> I mean, when it comes to store things in the cache, and you have the PC keys
[17:49] <sebersole> right, which is why isaid:
[17:50] <sannegrinovero> *ideally* one would like to reuse the PC keys as-is (<span class="code-keyword" style="color: #000091">if</span> configuration allows - like again no multi-tenant)
[17:50] <sebersole> [10:46] <sebersole> it depends what you envision practicallty inside these cache key impls
[17:50] <sannegrinovero> yes
[17:50] <sebersole> but imagine:
[17:51] <sebersole> EntityCacheKeyImpl <span class="code-keyword" style="color: #000091">implements</span> EntityCacheKey { <span class="code-keyword" style="color: #000091">private</span> <span class="code-keyword" style="color: #000091">final</span> EntityKey entityKey; <span class="code-keyword" style="color: #000091">private</span> <span class="code-keyword" style="color: #000091">final</span> tenantId; ... }
[17:51] <sebersole> so its ^^ toEntityKey is simple
[17:51] <sebersole> and saves some instantiations
[17:51] <sannegrinovero> right that's the stronger benefit
[17:52] <sannegrinovero> but I was now looking at the possibility to also save some allocation in the inverse transformation
[17:52] <sebersole> wdym?
[17:52] <sannegrinovero> which is a minor, more questionable win
[17:52] <sebersole> EntityKey->EntityCacheKey?
[17:52] <sannegrinovero> yes
[17:53] <sannegrinovero> to be able to use the EntityKey instance as key within the caches.
[17:53] <sebersole> well i think we already track that
[17:53] <sannegrinovero> track?
[17:54] <sebersole> well that could happen, sure, but only <span class="code-keyword" style="color: #000091">if</span> the other <span class="code-quote" style="color: #009100">"selectors"</span> (tenantId, etc) are the same across sessions
[17:54] <sannegrinovero> yes
[17:54] <sannegrinovero> it's not something we can enforce, especially not as a type because of those reasons
[17:54] <sebersole> yeah, i thought EntityEntry cached the cache key
[17:54] <sannegrinovero> but the key factory or a smart cache implementor could take advantage from some configurations
[17:55] <sebersole> well as far as tenancy goes...
[17:55] <sebersole> you dont really need a config
[17:56] <sebersole> <span class="code-keyword" style="color: #000091">if</span> the tenantId is ever <span class="code-keyword" style="color: #000091">null</span> you know the SF is not using multi-trenancy
[17:56] <sannegrinovero> right, I just mean that conceptually the implementor could be able to <span class="code-keyword" style="color: #000091">do</span> some smart choices in some circumstances - even <span class="code-keyword" style="color: #000091">if</span> not all.
[17:56] <sebersole> and the EntityKey *shoul d be* safe to use (more or less) as the cache key
[17:57] <sebersole> but of course, with <span class="code-keyword" style="color: #000091">this</span> <span class="code-keyword" style="color: #000091">new</span> split in EntityEntry that is much more difficult mnow
[17:57] <sannegrinovero> it gets a bit tricky as those keys will need to implement a safe equals contract
[17:57] <sannegrinovero> <span class="code-keyword" style="color: #000091">if</span> potentially multiple different implementations of those keys are in the same cache
[17:57] <sebersole> yeah, i see why you'd want to use EntityKey as the cache key, but...
[17:58] <sebersole> i just dont think its feasible
[17:58] <sebersole> doing so would put a lot of stress on the PC to understand <span class="code-keyword" style="color: #000091">this</span> too
[17:58] <sebersole> <span class="code-quote" style="color: #009100">"hey can my EntityKeys be used as a cache key? <span class="code-keyword" style="color: #000091; color: #009100">if</span> so, I need to build these special ones..."</span>
[17:58] <sannegrinovero> it's complex <span class="code-keyword" style="color: #000091">if</span> we don't <span class="code-keyword" style="color: #000091">do</span> a further step. but what <span class="code-keyword" style="color: #000091">if</span> all EntityKey were created by that same key factory?
[17:59] <sebersole> i think thats wrong too
[17:59] <sebersole> why should the l2 cache be involved in building PC keys?
[17:59] <sebersole> again perf wise i get it
[17:59] <sannegrinovero> ok, <span class="code-keyword" style="color: #000091">if</span> it doesn't fit the concept then let's stop at the plan above
[17:59] <sebersole> conceptually, design-wise.... it just does nt fit
[18:00] <sannegrinovero> design-wise, I see it as we need a unique way to identify an entity, and you allow the 2nd level cache implementor to set the factory globally.
[18:00] <sebersole> well
[18:01] <sebersole> <span class="code-quote" style="color: #009100">"unique way to identify an entity"</span> is a big gloss over :)
[18:01] <sebersole> thats the problem
[18:01] <sannegrinovero> well, per persister ;)
[18:01] <sebersole> its not even per persister
[18:01] <sebersole> well, i guess <span class="code-keyword" style="color: #000091">if</span> you include tenant it in that sure
[18:02] <sebersole> <span class="code-keyword" style="color: #000091">do</span> remember though that *you* were the one that had me remove tenantId from EntityKey ;)
[18:02] <sannegrinovero> yes I remember :)
[18:02] <sebersole> otherwise <span class="code-keyword" style="color: #000091">this</span> is not a discussion :)
[18:03] <sebersole> but i <span class="code-keyword" style="color: #000091">do</span> like the <span class="code-keyword" style="color: #000091">rest</span> of the design we scioped out
[18:03] <sebersole> feels right
[18:03] <sannegrinovero> but ok the plan you described above seems very sound, up to the 2/b part which is of lower value
[18:04] <sebersole> sannegrinovero: i dont think it is :)
[18:04] <sebersole> really its just an overloaded method form
[18:05] <sannegrinovero> no? I thought we had just decided that <span class="code-keyword" style="color: #000091">this</span> last part gets too complex, <span class="code-keyword" style="color: #000091">for</span> low benefits
[18:05] <sebersole> well you were talking about actually *using* the EntityKey and the EntityCacheKey
[18:05] <sebersole> thats different
[18:05] <sebersole> 2.a is <span class="code-keyword" style="color: #000091">this</span>:
[18:07] <sebersole> EntityCacheKey cacheKey = regionFactory().getKeyFactory().createEntityCacheKey( entityName, key, tenantId, .. )
[18:07] <sebersole> 2.b is simply an overload:
[18:07] <sebersole> EntityCacheKey cacheKey = regionFactory().getKeyFactory().createEntityCacheKey( theEntityKey, tenantId, .. )
[18:08] <sannegrinovero> ok we can add it as convenience, but we agree that it's likely the implementor will need to allocate a <span class="code-keyword" style="color: #000091">new</span> EntityCacheKey, right?
[18:09] <sannegrinovero> instance reuse seems out of reach
[18:09] <sebersole> well, not necessarily
[18:10] <sebersole> but thats not my primary design goal
[18:10] <sebersole> but
[18:10] <sebersole> look at it...
[18:10] <sebersole> regionFactory().getKeyFactory().createEntityCacheKey( theEntityKey, tenantId, .. )
[18:10] <sebersole> and keep a few things in mind...
[18:11] <sebersole> 1) the *sole* contract fior EntityCacheKey is #toEntityKey
[18:11] <sannegrinovero> right, it looks sexy :) And it doesn't hurt to allow the implementor to <span class="code-keyword" style="color: #000091">do</span> something smarter eventually than what we've thought of today.
[18:11] <sebersole> so its actually possible <span class="code-keyword" style="color: #000091">for</span> the EntityKey to implement that contract too
[18:11] <sebersole> with a caveat
[18:12] <sebersole> <span class="code-quote" style="color: #009100">"hey... EntityKey *can* act as a EntityCacheKey, but only in non-multi-tenant environments"</span>
[18:12] <sebersole> and createEntityCacheKey knows that
[18:12] <sebersole> ergo...
[18:12] <sebersole> :)
[18:13] <sannegrinovero> but only <span class="code-keyword" style="color: #000091">if</span> EntityKey <span class="code-keyword" style="color: #000091">implements</span> EntityCacheKey, <span class="code-keyword" style="color: #000091">for</span> typesafety
[18:14] <sannegrinovero> I mean, to have the method compile..
[18:14] <sebersole> @Override <span class="code-keyword" style="color: #000091">public</span> EntityCacheKey createEntityCacheKey(EntityKey theEntityKey, <span class="code-object" style="color: #910091">String</span> tenantId)) { <span class="code-keyword" style="color: #000091">return</span> tenantId == <span class="code-keyword" style="color: #000091">null</span> ? theEntityKey : <span class="code-keyword" style="color: #000091">new</span> EntityCacheKeyImpl( theEntityKey, tenantId ); }
[18:14] <sebersole> [11:11] <sebersole> 1) the *sole* contract fior EntityCacheKey is #toEntityKey
[18:14] <sebersole> [11:11] <sebersole> so its actually possible <span class="code-keyword" style="color: #000091">for</span> the EntityKey to implement that contract too
[18:14] <sebersole> [11:11] <sebersole> with a caveat
[18:14] <sebersole> [11:12] <sebersole> <span class="code-quote" style="color: #009100">"hey... EntityKey *can* act as a EntityCacheKey, but only in non-multi-tenant environments"</span>
[18:14] <sebersole> :)
[18:15] <sannegrinovero> ok, I was thinking of <span class="code-quote" style="color: #009100">"conceptually"</span> not to actually have it implement the Java <span class="code-keyword" style="color: #000091">interface</span>
[18:15] <sebersole> too bad we cant groovy <span class="code-keyword" style="color: #000091">this</span> :)
[18:15] <sannegrinovero> wouldn't it then be confusing? One might think he can pass the EntityKey directly into the caching methods, without invoking the createEntityCacheKey method first to create the <span class="code-quote" style="color: #009100">"right kind"</span> of EntityCacheKey
[18:16] <sebersole> well that would be a bug right?
[18:16] <sebersole> ;)
[18:16] <sannegrinovero> yes we could <span class="code-keyword" style="color: #000091">catch</span> it with tests
[18:16] <sebersole> look
[18:16] <sebersole> like i said, that part is not my primary design goal here
[18:17] <sebersole> but
[18:19] <sebersole> it is possible <span class="code-keyword" style="color: #000091">if</span> we just have EntityKey implement EntityCacheKey
[18:19] <sebersole> its not ideal
[18:21] <sebersole> we could also use reflection based java duck typing
[18:21] <sebersole> to make the EntityKey act like a EntityCacheKey in those cases
[18:22] <sebersole> not sure how well that works with serialization etc though
[18:23] <sannegrinovero> I think we could leave <span class="code-keyword" style="color: #000091">this</span> lust detail open
[18:23] <sebersole> seems like an awful lot os hacks and drawbacks, which to me generally shows that you are trying to fit a square peg oin round hole
[18:23] <sannegrinovero> the contract changes seem clear
[18:23] <sannegrinovero> right, which is why above I was inclined to say we wouldn't benefit from <span class="code-keyword" style="color: #000091">this</span> last point <span class="code-keyword" style="color: #000091">for</span> instance reuse, but really <span class="code-keyword" style="color: #000091">this</span> last one is the lowest value
[18:24] <sebersole> as <span class="code-object" style="color: #910091">long</span> as you are not sayin that method should not exist, ok
[18:24] <sannegrinovero> so I'd vote to keep the EntityKey contract <span class="code-quote" style="color: #009100">"clean"</span>, let the implementor figure out a clever workaround <span class="code-keyword" style="color: #000091">if</span> he wishes so
[18:25] <sannegrinovero> no the method is fine
[18:25] <sannegrinovero> it looks good and fits the purpose
[18:25] <sebersole> i agree that duck typing EntityKey into an EntiytCacheKey is too much
[18:25] <sebersole> right
[18:25] <sannegrinovero> BTW the implementor could <span class="code-keyword" style="color: #000091">try</span> casting the key to its <span class="code-keyword" style="color: #000091">private</span> implementation - it might have been self-generated -
[18:25] <sannegrinovero> and his custom implementation *could* be able to fullfill the other contract too..
[18:26] <sannegrinovero> just not ORM's problem ;)
[18:26] <sannegrinovero> thanks sebersole, I'll log <span class="code-keyword" style="color: #000091">this</span> on JIRA there is lots of good ideas. I'll see how far I can apply it to reality :)
[18:26] <sebersole> nice!
[18:26] <sebersole> looking forward to it
[18:26] <sebersole> love good design discussions :)
[18:26] <sannegrinovero> +1 !</pre>
</div>
</div>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td class="email-content-main mobile-expand " style="padding: 0px; border-collapse: collapse; border-left: 1px solid #ccc; border-right: 1px solid #ccc; border-top: 0; border-bottom: 0; padding: 0 15px 0 16px; background-color: #fff">
<table id="actions-pattern" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 1px">
<tr>
<td id="actions-pattern-container" valign="middle" style="padding: 0px; border-collapse: collapse; padding: 10px 0 10px 24px; vertical-align: middle; padding-left: 0">
<table align="left" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td class="actions-pattern-action-icon-container" style="padding: 0px; border-collapse: collapse; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 0px; vertical-align: middle"> <a href="https://hibernate.atlassian.net/browse/HHH-9780#add-comment" target="_blank" title="Add Comment" style="color: #3b73af; text-decoration: none"> <img class="actions-pattern-action-icon-image" src="cid:jira-generated-image-static-comment-icon-527a5082-db0e-4652-8783-5de41c56bd9f" alt="Add Comment" title="Add Comment" height="16" width="16" border="0" style="vertical-align: middle" /> </a>
</td>
<td class="actions-pattern-action-text-container" style="padding: 0px; border-collapse: collapse; font-family: Arial, sans-serif; font-size: 14px; line-height: 20px; mso-line-height-rule: exactly; mso-text-raise: 4px; padding-left: 5px"> <a href="https://hibernate.atlassian.net/browse/HHH-9780#add-comment" target="_blank" title="Add Comment" style="color: #3b73af; text-decoration: none">Add Comment</a>
</td>
</tr>
</table>
</td>
</tr>
</table>
</td>
</tr>
<!-- there needs to be content in the cell for it to render in some clients -->
<tr>
<td class="email-content-rounded-bottom mobile-expand" style="padding: 0px; border-collapse: collapse; color: #fff; padding: 0 15px 0 16px; height: 5px; line-height: 5px; background-color: #fff; border-top: 0; border-left: 1px solid #ccc; border-bottom: 1px solid #ccc; border-right: 1px solid #ccc; border-bottom-right-radius: 5px; border-bottom-left-radius: 5px; mso-line-height-rule: exactly">
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td id="footer-pattern" style="padding: 0px; border-collapse: collapse; padding: 12px 20px">
<table id="footer-pattern-container" cellspacing="0" cellpadding="0" border="0" style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td id="footer-pattern-text" class="mobile-resize-text" width="100%" style="padding: 0px; border-collapse: collapse; color: #999; font-size: 12px; line-height: 18px; font-family: Arial, sans-serif; mso-line-height-rule: exactly; mso-text-raise: 2px">
This message was sent by Atlassian JIRA <span id="footer-build-information">(v6.5-OD-03-002#65000-<span title="b8f65f822a3fd2ca02be441f51a3c335a581ff1e" data-commit-id="b8f65f822a3fd2ca02be441f51a3c335a581ff1e}">sha1:b8f65f8</span>)</span>
</td>
<td id="footer-pattern-logo-desktop-container" valign="top" style="padding: 0px; border-collapse: collapse; padding-left: 20px; vertical-align: top">
<table style="border-collapse: collapse; mso-table-lspace: 0pt; mso-table-rspace: 0pt">
<tr>
<td id="footer-pattern-logo-desktop-padding" style="padding: 0px; border-collapse: collapse; padding-top: 3px"> <img id="footer-pattern-logo-desktop" src="cid:jira-generated-image-static-footer-desktop-logo-44ccccc6-7962-42e6-8f4b-c8362440db53" alt="Atlassian logo" title="Atlassian logo" width="169" height="36" class="image_fix" />
</td>
</tr>
</table>
</td>
</tr>
</table>
</td>
</tr>
</table>
</body>
</html>