[keycloak-dev] federation commited need feedback

[Bill Burke]

On 7/24/2014 10:32 AM, Stian Thorgersen wrote:
>
>
> ----- Original Message -----
>> From: "Bill Burke" <bburke at redhat.com>
>> To: "Stian Thorgersen" <stian at redhat.com>
>> Cc: keycloak-dev at lists.jboss.org
>> Sent: Thursday, 24 July, 2014 2:47:57 PM
>> Subject: Re: [keycloak-dev] federation commited need feedback
>>
>>
>>
>> On 7/24/2014 9:25 AM, Stian Thorgersen wrote:
>>> Looks good. Only two comments from me:
>>>
>>> 1. FederationManager.getFederationProvider uses factories directly
>>>
>>> Why is this? This will cause the provider instance not to be registered
>>> with the session, so won't be closed automatically when the session is
>>> closed, nor will it be able to attach to the transaction.
>>>
>>
>> FederationProviders are not singletons within KeycloakSession like
>> RealmProvider, UserProvider, and UserSessionProvider are.  You can have
>> multiple FederationProvider instances per realm.
>>
>>
>> public interface FederationProviderFactory extends
>> ProviderFactory<FederationProvider> {
>>       FederationProvider getInstance(KeycloakSession session,
>> FederationProviderModel model);
>> }
>>
>>
>> FederationProviders implementations have the option to enlist themselves
>> with the KeycloakSession transaction manager.  LDAP (really Picketlink)
>> doesn't have the concept of a session or transaction, so it doesn't
>> enlist itself.
>>
>> I guess we do need a enlistForClose() method on KeycloakSession though.
>
> As long as the lifespan of a FederationProvider is the same as the KeycloakSession they should be created through the KeycloakSession IMO. We just need to add an additional createProvider that can create a instance tied to a specific realm.
>

I disagree.  Its really up to the implementation on whether the provider 
is managed by KeycloakSession or not or whether its lifespan is tied to 
KeycloakSession or not.

We already manage transactions in this manner.  For example 
JpaUserProvider does not enlist its own KeycloakTransaction and its 
close() method is a no-op.  CacheUserProvider and JpaConnectionProvider 
enlist their own transaction objects manually.  JpaConnectionProvider's 
close() method actually does something.

-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com