From e06b8782cc9781660dda860daef76bdf546fe10a Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Fri, 21 Jan 2022 15:55:37 -0800 Subject: [PATCH 1/6] Add AZ::Dom::Patch, a Generic DOM analog to JSON patch - Currently supports JSON patch operations (add/remove/replace/copy/move/test) - `GenerateHierarchicalDeltaPatch` provides a patch generation mechanism that produces forward and inverse patches - Patch application comes with a `PatchApplicationStrategy` functor that allows customizing behavior for patch failure that may be useful for the prefab system - Serialization to/from JSON patch by way of `AZ::Dom::Value` is supported Benchmarks provided, split into three categories on the Dom value benchmark payload (payloads with up to 10k entries populated with strings up to 100 characters in length): - Patch generation based on a deep copy of the affected data ``` DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_DeepCopy/10/5 0.024 ms 0.024 ms 29867 items_per_second=41.5541k/s DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_DeepCopy/10/500 0.024 ms 0.024 ms 29867 items_per_second=41.5541k/s DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_DeepCopy/100/5 0.346 ms 0.345 ms 2036 items_per_second=2.89564k/s DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_DeepCopy/100/500 0.375 ms 0.377 ms 1867 items_per_second=2.65529k/s DomPatchBenchmark/AzDomPatch_Generate_TopLevelReplace/10/5 0.003 ms 0.003 ms 203636 items_per_second=289.616k/s DomPatchBenchmark/AzDomPatch_Generate_TopLevelReplace/10/500 0.004 ms 0.004 ms 194783 items_per_second=283.321k/s DomPatchBenchmark/AzDomPatch_Generate_TopLevelReplace/100/5 0.003 ms 0.003 ms 203636 items_per_second=289.616k/s DomPatchBenchmark/AzDomPatch_Generate_TopLevelReplace/100/500 0.004 ms 0.004 ms 194783 items_per_second=271.002k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_DeepCopy/10/5 0.023 ms 0.024 ms 29867 items_per_second=42.4775k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_DeepCopy/10/500 0.023 ms 0.023 ms 28000 items_per_second=42.6667k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_DeepCopy/100/5 0.341 ms 0.337 ms 2133 items_per_second=2.96765k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_DeepCopy/100/500 0.365 ms 0.361 ms 1948 items_per_second=2.77049k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_DeepCopy/10/5 0.023 ms 0.023 ms 29867 items_per_second=43.4429k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_DeepCopy/10/500 0.023 ms 0.024 ms 29867 items_per_second=42.4775k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_DeepCopy/100/5 0.330 ms 0.330 ms 2133 items_per_second=3.0336k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_DeepCopy/100/500 0.359 ms 0.360 ms 1867 items_per_second=2.77879k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_DeepCopy/10/5 0.023 ms 0.022 ms 29867 items_per_second=44.4532k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_DeepCopy/10/500 0.023 ms 0.023 ms 32000 items_per_second=43.5745k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_DeepCopy/100/5 0.329 ms 0.330 ms 2133 items_per_second=3.0336k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_DeepCopy/100/500 0.357 ms 0.361 ms 1948 items_per_second=2.77049k/s ``` - Patch generation based on a shallow copy of the affected data (this is faster because when using Dom::Value to copy and mutate, we can bypass expensive array and object comparisons for identical values) ``` DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_ShallowCopy/10/5 0.010 ms 0.010 ms 74667 items_per_second=99.556k/s DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_ShallowCopy/10/500 0.010 ms 0.010 ms 74667 items_per_second=97.5242k/s DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_ShallowCopy/100/5 0.079 ms 0.078 ms 8960 items_per_second=12.7431k/s DomPatchBenchmark/AzDomPatch_Generate_SimpleReplace_ShallowCopy/100/500 0.087 ms 0.087 ms 8960 items_per_second=11.4688k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_ShallowCopy/10/5 0.009 ms 0.009 ms 74667 items_per_second=116.553k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_ShallowCopy/10/500 0.009 ms 0.009 ms 74667 items_per_second=113.778k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_ShallowCopy/100/5 0.072 ms 0.071 ms 8960 items_per_second=13.9863k/s DomPatchBenchmark/AzDomPatch_Generate_KeyRemove_ShallowCopy/100/500 0.087 ms 0.088 ms 7467 items_per_second=11.3783k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_ShallowCopy/10/5 0.014 ms 0.014 ms 49778 items_per_second=72.4044k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_ShallowCopy/10/500 0.014 ms 0.014 ms 56000 items_per_second=70.2745k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_ShallowCopy/100/5 0.118 ms 0.117 ms 5600 items_per_second=8.53333k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayAppend_ShallowCopy/100/500 0.140 ms 0.141 ms 4978 items_per_second=7.07982k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_ShallowCopy/10/5 0.009 ms 0.009 ms 89600 items_per_second=108.196k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_ShallowCopy/10/500 0.009 ms 0.009 ms 74667 items_per_second=108.607k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_ShallowCopy/100/5 0.068 ms 0.068 ms 11200 items_per_second=14.6286k/s DomPatchBenchmark/AzDomPatch_Generate_ArrayPrepend_ShallowCopy/100/500 0.082 ms 0.082 ms 8960 items_per_second=12.2009k/s ``` - Patch application ``` DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_ShallowCopy/10/5 0.001 ms 0.001 ms 560000 items_per_second=874.146k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_ShallowCopy/10/500 0.001 ms 0.001 ms 560000 items_per_second=874.146k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_ShallowCopy/100/5 0.004 ms 0.004 ms 172308 items_per_second=250.63k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_ShallowCopy/100/500 0.004 ms 0.004 ms 179200 items_per_second=260.655k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_DeepCopy/10/5 0.005 ms 0.005 ms 112000 items_per_second=193.73k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_DeepCopy/10/500 0.005 ms 0.005 ms 112000 items_per_second=193.73k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_DeepCopy/100/5 0.052 ms 0.052 ms 10000 items_per_second=19.3939k/s DomPatchBenchmark/AzDomPatch_Apply_SimpleReplace_DeepCopy/100/500 0.052 ms 0.052 ms 11200 items_per_second=19.373k/s DomPatchBenchmark/AzDomPatch_Apply_TopLevelReplace/10/5 0.001 ms 0.001 ms 640000 items_per_second=910.222k/s DomPatchBenchmark/AzDomPatch_Apply_TopLevelReplace/10/500 0.001 ms 0.001 ms 640000 items_per_second=910.222k/s DomPatchBenchmark/AzDomPatch_Apply_TopLevelReplace/100/5 0.001 ms 0.001 ms 640000 items_per_second=910.222k/s DomPatchBenchmark/AzDomPatch_Apply_TopLevelReplace/100/500 0.001 ms 0.001 ms 640000 items_per_second=910.222k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_ShallowCopy/10/5 0.001 ms 0.001 ms 560000 items_per_second=896k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_ShallowCopy/10/500 0.001 ms 0.001 ms 640000 items_per_second=871.489k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_ShallowCopy/100/5 0.004 ms 0.004 ms 160000 items_per_second=232.727k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_ShallowCopy/100/500 0.004 ms 0.004 ms 165926 items_per_second=235.984k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_DeepCopy/10/5 0.005 ms 0.005 ms 112000 items_per_second=193.73k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_DeepCopy/10/500 0.005 ms 0.005 ms 112000 items_per_second=193.73k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_DeepCopy/100/5 0.053 ms 0.053 ms 10000 items_per_second=18.8235k/s DomPatchBenchmark/AzDomPatch_Apply_KeyRemove_DeepCopy/100/500 0.051 ms 0.052 ms 10000 items_per_second=19.3939k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_ShallowCopy/10/5 0.001 ms 0.001 ms 497778 items_per_second=692.561k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_ShallowCopy/10/500 0.001 ms 0.001 ms 497778 items_per_second=692.561k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_ShallowCopy/100/5 0.006 ms 0.006 ms 112000 items_per_second=174.829k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_ShallowCopy/100/500 0.006 ms 0.006 ms 100000 items_per_second=177.778k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_DeepCopy/10/5 0.005 ms 0.005 ms 112000 items_per_second=193.73k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_DeepCopy/10/500 0.005 ms 0.005 ms 100000 items_per_second=188.235k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_DeepCopy/100/5 0.053 ms 0.052 ms 11200 items_per_second=19.373k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayAppend_DeepCopy/100/500 0.052 ms 0.053 ms 11200 items_per_second=18.8632k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_ShallowCopy/10/5 0.001 ms 0.001 ms 560000 items_per_second=874.146k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_ShallowCopy/10/500 0.001 ms 0.001 ms 560000 items_per_second=874.146k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_ShallowCopy/100/5 0.004 ms 0.004 ms 179200 items_per_second=260.655k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_ShallowCopy/100/500 0.004 ms 0.004 ms 179200 items_per_second=260.655k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_DeepCopy/10/5 0.005 ms 0.005 ms 100000 items_per_second=193.939k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_DeepCopy/10/500 0.005 ms 0.005 ms 100000 items_per_second=193.939k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_DeepCopy/100/5 0.052 ms 0.052 ms 11200 items_per_second=19.373k/s DomPatchBenchmark/AzDomPatch_Apply_ArrayPrepend_DeepCopy/100/500 0.053 ms 0.053 ms 10000 items_per_second=18.8235k/s ``` At a glance, patch generation using `GenerateHierarchicalDeltaPatch` is slower than applying its created patches, but not prohibitively so. Ideally patches shouldn't be recreated unnecessarily, but especially when diffing `Value`s that have been copied and then mutated, a generate + apply operation similar to what prefabs currently do is reasonably fast. Signed-off-by: Nicholas Van Sickle --- Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp | 959 ++++++++++++++++++ Code/Framework/AzCore/AzCore/DOM/DomPatch.h | 191 ++++ Code/Framework/AzCore/AzCore/DOM/DomPath.cpp | 15 +- .../AzCore/AzCore/azcore_files.cmake | 2 + .../AzCore/Tests/DOM/DomPatchBenchmarks.cpp | 179 ++++ .../AzCore/Tests/DOM/DomPatchTests.cpp | 562 ++++++++++ .../AzCore/Tests/azcoretests_files.cmake | 2 + 7 files changed, 1904 insertions(+), 6 deletions(-) create mode 100644 Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp create mode 100644 Code/Framework/AzCore/AzCore/DOM/DomPatch.h create mode 100644 Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp create mode 100644 Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp new file mode 100644 index 0000000000..963308e9c8 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp @@ -0,0 +1,959 @@ +/* + * 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 +#include + +namespace AZ::Dom +{ + PatchOperation::PatchOperation(Path destinationPath, Type type, Value value) + : m_domPath(destinationPath) + , m_type(type) + , m_value(value) + { + } + + PatchOperation::PatchOperation(Path destinationPath, Type type, Path sourcePath) + : m_domPath(destinationPath) + , m_type(type) + , m_value(sourcePath) + { + } + + PatchOperation::PatchOperation(Path destinationPath, Type type) + : m_domPath(destinationPath) + , m_type(type) + { + } + + bool PatchOperation::operator==(const PatchOperation& rhs) const + { + if (m_type != rhs.m_type) + { + return false; + } + + switch (m_type) + { + case Type::Add: + return m_domPath == rhs.m_domPath && Utils::DeepCompareIsEqual(GetValue(), rhs.GetValue()); + case Type::Remove: + return m_domPath == rhs.m_domPath; + case Type::Replace: + return m_domPath == rhs.m_domPath && Utils::DeepCompareIsEqual(GetValue(), rhs.GetValue()); + case Type::Copy: + return m_domPath == rhs.m_domPath && GetSourcePath() == rhs.GetSourcePath(); + case Type::Move: + return m_domPath == rhs.m_domPath && GetSourcePath() == rhs.GetSourcePath(); + case Type::Test: + return m_domPath == rhs.m_domPath && Utils::DeepCompareIsEqual(GetValue(), rhs.GetValue()); + default: + AZ_Assert(false, "PatchOperation::GetDomRepresentation: invalid patch type specified"); + return false; + } + } + + bool PatchOperation::operator!=(const PatchOperation& rhs) const + { + return !operator==(rhs); + } + + PatchOperation::Type PatchOperation::GetType() const + { + return m_type; + } + + void PatchOperation::SetType(Type type) + { + m_type = type; + } + + const Path& PatchOperation::GetDestinationPath() const + { + return m_domPath; + } + + void PatchOperation::SetDestinationPath(Path path) + { + m_domPath = path; + } + + const Value& PatchOperation::GetValue() const + { + return AZStd::get(m_value); + } + + void PatchOperation::SetValue(Value value) + { + m_value = AZStd::move(value); + } + + const Path& PatchOperation::GetSourcePath() const + { + return AZStd::get(m_value); + } + + void PatchOperation::SetSourcePath(Path path) + { + m_value = AZStd::move(path); + } + + AZ::Outcome PatchOperation::Apply(Value rootElement) const + { + PatchOutcome outcome = ApplyInPlace(rootElement); + if (!outcome.IsSuccess()) + { + return AZ::Failure(outcome.TakeError()); + } + return AZ::Success(AZStd::move(rootElement)); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyInPlace(Value& rootElement) const + { + switch (m_type) + { + case Type::Add: + return ApplyAdd(rootElement); + case Type::Remove: + return ApplyRemove(rootElement); + case Type::Replace: + return ApplyReplace(rootElement); + case Type::Copy: + return ApplyCopy(rootElement); + case Type::Move: + return ApplyMove(rootElement); + case Type::Test: + return ApplyTest(rootElement); + } + return AZ::Failure("Unsupported DOM patch operation specified"); + } + + Value PatchOperation::GetDomRepresentation() const + { + Value serializedPatch(Dom::Type::Object); + switch (m_type) + { + case Type::Add: + serializedPatch["op"].SetString("add"); + serializedPatch["path"].CopyFromString(GetDestinationPath().ToString()); + serializedPatch["value"] = GetValue(); + break; + case Type::Remove: + serializedPatch["op"].SetString("remove"); + serializedPatch["path"].CopyFromString(GetDestinationPath().ToString()); + break; + case Type::Replace: + serializedPatch["op"].SetString("replace"); + serializedPatch["path"].CopyFromString(GetDestinationPath().ToString()); + serializedPatch["value"] = GetValue(); + break; + case Type::Copy: + serializedPatch["op"].SetString("copy"); + serializedPatch["from"].CopyFromString(GetSourcePath().ToString()); + serializedPatch["path"].CopyFromString(GetDestinationPath().ToString()); + break; + case Type::Move: + serializedPatch["op"].SetString("move"); + serializedPatch["from"].CopyFromString(GetSourcePath().ToString()); + serializedPatch["path"].CopyFromString(GetDestinationPath().ToString()); + break; + case Type::Test: + serializedPatch["op"].SetString("test"); + serializedPatch["path"].CopyFromString(GetDestinationPath().ToString()); + serializedPatch["value"] = GetValue(); + break; + default: + AZ_Assert(false, "PatchOperation::GetDomRepresentation: invalid patch type specified"); + } + return serializedPatch; + } + + AZ::Outcome PatchOperation::CreateFromDomRepresentation(Value domValue) + { + if (!domValue.IsObject()) + { + return AZ::Failure("PatchOperation failed to load: PatchOperation must be specified as an Object"); + } + + auto loadField = [&](const char* field, AZStd::optional type = {}) -> AZ::Outcome + { + auto it = domValue.FindMember(field); + if (it == domValue.MemberEnd()) + { + return AZ::Failure(AZStd::string::format("PatchOperation failed to load: no \"%s\" specified", field)); + } + + if (type.has_value() && it->second.GetType() != type) + { + return AZ::Failure(AZStd::string::format("PatchOperation failed to load: \"%s\" is invalid", field)); + } + + return AZ::Success(it->second); + }; + + auto opLoad = loadField("op", Dom::Type::String); + if (!opLoad.IsSuccess()) + { + return AZ::Failure(opLoad.TakeError()); + } + AZStd::string_view op = opLoad.GetValue().GetString(); + if (op == "add") + { + auto pathLoad = loadField("path", Dom::Type::String); + if (!pathLoad.IsSuccess()) + { + return AZ::Failure(pathLoad.TakeError()); + } + auto valueLoad = loadField("value"); + if (!valueLoad.IsSuccess()) + { + return AZ::Failure(valueLoad.TakeError()); + } + + return AZ::Success(PatchOperation::AddOperation(Path(pathLoad.GetValue().GetString()), valueLoad.TakeValue())); + } + else if (op == "remove") + { + auto pathLoad = loadField("path", Dom::Type::String); + if (!pathLoad.IsSuccess()) + { + return AZ::Failure(pathLoad.TakeError()); + } + + return AZ::Success(PatchOperation::RemoveOperation(Path(pathLoad.GetValue().GetString()))); + } + else if (op == "replace") + { + auto pathLoad = loadField("path", Dom::Type::String); + if (!pathLoad.IsSuccess()) + { + return AZ::Failure(pathLoad.TakeError()); + } + auto valueLoad = loadField("value"); + if (!valueLoad.IsSuccess()) + { + return AZ::Failure(valueLoad.TakeError()); + } + + return AZ::Success(PatchOperation::ReplaceOperation(Path(pathLoad.GetValue().GetString()), valueLoad.TakeValue())); + } + else if (op == "copy") + { + auto destLoad = loadField("path", Dom::Type::String); + if (!destLoad.IsSuccess()) + { + return AZ::Failure(destLoad.TakeError()); + } + auto sourceLoad = loadField("from", Dom::Type::String); + if (!sourceLoad.IsSuccess()) + { + return AZ::Failure(sourceLoad.TakeError()); + } + + return AZ::Success(PatchOperation::CopyOperation(Path(destLoad.GetValue().GetString()), Path(sourceLoad.GetValue().GetString()))); + } + else if (op == "move") + { + auto destLoad = loadField("path", Dom::Type::String); + if (!destLoad.IsSuccess()) + { + return AZ::Failure(destLoad.TakeError()); + } + auto sourceLoad = loadField("from", Dom::Type::String); + if (!sourceLoad.IsSuccess()) + { + return AZ::Failure(sourceLoad.TakeError()); + } + + return AZ::Success(PatchOperation::MoveOperation(Path(destLoad.GetValue().GetString()), Path(sourceLoad.GetValue().GetString()))); + } + else if (op == "test") + { + auto pathLoad = loadField("path", Dom::Type::String); + if (!pathLoad.IsSuccess()) + { + return AZ::Failure(pathLoad.TakeError()); + } + auto valueLoad = loadField("value"); + if (!valueLoad.IsSuccess()) + { + return AZ::Failure(valueLoad.TakeError()); + } + + return AZ::Success(PatchOperation::TestOperation(Path(pathLoad.GetValue().GetString()), valueLoad.TakeValue())); + } + else + { + return AZ::Failure("PatchOperation failed to create DOM representation: invalid \"op\" specified"); + } + } + + AZ::Outcome PatchOperation::GetInverse(Value stateBeforeApplication) const + { + switch (m_type) + { + case Type::Add: + { + // Add -> Replace (if value already existed in an object) otherwise + // Add -> Remove + if (m_domPath.Size() > 0 && m_domPath[m_domPath.Size() - 1].IsKey()) + { + const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); + if (existingValue != nullptr) + { + return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); + } + } + return AZ::Success(PatchOperation::RemoveOperation(m_domPath)); + } + case Type::Remove: + { + // Remove -> Add + const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); + if (existingValue == nullptr) + { + return AZ::Failure( + AZStd::string::format("Unable to invert DOM remove patch, source path not found: %s", m_domPath.ToString().data())); + } + return AZ::Success(PatchOperation::AddOperation(m_domPath, *existingValue)); + } + case Type::Replace: + { + // Replace -> Replace (with old value) + const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); + if (existingValue == nullptr) + { + return AZ::Failure(AZStd::string::format( + "Unable to invert DOM replace patch, source path not found: %s", m_domPath.ToString().data())); + } + return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); + } + case Type::Copy: + { + // Copy -> Replace (with old value) + const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); + if (existingValue == nullptr) + { + return AZ::Failure( + AZStd::string::format("Unable to invert DOM copy patch, source path not found: %s", m_domPath.ToString().data())); + } + return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); + } + case Type::Move: + { + // Move -> Replace, using the common ancestor of the two paths as the replacement + // This is not a minimal inverse, which would be two replace operations at each path + const Path& destPath = m_domPath; + const Path& sourcePath = GetSourcePath(); + + Path commonAncestor; + for (size_t i = 0; i < destPath.Size() && i < sourcePath.Size(); ++i) + { + if (destPath[i] != sourcePath[i]) + { + break; + } + + commonAncestor.Push(destPath[i]); + } + + const Value* existingValue = stateBeforeApplication.FindChild(commonAncestor); + if (existingValue == nullptr) + { + return AZ::Failure(AZStd::string::format( + "Unable to invert DOM move patch, common ancestor path not found: %s", commonAncestor.ToString().data())); + } + return AZ::Success(PatchOperation::ReplaceOperation(commonAncestor, *existingValue)); + } + case Type::Test: + { + // Test -> Test (no change) + // When inverting a sequence of patches, applying them in reverse order should allow the test to continue to succeed + return AZ::Success(*this); + } + } + return AZ::Failure("Unable to invert DOM patch, unknown type specified"); + } + + AZ::Outcome PatchOperation::LookupPath( + Value& rootElement, const Path& path, AZ::u8 existenceCheckFlags) + { + const bool verifyFullPath = existenceCheckFlags & VerifyFullPath; + const bool allowEndOfArray = existenceCheckFlags & AllowEndOfArray; + + Path target = path; + if (target.Size() == 0) + { + Value wrapper(Dom::Type::Array); + wrapper.ArrayPushBack(rootElement); + return AZ::Success({ wrapper, PathEntry(0) }); + } + + if (verifyFullPath || !allowEndOfArray) + { + for (size_t i = 0; i < path.Size(); ++i) + { + const PathEntry& entry = path[i]; + if (entry.IsEndOfArray() && (!allowEndOfArray || i != path.Size() - 1)) + { + return AZ::Failure("Append to array index (\"-\") specified for path that must already exist"); + } + } + } + + PathEntry destinationIndex = target[target.Size() - 1]; + target.Pop(); + + Value* targetValue = rootElement.FindMutableChild(target); + if (targetValue == nullptr) + { + return AZ::Failure(AZStd::string::format("Path not found (%s)", target.ToString().data())); + } + + if (destinationIndex.IsIndex() || destinationIndex.IsEndOfArray()) + { + if (!targetValue->IsArray() && !targetValue->IsNode()) + { + return AZ::Failure("Array index specified for a value that is not an array or node"); + } + + if (destinationIndex.IsIndex() && destinationIndex.GetIndex() >= targetValue->ArraySize()) + { + return AZ::Failure("Array index out bounds"); + } + } + else + { + if (!targetValue->IsObject() && !targetValue->IsNode()) + { + return AZ::Failure("Key specified for a value that is not an object or node"); + } + + if (verifyFullPath) + { + if (auto it = targetValue->FindMember(destinationIndex.GetKey()); it == targetValue->MemberEnd()) + { + return AZ::Failure("Key not found in container"); + } + } + } + + return AZ::Success({ *targetValue, AZStd::move(destinationIndex) }); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyAdd(Value& rootElement) const + { + auto pathLookup = LookupPath(rootElement, m_domPath, AllowEndOfArray); + if (!pathLookup.IsSuccess()) + { + return AZ::Failure(pathLookup.TakeError()); + } + const PathContext& context = pathLookup.GetValue(); + const PathEntry& destinationIndex = context.m_key; + Value& targetValue = context.m_value; + + if (destinationIndex.IsIndex() || destinationIndex.IsEndOfArray()) + { + if (destinationIndex.IsEndOfArray()) + { + targetValue.ArrayPushBack(GetValue()); + } + else + { + const size_t index = destinationIndex.GetIndex(); + auto& arrayToChange = targetValue.GetMutableArray(); + arrayToChange.insert(arrayToChange.begin() + index, GetValue()); + } + } + else + { + targetValue[destinationIndex] = GetValue(); + } + return AZ::Success(); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyRemove(Value& rootElement) const + { + auto pathLookup = LookupPath(rootElement, m_domPath, VerifyFullPath | AllowEndOfArray); + if (!pathLookup.IsSuccess()) + { + return AZ::Failure(pathLookup.TakeError()); + } + const PathContext& context = pathLookup.GetValue(); + const PathEntry& destinationIndex = context.m_key; + Value& targetValue = context.m_value; + + if (destinationIndex.IsIndex() || destinationIndex.IsEndOfArray()) + { + size_t index = destinationIndex.IsEndOfArray() ? targetValue.ArraySize() - 1 : destinationIndex.GetIndex(); + targetValue.ArrayErase(targetValue.MutableArrayBegin() + index); + } + else + { + auto it = targetValue.FindMutableMember(destinationIndex.GetKey()); + targetValue.EraseMember(it); + } + return AZ::Success(); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyReplace(Value& rootElement) const + { + auto pathLookup = LookupPath(rootElement, m_domPath, VerifyFullPath); + if (!pathLookup.IsSuccess()) + { + return AZ::Failure(pathLookup.TakeError()); + } + + rootElement[m_domPath] = GetValue(); + return AZ::Success(); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyCopy(Value& rootElement) const + { + auto sourceLookup = LookupPath(rootElement, GetSourcePath(), VerifyFullPath); + if (!sourceLookup.IsSuccess()) + { + return AZ::Failure(sourceLookup.TakeError()); + } + + auto destLookup = LookupPath(rootElement, m_domPath, AllowEndOfArray); + if (!destLookup.IsSuccess()) + { + return AZ::Failure(destLookup.TakeError()); + } + + rootElement[m_domPath] = rootElement[GetSourcePath()]; + return AZ::Success(); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyMove(Value& rootElement) const + { + auto sourceLookup = LookupPath(rootElement, GetSourcePath(), VerifyFullPath); + if (!sourceLookup.IsSuccess()) + { + return AZ::Failure(sourceLookup.TakeError()); + } + + auto destLookup = LookupPath(rootElement, m_domPath, AllowEndOfArray); + if (!destLookup.IsSuccess()) + { + return AZ::Failure(destLookup.TakeError()); + } + + Value valueToMove = rootElement[GetSourcePath()]; + const PathContext& sourceContext = sourceLookup.GetValue(); + if (sourceContext.m_key.IsEndOfArray()) + { + sourceContext.m_value.ArrayPopBack(); + } + else if (sourceContext.m_key.IsIndex()) + { + sourceContext.m_value.ArrayErase(sourceContext.m_value.MutableArrayBegin() + sourceContext.m_key.GetIndex()); + } + else + { + sourceContext.m_value.EraseMember(sourceContext.m_key.GetKey()); + } + + rootElement[m_domPath] = AZStd::move(valueToMove); + return AZ::Success(); + } + + PatchOperation::PatchOutcome PatchOperation::ApplyTest(Value& rootElement) const + { + auto pathLookup = LookupPath(rootElement, m_domPath, VerifyFullPath); + if (!pathLookup.IsSuccess()) + { + return AZ::Failure(pathLookup.TakeError()); + } + + if (!Utils::DeepCompareIsEqual(rootElement[m_domPath], GetValue())) + { + return AZ::Failure("Test failed, values don't match"); + } + + return AZ::Success(); + } + + namespace PatchApplicationStrategy + { + void HaltOnFailure(PatchApplicationState& state) + { + if (!state.m_outcome.IsSuccess()) + { + state.m_shouldContinue = false; + } + } + + void IgnoreFailureAndContinue([[maybe_unused]] PatchApplicationState& state) + { + } + } // namespace PatchApplicationStrategy + + Patch::Patch(AZStd::initializer_list init) + : m_operations(init) + { + } + + bool Patch::operator==(const Patch& rhs) const + { + if (m_operations.size() != rhs.m_operations.size()) + { + return false; + } + + for (size_t i = 0; i < m_operations.size(); ++i) + { + if (m_operations[i] != rhs.m_operations[i]) + { + return false; + } + } + + return true; + } + + bool Patch::operator!=(const Patch& rhs) const + { + return !operator==(rhs); + } + + const Patch::OperationsContainer& Patch::GetOperations() const + { + return m_operations; + } + + void Patch::PushBack(PatchOperation op) + { + m_operations.push_back(AZStd::move(op)); + } + + void Patch::PushFront(PatchOperation op) + { + m_operations.insert(m_operations.begin(), AZStd::move(op)); + } + + void Patch::Pop() + { + m_operations.pop_back(); + } + + void Patch::Clear() + { + m_operations.clear(); + } + + const PatchOperation& Patch::At(size_t index) const + { + return m_operations[index]; + } + + size_t Patch::Size() const + { + return m_operations.size(); + } + + PatchOperation& Patch::operator[](size_t index) + { + return m_operations[index]; + } + + const PatchOperation& Patch::operator[](size_t index) const + { + return m_operations[index]; + } + + Patch::OperationsContainer::iterator Patch::begin() + { + return m_operations.begin(); + } + + Patch::OperationsContainer::iterator Patch::end() + { + return m_operations.end(); + } + + Patch::OperationsContainer::const_iterator Patch::begin() const + { + return m_operations.cbegin(); + } + + Patch::OperationsContainer::const_iterator Patch::end() const + { + return m_operations.cend(); + } + + Patch::OperationsContainer::const_iterator Patch::cbegin() const + { + return m_operations.cbegin(); + } + + Patch::OperationsContainer::const_iterator Patch::cend() const + { + return m_operations.cend(); + } + + size_t Patch::size() const + { + return m_operations.size(); + } + + AZ::Outcome Patch::Apply(Value rootElement, StrategyFunctor strategy) const + { + auto result = ApplyInPlace(rootElement, strategy); + if (!result.IsSuccess()) + { + return AZ::Failure(result.TakeError()); + } + return AZ::Success(AZStd::move(rootElement)); + } + + AZ::Outcome Patch::ApplyInPlace(Value& rootElement, StrategyFunctor strategy) const + { + PatchApplicationState state; + state.m_currentState = &rootElement; + state.m_patch = this; + + for (const PatchOperation& operation : m_operations) + { + state.m_lastOperation = &operation; + state.m_outcome = operation.ApplyInPlace(rootElement); + strategy(state); + if (!state.m_shouldContinue) + { + break; + } + } + return state.m_outcome; + } + + Value Patch::GetDomRepresentation() const + { + Value domValue(Dom::Type::Array); + for (const PatchOperation& operation : m_operations) + { + domValue.ArrayPushBack(operation.GetDomRepresentation()); + } + return domValue; + } + + AZ::Outcome Patch::CreateFromDomRepresentation(Value domValue) + { + if (!domValue.IsArray()) + { + return AZ::Failure("Patch must be an array"); + } + + Patch patch; + for (auto it = domValue.ArrayBegin(); it != domValue.ArrayEnd(); ++it) + { + auto operationLoadResult = PatchOperation::CreateFromDomRepresentation(*it); + if (!operationLoadResult.IsSuccess()) + { + return AZ::Failure(operationLoadResult.TakeError()); + } + patch.PushBack(operationLoadResult.TakeValue()); + } + return AZ::Success(AZStd::move(patch)); + } + + PatchOperation PatchOperation::AddOperation(Path destinationPath, Value value) + { + return PatchOperation(AZStd::move(destinationPath), PatchOperation::Type::Add, AZStd::move(value)); + } + + PatchOperation PatchOperation::RemoveOperation(Path pathToRemove) + { + return PatchOperation(AZStd::move(pathToRemove), PatchOperation::Type::Remove); + } + + PatchOperation PatchOperation::ReplaceOperation(Path destinationPath, Value value) + { + return PatchOperation(AZStd::move(destinationPath), PatchOperation::Type::Replace, AZStd::move(value)); + } + + PatchOperation PatchOperation::CopyOperation(Path destinationPath, Path sourcePath) + { + return PatchOperation(AZStd::move(destinationPath), PatchOperation::Type::Copy, AZStd::move(sourcePath)); + } + + PatchOperation PatchOperation::MoveOperation(Path destinationPath, Path sourcePath) + { + return PatchOperation(AZStd::move(destinationPath), PatchOperation::Type::Move, AZStd::move(sourcePath)); + } + + PatchOperation PatchOperation::TestOperation(Path testPath, Value value) + { + return PatchOperation(AZStd::move(testPath), PatchOperation::Type::Test, AZStd::move(value)); + } + + PatchInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState) + { + PatchInfo patches; + + auto AddPatch = [&patches](PatchOperation op, PatchOperation inverse) + { + patches.m_forwardPatches.PushBack(AZStd::move(op)); + patches.m_inversePatches.PushFront(AZStd::move(inverse)); + }; + + AZStd::function CompareValues; + + struct PendingComparison + { + Path m_path; + const Value& m_before; + const Value& m_after; + + PendingComparison(Path path, const Value& before, const Value& after) + : m_path(AZStd::move(path)) + , m_before(before) + , m_after(after) + { + } + }; + AZStd::queue entriesToCompare; + + AZStd::unordered_set desiredKeys; + auto CompareObjects = [&](const Path& path, const Value& before, const Value& after) + { + desiredKeys.clear(); + Path subPath = path; + for (auto it = after.MemberBegin(); it != after.MemberEnd(); ++it) + { + desiredKeys.insert(it->first.GetHash()); + subPath.Push(it->first); + auto beforeIt = before.FindMember(it->first); + if (beforeIt == before.MemberEnd()) + { + AddPatch(PatchOperation::AddOperation(subPath, it->second), PatchOperation::RemoveOperation(subPath)); + } + else + { + entriesToCompare.emplace(subPath, beforeIt->second, it->second); + } + subPath.Pop(); + } + + for (auto it = before.MemberBegin(); it != before.MemberEnd(); ++it) + { + if (!desiredKeys.contains(it->first.GetHash())) + { + subPath.Push(it->first); + AddPatch(PatchOperation::RemoveOperation(subPath), PatchOperation::AddOperation(subPath, it->second)); + subPath.Pop(); + } + } + }; + + auto CompareArrays = [&](const Path& path, const Value& before, const Value& after) + { + const size_t beforeSize = before.ArraySize(); + const size_t afterSize = after.ArraySize(); + + // If more than replaceThreshold values differ, do a replace operation instead + constexpr size_t replaceThreshold = 3; + size_t changedValueCount = 0; + for (size_t i = 0; i < afterSize; ++i) + { + if (i < beforeSize) + { + if (before[i] != after[i]) + { + ++changedValueCount; + if (changedValueCount >= replaceThreshold) + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + return; + } + } + } + } + + Path subPath = path; + for (size_t i = 0; i < afterSize; ++i) + { + if (i >= beforeSize) + { + subPath.Push(PathEntry(PathEntry::EndOfArrayIndex)); + AddPatch(PatchOperation::AddOperation(subPath, after[i]), PatchOperation::RemoveOperation(subPath)); + subPath.Pop(); + } + else + { + subPath.Push(PathEntry(i)); + entriesToCompare.emplace(subPath, before[i], after[i]); + subPath.Pop(); + } + } + + if (beforeSize > afterSize) + { + subPath.Push(PathEntry(PathEntry::EndOfArrayIndex)); + for (size_t i = beforeSize; i > afterSize; --i) + { + AddPatch(PatchOperation::RemoveOperation(subPath), PatchOperation::AddOperation(subPath, before[i - 1])); + } + } + }; + + auto CompareNodes = [&](const Path& path, const Value& before, const Value& after) + { + if (before.GetNodeName() != after.GetNodeName()) + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + } + else + { + CompareObjects(path, before, after); + CompareArrays(path, before, after); + } + }; + + CompareValues = [&](const Path& path, const Value& before, const Value& after) + { + if (before.GetType() != after.GetType()) + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + } + else if (before == after) + { + // If a shallow comparison succeeds we're pointing to an identical value or container + // and don't need to drill down. + return; + } + else if (before.IsObject()) + { + CompareObjects(path, before, after); + } + else if (before.IsArray()) + { + CompareArrays(path, before, after); + } + else if (before.IsNode()) + { + CompareNodes(path, before, after); + } + else + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + } + }; + + entriesToCompare.emplace(Path(), beforeState, afterState); + while (!entriesToCompare.empty()) + { + PendingComparison& comparison = entriesToCompare.front(); + CompareValues(comparison.m_path, comparison.m_before, comparison.m_after); + entriesToCompare.pop(); + } + return patches; + } +} // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h new file mode 100644 index 0000000000..9d9ad7560a --- /dev/null +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h @@ -0,0 +1,191 @@ +/* + * 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 + +namespace AZ::Dom +{ + //! A patch operation that represents an atomic operation for mutating or validating a Value. + //! PatchOperations can be created with helper methods in Patch. /see Patch + class PatchOperation final + { + public: + using PatchOutcome = AZ::Outcome; + + //! The operation to perform. + enum class Type + { + Add, //!< Inserts or replaces the value at DestinationPath with Value + Remove, //!< Removes the entry at DestinationPath + Replace, //!< Replaces the value at DestinationPath with Value + Copy, //!< Copies the contents of SourcePath to DestinationPath + Move, //!< Moves the contents of SourcePath to DestinationPath + Test //!< Ensures the contents of DestinationPath match Value or fails, performs no mutations + }; + + PatchOperation() = default; + PatchOperation(const PatchOperation&) = default; + PatchOperation(PatchOperation&&) = default; + + explicit PatchOperation(Path destinationPath, Type type, Value value); + explicit PatchOperation(Path destionationPath, Type type, Path sourcePath); + explicit PatchOperation(Path path, Type type); + + static PatchOperation AddOperation(Path destinationPath, Value value); + static PatchOperation RemoveOperation(Path pathToRemove); + static PatchOperation ReplaceOperation(Path destinationPath, Value value); + static PatchOperation CopyOperation(Path destinationPath, Path sourcePath); + static PatchOperation MoveOperation(Path destinationPath, Path sourcePath); + static PatchOperation TestOperation(Path testPath, Value value); + + PatchOperation& operator=(const PatchOperation&) = default; + PatchOperation& operator=(PatchOperation&&) = default; + + bool operator==(const PatchOperation& rhs) const; + bool operator!=(const PatchOperation& rhs) const; + + Type GetType() const; + void SetType(Type type); + + const Path& GetDestinationPath() const; + void SetDestinationPath(Path path); + + const Value& GetValue() const; + void SetValue(Value value); + + const Path& GetSourcePath() const; + void SetSourcePath(Path path); + + AZ::Outcome Apply(Value rootElement) const; + PatchOutcome ApplyInPlace(Value& rootElement) const; + + Value GetDomRepresentation() const; + static AZ::Outcome CreateFromDomRepresentation(Value domValue); + + AZ::Outcome GetInverse(Value stateBeforeApplication) const; + + private: + struct PathContext + { + Value& m_value; + PathEntry m_key; + }; + + static constexpr AZ::u8 DefaultExistenceCheck = 0x0; + static constexpr AZ::u8 VerifyFullPath = 0x1; + static constexpr AZ::u8 AllowEndOfArray = 0x2; + + static AZ::Outcome LookupPath( + Value& rootElement, const Path& path, AZ::u8 existenceCheckFlags = DefaultExistenceCheck); + + PatchOutcome ApplyAdd(Value& rootElement) const; + PatchOutcome ApplyRemove(Value& rootElement) const; + PatchOutcome ApplyReplace(Value& rootElement) const; + PatchOutcome ApplyCopy(Value& rootElement) const; + PatchOutcome ApplyMove(Value& rootElement) const; + PatchOutcome ApplyTest(Value& rootElement) const; + + Path m_domPath; + Type m_type; + AZStd::variant m_value; + }; + + class Patch; + + //! The current state of a Patch application operation. + struct PatchApplicationState + { + //! The patch being applied. + const Patch* m_patch = nullptr; + //! The last operation attempted. + const PatchOperation* m_lastOperation = nullptr; + //! The outcome of the last operation, may be overridden to produce a different failure outcome. + PatchOperation::PatchOutcome m_outcome; + //! The current state of the value being patched, will be returned if the patch operation succeeds. + Value* m_currentState = nullptr; + //! If set to false, the patch operation should halt. + bool m_shouldContinue = true; + }; + + namespace PatchApplicationStrategy + { + //! The default patching strategy. Applies all operations in a patch, but halts if any one operation fails. + void HaltOnFailure(PatchApplicationState& state); + //! Patching strategy that attemps to apply all operations in a patch, but ignores operation failures and continues. + void IgnoreFailureAndContinue(PatchApplicationState& state); + } // namespace PatchApplicationStrategy + + //! A set of operations that can be applied to a Value to produce a new Value. + //! \see PatchOperation + class Patch final + { + public: + using StrategyFunctor = AZStd::function; + using OperationsContainer = AZStd::vector; + + Patch() = default; + Patch(const Patch&) = default; + Patch(Patch&&) = default; + Patch(AZStd::initializer_list init); + + template + Patch(InputIterator first, InputIterator last) + : m_operations(first, last) + { + } + + Patch& operator=(const Patch&) = default; + Patch& operator=(Patch&&) = default; + + bool operator==(const Patch& rhs) const; + bool operator!=(const Patch& rhs) const; + + const OperationsContainer& GetOperations() const; + void PushBack(PatchOperation op); + void PushFront(PatchOperation op); + void Pop(); + void Clear(); + const PatchOperation& At(size_t index) const; + size_t Size() const; + + PatchOperation& operator[](size_t index); + const PatchOperation& operator[](size_t index) const; + + OperationsContainer::iterator begin(); + OperationsContainer::iterator end(); + OperationsContainer::const_iterator begin() const; + OperationsContainer::const_iterator end() const; + OperationsContainer::const_iterator cbegin() const; + OperationsContainer::const_iterator cend() const; + size_t size() const; + + AZ::Outcome Apply(Value rootElement, StrategyFunctor strategy = PatchApplicationStrategy::HaltOnFailure) const; + AZ::Outcome ApplyInPlace(Value& rootElement, StrategyFunctor strategy = PatchApplicationStrategy::HaltOnFailure) const; + + Value GetDomRepresentation() const; + static AZ::Outcome CreateFromDomRepresentation(Value domValue); + + private: + OperationsContainer m_operations; + }; + + //! A set of patches for applying a change and doing the inverse operation (i.e. undoing it). + struct PatchInfo + { + Patch m_forwardPatches; + Patch m_inversePatches; + }; + + //! Generates a set of patches such that m_forwardPatches.Apply(beforeState) shall produce a document equivalent to afterState, and + //! a subsequent m_inversePatches.Apply(beforeState) shall produce the original document. This patch generation strategy does a + //! hierarchical comparison and is not guaranteed to create the minimal set of patches required to transform between the two states. + PatchInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState); +} // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp index bc50a8513c..a5d130d48c 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp @@ -53,17 +53,20 @@ namespace AZ::Dom bool PathEntry::operator==(size_t value) const { - return IsIndex() && GetIndex() == value; + const size_t* internalValue = AZStd::get_if(&m_value); + return internalValue == nullptr ? false : (*internalValue) == value; } bool PathEntry::operator==(const AZ::Name& key) const { - return IsKey() && GetKey() == key; + const AZ::Name* internalValue = AZStd::get_if(&m_value); + return internalValue == nullptr ? false : (*internalValue) == key; } bool PathEntry::operator==(AZStd::string_view key) const { - return IsKey() && GetKey() == AZ::Name(key); + const AZ::Name* internalValue = AZStd::get_if(&m_value); + return internalValue == nullptr ? false : (*internalValue) == AZ::Name(key); } bool PathEntry::operator!=(const PathEntry& other) const @@ -73,17 +76,17 @@ namespace AZ::Dom bool PathEntry::operator!=(size_t value) const { - return !IsIndex() || GetIndex() != value; + return !operator==(value); } bool PathEntry::operator!=(const AZ::Name& key) const { - return !IsKey() || GetKey() != key; + return !operator==(key); } bool PathEntry::operator!=(AZStd::string_view key) const { - return !IsKey() || GetKey() != AZ::Name(key); + return !operator==(key); } void PathEntry::SetEndOfArray() diff --git a/Code/Framework/AzCore/AzCore/azcore_files.cmake b/Code/Framework/AzCore/AzCore/azcore_files.cmake index 2c0c318648..c5928f1c84 100644 --- a/Code/Framework/AzCore/AzCore/azcore_files.cmake +++ b/Code/Framework/AzCore/AzCore/azcore_files.cmake @@ -116,6 +116,8 @@ set(FILES Debug/TraceReflection.h DOM/DomBackend.cpp DOM/DomBackend.h + DOM/DomPatch.cpp + DOM/DomPatch.h DOM/DomPath.cpp DOM/DomPath.h DOM/DomUtils.cpp diff --git a/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp new file mode 100644 index 0000000000..de19ee1783 --- /dev/null +++ b/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp @@ -0,0 +1,179 @@ +/* + * 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 +#include +#include +#include + +namespace AZ::Dom::Benchmark +{ + class DomPatchBenchmark : public Tests::DomBenchmarkFixture + { + public: + void TearDownHarness() override + { + m_before = {}; + m_after = {}; + Tests::DomBenchmarkFixture::TearDownHarness(); + } + + void SimpleReplace(benchmark::State& state, bool deepCopy, bool apply) + { + m_before = GenerateDomBenchmarkPayload(state.range(0), state.range(1)); + m_after = deepCopy ? Utils::DeepCopy(m_before) : m_before; + m_after["entries"]["Key0"] = Value("replacement string", true); + + RunBenchmarkInternal(state, apply); + } + + void TopLevelReplace(benchmark::State& state, bool apply) + { + m_before = GenerateDomBenchmarkPayload(state.range(0), state.range(1)); + m_after = Value(Type::Object); + m_after["UnrelatedKey"] = Value(42); + + RunBenchmarkInternal(state, apply); + } + + void KeyRemove(benchmark::State& state, bool deepCopy, bool apply) + { + m_before = GenerateDomBenchmarkPayload(state.range(0), state.range(1)); + m_after = deepCopy ? Utils::DeepCopy(m_before) : m_before; + m_after["entries"].RemoveMember("Key1"); + + RunBenchmarkInternal(state, apply); + } + + void ArrayAppend(benchmark::State& state, bool deepCopy, bool apply) + { + m_before = GenerateDomBenchmarkPayload(state.range(0), state.range(1)); + m_after = deepCopy ? Utils::DeepCopy(m_before) : m_before; + m_after["entries"]["Key2"].ArrayPushBack(Value(0)); + + RunBenchmarkInternal(state, apply); + } + + void ArrayPrepend(benchmark::State& state, bool deepCopy, bool apply) + { + m_before = GenerateDomBenchmarkPayload(state.range(0), state.range(1)); + m_after = deepCopy ? Utils::DeepCopy(m_before) : m_before; + auto& arr = m_after["entries"]["Key2"].GetMutableArray(); + arr.insert(arr.begin(), Value(42)); + + RunBenchmarkInternal(state, apply); + } + + private: + void RunBenchmarkInternal(benchmark::State& state, bool apply) + { + if (apply) + { + auto patchInfo = GenerateHierarchicalDeltaPatch(m_before, m_after); + for (auto _ : state) + { + auto patchResult = patchInfo.m_forwardPatches.Apply(m_before); + benchmark::DoNotOptimize(patchResult); + } + } + else + { + for (auto _ : state) + { + auto patchInfo = GenerateHierarchicalDeltaPatch(m_before, m_after); + benchmark::DoNotOptimize(patchInfo); + } + } + + state.SetItemsProcessed(state.iterations()); + } + + Value m_before; + Value m_after; + }; + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_SimpleReplace_ShallowCopy)(benchmark::State& state) + { + SimpleReplace(state, false, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_SimpleReplace_ShallowCopy) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_SimpleReplace_DeepCopy)(benchmark::State& state) + { + SimpleReplace(state, true, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_SimpleReplace_DeepCopy) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_TopLevelReplace)(benchmark::State& state) + { + TopLevelReplace(state, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_TopLevelReplace) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_KeyRemove_ShallowCopy)(benchmark::State& state) + { + KeyRemove(state, false, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_KeyRemove_ShallowCopy) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_KeyRemove_DeepCopy)(benchmark::State& state) + { + KeyRemove(state, true, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_KeyRemove_DeepCopy) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_ArrayAppend_ShallowCopy)(benchmark::State& state) + { + ArrayAppend(state, false, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_ArrayAppend_ShallowCopy) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_ArrayAppend_DeepCopy)(benchmark::State& state) + { + ArrayAppend(state, true, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_ArrayAppend_DeepCopy) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Generate_ArrayPrepend)(benchmark::State& state) + { + ArrayPrepend(state, true, false); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Generate_ArrayPrepend) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Apply_SimpleReplace)(benchmark::State& state) + { + SimpleReplace(state, true, true); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Apply_SimpleReplace) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Apply_TopLevelReplace)(benchmark::State& state) + { + TopLevelReplace(state, true); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Apply_TopLevelReplace) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Apply_KeyRemove)(benchmark::State& state) + { + KeyRemove(state, true, true); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Apply_KeyRemove) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Apply_ArrayAppend)(benchmark::State& state) + { + ArrayAppend(state, true, true); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Apply_ArrayAppend) + + BENCHMARK_DEFINE_F(DomPatchBenchmark, AzDomPatch_Apply_ArrayPrepend)(benchmark::State& state) + { + ArrayPrepend(state, true, true); + } + DOM_REGISTER_SERIALIZATION_BENCHMARK_MS(DomPatchBenchmark, AzDomPatch_Apply_ArrayPrepend) +} // namespace AZ::Dom::Benchmark diff --git a/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp b/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp new file mode 100644 index 0000000000..a0e4f349a5 --- /dev/null +++ b/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp @@ -0,0 +1,562 @@ +/* + * 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 + +namespace AZ::Dom::Tests +{ + class DomPatchTests : public DomTestFixture + { + public: + void SetUp() override + { + DomTestFixture::SetUp(); + + m_dataset = Value(Type::Object); + m_dataset["arr"].SetArray(); + + m_dataset["node"].SetNode("SomeNode"); + m_dataset["node"]["int"] = 5; + m_dataset["node"]["null"] = Value(); + + for (int i = 0; i < 5; ++i) + { + m_dataset["arr"].ArrayPushBack(Value(i)); + m_dataset["node"].ArrayPushBack(Value(i * 2)); + } + + m_dataset["obj"].SetObject(); + m_dataset["obj"]["foo"] = true; + m_dataset["obj"]["bar"] = false; + + m_deltaDataset = m_dataset; + } + + void TearDown() override + { + m_dataset = m_deltaDataset = Value(); + + DomTestFixture::TearDown(); + } + + PatchInfo GenerateAndVerifyDelta() + { + PatchInfo info = GenerateHierarchicalDeltaPatch(m_dataset, m_deltaDataset); + + auto result = info.m_forwardPatches.Apply(m_dataset); + EXPECT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(result.GetValue(), m_deltaDataset)); + + result = info.m_inversePatches.Apply(result.GetValue()); + EXPECT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(result.GetValue(), m_dataset)); + + // Verify serialization of the patches + auto VerifySerialization = [](const Patch& patch) + { + Value serializedPatch = patch.GetDomRepresentation(); + auto deserializePatchResult = Patch::CreateFromDomRepresentation(serializedPatch); + EXPECT_TRUE(deserializePatchResult.IsSuccess()); + EXPECT_EQ(deserializePatchResult.GetValue(), patch); + }; + VerifySerialization(info.m_forwardPatches); + VerifySerialization(info.m_inversePatches); + + return info; + } + + Value m_dataset; + Value m_deltaDataset; + }; + + TEST_F(DomPatchTests, AddOperation_InsertInObject_Succeeds) + { + Path p("/obj/baz"); + PatchOperation op = PatchOperation::AddOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()[p].GetInt64(), 42); + } + + TEST_F(DomPatchTests, AddOperation_ReplaceInObject_Succeeds) + { + Path p("/obj/foo"); + PatchOperation op = PatchOperation::AddOperation(p, Value(false)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()[p].GetBool(), false); + } + + TEST_F(DomPatchTests, AddOperation_InsertObjectKeyInArray_Fails) + { + Path p("/arr/key"); + PatchOperation op = PatchOperation::AddOperation(p, Value(999)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, AddOperation_AppendInArray_Succeeds) + { + Path p("/arr/-"); + PatchOperation op = PatchOperation::AddOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()["arr"][5].GetInt64(), 42); + } + + TEST_F(DomPatchTests, AddOperation_InsertKeyInNode_Succeeds) + { + Path p("/node/attr"); + PatchOperation op = PatchOperation::AddOperation(p, Value(500)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()[p].GetInt64(), 500); + } + + TEST_F(DomPatchTests, AddOperation_ReplaceIndexInNode_Succeeds) + { + Path p("/node/0"); + PatchOperation op = PatchOperation::AddOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()[p].GetInt64(), 42); + } + + TEST_F(DomPatchTests, AddOperation_AppendInNode_Succeeds) + { + Path p("/node/-"); + PatchOperation op = PatchOperation::AddOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()["node"][5].GetInt64(), 42); + } + + TEST_F(DomPatchTests, AddOperation_InvalidPath_Fails) + { + Path p("/non/existent/path"); + PatchOperation op = PatchOperation::AddOperation(p, Value(0)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, RemoveOperation_RemoveKeyFromObject_Succeeds) + { + Path p("/obj/foo"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_FALSE(result.GetValue()["obj"].HasMember("foo")); + } + + TEST_F(DomPatchTests, RemoveOperation_RemoveIndexFromArray_Succeeds) + { + Path p("/arr/0"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()["arr"].ArraySize(), 4); + EXPECT_EQ(result.GetValue()["arr"][0].GetInt64(), 1); + } + + TEST_F(DomPatchTests, RemoveOperation_PopArray_Succeeds) + { + Path p("/arr/-"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + EXPECT_EQ(result.GetValue()["arr"].ArraySize(), 4); + } + + TEST_F(DomPatchTests, RemoveOperation_RemoveKeyFromNode_Succeeds) + { + Path p("/node/int"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_FALSE(result.GetValue()["node"].HasMember("int")); + } + + TEST_F(DomPatchTests, RemoveOperation_RemoveIndexFromNode_Succeeds) + { + Path p("/node/1"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()["node"].ArraySize(), 4); + EXPECT_EQ(result.GetValue()["node"][1].GetInt64(), 4); + } + + TEST_F(DomPatchTests, RemoveOperation_PopIndexFromNode_Succeeds) + { + Path p("/node/-"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()["node"].ArraySize(), 4); + } + + TEST_F(DomPatchTests, RemoveOperation_RemoveKeyFromArray_Fails) + { + Path p("/arr/foo"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, RemoveOperation_InvalidPath_Fails) + { + Path p("/non/existent/path"); + PatchOperation op = PatchOperation::RemoveOperation(p); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, ReplaceOperation_InsertInObject_Fails) + { + Path p("/obj/baz"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, ReplaceOperation_ReplaceInObject_Succeeds) + { + Path p("/obj/foo"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(false)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()[p].GetBool(), false); + } + + TEST_F(DomPatchTests, ReplaceOperation_InsertObjectKeyInArray_Fails) + { + Path p("/arr/key"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(999)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, ReplaceOperation_AppendInArray_Fails) + { + Path p("/arr/-"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, ReplaceOperation_InsertKeyInNode_Fails) + { + Path p("/node/attr"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(500)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, ReplaceOperation_ReplaceIndexInNode_Succeeds) + { + Path p("/node/0"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_EQ(result.GetValue()[p].GetInt64(), 42); + } + + TEST_F(DomPatchTests, ReplaceOperation_AppendInNode_Fails) + { + Path p("/node/-"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(42)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, ReplaceOperation_InvalidPath_Fails) + { + Path p("/non/existent/path"); + PatchOperation op = PatchOperation::ReplaceOperation(p, Value(0)); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, CopyOperation_ArrayToObject_Succeeds) + { + Path dest("/obj/arr"); + Path src("/arr"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(m_dataset[src], result.GetValue()[dest])); + EXPECT_TRUE(Utils::DeepCompareIsEqual(result.GetValue()[src], result.GetValue()[dest])); + } + + TEST_F(DomPatchTests, CopyOperation_ObjectToArrayInRange_Succeeds) + { + Path dest("/arr/0"); + Path src("/obj"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(m_dataset[src], result.GetValue()[dest])); + EXPECT_TRUE(Utils::DeepCompareIsEqual(result.GetValue()[src], result.GetValue()[dest])); + } + + TEST_F(DomPatchTests, CopyOperation_ObjectToArrayOutOfRange_Fails) + { + Path dest("/arr/5"); + Path src("/obj"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, CopyOperation_ObjectToNodeChildInRange_Succeeds) + { + Path dest("/node/0"); + Path src("/obj"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(m_dataset[src], result.GetValue()[dest])); + EXPECT_TRUE(Utils::DeepCompareIsEqual(result.GetValue()[src], result.GetValue()[dest])); + } + + TEST_F(DomPatchTests, CopyOperation_ObjectToNodeChildOutOfRange_Fails) + { + Path dest("/node/5"); + Path src("/obj"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, CopyOperation_InvalidSourcePath_Fails) + { + Path dest("/node/0"); + Path src("/invalid/path"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, CopyOperation_InvalidDestinationPath_Fails) + { + Path dest("/invalid/path"); + Path src("/arr/0"); + PatchOperation op = PatchOperation::CopyOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, MoveOperation_ArrayToObject_Succeeds) + { + Path dest("/obj/arr"); + Path src("/arr"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(m_dataset[src], result.GetValue()[dest])); + EXPECT_FALSE(result.GetValue().HasMember("arr")); + } + + TEST_F(DomPatchTests, MoveOperation_ObjectToArrayInRange_Succeeds) + { + Path dest("/arr/0"); + Path src("/obj"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(m_dataset[src], result.GetValue()[dest])); + EXPECT_FALSE(result.GetValue().HasMember("obj")); + } + + TEST_F(DomPatchTests, MoveOperation_ObjectToArrayOutOfRange_Fails) + { + Path dest("/arr/5"); + Path src("/obj"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, MoveOperation_ObjectToNodeChildInRange_Succeeds) + { + Path dest("/node/0"); + Path src("/obj"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + EXPECT_TRUE(Utils::DeepCompareIsEqual(m_dataset[src], result.GetValue()[dest])); + EXPECT_FALSE(result.GetValue().HasMember("obj")); + } + + TEST_F(DomPatchTests, MoveOperation_ObjectToNodeChildOutOfRange_Fails) + { + Path dest("/node/5"); + Path src("/obj"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, MoveOperation_InvalidSourcePath_Fails) + { + Path dest("/node/0"); + Path src("/invalid/path"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, MoveOperation_InvalidDestinationPath_Fails) + { + Path dest("/invalid/path"); + Path src("/arr/0"); + PatchOperation op = PatchOperation::MoveOperation(dest, src); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestOperation_TestCorrectValue_Succeeds) + { + Path path("/arr/1"); + Value value(1); + PatchOperation op = PatchOperation::TestOperation(path, value); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestOperation_TestIncorrectValue_Fails) + { + Path path("/arr/1"); + Value value(55); + PatchOperation op = PatchOperation::TestOperation(path, value); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestOperation_TestCorrectComplexValue_Succeeds) + { + Path path; + Value value = m_dataset; + PatchOperation op = PatchOperation::TestOperation(path, value); + auto result = op.Apply(m_dataset); + ASSERT_TRUE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestOperation_TestIncorrectComplexValue_Fails) + { + Path path; + Value value = m_dataset; + value["arr"][4] = 9; + PatchOperation op = PatchOperation::TestOperation(path, value); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestOperation_TestInvalidPath_Fails) + { + Path path("/invalid/path"); + Value value; + PatchOperation op = PatchOperation::TestOperation(path, value); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestOperation_TestInsertArrayPath_Fails) + { + Path path("/arr/-"); + Value value(4); + PatchOperation op = PatchOperation::TestOperation(path, value); + auto result = op.Apply(m_dataset); + ASSERT_FALSE(result.IsSuccess()); + } + + TEST_F(DomPatchTests, TestPatch_ReplaceArrayValue) + { + m_deltaDataset["arr"][0] = 5; + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_AppendArrayValue) + { + m_deltaDataset["arr"].ArrayPushBack(Value(7)); + auto result = GenerateAndVerifyDelta(); + + // Ensure the generated patch uses the array append operation + ASSERT_EQ(result.m_forwardPatches.Size(), 1); + EXPECT_TRUE(result.m_forwardPatches[0].GetDestinationPath()[1].IsEndOfArray()); + } + + TEST_F(DomPatchTests, TestPatch_AppendArrayValues) + { + m_deltaDataset["arr"].ArrayPushBack(Value(7)); + m_deltaDataset["arr"].ArrayPushBack(Value(8)); + m_deltaDataset["arr"].ArrayPushBack(Value(9)); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_InsertArrayValue) + { + auto& arr = m_deltaDataset["arr"].GetMutableArray(); + arr.insert(arr.begin(), Value(42)); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_InsertObjectKey) + { + m_deltaDataset["obj"]["newKey"].CopyFromString("test"); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_DeleteObjectKey) + { + m_deltaDataset["obj"].RemoveMember("foo"); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_AppendNodeValues) + { + m_deltaDataset["node"].ArrayPushBack(Value(7)); + m_deltaDataset["node"].ArrayPushBack(Value(8)); + m_deltaDataset["node"].ArrayPushBack(Value(9)); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_InsertNodeValue) + { + auto& node = m_deltaDataset["node"].GetMutableNode(); + node.GetChildren().insert(node.GetChildren().begin(), Value(42)); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_InsertNodeKey) + { + m_deltaDataset["node"]["newKey"].CopyFromString("test"); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_DeleteNodeKey) + { + m_deltaDataset["node"].RemoveMember("int"); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_RenameNode) + { + m_deltaDataset["node"].SetNodeName("RenamedNode"); + GenerateAndVerifyDelta(); + } + + TEST_F(DomPatchTests, TestPatch_ReplaceRoot) + { + m_deltaDataset = Value(Type::Array); + m_deltaDataset.ArrayPushBack(Value(2)); + m_deltaDataset.ArrayPushBack(Value(4)); + m_deltaDataset.ArrayPushBack(Value(6)); + GenerateAndVerifyDelta(); + } +} // namespace AZ::Dom::Tests diff --git a/Code/Framework/AzCore/Tests/azcoretests_files.cmake b/Code/Framework/AzCore/Tests/azcoretests_files.cmake index 24a6601cf0..86bf943a5a 100644 --- a/Code/Framework/AzCore/Tests/azcoretests_files.cmake +++ b/Code/Framework/AzCore/Tests/azcoretests_files.cmake @@ -221,6 +221,8 @@ set(FILES DOM/DomJsonBenchmarks.cpp DOM/DomPathTests.cpp DOM/DomPathBenchmarks.cpp + DOM/DomPatchTests.cpp + DOM/DomPatchBenchmarks.cpp DOM/DomValueTests.cpp DOM/DomValueBenchmarks.cpp ) From 73f166241e33edd33d2211c8c91a1dcf786fb3e5 Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Thu, 27 Jan 2022 13:15:30 -0800 Subject: [PATCH 2/6] Address review feedback Signed-off-by: Nicholas Van Sickle --- Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp | 101 ++++++++++-------- Code/Framework/AzCore/AzCore/DOM/DomPatch.h | 35 ++++-- Code/Framework/AzCore/AzCore/DOM/DomPath.cpp | 18 +++- Code/Framework/AzCore/AzCore/DOM/DomPath.h | 11 +- .../AzCore/Tests/DOM/DomPathBenchmarks.cpp | 21 ++++ .../AzCore/Tests/DOM/DomPathTests.cpp | 19 ++++ 6 files changed, 143 insertions(+), 62 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp index 963308e9c8..6e53de0689 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp @@ -8,8 +8,8 @@ #include #include -#include #include +#include namespace AZ::Dom { @@ -257,7 +257,8 @@ namespace AZ::Dom return AZ::Failure(sourceLoad.TakeError()); } - return AZ::Success(PatchOperation::CopyOperation(Path(destLoad.GetValue().GetString()), Path(sourceLoad.GetValue().GetString()))); + return AZ::Success( + PatchOperation::CopyOperation(Path(destLoad.GetValue().GetString()), Path(sourceLoad.GetValue().GetString()))); } else if (op == "move") { @@ -272,7 +273,8 @@ namespace AZ::Dom return AZ::Failure(sourceLoad.TakeError()); } - return AZ::Success(PatchOperation::MoveOperation(Path(destLoad.GetValue().GetString()), Path(sourceLoad.GetValue().GetString()))); + return AZ::Success( + PatchOperation::MoveOperation(Path(destLoad.GetValue().GetString()), Path(sourceLoad.GetValue().GetString()))); } else if (op == "test") { @@ -319,8 +321,9 @@ namespace AZ::Dom const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); if (existingValue == nullptr) { - return AZ::Failure( - AZStd::string::format("Unable to invert DOM remove patch, source path not found: %s", m_domPath.ToString().data())); + AZStd::string errorMessage = "Unable to invert DOM remove patch, source path not found: "; + m_domPath.AppendToString(errorMessage); + return AZ::Failure(AZStd::move(errorMessage)); } return AZ::Success(PatchOperation::AddOperation(m_domPath, *existingValue)); } @@ -330,8 +333,9 @@ namespace AZ::Dom const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); if (existingValue == nullptr) { - return AZ::Failure(AZStd::string::format( - "Unable to invert DOM replace patch, source path not found: %s", m_domPath.ToString().data())); + AZStd::string errorMessage = "Unable to invert DOM replace patch, source path not found: "; + m_domPath.AppendToString(errorMessage); + return AZ::Failure(AZStd::move(errorMessage)); } return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); } @@ -341,8 +345,9 @@ namespace AZ::Dom const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); if (existingValue == nullptr) { - return AZ::Failure( - AZStd::string::format("Unable to invert DOM copy patch, source path not found: %s", m_domPath.ToString().data())); + AZStd::string errorMessage = "Unable to invert DOM copy patch, source path not found: "; + m_domPath.AppendToString(errorMessage); + return AZ::Failure(AZStd::move(errorMessage)); } return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); } @@ -367,8 +372,9 @@ namespace AZ::Dom const Value* existingValue = stateBeforeApplication.FindChild(commonAncestor); if (existingValue == nullptr) { - return AZ::Failure(AZStd::string::format( - "Unable to invert DOM move patch, common ancestor path not found: %s", commonAncestor.ToString().data())); + AZStd::string errorMessage = "Unable to invert DOM copy patch, common ancestor path not found: "; + commonAncestor.AppendToString(errorMessage); + return AZ::Failure(AZStd::move(errorMessage)); } return AZ::Success(PatchOperation::ReplaceOperation(commonAncestor, *existingValue)); } @@ -383,10 +389,10 @@ namespace AZ::Dom } AZ::Outcome PatchOperation::LookupPath( - Value& rootElement, const Path& path, AZ::u8 existenceCheckFlags) + Value& rootElement, const Path& path, ExistenceCheckFlags flags) { - const bool verifyFullPath = existenceCheckFlags & VerifyFullPath; - const bool allowEndOfArray = existenceCheckFlags & AllowEndOfArray; + const bool verifyFullPath = (flags & ExistenceCheckFlags::VerifyFullPath) != ExistenceCheckFlags::DefaultExistenceCheck; + const bool allowEndOfArray = (flags & ExistenceCheckFlags::AllowEndOfArray) != ExistenceCheckFlags::DefaultExistenceCheck; Path target = path; if (target.Size() == 0) @@ -414,7 +420,9 @@ namespace AZ::Dom Value* targetValue = rootElement.FindMutableChild(target); if (targetValue == nullptr) { - return AZ::Failure(AZStd::string::format("Path not found (%s)", target.ToString().data())); + AZStd::string errorMessage = "Path not found: "; + target.AppendToString(errorMessage); + return AZ::Failure(AZStd::move(errorMessage)); } if (destinationIndex.IsIndex() || destinationIndex.IsEndOfArray()) @@ -450,7 +458,7 @@ namespace AZ::Dom PatchOperation::PatchOutcome PatchOperation::ApplyAdd(Value& rootElement) const { - auto pathLookup = LookupPath(rootElement, m_domPath, AllowEndOfArray); + auto pathLookup = LookupPath(rootElement, m_domPath, ExistenceCheckFlags::AllowEndOfArray); if (!pathLookup.IsSuccess()) { return AZ::Failure(pathLookup.TakeError()); @@ -481,7 +489,7 @@ namespace AZ::Dom PatchOperation::PatchOutcome PatchOperation::ApplyRemove(Value& rootElement) const { - auto pathLookup = LookupPath(rootElement, m_domPath, VerifyFullPath | AllowEndOfArray); + auto pathLookup = LookupPath(rootElement, m_domPath, ExistenceCheckFlags::VerifyFullPath | ExistenceCheckFlags::AllowEndOfArray); if (!pathLookup.IsSuccess()) { return AZ::Failure(pathLookup.TakeError()); @@ -505,7 +513,7 @@ namespace AZ::Dom PatchOperation::PatchOutcome PatchOperation::ApplyReplace(Value& rootElement) const { - auto pathLookup = LookupPath(rootElement, m_domPath, VerifyFullPath); + auto pathLookup = LookupPath(rootElement, m_domPath, ExistenceCheckFlags::VerifyFullPath); if (!pathLookup.IsSuccess()) { return AZ::Failure(pathLookup.TakeError()); @@ -517,13 +525,13 @@ namespace AZ::Dom PatchOperation::PatchOutcome PatchOperation::ApplyCopy(Value& rootElement) const { - auto sourceLookup = LookupPath(rootElement, GetSourcePath(), VerifyFullPath); + auto sourceLookup = LookupPath(rootElement, GetSourcePath(), ExistenceCheckFlags::VerifyFullPath); if (!sourceLookup.IsSuccess()) { return AZ::Failure(sourceLookup.TakeError()); } - auto destLookup = LookupPath(rootElement, m_domPath, AllowEndOfArray); + auto destLookup = LookupPath(rootElement, m_domPath, ExistenceCheckFlags::AllowEndOfArray); if (!destLookup.IsSuccess()) { return AZ::Failure(destLookup.TakeError()); @@ -535,13 +543,13 @@ namespace AZ::Dom PatchOperation::PatchOutcome PatchOperation::ApplyMove(Value& rootElement) const { - auto sourceLookup = LookupPath(rootElement, GetSourcePath(), VerifyFullPath); + auto sourceLookup = LookupPath(rootElement, GetSourcePath(), ExistenceCheckFlags::VerifyFullPath); if (!sourceLookup.IsSuccess()) { return AZ::Failure(sourceLookup.TakeError()); } - auto destLookup = LookupPath(rootElement, m_domPath, AllowEndOfArray); + auto destLookup = LookupPath(rootElement, m_domPath, ExistenceCheckFlags::AllowEndOfArray); if (!destLookup.IsSuccess()) { return AZ::Failure(destLookup.TakeError()); @@ -568,7 +576,7 @@ namespace AZ::Dom PatchOperation::PatchOutcome PatchOperation::ApplyTest(Value& rootElement) const { - auto pathLookup = LookupPath(rootElement, m_domPath, VerifyFullPath); + auto pathLookup = LookupPath(rootElement, m_domPath, ExistenceCheckFlags::VerifyFullPath); if (!pathLookup.IsSuccess()) { return AZ::Failure(pathLookup.TakeError()); @@ -670,32 +678,32 @@ namespace AZ::Dom return m_operations[index]; } - Patch::OperationsContainer::iterator Patch::begin() + auto Patch::begin() -> OperationsContainer::iterator { return m_operations.begin(); } - Patch::OperationsContainer::iterator Patch::end() + auto Patch::end() -> OperationsContainer::iterator { return m_operations.end(); } - Patch::OperationsContainer::const_iterator Patch::begin() const + auto Patch::begin() const -> OperationsContainer::const_iterator { return m_operations.cbegin(); } - Patch::OperationsContainer::const_iterator Patch::end() const + auto Patch::end() const -> OperationsContainer::const_iterator { return m_operations.cend(); } - Patch::OperationsContainer::const_iterator Patch::cbegin() const + auto Patch::cbegin() const -> OperationsContainer::const_iterator { return m_operations.cbegin(); } - Patch::OperationsContainer::const_iterator Patch::cend() const + auto Patch::cend() const -> OperationsContainer::const_iterator { return m_operations.cend(); } @@ -794,7 +802,8 @@ namespace AZ::Dom return PatchOperation(AZStd::move(testPath), PatchOperation::Type::Test, AZStd::move(value)); } - PatchInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState) + PatchInfo GenerateHierarchicalDeltaPatch( + const Value& beforeState, const Value& afterState, const DeltaPatchGenerationParameters& params) { PatchInfo patches; @@ -804,7 +813,7 @@ namespace AZ::Dom patches.m_inversePatches.PushFront(AZStd::move(inverse)); }; - AZStd::function CompareValues; + AZStd::function compareValues; struct PendingComparison { @@ -822,7 +831,7 @@ namespace AZ::Dom AZStd::queue entriesToCompare; AZStd::unordered_set desiredKeys; - auto CompareObjects = [&](const Path& path, const Value& before, const Value& after) + auto compareObjects = [&](const Path& path, const Value& before, const Value& after) { desiredKeys.clear(); Path subPath = path; @@ -853,22 +862,22 @@ namespace AZ::Dom } }; - auto CompareArrays = [&](const Path& path, const Value& before, const Value& after) + auto compareArrays = [&](const Path& path, const Value& before, const Value& after) { const size_t beforeSize = before.ArraySize(); const size_t afterSize = after.ArraySize(); // If more than replaceThreshold values differ, do a replace operation instead - constexpr size_t replaceThreshold = 3; - size_t changedValueCount = 0; - for (size_t i = 0; i < afterSize; ++i) + if (params.m_replaceThreshold != DeltaPatchGenerationParameters::NoReplace) { - if (i < beforeSize) + size_t changedValueCount = 0; + const size_t entriesToEnumerate = AZStd::min(beforeSize, afterSize); + for (size_t i = 0; i < entriesToEnumerate; ++i) { if (before[i] != after[i]) { ++changedValueCount; - if (changedValueCount >= replaceThreshold) + if (changedValueCount >= params.m_replaceThreshold) { AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); return; @@ -904,7 +913,7 @@ namespace AZ::Dom } }; - auto CompareNodes = [&](const Path& path, const Value& before, const Value& after) + auto compareNodes = [&](const Path& path, const Value& before, const Value& after) { if (before.GetNodeName() != after.GetNodeName()) { @@ -912,12 +921,12 @@ namespace AZ::Dom } else { - CompareObjects(path, before, after); - CompareArrays(path, before, after); + compareObjects(path, before, after); + compareArrays(path, before, after); } }; - CompareValues = [&](const Path& path, const Value& before, const Value& after) + compareValues = [&](const Path& path, const Value& before, const Value& after) { if (before.GetType() != after.GetType()) { @@ -931,15 +940,15 @@ namespace AZ::Dom } else if (before.IsObject()) { - CompareObjects(path, before, after); + compareObjects(path, before, after); } else if (before.IsArray()) { - CompareArrays(path, before, after); + compareArrays(path, before, after); } else if (before.IsNode()) { - CompareNodes(path, before, after); + compareNodes(path, before, after); } else { @@ -951,7 +960,7 @@ namespace AZ::Dom while (!entriesToCompare.empty()) { PendingComparison& comparison = entriesToCompare.front(); - CompareValues(comparison.m_path, comparison.m_before, comparison.m_after); + compareValues(comparison.m_path, comparison.m_before, comparison.m_after); entriesToCompare.pop(); } return patches; diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h index 9d9ad7560a..633a611c73 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h @@ -35,9 +35,9 @@ namespace AZ::Dom PatchOperation(const PatchOperation&) = default; PatchOperation(PatchOperation&&) = default; - explicit PatchOperation(Path destinationPath, Type type, Value value); - explicit PatchOperation(Path destionationPath, Type type, Path sourcePath); - explicit PatchOperation(Path path, Type type); + PatchOperation(Path destionationPath, Type type, Value value); + PatchOperation(Path destionationPath, Type type, Path sourcePath); + PatchOperation(Path path, Type type); static PatchOperation AddOperation(Path destinationPath, Value value); static PatchOperation RemoveOperation(Path pathToRemove); @@ -72,6 +72,13 @@ namespace AZ::Dom AZ::Outcome GetInverse(Value stateBeforeApplication) const; + enum class ExistenceCheckFlags : AZ::u8 + { + DefaultExistenceCheck = 0x0, + VerifyFullPath = 0x1, + AllowEndOfArray = 0x2, + }; + private: struct PathContext { @@ -79,12 +86,8 @@ namespace AZ::Dom PathEntry m_key; }; - static constexpr AZ::u8 DefaultExistenceCheck = 0x0; - static constexpr AZ::u8 VerifyFullPath = 0x1; - static constexpr AZ::u8 AllowEndOfArray = 0x2; - static AZ::Outcome LookupPath( - Value& rootElement, const Path& path, AZ::u8 existenceCheckFlags = DefaultExistenceCheck); + Value& rootElement, const Path& path, ExistenceCheckFlags existenceCheckFlags = ExistenceCheckFlags::DefaultExistenceCheck); PatchOutcome ApplyAdd(Value& rootElement) const; PatchOutcome ApplyRemove(Value& rootElement) const; @@ -93,11 +96,13 @@ namespace AZ::Dom PatchOutcome ApplyMove(Value& rootElement) const; PatchOutcome ApplyTest(Value& rootElement) const; + AZStd::variant m_value; Path m_domPath; Type m_type; - AZStd::variant m_value; }; + AZ_DEFINE_ENUM_BITWISE_OPERATORS(PatchOperation::ExistenceCheckFlags); + class Patch; //! The current state of a Patch application operation. @@ -184,8 +189,18 @@ namespace AZ::Dom Patch m_inversePatches; }; + //! Parameters for GenerateHierarchicalDeltaPatch. + struct DeltaPatchGenerationParameters + { + static constexpr size_t NoReplace = AZStd::numeric_limits::max(); + + //! The threshold of changed values in a node or array which, if exceeded, will cause the generation to create an + //! entire "replace" oepration instead. If set to NoReplace, no replacement will occur. + size_t m_replaceThreshold = 3; + }; + //! Generates a set of patches such that m_forwardPatches.Apply(beforeState) shall produce a document equivalent to afterState, and //! a subsequent m_inversePatches.Apply(beforeState) shall produce the original document. This patch generation strategy does a //! hierarchical comparison and is not guaranteed to create the minimal set of patches required to transform between the two states. - PatchInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState); + PatchInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState, const DeltaPatchGenerationParameters& params = {}); } // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp index a5d130d48c..f98dad4adb 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp @@ -54,19 +54,19 @@ namespace AZ::Dom bool PathEntry::operator==(size_t value) const { const size_t* internalValue = AZStd::get_if(&m_value); - return internalValue == nullptr ? false : (*internalValue) == value; + return internalValue != nullptr && *internalValue == value; } bool PathEntry::operator==(const AZ::Name& key) const { const AZ::Name* internalValue = AZStd::get_if(&m_value); - return internalValue == nullptr ? false : (*internalValue) == key; + return internalValue != nullptr && *internalValue == key; } bool PathEntry::operator==(AZStd::string_view key) const { const AZ::Name* internalValue = AZStd::get_if(&m_value); - return internalValue == nullptr ? false : (*internalValue) == AZ::Name(key); + return internalValue != nullptr && *internalValue == AZ::Name(key); } bool PathEntry::operator!=(const PathEntry& other) const @@ -323,13 +323,13 @@ namespace AZ::Dom return size; } - void Path::FormatString(char* stringBuffer, size_t bufferSize) const + size_t Path::FormatString(char* stringBuffer, size_t bufferSize) const { size_t bufferIndex = 0; auto putChar = [&](char c) { - if (bufferIndex == bufferSize) + if (bufferIndex >= bufferSize) { return; } @@ -360,6 +360,11 @@ namespace AZ::Dom for (const PathEntry& entry : m_entries) { + if (bufferIndex >= bufferSize) + { + return bufferIndex; + } + putChar(PathSeparator); if (entry.IsEndOfArray()) { @@ -375,7 +380,10 @@ namespace AZ::Dom } } + size_t bytesWritten = bufferIndex; putChar('\0'); + + return bytesWritten; } AZStd::string Path::ToString() const diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.h b/Code/Framework/AzCore/AzCore/DOM/DomPath.h index 39f7c7da98..f1a95b4e70 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.h @@ -128,10 +128,19 @@ namespace AZ::Dom size_t GetStringLength() const; //! Formats a JSON-pointer style path string into the target buffer. //! This operation will fail if bufferSize < GetStringLength() + 1 - void FormatString(char* stringBuffer, size_t bufferSize) const; + //! \return The number of bytes written, excepting the null terminator. + size_t FormatString(char* stringBuffer, size_t bufferSize) const; //! Returns a JSON-pointer style path string for this path. AZStd::string ToString() const; + void AppendToString(AZStd::string& output) const; + template + void AppendToString(T& output) const + { + const size_t startIndex = output.length(); + output.resize_no_construct(startIndex + FormatString(output.data() + startIndex, output.capacity() - startIndex)); + } + //! Reads a JSON-pointer style path from pathString and replaces this path's contents. //! Paths are accepted in the following forms: //! "/path/to/foo/0" diff --git a/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp index 624b24cd20..c3c8139f6c 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp @@ -96,4 +96,25 @@ namespace AZ::Dom::Benchmark state.SetItemsProcessed(3 * state.iterations()); } BENCHMARK_REGISTER_F(DomPathBenchmark, DomPathEntry_IsEndOfArray); + + BENCHMARK_DEFINE_F(DomPathBenchmark, DomPathEntry_Comparison)(benchmark::State& state) + { + PathEntry name("name"); + PathEntry index(0); + PathEntry endOfArray; + endOfArray.SetEndOfArray(); + + for (auto _ : state) + { + name == name; + name == index; + name == endOfArray; + index == index; + index == endOfArray; + endOfArray == endOfArray; + } + + state.SetItemsProcessed(6 * state.iterations()); + } + BENCHMARK_REGISTER_F(DomPathBenchmark, DomPathEntry_Comparison); } diff --git a/Code/Framework/AzCore/Tests/DOM/DomPathTests.cpp b/Code/Framework/AzCore/Tests/DOM/DomPathTests.cpp index 54bff9f29b..5f3aa946b2 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomPathTests.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomPathTests.cpp @@ -174,4 +174,23 @@ namespace AZ::Dom::Tests p.AppendToString(s); EXPECT_EQ(s, "/foo/0/foo/0"); } + + TEST_F(DomPathTests, MixedPath_AppendToFixedString) + { + Path p("/foo/0"); + + { + AZStd::fixed_string<7> s; + p.AppendToString(s); + EXPECT_EQ(s, "/foo/0"); + } + + { + AZStd::fixed_string<9> s; + p.AppendToString(s); + EXPECT_EQ(s, "/foo/0"); + p.AppendToString(s); + EXPECT_EQ(s, "/foo/0/fo"); + } + } } // namespace AZ::Dom::Tests From 155cc5845b656b1a9c31b100ac6e5379e24e74eb Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Thu, 27 Jan 2022 13:44:35 -0800 Subject: [PATCH 3/6] Add Path::IsEmpty Signed-off-by: Nicholas Van Sickle --- Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp | 2 +- Code/Framework/AzCore/AzCore/DOM/DomPath.cpp | 5 +++++ Code/Framework/AzCore/AzCore/DOM/DomPath.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp index 6e53de0689..44c16baa16 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp @@ -395,7 +395,7 @@ namespace AZ::Dom const bool allowEndOfArray = (flags & ExistenceCheckFlags::AllowEndOfArray) != ExistenceCheckFlags::DefaultExistenceCheck; Path target = path; - if (target.Size() == 0) + if (target.IsEmpty()) { Value wrapper(Dom::Type::Array); wrapper.ArrayPushBack(rootElement); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp index f98dad4adb..5f6438518f 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp @@ -246,6 +246,11 @@ namespace AZ::Dom return m_entries.size(); } + bool Path::IsEmpty() const + { + return m_entries.empty(); + } + PathEntry& Path::operator[](size_t index) { return m_entries[index]; diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.h b/Code/Framework/AzCore/AzCore/DOM/DomPath.h index f1a95b4e70..7a031a2c68 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.h @@ -111,6 +111,7 @@ namespace AZ::Dom void Clear(); PathEntry At(size_t index) const; size_t Size() const; + bool IsEmpty() const; PathEntry& operator[](size_t index); const PathEntry& operator[](size_t index) const; From 5b8176e99bba504a8c11488deac0f06f7a5b67b1 Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Mon, 7 Feb 2022 16:01:29 -0800 Subject: [PATCH 4/6] Address some review feedback Signed-off-by: Nicholas Van Sickle --- Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp | 18 +++++++++--------- Code/Framework/AzCore/AzCore/DOM/DomPatch.h | 8 +++++--- .../AzCore/Tests/DOM/DomPathBenchmarks.cpp | 12 ++++++------ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp index 44c16baa16..f89cb313e7 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp @@ -14,21 +14,21 @@ namespace AZ::Dom { PatchOperation::PatchOperation(Path destinationPath, Type type, Value value) - : m_domPath(destinationPath) + : m_domPath(AZStd::move(destinationPath)) , m_type(type) - , m_value(value) + , m_value(AZStd::move(value)) { } PatchOperation::PatchOperation(Path destinationPath, Type type, Path sourcePath) - : m_domPath(destinationPath) + : m_domPath(AZStd::move(destinationPath)) , m_type(type) - , m_value(sourcePath) + , m_value(AZStd::move(sourcePath)) { } PatchOperation::PatchOperation(Path destinationPath, Type type) - : m_domPath(destinationPath) + : m_domPath(AZStd::move(destinationPath)) , m_type(type) { } @@ -690,22 +690,22 @@ namespace AZ::Dom auto Patch::begin() const -> OperationsContainer::const_iterator { - return m_operations.cbegin(); + return m_operations.begin(); } auto Patch::end() const -> OperationsContainer::const_iterator { - return m_operations.cend(); + return m_operations.end(); } auto Patch::cbegin() const -> OperationsContainer::const_iterator { - return m_operations.cbegin(); + return m_operations.begin(); } auto Patch::cend() const -> OperationsContainer::const_iterator { - return m_operations.cend(); + return m_operations.end(); } size_t Patch::size() const diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h index 633a611c73..40ef725b57 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h @@ -10,6 +10,7 @@ #include #include +#include namespace AZ::Dom { @@ -108,12 +109,12 @@ namespace AZ::Dom //! The current state of a Patch application operation. struct PatchApplicationState { + //! The outcome of the last operation, may be overridden to produce a different failure outcome. + PatchOperation::PatchOutcome m_outcome; //! The patch being applied. const Patch* m_patch = nullptr; //! The last operation attempted. const PatchOperation* m_lastOperation = nullptr; - //! The outcome of the last operation, may be overridden to produce a different failure outcome. - PatchOperation::PatchOutcome m_outcome; //! The current state of the value being patched, will be returned if the patch operation succeeds. Value* m_currentState = nullptr; //! If set to false, the patch operation should halt. @@ -134,7 +135,7 @@ namespace AZ::Dom { public: using StrategyFunctor = AZStd::function; - using OperationsContainer = AZStd::vector; + using OperationsContainer = AZStd::deque; Patch() = default; Patch(const Patch&) = default; @@ -193,6 +194,7 @@ namespace AZ::Dom struct DeltaPatchGenerationParameters { static constexpr size_t NoReplace = AZStd::numeric_limits::max(); + static constexpr size_t AlwaysFullReplace = 0; //! The threshold of changed values in a node or array which, if exceeded, will cause the generation to create an //! entire "replace" oepration instead. If set to NoReplace, no replacement will occur. diff --git a/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp index c3c8139f6c..4741900aa1 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomPathBenchmarks.cpp @@ -106,12 +106,12 @@ namespace AZ::Dom::Benchmark for (auto _ : state) { - name == name; - name == index; - name == endOfArray; - index == index; - index == endOfArray; - endOfArray == endOfArray; + benchmark::DoNotOptimize(name == name); + benchmark::DoNotOptimize(name == index); + benchmark::DoNotOptimize(name == endOfArray); + benchmark::DoNotOptimize(index == index); + benchmark::DoNotOptimize(index == endOfArray); + benchmark::DoNotOptimize(endOfArray == endOfArray); } state.SetItemsProcessed(6 * state.iterations()); From d1bb5a0543268ec73223aa24eea093f864a830f4 Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Tue, 8 Feb 2022 13:44:08 -0800 Subject: [PATCH 5/6] Move DOM delta comparison to its own file, enhance inverting moves Signed-off-by: Nicholas Van Sickle --- .../AzCore/AzCore/DOM/DomComparison.cpp | 178 ++++++++++++++ .../AzCore/AzCore/DOM/DomComparison.h | 37 +++ Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp | 219 ++---------------- Code/Framework/AzCore/AzCore/DOM/DomPatch.h | 26 +-- .../AzCore/AzCore/azcore_files.cmake | 2 + .../AzCore/Tests/DOM/DomPatchTests.cpp | 5 +- 6 files changed, 247 insertions(+), 220 deletions(-) create mode 100644 Code/Framework/AzCore/AzCore/DOM/DomComparison.cpp create mode 100644 Code/Framework/AzCore/AzCore/DOM/DomComparison.h diff --git a/Code/Framework/AzCore/AzCore/DOM/DomComparison.cpp b/Code/Framework/AzCore/AzCore/DOM/DomComparison.cpp new file mode 100644 index 0000000000..dc21761670 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/DOM/DomComparison.cpp @@ -0,0 +1,178 @@ +/* + * 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::Dom +{ + PatchUndoRedoInfo GenerateHierarchicalDeltaPatch( + const Value& beforeState, const Value& afterState, const DeltaPatchGenerationParameters& params) + { + PatchUndoRedoInfo patches; + + auto AddPatch = [&patches](PatchOperation op, PatchOperation inverse) + { + patches.m_forwardPatches.PushBack(AZStd::move(op)); + patches.m_inversePatches.PushFront(AZStd::move(inverse)); + }; + + AZStd::function compareValues; + + struct PendingComparison + { + Path m_path; + const Value& m_before; + const Value& m_after; + + PendingComparison(Path path, const Value& before, const Value& after) + : m_path(AZStd::move(path)) + , m_before(before) + , m_after(after) + { + } + }; + AZStd::queue entriesToCompare; + + AZStd::unordered_set desiredKeys; + auto compareObjects = [&](const Path& path, const Value& before, const Value& after) + { + desiredKeys.clear(); + Path subPath = path; + for (auto it = after.MemberBegin(); it != after.MemberEnd(); ++it) + { + desiredKeys.insert(it->first.GetHash()); + subPath.Push(it->first); + auto beforeIt = before.FindMember(it->first); + if (beforeIt == before.MemberEnd()) + { + AddPatch(PatchOperation::AddOperation(subPath, it->second), PatchOperation::RemoveOperation(subPath)); + } + else + { + entriesToCompare.emplace(subPath, beforeIt->second, it->second); + } + subPath.Pop(); + } + + for (auto it = before.MemberBegin(); it != before.MemberEnd(); ++it) + { + if (!desiredKeys.contains(it->first.GetHash())) + { + subPath.Push(it->first); + AddPatch(PatchOperation::RemoveOperation(subPath), PatchOperation::AddOperation(subPath, it->second)); + subPath.Pop(); + } + } + }; + + auto compareArrays = [&](const Path& path, const Value& before, const Value& after) + { + const size_t beforeSize = before.ArraySize(); + const size_t afterSize = after.ArraySize(); + + // If more than replaceThreshold values differ, do a replace operation instead + if (params.m_replaceThreshold != DeltaPatchGenerationParameters::NoReplace) + { + size_t changedValueCount = 0; + const size_t entriesToEnumerate = AZStd::min(beforeSize, afterSize); + for (size_t i = 0; i < entriesToEnumerate; ++i) + { + if (before[i] != after[i]) + { + ++changedValueCount; + if (changedValueCount >= params.m_replaceThreshold) + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + return; + } + } + } + } + + Path subPath = path; + for (size_t i = 0; i < afterSize; ++i) + { + if (i >= beforeSize) + { + subPath.Push(PathEntry(PathEntry::EndOfArrayIndex)); + AddPatch(PatchOperation::AddOperation(subPath, after[i]), PatchOperation::RemoveOperation(subPath)); + subPath.Pop(); + } + else + { + subPath.Push(PathEntry(i)); + entriesToCompare.emplace(subPath, before[i], after[i]); + subPath.Pop(); + } + } + + if (beforeSize > afterSize) + { + subPath.Push(PathEntry(PathEntry::EndOfArrayIndex)); + for (size_t i = beforeSize; i > afterSize; --i) + { + AddPatch(PatchOperation::RemoveOperation(subPath), PatchOperation::AddOperation(subPath, before[i - 1])); + } + } + }; + + auto compareNodes = [&](const Path& path, const Value& before, const Value& after) + { + if (before.GetNodeName() != after.GetNodeName()) + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + } + else + { + compareObjects(path, before, after); + compareArrays(path, before, after); + } + }; + + compareValues = [&](const Path& path, const Value& before, const Value& after) + { + if (before.GetType() != after.GetType()) + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + } + else if (before == after) + { + // If a shallow comparison succeeds we're pointing to an identical value or container + // and don't need to drill down. + return; + } + else if (before.IsObject()) + { + compareObjects(path, before, after); + } + else if (before.IsArray()) + { + compareArrays(path, before, after); + } + else if (before.IsNode()) + { + compareNodes(path, before, after); + } + else + { + AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); + } + }; + + entriesToCompare.emplace(Path(), beforeState, afterState); + while (!entriesToCompare.empty()) + { + PendingComparison& comparison = entriesToCompare.front(); + compareValues(comparison.m_path, comparison.m_before, comparison.m_after); + entriesToCompare.pop(); + } + return patches; + } +} diff --git a/Code/Framework/AzCore/AzCore/DOM/DomComparison.h b/Code/Framework/AzCore/AzCore/DOM/DomComparison.h new file mode 100644 index 0000000000..f882887cba --- /dev/null +++ b/Code/Framework/AzCore/AzCore/DOM/DomComparison.h @@ -0,0 +1,37 @@ +/* + * 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 + +namespace AZ::Dom +{ + //! A set of patches for applying a change and doing the inverse operation. + struct PatchUndoRedoInfo + { + Patch m_forwardPatches; + Patch m_inversePatches; + }; + + //! Parameters for GenerateHierarchicalDeltaPatch. + struct DeltaPatchGenerationParameters + { + static constexpr size_t NoReplace = AZStd::numeric_limits::max(); + static constexpr size_t AlwaysFullReplace = 0; + + //! The threshold of changed values in a node or array which, if exceeded, will cause the generation to create an + //! entire "replace" oepration instead. If set to NoReplace, no replacement will occur. + size_t m_replaceThreshold = 3; + }; + + //! Generates a set of patches such that m_forwardPatches.Apply(beforeState) shall produce a document equivalent to afterState, and + //! a subsequent m_inversePatches.Apply(beforeState) shall produce the original document. This patch generation strategy does a + //! hierarchical comparison and is not guaranteed to create the minimal set of patches required to transform between the two states. + PatchUndoRedoInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState, const DeltaPatchGenerationParameters& params = {}); +} // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp index f89cb313e7..dc0bff4e7c 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.cpp @@ -8,8 +8,6 @@ #include #include -#include -#include namespace AZ::Dom { @@ -297,7 +295,7 @@ namespace AZ::Dom } } - AZ::Outcome PatchOperation::GetInverse(Value stateBeforeApplication) const + AZ::Outcome, AZStd::string> PatchOperation::GetInverse(Value stateBeforeApplication) const { switch (m_type) { @@ -310,10 +308,10 @@ namespace AZ::Dom const Value* existingValue = stateBeforeApplication.FindChild(m_domPath); if (existingValue != nullptr) { - return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); + return AZ::Success({PatchOperation::ReplaceOperation(m_domPath, *existingValue)}); } } - return AZ::Success(PatchOperation::RemoveOperation(m_domPath)); + return AZ::Success({PatchOperation::RemoveOperation(m_domPath)}); } case Type::Remove: { @@ -325,7 +323,7 @@ namespace AZ::Dom m_domPath.AppendToString(errorMessage); return AZ::Failure(AZStd::move(errorMessage)); } - return AZ::Success(PatchOperation::AddOperation(m_domPath, *existingValue)); + return AZ::Success({PatchOperation::AddOperation(m_domPath, *existingValue)}); } case Type::Replace: { @@ -337,7 +335,7 @@ namespace AZ::Dom m_domPath.AppendToString(errorMessage); return AZ::Failure(AZStd::move(errorMessage)); } - return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); + return AZ::Success({PatchOperation::ReplaceOperation(m_domPath, *existingValue)}); } case Type::Copy: { @@ -349,40 +347,37 @@ namespace AZ::Dom m_domPath.AppendToString(errorMessage); return AZ::Failure(AZStd::move(errorMessage)); } - return AZ::Success(PatchOperation::ReplaceOperation(m_domPath, *existingValue)); + return AZ::Success({PatchOperation::ReplaceOperation(m_domPath, *existingValue)}); } case Type::Move: { - // Move -> Replace, using the common ancestor of the two paths as the replacement - // This is not a minimal inverse, which would be two replace operations at each path - const Path& destPath = m_domPath; - const Path& sourcePath = GetSourcePath(); - - Path commonAncestor; - for (size_t i = 0; i < destPath.Size() && i < sourcePath.Size(); ++i) + const Value* sourceValue = stateBeforeApplication.FindChild(GetSourcePath()); + if (sourceValue == nullptr) { - if (destPath[i] != sourcePath[i]) - { - break; - } - - commonAncestor.Push(destPath[i]); - } - - const Value* existingValue = stateBeforeApplication.FindChild(commonAncestor); - if (existingValue == nullptr) - { - AZStd::string errorMessage = "Unable to invert DOM copy patch, common ancestor path not found: "; - commonAncestor.AppendToString(errorMessage); + AZStd::string errorMessage = "Unable to invert DOM copy patch, source path not found: "; + m_domPath.AppendToString(errorMessage); return AZ::Failure(AZStd::move(errorMessage)); } - return AZ::Success(PatchOperation::ReplaceOperation(commonAncestor, *existingValue)); + + // If there was a value at the destination path, invert with an add / replace + const Value* destinationValue = stateBeforeApplication.FindChild(GetDestinationPath()); + if (destinationValue != nullptr) + { + InversePatches result({PatchOperation::AddOperation(GetSourcePath(), *sourceValue)}); + result.push_back(PatchOperation::ReplaceOperation(GetDestinationPath(), *destinationValue)); + return AZ::Success({ + PatchOperation::AddOperation(GetSourcePath(), *sourceValue), + PatchOperation::ReplaceOperation(GetDestinationPath(), *destinationValue), + }); + } + // Otherwise, just do a move + return AZ::Success({PatchOperation::MoveOperation(GetDestinationPath(), GetSourcePath())}); } case Type::Test: { // Test -> Test (no change) // When inverting a sequence of patches, applying them in reverse order should allow the test to continue to succeed - return AZ::Success(*this); + return AZ::Success({*this}); } } return AZ::Failure("Unable to invert DOM patch, unknown type specified"); @@ -801,168 +796,4 @@ namespace AZ::Dom { return PatchOperation(AZStd::move(testPath), PatchOperation::Type::Test, AZStd::move(value)); } - - PatchInfo GenerateHierarchicalDeltaPatch( - const Value& beforeState, const Value& afterState, const DeltaPatchGenerationParameters& params) - { - PatchInfo patches; - - auto AddPatch = [&patches](PatchOperation op, PatchOperation inverse) - { - patches.m_forwardPatches.PushBack(AZStd::move(op)); - patches.m_inversePatches.PushFront(AZStd::move(inverse)); - }; - - AZStd::function compareValues; - - struct PendingComparison - { - Path m_path; - const Value& m_before; - const Value& m_after; - - PendingComparison(Path path, const Value& before, const Value& after) - : m_path(AZStd::move(path)) - , m_before(before) - , m_after(after) - { - } - }; - AZStd::queue entriesToCompare; - - AZStd::unordered_set desiredKeys; - auto compareObjects = [&](const Path& path, const Value& before, const Value& after) - { - desiredKeys.clear(); - Path subPath = path; - for (auto it = after.MemberBegin(); it != after.MemberEnd(); ++it) - { - desiredKeys.insert(it->first.GetHash()); - subPath.Push(it->first); - auto beforeIt = before.FindMember(it->first); - if (beforeIt == before.MemberEnd()) - { - AddPatch(PatchOperation::AddOperation(subPath, it->second), PatchOperation::RemoveOperation(subPath)); - } - else - { - entriesToCompare.emplace(subPath, beforeIt->second, it->second); - } - subPath.Pop(); - } - - for (auto it = before.MemberBegin(); it != before.MemberEnd(); ++it) - { - if (!desiredKeys.contains(it->first.GetHash())) - { - subPath.Push(it->first); - AddPatch(PatchOperation::RemoveOperation(subPath), PatchOperation::AddOperation(subPath, it->second)); - subPath.Pop(); - } - } - }; - - auto compareArrays = [&](const Path& path, const Value& before, const Value& after) - { - const size_t beforeSize = before.ArraySize(); - const size_t afterSize = after.ArraySize(); - - // If more than replaceThreshold values differ, do a replace operation instead - if (params.m_replaceThreshold != DeltaPatchGenerationParameters::NoReplace) - { - size_t changedValueCount = 0; - const size_t entriesToEnumerate = AZStd::min(beforeSize, afterSize); - for (size_t i = 0; i < entriesToEnumerate; ++i) - { - if (before[i] != after[i]) - { - ++changedValueCount; - if (changedValueCount >= params.m_replaceThreshold) - { - AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); - return; - } - } - } - } - - Path subPath = path; - for (size_t i = 0; i < afterSize; ++i) - { - if (i >= beforeSize) - { - subPath.Push(PathEntry(PathEntry::EndOfArrayIndex)); - AddPatch(PatchOperation::AddOperation(subPath, after[i]), PatchOperation::RemoveOperation(subPath)); - subPath.Pop(); - } - else - { - subPath.Push(PathEntry(i)); - entriesToCompare.emplace(subPath, before[i], after[i]); - subPath.Pop(); - } - } - - if (beforeSize > afterSize) - { - subPath.Push(PathEntry(PathEntry::EndOfArrayIndex)); - for (size_t i = beforeSize; i > afterSize; --i) - { - AddPatch(PatchOperation::RemoveOperation(subPath), PatchOperation::AddOperation(subPath, before[i - 1])); - } - } - }; - - auto compareNodes = [&](const Path& path, const Value& before, const Value& after) - { - if (before.GetNodeName() != after.GetNodeName()) - { - AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); - } - else - { - compareObjects(path, before, after); - compareArrays(path, before, after); - } - }; - - compareValues = [&](const Path& path, const Value& before, const Value& after) - { - if (before.GetType() != after.GetType()) - { - AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); - } - else if (before == after) - { - // If a shallow comparison succeeds we're pointing to an identical value or container - // and don't need to drill down. - return; - } - else if (before.IsObject()) - { - compareObjects(path, before, after); - } - else if (before.IsArray()) - { - compareArrays(path, before, after); - } - else if (before.IsNode()) - { - compareNodes(path, before, after); - } - else - { - AddPatch(PatchOperation::ReplaceOperation(path, after), PatchOperation::ReplaceOperation(path, before)); - } - }; - - entriesToCompare.emplace(Path(), beforeState, afterState); - while (!entriesToCompare.empty()) - { - PendingComparison& comparison = entriesToCompare.front(); - compareValues(comparison.m_path, comparison.m_before, comparison.m_after); - entriesToCompare.pop(); - } - return patches; - } } // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h index 40ef725b57..2d591a8079 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPatch.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPatch.h @@ -71,7 +71,8 @@ namespace AZ::Dom Value GetDomRepresentation() const; static AZ::Outcome CreateFromDomRepresentation(Value domValue); - AZ::Outcome GetInverse(Value stateBeforeApplication) const; + using InversePatches = AZStd::fixed_vector; + AZ::Outcome, AZStd::string> GetInverse(Value stateBeforeApplication) const; enum class ExistenceCheckFlags : AZ::u8 { @@ -182,27 +183,4 @@ namespace AZ::Dom private: OperationsContainer m_operations; }; - - //! A set of patches for applying a change and doing the inverse operation (i.e. undoing it). - struct PatchInfo - { - Patch m_forwardPatches; - Patch m_inversePatches; - }; - - //! Parameters for GenerateHierarchicalDeltaPatch. - struct DeltaPatchGenerationParameters - { - static constexpr size_t NoReplace = AZStd::numeric_limits::max(); - static constexpr size_t AlwaysFullReplace = 0; - - //! The threshold of changed values in a node or array which, if exceeded, will cause the generation to create an - //! entire "replace" oepration instead. If set to NoReplace, no replacement will occur. - size_t m_replaceThreshold = 3; - }; - - //! Generates a set of patches such that m_forwardPatches.Apply(beforeState) shall produce a document equivalent to afterState, and - //! a subsequent m_inversePatches.Apply(beforeState) shall produce the original document. This patch generation strategy does a - //! hierarchical comparison and is not guaranteed to create the minimal set of patches required to transform between the two states. - PatchInfo GenerateHierarchicalDeltaPatch(const Value& beforeState, const Value& afterState, const DeltaPatchGenerationParameters& params = {}); } // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/azcore_files.cmake b/Code/Framework/AzCore/AzCore/azcore_files.cmake index ad7d7cbdef..b7821f1d65 100644 --- a/Code/Framework/AzCore/AzCore/azcore_files.cmake +++ b/Code/Framework/AzCore/AzCore/azcore_files.cmake @@ -128,6 +128,8 @@ set(FILES DOM/DomValueWriter.h DOM/DomVisitor.cpp DOM/DomVisitor.h + DOM/DomComparison.cpp + DOM/DomComparison.h DOM/Backends/JSON/JsonBackend.h DOM/Backends/JSON/JsonSerializationUtils.cpp DOM/Backends/JSON/JsonSerializationUtils.h diff --git a/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp b/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp index a0e4f349a5..b60c09330f 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomPatchTests.cpp @@ -7,6 +7,7 @@ */ #include +#include #include namespace AZ::Dom::Tests @@ -45,9 +46,9 @@ namespace AZ::Dom::Tests DomTestFixture::TearDown(); } - PatchInfo GenerateAndVerifyDelta() + PatchUndoRedoInfo GenerateAndVerifyDelta() { - PatchInfo info = GenerateHierarchicalDeltaPatch(m_dataset, m_deltaDataset); + PatchUndoRedoInfo info = GenerateHierarchicalDeltaPatch(m_dataset, m_deltaDataset); auto result = info.m_forwardPatches.Apply(m_dataset); EXPECT_TRUE(result.IsSuccess()); From 42c2243eaac0ac8174ffef52808b2f5f08d49352 Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Tue, 8 Feb 2022 14:34:04 -0800 Subject: [PATCH 6/6] Fix benchmark non-unity build Signed-off-by: Nicholas Van Sickle --- Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp index de19ee1783..e44ba3ce67 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomPatchBenchmarks.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include