use only plugin stack from constructor in AuthorizationFramework

fixes #1918

Author: Vladimir Kotal

plugins/LdapPlugin/src/opengrok/auth/plugin/AbstractLdapPlugin.java

@@ -32,11 +32,9 @@
 import opengrok.auth.plugin.ldap.AbstractLdapProvider;
 import opengrok.auth.plugin.ldap.FakeLdapFacade;
 import opengrok.auth.plugin.ldap.LdapFacade;
-import org.opensolaris.opengrok.authorization.AuthorizationFramework;
 import org.opensolaris.opengrok.authorization.IAuthorizationPlugin;
 import org.opensolaris.opengrok.configuration.Group;
 import org.opensolaris.opengrok.configuration.Project;
-import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
 
 /**
  * Abstract class for all plug-ins working with LDAP. Takes care of

src/org/opensolaris/opengrok/authorization/AuthorizationFramework.java

@@ -86,11 +86,6 @@ public final class AuthorizationFramework {
      * stored in HTTP session.
      *
      * Starting at 0 and increases with every reload.
-     *
-     * External consumers should call RuntimeEnvironment.getPluginVersion() 
-     * to get this number.
-     *
-     * @see RuntimeEnvironment#getPluginVersion()
      */
     private long pluginVersion = 0;
 
@@ -550,7 +545,7 @@ public Object run() {
         });
 
         // clone a new stack not interfering with the current stack
-        AuthorizationStack newStack = RuntimeEnvironment.getInstance().getPluginStack().clone();
+        AuthorizationStack newStack = getStack().clone();
 
         // load all other possible plugin classes
         loadClasses(newStack,
@@ -595,7 +590,6 @@ public Object run() {
      * Assumes the {@code lock} is held for reading.
      *
      * @return the current version number
-     * @see RuntimeEnvironment#getPluginVersion()
      */
     private long getPluginVersion() {
         return pluginVersion;
@@ -642,9 +636,9 @@ private boolean isSessionInvalid(HttpSession session) {
      * <h3>Order of plugin invocation</h3>
      *
      * <p>
-     * The order of plugin invocation is given by the configuration
-     * {@link RuntimeEnvironment#getPluginStack()} and appropriate actions are
-     * taken when traversing the stack with set of keywords, such as:</p>
+     * The order of plugin invocation is given by the stack and appropriate
+     * actions are taken when traversing the stack with set of keywords,
+     * such as:</p>
      *
      * <h4>required</h4>
      * Failure of such a plugin will ultimately lead to the authorization

src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java

@@ -1663,7 +1663,7 @@ public void loadStatistics(InputStream in) throws IOException, ParseException {
      */
     synchronized public AuthorizationFramework getAuthorizationFramework() {
         if (authFramework == null) {
-            authFramework = new AuthorizationFramework(threadConfig.get().getPluginDirectory());
+            authFramework = new AuthorizationFramework(getPluginDirectory(), getPluginStack());
         }
         return authFramework;
     }
@@ -1750,6 +1750,7 @@ public void applyConfig(Configuration config, boolean reindex) {
 
         // set the new plugin directory and reload the authorization framework
         getAuthorizationFramework().setPluginDirectory(config.getPluginDirectory());
+        getAuthorizationFramework().setStack(config.getPluginStack());
         getAuthorizationFramework().reload();
     }
 

src/org/opensolaris/opengrok/web/WebappListener.java

@@ -17,7 +17,7 @@
  * CDDL HEADER END
  */
 
- /*
+/*
  * Copyright (c) 2007, 2017, Oracle and/or its affiliates. All rights reserved.
  */
 package org.opensolaris.opengrok.web;
@@ -80,7 +80,7 @@ public void contextInitialized(final ServletContextEvent servletContextEvent) {
          * (reading the configuration) failed then the plugin directory is
          * possibly {@code null} causing the framework to allow every request.
          */
-        env.setAuthorizationFramework(new AuthorizationFramework(env.getPluginDirectory()));
+        env.setAuthorizationFramework(new AuthorizationFramework(env.getPluginDirectory(), env.getPluginStack()));
 
         String address = context.getInitParameter("ConfigAddress");
         if (address != null && address.length() > 0) {

test/org/opensolaris/opengrok/authorization/AuthorizationFrameworkReloadTest.java

@@ -107,15 +107,8 @@ public void testReloadCycle() {
         stack.add(new AuthorizationPlugin(AuthControlFlag.REQUIRED, 
                 "opengrok.auth.plugin.FalsePlugin"));
         stack.setForProjects(projectName);
-        RuntimeEnvironment re = RuntimeEnvironment.getInstance();
-        // Unfortunately even though the AuthorizationFramework is instantiated with given stack,
-        // this does not work because the reload() method called from the constructor grabs
-        // the stack from RuntimeEnvironment and not from the AuthorizationFramework itself.
-        re.setPluginStack(stack);
-        re.setPluginDirectory(pluginDirectory.getPath());
-        
         AuthorizationFramework framework = 
-                new AuthorizationFramework(pluginDirectory.getPath());
+                new AuthorizationFramework(pluginDirectory.getPath(), stack);
 
         // Perform simple sanity check before long run is entered. If this fails,
         // it will be waste of time to continue with the test.
@@ -125,13 +118,14 @@ public void testReloadCycle() {
 
         // Create a thread that does reload() every now and then.
         runThread = true;
+        final int maxReloadSleep = 10;
         Thread t = new Thread(new Runnable() {
             @Override
             public void run() {
                 while (runThread) {
                     framework.reload();
                     try {
-                        Thread.sleep(10);
+                        Thread.sleep((long) (Math.random() % maxReloadSleep) + 1);
                     } catch (InterruptedException ex) {
                     }
                 }
@@ -142,12 +136,12 @@ public void run() {
         reloads = stats.getRequest("authorization_stack_reload");
         assertNotNull(reloads);
         // Process number or requests and check that framework decision is consistent.
-        for (int i = 0; i < 100; i++) {
+        for (int i = 0; i < 1000; i++) {
             req = new DummyHttpServletRequest();
             assertFalse(framework.isAllowed(req, p));
             try {
-                // XXX random sleep
-                Thread.sleep(50);
+                // Should run more frequently than the thread performing reload().
+                Thread.sleep((long) (Math.random() % (maxReloadSleep / 3)) + 1);
             } catch (InterruptedException ex) {
             }
         }