Fix setting properties with group assignment

[mstilkerich]

Setting properties that are part of a group is broken since version 4.0.0. The Component::remove() method for the specific case of a property given by its name (as string) that includes a group prefix will throw an exception. However, I believe the exception is only intended for the usage of this function where the property to remove is given as an object reference, not as a string, where the given property/component is not a child of component the remove() method is called on.

Example code to trigger the exception:

<?php

use Sabre\VObject;
use Sabre\VObject\Component\VCard;
include_once dirname(__FILE__) . "/vendor/autoload.php";

$vcard = new VObject\Component\VCard([
        'VERSION' => '3.0',
        'item2.X-ABLabel' => "Foo", // triggers exception
]);

$vcard->{"item1.X-ABLabel"} = "SpecialLabel"; // also triggers exception

print $vcard->serialize();

The exception will be triggered for both the usage in the constructor as well as by a separate set operation.

[codecov]

Codecov Report

Merging #523 (31c9187) into master (38bc2d2) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #523      +/-   ##
============================================
+ Coverage     98.69%   98.73%   +0.04%     
  Complexity     1759     1759              
============================================
  Files            66       66              
  Lines          4279     4279              
============================================
+ Hits           4223     4225       +2     
+ Misses           56       54       -2     

Impacted Files	Coverage Δ	Complexity Δ
lib/Component.php	99.55% <100.00%> (+0.88%)	92.00 <0.00> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38bc2d2...31c9187. Read the comment docs.

[DeepDiver1975]

Thanks a lot for your contribution!

Can I ask you to add a unit test for this case? 👍

[phil-davis]

Code looks reasonable. Just needs unit test(s) so this does not get broken again in future.

[mstilkerich]

Hello, I added a test case.

[mstilkerich]

Hello, I added the requested test. Is there still a problem?

[phil-davis]

With the new test case on master

There was 1 error:

1) Sabre\VObject\ComponentTest::testAddGroupProperties
InvalidArgumentException: The item you passed to remove() was not a child of this component

/home/phil/git/sabre-io/vobject/lib/Component.php:165
/home/phil/git/sabre-io/vobject/lib/Component.php:483
/home/phil/git/sabre-io/vobject/tests/VObject/ComponentTest.php:82

ERRORS!
Tests: 1721, Assertions: 2639, Errors: 1.
Script phpunit --configuration tests/phpunit.xml handling the phpunit event returned with error code 2

That's good, the test fails. And passes with the fix.

test has been added