[keycloak-dev] AuthProvider refactor details

[Marek Posolda]

On 21.7.2014 15:46, Bill Burke wrote:
> for getUsers/search, I can do a union of local storage and external 
> storage. 
hmm... I wonder about the case when you have data like this:
- User "john", which was synced from LDAP to local UserProvider and 
hence now is in both LDAP and local UserProvider
- User "local_foo" which was registered just locally in Keycloak and not 
available in LDAP
- User "ldap_foo" which was recently added directly to LDAP and not yet 
synced in Keycloak ldap UserProvider

So now when admin invokes UserProvider.getUsers() it should return him 3 
users like: "john", "local_foo", "ldap_foo"

But if I understand correctly, with union it returns 4 users like: 
"john", "john", "local_foo", "ldap_foo", because user "john" is in both. 
So how to handle this? Merge both "john" users just during this call? 
Does it scale when we have 10K users in LDAP and 10K users in local 
UserProvider (some shared, some just in either LDAP or local provider)? 
Personally I don't believe that federation will scale well here...
> For getUserCount I can't do this as I would have to iterate over every 
> UserModel, which defeats the purpose of optimized, 1-query, getUserCount.
>
> On 7/21/2014 4:34 AM, Marek Posolda wrote:
>> I am not sure I understand why "getUserCount" is an issue and for
>> example "getUsers" or "searchForUser" is not? Am I understand correctly
>> that implementor of FederationProvider can choose which method to
>> override, so for example if he won't override "getUsers" or
>> "getUserCount" then FederationManager just delegates those to "local"
>> UserProvider?
>>
>> If getUserCount() is an issue, I can remove it and refactor export to
>> not using it and instead use approach like
>
> It is not an issue as only export uses it.  Export will only be 
> exporting local storage.
>
>> : "checkout users until
>> current page is not full", which would mean that there are no more users
>> to export (Like for example if I have 50 users and usersPerPage is 20,
>> then page1 has 20 users, page2 has 20 users and page3 has just 10 users,
>> so export process can recognize that it is last page).
>>
>> But still getUserCount() may be useful if we later want to add
>> pagination support to admin console (in this case, we may need "count"
>> methods also for searchFor** -
>> https://issues.jboss.org/browse/KEYCLOAK-564 )
>>
>
> Don't need a count, just if size() < maxRequested, you know you're at 
> the end.
yeah, it's ok if we support just 3 buttons like "<<"  "<" ">" as it's 
now. If we want to add also ">>" I think we would need to add support 
for counting, so users have possibility to go directly to the last page.

Marek