diff --git a/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp b/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp index 7e9accffd7..ba66d56379 100644 --- a/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp +++ b/Code/Framework/AzCore/AzCore/Component/ComponentApplication.cpp @@ -192,82 +192,101 @@ namespace AZ }; - //! SettingsRegistry notifier handler which updates relevant registry settings based - //! on an update to '/Amazon/AzCore/Bootstrap/project_path' key. - struct UpdateProjectSettingsEventHandler + //! SettingsRegistry notifier handler which is responsible for loading + //! the project.json file at the new project path + //! if an update to '/project_path' key occurs. + struct ProjectPathChangedEventHandler { - UpdateProjectSettingsEventHandler(AZ::SettingsRegistryInterface& registry, AZ::CommandLine& commandLine) + ProjectPathChangedEventHandler(AZ::SettingsRegistryInterface& registry) : m_registry{ registry } - , m_commandLine{ commandLine } { } void operator()(AZStd::string_view path, AZ::SettingsRegistryInterface::Type) { + // Update the project settings when the project path is set using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString; - // #1 Update the project settings when the project path is set const auto projectPathKey = FixedValueString(AZ::SettingsRegistryMergeUtils::BootstrapSettingsRootKey) + "/project_path"; + AZ::IO::FixedMaxPath newProjectPath; if (SettingsRegistryMergeUtils::IsPathAncestorDescendantOrEqual(projectPathKey, path) && m_registry.Get(newProjectPath.Native(), projectPathKey) && newProjectPath != m_oldProjectPath) { - UpdateProjectSettingsFromProjectPath(AZ::IO::PathView(newProjectPath)); + // Update old Project path before attempting to merge in new Settings Registry values in order to prevent recursive calls + m_oldProjectPath = newProjectPath; + + // Merge the project.json file into settings registry under ProjectSettingsRootKey path. + AZ::IO::FixedMaxPath projectMetadataFile{ AZ::SettingsRegistryMergeUtils::FindEngineRoot(m_registry) / newProjectPath }; + projectMetadataFile /= "project.json"; + m_registry.MergeSettingsFile(projectMetadataFile.Native(), + AZ::SettingsRegistryInterface::Format::JsonMergePatch, AZ::SettingsRegistryMergeUtils::ProjectSettingsRootKey); + + // Update all the runtime file paths based on the new "project_path" value. + AZ::SettingsRegistryMergeUtils::MergeSettingsToRegistry_AddRuntimeFilePaths(m_registry); } + } + + private: + AZ::IO::FixedMaxPath m_oldProjectPath; + AZ::SettingsRegistryInterface& m_registry; + }; - // #2 Update the project specialization when the project name is set + //! SettingsRegistry notifier handler which adds the project name as a specialization tag + //! to the registry + //! if an update to '/project_name' key occurs. + struct ProjectNameChangedEventHandler + { + ProjectNameChangedEventHandler(AZ::SettingsRegistryInterface& registry) + : m_registry{ registry } + { + } + + void operator()(AZStd::string_view path, AZ::SettingsRegistryInterface::Type) + { + // Update the project specialization when the project name is set + using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString; const auto projectNameKey = FixedValueString(AZ::SettingsRegistryMergeUtils::ProjectSettingsRootKey) + "/project_name"; + FixedValueString newProjectName; if (SettingsRegistryMergeUtils::IsPathAncestorDescendantOrEqual(projectNameKey, path) && m_registry.Get(newProjectName, projectNameKey) && newProjectName != m_oldProjectName) { - UpdateProjectSpecializationFromProjectName(newProjectName); - } - - // #3 Update the ComponentApplication CommandLine instance when the command line settings are merged into the Settings Registry - if (path == AZ::SettingsRegistryMergeUtils::CommandLineValueChangedKey) - { - UpdateCommandLine(); + // Add the project_name as a specialization for loading the build system dependency .setreg files + auto newProjectNameSpecialization = FixedValueString::format("%s/%.*s", AZ::SettingsRegistryMergeUtils::SpecializationsRootKey, + aznumeric_cast(newProjectName.size()), newProjectName.data()); + auto oldProjectNameSpecialization = FixedValueString::format("%s/%s", AZ::SettingsRegistryMergeUtils::SpecializationsRootKey, + m_oldProjectName.c_str()); + m_registry.Remove(oldProjectNameSpecialization); + m_oldProjectName = newProjectName; + m_registry.Set(newProjectNameSpecialization, true); } } - //! Add the project name as a specialization underneath the /Amazon/AzCore/Settings/Specializations path - //! and remove the current project name specialization if one exists. - void UpdateProjectSpecializationFromProjectName(AZStd::string_view newProjectName) - { - using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString; - // Add the project_name as a specialization for loading the build system dependency .setreg files - auto newProjectNameSpecialization = FixedValueString::format("%s/%.*s", AZ::SettingsRegistryMergeUtils::SpecializationsRootKey, - aznumeric_cast(newProjectName.size()), newProjectName.data()); - auto oldProjectNameSpecialization = FixedValueString::format("%s/%s", AZ::SettingsRegistryMergeUtils::SpecializationsRootKey, - m_oldProjectName.c_str()); - m_registry.Remove(oldProjectNameSpecialization); - m_oldProjectName = newProjectName; - m_registry.Set(newProjectNameSpecialization, true); - } + private: + AZ::SettingsRegistryInterface::FixedValueString m_oldProjectName; + AZ::SettingsRegistryInterface& m_registry; + }; - void UpdateProjectSettingsFromProjectPath(AZ::IO::PathView newProjectPath) + //! SettingsRegistry notifier handler which updates relevant registry settings based + //! on an update to '/Amazon/AzCore/Bootstrap/project_path' key. + struct UpdateCommandLineEventHandler + { + UpdateCommandLineEventHandler(AZ::SettingsRegistryInterface& registry, AZ::CommandLine& commandLine) + : m_registry{ registry } + , m_commandLine{ commandLine } { - // Update old Project path before attempting to merge in new Settings Registry values in order to prevent recursive calls - m_oldProjectPath = newProjectPath; - - // Merge the project.json file into settings registry under ProjectSettingsRootKey path. - AZ::IO::FixedMaxPath projectMetadataFile{ AZ::SettingsRegistryMergeUtils::FindEngineRoot(m_registry) / newProjectPath }; - projectMetadataFile /= "project.json"; - m_registry.MergeSettingsFile(projectMetadataFile.Native(), - AZ::SettingsRegistryInterface::Format::JsonMergePatch, AZ::SettingsRegistryMergeUtils::ProjectSettingsRootKey); - - // Update all the runtime file paths based on the new "project_path" value. - AZ::SettingsRegistryMergeUtils::MergeSettingsToRegistry_AddRuntimeFilePaths(m_registry); } - void UpdateCommandLine() + void operator()(AZStd::string_view path, AZ::SettingsRegistryInterface::Type) { - AZ::SettingsRegistryMergeUtils::GetCommandLineFromRegistry(m_registry, m_commandLine); + // Update the ComponentApplication CommandLine instance when the command line settings are merged into the Settings Registry + if (path == AZ::SettingsRegistryMergeUtils::CommandLineValueChangedKey) + { + AZ::SettingsRegistryMergeUtils::GetCommandLineFromRegistry(m_registry, m_commandLine); + } } private: - AZ::IO::FixedMaxPath m_oldProjectPath; - AZ::SettingsRegistryInterface::FixedValueString m_oldProjectName; AZ::SettingsRegistryInterface& m_registry; AZ::CommandLine& m_commandLine; }; @@ -462,7 +481,12 @@ namespace AZ // 1. The 'project_path' key changes // 2. The project specialization when the 'project-name' key changes // 3. The ComponentApplication command line when the command line is stored to the registry - m_projectChangedHandler = m_settingsRegistry->RegisterNotifier(UpdateProjectSettingsEventHandler{ *m_settingsRegistry, m_commandLine }); + m_projectPathChangedHandler = m_settingsRegistry->RegisterNotifier(ProjectPathChangedEventHandler{ + *m_settingsRegistry }); + m_projectNameChangedHandler = m_settingsRegistry->RegisterNotifier(ProjectNameChangedEventHandler{ + *m_settingsRegistry }); + m_commandLineUpdatedHandler = m_settingsRegistry->RegisterNotifier(UpdateCommandLineEventHandler{ + *m_settingsRegistry, m_commandLine }); // Merge Command Line arguments constexpr bool executeRegDumpCommands = false; @@ -515,11 +539,12 @@ namespace AZ Destroy(); } - // The m_projectChangedHandler stores an AZStd::function internally - // which allocates using the AZ SystemAllocator - // m_projectChangedHandler is being default value initialized - // to clear out the AZStd::function - m_projectChangedHandler = {}; + // The SettingsRegistry Notify handlers stores an AZStd::function internally + // which may allocates using the AZ SystemAllocator(if the functor > 16 bytes) + // The handlers are being default value initialized to clear out the AZStd::function + m_commandLineUpdatedHandler = {}; + m_projectNameChangedHandler = {}; + m_projectPathChangedHandler = {}; // Delete the AZ::IConsole if it was created by this application instance if (m_ownsConsole) diff --git a/Code/Framework/AzCore/AzCore/Component/ComponentApplication.h b/Code/Framework/AzCore/AzCore/Component/ComponentApplication.h index 8e95ff5fa3..f2b1bb8905 100644 --- a/Code/Framework/AzCore/AzCore/Component/ComponentApplication.h +++ b/Code/Framework/AzCore/AzCore/Component/ComponentApplication.h @@ -390,7 +390,9 @@ namespace AZ AZ::IO::FixedMaxPath m_engineRoot; AZ::IO::FixedMaxPath m_appRoot; - AZ::SettingsRegistryInterface::NotifyEventHandler m_projectChangedHandler; + AZ::SettingsRegistryInterface::NotifyEventHandler m_projectPathChangedHandler; + AZ::SettingsRegistryInterface::NotifyEventHandler m_projectNameChangedHandler; + AZ::SettingsRegistryInterface::NotifyEventHandler m_commandLineUpdatedHandler; // ConsoleFunctorHandle is responsible for unregistering the Settings Registry Console // from the m_console member when it goes out of scope diff --git a/Code/Framework/AzCore/AzCore/Console/Console.cpp b/Code/Framework/AzCore/AzCore/Console/Console.cpp index 79e1f79697..d6781e1370 100644 --- a/Code/Framework/AzCore/AzCore/Console/Console.cpp +++ b/Code/Framework/AzCore/AzCore/Console/Console.cpp @@ -639,9 +639,10 @@ namespace AZ { // Make sure the there is a JSON object at the ConsoleRuntimeCommandKey or ConsoleAutoexecKey // So that JSON Patch is able to add values underneath that object (JSON Patch doesn't create intermediate objects) - settingsRegistry.MergeSettings(R"({ "Amazon": { "AzCore": { "Runtime": { "ConsoleCommands": {} } } })" - R"(,"O3DE": { "Autoexec": { "ConsoleCommands": {} } } })", - SettingsRegistryInterface::Format::JsonMergePatch); + settingsRegistry.MergeSettings(R"({})", SettingsRegistryInterface::Format::JsonMergePatch, + IConsole::ConsoleRuntimeCommandKey); + settingsRegistry.MergeSettings(R"({})", SettingsRegistryInterface::Format::JsonMergePatch, + IConsole::ConsoleAutoexecCommandKey); m_consoleCommandKeyHandler = settingsRegistry.RegisterNotifier(ConsoleCommandKeyNotificationHandler{ settingsRegistry, *this }); JsonApplyPatchSettings applyPatchSettings; diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistry.h b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistry.h index 40022bdc04..c98e6b7a63 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistry.h +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistry.h @@ -192,26 +192,34 @@ namespace AZ //! @param path An offset at which traversal should start. //! @return Whether or not entries could be visited. virtual bool Visit(const VisitorCallback& callback, AZStd::string_view path) const = 0; + //! Register a callback that will be called whenever an entry gets a new/updated value. + //! //! @callback The function to call when an entry gets a new/updated value. - [[nodiscard]] virtual NotifyEventHandler RegisterNotifier(const NotifyCallback& callback) = 0; - //! Register a callback that will be called whenever an entry gets a new/updated value. - //! @callback The function to call when an entry gets a new/updated value. - [[nodiscard]] virtual NotifyEventHandler RegisterNotifier(NotifyCallback&& callback) = 0; + //! @return NotifyEventHandler instance which must persist to receive event signal + [[nodiscard]] virtual NotifyEventHandler RegisterNotifier(NotifyCallback callback) = 0; + //! Register a notify event handler with the NotifyEvent. + //! The handler will be called whenever an entry gets a new/updated value. + //! @param handler The handler to register with the NotifyEvent. + virtual void RegisterNotifier(NotifyEventHandler& handler) = 0; //! Register a function that will be called before a file is merged. - //! @callback The function to call before a file is merged. - [[nodiscard]] virtual PreMergeEventHandler RegisterPreMergeEvent(const PreMergeEventCallback& callback) = 0; - //! Register a function that will be called before a file is merged. - //! @callback The function to call before a file is merged. - [[nodiscard]] virtual PreMergeEventHandler RegisterPreMergeEvent(PreMergeEventCallback&& callback) = 0; + //! @param callback The function to call before a file is merged. + //! @return PreMergeEventHandler instance which must persist to receive event signal + [[nodiscard]] virtual PreMergeEventHandler RegisterPreMergeEvent(PreMergeEventCallback callback) = 0; + //! Register a pre-merge handler with the PreMergeEvent. + //! The handler will be called before a file is merged. + //! @param handler The hanlder to register with the PreMergeEvent. + virtual void RegisterPreMergeEvent(PreMergeEventHandler& handler) = 0; //! Register a function that will be called after a file is merged. - //! @callback The function to call after a file is merged. - [[nodiscard]] virtual PostMergeEventHandler RegisterPostMergeEvent(const PostMergeEventCallback& callback) = 0; - //! Register a function that will be called after a file is merged. - //! @callback The function to call after a file is merged. - [[nodiscard]] virtual PostMergeEventHandler RegisterPostMergeEvent(PostMergeEventCallback&& callback) = 0; + //! @param callback The function to call after a file is merged. + //! @return PostMergeEventHandler instance which must persist to receive event signal + [[nodiscard]] virtual PostMergeEventHandler RegisterPostMergeEvent(PostMergeEventCallback callback) = 0; + //! Register a post-merge hahndler with the PostMergeEvent. + //! The handler will be called after a file is merged. + //! @param handler The handler to register with the PostmergeEVent. + virtual void RegisterPostMergeEvent(PostMergeEventHandler& hanlder) = 0; //! Gets the boolean value at the provided path. //! @param result The target to write the result to. @@ -326,23 +334,25 @@ namespace AZ //! - all digits and dot -> floating point number //! - Everything else is considered a string. //! @param argument The command line argument. - //! @param structure which contains functors which determine what characters are delimiters + //! @param anchorKey The key where the merged command line argument will be anchored under + //! @param commandLineSettings structure which contains functors which determine what characters are delimiters //! @return True if the command line argument could be parsed, otherwise false. - virtual bool MergeCommandLineArgument(AZStd::string_view argument, AZStd::string_view rootKey = "", + virtual bool MergeCommandLineArgument(AZStd::string_view argument, AZStd::string_view anchorKey = "", const CommandLineArgumentSettings& commandLineSettings = {}) = 0; //! Merges the json data provided into the settings registry. //! @param data The json data stored in a string. //! @param format The format of the provided data. + //! @param anchorKey The key where the merged json content will be anchored under. //! @return True if the data was successfully merged, otherwise false. - virtual bool MergeSettings(AZStd::string_view data, Format format) = 0; + virtual bool MergeSettings(AZStd::string_view data, Format format, AZStd::string_view anchorKey = "") = 0; //! Loads a settings file and merges it into the registry. //! @param path The path to the registry file. //! @param format The format of the text data in the file at the provided path. - //! @param rootKey The key where the root of the settings file will be stored under. + //! @param anchorKey The key where the content of the settings file will be anchored. //! @param scratchBuffer An optional buffer that's used to load the file into. Use this when loading multiple patches to //! reduce the number of intermediate memory allocations. //! @return True if the registry file was successfully merged, otherwise false. - virtual bool MergeSettingsFile(AZStd::string_view path, Format format, AZStd::string_view rootKey = "", + virtual bool MergeSettingsFile(AZStd::string_view path, Format format, AZStd::string_view anchorKey = "", AZStd::vector* scratchBuffer = nullptr) = 0; //! Loads all settings files in a folder and merges them into the registry. //! With the specializations "a" and "b" and platform "c" the files would be loaded in the order: @@ -357,11 +367,12 @@ namespace AZ //! @param platform An optional name of a platform. Platform overloads are located at /Platform// //! Files in a platform are applied in the same order as for the main folder but always after the same file //! in the main folder. + //! @param anchorKey The registry path location where the settings will be anchored //! @param scratchBuffer An optional buffer that's used to load the file into. Use this when loading multiple patches to //! reduce the number of intermediate memory allocations. //! @return True if the registry folder was successfully merged, otherwise false. virtual bool MergeSettingsFolder(AZStd::string_view path, const Specializations& specializations, - AZStd::string_view platform = {}, AZStd::string_view rootKey = "", AZStd::vector* scratchBuffer = nullptr) = 0; + AZStd::string_view platform = {}, AZStd::string_view anchorKey = "", AZStd::vector* scratchBuffer = nullptr) = 0; //! Stores the settings structure which is used when merging settings to the Settings Registry //! using JSON Merge Patch or JSON Merge Patch. diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp index 92f546815e..cbf25c29d4 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include @@ -21,6 +21,34 @@ #include #include +namespace AZ::SettingsRegistryImplInternal +{ + AZ::SettingsRegistryInterface::Type RapidjsonToSettingsRegistryType(const rapidjson::Value& value) + { + using Type = AZ::SettingsRegistryInterface::Type; + switch (value.GetType()) + { + case rapidjson::Type::kNullType: + return Type::Null; + case rapidjson::Type::kFalseType: + return Type::Boolean; + case rapidjson::Type::kTrueType: + return Type::Boolean; + case rapidjson::Type::kObjectType: + return Type::Object; + case rapidjson::Type::kArrayType: + return Type::Array; + case rapidjson::Type::kStringType: + return Type::String; + case rapidjson::Type::kNumberType: + return value.IsDouble() ? Type::FloatingPoint : + Type::Integer; + } + + return Type::NoType; + } +} + namespace AZ { template @@ -28,7 +56,7 @@ namespace AZ { if (path.empty()) { - // rapidjson::Pointer assets that the supplied string + // rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; @@ -70,7 +98,7 @@ namespace AZ { if (path.empty()) { - // rapidjson::Pointer assets that the supplied string + // rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; @@ -161,7 +189,7 @@ namespace AZ { if (path.empty()) { - // rapidjson::Pointer assets that the supplied string + // rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; @@ -212,9 +240,9 @@ namespace AZ return Visit(visitor, path); } - auto SettingsRegistryImpl::RegisterNotifier(const NotifyCallback& callback) -> NotifyEventHandler + auto SettingsRegistryImpl::RegisterNotifier(NotifyCallback callback) -> NotifyEventHandler { - NotifyEventHandler notifyHandler{ callback }; + NotifyEventHandler notifyHandler{ AZStd::move(callback) }; { AZStd::scoped_lock lock(m_notifierMutex); notifyHandler.Connect(m_notifiers); @@ -222,14 +250,10 @@ namespace AZ return notifyHandler; } - auto SettingsRegistryImpl::RegisterNotifier(NotifyCallback&& callback) -> NotifyEventHandler + auto SettingsRegistryImpl::RegisterNotifier(NotifyEventHandler& notifyHandler) -> void { - NotifyEventHandler notifyHandler{ AZStd::move(callback) }; - { - AZStd::scoped_lock lock(m_notifierMutex); - notifyHandler.Connect(m_notifiers); - } - return notifyHandler; + AZStd::scoped_lock lock(m_notifierMutex); + notifyHandler.Connect(m_notifiers); } void SettingsRegistryImpl::ClearNotifiers() @@ -238,9 +262,9 @@ namespace AZ m_notifiers.DisconnectAllHandlers(); } - auto SettingsRegistryImpl::RegisterPreMergeEvent(const PreMergeEventCallback& callback) -> PreMergeEventHandler + auto SettingsRegistryImpl::RegisterPreMergeEvent(PreMergeEventCallback callback) -> PreMergeEventHandler { - PreMergeEventHandler preMergeHandler{ callback }; + PreMergeEventHandler preMergeHandler{ AZStd::move(callback) }; { AZStd::scoped_lock lock(m_settingMutex); preMergeHandler.Connect(m_preMergeEvent); @@ -248,19 +272,15 @@ namespace AZ return preMergeHandler; } - auto SettingsRegistryImpl::RegisterPreMergeEvent(PreMergeEventCallback&& callback) -> PreMergeEventHandler + auto SettingsRegistryImpl::RegisterPreMergeEvent(PreMergeEventHandler& preMergeHandler) -> void { - PreMergeEventHandler preMergeHandler{ AZStd::move(callback) }; - { - AZStd::scoped_lock lock(m_settingMutex); - preMergeHandler.Connect(m_preMergeEvent); - } - return preMergeHandler; + AZStd::scoped_lock lock(m_settingMutex); + preMergeHandler.Connect(m_preMergeEvent); } - auto SettingsRegistryImpl::RegisterPostMergeEvent(const PostMergeEventCallback& callback) -> PostMergeEventHandler + auto SettingsRegistryImpl::RegisterPostMergeEvent(PostMergeEventCallback callback) -> PostMergeEventHandler { - PostMergeEventHandler postMergeHandler{ callback }; + PostMergeEventHandler postMergeHandler{ AZStd::move(callback) }; { AZStd::scoped_lock lock(m_settingMutex); postMergeHandler.Connect(m_postMergeEvent); @@ -268,14 +288,10 @@ namespace AZ return postMergeHandler; } - auto SettingsRegistryImpl::RegisterPostMergeEvent(PostMergeEventCallback&& callback) -> PostMergeEventHandler + auto SettingsRegistryImpl::RegisterPostMergeEvent(PostMergeEventHandler& postMergeHandler) -> void { - PostMergeEventHandler postMergeHandler{ AZStd::move(callback) }; - { - AZStd::scoped_lock lock(m_settingMutex); - postMergeHandler.Connect(m_postMergeEvent); - } - return postMergeHandler; + AZStd::scoped_lock lock(m_settingMutex); + postMergeHandler.Connect(m_postMergeEvent); } void SettingsRegistryImpl::ClearMergeEvents() @@ -297,7 +313,36 @@ namespace AZ localNotifierEvent = AZStd::move(m_notifiers); } - localNotifierEvent.Signal(jsonPath, type); + // Signal the NotifyEvent for each queued argument + decltype(m_signalNotifierQueue) localNotifierQueue; + { + AZStd::scoped_lock signalLock(m_signalMutex); + m_signalNotifierQueue.push_back({ FixedValueString{jsonPath}, type }); + // If the signal count was 0, then a dispatch is in progress + if (m_signalCount++ == 0) + { + AZStd::swap(localNotifierQueue, m_signalNotifierQueue); + } + } + + while (!localNotifierQueue.empty()) + { + for (SignalNotifierArgs notifierArgs : localNotifierQueue) + { + localNotifierEvent.Signal(notifierArgs.m_jsonPath, notifierArgs.m_type); + } + // Clear the local notifier queue and check if more notifiers have been added + localNotifierQueue = {}; + { + AZStd::scoped_lock signalLock(m_signalMutex); + AZStd::swap(localNotifierQueue, m_signalNotifierQueue); + } + } + + { + AZStd::scoped_lock signalLock(m_signalMutex); + --m_signalCount; + } { // Swap the local handlers with the current m_notifiers which @@ -314,39 +359,19 @@ namespace AZ { if (path.empty()) { - //rapidjson::Pointer assets that the supplied string + //rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; } - rapidjson::Pointer pointer(path.data(), path.length()); if (pointer.IsValid()) { AZStd::scoped_lock lock(m_settingMutex); - const rapidjson::Value* value = pointer.Get(m_settings); - if (value) + if (const rapidjson::Value* value = pointer.Get(m_settings); value != nullptr) { - switch (value->GetType()) - { - case rapidjson::Type::kNullType: - return Type::Null; - case rapidjson::Type::kFalseType: - return Type::Boolean; - case rapidjson::Type::kTrueType: - return Type::Boolean; - case rapidjson::Type::kObjectType: - return Type::Object; - case rapidjson::Type::kArrayType: - return Type::Array; - case rapidjson::Type::kStringType: - return Type::String; - case rapidjson::Type::kNumberType: - return - value->IsDouble() ? Type::FloatingPoint : - Type::Integer; - } + return SettingsRegistryImplInternal::RapidjsonToSettingsRegistryType(*value); } } return Type::NoType; @@ -392,7 +417,7 @@ namespace AZ { if (path.empty()) { - // rapidjson::Pointer assets that the supplied string + // rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; @@ -471,13 +496,12 @@ namespace AZ { if (path.empty()) { - //rapidjson::Pointer assets that the supplied string + // rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; } - rapidjson::Pointer pointer(path.data(), path.length()); if (pointer.IsValid()) { @@ -486,10 +510,14 @@ namespace AZ value, nullptr, valueTypeID, m_serializationSettings); if (jsonResult.GetProcessing() != JsonSerializationResult::Processing::Halted) { - AZStd::scoped_lock lock(m_settingMutex); - rapidjson::Value& setting = pointer.Create(m_settings, m_settings.GetAllocator()); - setting = AZStd::move(store); - SignalNotifier(path, Type::Object); + auto anchorType = Type::NoType; + { + AZStd::scoped_lock lock(m_settingMutex); + rapidjson::Value& setting = pointer.Create(m_settings, m_settings.GetAllocator()); + setting = AZStd::move(store); + anchorType = SettingsRegistryImplInternal::RapidjsonToSettingsRegistryType(setting); + } + SignalNotifier(path, anchorType); return true; } } @@ -500,7 +528,7 @@ namespace AZ { if (path.empty()) { - // rapidjson::Pointer assets that the supplied string + // rapidjson::Pointer asserts that the supplied string // is not nullptr even if the supplied size is 0 // Setting to empty string to prevent assert path = ""; @@ -605,7 +633,7 @@ namespace AZ return Set(key, value); } - bool SettingsRegistryImpl::MergeSettings(AZStd::string_view data, Format format) + bool SettingsRegistryImpl::MergeSettings(AZStd::string_view data, Format format, AZStd::string_view anchorKey) { rapidjson::Document jsonPatch; constexpr int flags = rapidjson::kParseStopWhenDoneFlag | rapidjson::kParseCommentsFlag | rapidjson::kParseTrailingCommasFlag; @@ -631,17 +659,43 @@ namespace AZ return false; } - AZStd::scoped_lock lock(m_settingMutex); + rapidjson::Pointer anchorPath; + if (!anchorKey.empty()) + { + anchorPath = rapidjson::Pointer(anchorKey.data(), anchorKey.size()); + if (!anchorPath.IsValid()) + { + rapidjson::Pointer pointer(AZ_SETTINGS_REGISTRY_HISTORY_KEY "/-"); + AZ_Error("Settings Registry", false, R"(Anchor path "%.*s" is invalid.)", AZ_STRING_ARG(anchorKey)); + AZStd::scoped_lock lock(m_settingMutex); + pointer.Create(m_settings, m_settings.GetAllocator()).SetObject() + .AddMember(rapidjson::StringRef("Error"), rapidjson::StringRef("Invalid anchor key."), m_settings.GetAllocator()) + .AddMember(rapidjson::StringRef("Path"), + rapidjson::Value(anchorKey.data(), aznumeric_caster(anchorKey.size()), m_settings.GetAllocator()), + m_settings.GetAllocator()); + return false; + } + } - JsonSerializationResult::ResultCode mergeResult = - JsonSerialization::ApplyPatch(m_settings, m_settings.GetAllocator(), jsonPatch, mergeApproach); - if (mergeResult.GetProcessing() != JsonSerializationResult::Processing::Completed) + auto anchorType = AZ::SettingsRegistryInterface::Type::NoType; { - AZ_Error("Settings Registry", false, "Failed to fully merge data into registry."); - return false; + AZStd::scoped_lock lock(m_settingMutex); + rapidjson::Value& anchorRoot = anchorPath.IsValid() ? anchorPath.Create(m_settings, m_settings.GetAllocator()) + : m_settings; + + JsonSerializationResult::ResultCode mergeResult = + JsonSerialization::ApplyPatch(anchorRoot, m_settings.GetAllocator(), jsonPatch, mergeApproach); + if (mergeResult.GetProcessing() != JsonSerializationResult::Processing::Completed) + { + AZ_Error("Settings Registry", false, "Failed to fully merge data into registry."); + return false; + } + + // The settings have been successfully merged, query the type at the anchor key + anchorType = SettingsRegistryImplInternal::RapidjsonToSettingsRegistryType(anchorRoot); } - SignalNotifier("", Type::Object); + SignalNotifier(anchorKey, anchorType); return true; } @@ -1225,10 +1279,12 @@ namespace AZ ScopedMergeEvent scopedMergeEvent(m_preMergeEvent, m_postMergeEvent, path, rootKey); JsonSerializationResult::ResultCode mergeResult(JsonSerializationResult::Tasks::Merge); + auto anchorType = Type::NoType; if (rootKey.empty()) { AZStd::scoped_lock lock(m_settingMutex); mergeResult = JsonSerialization::ApplyPatch(m_settings, m_settings.GetAllocator(), jsonPatch, mergeApproach, m_applyPatchSettings); + anchorType = SettingsRegistryImplInternal::RapidjsonToSettingsRegistryType(m_settings); } else { @@ -1238,6 +1294,7 @@ namespace AZ AZStd::scoped_lock lock(m_settingMutex); Value& rootValue = root.Create(m_settings, m_settings.GetAllocator()); mergeResult = JsonSerialization::ApplyPatch(rootValue, m_settings.GetAllocator(), jsonPatch, mergeApproach, m_applyPatchSettings); + anchorType = SettingsRegistryImplInternal::RapidjsonToSettingsRegistryType(rootValue); } else { @@ -1265,7 +1322,7 @@ namespace AZ pointer.Create(m_settings, m_settings.GetAllocator()).SetString(path, m_settings.GetAllocator()); } - SignalNotifier("", Type::Object); + SignalNotifier(rootKey, anchorType); return true; } diff --git a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.h b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.h index ac214711b8..1cdb2b3b06 100644 --- a/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.h +++ b/Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.h @@ -48,14 +48,14 @@ namespace AZ Type GetType(AZStd::string_view path) const override; bool Visit(Visitor& visitor, AZStd::string_view path) const override; bool Visit(const VisitorCallback& callback, AZStd::string_view path) const override; - [[nodiscard]] NotifyEventHandler RegisterNotifier(const NotifyCallback& callback) override; - [[nodiscard]] NotifyEventHandler RegisterNotifier(NotifyCallback&& callback) override; + [[nodiscard]] NotifyEventHandler RegisterNotifier(NotifyCallback callback) override; + void RegisterNotifier(NotifyEventHandler& hanlder) override; void ClearNotifiers(); - [[nodiscard]] PreMergeEventHandler RegisterPreMergeEvent(const PreMergeEventCallback& callback) override; - [[nodiscard]] PreMergeEventHandler RegisterPreMergeEvent(PreMergeEventCallback&& callback) override; - [[nodiscard]] PostMergeEventHandler RegisterPostMergeEvent(const PostMergeEventCallback& callback) override; - [[nodiscard]] PostMergeEventHandler RegisterPostMergeEvent(PostMergeEventCallback&& callback) override; + [[nodiscard]] PreMergeEventHandler RegisterPreMergeEvent(PreMergeEventCallback callback) override; + void RegisterPreMergeEvent(PreMergeEventHandler& handler) override; + [[nodiscard]] PostMergeEventHandler RegisterPostMergeEvent(PostMergeEventCallback callback) override; + void RegisterPostMergeEvent(PostMergeEventHandler& handler) override; void ClearMergeEvents(); bool Get(bool& result, AZStd::string_view path) const override; @@ -76,13 +76,13 @@ namespace AZ bool Remove(AZStd::string_view path) override; - bool MergeCommandLineArgument(AZStd::string_view argument, AZStd::string_view rootKey, + bool MergeCommandLineArgument(AZStd::string_view argument, AZStd::string_view anchorKey, const CommandLineArgumentSettings& commandLineSettings) override; - bool MergeSettings(AZStd::string_view data, Format format) override; - bool MergeSettingsFile(AZStd::string_view path, Format format, AZStd::string_view rootKey, + bool MergeSettings(AZStd::string_view data, Format format, AZStd::string_view anchorKey = "") override; + bool MergeSettingsFile(AZStd::string_view path, Format format, AZStd::string_view anchorKey = "", AZStd::vector* scratchBuffer = nullptr) override; bool MergeSettingsFolder(AZStd::string_view path, const Specializations& specializations, - AZStd::string_view platform, AZStd::string_view rootKey = "", AZStd::vector* scratchBuffer = nullptr) override; + AZStd::string_view platform, AZStd::string_view anchorKey = "", AZStd::vector* scratchBuffer = nullptr) override; void SetApplyPatchSettings(const AZ::JsonApplyPatchSettings& applyPatchSettings) override; void GetApplyPatchSettings(AZ::JsonApplyPatchSettings& applyPatchSettings) override; @@ -121,6 +121,19 @@ namespace AZ PreMergeEvent m_preMergeEvent; PostMergeEvent m_postMergeEvent; + //! NOTE: During SignalNotifier, the registered notify event handlers are moved to a local NotifyEvent + //! Therefore setting a value within the registry during signaling will queue future SignalNotifer calls + //! These calls will then be invoked after the current signaling has completex + //! This is done to avoid deadlock if another thread attempts to access register a notifier or signal one + mutable AZStd::mutex m_signalMutex; + struct SignalNotifierArgs + { + FixedValueString m_jsonPath; + Type m_type; + }; + AZStd::deque m_signalNotifierQueue; + AZStd::atomic_int m_signalCount{}; + rapidjson::Document m_settings; JsonSerializerSettings m_serializationSettings; JsonDeserializerSettings m_deserializationSettings; diff --git a/Code/Framework/AzCore/AzCore/UnitTest/Mocks/MockSettingsRegistry.h b/Code/Framework/AzCore/AzCore/UnitTest/Mocks/MockSettingsRegistry.h index 91dc0924c8..3ab82528c0 100644 --- a/Code/Framework/AzCore/AzCore/UnitTest/Mocks/MockSettingsRegistry.h +++ b/Code/Framework/AzCore/AzCore/UnitTest/Mocks/MockSettingsRegistry.h @@ -23,12 +23,12 @@ namespace AZ MOCK_CONST_METHOD1(GetType, Type(AZStd::string_view)); MOCK_CONST_METHOD2(Visit, bool(Visitor&, AZStd::string_view)); MOCK_CONST_METHOD2(Visit, bool(const VisitorCallback&, AZStd::string_view)); - MOCK_METHOD1(RegisterNotifier, NotifyEventHandler(const NotifyCallback&)); - MOCK_METHOD1(RegisterNotifier, NotifyEventHandler(NotifyCallback&&)); - MOCK_METHOD1(RegisterPreMergeEvent, PreMergeEventHandler(const PreMergeEventCallback&)); - MOCK_METHOD1(RegisterPreMergeEvent, PreMergeEventHandler(PreMergeEventCallback&&)); - MOCK_METHOD1(RegisterPostMergeEvent, PostMergeEventHandler(const PostMergeEventCallback&)); - MOCK_METHOD1(RegisterPostMergeEvent, PostMergeEventHandler(PostMergeEventCallback&&)); + MOCK_METHOD1(RegisterNotifier, NotifyEventHandler(NotifyCallback)); + MOCK_METHOD1(RegisterNotifier, void(NotifyEventHandler&)); + MOCK_METHOD1(RegisterPreMergeEvent, PreMergeEventHandler(PreMergeEventCallback)); + MOCK_METHOD1(RegisterPreMergeEvent, void(PreMergeEventHandler&)); + MOCK_METHOD1(RegisterPostMergeEvent, PostMergeEventHandler(PostMergeEventCallback)); + MOCK_METHOD1(RegisterPostMergeEvent, void(PostMergeEventHandler&)); MOCK_CONST_METHOD2(Get, bool(bool&, AZStd::string_view)); MOCK_CONST_METHOD2(Get, bool(s64&, AZStd::string_view)); @@ -49,7 +49,7 @@ namespace AZ MOCK_METHOD1(Remove, bool(AZStd::string_view)); MOCK_METHOD3(MergeCommandLineArgument, bool(AZStd::string_view, AZStd::string_view, const CommandLineArgumentSettings&)); - MOCK_METHOD2(MergeSettings, bool(AZStd::string_view, Format)); + MOCK_METHOD3(MergeSettings, bool(AZStd::string_view, Format, AZStd::string_view)); MOCK_METHOD4(MergeSettingsFile, bool(AZStd::string_view, Format, AZStd::string_view, AZStd::vector*)); MOCK_METHOD5( MergeSettingsFolder, diff --git a/Code/Framework/AzCore/Tests/Settings/SettingsRegistryTests.cpp b/Code/Framework/AzCore/Tests/Settings/SettingsRegistryTests.cpp index 2da5701207..c06288c037 100644 --- a/Code/Framework/AzCore/Tests/Settings/SettingsRegistryTests.cpp +++ b/Code/Framework/AzCore/Tests/Settings/SettingsRegistryTests.cpp @@ -1299,6 +1299,35 @@ namespace SettingsRegistryTests EXPECT_FALSE(m_registry->MergeCommandLineArgument(" ", {}, {})); } + // + // MergeSettings + // + TEST_F(SettingsRegistryTest, MergeSettings_MergeJsonWithAnchorKey_StoresSettingsUnderneathKey) + { + constexpr AZStd::string_view anchorKey = "/Anchor/Root/0"; + constexpr auto mergeFormat = AZ::SettingsRegistryInterface::Format::JsonMergePatch; + EXPECT_TRUE(m_registry->MergeSettings(R"({ "Test": "1" })", mergeFormat, anchorKey)); + EXPECT_EQ(AZ::SettingsRegistryInterface::Type::Array, m_registry->GetType("/Anchor/Root")); + EXPECT_EQ(AZ::SettingsRegistryInterface::Type::Object, m_registry->GetType("/Anchor/Root/0")); + EXPECT_EQ(AZ::SettingsRegistryInterface::Type::String, m_registry->GetType("/Anchor/Root/0/Test")); + } + + TEST_F(SettingsRegistryTest, MergeSettings_NotifierSignals_AtAnchorKeyAndStoresMergeType) + { + AZStd::string_view anchorKey = "/Anchor/Root"; + bool callbackInvoked{}; + auto callback = [anchorKey, &callbackInvoked](AZStd::string_view path, AZ::SettingsRegistryInterface::Type type) + { + EXPECT_EQ(anchorKey, path); + EXPECT_EQ(AZ::SettingsRegistryInterface::Type::Array, type); + callbackInvoked = true; + }; + auto testNotifier1 = m_registry->RegisterNotifier(callback); + constexpr auto mergeFormat = AZ::SettingsRegistryInterface::Format::JsonMergePatch; + EXPECT_TRUE(m_registry->MergeSettings(R"([ "Test" ])", mergeFormat, anchorKey)); + EXPECT_TRUE(callbackInvoked); + } + // // MergeSettingsFile // @@ -1331,7 +1360,7 @@ namespace SettingsRegistryTests auto callback = [this](AZStd::string_view path, AZ::SettingsRegistryInterface::Type) { - EXPECT_TRUE(path.empty()); + EXPECT_EQ("/Path", path); AZ::s64 value = -1; bool result = m_registry->Get(value, "/Path/Test"); EXPECT_TRUE(result); diff --git a/Gems/SceneProcessing/Code/Tests/SceneBuilder/SceneBuilderTests.cpp b/Gems/SceneProcessing/Code/Tests/SceneBuilder/SceneBuilderTests.cpp index ba1a186e5a..3a7b20553e 100644 --- a/Gems/SceneProcessing/Code/Tests/SceneBuilder/SceneBuilderTests.cpp +++ b/Gems/SceneProcessing/Code/Tests/SceneBuilder/SceneBuilderTests.cpp @@ -9,9 +9,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -269,47 +271,6 @@ TEST_F(SourceDependencyTests, SourceDependencyTest) ASSERT_STREQ(dependencies[1].m_sourceFileDependencyPath.c_str(), "value.png"); } -struct SettingsRegistryMock : AZ::Interface::Registrar -{ - bool Get(FixedValueString& result, AZStd::string_view) const override - { - result = "cache"; - return true; - } - - void SetApplyPatchSettings(const AZ::JsonApplyPatchSettings& /*applyPatchSettings*/) override{} - void GetApplyPatchSettings(AZ::JsonApplyPatchSettings& /*applyPatchSettings*/) override{} - - MOCK_CONST_METHOD1(GetType, Type (AZStd::string_view)); - MOCK_CONST_METHOD2(Visit, bool (Visitor&, AZStd::string_view)); - MOCK_CONST_METHOD2(Visit, bool (const VisitorCallback&, AZStd::string_view)); - MOCK_METHOD1(RegisterNotifier, NotifyEventHandler (const NotifyCallback&)); - MOCK_METHOD1(RegisterNotifier, NotifyEventHandler (NotifyCallback&&)); - MOCK_METHOD1(RegisterPreMergeEvent, PreMergeEventHandler (const PreMergeEventCallback&)); - MOCK_METHOD1(RegisterPreMergeEvent, PreMergeEventHandler (PreMergeEventCallback&&)); - MOCK_METHOD1(RegisterPostMergeEvent, PostMergeEventHandler (const PostMergeEventCallback&)); - MOCK_METHOD1(RegisterPostMergeEvent, PostMergeEventHandler (PostMergeEventCallback&&)); - MOCK_CONST_METHOD2(Get, bool (bool&, AZStd::string_view)); - MOCK_CONST_METHOD2(Get, bool (s64&, AZStd::string_view)); - MOCK_CONST_METHOD2(Get, bool (u64&, AZStd::string_view)); - MOCK_CONST_METHOD2(Get, bool (double&, AZStd::string_view)); - MOCK_CONST_METHOD2(Get, bool (AZStd::string&, AZStd::string_view)); - MOCK_CONST_METHOD3(GetObject, bool (void*, Uuid, AZStd::string_view)); - MOCK_METHOD2(Set, bool (AZStd::string_view, bool)); - MOCK_METHOD2(Set, bool (AZStd::string_view, s64)); - MOCK_METHOD2(Set, bool (AZStd::string_view, u64)); - MOCK_METHOD2(Set, bool (AZStd::string_view, double)); - MOCK_METHOD2(Set, bool (AZStd::string_view, AZStd::string_view)); - MOCK_METHOD2(Set, bool (AZStd::string_view, const char*)); - MOCK_METHOD3(SetObject, bool (AZStd::string_view, const void*, Uuid)); - MOCK_METHOD1(Remove, bool (AZStd::string_view)); - MOCK_METHOD3(MergeCommandLineArgument, bool (AZStd::string_view, AZStd::string_view, const CommandLineArgumentSettings&)); - MOCK_METHOD2(MergeSettings, bool (AZStd::string_view, Format)); - MOCK_METHOD4(MergeSettingsFile, bool (AZStd::string_view, Format, AZStd::string_view, AZStd::vector*)); - MOCK_METHOD5(MergeSettingsFolder, bool (AZStd::string_view, const Specializations&, AZStd::string_view, AZStd::string_view, AZStd::vector*)); - MOCK_METHOD1(SetUseFileIO, void (bool)); -}; - struct SourceDependencyMockedIOTests : UnitTest::ScopedAllocatorSetupFixture , UnitTest::SetRestoreFileIOBaseRAII { @@ -355,7 +316,15 @@ struct SourceDependencyMockedIOTests : UnitTest::ScopedAllocatorSetupFixture TEST_F(SourceDependencyMockedIOTests, RegularManifestHasPriority) { ImportHandler handler; - SettingsRegistryMock settingsRegistry; + MockSettingsRegistry settingsRegistry; + SettingsRegistry::Register(&settingsRegistry); + using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString; + auto MockGetFixedStringCall = [](FixedValueString& result, AZStd::string_view) -> bool + { + result = "cache"; + return true; + }; + ON_CALL(settingsRegistry, Get(::testing::An(), ::testing::_)).WillByDefault(MockGetFixedStringCall); AssetBuilderSDK::CreateJobsRequest request; AssetBuilderSDK::CreateJobsResponse response; @@ -371,12 +340,21 @@ TEST_F(SourceDependencyMockedIOTests, RegularManifestHasPriority) ASSERT_TRUE(SceneBuilderWorker::ManifestDependencyCheck(request, response)); ASSERT_EQ(response.m_sourceFileDependencyList.size(), 2); + SettingsRegistry::Unregister(&settingsRegistry); } TEST_F(SourceDependencyMockedIOTests, GeneratedManifestTest) { ImportHandler handler; - SettingsRegistryMock settingsRegistry; + MockSettingsRegistry settingsRegistry; + SettingsRegistry::Register(&settingsRegistry); + using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString; + auto MockGetFixedStringCall = [](FixedValueString& result, AZStd::string_view) -> bool + { + result = "cache"; + return true; + }; + ON_CALL(settingsRegistry, Get(::testing::An(), ::testing::_)).WillByDefault(MockGetFixedStringCall); AssetBuilderSDK::CreateJobsRequest request; AssetBuilderSDK::CreateJobsResponse response; @@ -392,4 +370,5 @@ TEST_F(SourceDependencyMockedIOTests, GeneratedManifestTest) ASSERT_TRUE(SceneBuilderWorker::ManifestDependencyCheck(request, response)); ASSERT_EQ(response.m_sourceFileDependencyList.size(), 2); + SettingsRegistry::Unregister(&settingsRegistry); }