SlotDiffRenderer: add utility method for parameter type checks

Change-Id: I0161070fd0330d4945cec2f76f4fd8128a9793b9

diff --git a/includes/diff/DifferenceEngineSlotDiffRenderer.php b/includes/diff/DifferenceEngineSlotDiffRenderer.php
index 0c632b9..c389a6f 100644 (file)
--- a/includes/diff/DifferenceEngineSlotDiffRenderer.php
+++ b/includes/diff/DifferenceEngineSlotDiffRenderer.php
@@ -46,15 +46,7 @@ class DifferenceEngineSlotDiffRenderer extends SlotDiffRenderer {
 
        /** @inheritDoc */
        public function getDiff( Content $oldContent = null, Content $newContent = null ) {
-               if ( !$oldContent && !$newContent ) {
-                       throw new InvalidArgumentException( '$oldContent and $newContent cannot both be null' );
-               }
-               if ( !$oldContent || !$newContent ) {
-                       $someContent = $newContent ?: $oldContent;
-                       $emptyContent = $someContent->getContentHandler()->makeEmptyContent();
-                       $oldContent = $oldContent ?: $emptyContent;
-                       $newContent = $newContent ?: $emptyContent;
-               }
+               $this->normalizeContents( $oldContent, $newContent );
                return $this->differenceEngine->generateContentDiffBody( $oldContent, $newContent );
        }
 

diff --git a/includes/diff/SlotDiffRenderer.php b/includes/diff/SlotDiffRenderer.php
index f20b416..b30607f 100644 (file)
--- a/includes/diff/SlotDiffRenderer.php
+++ b/includes/diff/SlotDiffRenderer.php
@@ -20,6 +20,7 @@
  * @file
  * @ingroup DifferenceEngine
  */
+use Wikimedia\Assert\Assert;
 
 /**
  * Renders a diff for a single slot (that is, a diff between two content objects).
@@ -62,4 +63,35 @@ abstract class SlotDiffRenderer {
                return [];
        }
 
+       /**
+        * Helper method to normalize the input of getDiff().
+        * Verifies that at least one of $oldContent and $newContent is not null, verifies that
+        * they are instances of one of the allowed classes (if provided), and replaces null with
+        * empty content.
+        * @param Content|null &$oldContent
+        * @param Content|null &$newContent
+        * @param string|array|null $allowedClasses
+        */
+       protected function normalizeContents(
+               Content &$oldContent = null, Content &$newContent = null, $allowedClasses = null
+       ) {
+               if ( !$oldContent && !$newContent ) {
+                       throw new InvalidArgumentException( '$oldContent and $newContent cannot both be null' );
+               }
+
+               if ( $allowedClasses ) {
+                       if ( is_array( $allowedClasses ) ) {
+                               $allowedClasses = implode( '|', $allowedClasses );
+                       }
+                       Assert::parameterType( $allowedClasses . '|null', $oldContent, '$oldContent' );
+                       Assert::parameterType( $allowedClasses . '|null', $newContent, '$newContent' );
+               }
+
+               if ( !$oldContent ) {
+                       $oldContent = $newContent->getContentHandler()->makeEmptyContent();
+               } elseif ( !$newContent ) {
+                       $newContent = $oldContent->getContentHandler()->makeEmptyContent();
+               }
+       }
+
 }

diff --git a/includes/diff/TextSlotDiffRenderer.php b/includes/diff/TextSlotDiffRenderer.php
index 9c60705..1cc6646 100644 (file)
--- a/includes/diff/TextSlotDiffRenderer.php
+++ b/includes/diff/TextSlotDiffRenderer.php
@@ -116,19 +116,7 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
 
        /** @inheritDoc */
        public function getDiff( Content $oldContent = null, Content $newContent = null ) {
-               if ( !$oldContent && !$newContent ) {
-                       throw new InvalidArgumentException( '$oldContent and $newContent cannot both be null' );
-               } elseif ( $oldContent && !( $oldContent instanceof TextContent ) ) {
-                       throw new InvalidArgumentException( __CLASS__ . ' does not handle ' . get_class( $oldContent ) );
-               } elseif ( $newContent && !( $newContent instanceof TextContent ) ) {
-                       throw new InvalidArgumentException( __CLASS__ . ' does not handle ' . get_class( $newContent ) );
-               }
-
-               if ( !$oldContent ) {
-                       $oldContent = $newContent->getContentHandler()->makeEmptyContent();
-               } elseif ( !$newContent ) {
-                       $newContent = $oldContent->getContentHandler()->makeEmptyContent();
-               }
+               $this->normalizeContents( $oldContent, $newContent, TextContent::class );
 
                $oldText = $oldContent->serialize();
                $newText = $newContent->serialize();

diff --git a/tests/phpunit/includes/diff/SlotDiffRendererTest.php b/tests/phpunit/includes/diff/SlotDiffRendererTest.php
new file mode 100644 (file)
index 0000000..58b433f
--- /dev/null
+++ b/tests/phpunit/includes/diff/SlotDiffRendererTest.php
@@ -0,0 +1,77 @@
+<?php
+use Wikimedia\Assert\ParameterTypeException;
+use Wikimedia\TestingAccessWrapper;
+
+/**
+ * @covers SlotDiffRenderer
+ */
+class SlotDiffRendererTest extends \PHPUnit\Framework\TestCase {
+
+       /**
+        * @dataProvider provideNormalizeContents
+        */
+       public function testNormalizeContents(
+               $oldContent, $newContent, $allowedClasses,
+               $expectedOldContent, $expectedNewContent, $expectedExceptionClass
+       ) {
+               $slotDiffRenderer = $this->getMockBuilder( SlotDiffRenderer::class )
+                       ->getMock();
+               try {
+                       // __call needs help deciding which parameter to take by reference
+                       call_user_func_array( [ TestingAccessWrapper::newFromObject( $slotDiffRenderer ),
+                               'normalizeContents' ], [ &$oldContent, &$newContent, $allowedClasses ] );
+                       $this->assertEquals( $expectedOldContent, $oldContent );
+                       $this->assertEquals( $expectedNewContent, $newContent );
+               } catch ( Exception $e ) {
+                       if ( !$expectedExceptionClass ) {
+                               throw $e;
+                       }
+                       $this->assertInstanceOf( $expectedExceptionClass, $e );
+               }
+       }
+
+       public function provideNormalizeContents() {
+               return [
+                       'both null' => [ null, null, null, null, null, InvalidArgumentException::class ],
+                       'left null' => [
+                               null, new WikitextContent( 'abc' ), null,
+                               new WikitextContent( '' ), new WikitextContent( 'abc' ), null,
+                       ],
+                       'right null' => [
+                               new WikitextContent( 'def' ), null, null,
+                               new WikitextContent( 'def' ), new WikitextContent( '' ), null,
+                       ],
+                       'type filter' => [
+                               new WikitextContent( 'abc' ), new WikitextContent( 'def' ), WikitextContent::class,
+                               new WikitextContent( 'abc' ), new WikitextContent( 'def' ), null,
+                       ],
+                       'type filter (subclass)' => [
+                               new WikitextContent( 'abc' ), new WikitextContent( 'def' ), TextContent::class,
+                               new WikitextContent( 'abc' ), new WikitextContent( 'def' ), null,
+                       ],
+                       'type filter (null)' => [
+                               new WikitextContent( 'abc' ), null, TextContent::class,
+                               new WikitextContent( 'abc' ), new WikitextContent( '' ), null,
+                       ],
+                       'type filter failure (left)' => [
+                               new TextContent( 'abc' ), new WikitextContent( 'def' ), WikitextContent::class,
+                               null, null, ParameterTypeException::class,
+                       ],
+                       'type filter failure (right)' => [
+                               new WikitextContent( 'abc' ), new TextContent( 'def' ), WikitextContent::class,
+                               null, null, ParameterTypeException::class,
+                       ],
+                       'type filter (array syntax)' => [
+                               new WikitextContent( 'abc' ), new JsonContent( 'def' ),
+                               [ JsonContent::class, WikitextContent::class ],
+                               new WikitextContent( 'abc' ), new JsonContent( 'def' ), null,
+                       ],
+                       'type filter failure (array syntax)' => [
+                               new WikitextContent( 'abc' ), new CssContent( 'def' ),
+                               [ JsonContent::class, WikitextContent::class ],
+                               null, null, ParameterTypeException::class,
+                       ],
+               ];
+       }
+
+}

diff --git a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php
index ec45e29..e08efac 100644 (file)
--- a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php
+++ b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php
@@ -1,5 +1,7 @@
 <?php
 
+use Wikimedia\Assert\ParameterTypeException;
+
 /**
  * @covers TextSlotDiffRenderer
  */
@@ -63,12 +65,12 @@ class TextSlotDiffRendererTest extends MediaWikiTestCase {
                        'non-text left content' => [
                                $this->makeContent( '', 'testing-nontext' ),
                                $this->makeContent( "aaa\nbbb\nccc" ),
-                               new InvalidArgumentException( 'TextSlotDiffRenderer does not handle DummyNonTextContent' ),
+                               new ParameterTypeException( '$oldContent', 'TextContent|null' ),
                        ],
                        'non-text right content' => [
                                $this->makeContent( "aaa\nbbb\nccc" ),
                                $this->makeContent( '', 'testing-nontext' ),
-                               new InvalidArgumentException( 'TextSlotDiffRenderer does not handle DummyNonTextContent' ),
+                               new ParameterTypeException( '$newContent', 'TextContent|null' ),
                        ],
                ];
        }