<p dir="ltr">Is this really critical for 1.9.x? I'm worried about us intruding more complexity so late and I'd rather postpone to 2.x.</p>
<div class="gmail_quote">On 22 Mar 2016 15:36, "Marek Posolda" <<a href="mailto:mposolda@redhat.com">mposolda@redhat.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 22/03/16 15:18, Bill Burke wrote:<br>
> Some comments on code:<br>
><br>
> * Why have a special flag in invalidateUser? Just invalidate the links<br>
> if they exist. Aggressive eviction isn't a bad thing, is it?<br>
The only reason was performance. The flag "false" is used when you're<br>
adding new user. But this new user is not yet in cache, so calling<br>
"getFederatedIdentities" will always need to send DB query to get all<br>
links of user. But when you're adding new user, you know that this user<br>
doesn't yet have any social links. So in the end, without the flag there<br>
will be one more DB query, which will always return empty set.<br>
<br>
Currently we don't support store/load of social links from federation<br>
provider. But maybe in the future, if we support it and we chain cache<br>
on top of federation storage, it could theoretically happen that new<br>
user have social links at the moment when he is registered. So I can do<br>
what you proposed as it seems to be a bit safer for the future.<br>
<br>
Marek<br>
><br>
> On a side note, cache needs more refactoring, we're reusing the same<br>
> pattern over and over. Wonder if functions can help.<br>
><br>
><br>
> On 3/22/2016 5:42 AM, Marek Posolda wrote:<br>
>> Until now, we don't have support for caching of identityProvider<br>
>> (social) links. So every social login or every going to "federated<br>
>> identities" tab in account management needs to send DB queries. I was<br>
>> looking at fixing it and I've send PR<br>
>> <a href="https://github.com/keycloak/keycloak/pull/2404" rel="noreferrer" target="_blank">https://github.com/keycloak/keycloak/pull/2404</a> . It turned to be a bit<br>
>> tricky because of:<br>
>><br>
>> 1) Caching needs to be done on both directions. For social login, you<br>
>> need to lookup user by social link. But on the other hand, you also need<br>
>> to look all social/identityProvider links of particular user when you go<br>
>> to account management etc.<br>
>><br>
>> 2) Because of "store token" option, the link may need to be updated in<br>
>> DB quite often (in theory even during each social login).<br>
>><br>
>> I was thinking that storing links directly on CachedUser doesn't work<br>
>> very well, because during each update of social link (which may be often<br>
>> because of "store token") the user would need to be fully invalidated<br>
>> from cache. Having separate cache entry for each social link also has<br>
>> some downsides (many items in cache, need to have separate entry for<br>
>> store the all links of user anyway). So I ended up with having the cache<br>
>> entry, which contains list of all links of particular user. It needs to<br>
>> be updated when any social link is added,removed or updated. This seemed<br>
>> to me like good compromise. WDYT?<br>
>><br>
>> There are also entries for lookup user by federated identity, so you<br>
>> don't need to query DB during social login.<br>
>><br>
>> So ATM there are not DB queries during social login or during go to<br>
>> "federated identities" . I hope I handled all corner cases and<br>
>> invalidations correctly, but if someone want to take look it will be<br>
>> good. I don't want to add new regressions atm :-)<br>
>><br>
>> Marek<br>
>><br>
>><br>
>><br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> keycloak-dev mailing list<br>
>> <a href="mailto:keycloak-dev@lists.jboss.org">keycloak-dev@lists.jboss.org</a><br>
>> <a href="https://lists.jboss.org/mailman/listinfo/keycloak-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/keycloak-dev</a><br>
<br>
_______________________________________________<br>
keycloak-dev mailing list<br>
<a href="mailto:keycloak-dev@lists.jboss.org">keycloak-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/keycloak-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/keycloak-dev</a><br>
</blockquote></div>