[TMP] IBX-12046: Removed Symfony 8 deprecated code usage - #822
Conversation
Replaced DefaultRouter's inheritance from the @Final FrameworkBundle Router with a decorator of router.default, dropped symfony/templating from the base Controller, and aligned Security, Validator, HttpFoundation and OptionsResolver usages with the Symfony 7.4 contracts so the unit and integration suites report no direct Symfony deprecations.
Missing-type ignores no longer match now typed methods, tripping reportUnmatchedIgnoredErrors.
ViniTou
changed the title
d189346 to
6f649b0
Compare
6f649b0 to
1f8e6a3
Compare
38dfb70 to
b3b24cd
Compare
…tifier 6.0 is a major and Symfony 8 removes options-array support from Constraint itself, so the contract now only accepts named/positional arguments. The three subclasses in the org (discounts, segmentation) are final and their production usages go through mapping loaders or attributes, which already pass named arguments; only test call sites changed (ibexa/discounts#361, ibexa/segmentation#225).
It is an implementation detail of generate(); the reusable logic lives in the internal RequestContextFactory, which has its own test, and nothing in the organization calls the router method.
a2a18b0 to
6d52b84
Compare
Both query controllers receive the page number as a query-string parameter (main-request content view, or the fragment's own URI); request attributes and the POST body were only consulted because Request::get() did so generically. InputBag::getInt() also turns garbage values into a 400 instead of a Pagerfanta exception.
The pin lives in the workflow step rather than composer.json: an inline alias is only honoured in a root composer.json, so pinning inside the package conflicts with the ibexa/oss root requirement when the functional job builds the full project (that job takes the branch from dependencies.json instead).
| public function eraseCredentials(): void | ||
| { | ||
| $this->wrappedUser->eraseCredentials(); | ||
| $wrappedUserReflection = new \ReflectionObject($this->wrappedUser); |
There was a problem hiding this comment.
❤️
Since the method is now deprecated, Symfony 7.3+ no longer calls it, so the only remaining caller is manual application code. The ReflectionObject, hasMethod(), isPublic() and invoke() sequence is a lot of for that path, and isPublic() can never be false for an interface method. Could we drop the forwarding, or reduce it to if (method_exists($this->wrappedUser, 'eraseCredentials')) { $this->wrappedUser->eraseCredentials(); }?
There was a problem hiding this comment.
I want to drop this in actual bump to symfony 8, as I am not sure that this method was not called anywhere in sf7 stack.
There was a problem hiding this comment.
Dropped the isPublic() check, unreachable for an interface method: 01af398
| // Now putting back SiteAccess URI if needed. | ||
| if ($isSiteAccessAware && $siteAccess !== null && $siteAccess->matcher instanceof URILexer) { | ||
| if ($referenceType === self::ABSOLUTE_URL || $referenceType === self::NETWORK_PATH) { | ||
| $scheme = $context->getScheme(); | ||
| $port = ''; | ||
| if ($scheme === 'http' && $context->getHttpPort() !== 80) { | ||
| $port = ':' . $context->getHttpPort(); | ||
| } elseif ($scheme === 'https' && $context->getHttpsPort() !== 443) { | ||
| $port = ':' . $context->getHttpsPort(); | ||
| } | ||
|
|
||
| $base = $context->getHost() . $port . $context->getBaseUrl(); | ||
| } else { | ||
| $base = $context->getBaseUrl(); | ||
| } | ||
|
|
||
| $base = $context->getHost() . $port . $context->getBaseUrl(); | ||
| } else { | ||
| $base = $context->getBaseUrl(); | ||
| $linkUri = $base ? substr($url, strpos($url, $base) + strlen($base)) : $url; | ||
| $url = str_replace($linkUri, $siteAccess->matcher->analyseLink($linkUri), $url); | ||
| } |
There was a problem hiding this comment.
It looks like it is a self-contained block that would read better as a private helper, like
if ($isSiteAccessAware && $siteAccess !== null && $siteAccess->matcher instanceof URILexer) {
$url = $this->prependSiteAccessUri($url, $context, $referenceType, $siteAccess->matcher);
}
There was a problem hiding this comment.
probably, I keeped it this was to match how it look like in current version, but I dont see a reason to refactor this if changes are being made anyway.
There was a problem hiding this comment.
Extracted into DefaultRouter::prependSiteAccessUri(): f1f7466
| public function getInnerRouter(): RouterInterface&RequestMatcherInterface | ||
| { | ||
| return $this->innerRouter; | ||
| } |
There was a problem hiding this comment.
This public getter exists only for DownloadControllerRequestFlowTest. The decorated service is already reachable in tests by the router.default.inner id (or the container.decorator tag). It may be worth dropping it to avoid widening the public API for a test assertion.
There was a problem hiding this comment.
Dropped getInnerRouter(), test fetches ibexa.routing.default_router.inner from the container instead: bd4286a
…rate() into a helper method
…r router from container
|
2 participants
Related PRs:
Description:
First step of the Symfony 8 upgrade, done on Symfony 7.4: every direct Symfony deprecation the core unit and integration suites reported with
SYMFONY_DEPRECATIONS_HELPER=max[direct]=0is gone (59 → 0), and the code paths Symfony 8.0 removes outright are replaced while still running on 7.4.Structural change —
DefaultRouterbecomes a decoratorSymfony\Bundle\FrameworkBundle\Routing\Routeris@finaland becomesfinalin 8.0, soIbexa\Bundle\Core\Routing\DefaultRouterno longer extends it.DefaultRouteris now afinaldecorator ofrouter.default— serviceibexa.routing.default_router,decorates: router.default— implementingRouterInterface,RequestMatcherInterface,WarmableInterfaceandSiteAccessAware.semanticPathinfomatching, same SiteAccess-awaregenerate()logic.setSiteAccess()stays forSiteAccessAware.RouterPass(which swapped the class ofrouter.default) is removed;ChainRoutingPassonly tagsrouter.defaultfor the chain.setConfigResolver()is dropped — nothing read it.Contract alignment
symfony/templating(frozen at 6.4) is dropped:Controller::render()renders throughtwig,getTemplateEngine()→getTwig().User::getSalt()is removed anderaseCredentials()is marked#[\Deprecated], as Symfony 7.3 asks.UserChecker::checkPostAuth()and the two voters take the new nullableTokenInterface/Voteargument.UniqueIdentifierandLocationIsContainerContentTypeget#[HasNamedArguments]constructors.UniqueIdentifiercontract — 6.0 is a major, and Symfony 8 drops it fromConstraintanyway. The three subclasses in the org (discounts, segmentation) arefinal, and their YAML mappings and attributes already resolve to named arguments through Symfony's loaders, so only test call sites change.Request::get()and nestedOptionsResolver::setDefault()closures use the 7.4 replacements;symfony/options-resolveris now an explicit requirement, since core calls it directly.Serializer\Annotation\Ignore→Serializer\Attribute\Ignore.Merge order
ibexa/rest— it defines its templated URI router as aDefaultRouterservice inheriting the FrameworkBundle Router definition, which breaks onceDefaultRouteris a decorator; the rest PR wraps an inner templated router instead.ibexa/discountsandibexa/segmentation— test call sites for the named-arguments constraints.For QA:
Routing is the sensitive area:
siteaccessroute parameter.nonSiteAccessAwareRoutessuch as_wdt/_profiler.cache:warmupanddebug:router.Documentation:
Ibexa\Bundle\Core\Routing\DefaultRouterisfinaland no longer extendsSymfony\Bundle\FrameworkBundle\Routing\Router; the SiteAccess-aware router is therouter.defaultdecoratoribexa.routing.default_router.Ibexa\Core\MVC\Symfony\Controller\Controller::getTemplateEngine()is replaced bygetTwig().Ibexa\Contracts\Core\Validation\Constraint\UniqueIdentifiermust be instantiated with named (or positional) arguments:new MyConstraint(identifierPath: 'identifier', existingIdPath: 'id'). The options-array form is no longer accepted.