birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[LYN-2964][LYN-2965] Improve user experience of using AWSScriptBehaviorS3 (#495)

Browse Source 
As AWS S3 GetObject doesn't provide proper file handling, move logic into custom request validation step
This commit is contained in:
Vincent Liu
2021-05-03 16:16:19 -07:00
committed by GitHub
parent 4485edf77d
commit dfe57c9d8f
3 changed files with 119 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/AWSCore/Code/Include/Public/ScriptCanvas/AWSScriptBehaviorS3.h
+11 -1
View File
@@ -75,6 +75,16 @@ namespace AWSCore
class AWSScriptBehaviorS3
: public AWSScriptBehaviorBase
{
static constexpr const char AWSScriptBehaviorS3Name[] = "AWSScriptBehaviorS3";
static constexpr const char OutputFileIsEmptyErrorMessage[] = "Request validation failed, output file is empty.";
static constexpr const char OutputFileMissFullPathErrorMessage[] = "Request validation failed, output file miss full path.";
static constexpr const char OutputFileIsDirectoryErrorMessage[] = "Request validation failed, output file is a directory.";
static constexpr const char OutputFileDirectoryNotExistErrorMessage[] = "Request validation failed, output file directory doesn't exist.";
static constexpr const char OutputFileIsReadOnlyErrorMessage[] = "Request validation failed, output file is read-only.";
static constexpr const char BucketNameIsEmptyErrorMessage[] = "Request validation failed, bucket name is empty";
static constexpr const char ObjectKeyNameIsEmptyErrorMessage[] = "Request validation failed, object key name is empty.";
static constexpr const char RegionNameIsEmptyErrorMessage[] = "Request validation failed, region name is empty.";
public:
AWS_SCRIPT_BEHAVIOR_DEFINITION(AWSScriptBehaviorS3, "{7F4E956C-7463-4236-B320-C992D36A9C6E}");
@@ -87,7 +97,7 @@ namespace AWSCore
private:
using S3NotificationFunctionType = void(AWSScriptBehaviorS3Notifications::*)(const AZStd::string&);
static bool ValidateGetObjectRequest(S3NotificationFunctionType notificationFunc,
const AZStd::string& bucket, const AZStd::string& objectKey, const AZStd::string& region, const AZStd::string& outFile);
const AZStd::string& bucket, const AZStd::string& objectKey, const AZStd::string& region, AZStd::string& outFile);
static bool ValidateHeadObjectRequest(S3NotificationFunctionType notificationFunc,
const AZStd::string& bucket, const AZStd::string& key, const AZStd::string& region);
Gems/AWSCore/Code/Source/ScriptCanvas/AWSScriptBehaviorS3.cpp
+42 -16
View File
@@ -43,7 +43,7 @@ namespace AWSCore
void AWSScriptBehaviorS3::ReflectBehaviors(AZ::BehaviorContext* behaviorContext)
{
behaviorContext->Class<AWSScriptBehaviorS3>("AWSScriptBehaviorS3")
behaviorContext->Class<AWSScriptBehaviorS3>(AWSScriptBehaviorS3Name)
->Attribute(AZ::Script::Attributes::Category, "AWSCore")
->Method("GetObject", &AWSScriptBehaviorS3::GetObject,
{{{"Bucket Resource KeyName", "The resource key name of the bucket in resource mapping config file."},
@@ -86,7 +86,9 @@ namespace AWSCore
void AWSScriptBehaviorS3::GetObjectRaw(
const AZStd::string& bucket, const AZStd::string& objectKey, const AZStd::string& region, const AZStd::string& outFile)
{
if (!ValidateGetObjectRequest(&AWSScriptBehaviorS3NotificationBus::Events::OnGetObjectError, bucket, objectKey, region, outFile))
AZStd::string normalizedOutFile = outFile;
if (!ValidateGetObjectRequest(
&AWSScriptBehaviorS3NotificationBus::Events::OnGetObjectError, bucket, objectKey, region, normalizedOutFile))
{
return;
}
@@ -112,10 +114,10 @@ namespace AWSCore
job->request.SetBucket(Aws::String(bucket.c_str()));
job->request.SetKey(Aws::String(objectKey.c_str()));
Aws::String outFileName(outFile.c_str());
Aws::String outFileName(normalizedOutFile.c_str());
job->request.SetResponseStreamFactory([outFileName]() {
return Aws::New<Aws::FStream>(
"AWSScriptBehaviorS3", outFileName.c_str(),
AWSScriptBehaviorS3Name, outFileName.c_str(),
std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc);
});
job->Start();
@@ -163,20 +165,44 @@ namespace AWSCore
}
bool AWSScriptBehaviorS3::ValidateGetObjectRequest(S3NotificationFunctionType notificationFunc,
const AZStd::string& bucket, const AZStd::string& objectKey, const AZStd::string& region, const AZStd::string& outFile)
const AZStd::string& bucket, const AZStd::string& objectKey, const AZStd::string& region, AZStd::string& outFile)
{
if (ValidateHeadObjectRequest(notificationFunc, bucket, objectKey, region))
{
if (!AzFramework::StringFunc::Path::IsValid(outFile.c_str()))
AzFramework::StringFunc::Path::Normalize(outFile);
if (outFile.empty())
{
AZ_Warning("AWSScriptBehaviorS3", false, "Request validation failed, outfile is not valid.");
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, "Request validation failed, outfile is not valid.");
AZ_Warning(AWSScriptBehaviorS3Name, false, OutputFileIsEmptyErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, OutputFileIsEmptyErrorMessage);
return false;
}
if (!AzFramework::StringFunc::Path::HasDrive(outFile.c_str()))
{
AZ_Warning(AWSScriptBehaviorS3Name, false, OutputFileMissFullPathErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, OutputFileMissFullPathErrorMessage);
return false;
}
if (AZ::IO::FileIOBase::GetInstance()->IsDirectory(outFile.c_str()))
{
AZ_Warning(AWSScriptBehaviorS3Name, false, OutputFileIsDirectoryErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, OutputFileIsDirectoryErrorMessage);
return false;
}
auto lastSeparator = outFile.find_last_of(AZ_CORRECT_FILESYSTEM_SEPARATOR);
if (lastSeparator != AZStd::string::npos)
{
auto parentPath = outFile.substr(0, lastSeparator);
if (!AZ::IO::FileIOBase::GetInstance()->Exists(parentPath.c_str()))
{
AZ_Warning(AWSScriptBehaviorS3Name, false, OutputFileDirectoryNotExistErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, OutputFileDirectoryNotExistErrorMessage);
return false;
}
}
if (AZ::IO::FileIOBase::GetInstance()->IsReadOnly(outFile.c_str()))
{
AZ_Warning("AWSScriptBehaviorS3", false, "Request validation failed, outfile is read-only.");
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, "Request validation failed, outfile is read-only.");
AZ_Warning(AWSScriptBehaviorS3Name, false, OutputFileIsReadOnlyErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, OutputFileIsReadOnlyErrorMessage);
return false;
}
return true;
@@ -189,20 +215,20 @@ namespace AWSCore
{
if (bucket.empty())
{
AZ_Warning("AWSScriptBehaviorS3", false, "Request validation failed, bucket name is required.");
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, "Request validation failed, bucket name is required.");
AZ_Warning(AWSScriptBehaviorS3Name, false, BucketNameIsEmptyErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, BucketNameIsEmptyErrorMessage);
return false;
}
if (objectKey.empty())
{
AZ_Warning("AWSScriptBehaviorS3", false, "Request validation failed, object key name is required.");
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, "Request validation failed, object key name is required.");
AZ_Warning(AWSScriptBehaviorS3Name, false, ObjectKeyNameIsEmptyErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, ObjectKeyNameIsEmptyErrorMessage);
return false;
}
if (region.empty())
{
AZ_Warning("AWSScriptBehaviorS3", false, "Request validation failed, region name is required.");
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, "Request validation failed, region name is required.");
AZ_Warning(AWSScriptBehaviorS3Name, false, RegionNameIsEmptyErrorMessage);
AWSScriptBehaviorS3NotificationBus::Broadcast(notificationFunc, RegionNameIsEmptyErrorMessage);
return false;
}
return true;
Gems/AWSCore/Code/Tests/ScriptCanvas/AWSScriptBehaviorS3Test.cpp
+66 -1
View File
@@ -11,6 +11,7 @@
*/
#include <AzCore/RTTI/BehaviorContext.h>
#include <AzFramework/StringFunc/StringFunc.h>
#include <AzTest/AzTest.h>
#include <ScriptCanvas/AWSScriptBehaviorS3.h>
@@ -38,7 +39,37 @@ public:
MOCK_METHOD1(OnGetObjectError, void(const AZStd::string&));
};
using AWSScriptBehaviorS3Test = UnitTest::ScopedAllocatorSetupFixture;
class AWSScriptBehaviorS3Test
: public AWSCoreFixture
{
public:
void CreateReadOnlyTestFile(const AZStd::string& filePath)
{
AZ::IO::SystemFile file;
if (!file.Open(
filePath.c_str(),
AZ::IO::SystemFile::OpenMode::SF_OPEN_CREATE | AZ::IO::SystemFile::SF_OPEN_CREATE_PATH | AZ::IO::SystemFile::SF_OPEN_WRITE_ONLY))
{
AZ_Assert(false, "Failed to open test file at %s", filePath.c_str());
}
AZStd::string testContent = "It is a test file";
if (file.Write(testContent.c_str(), testContent.size()) != testContent.size())
{
AZ_Assert(false, "Failed to write test file with content %s", testContent.c_str());
}
file.Close();
AZ_Assert(AZ::IO::SystemFile::SetWritable(filePath.c_str(), false), "Failed to mark test file as read-only");
}
void RemoveReadOnlyTestFile(const AZStd::string& filePath)
{
if (!filePath.empty())
{
AZ_Assert(AZ::IO::SystemFile::SetWritable(filePath.c_str(), true), "Failed to mark test file as writeable");
AZ_Assert(AZ::IO::SystemFile::Delete(filePath.c_str()), "Failed to delete test config file at %s", filePath.c_str());
}
}
};
TEST_F(AWSScriptBehaviorS3Test, HeadObjectRaw_CallWithEmptyBucketName_InvokeOnError)
{
@@ -96,6 +127,40 @@ TEST_F(AWSScriptBehaviorS3Test, GetObjectRaw_CallWithEmptyOutfileName_InvokeOnEr
AWSScriptBehaviorS3::GetObjectRaw("dummyBucket", "dummyObject", "dummyRegion", "");
}
TEST_F(AWSScriptBehaviorS3Test, GetObjectRaw_CallWithOutfileNameMissFullPath_InvokeOnError)
{
AWSScriptBehaviorS3NotificationBusHandlerMock s3HandlerMock;
EXPECT_CALL(s3HandlerMock, OnGetObjectError(::testing::_)).Times(1);
AWSScriptBehaviorS3::GetObjectRaw("dummyBucket", "dummyObject", "dummyRegion", "dummyOut.txt");
}
TEST_F(AWSScriptBehaviorS3Test, GetObjectRaw_CallWithOutfileNameIsDirectory_InvokeOnError)
{
AWSScriptBehaviorS3NotificationBusHandlerMock s3HandlerMock;
EXPECT_CALL(s3HandlerMock, OnGetObjectError(::testing::_)).Times(1);
AWSScriptBehaviorS3::GetObjectRaw("dummyBucket", "dummyObject", "dummyRegion", AZ::Test::GetCurrentExecutablePath());
}
TEST_F(AWSScriptBehaviorS3Test, GetObjectRaw_CallWithOutfileDirectoryNoExist_InvokeOnError)
{
AWSScriptBehaviorS3NotificationBusHandlerMock s3HandlerMock;
EXPECT_CALL(s3HandlerMock, OnGetObjectError(::testing::_)).Times(1);
AZStd::string dummyDirectory = AZStd::string::format("%s/dummyDirectory/dummyOut.txt", AZ::Test::GetCurrentExecutablePath().c_str());
AWSScriptBehaviorS3::GetObjectRaw("dummyBucket", "dummyObject", "dummyRegion", dummyDirectory);
}
TEST_F(AWSScriptBehaviorS3Test, GetObjectRaw_CallWithOutfileIsReadOnly_InvokeOnError)
{
AWSScriptBehaviorS3NotificationBusHandlerMock s3HandlerMock;
EXPECT_CALL(s3HandlerMock, OnGetObjectError(::testing::_)).Times(1);
AZStd::string randomTestFile = AZStd::string::format("%s/test%s.txt",
AZ::Test::GetCurrentExecutablePath().c_str(), AZ::Uuid::CreateRandom().ToString<AZStd::string>(false, false).c_str());
AzFramework::StringFunc::Path::Normalize(randomTestFile);
CreateReadOnlyTestFile(randomTestFile);
AWSScriptBehaviorS3::GetObjectRaw("dummyBucket", "dummyObject", "dummyRegion", randomTestFile);
RemoveReadOnlyTestFile(randomTestFile);
}
TEST_F(AWSScriptBehaviorS3Test, GetObject_NoBucketNameInResourceMappingFound_InvokeOnError)
{
AWSScriptBehaviorS3NotificationBusHandlerMock s3HandlerMock;