From 9e11f75a579ab7aaf9d834a816d546a9128a9898 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 2 Jul 2014 22:12:01 +0200 Subject: Added TODOs --- .../satellitedaemon/ftp/FtpDownloadWorker.java | 6 ++++-- .../satellitedaemon/ftp/FtpUploadWorker.java | 22 ++++++++++++++++++---- .../satellitedaemon/ftp/ThriftConnection.java | 2 ++ .../org/openslx/satellitedaemon/util/CrcFile.java | 2 ++ .../util/EncryptWithServerIdPublicKey.java | 3 +++ 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openslx/satellitedaemon/ftp/FtpDownloadWorker.java b/src/main/java/org/openslx/satellitedaemon/ftp/FtpDownloadWorker.java index 76f03cf..d315735 100644 --- a/src/main/java/org/openslx/satellitedaemon/ftp/FtpDownloadWorker.java +++ b/src/main/java/org/openslx/satellitedaemon/ftp/FtpDownloadWorker.java @@ -22,6 +22,8 @@ import org.openslx.satellitedaemon.Globals.PropInt; import org.openslx.satellitedaemon.Globals.PropString; import org.openslx.satellitedaemon.db.DbImage; +// TODO: Pretty much the same todos as in the uploader apply here + public class FtpDownloadWorker implements Runnable { private static Logger log = Logger.getLogger( FtpUploadWorker.class ); @@ -118,8 +120,8 @@ public class FtpDownloadWorker implements Runnable try { Thread.sleep( 5 * 60 * 1000 ); } catch ( InterruptedException e ) { - log.error( "FtpUploadWorker: Sleep interrupted" ); - e.printStackTrace(); + Thread.currentThread().interrupt(); + return; } } diff --git a/src/main/java/org/openslx/satellitedaemon/ftp/FtpUploadWorker.java b/src/main/java/org/openslx/satellitedaemon/ftp/FtpUploadWorker.java index 3ee624e..9276ed9 100644 --- a/src/main/java/org/openslx/satellitedaemon/ftp/FtpUploadWorker.java +++ b/src/main/java/org/openslx/satellitedaemon/ftp/FtpUploadWorker.java @@ -23,6 +23,8 @@ import org.openslx.satellitedaemon.Globals.PropInt; import org.openslx.satellitedaemon.Globals.PropString; import org.openslx.satellitedaemon.db.DbImage; +// TODO: Rename all this FTP* Stuff (also the constants in Globals) now that we're not using ftp anymore + public class FtpUploadWorker implements Runnable { private static Logger log = Logger.getLogger( FtpUploadWorker.class ); @@ -46,6 +48,8 @@ public class FtpUploadWorker implements Runnable try { // All the necessary KeyStore handling for the "context"-item. + // TODO: Don't do this again and again for every image, but once before the loop (or in the constructor) + // Or better yet in a central spot, as the downloadworker needs the context too. Maybe something like Globals.getMasterServerSslContext() keystore = KeyStore.getInstance( "JKS" ); keystore.load( new FileInputStream( Globals.getPropertyString( PropString.FTPSKEYSTOREPATH ) ), passphrase ); TrustManagerFactory tmf = TrustManagerFactory.getInstance( TrustManagerFactory.getDefaultAlgorithm() ); @@ -58,6 +62,7 @@ public class FtpUploadWorker implements Runnable UploadInfos upInfos = ThriftConnection.getUploadInfos( imDat ); if ( upInfos == null ) { log.error( "The UploadInfos returned by ThriftConnection Class are null" ); + // FIXME: And then..? If you just continue, you'll run into a null pointer exception } // creating the uploader with the "context"-item. @@ -70,19 +75,28 @@ public class FtpUploadWorker implements Runnable List blocks = upInfos.getMissingBlocks(); int start = 0; for ( int i = 0; i < blocks.size() - 1; i++ ) { - if ( blocks.get( i ) != ( blocks.get( i + 1 ) - 1 ) ) { + if ( blocks.get( i ) != ( blocks.get( i + 1 ) - 1 ) ) { // TODO: !? u.sendRange expects byte values, upInfos contains block indexes (16MiB) u.sendRange( start, i ); u.sendFile( image.path ); start = i + 1; } - if ( i == blocks.size() - 2 ) { + if ( i == blocks.size() - 2 ) { // TODO: != u.sendRange( start, blocks.size() - 1 ); u.sendFile( image.path ); } } } + // FIXME: Should be inside loop, otherwise you'll only ever upload the first 20 blocks upInfos = ThriftConnection.getUploadInfos( imDat ); + // TODO: Use more local try/catch-blocks, not a big one around everything, so you know what exactly caused the exception + // and can handle it properly. There are roughly 3 types of error/exception you can encounter: + // 1) Expected exception: Something you expect to go wrong, so you handle it in the catch block and let the code flow continue + // 2) Minor exception: Something went wrong and you can't continue trying to upload that image. Try to clean things up that + // you already did (like opening a file, opening a network connection) and then continue with the next image + // 3) Major messup: Something went wrong that will prevent the whole application from continuing to work. Try to finish or cancel + // any operation going on in your application and then exit (and don't forget to log meaningful information about the error) + // Basically all those empty catch blocks below should be moved up and filled with logic. } catch ( NoSuchAlgorithmException e ) { // TODO Auto-generated catch block e.printStackTrace(); @@ -107,8 +121,8 @@ public class FtpUploadWorker implements Runnable try { Thread.sleep( 5 * 60 * 1000 ); } catch ( InterruptedException e ) { - log.error( "FtpUploadWorker: Sleep interrupted" ); - e.printStackTrace(); + Thread.currentThread().interrupt(); + return; } } diff --git a/src/main/java/org/openslx/satellitedaemon/ftp/ThriftConnection.java b/src/main/java/org/openslx/satellitedaemon/ftp/ThriftConnection.java index 2fbc471..c831435 100644 --- a/src/main/java/org/openslx/satellitedaemon/ftp/ThriftConnection.java +++ b/src/main/java/org/openslx/satellitedaemon/ftp/ThriftConnection.java @@ -28,6 +28,8 @@ import org.openslx.satellitedaemon.Globals.PropString; import org.openslx.satellitedaemon.db.DbImage; import org.openslx.satellitedaemon.util.EncryptWithServerIdPublicKey; +// TODO: Handle all the auto-generated catch blocks in a meaningful way + /** * Handles the authentication with the Satellite Server and sends the FtpCredentials, which * are necessary for the upload of the image. diff --git a/src/main/java/org/openslx/satellitedaemon/util/CrcFile.java b/src/main/java/org/openslx/satellitedaemon/util/CrcFile.java index ab8f190..59679d3 100644 --- a/src/main/java/org/openslx/satellitedaemon/util/CrcFile.java +++ b/src/main/java/org/openslx/satellitedaemon/util/CrcFile.java @@ -11,6 +11,8 @@ import java.util.zip.CRC32; import org.apache.log4j.BasicConfigurator; import org.apache.log4j.Logger; +// TODO: Merge with code/classes from masterser and put into master-sync-shared + public class CrcFile { private static Logger log = Logger.getLogger( CrcFile.class ); diff --git a/src/main/java/org/openslx/satellitedaemon/util/EncryptWithServerIdPublicKey.java b/src/main/java/org/openslx/satellitedaemon/util/EncryptWithServerIdPublicKey.java index 357472d..9a064f8 100644 --- a/src/main/java/org/openslx/satellitedaemon/util/EncryptWithServerIdPublicKey.java +++ b/src/main/java/org/openslx/satellitedaemon/util/EncryptWithServerIdPublicKey.java @@ -18,6 +18,9 @@ import java.security.UnrecoverableKeyException; import java.security.cert.Certificate; import java.security.cert.CertificateException; +// TODO: More general naming; this isn't really limited to serverids... +// Might also be worth moving this encrypt/decrypt stuff from satserver and masterserver to the shared project (one class doing both) + public class EncryptWithServerIdPublicKey { KeyPair pair; -- cgit v1.2.3-55-g7522