Verifying ContentRange on response from GetObject

[pulimsr]

Verifying ContentRange on response from GetObject

Issue #, if available:

Description of changes:

Check all that applies:

• Did a review by yourself.
• Added proper tests to cover this PR. (If tests are not applicable, explain.)
• Checked if this PR is a breaking (APIs have been changed) change.
• Checked if this PR will not introduce cross-platform inconsistent behavior.
• Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

• Linux
• Windows
• Android
• MacOS
• IOS
• Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[kai-ion]

Should there also be a test with a false content range to test for failures?

[pulimsr]

I'll look into creating a mock S3 response with an incorrect ContentRange to test the failure path

[sbiscigl]

+1 i think even as going as far to have a "transfer manager unit test" might be good. you can largely copy the S3 unit tests to create the skeleton of it.

[sbiscigl]

is there any instance where we set range on the request where s3 does not return a range? would that be a error?

[sbiscigl]

+1 i think even as going as far to have a "transfer manager unit test" might be good. you can largely copy the S3 unit tests to create the skeleton of it.

[sbiscigl]

nit: else if(request.RangeHasBeenSet()) { ... } is preferred to else { if { ... }}

[sbiscigl]

I don't think this is what we meant by saying write unit tests for this. we dont want to copy and paste code to test it, we want to mock the interfaces that make actual calls such that we can test the code we added. I think the idea here is that you would create a transfer manager with a mocked out s3 client, so that when you run the transfer manager it uses this mocked out s3 client instead of a real one, so you can verify behavior without using a real network connection. something like this

#include <aws/core/Aws.h>
#include <aws/core/utils/threading/PooledThreadExecutor.h>
#include <aws/s3/S3Client.h>
#include <aws/transfer/TransferManager.h>

using namespace Aws;
using namespace Aws::Utils;
using namespace Aws::Utils::Logging;
using namespace Aws::Utils::Threading;
using namespace Aws::Transfer;
using namespace Aws::S3;

namespace {
const char* LOGTAG = "TestApplication";

class MockS3Client : public S3Client {
 public:
  Model::CompleteMultipartUploadOutcome
  CompleteMultipartUpload(const Model::CompleteMultipartUploadRequest& request) const override {
   //TODO: implement mock
  }

  Model::CreateMultipartUploadOutcome
  CreateMultipartUpload(const Model::CreateMultipartUploadRequest& request) const override {
    //TODO: implement mock
  }

  Model::GetObjectOutcome
  GetObject(const Model::GetObjectRequest& request) const override {
    //TODO: implement mock
  }

  Model::HeadObjectOutcome
  HeadObject(const Model::HeadObjectRequest& request) const override {
    //TODO: implement mock
  }

  Model::PutObjectOutcome
  PutObject(const Model::PutObjectRequest& request) const override {
    //TODO: implement mock
  }

  Model::UploadPartOutcome
  UploadPart(const Model::UploadPartRequest& request) const override {
    //TODO: implement mock
  }
};

}

class SdkContext {
 public:
  explicit SdkContext(SDKOptions&& options) : options_(std::move(options)) { InitAPI(options_); }
  ~SdkContext() { ShutdownAPI(options_); }

 private:
  SDKOptions options_;
};

auto main() -> int {
  SDKOptions options;
  options.loggingOptions.logLevel = LogLevel::Trace;
  SdkContext context(std::move(options));

  const auto executor = Aws::MakeUnique<PooledThreadExecutor>(LOGTAG, std::thread::hardware_concurrency());
  TransferManagerConfiguration configuration{executor.get()};
  configuration.s3Client = Aws::MakeShared<MockS3Client>("MockS3Client");
  // do testing operations
  return 0;
}

but wrapped by gtest