Fixed misc slice conversion bugs

* Fixed crash caused by nesting the same slice twice
If the same slice is nested at multiple levels in the same slice hierarchy, the second conversion would reregister the prefab and crash.  Now that case is detected and the slice isn't reconverted.

* Fixed json array patches where multiple elements are removed.
The patches now generate removals from back to front, instead of front to back, so that the indices remain valid as each patch is applied.
main
Mike Balfour 5 years ago committed by GitHub
parent 13c59385a7
commit ab3aa904f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -607,10 +607,12 @@ namespace AZ
}
else if (source.Size() > target.Size())
{
rapidjson::SizeType sourceCount = source.Size();
for (rapidjson::SizeType i = count; i < sourceCount; ++i)
// Loop backwards through the removals so that each removal has a valid index when processing in order.
for (rapidjson::SizeType i = source.Size(); i > count; --i)
{
ScopedStackedString entryName(element, i);
// (We use "i - 1" here instead of in the loop to ensure we don't wrap around our unsigned numbers in the case
// where count is 0.)
ScopedStackedString entryName(element, i - 1);
patch.PushBack(CreatePatchInternal_Remove(allocator, element), allocator);
resultCode.Combine(settings.m_reporting("Removed member from array in JSON Patch.",
ResultCode(Tasks::CreatePatch, Outcomes::Success), element));

@ -303,6 +303,29 @@ namespace JsonSerializationTests
R"( { "foo": [ "bar", "baz" ] })");
}
TEST_F(JsonPatchingSerializationTests, ApplyPatch_UseJsonPatchRemoveArrayMembersInCorrectOrder_ReportsSuccess)
{
CheckApplyPatch(
R"( { "foo": [ "bar", "qux", "baz" ] })",
R"( [
{ "op": "remove", "path": "/foo/2" },
{ "op": "remove", "path": "/foo/1" }
])",
R"( { "foo": [ "bar" ] })");
}
TEST_F(JsonPatchingSerializationTests, ApplyPatch_UseJsonPatchRemoveArrayMembersInWrongOrder_ReportsError)
{
using namespace AZ::JsonSerializationResult;
CheckApplyPatchOutcome(
R"( { "foo": [ "bar", "qux", "baz" ] })",
R"( [
{ "op": "remove", "path": "/foo/1" },
{ "op": "remove", "path": "/foo/2" }
])",
Outcomes::Invalid, Processing::Halted);
}
TEST_F(JsonPatchingSerializationTests, ApplyPatch_UseJsonPatchRemoveOperationInvalidParent_ReportError)
{
using namespace AZ::JsonSerializationResult;
@ -949,6 +972,27 @@ namespace JsonSerializationTests
);
}
TEST_F(JsonPatchingSerializationTests, CreatePatch_UseJsonPatchRemoveLastArrayEntries_MultipleOperationsInCorrectOrder)
{
CheckCreatePatch(
R"( [ "foo", "hello", "bar" ])", R"( [ "foo" ])",
R"( [
{ "op": "remove", "path": "/2" },
{ "op": "remove", "path": "/1" }
])");
}
TEST_F(JsonPatchingSerializationTests, CreatePatch_UseJsonPatchRemoveAllArrayEntries_MultipleOperationsInCorrectOrder)
{
CheckCreatePatch(
R"( [ "foo", "hello", "bar" ])", R"( [])",
R"( [
{ "op": "remove", "path": "/2" },
{ "op": "remove", "path": "/1" },
{ "op": "remove", "path": "/0" }
])");
}
TEST_F(JsonPatchingSerializationTests, CreatePatch_UseJsonPatchRemoveObjectFromArrayInMiddle_OperationToUpdateMember)
{
CheckCreatePatch(

@ -385,24 +385,36 @@ namespace AZ
return false;
}
// Now, convert the nested slice to a prefab.
bool nestedSliceResult = ConvertSliceFile(serializeContext, assetPath, isDryRun);
if (!nestedSliceResult)
{
AZ_Warning("Convert-Slice", nestedSliceResult, " Nested slice '%s' could not be converted.", assetPath.c_str());
return false;
}
// Check to see if we've already converted this slice at a higher level of slice nesting, or if this is our first
// occurrence and we need to convert it now.
// Find the prefab template we created for the newly-created nested prefab.
// To get the template, we need to take our absolute slice path and turn it into a project-relative prefab path.
// First, take our absolute slice path and turn it into a project-relative prefab path.
AZ::IO::Path nestedPrefabPath = assetPath;
nestedPrefabPath.ReplaceExtension("prefab");
auto prefabLoaderInterface = AZ::Interface<AzToolsFramework::Prefab::PrefabLoaderInterface>::Get();
nestedPrefabPath = prefabLoaderInterface->GenerateRelativePath(nestedPrefabPath);
// Now, see if we already have a template ID in memory for it.
AzToolsFramework::Prefab::TemplateId nestedTemplateId =
prefabSystemComponentInterface->GetTemplateIdFromFilePath(nestedPrefabPath);
// If we don't have a template ID yet, convert the nested slice to a prefab and get the template ID.
if (nestedTemplateId == AzToolsFramework::Prefab::InvalidTemplateId)
{
bool nestedSliceResult = ConvertSliceFile(serializeContext, assetPath, isDryRun);
if (!nestedSliceResult)
{
AZ_Warning("Convert-Slice", nestedSliceResult, " Nested slice '%s' could not be converted.", assetPath.c_str());
return false;
}
nestedTemplateId = prefabSystemComponentInterface->GetTemplateIdFromFilePath(nestedPrefabPath);
AZ_Assert(nestedTemplateId != AzToolsFramework::Prefab::InvalidTemplateId,
"Template ID for %s is invalid", nestedPrefabPath.c_str());
}
// Get the nested prefab template.
AzToolsFramework::Prefab::TemplateReference nestedTemplate =
prefabSystemComponentInterface->FindTemplate(nestedTemplateId);

Loading…
Cancel
Save