Added TODOs

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<Integer> 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;