From 2aeaad580c95478b201439f2c064f7c602494c13 Mon Sep 17 00:00:00 2001 From: jeroendedauw Date: Sun, 4 Dec 2016 13:20:13 +0100 Subject: [PATCH] Fixed geoservice usage for center parameter in the map printer Another step towards fully fixing https://github.com/JeroenDeDauw/Maps/issues/285 --- README.md | 6 +- .../src/queryprinters/SM_MapPrinter.php | 99 ++++++++++++------- includes/Maps_DisplayMapRenderer.php | 41 ++++++-- includes/Maps_Mapper.php | 2 +- includes/Maps_MappingService.php | 2 + includes/parserhooks/Maps_Coordinates.php | 1 + includes/parserhooks/Maps_Finddestination.php | 4 +- includes/parserhooks/Maps_Geodistance.php | 5 +- 8 files changed, 111 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 1234a26b3..3c1961730 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,8 @@ On [Packagist](https://packagist.org/packages/mediawiki/maps): ### Running the tests +As setup, run `composer install` inside of the Maps root directory. + You can run the MediaWiki independent tests by changing into the Maps root directory and running phpunit @@ -69,11 +71,13 @@ This is possible without having a MediaWiki installation or webserver. A clone o To run the tests with MediaWiki, change into `tests/phpunit` of your MediaWiki installation and run - php phpunit.php --wiki wiki --configuration suite.xml ../../extensions/Maps/tests/ + php phpunit.php --wiki wiki -c ../../extensions/Maps/phpunit.xml.dist Where you either update `wiki` to match your wikis name, or drop the parameter. The above command works without modification if you are using the [MediaWiki Vagrant](https://www.mediawiki.org/wiki/MediaWiki-Vagrant). +Beware that due to severe technical debt, some tests access the network. + ## Links * [Maps examples](https://www.semantic-mediawiki.org/wiki/Maps_examples) diff --git a/SemanticMaps/src/queryprinters/SM_MapPrinter.php b/SemanticMaps/src/queryprinters/SM_MapPrinter.php index 5f04eb3f7..f62f08a00 100644 --- a/SemanticMaps/src/queryprinters/SM_MapPrinter.php +++ b/SemanticMaps/src/queryprinters/SM_MapPrinter.php @@ -3,7 +3,9 @@ use Maps\Elements\Location; use Maps\Element; use Maps\Elements\BaseElement; +use Maps\LocationParser; use ParamProcessor\ParamDefinition; +use ValueParsers\ParserOptions as ValueParserOptions; /** * Query printer for maps. Is invoked via SMMapper. @@ -19,6 +21,11 @@ class SMMapPrinter extends SMW\ResultPrinter { private static $services = []; + /** + * @var LocationParser + */ + private $locationParser; + /** * @since 3.4 * FIXME: this is a temporary hack that should be replaced when SMW allows for dependency @@ -67,7 +74,7 @@ private function getParameterInfo() { $this->service->addParameterInfo( $params ); $params['staticlocations'] = [ - 'type' => 'mapslocation', + 'type' => 'mapslocation', // FIXME: geoservice is not used 'aliases' => [ 'locations', 'points' ], 'default' => [], 'islist' => true, @@ -151,8 +158,10 @@ public final function getResultText( SMWQueryResult $res, $outputmode ) { $params = $this->params; + $this->initializeLocationParser( $params ); + $queryHandler = new SMQueryHandler( $res, $outputmode ); - $queryHandler->setLinkStyle($params['link']); + $queryHandler->setLinkStyle( $params['link'] ); $queryHandler->setHeaderStyle($params['headers']); $queryHandler->setShowSubject( $params['showtitle'] ); $queryHandler->setTemplate( $params['template'] ); @@ -161,10 +170,11 @@ public final function getResultText( SMWQueryResult $res, $outputmode ) { $queryHandler->setActiveIcon( $params['activeicon'] ); $this->handleMarkerData( $params, $queryHandler ); - $locationAmount = count( $params['locations'] ); $params['ajaxquery'] = urlencode( $params['ajaxquery'] ); + $locationAmount = count( $params['locations'] ); + if ( $locationAmount > 0 ) { // We can only take care of the zoom defaulting here, // as not all locations are available in whats passed to Validator. @@ -195,6 +205,58 @@ public final function getResultText( SMWQueryResult $res, $outputmode ) { } } + private function initializeLocationParser( array $params ) { + $this->locationParser = new LocationParser( new ValueParserOptions( [ + 'geoService' => $params['geoservice'] + ] ) ); + } + + /** + * Converts the data in the coordinates parameter to JSON-ready objects. + * These get stored in the locations parameter, and the coordinates on gets deleted. + * + * @param array &$params + * @param SMQueryHandler $queryHandler + */ + private function handleMarkerData( array &$params, SMQueryHandler $queryHandler ) { + $params['centre'] = $this->getCenter( $params['centre'] ); + + $iconUrl = MapsMapper::getFileUrl( $params['icon'] ); + $visitedIconUrl = MapsMapper::getFileUrl( $params['visitedicon'] ); + + $params['locations'] = $this->getJsonForStaticLocations( + $params['staticlocations'], + $params, + $iconUrl, + $visitedIconUrl + ); + + unset( $params['staticlocations'] ); + + $this->addShapeData( $queryHandler->getShapes(), $params, $iconUrl, $visitedIconUrl ); + + if ( $params['format'] === 'openlayers' ) { + $params['layers'] = MapsDisplayMapRenderer::evilOpenLayersHack( $params['layers'] ); + } + } + + private function getCenter( $coordinatesOrAddress ) { + if ( $coordinatesOrAddress === false ) { + return false; + } + + try { + // FIXME: a Location makes no sense here, since the non-coordinate data is not used + $location = $this->locationParser->stringParse( $coordinatesOrAddress ); + } + catch ( \Exception $ex ) { + // TODO: somehow report this to the user + return false; + } + + return $location->getJSONObject(); + } + /** * Returns the HTML to display the map. * @@ -232,37 +294,6 @@ private function getMapHTML( array $params, Parser $parser, $mapName ) { private function getJSONObject( array $params, Parser $parser ) { return $params; } - - /** - * Converts the data in the coordinates parameter to JSON-ready objects. - * These get stored in the locations parameter, and the coordinates on gets deleted. - * - * @param array &$params - * @param SMQueryHandler $queryHandler - */ - private function handleMarkerData( array &$params, SMQueryHandler $queryHandler ) { - if ( is_object( $params['centre'] ) ) { - $params['centre'] = $params['centre']->getJSONObject(); - } - - $iconUrl = MapsMapper::getFileUrl( $params['icon'] ); - $visitedIconUrl = MapsMapper::getFileUrl( $params['visitedicon'] ); - - $params['locations'] = $this->getJsonForStaticLocations( - $params['staticlocations'], - $params, - $iconUrl, - $visitedIconUrl - ); - - unset( $params['staticlocations'] ); - - $this->addShapeData( $queryHandler->getShapes(), $params, $iconUrl, $visitedIconUrl ); - - if ( $params['format'] === 'openlayers' ) { - $params['layers'] = MapsDisplayMapRenderer::evilOpenLayersHack( $params['layers'] ); - } - } private function getJsonForStaticLocations( array $staticLocations, array $params, $iconUrl, $visitedIconUrl ) { /** diff --git a/includes/Maps_DisplayMapRenderer.php b/includes/Maps_DisplayMapRenderer.php index 758a59397..4daa97ed7 100644 --- a/includes/Maps_DisplayMapRenderer.php +++ b/includes/Maps_DisplayMapRenderer.php @@ -2,6 +2,7 @@ use Maps\Element; use Maps\Elements\Location; +use Maps\LocationParser; use ValueParsers\ParserOptions as ValueParserOptions; /** @@ -15,6 +16,11 @@ class MapsDisplayMapRenderer { private $service; + /** + * @var LocationParser + */ + private $locationParser; + public function __construct( MapsMappingService $service ) { $this->service = $service; } @@ -29,6 +35,8 @@ public function __construct( MapsMappingService $service ) { * @return string */ public final function renderMap( array $params, Parser $parser ) { + $this->initializeLocationParser( $params ); + $this->handleMarkerData( $params, $parser ); $mapName = $this->service->getMapId(); @@ -47,6 +55,12 @@ public final function renderMap( array $params, Parser $parser ) { return $output; } + private function initializeLocationParser( array $params ) { + $this->locationParser = new LocationParser( new ValueParserOptions( [ + 'geoService' => $params['geoservice'] + ] ) ); + } + /** * Returns the HTML to display the map. * @@ -77,9 +91,7 @@ protected function getMapHTML( array $params, $mapName ) { * These get stored in the locations parameter, and the coordinates on gets deleted. */ private function handleMarkerData( array &$params, Parser $parser ) { - if ( is_object( $params['centre'] ) ) { - $params['centre'] = $params['centre']->getJSONObject(); - } + $params['centre'] = $this->getCenter( $params['centre'] ); $parserClone = clone $parser; @@ -98,19 +110,32 @@ private function handleMarkerData( array &$params, Parser $parser ) { } } + private function getCenter( $coordinatesOrAddress ) { + if ( $coordinatesOrAddress === false ) { + return false; + } + + try { + // FIXME: a Location makes no sense here, since the non-coordinate data is not used + $location = $this->locationParser->stringParse( $coordinatesOrAddress ); + } + catch ( \Exception $ex ) { + // TODO: somehow report this to the user + return false; + } + + return $location->getJSONObject(); + } + private function getLocationJson( array $params, $parserClone ) { $iconUrl = MapsMapper::getFileUrl( $params['icon'] ); $visitedIconUrl = MapsMapper::getFileUrl( $params['visitedicon'] ); - $parser = new \Maps\LocationParser( new ValueParserOptions( [ - 'geoService' => $params['geoservice'] - ] ) ); - $locationJsonObjects = []; foreach ( $params['coordinates'] as $coordinatesOrAddress ) { try { - $location = $parser->stringParse( $coordinatesOrAddress ); + $location = $this->locationParser->stringParse( $coordinatesOrAddress ); } catch ( \Exception $ex ) { // TODO: somehow report this to the user diff --git a/includes/Maps_Mapper.php b/includes/Maps_Mapper.php index a91f8b23d..e1d9faf23 100644 --- a/includes/Maps_Mapper.php +++ b/includes/Maps_Mapper.php @@ -104,7 +104,7 @@ public static function getCommonParameters() { // TODO$manipulation->toJSONObj = true; $params['centre'] = [ - 'type' => 'mapslocation', + 'type' => 'string', 'aliases' => [ 'center' ], 'default' => false, 'manipulatedefault' => false, diff --git a/includes/Maps_MappingService.php b/includes/Maps_MappingService.php index 4c368e521..6af2943fa 100644 --- a/includes/Maps_MappingService.php +++ b/includes/Maps_MappingService.php @@ -229,4 +229,6 @@ public function getEarthZoom() { return 1; } + public abstract function getMapId( $increment = true ); + } diff --git a/includes/parserhooks/Maps_Coordinates.php b/includes/parserhooks/Maps_Coordinates.php index bb4949dbc..39e4e1fc0 100644 --- a/includes/parserhooks/Maps_Coordinates.php +++ b/includes/parserhooks/Maps_Coordinates.php @@ -1,4 +1,5 @@ */ - class MapsFinddestination extends ParserHook { /** @@ -41,7 +41,7 @@ protected function getParameterInfo( $type ) { $params['location'] = [ 'dependencies' => [ 'mappingservice', 'geoservice' ], - 'type' => 'mapslocation', + 'type' => 'mapslocation', // FIXME: geoservice is not used ]; $params['format'] = [ diff --git a/includes/parserhooks/Maps_Geodistance.php b/includes/parserhooks/Maps_Geodistance.php index 81e493500..f691a0c8c 100644 --- a/includes/parserhooks/Maps_Geodistance.php +++ b/includes/parserhooks/Maps_Geodistance.php @@ -9,7 +9,6 @@ * @licence GNU GPL v2+ * @author Jeroen De Dauw < jeroendedauw@gmail.com > */ - class MapsGeodistance extends ParserHook { /** @@ -61,13 +60,13 @@ protected function getParameterInfo( $type ) { ]; $params['location1'] = [ - 'type' => 'mapslocation', + 'type' => 'mapslocation', // FIXME: geoservice is not used 'aliases' => 'from', 'dependencies' => [ 'mappingservice', 'geoservice' ], ]; $params['location2'] = [ - 'type' => 'mapslocation', + 'type' => 'mapslocation', // FIXME: geoservice is not used 'aliases' => 'to', 'dependencies' => [ 'mappingservice', 'geoservice' ], ];