Thiemo Mättig (WMDE) has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/341837 )

Change subject: Deprecate ChangeRow::getField( 'info' ) in favor of getInfo
......................................................................

Deprecate ChangeRow::getField( 'info' ) in favor of getInfo

Note that I will make getInfo private again. Having it public is only
an intermediate step. What I'm doing in this patch is to narrow
interfaces. The goal is to get rid of unspecific getFields() and such.

Change-Id: Ie4ca005cf861b6b7957b2d15e660345fc0425cf9
---
M client/includes/Changes/ChangeHandler.php
M client/includes/Changes/ChangeRunCoalescer.php
M client/includes/RecentChanges/RecentChangeFactory.php
M lib/includes/Changes/ChangeRow.php
M lib/includes/Changes/DiffChange.php
M lib/includes/Changes/EntityChange.php
M lib/tests/phpunit/Changes/ChangeRowTest.php
M lib/tests/phpunit/Changes/TestChanges.php
M repo/includes/Store/Sql/SqlChangeStore.php
9 files changed, 32 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/37/341837/1

diff --git a/client/includes/Changes/ChangeHandler.php 
b/client/includes/Changes/ChangeHandler.php
index 80c24d1..923a916 100644
--- a/client/includes/Changes/ChangeHandler.php
+++ b/client/includes/Changes/ChangeHandler.php
@@ -289,11 +289,10 @@
         */
        private function getChangeIdForLog( Change $change ) {
                if ( $change instanceof EntityChange ) {
-                       //@todo: add getFields() to the Change interface, or 
provide getters!
-                       $fields = $change->getFields();
+                       $info = $change->getInfo();
 
-                       if ( isset( $fields['info']['change-ids'] ) ) {
-                               return implode( '|', 
$fields['info']['change-ids'] );
+                       if ( isset( $info['change-ids'] ) ) {
+                               return implode( '|', $info['change-ids'] );
                        }
                }
 
diff --git a/client/includes/Changes/ChangeRunCoalescer.php 
b/client/includes/Changes/ChangeRunCoalescer.php
index 5195404..b6df856 100644
--- a/client/includes/Changes/ChangeRunCoalescer.php
+++ b/client/includes/Changes/ChangeRunCoalescer.php
@@ -187,7 +187,7 @@
                //FIXME: size before & size after
                ) );
 
-               $info = $change->hasField( 'info' ) ? $change->getField( 'info' 
) : array();
+               $info = $change->getInfo();
                $info['change-ids'] = $ids;
                $info['changes'] = $changes;
                $change->setField( 'info', $info );
diff --git a/client/includes/RecentChanges/RecentChangeFactory.php 
b/client/includes/RecentChanges/RecentChangeFactory.php
index 2432acd..b9c3e6a 100644
--- a/client/includes/RecentChanges/RecentChangeFactory.php
+++ b/client/includes/RecentChanges/RecentChangeFactory.php
@@ -83,8 +83,9 @@
                $fields = $change->getFields();
                $fields['entity_type'] = 
$change->getEntityId()->getEntityType();
 
-               if ( isset( $fields['info']['changes'] ) ) {
-                       $changesForComment = $fields['info']['changes'];
+               $info = $change->getInfo();
+               if ( isset( $info['changes'] ) ) {
+                       $changesForComment = $info['changes'];
                } else {
                        $changesForComment = array( $change );
                }
@@ -178,10 +179,10 @@
                        return null;
                }
 
-               $fields = $change->getFields();
+               $info = $change->getInfo();
 
-               if ( isset( $fields['info']['changes'] ) ) {
-                       $changesForComment = $fields['info']['changes'];
+               if ( isset( $info['changes'] ) ) {
+                       $changesForComment = $info['changes'];
                } else {
                        $changesForComment = array( $change );
                }
diff --git a/lib/includes/Changes/ChangeRow.php 
b/lib/includes/Changes/ChangeRow.php
index 211abfb..9578928 100644
--- a/lib/includes/Changes/ChangeRow.php
+++ b/lib/includes/Changes/ChangeRow.php
@@ -53,25 +53,21 @@
        }
 
        /**
-        * Overwritten to unserialize the info field on the fly.
-        *
-        * @param string $name Field name
+        * @param string $name
         *
         * @throws MWException
         * @return mixed
         */
        public function getField( $name ) {
-               if ( $this->hasField( $name ) ) {
-                       $value = $this->fields[$name];
-               } else {
+               if ( !$this->hasField( $name ) ) {
                        throw new MWException( 'Attempted to get not-set field 
' . $name );
                }
 
-               if ( $name === 'info' && is_string( $value ) ) {
-                       $value = $this->unserializeInfo( $value );
+               if ( $name === 'info' ) {
+                       throw new MWException( 'Use getInfo instead' );
                }
 
-               return $value;
+               return $this->fields[$name];
        }
 
        /**
@@ -90,7 +86,7 @@
        }
 
        /**
-        * Returns the info array. The array is deserialized on the fly by 
getField().
+        * Returns the info array. The array is deserialized on the fly.
         * If $cache is set to 'cache', the deserialized version is stored for
         * later re-use.
         *
@@ -103,11 +99,15 @@
         *
         * @return array
         */
-       protected function getInfo( $cache = 'no' ) {
-               $info = $this->getField( 'info' );
+       public function getInfo( $cache = 'no' ) {
+               $info = $this->hasField( 'info' ) ? $this->getField( 'info' ) : 
[];
 
-               if ( $cache === 'cache' ) {
-                       $this->setField( 'info', $info );
+               if ( is_string( $info ) ) {
+                       $info = $this->unserializeInfo( $info );
+
+                       if ( $cache === 'cache' ) {
+                               $this->setField( 'info', $info );
+                       }
                }
 
                return $info;
diff --git a/lib/includes/Changes/DiffChange.php 
b/lib/includes/Changes/DiffChange.php
index 05184b7..e9f30a7 100644
--- a/lib/includes/Changes/DiffChange.php
+++ b/lib/includes/Changes/DiffChange.php
@@ -35,7 +35,7 @@
         * @param Diff $diff
         */
        public function setDiff( Diff $diff ) {
-               $info = $this->hasField( 'info' ) ? $this->getField( 'info' ) : 
array();
+               $info = $this->getInfo();
                $info['diff'] = $diff;
                $this->setField( 'info', $info );
        }
diff --git a/lib/includes/Changes/EntityChange.php 
b/lib/includes/Changes/EntityChange.php
index 9561dd9..322f5c1 100644
--- a/lib/includes/Changes/EntityChange.php
+++ b/lib/includes/Changes/EntityChange.php
@@ -136,7 +136,7 @@
                        $metadata['comment'] = $this->getComment();
                }
 
-               $info = $this->hasField( 'info' ) ? $this->getField( 'info' ) : 
array();
+               $info = $this->getInfo();
                $info['metadata'] = $metadata;
                $this->setField( 'info', $info );
        }
@@ -250,7 +250,7 @@
                $string .= ': ';
 
                $fields = $this->getFields();
-               $info = $this->hasField( 'info' ) ? $this->getField( 'info' ) : 
array();
+               $info = $this->getInfo();
                $meta = $this->getMetadata();
 
                if ( is_array( $info ) ) {
diff --git a/lib/tests/phpunit/Changes/ChangeRowTest.php 
b/lib/tests/phpunit/Changes/ChangeRowTest.php
index 17e0137..6e07f64 100644
--- a/lib/tests/phpunit/Changes/ChangeRowTest.php
+++ b/lib/tests/phpunit/Changes/ChangeRowTest.php
@@ -65,11 +65,11 @@
                $change->getField( 'field' );
        }
 
-       public function testGetFieldUnserializesInfo() {
+       public function testGetInfoUnserializesInfo() {
                $json = '{"field":"value"}';
                $expected = array( 'field' => 'value' );
                $change = $this->newChangeRow( [ 'info' => $json ] );
-               $this->assertSame( $expected, $change->getField( 'info' ) );
+               $this->assertSame( $expected, $change->getInfo() );
        }
 
        public function testReturnsFields() {
diff --git a/lib/tests/phpunit/Changes/TestChanges.php 
b/lib/tests/phpunit/Changes/TestChanges.php
index 96795d3..5222de7 100644
--- a/lib/tests/phpunit/Changes/TestChanges.php
+++ b/lib/tests/phpunit/Changes/TestChanges.php
@@ -206,7 +206,7 @@
                        /* @var ChangeRow $change */
                        foreach ( $changes as $change ) {
                                if ( $change->hasField( 'info' ) ) {
-                                       $info = $change->getField( 'info' );
+                                       $info = $change->getInfo();
 
                                        $info = array_intersect_key( $info, 
$infoFilter );
 
diff --git a/repo/includes/Store/Sql/SqlChangeStore.php 
b/repo/includes/Store/Sql/SqlChangeStore.php
index 0fa3ed1..dd27fc6 100644
--- a/repo/includes/Store/Sql/SqlChangeStore.php
+++ b/repo/includes/Store/Sql/SqlChangeStore.php
@@ -79,6 +79,7 @@
         */
        private function getValues( ChangeRow $change ) {
                $fields = $change->getFields();
+               $info = $change->serializeInfo( $change->getInfo() );
 
                return array(
                        'change_type' => $fields['type'],
@@ -86,7 +87,7 @@
                        'change_object_id' => isset( $fields['object_id'] ) ? 
$fields['object_id'] : '',
                        'change_revision_id' => isset( $fields['revision_id'] ) 
? $fields['revision_id'] : '0',
                        'change_user_id' => isset( $fields['user_id'] ) ? 
$fields['user_id'] : '0',
-                       'change_info' => $change->serializeInfo( 
$fields['info'] )
+                       'change_info' => $info,
                );
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/341837
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie4ca005cf861b6b7957b2d15e660345fc0425cf9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to