Skip to content

Commit

Permalink
Guards against completely trashing an existing array
Browse files Browse the repository at this point in the history
  • Loading branch information
@JohnathonKoster
JohnathonKoster committed Feb 27, 2023
1 parent 06edae5 commit aff31c0
Show file tree
Hide file tree
Showing 6 changed files with 590 additions and 1 deletion.
20 changes: 19 additions & 1 deletion src/Analyzers/ConfigAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,24 @@ public function hasNode($key)
return array_key_exists($key, $this->nodeMapping);
}

protected function willClobberTheExistingArray(Array_ $node, ArrayItem $incoming)
{
if ($incoming->key instanceof String_) {
return false;
}

$willWreckArray = true;

foreach ($node->items as $item) {
if (! ($item instanceof ArrayItem && $item->value instanceof Array_)) {
$willWreckArray = false;
break;
}
}

return $willWreckArray;
}

/**
* Attempts to replace a value at a known location with the provided node value.
*
Expand All @@ -278,7 +296,7 @@ public function replaceNodeValue($key, Node $node, $completeReplace = false)
$mergeItemKeyValue = $mergeItem->key->value;
}

if ($mergeItem->value instanceof Array_ && $mergeItemKeyValue === null) {
if ($mergeItem->value instanceof Array_ && $mergeItemKeyValue === null && !$this->willClobberTheExistingArray($currentNode->value, $mergeItem)) {
foreach ($mergeItem->value->items as $subMergeItem) {
$currentNode->value->items[] = $subMergeItem;
}
Expand Down
221 changes: 221 additions & 0 deletions tests/MergeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,225 @@ public function testMergeDoesNotRemoveExistingValues()

$this->assertEquals($expected, $doc);
}

public function testMergeDoesNotFlattenValues()
{
$updater = new ConfigUpdater();
$updater->open(__DIR__.'/configs/issues/018.php');

$updater->update([
'forms' => [
[
'id' => 'new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'new5',
],
[
'id' => 'b-new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'b-new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'b-new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'b-new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'b-new5',
],
],

],true);

$expected = Transformer::normalizeLineEndings(file_get_contents(__DIR__.'/expected/issues/018.php'));
$this->assertSame($expected, $updater->getDocument());

$updater = new ConfigUpdater();
$updater->open(__DIR__.'/expected/issues/018.php');

$updater->update([
'forms' => [
[
'id' => 'c-new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'c-new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'c-new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'c-new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'c-new5',
],
[
'id' => 'd-new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'd-new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'd-new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'd-new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'd-new5',
],
],

],true);

$expected = Transformer::normalizeLineEndings(file_get_contents(__DIR__.'/expected/issues/018a.php'));
$this->assertSame($expected, $updater->getDocument());


$updater = new ConfigUpdater();
$updater->open(__DIR__.'/expected/issues/018a.php');

$updater->update([
'forms' => [
[
'id' => 'c-new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'c-new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'c-new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'c-new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'c-new5',
],
[
'id' => 'd-new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'd-new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'd-new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'd-new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'd-new5',
],
],

],false);


$expected = Transformer::normalizeLineEndings(file_get_contents(__DIR__.'/expected/issues/018b.php'));
$this->assertSame($expected, $updater->getDocument());
}
}
36 changes: 36 additions & 0 deletions tests/configs/issues/018.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

return [
'forms' => [
[
'id' => 'id1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'id2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'id3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'id',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'aud1',
],
],
];
94 changes: 94 additions & 0 deletions tests/expected/issues/018.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<?php

return [
'forms' => [
[
'id' => 'id1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'id2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'id3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'id',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'aud1',
], [
'id' => 'new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'new5',
], [
'id' => 'b-new1',
'form' => 'contact',
'check_consent' => true,
'consent_field' => 'newsletter',
'disable_opt_in' => false,
'marketing_permissions_field_ids' => [
[
],
],
'merge_fields' => [
[
'id' => 'b-new2',
'field_name' => 'first_name',
'tag' => 'FNAME',
],
[
'id' => 'b-new3',
'field_name' => 'last_name',
'tag' => 'LNAME',
],
[
'id' => 'b-new4',
'field_name' => 'company',
'tag' => 'COMPANY',
],
],
'primary_email_field' => 'email',
'audience_id' => 'b-new5',
],
],
];
Loading

0 comments on commit aff31c0

Please sign in to comment.