diff --git a/src/CoreBundle/Controller/ListControllerTrait.php b/src/CoreBundle/Controller/ListControllerTrait.php index db9652f77..005975287 100644 --- a/src/CoreBundle/Controller/ListControllerTrait.php +++ b/src/CoreBundle/Controller/ListControllerTrait.php @@ -33,7 +33,7 @@ use MetaModels\Filter\FilterUrl; use MetaModels\Filter\FilterUrlBuilder; use MetaModels\Filter\Setting\IFilterSettingFactory; -use MetaModels\FrontendIntegration\FrontendFilterOptions; +use MetaModels\Filter\Setting\ParameterTypes; use MetaModels\Helper\SortingLinkGenerator; use MetaModels\IFactory; use MetaModels\IItem; @@ -343,20 +343,21 @@ private function getResponseInternal(Template $template, Model $model, Request $ private function getFilterParameters(FilterUrl $filterUrl, ItemList $itemRenderer): array { $filterSetting = $itemRenderer->getFilterSettings(); - /** @var array $wantedByType */ - $wantedByType = []; - // FIXME: improve this call - it does too much. - foreach ( - $filterSetting->getParameterFilterWidgets([], [], new FrontendFilterOptions()) as $widgetName => $widget - ) { - $wantedByType[$widgetName] = (string) ($widget['param_type'] ?? 'slugNget'); - } + // Obtain the types from the filter settings themselves - filter rules without frontend filter widget + // (i.e. the usual detail page rules) do not render a widget but still define a parameter type. + $wantedByType = ParameterTypes::fromSetting($filterSetting); $result = []; foreach ($filterSetting->getParameters() as $name) { - if (null !== $value = $this->tryReadFromSlugOrGet($filterUrl, $name, $wantedByType[$name] ?? 'slugNget')) { - $result[$name] = $value; + $paramType = $wantedByType[$name] ?? ParameterTypes::LEGACY_TYPE; + $value = $this->tryReadFromSlugOrGet($filterUrl, $name, $paramType); + if (null === $value) { + // Either not passed at all or passed via another URL type than the configured one - in both cases + // the parameter simply stays unused. It has been marked as used in tryReadFromSlugOrGet() so a + // slug of the wrong type does not end up in a 404 for unused route arguments. + continue; } + $result[$name] = $value; } return $result; diff --git a/src/Filter/Setting/Collection.php b/src/Filter/Setting/Collection.php index 52728e4ae..9c1e9a032 100644 --- a/src/Filter/Setting/Collection.php +++ b/src/Filter/Setting/Collection.php @@ -167,6 +167,21 @@ public function getParameters() return [] === $parameters ? [] : \array_merge(...$parameters); } + /** + * Retrieve the URL parameter type for all registered parameters of all contained settings. + * + * @return array The parameter types as array. parametername => type + */ + public function getParameterTypes() + { + $types = []; + foreach ($this->arrSettings as $objSetting) { + $types[] = ParameterTypes::fromSetting($objSetting); + } + + return [] === $types ? [] : \array_merge(...$types); + } + /** * {@inheritdoc} */ diff --git a/src/Filter/Setting/CustomSql.php b/src/Filter/Setting/CustomSql.php index 49af48caf..86ef540d4 100644 --- a/src/Filter/Setting/CustomSql.php +++ b/src/Filter/Setting/CustomSql.php @@ -50,6 +50,7 @@ use function array_intersect_key; use function array_key_exists; use function array_keys; +use function array_fill_keys; use function array_map; use function array_merge; use function array_reduce; @@ -234,6 +235,20 @@ public function getParameters() return $arrParams; } + /** + * Retrieve the URL parameter type for all registered parameters from the setting. + * + * @return array The parameter types as array. parametername => type + */ + public function getParameterTypes() + { + // Legacy settings without a value keep the lenient behaviour of accepting both variants. + return array_fill_keys( + $this->getParameters(), + (string) ($this->get('param_type') ?: ParameterTypes::LEGACY_TYPE) + ); + } + /** * {@inheritdoc} */ diff --git a/src/Filter/Setting/ExpressionRule.php b/src/Filter/Setting/ExpressionRule.php index 5e35b2300..fad952151 100644 --- a/src/Filter/Setting/ExpressionRule.php +++ b/src/Filter/Setting/ExpressionRule.php @@ -125,6 +125,21 @@ public function getParameters(): array return array_merge(...$parameters); } + /** + * Retrieve the URL parameter type for all registered parameters from the setting. + * + * @return array The parameter types as array. parametername => type + */ + public function getParameterTypes(): array + { + $types = []; + foreach ($this->children as $child) { + $types[] = ParameterTypes::fromSetting($child); + } + + return array_merge([], ...$types); + } + #[Override] public function getParameterDCA(): array { diff --git a/src/Filter/Setting/ICollection.php b/src/Filter/Setting/ICollection.php index c5adcc915..818b0295e 100644 --- a/src/Filter/Setting/ICollection.php +++ b/src/Filter/Setting/ICollection.php @@ -31,6 +31,15 @@ /** * This interface handles all filter setting abstraction. + * + * "getParameterTypes()" returns the URL parameter type for all registered parameters (parametername => type) of all + * contained filter settings, see ISimple for the possible types. + * + * Not implementing "getParameterTypes()" is deprecated, the method will get added to this interface in + * MetaModels 3.0. Until then, collections not providing it are treated as "slugNget" (the lenient legacy + * behaviour), see ParameterTypes::fromSetting(). + * + * @method array getParameterTypes() Retrieve the URL parameter type for all parameters. */ interface ICollection { diff --git a/src/Filter/Setting/ISimple.php b/src/Filter/Setting/ISimple.php index 3d5e69665..d915dbc20 100644 --- a/src/Filter/Setting/ISimple.php +++ b/src/Filter/Setting/ISimple.php @@ -30,6 +30,17 @@ /** * This interface handles the abstraction for a single filter setting. + * + * "getParameterTypes()" returns the URL parameter type for all registered parameters (parametername => type). The + * type determines from where the value of a parameter may get read and how the URL for it has to be built. Valid + * types are "slug" (key/value in the URL path), "get" (key=value in the query string) and the deprecated "slugNget" + * (both of them). See Simple::getParameterTypes() for the default implementation. + * + * Not implementing "getParameterTypes()" is deprecated, the method will get added to this interface in + * MetaModels 3.0. Until then, settings not providing it are treated as "slugNget" (the lenient legacy behaviour), + * see ParameterTypes::fromSetting(). + * + * @method array getParameterTypes() Retrieve the URL parameter type for all parameters. */ interface ISimple { diff --git a/src/Filter/Setting/ParameterTypes.php b/src/Filter/Setting/ParameterTypes.php new file mode 100644 index 000000000..bdef0f6a0 --- /dev/null +++ b/src/Filter/Setting/ParameterTypes.php @@ -0,0 +1,75 @@ + + * @copyright 2012-2026 The MetaModels team. + * @license https://github.com/MetaModels/core/blob/master/LICENSE LGPL-3.0-or-later + * @filesource + */ + +declare(strict_types=1); + +namespace MetaModels\Filter\Setting; + +use function array_fill_keys; +use function method_exists; + +/** + * Helper to obtain the URL parameter types from a filter setting or a filter setting collection. + * + * This provides the backwards compatibility layer for implementations not (yet) providing + * "getParameterTypes()" - the method will get added to ISimple and ICollection in MetaModels 3.0. Adding it to the + * interfaces before would break every implementation out there, therefore it is only announced via "@method" there. + * + * @internal + */ +final class ParameterTypes +{ + /** + * The lenient legacy type, accepting both slug and GET. + */ + public const LEGACY_TYPE = 'slugNget'; + + /** + * Obtain the URL parameter types of the passed filter setting or filter setting collection. + * + * @param ICollection|ISimple $setting The filter setting to obtain the types from. + * + * @return array The parameter types as array. parametername => type + */ + public static function fromSetting(ICollection|ISimple $setting): array + { + if (!method_exists($setting, 'getParameterTypes')) { + // Settings without any parameter can not be affected - stay silent for them. + if ([] === ($parameters = $setting->getParameters())) { + return []; + } + + // @codingStandardsIgnoreStart + @trigger_error( + 'Filter setting "' . $setting::class . '" does not implement "getParameterTypes()". ' . + 'The parameters are treated as "' . self::LEGACY_TYPE . '". ' . + 'The method will be required in MetaModels 3.0.', + E_USER_DEPRECATED + ); + // @codingStandardsIgnoreEnd + + return array_fill_keys($parameters, self::LEGACY_TYPE); + } + + /** @var array $types */ + $types = $setting->getParameterTypes(); + + return $types; + } +} diff --git a/src/Filter/Setting/Simple.php b/src/Filter/Setting/Simple.php index 1770cc4ca..2ba5a4141 100644 --- a/src/Filter/Setting/Simple.php +++ b/src/Filter/Setting/Simple.php @@ -571,6 +571,20 @@ public function getParameters() return []; } + /** + * Retrieve the URL parameter type for all registered parameters from the setting. + * + * @return array The parameter types as array. parametername => type + */ + public function getParameterTypes() + { + // Legacy settings without a value keep the lenient behaviour of accepting both variants. + return \array_fill_keys( + $this->getParameters(), + (string) ($this->get('param_type') ?: ParameterTypes::LEGACY_TYPE) + ); + } + /** * {@inheritdoc} */ diff --git a/src/Filter/Setting/WithChildren.php b/src/Filter/Setting/WithChildren.php index f419d438e..4691dcc08 100644 --- a/src/Filter/Setting/WithChildren.php +++ b/src/Filter/Setting/WithChildren.php @@ -76,6 +76,19 @@ public function getParameters() return $arrParams; } + /** + * {@inheritdoc} + */ + #[\Override] + public function getParameterTypes() + { + $arrTypes = []; + foreach ($this->arrChildren as $objSetting) { + $arrTypes = array_merge($arrTypes, ParameterTypes::fromSetting($objSetting)); + } + return $arrTypes; + } + /** * {@inheritdoc} */ diff --git a/src/FrontendIntegration/FrontendFilter.php b/src/FrontendIntegration/FrontendFilter.php index a0f296a0b..6fee8b21c 100644 --- a/src/FrontendIntegration/FrontendFilter.php +++ b/src/FrontendIntegration/FrontendFilter.php @@ -32,7 +32,6 @@ use ContaoCommunityAlliance\Contao\Bindings\ContaoEvents; use ContaoCommunityAlliance\Contao\Bindings\Events\Controller\RedirectEvent; use Contao\CoreBundle\Csrf\ContaoCsrfTokenManager; -use Contao\CoreBundle\Exception\PageNotFoundException; use Contao\CoreBundle\Exception\RedirectResponseException; use Contao\FrontendTemplate; use Contao\Input; @@ -435,10 +434,10 @@ protected function getFilters() ); // DAMN Contao - we have to "mark" the keys in the Input class as used as we get an 404 otherwise. + // This is also done for parameters passed via another URL type than the configured one. Their value is + // not used for filtering (see buildParameters()), but they must not end up in a 404 either. foreach ($wantedNames as $name) { - if ($all->hasSlug($name)) { - Input::get($name); - } + Input::get($name); } $values = \array_merge($all->getSlugParameters(), $all->getGetParameters()); @@ -450,13 +449,6 @@ protected function getFilters() $filterOptions ); - // 404 if a get-only filter parameter is accessed via slug. - foreach ($arrWidgets as $widgetName => $widget) { - if ('get' === ($widget['param_type'] ?? 'slug') && $all->hasSlug($widgetName)) { - throw new PageNotFoundException(); - } - } - // If we have POST data, we need to redirect now. if (Input::post('FORM_SUBMIT') === $this->formId) { foreach ($wantedNames as $widgetName) { diff --git a/src/Render/Setting/Collection.php b/src/Render/Setting/Collection.php index 9f8370533..a11fa3d17 100644 --- a/src/Render/Setting/Collection.php +++ b/src/Render/Setting/Collection.php @@ -31,6 +31,7 @@ use MetaModels\Filter\FilterUrl; use MetaModels\Filter\FilterUrlBuilder; use MetaModels\Filter\Setting\IFilterSettingFactory; +use MetaModels\Filter\Setting\ParameterTypes; use MetaModels\IItem; use MetaModels\IMetaModel; use MetaModels\ITranslatedMetaModel; @@ -339,18 +340,25 @@ public function buildJumpToUrlFor(IItem $item /**, ?int $referenceType */) if (!empty($information['filterSetting'])) { /** @var \MetaModels\Filter\Setting\ICollection $filterSetting */ - $filterSetting = $information['filterSetting']; - $parameterList = $filterSetting->generateFilterUrlFrom($item, $this); + $filterSetting = $information['filterSetting']; + $parameterList = $filterSetting->generateFilterUrlFrom($item, $this); + $parameterTypes = ParameterTypes::fromSetting($filterSetting); foreach ($parameterList as $strKey => $strValue) { // Sadly the filter values are currently encoded due to legacy reasons. // For MetaModels 3, they should be passed around decoded everywhere. - $filterUrl->setSlug($strKey, \rawurldecode($strValue))->setGet($strKey, ''); + $strValue = \rawurldecode($strValue); + // Build the URL as configured in the filter setting - "slugNget" and anything else use the slug. + if ('get' === ($parameterTypes[$strKey] ?? 'slug')) { + $filterUrl->setGet($strKey, $strValue)->setSlug($strKey, ''); + continue; + } + $filterUrl->setSlug($strKey, $strValue)->setGet($strKey, ''); } } $result['params'] = $parameterList; - $result['deep'] = !empty($filterUrl->getSlugParameters()); + $result['deep'] = !empty($filterUrl->getSlugParameters()) || !empty($filterUrl->getGetParameters()); $result['url'] = $this->filterUrlBuilder->generate( $filterUrl, diff --git a/tests/Filter/Setting/CollectionTest.php b/tests/Filter/Setting/CollectionTest.php index a76afe9a4..b3f3e3cd2 100644 --- a/tests/Filter/Setting/CollectionTest.php +++ b/tests/Filter/Setting/CollectionTest.php @@ -20,6 +20,8 @@ namespace MetaModels\Test\Filter\Setting; use MetaModels\Filter\Setting\Collection; +use MetaModels\Filter\Setting\ISimple; +use MetaModels\Filter\Setting\Simple; use MetaModels\FrontendIntegration\FrontendFilterOptions; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -52,4 +54,81 @@ public function testGetParametersReturnsEmptyArrayWhenNoSettings(): void self::assertSame([], $collection->getParameters()); } + + /** + * getParameterTypes() returns an empty array when the collection has no settings. + */ + public function testGetParameterTypesReturnsEmptyArrayWhenNoSettings(): void + { + $collection = new Collection([]); + + self::assertSame([], $collection->getParameterTypes()); + } + + /** + * getParameterTypes() collects the types of all contained settings - also for settings not rendering a + * frontend filter widget (i.e. the usual detail page filter rules). + */ + public function testGetParameterTypesCollectsTypesFromAllSettings(): void + { + $collection = new Collection([]); + $collection->addSetting($this->mockSetting(['alias' => 'get'])); + $collection->addSetting($this->mockSetting(['category' => 'slug', 'legacy' => 'slugNget'])); + + self::assertSame( + ['alias' => 'get', 'category' => 'slug', 'legacy' => 'slugNget'], + $collection->getParameterTypes() + ); + } + + /** + * Settings not implementing getParameterTypes() (BC layer) are treated as "slugNget". + */ + public function testGetParameterTypesFallsBackToSlugNgetForLegacySettings(): void + { + $legacySetting = $this->getMockForAbstractClass(ISimple::class); + $legacySetting->method('getParameters')->willReturn(['legacy_param']); + + $collection = new Collection([]); + $collection->addSetting($legacySetting); + + $previous = set_error_handler( + static function (int $severity, string $message) use (&$deprecation): bool { + unset($severity); + $deprecation = $message; + + return true; + }, + E_USER_DEPRECATED + ); + + try { + $types = $collection->getParameterTypes(); + } finally { + set_error_handler($previous); + } + + self::assertSame(['legacy_param' => 'slugNget'], $types); + self::assertStringContainsString('getParameterTypes()', (string) $deprecation); + } + + /** + * Mock a filter setting providing the passed parameter types. + * + * @param array $types The parameter types (parametername => type). + * + * @return ISimple + */ + private function mockSetting(array $types): ISimple + { + $setting = $this + ->getMockBuilder(Simple::class) + ->disableOriginalConstructor() + ->onlyMethods(['getParameters', 'getParameterTypes']) + ->getMockForAbstractClass(); + $setting->method('getParameters')->willReturn(array_keys($types)); + $setting->method('getParameterTypes')->willReturn($types); + + return $setting; + } } diff --git a/tests/Filter/Setting/SimpleParameterTypesTest.php b/tests/Filter/Setting/SimpleParameterTypesTest.php new file mode 100644 index 000000000..5b5e88587 --- /dev/null +++ b/tests/Filter/Setting/SimpleParameterTypesTest.php @@ -0,0 +1,133 @@ + + * @copyright 2012-2026 The MetaModels team. + * @license https://github.com/MetaModels/core/blob/master/LICENSE LGPL-3.0-or-later + * @filesource + */ + +declare(strict_types=1); + +namespace MetaModels\Test\Filter\Setting; + +use MetaModels\Filter\FilterUrlBuilder; +use MetaModels\Filter\Setting\ICollection; +use MetaModels\Filter\Setting\Simple; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Contracts\Translation\TranslatorInterface; + +/** + * Test the URL parameter types of simple filter settings. + * + * @covers \MetaModels\Filter\Setting\Simple + */ +#[CoversClass(Simple::class)] +class SimpleParameterTypesTest extends TestCase +{ + /** + * Data provider for the configurable URL types. + * + * @return array + */ + public static function providerParamType(): array + { + return [ + 'slug' => ['slug'], + 'get' => ['get'], + 'slugNget' => ['slugNget'], + ]; + } + + /** + * The configured param_type is reported for every parameter of the setting. + * + * This has to work for settings without frontend filter widget as well (the usual detail page filter rules), + * as the type is otherwise unknown and both slug and GET would be accepted. + * + * @param string $paramType The configured URL type. + */ + #[DataProvider('providerParamType')] + public function testReportsConfiguredType(string $paramType): void + { + $setting = $this->mockSimpleFilterSetting(['my_param'], ['param_type' => $paramType]); + + self::assertSame(['my_param' => $paramType], $setting->getParameterTypes()); + } + + /** + * Legacy settings without stored param_type keep the lenient behaviour of accepting slug and GET. + */ + public function testFallsBackToSlugNget(): void + { + $setting = $this->mockSimpleFilterSetting(['my_param'], []); + + self::assertSame(['my_param' => 'slugNget'], $setting->getParameterTypes()); + } + + /** + * All parameters of a setting share the configured type. + */ + public function testCoversAllParameters(): void + { + $setting = $this->mockSimpleFilterSetting(['from', 'to'], ['param_type' => 'get']); + + self::assertSame(['from' => 'get', 'to' => 'get'], $setting->getParameterTypes()); + } + + /** + * A setting without any parameter reports no types. + */ + public function testIsEmptyWithoutParameters(): void + { + $setting = $this->mockSimpleFilterSetting([], ['param_type' => 'get']); + + self::assertSame([], $setting->getParameterTypes()); + } + + /** + * Mock a Simple filter setting returning the passed parameter names. + * + * @param list $parameters The parameter names the setting shall report. + * @param array $properties The initialization data. + * + * @return Simple|MockObject + */ + private function mockSimpleFilterSetting(array $parameters, array $properties) + { + $filterUrlBuilder = $this->getMockBuilder(FilterUrlBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $setting = $this + ->getMockBuilder(Simple::class) + ->setConstructorArgs( + [ + $this->getMockForAbstractClass(ICollection::class), + $properties, + $this->getMockForAbstractClass(EventDispatcherInterface::class), + $filterUrlBuilder, + $this->getMockForAbstractClass(TranslatorInterface::class) + ] + ) + ->onlyMethods(['getParameters']) + ->getMockForAbstractClass(); + $setting->method('getParameters')->willReturn($parameters); + + return $setting; + } +}