Remove singleton IdentityManager accessors.

Woohoo.

Change-Id: Ib5dfa40b37670633791a39cc166ef53aeb2c2e79
Fixes-Issue: CLIENT-423
Reviewed-on: http://gerrit.dmdirc.com/3145
Reviewed-by: Greg Holmes <greg@dmdirc.com>
Automatic-Compile: DMDirc Build Manager

src/com/dmdirc/ClientModule.java

@@ -120,7 +120,6 @@ public class ClientModule {
         final IdentityManager identityManager =
                 new IdentityManager(baseDirectory, identitiesDirectory);
         ErrorManager.getErrorManager().initialise(identityManager);
-        IdentityManager.setIdentityManager(identityManager);
         identityManager.loadVersionIdentity();
 
         try {

src/com/dmdirc/config/ConfigFileBackedConfigProvider.java

@@ -42,6 +42,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import javax.annotation.Nullable;
+
 import org.slf4j.LoggerFactory;
 
 /**
@@ -57,6 +59,9 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
     private static final String PROFILE_DOMAIN = "profile";
     /** The target for this identity. */
     protected final ConfigTarget myTarget;
+    /** The identity manager to use for writable configs. */
+    @Nullable
+    private final IdentityManager identityManager;
     /** The configuration details for this identity. */
     protected final ConfigFile file;
     /** The global config manager. */
@@ -69,17 +74,16 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
     /**
      * Creates a new instance of Identity.
      *
-     * @param file         The file to load this identity from
-     * @param forceDefault Whether to force this identity to be loaded as default identity or not
+     * @param identityManager The manager to use for hackily reading global state.
+     * @param file            The file to load this identity from
+     * @param forceDefault    Whether to force this identity to be loaded as default identity or not
      *
      * @throws InvalidIdentityFileException Missing required properties
      * @throws IOException                  Input/output exception
      */
-    public ConfigFileBackedConfigProvider(final File file, final boolean forceDefault) throws
-            IOException,
-            InvalidIdentityFileException {
-        super();
-
+    public ConfigFileBackedConfigProvider(final IdentityManager identityManager, final File file,
+            final boolean forceDefault) throws IOException, InvalidIdentityFileException {
+        this.identityManager = identityManager;
         this.file = new ConfigFile(file);
         this.file.setAutomake(true);
         initFile(forceDefault);
@@ -100,10 +104,8 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
      * @throws IOException                  Input/output exception
      */
     public ConfigFileBackedConfigProvider(final InputStream stream, final boolean forceDefault)
-            throws IOException,
-            InvalidIdentityFileException {
-        super();
-
+            throws IOException, InvalidIdentityFileException {
+        this.identityManager = null;
         this.file = new ConfigFile(stream);
         this.file.setAutomake(true);
         initFile(forceDefault);
@@ -117,12 +119,13 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
     /**
      * Creates a new identity from the specified config file.
      *
-     * @param configFile The config file to use
-     * @param target     The target of this identity
+     * @param identityManager The manager to use for hackily reading global state.
+     * @param configFile      The config file to use
+     * @param target          The target of this identity
      */
-    public ConfigFileBackedConfigProvider(final ConfigFile configFile, final ConfigTarget target) {
-        super();
-
+    public ConfigFileBackedConfigProvider(final IdentityManager identityManager,
+            final ConfigFile configFile, final ConfigTarget target) {
+        this.identityManager = identityManager;
         this.file = configFile;
         this.file.setAutomake(true);
         this.myTarget = target;
@@ -329,7 +332,9 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
                 // covered by global defaults.
 
                 if (globalConfig == null) {
-                    globalConfig = new ConfigManager("", "", "", "");
+                    // TODO: This is horrible. Filtering of saves should be abstracted.
+                    globalConfig = (ConfigManager) identityManager
+                            .createAggregateConfig("", "", "", "");
                 }
 
                 globalConfig.removeIdentity(this);
@@ -436,14 +441,15 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
                 // like that until you manually change it again, as opposed
                 // to being removed as soon as you use a build from that
                 // channel.
+                // TODO: This behaviour should be managed by something else.
 
                 if (globalConfig == null) {
-                    globalConfig = new ConfigManager("", "", "", "");
+                    globalConfig = (ConfigManager) identityManager
+                            .createAggregateConfig("", "", "", "");
                 }
 
                 globalConfig.removeIdentity(this);
-                globalConfig.removeIdentity(IdentityManager.getIdentityManager().
-                        getVersionSettings());
+                globalConfig.removeIdentity(identityManager.getVersionSettings());
 
                 if (log.isTraceEnabled()) {
                     for (ConfigProvider source : globalConfig.getSources()) {
@@ -493,7 +499,7 @@ public class ConfigFileBackedConfigProvider extends BaseConfigProvider implement
             file.delete();
         }
 
-        IdentityManager.getIdentityManager().removeConfigProvider(this);
+        identityManager.removeConfigProvider(this);
     }
 
     /** {@inheritDoc} */

src/com/dmdirc/config/ConfigManager.java

@@ -58,6 +58,8 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
     private final MapList<String, ConfigChangeListener> listeners = new MapList<>();
     /** The config binder to use for this manager. */
     private final ConfigBinder binder = new ConfigBinder(this);
+    /** The manager to use to fetch global state. */
+    private final IdentityManager manager;
     /** The protocol this manager is for. */
     private String protocol;
     /** The ircd this manager is for. */
@@ -72,6 +74,7 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
     /**
      * Creates a new instance of ConfigManager.
      *
+     * @param manager  The manager to use to retrieve global state horribly.
      * @param protocol The protocol for this manager
      * @param ircd     The name of the ircd for this manager
      * @param network  The name of the network for this manager
@@ -80,14 +83,16 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
      * @since 0.6.3
      */
     ConfigManager(
+            final IdentityManager manager,
             final String protocol, final String ircd,
             final String network, final String server) {
-        this(protocol, ircd, network, server, "<Unknown>");
+        this(manager, protocol, ircd, network, server, "<Unknown>");
     }
 
     /**
      * Creates a new instance of ConfigManager.
      *
+     * @param manager  The manager to use to retrieve global state horribly.
      * @param protocol The protocol for this manager
      * @param ircd     The name of the ircd for this manager
      * @param network  The name of the network for this manager
@@ -96,10 +101,13 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
      *
      * @since 0.6.3
      */
-    ConfigManager(final String protocol, final String ircd,
+    ConfigManager(
+            final IdentityManager manager,
+            final String protocol, final String ircd,
             final String network, final String server, final String channel) {
         final String chanName = channel + "@" + network;
 
+        this.manager = manager;
         this.protocol = protocol;
         this.ircd = ircd;
         this.network = network;
@@ -163,8 +171,7 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
     @Override
     public Map<String, String> getOptions(final String domain) {
         if (VERSION_DOMAIN.equals(domain)) {
-            return IdentityManager.getIdentityManager()
-                    .getVersionSettings().getOptions(domain);
+            return manager.getVersionSettings().getOptions(domain);
         }
 
         final Map<String, String> res = new HashMap<>();
@@ -220,8 +227,7 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
      */
     protected ConfigProvider getScope(final String domain, final String option) {
         if (VERSION_DOMAIN.equals(domain)) {
-            return IdentityManager.getIdentityManager()
-                    .getVersionSettings();
+            return manager.getVersionSettings();
         }
 
         synchronized (sources) {
@@ -375,8 +381,7 @@ class ConfigManager extends BaseConfigProvider implements ConfigChangeListener,
             }
         }
 
-        final List<ConfigProvider> newSources = IdentityManager.getIdentityManager()
-                .getIdentitiesForManager(this);
+        final List<ConfigProvider> newSources = manager.getIdentitiesForManager(this);
         for (ConfigProvider identity : newSources) {
             log.trace("Testing new identity: {}", identity);
             checkIdentity(identity);

src/com/dmdirc/config/IdentityManager.java

@@ -65,8 +65,6 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     private static final String IDENTITY_DOMAIN = "identity";
     /** The domain used for profile settings. */
     private static final String PROFILE_DOMAIN = "profile";
-    /** A singleton instance of IdentityManager. */
-    private static IdentityManager instance;
     /** Base configuration directory where the main configuration file will be located. */
     private final String configDirectory;
     /** Directory to save and load identities in. */
@@ -105,7 +103,6 @@ public class IdentityManager implements IdentityFactory, IdentityController {
         this.identitiesDirectory = identitiesDirectory;
     }
 
-    /** {@inheritDoc} */
     @Override
     @Deprecated
     public String getConfigurationDirectory() {
@@ -139,7 +136,7 @@ public class IdentityManager implements IdentityFactory, IdentityController {
         addonSettings.put("name", "Addon defaults");
         addonConfigFile.addDomain("identity", addonSettings);
 
-        addonConfig = new ConfigFileBackedConfigProvider(addonConfigFile, target);
+        addonConfig = new ConfigFileBackedConfigProvider(this, addonConfigFile, target);
         addConfigProvider(addonConfig);
 
         if (!getGlobalConfiguration().hasOptionString("identity", "defaultsversion")) {
@@ -301,7 +298,7 @@ public class IdentityManager implements IdentityFactory, IdentityController {
         }
 
         try {
-            addConfigProvider(new ConfigFileBackedConfigProvider(file, false));
+            addConfigProvider(new ConfigFileBackedConfigProvider(this, file, false));
         } catch (InvalidIdentityFileException ex) {
             Logger.userError(ErrorLevel.MEDIUM,
                     "Invalid identity file: " + file.getAbsolutePath()
@@ -371,7 +368,7 @@ public class IdentityManager implements IdentityFactory, IdentityController {
                 file.createNewFile();
             }
 
-            config = new ConfigFileBackedConfigProvider(file, true);
+            config = new ConfigFileBackedConfigProvider(this, file, true);
             config.setOption("identity", "name", "Global config");
             addConfigProvider(config);
         } catch (IOException ex) {
@@ -692,37 +689,13 @@ public class IdentityManager implements IdentityFactory, IdentityController {
 
         configFile.write();
 
-        final ConfigFileBackedConfigProvider identity = new ConfigFileBackedConfigProvider(file,
-                false);
+        final ConfigFileBackedConfigProvider identity = new ConfigFileBackedConfigProvider(this,
+                file, false);
         addConfigProvider(identity);
 
         return identity;
     }
 
-    /**
-     * Gets a singleton instance of the Identity Manager.
-     *
-     * @return A singleton instance of the IdentityManager.
-     *
-     * @deprecated Shouldn't use global state.
-     */
-    @Deprecated
-    public static IdentityManager getIdentityManager() {
-        return instance;
-    }
-
-    /**
-     * Sets the singleton instance of the Identity Manager.
-     *
-     * @param identityManager The identity manager to use.
-     *
-     * @deprecated Shouldn't use global state.
-     */
-    @Deprecated
-    public static void setIdentityManager(final IdentityManager identityManager) {
-        instance = identityManager;
-    }
-
     /**
      * Finds and adds sources for the given manager, and adds it as an identity listener.
      *
@@ -743,7 +716,8 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     @Override
     public ConfigProviderMigrator createMigratableConfig(final String protocol,
             final String ircd, final String network, final String server) {
-        final ConfigManager configManager = new ConfigManager(protocol, ircd, network, server);
+        final ConfigManager configManager =
+                new ConfigManager(this, protocol, ircd, network, server);
         setUpConfigManager(configManager);
         return new ConfigManagerMigrator(configManager);
     }
@@ -752,8 +726,8 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     @Override
     public ConfigProviderMigrator createMigratableConfig(final String protocol,
             final String ircd, final String network, final String server, final String channel) {
-        final ConfigManager configManager = new ConfigManager(protocol, ircd, network, server,
-                channel);
+        final ConfigManager configManager =
+                new ConfigManager(this, protocol, ircd, network, server, channel);
         setUpConfigManager(configManager);
         return new ConfigManagerMigrator(configManager);
     }
@@ -761,7 +735,8 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     @Override
     public AggregateConfigProvider createAggregateConfig(final String protocol, final String ircd,
             final String network, final String server) {
-        final ConfigManager configManager = new ConfigManager(protocol, ircd, network, server);
+        final ConfigManager configManager =
+                new ConfigManager(this, protocol, ircd, network, server);
         setUpConfigManager(configManager);
         return configManager;
     }
@@ -769,8 +744,8 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     @Override
     public AggregateConfigProvider createAggregateConfig(final String protocol, final String ircd,
             final String network, final String server, final String channel) {
-        final ConfigManager configManager = new ConfigManager(protocol, ircd, network, server,
-                channel);
+        final ConfigManager configManager =
+                new ConfigManager(this, protocol, ircd, network, server, channel);
         setUpConfigManager(configManager);
         return configManager;
     }

test/com/dmdirc/config/ConfigManagerTest.java

@@ -19,6 +19,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
+
 package com.dmdirc.config;
 
 import com.dmdirc.interfaces.config.ConfigChangeListener;
@@ -26,6 +27,7 @@ import com.dmdirc.util.validators.PermissiveValidator;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 
 import static org.junit.Assert.*;
@@ -34,14 +36,17 @@ import static org.mockito.Mockito.*;
 @RunWith(MockitoJUnitRunner.class)
 public class ConfigManagerTest {
 
+    @Mock private IdentityManager identityManager;
+
     @Test
     public void testNonExistantOption() {
-        assertNull(new ConfigManager("", "", "", "").getOption("unit-test123", "foobar"));
+        assertNull(new ConfigManager(identityManager, "", "", "", "")
+                .getOption("unit-test123", "foobar"));
     }
 
     @Test
     public void testStats() {
-        final ConfigManager cm = new ConfigManager("", "", "", "");
+        final ConfigManager cm = new ConfigManager(identityManager, "", "", "", "");
         assertNull(ConfigManager.getStats().get("unit-test123.baz"));
         cm.hasOption("unit-test123", "baz", new PermissiveValidator<String>());
         assertNotNull(ConfigManager.getStats().get("unit-test123.baz"));
@@ -51,7 +56,7 @@ public class ConfigManagerTest {
     @Test
     public void testDomainListener() {
         final ConfigChangeListener listener = mock(ConfigChangeListener.class);
-        final ConfigManager cm = new ConfigManager("", "", "", "");
+        final ConfigManager cm = new ConfigManager(identityManager, "", "", "", "");
         cm.addChangeListener("unit-test", listener);
 
         cm.configChanged("foo", "bar");
@@ -64,7 +69,7 @@ public class ConfigManagerTest {
     @Test
     public void testDomainKeyListener() {
         final ConfigChangeListener listener = mock(ConfigChangeListener.class);
-        final ConfigManager cm = new ConfigManager("", "", "", "");
+        final ConfigManager cm = new ConfigManager(identityManager, "", "", "", "");
         cm.addChangeListener("unit-test", "foo", listener);
 
         cm.configChanged("foo", "bar");

test/com/dmdirc/config/IdentityTest.java

@@ -32,12 +32,17 @@ import java.util.Map;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
+@RunWith(MockitoJUnitRunner.class)
 public class IdentityTest {
 
+    @Mock private IdentityManager identityManager;
     private ConfigFileBackedConfigProvider myIdent;
     private ConfigTarget target;
 
@@ -46,7 +51,8 @@ public class IdentityTest {
         target = new ConfigTarget();
         target.setChannel("#unittest@unittest");
 
-        myIdent = new ConfigFileBackedConfigProvider(new ConfigFile(getClass().getResourceAsStream("identity2")), target);
+        myIdent = new ConfigFileBackedConfigProvider(identityManager,
+                new ConfigFile(getClass().getResourceAsStream("identity2")), target);
     }
 
     @Test
@@ -134,7 +140,8 @@ public class IdentityTest {
 
         myIdent.save();
 
-        myIdent = new ConfigFileBackedConfigProvider(myIdent.file.getFile(), false);
+        myIdent = new ConfigFileBackedConfigProvider(identityManager,
+                myIdent.file.getFile(), false);
 
         assertEquals("baz!", myIdent.getOption("foo", "bar"));
     }
@@ -145,19 +152,20 @@ public class IdentityTest {
         assertEquals(target.getType(), myIdent.getTarget().getType());
     }
 
-    @Test(expected=InvalidIdentityFileException.class)
+    @Test(expected = InvalidIdentityFileException.class)
     public void testNoName() throws IOException, InvalidIdentityFileException {
         new ConfigFileBackedConfigProvider(getClass().getResourceAsStream("identity1"), false);
     }
 
-    @Test(expected=InvalidIdentityFileException.class)
+    @Test(expected = InvalidIdentityFileException.class)
     public void testNoTarget() throws IOException, InvalidIdentityFileException {
         new ConfigFileBackedConfigProvider(getClass().getResourceAsStream("identity2"), false);
     }
 
     @Test
     public void testMigrate() throws IOException, InvalidIdentityFileException {
-        final ConfigFileBackedConfigProvider id = new ConfigFileBackedConfigProvider(getClass().getResourceAsStream("identity3"), false);
+        final ConfigFileBackedConfigProvider id = new ConfigFileBackedConfigProvider(getClass().
+                getResourceAsStream("identity3"), false);
 
         assertTrue(id.file.isKeyDomain("identity"));
         assertTrue(id.file.isKeyDomain("meep"));