[keycloak-dev] federation commited need feedback

[Marek Posolda]

On 24.7.2014 21:13, Bill Burke wrote:
>
>
> On 7/24/2014 1:42 PM, Marek Posolda wrote:
>> Right now what I am doing for export/import is creating session just for
>> retrieving ExportProvider or ImportProvider and then particular
>> ExportProvider or ImportProvider is starting it's own
>> session/transaction lifecycles whenever it needs them -
>> https://github.com/keycloak/keycloak/blob/master/export-import/export-import-api/src/main/java/org/keycloak/exportimport/ExportImportManager.java#L34 
>>
>> . It's kind of a hack IMO...
>>
>
> KeycloakSessionFactory now has:
>
>     <T extends Provider> ProviderFactory<T> 
> getProviderFactory(Class<T> clazz);
>     <T extends Provider> ProviderFactory<T> 
> getProviderFactory(Class<T> clazz, String id);
>
> So you could use them instead of creating a session.
yes, however ProviderFactory has "create" method, which wants 
KeycloakSession... IMO couple ProviderFactory/Provider works well for 
providers, which are supposed to be session scoped. For application 
scoped providers, which doesn't need KeycloakSession, it would work 
better if these can be retrieved directly from KeycloakSessionFactory 
and there is not couple but just "global provider" like this:


public interface GlobalProvider {

   public void init(Config.Scope config, KeycloakSessionFactory 
sessionFactory);
   public void close();
   public String getId();

}


Some other example from current code is TimerProvider, which actually 
creates KeycloakSession just for the purpose of retrieve the current 
instance of TimerProvider: 
https://github.com/keycloak/keycloak/blob/master/services/src/main/java/org/keycloak/services/resources/KeycloakApplication.java#L147 
. This is similar case as IMO TimerProvider component is not supposed to 
be session scoped but application scoped.
> Maybe there should be a separate FactoryFinder interface? If you want 
> to pull those methods into a FactoryFinder interface I guess that 
> could be done.
>
> Honestly, I don't understand why we need all these component layers 
> for export/import.  Seems a bit over-engineered in that regards, IMO, 
> and now you want to add even more component layers to manage something 
> that should be pretty straightforward.
Actually I want that to be simpler instead:-)
Instead of ProviderFactory/Provider couple, it would need just single 
GlobalProvider to avoid boilerplate code needed for ProviderFactory and 
creating KeycloakSession instance just for retrieve Provider.

Actually export/import supports exporting to zip, single JSON file or 
directory and it suits well to handle it with different providers IMO.

Marek