qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kw...@apache.org
Subject svn commit: r1213636 - in /qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server: security/auth/database/PlainPasswordFilePrincipalDatabase.java store/DerbyMessageStore.java
Date Tue, 13 Dec 2011 10:19:09 GMT
Author: kwall
Date: Tue Dec 13 10:19:08 2011
New Revision: 1213636

URL: http://svn.apache.org/viewvc?rev=1213636&view=rev
Log:
QPID-3676: Correct issues found by findbugs marked as blockers

Applied patch from Andrew MacBean <andymacbean@gmail.com>

Modified:
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java?rev=1213636&r1=1213635&r2=1213636&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java
(original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/database/PlainPasswordFilePrincipalDatabase.java
Tue Dec 13 10:19:08 2011
@@ -375,16 +375,8 @@ public class PlainPasswordFilePrincipalD
 
             BufferedReader reader = null;
             PrintStream writer = null;
-            
-            Random r = new Random();
-            File tmp;
-            do
-            {
-                tmp = new File(_passwordFile.getPath() + r.nextInt() + ".tmp");
-            }
-            while(tmp.exists());
-            
-            tmp.deleteOnExit();
+
+            final File tmp = createTempFileOnSameFilesystem(_passwordFile);
 
             try
             {
@@ -449,52 +441,72 @@ public class PlainPasswordFilePrincipalD
             }
             finally
             {
-                if (reader != null)
-                {
-                    reader.close();
-                }
-
                 if (writer != null)
                 {
                     writer.close();
                 }
-            }
-            
-            // Swap temp file to main password file.
-            File old = new File(_passwordFile.getAbsoluteFile() + ".old");
-            if (old.exists())
-            {
-                old.delete();
-            }
-            
-            if(!_passwordFile.renameTo(old))
-            {
-                //unable to rename the existing file to the backup name 
-                _logger.error("Could not backup the existing password file");
-                throw new IOException("Could not backup the existing password file");
-            }
-
-            if(!tmp.renameTo(_passwordFile))
-            {
-                //failed to rename the new file to the required filename
-                
-                if(!old.renameTo(_passwordFile))
+                if (reader != null)
                 {
-                    //unable to return the backup to required filename
-                    _logger.error("Could not rename the new password file into place, and
unable to restore original file");
-                    throw new IOException("Could not rename the new password file into place,
and unable to restore original file");
+                    reader.close();
                 }
-                
-                _logger.error("Could not rename the new password file into place");
-                throw new IOException("Could not rename the new password file into place");
             }
-            
+
+            swapTempFileToLive(_passwordFile, tmp);
+
         }
         finally
         {
             _userUpdate.unlock();
         }
     }
+
+    private void swapTempFileToLive(final File live, final File temp) throws IOException
+    {
+        // Remove any existing ".old" file
+        final File old = new File(live.getAbsoluteFile() + ".old");
+        if (old.exists())
+        {
+            old.delete();
+        }
+
+        // Create an new ".old" file
+        if(!live.renameTo(old))
+        {
+            //unable to rename the existing file to the backup name
+            _logger.error("Could not backup the existing password file");
+            throw new IOException("Could not backup the existing password file");
+        }
+
+        // Move temp file to be the new "live" file
+        if(!temp.renameTo(live))
+        {
+            //failed to rename the new file to the required filename
+            if(!old.renameTo(live))
+            {
+                //unable to return the backup to required filename
+                _logger.error("Could not rename the new password file into place, and unable
to restore original file");
+                throw new IOException("Could not rename the new password file into place,
and unable to restore original file");
+            }
+
+            _logger.error("Could not rename the new password file into place");
+            throw new IOException("Could not rename the new password file into place");
+        }
+    }
+
+    private File createTempFileOnSameFilesystem(final File liveFile)
+    {
+        File tmp;
+        final Random r = new Random();
+
+        do
+        {
+            tmp = new File(liveFile.getPath() + r.nextInt() + ".tmp");
+        }
+        while(tmp.exists());
+
+        tmp.deleteOnExit();
+        return tmp;
+    }
     
     public void reload() throws IOException
     {

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java?rev=1213636&r1=1213635&r2=1213636&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java
(original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java
Tue Dec 13 10:19:08 2011
@@ -142,6 +142,19 @@ public class DerbyMessageStore implement
     private boolean _configured;
 
 
+    private static final class CommitStoreFuture implements StoreFuture
+    {
+        public boolean isComplete()
+        {
+            return true;
+        }
+
+        public void waitForCompletion()
+        {
+
+        }
+    }
+
     private enum State
     {
         INITIAL,
@@ -910,19 +923,19 @@ public class DerbyMessageStore implement
             throws AMQStoreException
     {
         Connection conn = null;
+        PreparedStatement stmt = null;
 
         try
         {
             conn = newAutoCommitConnection();
             // exchange_name varchar(255) not null, queue_name varchar(255) not null, binding_key
varchar(255), arguments blob
-            PreparedStatement stmt = conn.prepareStatement(DELETE_FROM_BINDINGS);
+            stmt = conn.prepareStatement(DELETE_FROM_BINDINGS);
             stmt.setString(1, exchange.getNameShortString().toString() );
             stmt.setString(2, queue.getNameShortString().toString());
             stmt.setString(3, routingKey == null ? null : routingKey.toString());
-            
+
             int result = stmt.executeUpdate();
-            stmt.close();
-            
+
             if(result != 1)
             {
                  throw new AMQStoreException("Queue binding for queue with name " + queue.getNameShortString()
+ " to exchange "
@@ -936,21 +949,9 @@ public class DerbyMessageStore implement
         }
         finally
         {
-            if(conn != null)
-            {
-               try
-               {
-                   conn.close();
-               }
-               catch (SQLException e)
-               {
-                   _logger.error(e);
-               }
-            }
-
+            closePreparedStatement(stmt);
+            closeConnection(conn);
         }
-
-
     }
 
     public void createQueue(AMQQueue queue) throws AMQStoreException
@@ -1153,15 +1154,14 @@ public class DerbyMessageStore implement
         AMQShortString name = queue.getNameShortString();
         _logger.debug("public void removeQueue(AMQShortString name = " + name + "): called");
         Connection conn = null;
-
+        PreparedStatement stmt = null;
         try
         {
             conn = newAutoCommitConnection();
-            PreparedStatement stmt = conn.prepareStatement(DELETE_FROM_QUEUE);
+            stmt = conn.prepareStatement(DELETE_FROM_QUEUE);
             stmt.setString(1, name.toString());
             int results = stmt.executeUpdate();
-            stmt.close();
-            
+
             if (results == 0)
             {
                 throw new AMQStoreException("Queue " + name + " not found");
@@ -1173,18 +1173,8 @@ public class DerbyMessageStore implement
         }
         finally
         {
-            if(conn != null)
-            {
-               try
-               {
-                   conn.close();
-               }
-               catch (SQLException e)
-               {
-                   _logger.error(e);
-               }
-            }
-
+            closePreparedStatement(stmt);
+            closeConnection(conn);
         }
 
 
@@ -1317,19 +1307,7 @@ public class DerbyMessageStore implement
     public StoreFuture commitTranAsync(ConnectionWrapper connWrapper) throws AMQStoreException
     {
         commitTran(connWrapper);
-        return new StoreFuture()
-                    {
-                        public boolean isComplete()
-                        {
-                            return true;
-                        }
-
-                        public void waitForCompletion()
-                        {
-
-                        }
-                    };
-
+        return new CommitStoreFuture();
     }
 
     public void abortTran(ConnectionWrapper connWrapper) throws AMQStoreException
@@ -1572,6 +1550,7 @@ public class DerbyMessageStore implement
         {
             _logger.debug("Adding content chunk offset " + offset + " for message " +messageId);
         }
+        PreparedStatement stmt = null;
 
         try
         {
@@ -1580,7 +1559,7 @@ public class DerbyMessageStore implement
             byte[] chunkData = new byte[src.limit()];
             src.duplicate().get(chunkData);
 
-            PreparedStatement stmt = conn.prepareStatement(INSERT_INTO_MESSAGE_CONTENT);
+            stmt = conn.prepareStatement(INSERT_INTO_MESSAGE_CONTENT);
             stmt.setLong(1,messageId);
             stmt.setInt(2, offset);
             stmt.setInt(3, offset+chunkData.length);
@@ -1594,24 +1573,16 @@ public class DerbyMessageStore implement
             ByteArrayInputStream bis = new ByteArrayInputStream(chunkData);
             stmt.setBinaryStream(4, bis, chunkData.length);
             stmt.executeUpdate();
-            stmt.close();
         }
         catch (SQLException e)
         {
-            if(conn != null)
-            {
-                try
-                {
-                    conn.close();
-                }
-                catch (SQLException e1)
-                {
-
-                }
-            }
-
+            closeConnection(conn);
             throw new RuntimeException("Error adding content chunk offset " + offset + "
for message " + messageId + ": " + e.getMessage(), e);
         }
+        finally
+        {
+            closePreparedStatement(stmt);
+        }
 
     }
 
@@ -1619,13 +1590,13 @@ public class DerbyMessageStore implement
     public int getContent(long messageId, int offset, ByteBuffer dst)
     {
         Connection conn = null;
-
+        PreparedStatement stmt = null;
 
         try
         {
             conn = newAutoCommitConnection();
 
-            PreparedStatement stmt = conn.prepareStatement(SELECT_FROM_MESSAGE_CONTENT);
+            stmt = conn.prepareStatement(SELECT_FROM_MESSAGE_CONTENT);
             stmt.setLong(1,messageId);
             stmt.setInt(2, offset);
             stmt.setInt(3, offset+dst.remaining());
@@ -1656,28 +1627,18 @@ public class DerbyMessageStore implement
                 }
             }
 
-            stmt.close();
-            conn.close();
             return written;
 
         }
         catch (SQLException e)
         {
-            if(conn != null)
-            {
-                try
-                {
-                    conn.close();
-                }
-                catch (SQLException e1)
-                {
-
-                }
-            }
-
             throw new RuntimeException("Error retrieving content from offset " + offset +
" for message " + messageId + ": " + e.getMessage(), e);
         }
-
+        finally
+        {
+            closePreparedStatement(stmt);
+            closeConnection(conn);
+        }
 
 
     }
@@ -1849,5 +1810,33 @@ public class DerbyMessageStore implement
         }
     }
 
+    private void closeConnection(final Connection conn)
+    {
+        if(conn != null)
+        {
+           try
+           {
+               conn.close();
+           }
+           catch (SQLException e)
+           {
+               _logger.error("Problem closing connection", e);
+           }
+        }
+    }
 
+    private void closePreparedStatement(final PreparedStatement stmt)
+    {
+        if (stmt != null)
+        {
+            try
+            {
+                stmt.close();
+            }
+            catch(SQLException e)
+            {
+                _logger.error("Problem closing prepared statement", e);
+            }
+        }
+    }
 }



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org


Mime
View raw message