Skip to content

Commit 8ca5403

Browse files
tulinkryVladimir Kotal
authored andcommitted
removing singleton pattern from authorization framework (#1604)
fixes #1595
1 parent e347df4 commit 8ca5403

File tree

8 files changed

+98
-91
lines changed

8 files changed

+98
-91
lines changed

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

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
public final class AuthorizationFramework {
5858

5959
private static final Logger LOGGER = LoggerFactory.getLogger(AuthorizationFramework.class);
60-
private volatile static AuthorizationFramework instance = new AuthorizationFramework();
6160

6261
/**
6362
* Plugin directory.
@@ -94,14 +93,26 @@ public final class AuthorizationFramework {
9493
private int pluginVersion = 0;
9594

9695
/**
97-
* Plugin directory is set through RuntimeEnvironment.
96+
* Create a new instance of authorization framework with the plugin
97+
* directory and the default plugin stack.
9898
*
99-
* @return an instance of AuthorizationFramework
100-
* @see RuntimeEnvironment#getConfiguration
101-
* @see Configuration#setPluginDirectory
99+
* @param path the plugin directory path
102100
*/
103-
public static AuthorizationFramework getInstance() {
104-
return instance;
101+
public AuthorizationFramework(String path) {
102+
this(path, new AuthorizationStack(AuthControlFlag.REQUIRED, "default stack"));
103+
}
104+
105+
/**
106+
* Create a new instance of authorization framework with the plugin
107+
* directory and the plugin stack.
108+
*
109+
* @param path the plugin directory path
110+
* @param stack the top level stack configuration
111+
*/
112+
public AuthorizationFramework(String path, AuthorizationStack stack) {
113+
this.stack = stack;
114+
setPluginDirectory(path);
115+
reload();
105116
}
106117

107118
/**
@@ -201,14 +212,6 @@ public boolean shouldSkip(AuthorizationEntity authEntity) {
201212
});
202213
}
203214

204-
private AuthorizationFramework() {
205-
String path = RuntimeEnvironment.getInstance()
206-
.getPluginDirectory();
207-
stack = RuntimeEnvironment.getInstance().getPluginStack();
208-
setPluginDirectory(path);
209-
reload();
210-
}
211-
212215
/**
213216
* Return the java canonical name for the plugin class. If the canonical
214217
* name does not exist it returns the usual java name.
@@ -492,10 +495,16 @@ private String getClassName(JarEntry f) {
492495
/**
493496
* Calling this function forces the framework to reload its stack.
494497
*
495-
* Plugins are taken from the pluginDirectory.
498+
* <p>
499+
* Plugins are taken from the pluginDirectory.</p>
496500
*
501+
* <p>
497502
* Old instances of stack are removed and new list of stack is constructed.
498-
* Unload and load event is fired on each plugin.
503+
* Unload and load event is fired on each plugin.</p>
504+
*
505+
* <p>
506+
* This method is thread safe with respect to the currently running
507+
* authorization checks.</p>
499508
*
500509
* @see IAuthorizationPlugin#load(java.util.Map)
501510
* @see IAuthorizationPlugin#unload()

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

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,11 @@ public final class RuntimeEnvironment {
131131

132132
private Statistics statistics = new Statistics();
133133

134+
/**
135+
* Instance of authorization framework. Allowing every request by default.
136+
*/
137+
private AuthorizationFramework authFramework = new AuthorizationFramework(null);
138+
134139
/* Get thread pool used for top-level repository history generation. */
135140
public static synchronized ExecutorService getHistoryExecutor() {
136141
if (historyExecutor == null) {
@@ -1547,7 +1552,29 @@ public void loadStatistics(InputStream in) throws IOException, ParseException {
15471552
* @see AuthorizationFramework#getPluginVersion()
15481553
*/
15491554
public int getPluginVersion() {
1550-
return AuthorizationFramework.getInstance().getPluginVersion();
1555+
return authFramework.getPluginVersion();
1556+
}
1557+
1558+
/**
1559+
* Return the authorization framework used in this environment.
1560+
*
1561+
* @return the framework
1562+
*/
1563+
public AuthorizationFramework getAuthorizationFramework() {
1564+
return authFramework;
1565+
}
1566+
1567+
/**
1568+
* Set the authorization framework for this environment. Unload all
1569+
* previously load plugins.
1570+
*
1571+
* @param fw the new framework
1572+
*/
1573+
public void setAuthorizationFramework(AuthorizationFramework fw) {
1574+
if (this.authFramework != null) {
1575+
this.authFramework.removeAll(this.authFramework.getStack());
1576+
}
1577+
this.authFramework = fw;
15511578
}
15521579

15531580
private ServerSocket configServerSocket;
@@ -1610,8 +1637,8 @@ public void applyConfig(Configuration config, boolean reindex) {
16101637
config.refreshDateForLastIndexRun();
16111638
}
16121639

1613-
AuthorizationFramework.getInstance().setPluginDirectory(config.getPluginDirectory());
1614-
AuthorizationFramework.getInstance().reload();
1640+
authFramework.setPluginDirectory(config.getPluginDirectory());
1641+
authFramework.reload();
16151642
}
16161643

16171644
/**
@@ -1778,7 +1805,7 @@ public FileVisitResult postVisitDirectory(Path d, IOException exc) throws IOExce
17781805
}
17791806
if (reload) {
17801807
Thread.sleep(THREAD_SLEEP_TIME); // experimental wait if file is being written right now
1781-
AuthorizationFramework.getInstance().reload();
1808+
authFramework.reload();
17821809
}
17831810
if (!key.reset()) {
17841811
break;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1325,7 +1325,7 @@ public static PageConfig get(HttpServletRequest request) {
13251325

13261326
private PageConfig(HttpServletRequest req) {
13271327
this.req = req;
1328-
this.authFramework = AuthorizationFramework.getInstance();
1328+
this.authFramework = RuntimeEnvironment.getInstance().getAuthorizationFramework();
13291329
}
13301330

13311331
/**

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import org.json.simple.JSONArray;
6161
import org.json.simple.JSONObject;
6262
import org.opensolaris.opengrok.Info;
63-
import org.opensolaris.opengrok.authorization.AuthorizationFramework;
6463
import org.opensolaris.opengrok.configuration.Group;
6564
import org.opensolaris.opengrok.configuration.Project;
6665
import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
@@ -888,7 +887,7 @@ public static void dumpConfiguration(Appendable out) throws IOException,
888887
env.isAllowLeadingWildcard());
889888
printTableRow(out, "History cache", HistoryGuru.getInstance()
890889
.getCacheInfo());
891-
printTableRow(out, "Authorization", "<pre>" + AuthorizationFramework.getInstance().getStack().hierarchyToString() + "</pre>");
890+
printTableRow(out, "Authorization", "<pre>" + env.getAuthorizationFramework().getStack().hierarchyToString() + "</pre>");
892891
out.append("</table>");
893892
}
894893

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import javax.servlet.ServletRequestEvent;
3838
import javax.servlet.ServletRequestListener;
3939
import org.json.simple.parser.ParseException;
40-
import org.opensolaris.opengrok.authorization.AuthorizationFramework;
4140
import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
4241
import org.opensolaris.opengrok.logger.LoggerFactory;
4342

@@ -104,8 +103,6 @@ public void contextInitialized(final ServletContextEvent servletContextEvent) {
104103
LOGGER.log(Level.SEVERE, "Could not parse statistics from a file.", ex);
105104
}
106105

107-
AuthorizationFramework.getInstance(); // start + load
108-
109106
if (env.getConfiguration().getPluginDirectory() != null && env.isAuthorizationWatchdog()) {
110107
RuntimeEnvironment.getInstance().startWatchDogService(new File(env.getConfiguration().getPluginDirectory()));
111108
}

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

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,22 @@
2828
import java.util.Random;
2929
import java.util.stream.Collectors;
3030
import javax.servlet.http.HttpServletRequest;
31-
import org.junit.AfterClass;
3231
import org.junit.Assert;
33-
import org.junit.BeforeClass;
32+
import org.junit.Before;
3433
import org.junit.Test;
3534
import org.junit.runner.RunWith;
3635
import org.junit.runners.Parameterized;
3736
import org.opensolaris.opengrok.configuration.Group;
3837
import org.opensolaris.opengrok.configuration.Nameable;
3938
import org.opensolaris.opengrok.configuration.Project;
40-
import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
4139
import org.opensolaris.opengrok.web.DummyHttpServletRequest;
4240

4341
@RunWith(Parameterized.class)
4442
public class AuthorizationFrameworkTest {
4543

46-
private static String pluginDirectory;
4744
private static final Random random = new Random();
4845

46+
private AuthorizationFramework framework;
4947
private final StackSetup setup;
5048

5149
public AuthorizationFrameworkTest(StackSetup setup) {
@@ -651,42 +649,30 @@ public static StackSetup[][] params() {
651649

652650
@Test
653651
public void testPluginsGeneric() {
654-
AuthorizationFramework instance = getInstance();
655-
instance.setStack(setup.stack);
656-
instance.loadAllPlugins(setup.stack);
652+
framework.setStack(setup.stack);
653+
framework.loadAllPlugins(setup.stack);
657654

658655
boolean actual;
659656
String format = "%s <%s> was <%s> for entity %s";
660657

661658
for (TestCase innerSetup : setup.setup) {
662659
try {
663-
actual = instance.isAllowed(innerSetup.request, (Group) innerSetup.entity);
660+
actual = framework.isAllowed(innerSetup.request, (Group) innerSetup.entity);
664661
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, innerSetup.entity.getName()),
665662
innerSetup.expected,
666663
actual);
667664
} catch (ClassCastException ex) {
668-
actual = instance.isAllowed(innerSetup.request, (Project) innerSetup.entity);
665+
actual = framework.isAllowed(innerSetup.request, (Project) innerSetup.entity);
669666
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, innerSetup.entity.getName()),
670667
innerSetup.expected,
671668
actual);
672669
}
673670
}
674671
}
675672

676-
@BeforeClass
677-
public static void tearUpClass() {
678-
pluginDirectory = RuntimeEnvironment.getInstance().getConfiguration().getPluginDirectory();
679-
RuntimeEnvironment.getInstance().getConfiguration().setPluginDirectory(null);
680-
}
681-
682-
@AfterClass
683-
public static void tearDownClass() {
684-
RuntimeEnvironment.getInstance().getConfiguration().setPluginDirectory(pluginDirectory);
685-
}
686-
687-
static private AuthorizationFramework getInstance() {
688-
AuthorizationFramework.getInstance().removeAll(AuthorizationFramework.getInstance().getStack());
689-
return AuthorizationFramework.getInstance();
673+
@Before
674+
public void setUp() {
675+
framework = new AuthorizationFramework(null);
690676
}
691677

692678
static private Project createAllowedProject() {

test/org/opensolaris/opengrok/web/ProjectHelperTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,20 @@ public void testGetInstance() {
4848
Assert.assertNotNull("Project helper should not be null", result);
4949
Assert.assertSame(result.getClass(), ProjectHelper.class);
5050
}
51-
51+
5252
/**
5353
* Test if projects and groups are always reloaded fully from the env.
54-
*
54+
*
5555
* This ensures that when the RuntimeEnvironment changes that it also
5656
* updates the projects in the ui.
5757
*/
5858
@Test
5959
public void testSynchronization() {
60-
HashMap<String,Project> oldProjects = new HashMap<>(env.getProjects());
60+
HashMap<String, Project> oldProjects = new HashMap<>(env.getProjects());
6161
List<RepositoryInfo> oldRepositories = new ArrayList<>(env.getRepositories());
6262
Set<Group> oldGroups = new TreeSet<>(env.getGroups());
6363
Map<Project, List<RepositoryInfo>> oldMap = new TreeMap<>(getRepositoriesMap());
64-
invokeRemoveAll();
64+
env.getAuthorizationFramework().removeAll(env.getAuthorizationFramework().getStack());
6565

6666
cfg = PageConfig.get(getRequest());
6767
helper = cfg.getProjectHelper();

0 commit comments

Comments
 (0)