Automatically verify checksum after upload/download

[robinrodricks]

UploadFile() and DownloadFile() should provide an arg to verify the checksum after upload/download is complete. Failure of hash match should:

1. Throw an exception immediately
2. Retry a fixed number of times before giving up and throwing an exception
3. Keep the invalid file and return false
4. Delete the invalid file and return false

[jblacker]

@hgupta9 Want me to wrap this functionality into my work for #83?

[robinrodricks]

Sure, how do you plan on doing it? Could you make the API like this?

For error handling

For #83 - FtpError enum, [Flags] meaning you can combine flags

• FtpError.None = 0 - Ignore the error and continue (default)
• FtpError.Stop = 1 - Stop uploading/downloading immediately and return false (do not upload/download remaining files)
• FtpError.Throw = 2 - Throw immediately when an exception is encountered
• FtpError.DeleteSource = 4 - Delete the source files that uploaded/downloaded successfully

For checksum

For #92 - FtpVerify enum, [Flags] meaning you can combine flags

• FtpVerify.None = 0 - No verification of checksum (default)
• FtpVerify.Checksum = 1 - Verify checksum. Add other flags to delete the file or throw an exception on mismatch.
• FtpVerify.Retry = 2 - Verify checksum. If mismatch then retry a fixed number of times. Add other flags to delete the file or throw an exception.
• FtpVerify.Delete = 4 - If mismatch then delete the invalid file and return false
• FtpVerify.Throw = 8 - If mismatch then throw an exception immediately

For UploadFile/DownloadFile

• UploadFile(..., FtpError errorMode = FtpError.None, FtpVerify verifyMode = FtpVerify.None)

The code to read the flags would be something like:

bool verifyChecksum = ((verifyMode & FtpVerify.Checksum) == FtpVerify.Checksum) || ((verifyMode & FtpVerify.Retry) == FtpVerify.Retry);
bool verRetry = (verifyMode & FtpVerify.Retry) == FtpVerify.Retry;
bool verDelete = (verifyMode & FtpVerify.Delete) == FtpVerify.Delete;
bool verThrow = (verifyMode & FtpVerify.Throw) == FtpVerify.Throw;

This is just a suggestion. You can use my comments for the enum properties.

Do you have any better ideas?

[robinrodricks]

I hope your changes can be merged automatically since I have made substantial changes to the codebase, especially in FtpClient.

[robinrodricks]

As for the hashing, this should work:

// If server supports the HASH command
if (client.HashAlgorithms != FtpHashAlgorithm.NONE) {

	// Ask the server to compute the hash using whatever 
	// the default hash algorithm (probably SHA-1)
	FtpHash hash = client.GetHash("/path/to/remote/somefile.ext");

	// The FtpHash.Verify method computes the hash of the
	// specified file or stream based on the hash algorithm
	// the server computed its hash with.
	if (hash.Verify("/path/to/local/somefile.ext")) {
		// MATCH
	}else{
		// MISMATCH
	}
}

[robinrodricks]

@jblacker Can you re-fork my version since I've just made significant changes to FtpClient and the async methods as well.

[jblacker]

@hgupta9 Yeah. I keep pulling your changes into my fork. Don't worry.

Do you think it would be best to keep expanding the signature of should we create an Options object of some sort like a struct that contains all of the various optional settings?

Something like:

struct FtpOptions
{
    public FtpErrorHandling ErrorHandling { get; set; }
    public FtpValidation ValidationOption { get; set; }
     //etc...
}

[robinrodricks]

Good point, but I would prefer you expand the signature. I just hate objects inside objects. Plus it allows users to clearly see what the function is capable of. Its still in beta and this time I'm introducing breaking changes in the API (thus the major version increment) so if it appears overbearing we can move to an object later.

[jblacker]

I should have a PR for you on this and #83 this weekend.

[jblacker]

@hgupta9 In order to implement the retry option, I added an AttemptsAllowed property to FtpClient. Do you think retries should apply to more than just the verification, and instead for all failures?

[robinrodricks]

What other features could they apply to? Upload/download? I dunno. Some times retrying does not improve reliability it only reduces performance, so I dunno. Maybe if we can test that retrying actually succeeds the 2nd time when it fails the first time. Start off with verification for now, maybe later it can be applied to other things, unless you know for a fact that retrying would benefit other features - then add them in I guess.

[jblacker]

@hgupta9 You're right, it's probably unnecessary at this point for any other failure type.