From 5f29c02ce678cd168f21eb0cb00aa55edab2d9f3 Mon Sep 17 00:00:00 2001
From: "David M. Carr" <david@carrclan.us>
Date: Fri, 7 Jan 2011 17:11:24 -0500
Subject: [PATCH] Enhance unit test for PermissionUtil to cover more group
 permission scenarios. Reduce duplication of code in Permission constructors.

---
 .../java/sonia/scm/repository/Permission.java | 10 ++--
 .../scm/repository/PermissionUtilTest.java    | 49 +++++++++++++------
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/scm-core/src/main/java/sonia/scm/repository/Permission.java b/scm-core/src/main/java/sonia/scm/repository/Permission.java
index 50305239d1..a446717cb9 100644
--- a/scm-core/src/main/java/sonia/scm/repository/Permission.java
+++ b/scm-core/src/main/java/sonia/scm/repository/Permission.java
@@ -69,6 +69,7 @@ public class Permission implements Serializable
    */
   public Permission(String name)
   {
+    this();
     this.name = name;
   }
 
@@ -81,7 +82,7 @@ public class Permission implements Serializable
    */
   public Permission(String name, PermissionType type)
   {
-    this.name = name;
+    this(name);
     this.type = type;
   }
 
@@ -90,14 +91,13 @@ public class Permission implements Serializable
    *
    *
    * @param name
-   * @param groupPermission
    * @param type
+   * @param groupPermission
    */
-  public Permission(String name, boolean groupPermission, PermissionType type)
+  public Permission(String name, PermissionType type, boolean groupPermission)
   {
-    this.name = name;
+    this(name, type);
     this.groupPermission = groupPermission;
-    this.type = type;
   }
 
   //~--- get methods ----------------------------------------------------------
diff --git a/scm-core/src/test/java/sonia/scm/repository/PermissionUtilTest.java b/scm-core/src/test/java/sonia/scm/repository/PermissionUtilTest.java
index 5b56d42f62..2ebb29622e 100644
--- a/scm-core/src/test/java/sonia/scm/repository/PermissionUtilTest.java
+++ b/scm-core/src/test/java/sonia/scm/repository/PermissionUtilTest.java
@@ -118,21 +118,44 @@ public class PermissionUtilTest
   @Test
   public void testGroupPermissions()
   {
-    WebSecurityContext context = mockGroupCtx(new User("dent", "Arthur Dent",
-                                   "arthur.dent@hitchhiker.com"));
+    WebSecurityContext dent = mockGroupCtx(new User("dent", "Arthur Dent",
+                                   "arthur.dent@hitchhiker.com"),
+                                   "devel", "qa");
+    WebSecurityContext ford = mockGroupCtx(new User("ford", "Ford Prefect",
+                                   "ford.prefect@hitchhiker.com"), "devel");
+    WebSecurityContext zaphod = mockGroupCtx(new User("zaphod",
+                                   "Zaphod Beeblebrox",
+                                   "zaphod.beeblebrox@hitchhiker.com"), "qa");
+    WebSecurityContext trillian = mockGroupCtx(new User("trillian",
+                                   "Trillian Astra",
+                                   "trillian.astra@hitchhiker.com"));
     Repository r = new Repository();
 
     r.setPermissions(
         new ArrayList<Permission>(
             Arrays.asList(
               new Permission("dent"),
-              new Permission("devel", true, PermissionType.READ),
-              new Permission("qa", true, PermissionType.WRITE))));
-    assertTrue(PermissionUtil.hasPermission(r, context, PermissionType.READ));
-    assertTrue(PermissionUtil.hasPermission(r, context, PermissionType.WRITE));
-    assertFalse(PermissionUtil.hasPermission(r, context, PermissionType.OWNER));
+              new Permission("devel", PermissionType.WRITE, true),
+              new Permission("qa", PermissionType.READ, true))));
+    // member of both devel and qa
+    assertTrue(PermissionUtil.hasPermission(r, dent, PermissionType.READ));
+    assertTrue(PermissionUtil.hasPermission(r, dent, PermissionType.WRITE));
+    assertFalse(PermissionUtil.hasPermission(r, dent, PermissionType.OWNER));
+    // now, additionally the owner
     r.getPermissions().add(new Permission("dent", PermissionType.OWNER));
-    assertTrue(PermissionUtil.hasPermission(r, context, PermissionType.OWNER));
+    assertTrue(PermissionUtil.hasPermission(r, dent, PermissionType.OWNER));
+    // member of just devel
+    assertTrue(PermissionUtil.hasPermission(r, ford, PermissionType.READ));
+    assertTrue(PermissionUtil.hasPermission(r, ford, PermissionType.WRITE));
+    assertFalse(PermissionUtil.hasPermission(r, ford, PermissionType.OWNER));
+    // member of just qa
+    assertTrue(PermissionUtil.hasPermission(r, zaphod, PermissionType.READ));
+    assertFalse(PermissionUtil.hasPermission(r, zaphod, PermissionType.WRITE));
+    assertFalse(PermissionUtil.hasPermission(r, zaphod, PermissionType.OWNER));
+    // member of no groups
+    assertFalse(PermissionUtil.hasPermission(r, trillian, PermissionType.READ));
+    assertFalse(PermissionUtil.hasPermission(r, trillian, PermissionType.WRITE));
+    assertFalse(PermissionUtil.hasPermission(r, trillian, PermissionType.OWNER));
   }
 
   //~--- get methods ----------------------------------------------------------
@@ -197,14 +220,12 @@ public class PermissionUtilTest
    *
    * @return
    */
-  private WebSecurityContext mockGroupCtx(User user)
+  private WebSecurityContext mockGroupCtx(User user, String... groups)
   {
     WebSecurityContext context = mockCtx(user);
-    Set<String> groups = new HashSet<String>();
-
-    groups.add("devel");
-    groups.add("qa");
-    when(context.getGroups()).thenReturn(groups);
+    
+    Set<String> groupSet = new HashSet<String>(Arrays.asList(groups));
+    when(context.getGroups()).thenReturn(groupSet);
 
     return context;
   }