From 89660e8ac3327dc30d90794881882a58610fa364 Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Date: Sun, 26 Jun 2016 12:53:41 +0200
Subject: [PATCH] improve cache invalidation on permission change events

---
 .../scm/security/AuthorizationCollector.java  | 18 +++++++++++++----
 .../security/AuthorizationCollectorTest.java  | 20 ++++++++++++++++---
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java b/scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java
index 87803a9dc6..e0e26740cd 100644
--- a/scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java
+++ b/scm-webapp/src/main/java/sonia/scm/security/AuthorizationCollector.java
@@ -184,7 +184,7 @@ public class AuthorizationCollector
   }
   
   private void invalidateUserCache(final String username){
-    logger.debug("invalidate cache of user {}, because user properties have changed", username);
+    logger.debug("invalidate cache of user {}, because of a event which could change the permissions", username);
     cache.removeAll(new Filter<CacheKey>()
     {
       @Override
@@ -248,10 +248,11 @@ public class AuthorizationCollector
   }
 
   /**
-   * Method description
+   * Invalidates the whole cache if a group permission has changed and invalidates the cached entries of a user, if a
+   * user permission has changed.
    *
    *
-   * @param event
+   * @param event permission event
    */
   @Subscribe
   public void onEvent(StoredAssignedPermissionEvent event)
@@ -264,7 +265,16 @@ public class AuthorizationCollector
           event.getPermission().getId());
       }
 
-      cache.clear();
+      StoredAssignedPermission permission = event.getPermission();
+      if (permission.isGroupPermission())
+      {
+        logger.debug("clears the whole cache, because global group permission {} has changed", permission.getId());
+        cache.clear();
+      }
+      else 
+      {
+        invalidateUserCache(permission.getName());
+      }
     }
   }
 
diff --git a/scm-webapp/src/test/java/sonia/scm/security/AuthorizationCollectorTest.java b/scm-webapp/src/test/java/sonia/scm/security/AuthorizationCollectorTest.java
index 75e282fc4f..ea8a280987 100644
--- a/scm-webapp/src/test/java/sonia/scm/security/AuthorizationCollectorTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/security/AuthorizationCollectorTest.java
@@ -211,11 +211,25 @@ public class AuthorizationCollectorTest {
   @Test
   public void testOnStoredAssignedPermissionEvent()
   {
-    StoredAssignedPermission permission = new StoredAssignedPermission();
-    collector.onEvent(new StoredAssignedPermissionEvent(HandlerEvent.BEFORE_CREATE, permission));
+    StoredAssignedPermission groupPermission = new StoredAssignedPermission(
+      "123", new AssignedPermission("_authenticated", true, "repository:read:*")
+    );
+    collector.onEvent(new StoredAssignedPermissionEvent(HandlerEvent.BEFORE_CREATE, groupPermission));
     verify(cache, never()).clear();
     
-    collector.onEvent(new StoredAssignedPermissionEvent(HandlerEvent.CREATE, permission));
+    collector.onEvent(new StoredAssignedPermissionEvent(HandlerEvent.CREATE, groupPermission));
+    verify(cache).clear();
+    
+    
+    StoredAssignedPermission userPermission = new StoredAssignedPermission(
+      "123", new AssignedPermission("trillian", false, "repository:read:*")
+    );
+    collector.onEvent(new StoredAssignedPermissionEvent(HandlerEvent.BEFORE_CREATE, userPermission));
+    verify(cache, never()).removeAll(Mockito.any(Filter.class));
+    verify(cache).clear();
+    
+    collector.onEvent(new StoredAssignedPermissionEvent(HandlerEvent.CREATE, userPermission));
+    verify(cache).removeAll(Mockito.any(Filter.class));
     verify(cache).clear();
   }