Hi everyone! I decided to implement KEYCLOAK-1797 and started to look at the code (federation/ldap). I noticed lack of unit tests without which refactoring may be very error prone. I like writing test so I can write tests for that part. What are you thinking about it?? Best Regards, Andrzej _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

Hi Marek, Thanks for reply! I saw those test, but personally I prefer unit tests over integrated tests:) I really recommend this: https://vimeo.com/80533536 Best Regards, Andrzej 2015-10-26 8:41 GMT+01:00 Marek Posolda <mposolda(a)redhat.com <mailto:mposolda@redhat.com>>: Hi, most of the tests are in the testsuite/integration or testsuite/integration-arquillian, not in the modules itself. For the federation and ldap, you can take a look especially to package org.keycloak.testsuite.federation . Marek On 25/10/15 21:51, Andrzej Goławski wrote: > Hi everyone! > > I decided to implement KEYCLOAK-1797 and started to look at the > code (federation/ldap). I noticed lack of unit tests without > which refactoring may be very error prone. I like writing test so > I can write tests for that part. What are you thinking about it?? > > Best Regards, > Andrzej > > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/keycloak-dev

"For example for federation, you need realm and user DB and you need to have realm configured with federation Provider" In such cases you can use mocks, stubs and object factories. "There is not much you can test within single module" IMHO there are still few things which would be nice to test. Look at the first class in the module LDAP Config for example. There are few comments suggesting refactoring it in the feature. I find refactoring single class or classes with heavy integration test painful and insufficient. Look at spring framework code. There are plenty of small unit tests which test only one thing so that it is really well tested as a whole! I think good testing is especially important in case of open source - where everybody adds some code. For instance me :) For me LDAP it is a new topic, but I would like to add some code to this part ... so I expect to make o a lot of mistakes :D "Btv. what's your plan for KEYCLOAK-1797" And now the hardest part :) As I said, I'm new in this topic (LDAP) so I decided to wrap my head around it for a while - can you reccommend me any reading materials suitable for beginners? "And in your LDAP environment, is it often that new role is added as member to some other roles?" No .. but it is critical in my company. "I wonder if we need to always do "deep" search in runtime, or if we can instead do it just at some point and rely on Keycloak composite roles . If you always need deep search and do something based on it, it will be good to have a flag in configuration, which will allow to disable it (for performance reasons)." Thank you for the hint :) I couldn't agree more. Best regards, Andrzej 2015-10-26 14:11 GMT+01:00 Marek Posolda <mposolda(a)redhat.com <mailto:mposolda@redhat.com>>: We prefer integration tests as you usually need more things to have available. For example for federation, you need realm and user DB and you need to have realm configured with federationProvider. There is not much you can test within single module, so we have just very simple tests in individual modules (like LDAPDnTest or PasswordPolicyTest), but most of the stuff is tested in testsuite. For KEYCLOAK-1797 I prefer to take a look at LDAPRoleMappingsTest and possibly add new test methods here. Btv. what's your plan for KEYCLOAK-1797 ? And in your LDAP environment, is it often that new role is added as member to some other roles? I wonder if we need to always do "deep" search in runtime, or if we can instead do it just at some point and rely on Keycloak composite roles . If you always need deep search and do something based on it, it will be good to have a flag in configuration, which will allow to disable it (for performance reasons). Marek On 26/10/15 08:55, Andrzej Go?awski wrote: > Hi Marek, > > Thanks for reply! > I saw those test, but personally I prefer unit tests over > integrated tests:) > I really recommend this: https://vimeo.com/80533536 > > Best Regards, > Andrzej > > 2015-10-26 8:41 GMT+01:00 Marek Posolda <mposolda(a)redhat.com > <mailto:mposolda@redhat.com>>: > > Hi, > > most of the tests are in the testsuite/integration or > testsuite/integration-arquillian, not in the modules itself. > For the federation and ldap, you can take a look especially > to package org.keycloak.testsuite.federation . > > Marek > > On 25/10/15 21:51, Andrzej Go?awski wrote: >> Hi everyone! >> >> I decided to implement KEYCLOAK-1797 and started to look at >> the code (federation/ldap). I noticed lack of unit tests >> without which refactoring may be very error prone. I like >> writing test so I can write tests for that part. What are >> you thinking about it?? >> >> Best Regards, >> Andrzej >> >> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

Integration tests have many disguises, some are good others are bad. As Keycloak is not a framework or set of libraries, but a ready to use service it's a lot more important that we test the real thing than test individual bits and pieces. I agree it wouldn't hurt to have unit tests as well, but they are generally a pain to maintain. Especially if you want to refactor something. Mocks make things even worse in that regard. Personally I see unit tests and mocks more as a great development tool than actual quality assurance. End of the day what you care about is that your login page and your rest services behaves as expected there's just no need for unit tests. On 27 October 2015 at 04:26, Stan Silvert <ssilvert(a)redhat.com&gt; wrote: > You should never use mocks again. I elaborated on this topic in Bill's > blog several years ago and he wrote a followup article: > http://bill.burkecentral.com/2012/05/01/mocks-are-a-mockery/ > > > On 10/27/2015 4:41 AM, Andrzej Goławski wrote: > > "For example for federation, you need realm and user DB and you need to > have realm configured with federation Provider" > > In such cases you can use mocks, stubs and object factories. > > "There is not much you can test within single module" > > IMHO there are still few things which would be nice to test. Look at the > first class in the module LDAP Config for example. There are few comments > suggesting refactoring it in the feature. I find refactoring single class > or classes with heavy integration test painful and insufficient. Look at > spring framework code. There are plenty of small unit tests which test only > one thing so that it is really well tested as a whole! I think good testing > is especially important in case of open source - where everybody adds some > code. For instance me :) For me LDAP it is a new topic, but I would like to > add some code to this part ... so I expect to make o a lot of mistakes :D > "Btv. what's your plan for KEYCLOAK-1797" > And now the hardest part :) As I said, I'm new in this topic (LDAP) so I > decided to wrap my head around it for a while - can you reccommend me any > reading materials suitable for beginners? > > "And in your LDAP environment, is it often that new role is added as > member to some other roles?" > > No .. but it is critical in my company. > > "I wonder if we need to always do "deep" search in runtime, or if we can > instead do it just at some point and rely on Keycloak composite roles . If > you always need deep search and do something based on it, it will be good > to have a flag in configuration, which will allow to disable it (for > performance reasons)." > > Thank you for the hint :) I couldn't agree more. > > Best regards, > > Andrzej > > 2015-10-26 14:11 GMT+01:00 Marek Posolda <mposolda(a)redhat.com&gt;: > >> We prefer integration tests as you usually need more things to have >> available. For example for federation, you need realm and user DB and you >> need to have realm configured with federationProvider. There is not much >> you can test within single module, so we have just very simple tests in >> individual modules (like LDAPDnTest or PasswordPolicyTest), but most of the >> stuff is tested in testsuite. For KEYCLOAK-1797 I prefer to take a look at LDAPRoleMappingsTest >> and possibly add new test methods here. >> >> Btv. what's your plan for KEYCLOAK-1797 ? And in your LDAP environment, >> is it often that new role is added as member to some other roles? I wonder >> if we need to always do "deep" search in runtime, or if we can instead do >> it just at some point and rely on Keycloak composite roles . If you always >> need deep search and do something based on it, it will be good to have a >> flag in configuration, which will allow to disable it (for performance >> reasons). >> >> Marek >> >> On 26/10/15 08:55, Andrzej Goławski wrote: >> >> Hi Marek, >> >> Thanks for reply! >> I saw those test, but personally I prefer unit tests over integrated >> tests:) >> I really recommend this: https://vimeo.com/80533536 >> >> Best Regards, >> Andrzej >> >> 2015-10-26 8:41 GMT+01:00 Marek Posolda <mposolda(a)redhat.com&gt;: >> >>> Hi, >>> >>> most of the tests are in the testsuite/integration or >>> testsuite/integration-arquillian, not in the modules itself. For the >>> federation and ldap, you can take a look especially to package >>> org.keycloak.testsuite.federation . >>> >>> Marek >>> >>> On 25/10/15 21:51, Andrzej Goławski wrote: >>> >>> Hi everyone! >>> >>> I decided to implement KEYCLOAK-1797 and started to look at the code >>> (federation/ldap). I noticed lack of unit tests without which refactoring >>> may be very error prone. I like writing test so I can write tests for that >>> part. What are you thinking about it?? >>> >>> Best Regards, >>> Andrzej >>> >>> >>> _______________________________________________ >>> keycloak-dev mailing listkeycloak-dev@lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev >>> >>> >>> >> >> > > > _______________________________________________ > keycloak-dev mailing listkeycloak-dev@lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev > > > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

"For example for federation, you need realm and user DB and you need to have realm configured with federation Provider" In such cases you can use mocks, stubs and object factories. "There is not much you can test within single module" IMHO there are still few things which would be nice to test. Look at the first class in the module LDAP Config for example. There are few comments suggesting refactoring it in the feature. I find refactoring single class or classes with heavy integration test painful and insufficient. Look at spring framework code. There are plenty of small unit tests which test only one thing so that it is really well tested as a whole! I think good testing is especially important in case of open source - where everybody adds some code. For instance me :) For me LDAP it is a new topic, but I would like to add some code to this part ... so I expect to make o a lot of mistakes :D "Btv. what's your plan for KEYCLOAK-1797" And now the hardest part :) As I said, I'm new in this topic (LDAP) so I decided to wrap my head around it for a while - can you reccommend me any reading materials suitable for beginners?

"And in your LDAP environment, is it often that new role is added as member to some other roles?" No .. but it is critical in my company. "I wonder if we need to always do "deep" search in runtime, or if we can instead do it just at some point and rely on Keycloak composite roles . If you always need deep search and do something based on it, it will be good to have a flag in configuration, which will allow to disable it (for performance reasons)." Thank you for the hint :) I couldn't agree more. Best regards, Andrzej 2015-10-26 14:11 GMT+01:00 Marek Posolda <mposolda(a)redhat.com <mailto:mposolda@redhat.com>>: We prefer integration tests as you usually need more things to have available. For example for federation, you need realm and user DB and you need to have realm configured with federationProvider. There is not much you can test within single module, so we have just very simple tests in individual modules (like LDAPDnTest or PasswordPolicyTest), but most of the stuff is tested in testsuite. For KEYCLOAK-1797 I prefer to take a look at LDAPRoleMappingsTest and possibly add new test methods here. Btv. what's your plan for KEYCLOAK-1797 ? And in your LDAP environment, is it often that new role is added as member to some other roles? I wonder if we need to always do "deep" search in runtime, or if we can instead do it just at some point and rely on Keycloak composite roles . If you always need deep search and do something based on it, it will be good to have a flag in configuration, which will allow to disable it (for performance reasons). Marek On 26/10/15 08:55, Andrzej Goławski wrote: > Hi Marek, > > Thanks for reply! > I saw those test, but personally I prefer unit tests over > integrated tests:) > I really recommend this: https://vimeo.com/80533536 > > Best Regards, > Andrzej > > 2015-10-26 8:41 GMT+01:00 Marek Posolda <mposolda(a)redhat.com > <mailto:mposolda@redhat.com>>: > > Hi, > > most of the tests are in the testsuite/integration or > testsuite/integration-arquillian, not in the modules itself. > For the federation and ldap, you can take a look especially > to package org.keycloak.testsuite.federation . > > Marek > > On 25/10/15 21:51, Andrzej Goławski wrote: >> Hi everyone! >> >> I decided to implement KEYCLOAK-1797 and started to look at >> the code (federation/ldap). I noticed lack of unit tests >> without which refactoring may be very error prone. I like >> writing test so I can write tests for that part. What are >> you thinking about it?? >> >> Best Regards, >> Andrzej >> >> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)lists.jboss.org >> <mailto:keycloak-dev@lists.jboss.org> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > >

Thank you for materials and lots of hints !! :) "Thing is that Bill is actually working on adding Groups support to Keycloak and composite roles are going to be refactored or removed entirely and replaced by groups" Does it gonna be done with one commit, or maybe there are some ready to use parts in the model?

"Is it ok for you to wait for 1.7 release?" It will have to :) Best Regards, Andrzej 2015-10-27 22:11 GMT+01:00 Marek Posolda <mposolda(a)redhat.com <mailto:mposolda@redhat.com>>: On 27/10/15 09:41, Andrzej Goławski wrote: > "For example for federation, you need realm and user DB and you > need to have realm configured with federation Provider" > > In such cases you can use mocks, stubs and object factories. > > "There is not much you can test within single module" > > IMHO there are still few things which would be nice to test. Look > at the first class in the module LDAP Config for example. There > are few comments suggesting refactoring it in the feature. I find > refactoring single class or classes with heavy integration test > painful and insufficient. Look at spring framework code. There > are plenty of small unit tests which test only one thing so that > it is really well tested as a whole! I think good testing is > especially important in case of open source - where everybody > adds some code. For instance me :) For me LDAP it is a new topic, > but I would like to add some code to this part ... so I expect to > make o a lot of mistakes :D > "Btv. what's your plan for KEYCLOAK-1797" > And now the hardest part :) As I said, I'm new in this topic > (LDAP) so I decided to wrap my head around it for a while - can > you reccommend me any reading materials suitable for beginners? You can start with some generic Java + LDAP tutorial: https://docs.oracle.com/javase/tutorial/jndi/ops/index.html Then you can take a look at Keycloak Federation and LDAP documentation and to the code itself. And also I suggest to take a look at Keycloak composite roles. Personally I would like to avoid doing "deep" search at every request, but instead do the deep search just from time to time and use the keycloak composite roles. Method RoleLDAPFederationMapper.syncRolesFromLDAP is currently checking LDAP and it's syncing roles from LDAP into Keycloak DB. This method is not called at every request but just at some moments (For example during user's sync). This method can be extended to also do the "deep" search and assign composite roles to Keycloak based on LDAP memberships. In your example in JIRA, the role "Group1" wil be put as composite role of "Group1.1" in Keycloak DB. Then during normal user search, just the simple search is performed so just the LDAP role "Group1.1" is returned from the LDAP search as role of user TestUser. But Keycloak will treat the user to be member of role "Group1" as well because this role is composite role of "Group1.1" . So the final result is, that keycloak will treat "TestUser" to be member of both "Group1" and "Group1.1" . Thing is that Bill is actually working on adding Groups support to Keycloak and composite roles are going to be refactored or removed entirely and replaced by groups. So there is not very good time to introduce this feature now, but rather wait for 1.7 release once Group support is in. Is it ok for you to wait for 1.7 release? Marek > > "And in your LDAP environment, is it often that new role is added > as member to some other roles?" > > No .. but it is critical in my company. > > "I wonder if we need to always do "deep" search in runtime, or if > we can instead do it just at some point and rely on Keycloak > composite roles . If you always need deep search and do something > based on it, it will be good to have a flag in configuration, > which will allow to disable it (for performance reasons)." > > Thank you for the hint :) I couldn't agree more. > > Best regards, > > Andrzej > > 2015-10-26 14:11 GMT+01:00 Marek Posolda <mposolda(a)redhat.com > <mailto:mposolda@redhat.com>>: > > We prefer integration tests as you usually need more things > to have available. For example for federation, you need realm > and user DB and you need to have realm configured with > federationProvider. There is not much you can test within > single module, so we have just very simple tests in > individual modules (like LDAPDnTest or PasswordPolicyTest), > but most of the stuff is tested in testsuite. For > KEYCLOAK-1797 I prefer to take a look at LDAPRoleMappingsTest > and possibly add new test methods here. > > Btv. what's your plan for KEYCLOAK-1797 ? And in your LDAP > environment, is it often that new role is added as member to > some other roles? I wonder if we need to always do "deep" > search in runtime, or if we can instead do it just at some > point and rely on Keycloak composite roles . If you always > need deep search and do something based on it, it will be > good to have a flag in configuration, which will allow to > disable it (for performance reasons). > > Marek > > On 26/10/15 08:55, Andrzej Goławski wrote: >> Hi Marek, >> >> Thanks for reply! >> I saw those test, but personally I prefer unit tests over >> integrated tests:) >> I really recommend this: https://vimeo.com/80533536 >> >> Best Regards, >> Andrzej >> >> 2015-10-26 8:41 GMT+01:00 Marek Posolda <mposolda(a)redhat.com >> <mailto:mposolda@redhat.com>>: >> >> Hi, >> https://www.linkedin.com/comm/profile/view?id=AAsAAAIX5nMBKyxtfQeuzKzJrFF... >> most of the tests are in the testsuite/integration or >> testsuite/integration-arquillian, not in the modules >> itself. For the federation and ldap, you can take a look >> especially to package org.keycloak.testsuite.federation . >> >> Marek >> >> On 25/10/15 21:51, Andrzej Goławski wrote: >>> Hi everyone! >>> >>> I decided to implement KEYCLOAK-1797 and started to >>> look at the code (federation/ldap). I noticed lack of >>> unit tests without which refactoring may be very error >>> prone. I like writing test so I can write tests for >>> that part. What are you thinking about it?? >>> >>> Best Regards, >>> Andrzej >>> >>> >>> _______________________________________________ >>> keycloak-dev mailing list >>> keycloak-dev(a)lists.jboss.org >>> <mailto:keycloak-dev@lists.jboss.org> >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> >> > >