Merge pull request #568 from stianst/master

KEYCLOAK-562 Cache theme instances

forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java

@@ -13,10 +13,10 @@ import org.keycloak.account.freemarker.model.SessionsBean;
 import org.keycloak.account.freemarker.model.TotpBean;
 import org.keycloak.account.freemarker.model.UrlBean;
 import org.keycloak.audit.Event;
-import org.keycloak.freemarker.ExtendingThemeManager;
 import org.keycloak.freemarker.FreeMarkerException;
 import org.keycloak.freemarker.FreeMarkerUtil;
 import org.keycloak.freemarker.Theme;
+import org.keycloak.freemarker.ThemeProvider;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
@@ -73,10 +73,10 @@ public class FreeMarkerAccountProvider implements AccountProvider {
     public Response createResponse(AccountPages page) {
         Map<String, Object> attributes = new HashMap<String, Object>();
 
-        ExtendingThemeManager themeManager = new ExtendingThemeManager(session);
+        ThemeProvider themeProvider = session.getProvider(ThemeProvider.class, "extending");
         Theme theme;
         try {
-            theme = themeManager.createTheme(realm.getAccountTheme(), Theme.Type.ACCOUNT);
+            theme = themeProvider.getTheme(realm.getAccountTheme(), Theme.Type.ACCOUNT);
         } catch (IOException e) {
             logger.error("Failed to create theme", e);
             return Response.serverError().build();

forms/common-freemarker/src/main/java/org/keycloak/freemarker/ExtendingThemeManager.java

@@ -14,36 +14,49 @@ import java.util.List;
 import java.util.ListIterator;
 import java.util.Properties;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
  */
 public class ExtendingThemeManager implements ThemeProvider {
 
+    private final KeycloakSession session;
+    private final ConcurrentHashMap<ExtendingThemeManagerFactory.ThemeKey, Theme> themeCache;
     private List<ThemeProvider> providers;
     private String defaultTheme;
     private int staticMaxAge;
 
-    public ExtendingThemeManager(KeycloakSession session) {
+    public ExtendingThemeManager(KeycloakSession session, ConcurrentHashMap<ExtendingThemeManagerFactory.ThemeKey, Theme> themeCache) {
-        providers = new LinkedList();
+        this.session = session;
-
+        this.themeCache = themeCache;
-        for (ThemeProvider p : session.getAllProviders(ThemeProvider.class)) {
-            if (!p.getClass().equals(ExtendingThemeManager.class)) {
-                providers.add(p);
-            }
-        }
-
-        Collections.sort(providers, new Comparator<ThemeProvider>() {
-            @Override
-            public int compare(ThemeProvider o1, ThemeProvider o2) {
-                return o2.getProviderPriority() - o1.getProviderPriority();
-            }
-        });
-
         this.defaultTheme = Config.scope("theme").get("default", "keycloak");
         this.staticMaxAge = Config.scope("theme").getInt("staticMaxAge", -1);
     }
 
+    private List<ThemeProvider> getProviders() {
+        if (providers == null) {
+            providers = new LinkedList();
+
+            for (ThemeProvider p : session.getAllProviders(ThemeProvider.class)) {
+                if (!(p instanceof ExtendingThemeManager)) {
+                    if (!p.getClass().equals(ExtendingThemeManager.class)) {
+                        providers.add(p);
+                    }
+                }
+            }
+
+            Collections.sort(providers, new Comparator<ThemeProvider>() {
+                @Override
+                public int compare(ThemeProvider o1, ThemeProvider o2) {
+                    return o2.getProviderPriority() - o1.getProviderPriority();
+                }
+            });
+        }
+
+        return providers;
+    }
+
     public int getStaticMaxAge() {
         return staticMaxAge;
     }
@@ -54,11 +67,27 @@ public class ExtendingThemeManager implements ThemeProvider {
     }
 
     @Override
-    public Theme createTheme(String name, Theme.Type type) throws IOException {
+    public Theme getTheme(String name, Theme.Type type) throws IOException {
         if (name == null) {
             name = defaultTheme;
         }
 
+        if (themeCache != null) {
+            ExtendingThemeManagerFactory.ThemeKey key = ExtendingThemeManagerFactory.ThemeKey.get(name, type);
+            Theme theme = themeCache.get(key);
+            if (theme == null) {
+                theme = loadTheme(name, type);
+                if (themeCache.putIfAbsent(key, theme) != null) {
+                    theme = themeCache.get(key);
+                }
+            }
+            return theme;
+        } else {
+            return loadTheme(name, type);
+        }
+    }
+
+    private Theme loadTheme(String name, Theme.Type type) throws IOException {
         Theme theme = findTheme(name, type);
         if (theme.getParentName() != null) {
             List<Theme> themes = new LinkedList<Theme>();
@@ -88,7 +117,7 @@ public class ExtendingThemeManager implements ThemeProvider {
     @Override
     public Set<String> nameSet(Theme.Type type) {
         Set<String> themes = new HashSet<String>();
-        for (ThemeProvider p : providers) {
+        for (ThemeProvider p : getProviders()) {
             themes.addAll(p.nameSet(type));
         }
         return themes;
@@ -96,7 +125,7 @@ public class ExtendingThemeManager implements ThemeProvider {
 
     @Override
     public boolean hasTheme(String name, Theme.Type type) {
-        for (ThemeProvider p : providers) {
+        for (ThemeProvider p : getProviders()) {
             if (p.hasTheme(name, type)) {
                 return true;
             }
@@ -110,10 +139,10 @@ public class ExtendingThemeManager implements ThemeProvider {
     }
 
     private Theme findTheme(String name, Theme.Type type) {
-        for (ThemeProvider p : providers) {
+        for (ThemeProvider p : getProviders()) {
             if (p.hasTheme(name, type)) {
                 try {
-                    return p.createTheme(name, type);
+                    return p.getTheme(name, type);
                 } catch (IOException e) {
                     throw new RuntimeException("Failed to create " + type.toString().toLowerCase() + " theme", e);
                 }

forms/common-freemarker/src/main/java/org/keycloak/freemarker/ExtendingThemeManagerFactory.java

@@ -0,0 +1,90 @@
+package org.keycloak.freemarker;
+
+import org.keycloak.Config;
+import org.keycloak.models.KeycloakSession;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
+ */
+public class ExtendingThemeManagerFactory implements ThemeProviderFactory {
+
+    private ConcurrentHashMap<ThemeKey, Theme> themeCache = new ConcurrentHashMap<ThemeKey, Theme>();
+
+    private ExtendingThemeManager themeManager;
+
+    @Override
+    public ThemeProvider create(KeycloakSession session) {
+        return new ExtendingThemeManager(session, themeCache);
+    }
+
+    @Override
+    public void init(Config.Scope config) {
+        if(Config.scope("theme").getBoolean("cacheThemes", true)) {
+            themeCache = new ConcurrentHashMap<ThemeKey, Theme>();
+        }
+    }
+
+    @Override
+    public void close() {
+    }
+
+    @Override
+    public String getId() {
+        return "extending";
+    }
+
+    public static class ThemeKey {
+
+        private String name;
+        private Theme.Type type;
+
+        public static ThemeKey get(String name, Theme.Type type) {
+            return new ThemeKey(name, type);
+        }
+
+        private ThemeKey(String name, Theme.Type type) {
+            this.name = name;
+            this.type = type;
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+
+        public Theme.Type getType() {
+            return type;
+        }
+
+        public void setType(Theme.Type type) {
+            this.type = type;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            ThemeKey themeKey = (ThemeKey) o;
+
+            if (name != null ? !name.equals(themeKey.name) : themeKey.name != null) return false;
+            if (type != themeKey.type) return false;
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = name != null ? name.hashCode() : 0;
+            result = 31 * result + (type != null ? type.hashCode() : 0);
+            return result;
+        }
+
+    }
+
+}

forms/common-freemarker/src/main/java/org/keycloak/freemarker/FreeMarkerUtil.java

@@ -12,17 +12,18 @@ import java.net.URL;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
  */
 public class FreeMarkerUtil {
 
-    private Map<String, Template> cache;
+    private ConcurrentHashMap<String, Template> cache;
 
     public FreeMarkerUtil() {
-        if (Config.scope("theme").getBoolean("cacheTemplates", false)) {
+        if (Config.scope("theme").getBoolean("cacheTemplates", true)) {
-            cache = Collections.synchronizedMap(new HashMap<String, Template>());
+            cache = new ConcurrentHashMap<String, Template>();
         }
     }
 
@@ -34,7 +35,9 @@ public class FreeMarkerUtil {
                 template = cache.get(key);
                 if (template == null) {
                     template = getTemplate(templateName, theme);
-                    cache.put(key, template);
+                    if (cache.putIfAbsent(key, template) != null) {
+                        template = cache.get(key);
+                    }
                 }
             } else {
                 template = getTemplate(templateName, theme);

forms/common-freemarker/src/main/java/org/keycloak/freemarker/ThemeProvider.java

@@ -12,7 +12,7 @@ public interface ThemeProvider extends Provider {
 
     public int getProviderPriority();
 
-    public Theme createTheme(String name, Theme.Type type) throws IOException;
+    public Theme getTheme(String name, Theme.Type type) throws IOException;
 
     public Set<String> nameSet(Theme.Type type);
 

forms/common-freemarker/src/main/resources/META-INF/services/org.keycloak.freemarker.ThemeProviderFactory

@@ -0,0 +1 @@
+org.keycloak.freemarker.ExtendingThemeManagerFactory

forms/common-themes/src/main/java/org/keycloak/theme/DefaultKeycloakThemeProvider.java

@@ -37,7 +37,7 @@ public class DefaultKeycloakThemeProvider implements ThemeProvider {
     }
 
     @Override
-    public Theme createTheme(String name, Theme.Type type) throws IOException {
+    public Theme getTheme(String name, Theme.Type type) throws IOException {
         if (hasTheme(name, type)) {
             return new ClassLoaderTheme(name, type, getClass().getClassLoader());
         } else {

forms/common-themes/src/main/java/org/keycloak/theme/FolderThemeProvider.java

@@ -28,7 +28,7 @@ public class FolderThemeProvider implements ThemeProvider {
     }
 
     @Override
-    public Theme createTheme(String name, Theme.Type type) throws IOException {
+    public Theme getTheme(String name, Theme.Type type) throws IOException {
         if (hasTheme(name, type)) {
             return new FolderTheme(new File(getTypeDir(type), name), type);
         }

forms/email-freemarker/src/main/java/org/keycloak/email/freemarker/FreeMarkerEmailProvider.java

@@ -5,9 +5,9 @@ import org.keycloak.audit.Event;
 import org.keycloak.email.EmailException;
 import org.keycloak.email.EmailProvider;
 import org.keycloak.email.freemarker.beans.EventBean;
-import org.keycloak.freemarker.ExtendingThemeManager;
 import org.keycloak.freemarker.FreeMarkerUtil;
 import org.keycloak.freemarker.Theme;
+import org.keycloak.freemarker.ThemeProvider;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
@@ -79,8 +79,8 @@ public class FreeMarkerEmailProvider implements EmailProvider {
 
     private void send(String subjectKey, String template, Map<String, Object> attributes) throws EmailException {
         try {
-            ExtendingThemeManager themeManager = new ExtendingThemeManager(session);
+            ThemeProvider themeProvider = session.getProvider(ThemeProvider.class, "extending");
-            Theme theme = themeManager.createTheme(realm.getEmailTheme(), Theme.Type.EMAIL);
+            Theme theme = themeProvider.getTheme(realm.getEmailTheme(), Theme.Type.EMAIL);
 
             String subject =  theme.getMessages().getProperty(subjectKey);
             String body = freeMarker.processTemplate(attributes, template, theme);

forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/FreeMarkerLoginFormsProvider.java

@@ -4,10 +4,10 @@ import org.jboss.logging.Logger;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.email.EmailException;
 import org.keycloak.email.EmailProvider;
-import org.keycloak.freemarker.ExtendingThemeManager;
 import org.keycloak.freemarker.FreeMarkerException;
 import org.keycloak.freemarker.FreeMarkerUtil;
 import org.keycloak.freemarker.Theme;
+import org.keycloak.freemarker.ThemeProvider;
 import org.keycloak.login.LoginFormsPages;
 import org.keycloak.login.LoginFormsProvider;
 import org.keycloak.login.freemarker.model.CodeBean;
@@ -150,10 +150,10 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
 
         Map<String, Object> attributes = new HashMap<String, Object>();
 
-        ExtendingThemeManager themeManager = new ExtendingThemeManager(session);
+        ThemeProvider themeProvider = session.getProvider(ThemeProvider.class, "extending");
         Theme theme;
         try {
-            theme = themeManager.createTheme(realm.getLoginTheme(), Theme.Type.LOGIN);
+            theme = themeProvider.getTheme(realm.getLoginTheme(), Theme.Type.LOGIN);
         } catch (IOException e) {
             logger.error("Failed to create theme", e);
             return Response.serverError().build();

model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/MemUserSessionProvider.java

@@ -3,6 +3,7 @@ package org.keycloak.models.sessions.mem;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientSessionModel;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ModelDuplicateException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
@@ -213,7 +214,11 @@ public class MemUserSessionProvider implements UserSessionProvider {
     @Override
     public UsernameLoginFailureModel addUserLoginFailure(RealmModel realm, String username) {
         UsernameLoginFailureKey key = new UsernameLoginFailureKey(username, realm.getId());
-        return new UsernameLoginFailureAdapter(loginFailures.putIfAbsent(key, new UsernameLoginFailureEntity(username, realm.getId())));
+        UsernameLoginFailureEntity entity = new UsernameLoginFailureEntity(username, realm.getId());
+        if (loginFailures.putIfAbsent(key, entity) != null) {
+            throw new ModelDuplicateException();
+        }
+        return new UsernameLoginFailureAdapter(entity);
     }
 
     @Override

project-integrations/aerogear-ups/auth-server/src/main/java/org/aerogear/ups/security/AerogearThemeProvider.java

@@ -32,7 +32,7 @@ public class AerogearThemeProvider implements ThemeProvider {
     }
 
     @Override
-    public Theme createTheme(String name, Theme.Type type) throws IOException {
+    public Theme getTheme(String name, Theme.Type type) throws IOException {
         if (hasTheme(name, type)) {
             return new ClassLoaderTheme(name, type, getClass().getClassLoader());
         } else {

project-integrations/aerogear-ups/auth-server/src/main/resources/META-INF/keycloak-server.json

@@ -34,6 +34,7 @@
         "default": "keycloak",
         "staticMaxAge": 2592000,
         "cacheTemplates": true,
+        "cacheThemes": true,
         "folder": {
             "dir": "${jboss.server.config.dir}/themes"
         }

server/src/main/resources/META-INF/keycloak-server.json

@@ -41,6 +41,7 @@
         "default": "keycloak",
         "staticMaxAge": 2592000,
         "cacheTemplates": true,
+        "cacheThemes": true,
         "folder": {
           "dir": "${jboss.server.config.dir}/themes"
         }

services/src/main/java/org/keycloak/services/resources/ThemeResource.java

@@ -1,8 +1,9 @@
 package org.keycloak.services.resources;
 
 import org.jboss.logging.Logger;
-import org.keycloak.freemarker.ExtendingThemeManager;
+import org.keycloak.Config;
 import org.keycloak.freemarker.Theme;
+import org.keycloak.freemarker.ThemeProvider;
 import org.keycloak.models.KeycloakSession;
 
 import javax.activation.FileTypeMap;
@@ -42,13 +43,13 @@ public class ThemeResource {
     @Path("/{themeType}/{themeName}/{path:.*}")
     public Response getResource(@PathParam("themeType") String themType, @PathParam("themeName") String themeName, @PathParam("path") String path) {
         try {
-            ExtendingThemeManager themeManager = new ExtendingThemeManager(session);
+            ThemeProvider themeProvider = session.getProvider(ThemeProvider.class, "extending");
-            Theme theme = themeManager.createTheme(themeName, Theme.Type.valueOf(themType.toUpperCase()));
+            Theme theme = themeProvider.getTheme(themeName, Theme.Type.valueOf(themType.toUpperCase()));
             InputStream resource = theme.getResourceAsStream(path);
             if (resource != null) {
                 CacheControl cacheControl = new CacheControl();
                 cacheControl.setNoTransform(false);
-                cacheControl.setMaxAge(themeManager.getStaticMaxAge());
+                cacheControl.setMaxAge(Config.scope("theme").getInt("staticMaxAge", -1));
 
                 return Response.ok(resource).type(mimeTypes.getContentType(path)).cacheControl(cacheControl).build();
             } else {

services/src/main/java/org/keycloak/services/resources/admin/AdminConsole.java

@@ -6,8 +6,9 @@ import org.jboss.resteasy.annotations.cache.NoCache;
 import org.jboss.resteasy.spi.HttpRequest;
 import org.jboss.resteasy.spi.HttpResponse;
 import org.jboss.resteasy.spi.NotFoundException;
-import org.keycloak.freemarker.ExtendingThemeManager;
+import org.keycloak.Config;
 import org.keycloak.freemarker.Theme;
+import org.keycloak.freemarker.ThemeProvider;
 import org.keycloak.models.AdminRoles;
 import org.keycloak.models.ApplicationModel;
 import org.keycloak.models.Constants;
@@ -310,15 +311,15 @@ public class AdminConsole {
         }
 
         try {
-            ExtendingThemeManager themeManager = new ExtendingThemeManager(session);
+            ThemeProvider themeProvider = session.getProvider(ThemeProvider.class, "extending");
-            Theme theme = themeManager.createTheme(realm.getAdminTheme(), Theme.Type.ADMIN);
+            Theme theme = themeProvider.getTheme(realm.getAdminTheme(), Theme.Type.ADMIN);
             InputStream resource = theme.getResourceAsStream(path);
             if (resource != null) {
                 String contentType = mimeTypes.getContentType(path);
 
                 CacheControl cacheControl = new CacheControl();
                 cacheControl.setNoTransform(false);
-                cacheControl.setMaxAge(themeManager.getStaticMaxAge());
+                cacheControl.setMaxAge(Config.scope("theme").getInt("staticMaxAge", -1));
 
                 return Response.ok(resource).type(contentType).cacheControl(cacheControl).build();
             } else {

services/src/main/java/org/keycloak/services/resources/admin/ServerInfoAdminResource.java

@@ -4,6 +4,7 @@ import org.keycloak.audit.AuditListener;
 import org.keycloak.authentication.AuthenticationProvider;
 import org.keycloak.freemarker.ExtendingThemeManager;
 import org.keycloak.freemarker.Theme;
+import org.keycloak.freemarker.ThemeProvider;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.social.SocialProvider;
 import org.keycloak.util.ProviderLoader;
@@ -41,11 +42,11 @@ public class ServerInfoAdminResource {
     }
 
     private void setThemes(ServerInfoRepresentation info) {
-        ExtendingThemeManager themeManager = new ExtendingThemeManager(session);
+        ThemeProvider themeProvider = session.getProvider(ThemeProvider.class, "extending");
         info.themes = new HashMap<String, List<String>>();
 
         for (Theme.Type type : Theme.Type.values()) {
-            List<String> themes = new LinkedList<String>(themeManager.nameSet(type));
+            List<String> themes = new LinkedList<String>(themeProvider.nameSet(type));
             Collections.sort(themes);
 
             info.themes.put(type.toString().toLowerCase(), themes);

testsuite/integration/src/main/resources/META-INF/keycloak-server.json

@@ -38,6 +38,7 @@
         "default": "keycloak",
         "staticMaxAge": 2592000,
         "cacheTemplates": "${keycloak.theme.cacheTemplates:true}",
+        "cacheThemes": "${keycloak.theme.cacheThemes:true}",
         "folder": {
             "dir": "${keycloak.theme.dir}"
         }

testsuite/tools/src/main/resources/META-INF/keycloak-server.json

@@ -30,6 +30,7 @@
         "default": "keycloak",
         "staticMaxAge": 2592000,
         "cacheTemplates": true,
+        "cacheThemes": true,
         "folder": {
             "dir": "${jboss.server.config.dir}/themes"
         }