Make CircleParser more robust

Author: JeroenDeDauw

RELEASE-NOTES.md

@@ -10,6 +10,7 @@ Not released yet.
 * Raised the minimum MediaWiki version from 1.35 to 1.40
 * Added compatibility with MediaWiki 1.44
 * Fixed GeoJSON error by ignoring GeoJSON descriptions that are objects
+* Fixed fatal error when providing an invalid radius for circles
 
 ## Maps 11.0.1
 

src/WikitextParsers/CircleParser.php

@@ -37,9 +37,8 @@ public function __construct( $geocoder = null ) {
 	 */
 	public function parse( $value ): Circle {
 		$metaData = explode( $this->metaDataSeparator, $value );
-		$circleData = explode( ':', array_shift( $metaData ) );
 
-		$circle = new Circle( $this->stringToLatLongValue( $circleData[0] ), (float)$circleData[1] );
+		$circle = $this->buildCircle( array_shift( $metaData ) );
 
 		if ( $metaData !== [] ) {
 			$circle->setTitle( array_shift( $metaData ) );
@@ -72,6 +71,27 @@ public function parse( $value ): Circle {
 		return $circle;
 	}
 
+	private function buildCircle( string $circleWikitext ): Circle {
+		$circleData = explode( ':', $circleWikitext );
+
+		return new Circle(
+			$this->stringToLatLongValue( $circleData[0] ),
+			$this->extractRadius( $circleData )
+		);
+	}
+
+	private function extractRadius( array $circleData ): float {
+		if ( array_key_exists( 1, $circleData ) ) {
+			$radius = (float)$circleData[1];
+
+			if ( $radius > 0 ) {
+				return $radius;
+			}
+		}
+
+		return 1;
+	}
+
 	private function stringToLatLongValue( string $location ): LatLongValue {
 		$latLong = $this->geocoder->geocode( $location );
 

tests/Integration/Parsers/CircleParserTest.php

@@ -19,27 +19,41 @@
 class CircleParserTest extends TestCase {
 
 	public function testGivenCoordinateAndRadius_parserReturnsCircle() {
-		$parser = new CircleParser( new CoordinateFriendlyGeocoder( new NullGeocoder() ) );
-
-		$circle = $parser->parse( '57.421,23.90625:32684.605182' );
-
-		$this->assertInstanceOf( Circle::class, $circle );
+		$circle = $this->newCircleParser()->parse( '57.421,23.90625:32684.605182' );
 
 		$expectedLatLong = new LatLongValue( 57.421, 23.90625 );
 		$this->assertTrue( $expectedLatLong->equals( $circle->getCircleCentre() ) );
 
 		$this->assertSame( 32684.605182, $circle->getCircleRadius() );
 	}
 
-	public function testGivenTitleAndText_circleHasProvidedMetaData() {
-		$parser = new CircleParser( new CoordinateFriendlyGeocoder( new NullGeocoder() ) );
-
-		$circle = $parser->parse( '57.421,23.90625:32684.605182~title~text' );
+	private function newCircleParser(): CircleParser {
+		return new CircleParser( new CoordinateFriendlyGeocoder( new NullGeocoder() ) );
+	}
 
-		$this->assertInstanceOf( Circle::class, $circle );
+	public function testGivenTitleAndText_circleHasProvidedMetaData() {
+		$circle = $this->newCircleParser()->parse( '57.421,23.90625:32684.605182~title~text' );
 
 		$this->assertSame( 'title', $circle->getTitle() );
 		$this->assertSame( 'text', $circle->getText() );
 	}
 
+	public function testGivenNoRadius_radiusIsOne() {
+		$circle = $this->newCircleParser()->parse( '42,42' );
+
+		$this->assertSame( 1.0, $circle->getCircleRadius() );
+	}
+
+	public function testGivenNegative_radiusIsOne() {
+		$circle = $this->newCircleParser()->parse( '42,42:-5' );
+
+		$this->assertSame( 1.0, $circle->getCircleRadius() );
+	}
+
+	public function testGivenInvalid_radiusIsOne() {
+		$circle = $this->newCircleParser()->parse( '42,42:foo' );
+
+		$this->assertSame( 1.0, $circle->getCircleRadius() );
+	}
+
 }