Refactor test
Project: http://git-wip-us.apache.org/repos/asf/karaf/repo Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/01d0aae9 Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/01d0aae9 Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/01d0aae9 Branch: refs/heads/model_features Commit: 01d0aae9b62531919f00d96eb22119bf59d0209b Parents: 713d25d Author: Christian Schneider <ch...@die-schneider.net> Authored: Wed Aug 16 11:12:46 2017 +0200 Committer: Christian Schneider <ch...@die-schneider.net> Committed: Wed Aug 16 11:12:46 2017 +0200 ---------------------------------------------------------------------- jaas/modules/pom.xml | 6 + .../karaf/jaas/modules/PrincipalAssert.java | 40 +++ .../properties/PropertiesBackingEngineTest.java | 266 +++++++------------ .../properties/PropertiesLoginModuleTest.java | 42 +-- 4 files changed, 146 insertions(+), 208 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/karaf/blob/01d0aae9/jaas/modules/pom.xml ---------------------------------------------------------------------- diff --git a/jaas/modules/pom.xml b/jaas/modules/pom.xml index 8313005..b6be3d8 100644 --- a/jaas/modules/pom.xml +++ b/jaas/modules/pom.xml @@ -120,6 +120,12 @@ <version>${derby-version}</version> <scope>test</scope> </dependency> + <dependency> + <groupId>org.hamcrest</groupId> + <artifactId>hamcrest-all</artifactId> + <version>1.3</version> + <scope>test</scope> + </dependency> </dependencies> <build> http://git-wip-us.apache.org/repos/asf/karaf/blob/01d0aae9/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/PrincipalAssert.java ---------------------------------------------------------------------- diff --git a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/PrincipalAssert.java b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/PrincipalAssert.java new file mode 100644 index 0000000..c19fd2b --- /dev/null +++ b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/PrincipalAssert.java @@ -0,0 +1,40 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * under the License. + */ +package org.apache.karaf.jaas.modules; + +import static java.util.stream.Collectors.toList; + +import java.security.Principal; +import java.util.List; +import java.util.stream.Collectors; + +import javax.security.auth.Subject; + +import org.junit.Assert; + +public class PrincipalAssert { + + public static List<String> names(List<? extends Principal> principals) { + return principals.stream().map(r->r.getName()).collect(toList()); + } + + public static void assertPrincipalNamed(Subject subject, Class<? extends Principal> clazz, String expectedName) { + Long numMatching = subject.getPrincipals(clazz).stream() + .filter(pr -> expectedName.equals(pr.getName())) + .collect(Collectors.counting()); + Assert.assertEquals("Expected " + clazz.getSimpleName() + " principal in subject with name=" + expectedName, + 1l, numMatching.intValue()); + } +} http://git-wip-us.apache.org/repos/asf/karaf/blob/01d0aae9/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngineTest.java ---------------------------------------------------------------------- diff --git a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngineTest.java b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngineTest.java index b781cd1..0c811da 100644 --- a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngineTest.java +++ b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngineTest.java @@ -16,188 +16,104 @@ */ package org.apache.karaf.jaas.modules.properties; -import junit.framework.TestCase; +import static org.apache.karaf.jaas.modules.PrincipalAssert.names; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.io.File; +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + import org.apache.felix.utils.properties.Properties; import org.apache.karaf.jaas.boot.principal.GroupPrincipal; -import org.apache.karaf.jaas.boot.principal.RolePrincipal; import org.apache.karaf.jaas.boot.principal.UserPrincipal; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; -import java.io.File; -import java.io.IOException; +public class PropertiesBackingEngineTest { + private File f; -public class PropertiesBackingEngineTest extends TestCase { + @Before + public void start() throws IOException { + f = File.createTempFile(getClass().getName(), ".tmp"); + } + @Test public void testUserRoles() throws IOException { - File f = File.createTempFile(getClass().getName(), ".tmp"); - try { - Properties p = new Properties(f); - - PropertiesBackingEngine engine = new PropertiesBackingEngine(p); - engine.addUser("a", "aa"); - assertEquals(1, engine.listUsers().size()); - UserPrincipal upa = engine.listUsers().iterator().next(); - assertEquals("a", upa.getName()); - engine.addUser("b", "bb"); - - engine.addRole("a", "role1"); - engine.addRole("a", "role2"); - assertEquals(2, engine.listRoles(upa).size()); - - boolean foundR1 = false; - boolean foundR2 = false; - for (RolePrincipal rp : engine.listRoles(upa)) { - if ("role1".equals(rp.getName())) { - foundR1 = true; - } else if ("role2".equals(rp.getName())) { - foundR2 = true; - } - } - assertTrue(foundR1); - assertTrue(foundR2); - - engine.addGroup("a", "g"); - engine.addGroupRole("g", "role2"); - engine.addGroupRole("g", "role3"); - engine.addGroup("b", "g"); - engine.addGroup("b", "g2"); - engine.addGroupRole("g2", "role4"); - - assertEquals(2, engine.listUsers().size()); - UserPrincipal upa_1 = null; - UserPrincipal upb_1 = null; - for (UserPrincipal u : engine.listUsers()) { - if ("a".equals(u.getName())) { - upa_1 = u; - } else if ("b".equals(u.getName())) { - upb_1 = u; - } - } - assertNotNull(upa_1); - assertNotNull(upb_1); - - assertEquals(3, engine.listRoles(upa).size()); - boolean foundR1_2 = false; - boolean foundR2_2 = false; - boolean foundR3_2 = false; - for (RolePrincipal rp : engine.listRoles(upa)) { - if ("role1".equals(rp.getName())) { - foundR1_2 = true; - } else if ("role2".equals(rp.getName())) { - foundR2_2 = true; - } else if ("role3".equals(rp.getName())) { - foundR3_2 = true; - } - } - assertTrue(foundR1_2); - assertTrue(foundR2_2); - assertTrue(foundR3_2); - - // check that the loading works - PropertiesBackingEngine engine2 = new PropertiesBackingEngine(new Properties(f)); - assertEquals(2, engine2.listUsers().size()); - UserPrincipal upa_2 = null; - UserPrincipal upb_2 = null; - for (UserPrincipal u : engine2.listUsers()) { - if ("a".equals(u.getName())) { - upa_2 = u; - } else if ("b".equals(u.getName())) { - upb_2 = u; - } - } - assertNotNull(upa_2); - assertNotNull(upb_2); - - assertEquals(3, engine2.listRoles(upa_2).size()); - boolean foundR1_3 = false; - boolean foundR2_3 = false; - boolean foundR3_3 = false; - for (RolePrincipal rp : engine2.listRoles(upa_2)) { - if ("role1".equals(rp.getName())) { - foundR1_3 = true; - } else if ("role2".equals(rp.getName())) { - foundR2_3 = true; - } else if ("role3".equals(rp.getName())) { - foundR3_3 = true; - } - } - assertTrue(foundR1_3); - assertTrue(foundR2_3); - assertTrue(foundR3_3); - - assertEquals(3, engine2.listRoles(upb_2).size()); - boolean foundR2_4 = false; - boolean foundR3_4 = false; - boolean foundR4_4 = false; - for (RolePrincipal rp : engine2.listRoles(upb_2)) { - if ("role2".equals(rp.getName())) { - foundR2_4 = true; - } else if ("role3".equals(rp.getName())) { - foundR3_4 = true; - } else if ("role4".equals(rp.getName())) { - foundR4_4 = true; - } - } - assertTrue(foundR2_4); - assertTrue(foundR3_4); - assertTrue(foundR4_4); - - // removing some stuff - UserPrincipal upb = null; - for (UserPrincipal up : engine.listUsers()) { - if ("b".equals(up.getName())) { - upb = up; - } - } - assertEquals(1, engine.listGroups(upa).size()); - assertEquals(2, engine.listGroups(upb).size()); - - GroupPrincipal gp = engine.listGroups(upa).iterator().next(); - engine.deleteGroupRole("g", "role2"); - assertEquals(1, engine.listRoles(gp).size()); - assertEquals("role3", engine.listRoles(gp).iterator().next().getName()); - - // check that the user roles are reported correctly - assertEquals("role2 should still be there as it was added to the user directly too", 3, engine.listRoles(upa).size()); - boolean foundR1_5 = false; - boolean foundR2_5 = false; - boolean foundR3_5 = false; - for (RolePrincipal rp : engine.listRoles(upa)) { - if ("role1".equals(rp.getName())) { - foundR1_5 = true; - } else if ("role2".equals(rp.getName())) { - foundR2_5 = true; - } else if ("role3".equals(rp.getName())) { - foundR3_5 = true; - } - } - assertTrue(foundR1_5); - assertTrue(foundR2_5); - assertTrue(foundR3_5); - - assertEquals(2, engine.listRoles(upb).size()); - boolean foundR3_6 = false; - boolean foundR4_6 = false; - for (RolePrincipal rp : engine.listRoles(upb)) { - if ("role3".equals(rp.getName())) { - foundR3_6 = true; - } else if ("role4".equals(rp.getName())) { - foundR4_6 = true; - } - } - assertTrue(foundR3_6); - assertTrue(foundR4_6); - - engine.deleteGroup("b", "g"); - engine.deleteGroup("b", "g2"); - assertEquals(0, engine.listRoles(upb).size()); - - engine.deleteUser("b"); - engine.deleteUser("a"); - assertEquals("Properties should be empty now", 0, p.size()); - } finally { - if (!f.delete()) { - fail("Could not delete temporary file: " + f); - } + Properties p = new Properties(f); + + PropertiesBackingEngine engine = new PropertiesBackingEngine(p); + engine.addUser("a", "aa"); + engine.addUser("b", "bb"); + + engine.addRole("a", "role1"); + engine.addRole("a", "role2"); + + UserPrincipal upa = getUser(engine, "a"); + Assert.assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2")); + + engine.addGroup("a", "g"); + engine.addGroupRole("g", "role2"); + engine.addGroupRole("g", "role3"); + engine.addGroup("b", "g"); + engine.addGroup("b", "g2"); + engine.addGroupRole("g2", "role4"); + + Assert.assertThat(names(engine.listUsers()), containsInAnyOrder("a", "b")); + Assert.assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2", "role3")); + + checkLoading(); + + // removing some stuff + UserPrincipal upb = getUser(engine, "b"); + assertEquals(1, engine.listGroups(upa).size()); + assertEquals(2, engine.listGroups(upb).size()); + + GroupPrincipal gp = engine.listGroups(upa).iterator().next(); + engine.deleteGroupRole("g", "role2"); + Assert.assertThat(names(engine.listRoles(gp)), containsInAnyOrder("role3")); + + // role2 should still be there as it was added to the user directly too + Assert.assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2", "role3")); + Assert.assertThat(names(engine.listRoles(upb)), containsInAnyOrder("role3", "role4")); + + engine.deleteGroup("b", "g"); + engine.deleteGroup("b", "g2"); + assertEquals(0, engine.listRoles(upb).size()); + + engine.deleteUser("b"); + engine.deleteUser("a"); + assertEquals("Properties should be empty now", 0, p.size()); + } + + private void checkLoading() throws IOException { + PropertiesBackingEngine engine = new PropertiesBackingEngine(new Properties(f)); + assertEquals(2, engine.listUsers().size()); + UserPrincipal upa_2 = getUser(engine, "a"); + UserPrincipal upb_2 = getUser(engine, "b"); + + assertEquals(3, engine.listRoles(upa_2).size()); + Assert.assertThat(names(engine.listRoles(upa_2)), containsInAnyOrder("role1", "role2", "role3")); + + assertEquals(3, engine.listRoles(upb_2).size()); + Assert.assertThat(names(engine.listRoles(upb_2)), containsInAnyOrder("role2", "role3", "role4")); + } + + private UserPrincipal getUser(PropertiesBackingEngine engine, String name) { + List<UserPrincipal> matchingUsers = engine.listUsers().stream() + .filter(user->name.equals(user.getName())).collect(Collectors.toList()); + Assert.assertFalse("User with name " + name + " was not found", matchingUsers.isEmpty()); + return matchingUsers.iterator().next(); + } + + @After + public void cleanup() { + if (!f.delete()) { + fail("Could not delete temporary file: " + f); } } http://git-wip-us.apache.org/repos/asf/karaf/blob/01d0aae9/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesLoginModuleTest.java ---------------------------------------------------------------------- diff --git a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesLoginModuleTest.java b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesLoginModuleTest.java index 5d69d20..26d90a7 100644 --- a/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesLoginModuleTest.java +++ b/jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesLoginModuleTest.java @@ -16,14 +16,16 @@ */ package org.apache.karaf.jaas.modules.properties; +import static org.apache.karaf.jaas.modules.PrincipalAssert.assertPrincipalNamed; + import java.io.File; import java.io.IOException; -import java.security.Principal; import java.util.HashMap; import java.util.Map; import javax.security.auth.Subject; -import javax.security.auth.callback.*; +import javax.security.auth.callback.CallbackHandler; +import javax.security.auth.callback.UnsupportedCallbackException; import javax.security.auth.login.FailedLoginException; import javax.security.auth.login.LoginException; @@ -59,19 +61,8 @@ public class PropertiesLoginModuleTest { Assert.assertEquals(2, subject.getPrincipals().size()); - boolean foundUser = false; - boolean foundRole = false; - for (Principal pr : subject.getPrincipals()) { - if (pr instanceof UserPrincipal) { - Assert.assertEquals("abc", pr.getName()); - foundUser = true; - } else if (pr instanceof RolePrincipal) { - Assert.assertEquals("myrole", pr.getName()); - foundRole = true; - } - } - Assert.assertTrue(foundUser); - Assert.assertTrue(foundRole); + assertPrincipalNamed(subject, UserPrincipal.class, "abc"); + assertPrincipalNamed(subject, RolePrincipal.class, "myrole"); Assert.assertTrue(module.logout()); Assert.assertEquals("Principals should be gone as the user has logged out", 0, subject.getPrincipals().size()); @@ -131,24 +122,9 @@ public class PropertiesLoginModuleTest { Assert.assertTrue(module.commit()); Assert.assertEquals(3, subject.getPrincipals().size()); - boolean foundUser = false; - boolean foundRole = false; - boolean foundGroup = false; - for (Principal pr : subject.getPrincipals()) { - if (pr instanceof UserPrincipal) { - Assert.assertEquals("pqr", pr.getName()); - foundUser = true; - } else if (pr instanceof GroupPrincipal) { - Assert.assertEquals("group1", pr.getName()); - foundGroup = true; - } else if (pr instanceof RolePrincipal) { - Assert.assertEquals("r1", pr.getName()); - foundRole = true; - } - } - Assert.assertTrue(foundUser); - Assert.assertTrue(foundGroup); - Assert.assertTrue(foundRole); + assertPrincipalNamed(subject, UserPrincipal.class, "pqr"); + assertPrincipalNamed(subject, GroupPrincipal.class, "group1"); + assertPrincipalNamed(subject, RolePrincipal.class, "r1"); } finally { if (!f.delete()) { Assert.fail("Could not delete temporary file: " + f);