Merge branch 'master' of git.openslx.org:bwlp/satellite-daemon

Conflicts:
	src/main/java/org/openslx/satellitedaemon/ftp/FtpUploadWorker.java

5 files changed, 50 insertions, 27 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 4fe2d61..aa750f2 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;
 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 );
@@ -50,6 +52,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() );
@@ -63,7 +67,11 @@ public class FtpUploadWorker implements Runnable
 					UploadInfos upInfos = ThriftConnection.getUploadInfos( imDat, crcPath );
 					if ( upInfos == null ) {
 						log.error( "The UploadInfos returned by ThriftConnection Class are null" );
+
 						return;
+
+						// FIXME: And then..? If you just continue, you'll run into a null pointer exception
+
 					}
 					log.info( "Got upInfos. Trying to create Uploader with token: " + upInfos.token );
 
@@ -72,31 +80,37 @@ public class FtpUploadWorker implements Runnable
 					u.sendToken( upInfos.token );
 
 					// continue sending Blocks until getMissingBlocks is empty.
-//					while ( !upInfos.getMissingBlocks().isEmpty() ) {
-//						// Send all Blocks from upInfos.getMissingBlocks() in ranges.
-//						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 ) ) {
-//								log.info( "Sending Blocks numbered from " + start + " up to " + i );
-//								u.sendRange( start *16*1024*1024, i*16*1024*1024 );
-//								u.sendFile( image.path );
-//								log.info( "... DONE!" );
-//								start = i + 1;
-//							}
-//							if ( i == blocks.size() - 2 ) {
-//								log.info( "Sending Blocks numbered from " + start + " up to " + i );
-//								u.sendRange( start*16*1024*1024, (blocks.size() - 1)*16*1024*1024 );
-//								u.sendFile( image.path );
-//							}
-//						}
-//					}
-//					upInfos = ThriftConnection.getUploadInfos( imDat );
-					log.info("Using file:" + image.path );
-					u.sendRange( 254, 254 + 255 );
-					u.sendFile( image.path );
+
 					
 
+					while ( !upInfos.getMissingBlocks().isEmpty() ) {
+						// Send all Blocks from upInfos.getMissingBlocks() in ranges.
+						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 ) ) { // 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 ) { // 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();
@@ -122,8 +136,8 @@ public class FtpUploadWorker implements Runnable
 				Thread.sleep( 5 * 60 * 1000 );
 				//				Thread.sleep( 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 5b4dbcd..cacd6c8 100644
--- a/src/main/java/org/openslx/satellitedaemon/ftp/ThriftConnection.java
+++ b/src/main/java/org/openslx/satellitedaemon/ftp/ThriftConnection.java
@@ -29,6 +29,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;