From 84953e597b70799533b334eba931844ef764aee8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Feb 2026 12:59:35 +0000 Subject: [PATCH 1/2] Initial plan From 3b6c777961161ae61c79e4345871c0d2a5fa8306 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Feb 2026 13:07:18 +0000 Subject: [PATCH 2/2] Structural refactoring: type hints, return types, and VendorSpecificHandler injection Co-authored-by: frwg <44997160+frwg@users.noreply.github.com> --- .../Component/ElementInventoryService.php | 2 +- .../Source/Tunnel/InstanceTunnelService.php | 15 +++--- .../CoreBundle/Resources/config/services.xml | 1 + .../PrintBundle/Component/Template.php | 52 +++++++++---------- .../WmsSourceInstanceConfigGenerator.php | 19 +++---- .../Component/VendorSpecificHandler.php | 22 ++++---- .../WmsBundle/Resources/config/services.xml | 2 + 7 files changed, 58 insertions(+), 55 deletions(-) diff --git a/src/Mapbender/CoreBundle/Component/ElementInventoryService.php b/src/Mapbender/CoreBundle/Component/ElementInventoryService.php index 690810a4c6..86582f7d1e 100644 --- a/src/Mapbender/CoreBundle/Component/ElementInventoryService.php +++ b/src/Mapbender/CoreBundle/Component/ElementInventoryService.php @@ -43,7 +43,7 @@ class ElementInventoryService extends ElementConfigFilter implements HttpHandler /** @var ElementServiceInterface[] */ protected $serviceElements = array(); - public function __construct($disabledClasses) + public function __construct(array $disabledClasses) { $this->disabledClassesFromConfig = $disabledClasses ?: array(); } diff --git a/src/Mapbender/CoreBundle/Component/Source/Tunnel/InstanceTunnelService.php b/src/Mapbender/CoreBundle/Component/Source/Tunnel/InstanceTunnelService.php index da720eaa74..96592a4d32 100644 --- a/src/Mapbender/CoreBundle/Component/Source/Tunnel/InstanceTunnelService.php +++ b/src/Mapbender/CoreBundle/Component/Source/Tunnel/InstanceTunnelService.php @@ -39,6 +39,8 @@ class InstanceTunnelService protected $tokenStorage; /** @var EntityManagerInterface */ protected $entityManager; + /** @var VendorSpecificHandler */ + protected $vendorSpecificHandler; /** @var Endpoint[] */ protected $bufferedEndPoints = array(); /** @var string */ @@ -52,18 +54,21 @@ class InstanceTunnelService * @param TypeDirectoryService $sourceTypeDirectory * @param TokenStorageInterface $tokenStorage * @param EntityManagerInterface $entityManager + * @param VendorSpecificHandler $vendorSpecificHandler */ public function __construct(HttpTransportInterface $httpTransprot, RouterInterface $router, TypeDirectoryService $sourceTypeDirectory, TokenStorageInterface $tokenStorage, - EntityManagerInterface $entityManager) + EntityManagerInterface $entityManager, + ?VendorSpecificHandler $vendorSpecificHandler = null) { $this->httpTransport = $httpTransprot; $this->router = $router; $this->sourceTypeDirectory = $sourceTypeDirectory; $this->tokenStorage = $tokenStorage; $this->entityManager = $entityManager; + $this->vendorSpecificHandler = $vendorSpecificHandler ?? new VendorSpecificHandler(); // @todo: TBD if it's worth making this configurable $this->tunnelRouteName = 'mapbender_core_instancetunnel_instancetunnel'; $this->legendTunnelRouteName = 'mapbender_core_instancetunnel_instancetunnellegend'; @@ -103,8 +108,7 @@ public function makeEndpoint(Application $application, SourceInstance $instance) */ public function getPublicBaseUrl(Endpoint $endpoint) { - $vsHandler = new VendorSpecificHandler(); - $vsParams = $vsHandler->getPublicParams($endpoint->getSourceInstance(), $this->tokenStorage->getToken()); + $vsParams = $this->vendorSpecificHandler->getPublicParams($endpoint->getSourceInstance(), $this->tokenStorage->getToken()); $params = array_replace($vsParams, array( 'slug' => $endpoint->getApplicationEntity()->getSlug(), 'instanceId' => $endpoint->getSourceInstance()->getId(), @@ -221,10 +225,9 @@ protected function matchRouteParams(Application $application, array $routerMatch * @param SourceInstance $instance * @return string[] */ - public function getHiddenParams(SourceInstance $instance) + public function getHiddenParams(SourceInstance $instance): array { - $vsHandler = new VendorSpecificHandler(); - return $vsHandler->getHiddenParams($instance, $this->tokenStorage->getToken()); + return $this->vendorSpecificHandler->getHiddenParams($instance, $this->tokenStorage->getToken()); } /** diff --git a/src/Mapbender/CoreBundle/Resources/config/services.xml b/src/Mapbender/CoreBundle/Resources/config/services.xml index 0847f4ff5c..2202fc290b 100644 --- a/src/Mapbender/CoreBundle/Resources/config/services.xml +++ b/src/Mapbender/CoreBundle/Resources/config/services.xml @@ -182,6 +182,7 @@ + diff --git a/src/Mapbender/PrintBundle/Component/Template.php b/src/Mapbender/PrintBundle/Component/Template.php index 9598dbd8b6..2db9410294 100644 --- a/src/Mapbender/PrintBundle/Component/Template.php +++ b/src/Mapbender/PrintBundle/Component/Template.php @@ -28,36 +28,32 @@ class Template implements \ArrayAccess const ORIENTATION_PORTRAIT = 'portrait'; /** @var float in mm*/ - protected $width; + protected float $width; /** @var float in mm*/ - protected $height; - /** @var string */ - protected $orientation; - /** @var RegionCollection */ - protected $textFields; - /** @var RegionCollection */ - protected $regions; + protected float $height; + protected string $orientation; + protected RegionCollection $textFields; + protected RegionCollection $regions; /** * @param float $width in mm * @param float $height in mm * @param string $orientation */ - public function __construct($width, $height, $orientation) + public function __construct(float $width, float $height, string $orientation) { - if ($width <= 0 || $height <=0) { - throw new \InvalidArgumentException("Invalid width / height " - . print_r($width, true) . ' ' . print_r($height, true)); + if ($width <= 0 || $height <= 0) { + throw new \InvalidArgumentException("Invalid width / height " . $width . ' ' . $height); } - $this->width = floatval($width); - $this->height = floatval($height); + $this->width = $width; + $this->height = $height; switch ($orientation) { case self::ORIENTATION_LANDSCAPE: case self::ORIENTATION_PORTRAIT: $this->orientation = $orientation; break; default: - throw new \InvalidArgumentException("Invalid orientation " . print_r($orientation, true)); + throw new \InvalidArgumentException("Invalid orientation " . $orientation); } $this->textFields = new RegionCollection(); @@ -67,7 +63,7 @@ public function __construct($width, $height, $orientation) /** * @return string */ - public function getOrientation() + public function getOrientation(): string { return $this->orientation; } @@ -75,7 +71,7 @@ public function getOrientation() /** * @return float */ - public function getWidth() + public function getWidth(): float { return $this->width; } @@ -83,7 +79,7 @@ public function getWidth() /** * @return float */ - public function getHeight() + public function getHeight(): float { return $this->height; } @@ -91,7 +87,7 @@ public function getHeight() /** * @return RegionCollection|TemplateRegion[] */ - public function getRegions() + public function getRegions(): RegionCollection { return $this->regions; } @@ -99,34 +95,34 @@ public function getRegions() /** * @return RegionCollection|TemplateRegion[] */ - public function getTextFields() + public function getTextFields(): RegionCollection { return $this->textFields; } /** - * @param string + * @param string $name * @return TemplateRegion */ - public function getRegion($name) + public function getRegion($name): TemplateRegion { return $this->regions->getMember($name); } /** - * @param string + * @param string $name * @return bool */ - public function hasRegion($name) + public function hasRegion($name): bool { return $this->regions->hasMember($name); } /** - * @param string + * @param string $name * @return bool */ - public function hasTextField($name) + public function hasTextField($name): bool { return $this->textFields->hasMember($name); } @@ -134,7 +130,7 @@ public function hasTextField($name) /** * @param TemplateRegion $region */ - public function addRegion($region) + public function addRegion($region): void { $region->setParentTemplate($this); $this->regions->addMember($region->getName(), $region); @@ -143,7 +139,7 @@ public function addRegion($region) /** * @param TemplateRegion $field */ - public function addTextField($field) + public function addTextField($field): void { $field->setParentTemplate($this); $this->textFields->addMember($field->getName(), $field); diff --git a/src/Mapbender/WmsBundle/Component/Presenter/WmsSourceInstanceConfigGenerator.php b/src/Mapbender/WmsBundle/Component/Presenter/WmsSourceInstanceConfigGenerator.php index 117be8e6f7..0c0af50c7c 100644 --- a/src/Mapbender/WmsBundle/Component/Presenter/WmsSourceInstanceConfigGenerator.php +++ b/src/Mapbender/WmsBundle/Component/Presenter/WmsSourceInstanceConfigGenerator.php @@ -28,13 +28,16 @@ */ class WmsSourceInstanceConfigGenerator extends SourceInstanceConfigGenerator { + protected VendorSpecificHandler $vendorSpecificHandler; public function __construct( protected UrlProcessor $urlProcessor, protected TokenStorageInterface $tokenStorage, protected EntityManagerInterface $em, - protected ?string $defaultLayerOrder) + protected ?string $defaultLayerOrder, + ?VendorSpecificHandler $vendorSpecificHandler = null) { + $this->vendorSpecificHandler = $vendorSpecificHandler ?? new VendorSpecificHandler(); } /** @@ -237,7 +240,7 @@ public function postProcessUrls(Application $application, WmsInstance $sourceIns * @param WmsInstance $sourceInstance * @return string */ - public function getUrlOption(WmsInstance $sourceInstance) + public function getUrlOption(WmsInstance $sourceInstance): string { $url = $sourceInstance->getSource()->getGetMap()->getHttpGet(); if (!$this->useTunnel($sourceInstance)) { @@ -252,8 +255,7 @@ public function getUrlOption(WmsInstance $sourceInstance) } } $userToken = $this->tokenStorage->getToken(); - $vsHandler = new VendorSpecificHandler(); - $params = $vsHandler->getPublicParams($sourceInstance, $userToken); + $params = $this->vendorSpecificHandler->getPublicParams($sourceInstance, $userToken); return UrlUtil::validateUrl($url, $params); } @@ -263,7 +265,7 @@ public function getUrlOption(WmsInstance $sourceInstance) * @param WmsInstance $sourceInstance * @return float[][] */ - public function getBboxConfiguration(WmsInstance $sourceInstance) + public function getBboxConfiguration(WmsInstance $sourceInstance): array { $rootLayer = $this->getRootLayerFromCache($sourceInstance); return $this->getLayerBboxConfiguration($rootLayer); @@ -344,7 +346,7 @@ protected function getAvailableStyles(Application $application, WmsInstance $ins * @param WmsInstance $sourceInstance * @return array[] */ - public function getDimensionsConfiguration(WmsInstance $sourceInstance) + public function getDimensionsConfiguration(WmsInstance $sourceInstance): array { $dimensionConfigs = array(); $sourceDimensions = array(); @@ -447,10 +449,9 @@ public function useTunnel(SourceInstance $sourceInstance): bool } /** @var WmsInstance $sourceInstance */ - $vsHandler = new VendorSpecificHandler(); return (!!$sourceInstance->getSource()->getUsername()) || - $vsHandler->hasHiddenParams($sourceInstance) || + $this->vendorSpecificHandler->hasHiddenParams($sourceInstance) || $sourceInstance->getProxy(); } @@ -468,7 +469,7 @@ public function useProxy(WmsInstance $sourceInstance): bool * @return mixed[] * @todo: this should and can be part of the initial generation */ - protected function proxifyLayerUrls($layerConfig, ?SourceInstance $sourceInstance = null) + protected function proxifyLayerUrls(array $layerConfig, ?SourceInstance $sourceInstance = null): array { /** @var ?WmsInstance $sourceInstance */ if (isset($layerConfig['children'])) { diff --git a/src/Mapbender/WmsBundle/Component/VendorSpecificHandler.php b/src/Mapbender/WmsBundle/Component/VendorSpecificHandler.php index 88ca2ae3cc..47a78529f6 100644 --- a/src/Mapbender/WmsBundle/Component/VendorSpecificHandler.php +++ b/src/Mapbender/WmsBundle/Component/VendorSpecificHandler.php @@ -30,7 +30,7 @@ class VendorSpecificHandler * @param string $input * @return string|null */ - public function findDynamicValuePortion($input) + public function findDynamicValuePortion(string $input): ?string { $matches = array(); if (\preg_match('#\$[a-z]+\$#i', $input, $matches)) { @@ -47,12 +47,12 @@ public function findDynamicValuePortion($input) * @param string $value * @return boolean true if a value is dynamic. */ - public function isValueDynamic($value) + public function isValueDynamic(string $value): bool { return !!$this->findDynamicValuePortion($value); } - public function isValuePublic(VendorSpecific $vendorspec) + public function isValuePublic(VendorSpecific $vendorspec): bool { return !$vendorspec->getHidden(); } @@ -62,7 +62,7 @@ public function isValuePublic(VendorSpecific $vendorspec) * @param TokenInterface|null $userToken * @return string[] */ - public function getPublicParams(SourceInstance $instance, TokenInterface $userToken=null) + public function getPublicParams(SourceInstance $instance, TokenInterface $userToken=null): array { $user = $this->getUserFromToken($userToken); $params = array(); @@ -80,7 +80,7 @@ public function getPublicParams(SourceInstance $instance, TokenInterface $userTo * @param TokenInterface|null $userToken * @return string[] */ - public function getHiddenParams(SourceInstance $instance, TokenInterface $userToken=null) + public function getHiddenParams(SourceInstance $instance, TokenInterface $userToken=null): array { $user = $this->getUserFromToken($userToken); $params = array(); @@ -98,7 +98,7 @@ public function getHiddenParams(SourceInstance $instance, TokenInterface $userTo * @param TokenInterface|null $userToken * @return string[] */ - public function getAllParams(SourceInstance $instance, TokenInterface $userToken=null) + public function getAllParams(SourceInstance $instance, TokenInterface $userToken=null): array { $user = $this->getUserFromToken($userToken); $params = array(); @@ -116,7 +116,7 @@ public function getAllParams(SourceInstance $instance, TokenInterface $userToken * @param SourceInstance|WmsInstance $instance; NOTE: lax typing to avoid conflicts with WMTS * @return bool */ - public function hasHiddenParams(SourceInstance $instance) + public function hasHiddenParams(SourceInstance $instance): bool { foreach ($instance->getVendorspecifics() as $key => $vendorspec) { if ($this->isVendorSpecificValueValid($vendorspec) && !$this->isValuePublic($vendorspec)) { @@ -130,7 +130,7 @@ public function hasHiddenParams(SourceInstance $instance) * @param TokenInterface $userToken * @return UserInterface|null */ - protected function getUserFromToken(TokenInterface $userToken=null) + protected function getUserFromToken(TokenInterface $userToken=null): ?UserInterface { if (!$userToken || $userToken instanceof NullToken) { return null; @@ -154,7 +154,7 @@ protected function getUserFromToken(TokenInterface $userToken=null) * @param UserInterface|null $object * @return string|null */ - public function getVendorSpecificValue(VendorSpecific $vs, $object) + public function getVendorSpecificValue(VendorSpecific $vs, $object): ?string { $value = $vs->getDefault(); if ($vs->getVstype() !== VendorSpecific::TYPE_VS_SIMPLE) { @@ -172,7 +172,7 @@ public function getVendorSpecificValue(VendorSpecific $vs, $object) * @param string $attributeName * @return string|null */ - protected function extractDynamicReference(VendorSpecific $vs, $object, $attributeName) + protected function extractDynamicReference(VendorSpecific $vs, $object, string $attributeName): ?string { if (!$object || !is_object($object)) { return null; @@ -205,7 +205,7 @@ protected function extractDynamicReference(VendorSpecific $vs, $object, $attribu } } - public function isVendorSpecificValueValid(VendorSpecific $vs) + public function isVendorSpecificValueValid(VendorSpecific $vs): bool { if ($vs->getDefault()) { return true; diff --git a/src/Mapbender/WmsBundle/Resources/config/services.xml b/src/Mapbender/WmsBundle/Resources/config/services.xml index 9c299d648c..8816c00185 100644 --- a/src/Mapbender/WmsBundle/Resources/config/services.xml +++ b/src/Mapbender/WmsBundle/Resources/config/services.xml @@ -33,11 +33,13 @@ + %wms.default_layer_order% +