Author: oheger
Date: Sat Sep 28 19:46:30 2013
New Revision: 1527248

URL: http://svn.apache.org/r1527248
Log:
Some changes in FileSystem class.

Removed the static default file system field. It was thread-hostile. (The
file system now has to be provided explicitly by clients.) Changed the way the
logger is accessed, do not call a non-final, non-private method in
constructor. Member fields have been made volatile, they might be accessed by
different threads.

Added:
    
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/io/TestDefaultFileSystem.java
Modified:
    
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/io/FileSystem.java

Modified: 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/io/FileSystem.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/io/FileSystem.java?rev=1527248&r1=1527247&r2=1527248&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/io/FileSystem.java
 (original)
+++ 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/io/FileSystem.java
 Sat Sep 28 19:46:30 2013
@@ -24,7 +24,6 @@ import java.net.URL;
 
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.commons.logging.impl.NoOpLog;
 
 /**
@@ -36,22 +35,14 @@ import org.apache.commons.logging.impl.N
  */
 public abstract class FileSystem
 {
-    /** The name of the system property that can be used to set the file 
system class name */
-    private static final String FILE_SYSTEM = 
"org.apache.commons.configuration.filesystem";
-
-    /** The default file system */
-    private static FileSystem fileSystem;
+    /** Constant for the default logger. */
+    private static final Log DEFAULT_LOG = new NoOpLog();
 
     /** The Logger */
-    private Log log;
+    private volatile Log log;
 
     /** FileSystem options provider */
-    private FileOptionsProvider optionsProvider;
-
-    public FileSystem()
-    {
-        setLogger(null);
-    }
+    private volatile FileOptionsProvider optionsProvider;
 
     /**
      * Returns the logger used by this FileSystem.
@@ -60,7 +51,8 @@ public abstract class FileSystem
      */
     public Log getLogger()
     {
-        return log;
+        Log result = log;
+        return (result != null) ? result : DEFAULT_LOG;
     }
 
     /**
@@ -74,77 +66,7 @@ public abstract class FileSystem
      */
     public void setLogger(Log log)
     {
-        this.log = (log != null) ? log : new NoOpLog();
-    }
-
-    static
-    {
-        String fsClassName = System.getProperty(FILE_SYSTEM);
-        if (fsClassName != null)
-        {
-            Log log = LogFactory.getLog(FileSystem.class);
-
-            try
-            {
-                Class<?> clazz = Class.forName(fsClassName);
-                if (FileSystem.class.isAssignableFrom(clazz))
-                {
-                    fileSystem = (FileSystem) clazz.newInstance();
-                    if (log.isDebugEnabled())
-                    {
-                        log.debug("Using " + fsClassName);
-                    }
-                }
-            }
-            catch (InstantiationException ex)
-            {
-                log.error("Unable to create " + fsClassName, ex);
-            }
-            catch (IllegalAccessException ex)
-            {
-                log.error("Unable to create " + fsClassName, ex);
-            }
-            catch (ClassNotFoundException ex)
-            {
-                log.error("Unable to create " + fsClassName, ex);
-            }
-        }
-
-        if (fileSystem == null)
-        {
-            fileSystem = new DefaultFileSystem();
-        }
-    }
-
-    /**
-     * Set the FileSystem to use.
-     * @param fs The FileSystem
-     * @throws NullPointerException if the FileSystem parameter is null.
-     */
-    public static void setDefaultFileSystem(FileSystem fs) throws 
NullPointerException
-    {
-        if (fs == null)
-        {
-            throw new NullPointerException("A FileSystem implementation is 
required");
-        }
-        fileSystem = fs;
-    }
-
-    /**
-     * Reset the FileSystem to the default.
-     */
-    public static void resetDefaultFileSystem()
-    {
-        fileSystem = new DefaultFileSystem();
-    }
-
-    /**
-     * Retrieve the FileSystem being used.
-     * @return The FileSystem.
-     */
-    public static FileSystem getDefaultFileSystem()
-    {
-        return fileSystem;
+        this.log = log;
     }
 
     /**

Added: 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/io/TestDefaultFileSystem.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/io/TestDefaultFileSystem.java?rev=1527248&view=auto
==============================================================================
--- 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/io/TestDefaultFileSystem.java
 (added)
+++ 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/io/TestDefaultFileSystem.java
 Sat Sep 28 19:46:30 2013
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.configuration.io;
+
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.commons.logging.impl.NoOpLog;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Test class for {@code DefaultFileSystem}. Note: This class tests only basic
+ * functionality. Other parts are tested by actual access to configuration 
files
+ * in other test classes.
+ *
+ * @version $Id: $
+ */
+public class TestDefaultFileSystem
+{
+    /** The file system to be tested. */
+    private DefaultFileSystem fileSystem;
+
+    @Before
+    public void setUp() throws Exception
+    {
+        fileSystem = new DefaultFileSystem();
+    }
+
+    /**
+     * Tests the default logger.
+     */
+    @Test
+    public void testDefaultLogger()
+    {
+        assertTrue("Wrong default logger",
+                fileSystem.getLogger() instanceof NoOpLog);
+    }
+
+    /**
+     * Tests whether the logger can be changed.
+     */
+    @Test
+    public void testSetLogger()
+    {
+        Log log = LogFactory.getLog(getClass());
+        fileSystem.setLogger(log);
+        assertSame("Logger not set", log, fileSystem.getLogger());
+    }
+}


Reply via email to