3:56 a.m.
I've combined KeycloakSession and ProviderSession into KeycloakSession. There's a
single implementation of this (DefaultKeycloakSession).
Model implementations now implement ModelProvider instead of KeycloakSession.
11:25 a.m.
Ugh...Why? Would have been cool to discuss this before you did this.
There was a clear separation of things prior to this change:
* ProviderSession was for resolving various session based APIs I thought.
* KeycloakSession was analogous to EntityManager or HibernateSession.
Also, you don't go committing stuff that breaks existing code...i.e. the
caching stuff I did.
On 6/27/2014 4:56 AM, Stian Thorgersen wrote:
I've combined KeycloakSession and ProviderSession into
KeycloakSession. There's a single implementation of this (DefaultKeycloakSession).
Model implementations now implement ModelProvider instead of KeycloakSession.
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
11:37 a.m.
New subject: Refactoring to KeycloakSession and ProviderSession
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: keycloak-dev(a)lists.jboss.org
Sent: Monday, 30 June, 2014 5:25:31 PM
Subject: Re: [keycloak-dev] Refactoring to KeycloakSession and ProviderSession
Ugh...Why? Would have been cool to discuss this before you did this.
There was a clear separation of things prior to this change:
* ProviderSession was for resolving various session based APIs I thought.
* KeycloakSession was analogous to EntityManager or HibernateSession.
I didn't really think it was a big deal, as the separation is still there. What used
to be KeycloakSession is now ModelProvider, and the new KeycloakSession is just a wrapper
to be able to get to both from the same @Context injection.
Also, you don't go committing stuff that breaks existing code...i.e. the
caching stuff I did.
I commented out the cache stuff as it wasn't working, see
https://issues.jboss.org/browse/KEYCLOAK-527. I tried to fix it, but couldn't quite
figure out how to go about doing it, so thought it was best to disable it until you got
back and could have a look. I was going to send you an email about it, but of course I
forgot that :(
On 6/27/2014 4:56 AM, Stian Thorgersen wrote:
> I've combined KeycloakSession and ProviderSession into KeycloakSession.
> There's a single implementation of this (DefaultKeycloakSession).
>
> Model implementations now implement ModelProvider instead of
> KeycloakSession.
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
11:58 a.m.
On 6/30/2014 12:37 PM, Stian Thorgersen wrote:
----- Original Message -----
> From: "Bill Burke" <bburke(a)redhat.com>
> To: keycloak-dev(a)lists.jboss.org
> Sent: Monday, 30 June, 2014 5:25:31 PM
> Subject: Re: [keycloak-dev] Refactoring to KeycloakSession and ProviderSession
>
> Ugh...Why? Would have been cool to discuss this before you did this.
> There was a clear separation of things prior to this change:
>
> * ProviderSession was for resolving various session based APIs I thought.
> * KeycloakSession was analogous to EntityManager or HibernateSession.
I didn't really think it was a big deal, as the separation is still there. What used
to be KeycloakSession is now ModelProvider, and the new KeycloakSession is just a wrapper
to be able to get to both from the same @Context injection.
KeycloakSession now mixes up the provider API and the data model. What
exactly is the benefit of that? I thought we were moving to
*separating* things out not combining them.. Realm meta-model, User
metamodel, and User Session meta model.
What I'd like to see is ProviderSession becoming a mini-transaction
manager where our Filter calls commit/rollback on the ProviderSession
which in turn, calls commit/rollback on all our model APIs.
>
> Also, you don't go committing stuff that breaks existing code...i.e. the
> caching stuff I did.
I commented out the cache stuff as it wasn't working, see
https://issues.jboss.org/browse/KEYCLOAK-527. I tried to fix it, but couldn't quite
figure out how to go about doing it, so thought it was best to disable it until you got
back and could have a look. I was going to send you an email about it, but of course I
forgot that :(
I guess we don't have any tests for adding/removing scope mappings :(
FYI, it is just a matter of using EntityManager.getReference(). Commit
incoming.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
2:48 a.m.
On 30.6.2014 18:58, Bill Burke wrote:
On 6/30/2014 12:37 PM, Stian Thorgersen wrote:
>
> ----- Original Message -----
>> From: "Bill Burke" <bburke(a)redhat.com>
>> To: keycloak-dev(a)lists.jboss.org
>> Sent: Monday, 30 June, 2014 5:25:31 PM
>> Subject: Re: [keycloak-dev] Refactoring to KeycloakSession and ProviderSession
>>
>> Ugh...Why? Would have been cool to discuss this before you did this.
>> There was a clear separation of things prior to this change:
>>
>> * ProviderSession was for resolving various session based APIs I thought.
>> * KeycloakSession was analogous to EntityManager or HibernateSession.
> I didn't really think it was a big deal, as the separation is still there. What
used to be KeycloakSession is now ModelProvider, and the new KeycloakSession is just a
wrapper to be able to get to both from the same @Context injection.
>
KeycloakSession now mixes up the provider API and the data model. What
exactly is the benefit of that? I thought we were moving to
*separating* things out not combining them.. Realm meta-model, User
metamodel, and User Session meta model.
I wonder if we can abstract the fact that
there are 3 different
meta-models from the code, which consumes it? In other words, still have
same API to invoke the "model" like it currently is (current
KeycloakSession) but have KeycloakSession implementation to just
delegate the invocations to proper meta-models (Realm, User,
UserSession)? This would mean that RealmModelProvider, UserModelProvider
and UserSessionModelProvider will be invoked just from
DefaultKeycloakSession.
This would mean that consuming code will be simpler and doesn't need to
retrieve 3 different providers from KeycloakSession (or
ProviderSession), but just one.
What I'd like to see is ProviderSession becoming a mini-transaction
manager where our Filter calls commit/rollback on the ProviderSession
which in turn, calls commit/rollback on all our model APIs.
+1, I think that it
will be in line with the design of KeycloakSession
of the only externally "consumed" part. So KeycloakSession
begin/commit/rollback will be called by Filter, and it will delegate
calls to begin/commit/rollback to all 3 meta-models.
Separation of "ProviderSession" and "KeycloakSession" makes sense to
me,
but IMO stuff like transaction management belongs more to
KeycloakSession than to ProviderSession.
My vote is for restore previous "ProviderSession" for accessing various
components and rename "KeycloakSession" to something like
"ModelSession"
or "PersistentSession" . KeycloakSession doesn't mean much and it's not
so clear that it's component for dealing with model/persistent stuff.
Marek
>> Also, you don't go committing stuff that breaks existing code...i.e. the
>> caching stuff I did.
> I commented out the cache stuff as it wasn't working, see
https://issues.jboss.org/browse/KEYCLOAK-527. I tried to fix it, but couldn't quite
figure out how to go about doing it, so thought it was best to disable it until you got
back and could have a look. I was going to send you an email about it, but of course I
forgot that :(
>
I guess we don't have any tests for adding/removing scope mappings :(
FYI, it is just a matter of using EntityManager.getReference(). Commit
incoming.
3831
days inactive
3835
days old
4 comments
3 participants
participants (3)
-
Bill Burke -
Marek Posolda -
Stian Thorgersen