From 2ce7bbd945815d62ea29fa660466d47840e36e85 Mon Sep 17 00:00:00 2001 From: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> Date: Fri, 24 Sep 2021 17:36:26 -0500 Subject: [PATCH] Moved the SettingsRegistryFileReader class to more general IO Reader (#4189) The new FileReader class allows using either a FileIOBase or SystemFile classes for reading files. If a non-nullptr FileIOBase is supplied, the FileIOBase is used, otherwise SystemFile is used. Updated the SettingsRegistryMergeUtils `MergeSettingsToRegistry_ConfigFile` function to use the FileReader. This allows .cfg such as the autoexec.cfg to be read using FileIO, therefore allowing it to be read from the engine.pak file. Because the AZ::Console::ExecuteConfigFile uses the `MergeSettingsToRegistry_ConfigFile` function for merging *.cfg files to the SettingsRegistry, this allows the AZ::Console to run console commands from files within pak files thanks to the ArchiveFileIO being the default FileIOBase instance Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- .../Framework/AzCore/AzCore/IO/FileReader.cpp | 200 ++++++++++++++++++ Code/Framework/AzCore/AzCore/IO/FileReader.h | 92 ++++++++ .../Framework/AzCore/AzCore/IO/SystemFile.cpp | 4 +- Code/Framework/AzCore/AzCore/IO/SystemFile.h | 4 +- .../AzCore/Settings/SettingsRegistryImpl.cpp | 115 +--------- .../Settings/SettingsRegistryMergeUtils.cpp | 36 +++- .../Settings/SettingsRegistryMergeUtils.h | 9 + .../AzCore/AzCore/azcore_files.cmake | 2 + .../AzCore/Tests/IO/FileReaderTests.cpp | 72 +++++++ .../AzCore/Tests/azcoretests_files.cmake | 1 + 10 files changed, 415 insertions(+), 120 deletions(-) create mode 100644 Code/Framework/AzCore/AzCore/IO/FileReader.cpp create mode 100644 Code/Framework/AzCore/AzCore/IO/FileReader.h create mode 100644 Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp diff --git a/Code/Framework/AzCore/AzCore/IO/FileReader.cpp b/Code/Framework/AzCore/AzCore/IO/FileReader.cpp new file mode 100644 index 0000000000..94118cdfe4 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/IO/FileReader.cpp @@ -0,0 +1,200 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ + +#include +#include +#include + +namespace AZ::IO +{ + FileReader::FileReader() = default; + + FileReader::FileReader(AZ::IO::FileIOBase* fileIoBase, const char* filePath) + { + Open(fileIoBase, filePath); + } + + FileReader::~FileReader() + { + Close(); + } + + FileReader::FileReader(FileReader&& other) + { + AZStd::swap(m_file, other.m_file); + AZStd::swap(m_fileIoBase, other.m_fileIoBase); + } + + FileReader& FileReader::operator=(FileReader&& other) + { + // Close the current file and take over other file + Close(); + m_file = AZStd::move(other.m_file); + m_fileIoBase = AZStd::move(other.m_fileIoBase); + other.m_file = AZStd::monostate{}; + other.m_fileIoBase = {}; + + return *this; + } + + bool FileReader::Open(AZ::IO::FileIOBase* fileIoBase, const char* filePath) + { + // Close file if the FileReader has an instance open + Close(); + + if (fileIoBase != nullptr) + { + AZ::IO::HandleType fileHandle; + if (fileIoBase->Open(filePath, IO::OpenMode::ModeRead, fileHandle)) + { + m_file = fileHandle; + m_fileIoBase = fileIoBase; + return true; + } + } + else + { + AZ::IO::SystemFile file; + if (file.Open(filePath, IO::SystemFile::OpenMode::SF_OPEN_READ_ONLY)) + { + m_file = AZStd::move(file); + return true; + } + } + + return false; + } + + bool FileReader::IsOpen() const + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + return *fileHandle != AZ::IO::InvalidHandle; + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->IsOpen(); + } + + return false; + } + + void FileReader::Close() + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (AZ::IO::FileIOBase* fileIo = m_fileIoBase; fileIo != nullptr) + { + fileIo->Close(*fileHandle); + } + } + + m_file = AZStd::monostate{}; + m_fileIoBase = {}; + } + + auto FileReader::Length() const -> SizeType + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (SizeType fileSize{}; m_fileIoBase->Size(*fileHandle, fileSize)) + { + return fileSize; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Length(); + } + + return 0; + } + + auto FileReader::Read(SizeType byteSize, void* buffer) -> SizeType + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (SizeType bytesRead{}; m_fileIoBase->Read(*fileHandle, buffer, byteSize, false, &bytesRead)) + { + return bytesRead; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Read(byteSize, buffer); + } + + return 0; + } + + auto FileReader::Tell() const -> SizeType + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + if (SizeType fileOffset{}; m_fileIoBase->Tell(*fileHandle, fileOffset)) + { + return fileOffset; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Tell(); + } + + return 0; + } + + bool FileReader::Seek(AZ::s64 offset, SeekType type) + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + return m_fileIoBase->Seek(*fileHandle, offset, type); + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + systemFile->Seek(offset, static_cast(type)); + return true; + } + + return false; + } + + bool FileReader::Eof() const + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + return m_fileIoBase->Eof(*fileHandle); + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + return systemFile->Eof(); + } + + return false; + } + + bool FileReader::GetFilePath(AZ::IO::FixedMaxPath& filePath) const + { + if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) + { + AZ::IO::FixedMaxPathString& pathStringRef = filePath.Native(); + if (m_fileIoBase->GetFilename(*fileHandle, pathStringRef.data(), pathStringRef.capacity())) + { + pathStringRef.resize_no_construct(AZStd::char_traits::length(pathStringRef.data())); + return true; + } + } + else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) + { + filePath = systemFile->Name(); + return true; + } + + return false; + } +} diff --git a/Code/Framework/AzCore/AzCore/IO/FileReader.h b/Code/Framework/AzCore/AzCore/IO/FileReader.h new file mode 100644 index 0000000000..4fdb18b2b2 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/IO/FileReader.h @@ -0,0 +1,92 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#pragma once + +#include +#include +#include + +namespace AZ::IO +{ + class FileIOBase; + enum class SeekType : AZ::u32; + + //! Structure which encapsulates delegates File Read operations + //! to either the FileIOBase or SystemFile classes based if a FileIOBase* instance has been supplied + //! to the FileSystemReader class + //! the SettingsRegistry option to use FileIO + class FileReader + { + using HandleType = AZ::u32; + using FileHandleType = AZStd::variant; + public: + using SizeType = AZ::u64; + + //! Creates FileReader instance in the default state with no file opend + FileReader(); + ~FileReader(); + + //! Creates a new FileReader instance and attempts to open the file at the supplied path + //! Uses the FileIOBase instance if supplied + //! @param fileIOBase pointer to fileIOBase instance + //! @param null-terminated filePath to open + FileReader(AZ::IO::FileIOBase* fileIoBase, const char* filePath); + + //! Takes ownership of the supplied FileReader handle + FileReader(FileReader&& other); + + //! Moves ownership of FileReader handle to this instance + FileReader& operator=(FileReader&& other); + + //! Opens a File using the FileIOBase instance if non-nullptr + //! Otherwise fall back to use SystemFile + //! @param fileIOBase pointer to fileIOBase instance + //! @param null-terminated filePath to open + //! @return true if the File is opened successfully + bool Open(AZ::IO::FileIOBase* fileIoBase, const char* filePath); + + //! Returns true if a file is currently open + //! @return true if the file is open + bool IsOpen() const; + + //! Closes the File + void Close(); + + //! Retrieve the length of the OpenFile + SizeType Length() const; + + //! Attempts to read up to byte size bytes into the supplied buffer + //! @param byteSize - Maximum number of bytes to read + //! @param buffer - Buffer to read bytes into + //! @returns the number of bytes read if the file is open, otherwise 0 + SizeType Read(SizeType byteSize, void* buffer); + + //! Returns the current file offset + //! @returns file offset if the file is open, otherwise 0 + SizeType Tell() const; + + //! Seeks within the open file to the offset supplied + //! @param offset File offset to seek to + //! @param type parameter to indicate the reference point to start the seek from + //! @returns true if the file is open and the seek succeeded + bool Seek(AZ::s64 offset, SeekType type); + + //! Returns true if the file is open and in the EOF state + bool Eof() const; + + //! Store the file path of the open file into the output file path parameter + //! The filePath reference is left unmodified, if the path was not stored + //! @return true if the filePath was stored + bool GetFilePath(AZ::IO::FixedMaxPath& filePath) const; + + private: + + FileHandleType m_file; + AZ::IO::FileIOBase* m_fileIoBase{}; + }; +} diff --git a/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp b/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp index 5bff79b422..651abb89fe 100644 --- a/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp +++ b/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp @@ -160,12 +160,12 @@ void SystemFile::Seek(SeekSizeType offset, SeekMode mode) Platform::Seek(m_handle, this, offset, mode); } -SystemFile::SizeType SystemFile::Tell() +SystemFile::SizeType SystemFile::Tell() const { return Platform::Tell(m_handle, this); } -bool SystemFile::Eof() +bool SystemFile::Eof() const { return Platform::Eof(m_handle, this); } diff --git a/Code/Framework/AzCore/AzCore/IO/SystemFile.h b/Code/Framework/AzCore/AzCore/IO/SystemFile.h index 8a5b2b2521..551ce89ce7 100644 --- a/Code/Framework/AzCore/AzCore/IO/SystemFile.h +++ b/Code/Framework/AzCore/AzCore/IO/SystemFile.h @@ -72,9 +72,9 @@ namespace AZ /// Seek in current file. void Seek(SeekSizeType offset, SeekMode mode); /// Get the cursor position in the current file. - SizeType Tell(); + SizeType Tell() const; /// Is the cursor at the end of the file? - bool Eof(); + bool Eof() const; /// Get the time the file was last modified. AZ::u64 ModificationTime(); /// Read data from a file synchronous. Return number of bytes actually read in the buffer. diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp index 7ef1fa661d..92f546815e 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1116,118 +1117,6 @@ namespace AZ } } - //! Structure which encapsulates Commands to either the FileIOBase or SystemFile classes based on - //! the SettingsRegistry option to use FileIO - struct SettingsRegistryFileReader - { - using FileHandleType = AZStd::variant; - - SettingsRegistryFileReader() = default; - SettingsRegistryFileReader(bool useFileIo, const char* filePath) - { - Open(useFileIo, filePath); - } - - ~SettingsRegistryFileReader() - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (AZ::IO::FileIOBase* fileIo = AZ::IO::FileIOBase::GetInstance(); fileIo != nullptr) - { - fileIo->Close(*fileHandle); - } - } - } - - bool Open(bool useFileIo, const char* filePath) - { - Close(); - if (AZ::IO::FileIOBase* fileIo = useFileIo ? AZ::IO::FileIOBase::GetInstance() : nullptr; fileIo != nullptr) - { - AZ::IO::HandleType fileHandle; - if (fileIo->Open(filePath, IO::OpenMode::ModeRead, fileHandle)) - { - m_file = fileHandle; - return true; - } - } - else - { - AZ::IO::SystemFile file; - if (file.Open(filePath, IO::SystemFile::OpenMode::SF_OPEN_READ_ONLY)) - { - m_file = AZStd::move(file); - return true; - } - } - - return false; - } - - bool IsOpen() const - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - return *fileHandle != AZ::IO::InvalidHandle; - } - else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) - { - return systemFile->IsOpen(); - } - - return false; - } - - void Close() - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (AZ::IO::FileIOBase* fileIo = AZ::IO::FileIOBase::GetInstance(); fileIo != nullptr) - { - fileIo->Close(*fileHandle); - } - } - - m_file = AZStd::monostate{}; - } - - u64 Length() const - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (u64 fileSize{}; AZ::IO::FileIOBase::GetInstance()->Size(*fileHandle, fileSize)) - { - return fileSize; - } - } - else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) - { - return systemFile->Length(); - } - - return 0; - } - - AZ::IO::SizeType Read(AZ::IO::SizeType byteSize, void* buffer) - { - if (auto fileHandle = AZStd::get_if(&m_file); fileHandle != nullptr) - { - if (AZ::u64 bytesRead{}; AZ::IO::FileIOBase::GetInstance()->Read(*fileHandle, buffer, byteSize, false, &bytesRead)) - { - return bytesRead; - } - } - else if (auto systemFile = AZStd::get_if(&m_file); systemFile != nullptr) - { - return systemFile->Read(byteSize, buffer); - } - - return 0; - } - - FileHandleType m_file; - }; - bool SettingsRegistryImpl::MergeSettingsFileInternal(const char* path, Format format, AZStd::string_view rootKey, AZStd::vector& scratchBuffer) { @@ -1236,7 +1125,7 @@ namespace AZ Pointer pointer(AZ_SETTINGS_REGISTRY_HISTORY_KEY "/-"); - SettingsRegistryFileReader fileReader(m_useFileIo, path); + FileReader fileReader(m_useFileIo ? AZ::IO::FileIOBase::GetInstance(): nullptr, path); if (!fileReader.IsOpen()) { AZ_Error("Settings Registry", false, R"(Unable to open registry file "%s".)", path); diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp index 5290c9b02a..da7110e36e 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp @@ -6,6 +6,8 @@ * */ +#include +#include #include #include #include @@ -388,8 +390,36 @@ namespace AZ::SettingsRegistryMergeUtils const ConfigParserSettings& configParserSettings) { auto configPath = FindEngineRoot(registry) / filePath; - IO::SystemFile configFile; - if (!configFile.Open(configPath.c_str(), IO::SystemFile::OpenMode::SF_OPEN_READ_ONLY)) + IO::FileReader configFile; + bool configFileOpened{}; + switch (configParserSettings.m_fileReaderClass) + { + case ConfigParserSettings::FileReaderClass::UseFileIOIfAvailableFallbackToSystemFile: + { + auto fileIo = AZ::IO::FileIOBase::GetInstance(); + configFileOpened = configFile.Open(fileIo, configPath.c_str()); + break; + } + case ConfigParserSettings::FileReaderClass::UseSystemFileOnly: + { + configFileOpened = configFile.Open(nullptr, configPath.c_str()); + break; + } + case ConfigParserSettings::FileReaderClass::UseFileIOOnly: + { + auto fileIo = AZ::IO::FileIOBase::GetInstance(); + if (fileIo == nullptr) + { + return false; + } + configFileOpened = configFile.Open(fileIo, configPath.c_str()); + break; + } + default: + AZ_Error("SettingsRegistryMergeUtils", false, "An Invalid FileReaderClass enum value has been supplied"); + return false; + } + if (!configFileOpened) { AZ_Warning("SettingsRegistryMergeUtils", false, R"(Unable to open file "%s")", configPath.c_str()); return false; @@ -480,7 +510,7 @@ namespace AZ::SettingsRegistryMergeUtils AZ_Error("SettingsRegistryMergeUtils", false, R"(The config file "%s" contains a line which is longer than the max line length of %zu.)" "\n" R"(Parsing will halt. The line content so far is:)" "\n" - R"("%.*s")" "\n", configFile.Name(), configBuffer.max_size(), + R"("%.*s")" "\n", configPath.c_str(), configBuffer.max_size(), aznumeric_cast(configBuffer.size()), configBuffer.data()); configFileParsed = false; break; diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h index 02346c2ba1..daa64c0343 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.h @@ -155,6 +155,15 @@ namespace AZ::SettingsRegistryMergeUtils //! structure which is forwarded to the SettingsRegistryInterface MergeCommandLineArgument function //! The structure contains a functor which returns true if a character is a valid delimiter SettingsRegistryInterface::CommandLineArgumentSettings m_commandLineSettings; + + //! enumeration to indicate if AZ::IO::FileIOBase should be used to open the config file over AZ::IO::SystemFile + enum class FileReaderClass + { + UseFileIOIfAvailableFallbackToSystemFile, + UseSystemFileOnly, + UseFileIOOnly + }; + FileReaderClass m_fileReaderClass = FileReaderClass::UseFileIOIfAvailableFallbackToSystemFile; }; //! Loads basic configuration files which have structures similar to Windows INI files //! It is inspired by the Python configparser module: https://docs.python.org/3.10/library/configparser.html diff --git a/Code/Framework/AzCore/AzCore/azcore_files.cmake b/Code/Framework/AzCore/AzCore/azcore_files.cmake index 6c498c3335..aa07959997 100644 --- a/Code/Framework/AzCore/AzCore/azcore_files.cmake +++ b/Code/Framework/AzCore/AzCore/azcore_files.cmake @@ -166,6 +166,8 @@ set(FILES IO/FileIO.cpp IO/FileIO.h IO/FileIOEventBus.h + IO/FileReader.cpp + IO/FileReader.h IO/IOUtils.h IO/IOUtils.cpp IO/IStreamer.h diff --git a/Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp b/Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp new file mode 100644 index 0000000000..691b3f2821 --- /dev/null +++ b/Code/Framework/AzCore/Tests/IO/FileReaderTests.cpp @@ -0,0 +1,72 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ +#include +#include +#include + +namespace UnitTest +{ + template + class FileReaderTestFixture + : public ScopedAllocatorSetupFixture + { + public: + void SetUp() override + { + if constexpr (AZStd::is_same_v) + { + m_fileIo = AZStd::make_unique(); + } + } + + void TearDown() override + { + m_fileIo.reset(); + } + + protected: + AZStd::unique_ptr m_fileIo{}; + }; + + using FileIOTypes = ::testing::Types; + + TYPED_TEST_CASE(FileReaderTestFixture, FileIOTypes); + + TYPED_TEST(FileReaderTestFixture, ConstructorWithFilePath_OpensFileSuccessfully) + { + AZ::IO::FileReader fileReader(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + EXPECT_TRUE(fileReader.IsOpen()); + } + + TYPED_TEST(FileReaderTestFixture, Open_OpensFileSucessfully) + { + AZ::IO::FileReader fileReader; + fileReader.Open(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + EXPECT_TRUE(fileReader.IsOpen()); + } + + TYPED_TEST(FileReaderTestFixture, Eof_OnNULDeviceFile_Succeeds) + { + AZ::IO::FileReader fileReader(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + EXPECT_TRUE(fileReader.Eof()); + } + + TYPED_TEST(FileReaderTestFixture, GetFilePath_ReturnsNULDeviceFilename_Succeeds) + { + AZ::IO::FileReader fileReader(this->m_fileIo.get(), AZ::IO::SystemFile::GetNullFilename()); + AZ::IO::FixedMaxPath filePath; + EXPECT_TRUE(fileReader.GetFilePath(filePath)); + AZ::IO::FixedMaxPath nulFilename{ AZ::IO::SystemFile::GetNullFilename() }; + if (this->m_fileIo) + { + EXPECT_TRUE(this->m_fileIo->ResolvePath(nulFilename, nulFilename)); + } + EXPECT_EQ(nulFilename, filePath); + } + +} // namespace UnitTest diff --git a/Code/Framework/AzCore/Tests/azcoretests_files.cmake b/Code/Framework/AzCore/Tests/azcoretests_files.cmake index d4d107f094..c36d37d874 100644 --- a/Code/Framework/AzCore/Tests/azcoretests_files.cmake +++ b/Code/Framework/AzCore/Tests/azcoretests_files.cmake @@ -37,6 +37,7 @@ set(FILES FileIOBaseTestTypes.h Geometry2DUtils.cpp Interface.cpp + IO/FileReaderTests.cpp IO/Path/PathTests.cpp IPC.cpp Jobs.cpp