IBX-8957: Fixed deserializing SiteAccess Matchers for ESI

[vidarl]

🎫 Issue	IBX-8957

Description:

When unserializing SiteaccessMatcher in ESI, custom matchers was only fetch from registry. Serialized configuration of the matcher was not restored.

In case of sitefactoy's SiteAccessMatcher, it meant that mapKey was never restored, causing incorrect links to be generated.
(Map is an ancestor of SiteAccessMatcher)

---

Maintainer update: this issue uncovered a few other underlying issues. We were using traits in production code to configure serializers. Instead I've (@alongosz) changed it to configured ibexa.core.mvc.serializer service.

Moreover the same mirrored code was used to serialize and deserialize data and store them into 3 separate request attributes. While the latter case needs to be tackled separately via IBX-9102, I've made the code uniformly use the same serializer for both serialization and deserialization of data. The SerializerTrait is left only for unit test purposes.

I've introduced ...\Fragment\SiteAccessSerializer service to wrap what SiteAccessSerializationTrait was doing for production code.

The end goal is to have serialization/deserialization handled by configured Symfony serializers entirely. The question now is if we should move parts of the scope resolved here to IBX-9102, or should we keep this and only improve in IBX-9102?

---

For QA:

See ticket for instructions on how to reproduce.

Maintainer update: sanities on SiteAccess matchers, including compound matchers are needed.

Regression run: 🟢 ibexa/commerce#1081
Matchers tests: 🟢 ibexa/experience#500

Documentation:

[konradoboza]

I think we should cover this scenario within tests/lib/MVC/Symfony/EventListener/SiteAccessMatchListenerTest.

[vidarl]

I think we should cover this scenario within tests/lib/MVC/Symfony/EventListener/SiteAccessMatchListenerTest.

This needs to be tested with a matcher which is in the siteAccessMatcherRegistry, but I don't think any of the build-in matchers are there.. ( the fix is inside the if ($this->siteAccessMatcherRegistry->hasMatcher($matcherClass)) { condition )

So, then I would need to make a new test inside SiteFactory based on tests/lib/MVC/Symfony/EventListener/SiteAccessMatchListenerTest ?

@konradoboza

[konradoboza]

@vidarl
there should be another test case within SiteAccessMatchListenerTest mocking that separately, e.g.:

$matcherRegistryMock = $this->createMock(SiteAccessMatcherRegistryInterface::class);
$matcherRegistryMock->method('hasMatcher')->willReturn(true);
$matcherRegistryMock->method('getMatcher')->willReturn($matcherObject)

I just wanted to have it covered as it seems the shape of the object changes substantially. If the test boils down to tons of mocking and only checking if deserialize method was called once, there is no need. Otherwise, it brings value.

[konradoboza]

Looks okay-ish but I would like to see the test covering this scenario but this is kind hard to deliver after internal sync. Maybe @alongosz has some tips on how to test that so it brings any value (without mocking tons of classes).

[vidarl]

I have written a test now.
I was not able to mock the CustomMatcher as it:

• Needs to both extend Ibexa\Core\MVC\Symfony\SiteAccess\Matcher and implement Ibexa\Bundle\Core\SiteAccess\Matcher
• Have some property that is serialized and which can be validated if it is also deserialized or not

SonarCloud fails due to duplication code in the test. I asssume this is OK ?
@konradoboza , @alongosz

[vidarl]

SonarCloud is happy too now....

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarCloud

[micszo]

Reproduced and retested.

Sanities with matchers OK.