jenkins-bot has submitted this change and it was merged.
Change subject: Cache created objects within the mapper
......................................................................
Cache created objects within the mapper
Change-Id: Idc7c496683ecd03ff91d9276374cef738157b094
---
M Flow.php
M container.php
M includes/Data/ObjectManager.php
A tests/CachingObjectMapperTest.php
4 files changed, 155 insertions(+), 18 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Flow.php b/Flow.php
index 7bbd1fc..cba4b95 100755
--- a/Flow.php
+++ b/Flow.php
@@ -125,6 +125,7 @@
$wgAutoloadClasses['Flow\Data\BasicDbStorage'] = $dir .
'includes/Data/ObjectManager.php';
$wgAutoloadClasses['Flow\Data\ObjectMapper'] = $dir .
'includes/Data/ObjectManager.php';
$wgAutoloadClasses['Flow\Data\BasicObjectMapper'] = $dir .
'includes/Data/ObjectManager.php';
+$wgAutoloadClasses['Flow\Data\CachingObjectMapper'] = $dir .
'includes/Data/ObjectManager.php';
$wgAutoloadClasses['Flow\Data\BufferedCache'] = $dir .
'includes/Data/ObjectManager.php';
$wgAutoloadClasses['Flow\Data\LocalBufferedCache'] = $dir .
'includes/Data/ObjectManager.php';
$wgAutoloadClasses['Flow\Data\SortArrayByKeys'] = $dir .
'includes/Data/ObjectManager.php';
diff --git a/container.php b/container.php
index cbec8d8..8e70d2d 100644
--- a/container.php
+++ b/container.php
@@ -113,6 +113,7 @@
use Flow\Data\BufferedCache;
use Flow\Data\LocalBufferedCache;
use Flow\Data\BasicObjectMapper;
+use Flow\Data\CachingObjectMapper;
use Flow\Data\BasicDbStorage;
use Flow\Data\PostRevisionStorage;
use Flow\Data\HeaderRevisionStorage;
@@ -140,16 +141,17 @@
} );
// Per wiki workflow definitions (types of workflows)
$c['storage.definition'] = $c->share( function( $c ) {
+ $primaryKey = array( 'definition_id' );
$cache = $c['memcache.buffered'];
- $mapper = BasicObjectMapper::model( 'Flow\\Model\\Definition' );
+ $mapper = CachingObjectMapper::model( 'Flow\\Model\\Definition',
$primaryKey );
$storage = new BasicDbStorage(
// factory and table
$c['db.factory'], 'flow_definition',
// pk
- array( 'definition_id' )
+ $primaryKey
);
$indexes = array(
- new UniqueFeatureIndex( $cache, $storage, 'flow_definition:pk',
array( 'definition_id' ) ),
+ new UniqueFeatureIndex( $cache, $storage, 'flow_definition:pk',
$primaryKey ),
new UniqueFeatureIndex( $cache, $storage,
'flow_definition:name', array( 'definition_wiki', 'definition_name' ) ),
);
@@ -157,15 +159,16 @@
} );
// Individual workflow instances
$c['storage.workflow'] = $c->share( function( $c ) {
+ $primaryKey = array( 'workflow_id' );
$cache = $c['memcache.buffered'];
- $mapper = BasicObjectMapper::model( 'Flow\\Model\\Workflow' );
+ $mapper = CachingObjectMapper::model( 'Flow\\Model\\Workflow',
$primaryKey );
$storage = new BasicDbStorage(
// factory and table
$c['db.factory'], 'flow_workflow',
// pk
- array( 'workflow_id' )
+ $primaryKey
);
- $pk = new UniqueFeatureIndex( $cache, $storage, 'flow_workflow:pk',
array( 'workflow_id' ) );
+ $pk = new UniqueFeatureIndex( $cache, $storage, 'flow_workflow:pk',
$primaryKey );
$indexes = array(
$pk,
// This is actually a unique index, but it wants the shallow
functionality.
@@ -203,14 +206,20 @@
$c['storage.board_history'] = $c->share( function( $c ) {
$cache = $c['memcache.buffered'];
$mapper = new BasicObjectMapper(
- function( $rev ) {
- return $rev->toStorageRow( $rev );
+ function( $rev ) use( $c ) {
+ if ( $rev instanceof PostRevision ) {
+ return $c['storage.post.mapper']->toStorageRow(
$rev );
+ } elseif ( $rev instanceof Header ) {
+ return
$c['storage.header.mapper']->toStorageRow( $rev );
+ } else {
+ throw new \Flow\Exception\InvalidDataException(
'Invalid class for board history entry: ' . get_class( $rev ), 'fail-load-data'
);
+ }
},
- function( array $row, $obj = null ) {
+ function( array $row, $obj = null ) use( $c ) {
if ( $row['rev_type'] === 'header' ) {
- return Header::fromStorageRow( $row, $obj );
+ return
$c['storage.header.mapper']->fromStorageRow( $row, $obj );
} elseif ( $row['rev_type'] === 'post' ) {
- return PostRevision::fromStorageRow( $row, $obj
);
+ return
$c['storage.post.mapper']->fromStorageRow( $row, $obj );
} else {
throw new \Flow\Exception\InvalidDataException(
'Invalid rev_type for board history entry: ' . $row['rev_type'],
'fail-load-data' );
}
@@ -244,11 +253,13 @@
),
);
} );
+$c['storage.header.mapper'] = $c->share( function( $c ) {
+ return CachingObjectMapper::model( 'Flow\\Model\\Header', array(
'rev_id' ) );
+} );
$c['storage.header'] = $c->share( function( $c ) {
global $wgFlowExternalStore;
$cache = $c['memcache.buffered'];
- $mapper = BasicObjectMapper::model( 'Flow\\Model\\Header' );
$storage = new HeaderRevisionStorage( $c['db.factory'],
$wgFlowExternalStore );
$pk = new UniqueFeatureIndex(
@@ -272,7 +283,7 @@
),
);
- return new ObjectManager( $mapper, $storage, $indexes,
$c['storage.header.lifecycle-handlers'] );
+ return new ObjectManager( $c['storage.header.mapper'], $storage,
$indexes, $c['storage.header.lifecycle-handlers'] );
} );
// List of topic workflows and their owning discussion workflow
@@ -281,13 +292,14 @@
// Would also need object mapper adjustments to return array
// of two objects.
$c['storage.topic_list'] = $c->share( function( $c ) {
+ $primaryKey = array( 'topic_list_id', 'topic_id' );
$cache = $c['memcache.buffered'];
- $mapper = BasicObjectMapper::model( 'Flow\\Model\\TopicListEntry' );
+ $mapper = CachingObjectMapper::model( 'Flow\\Model\\TopicListEntry',
$primaryKey );
$storage = new BasicDbStorage(
// factory and table
$c['db.factory'], 'flow_topic_list',
// pk
- array( 'topic_list_id', 'topic_id' )
+ $primaryKey
);
$indexes = array(
new TopKIndex(
@@ -322,12 +334,13 @@
$c['collection.cache'],
);
} );
-
+$c['storage.post.mapper'] = $c->share( function( $c ) {
+ return CachingObjectMapper::model( 'Flow\\Model\\PostRevision', array(
'rev_id' ) );
+} );
$c['storage.post'] = $c->share( function( $c ) {
global $wgFlowExternalStore;
$cache = $c['memcache.buffered'];
$treeRepo = $c['repository.tree'];
- $mapper = BasicObjectMapper::model( 'Flow\\Model\\PostRevision' );
$storage = new PostRevisionStorage( $c['db.factory'],
$wgFlowExternalStore, $treeRepo );
$pk = new UniqueFeatureIndex( $cache, $storage, 'flow_revision:v4:pk',
array( 'rev_id' ) );
$indexes = array(
@@ -364,7 +377,7 @@
) ),
);
- return new ObjectManager( $mapper, $storage, $indexes,
$c['storage.post.lifecycle-handlers'] );
+ return new ObjectManager( $c['storage.post.mapper'], $storage,
$indexes, $c['storage.post.lifecycle-handlers'] );
} );
// Storage implementation for user subscriptions, separate from
storage.user_subs so it
// can be used in storage.user_subs.user_index as well.
diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php
index 761a5e1..787a611 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -724,6 +724,11 @@
* );
*/
class BasicObjectMapper implements ObjectMapper {
+
+ protected $toStorageRow;
+
+ protected $fromStorageRow;
+
public function __construct( $toStorageRow, $fromStorageRow ) {
$this->toStorageRow = $toStorageRow;
$this->fromStorageRow = $fromStorageRow;
@@ -736,11 +741,98 @@
public function toStorageRow( $object ) {
return call_user_func( $this->toStorageRow, $object );
}
+
public function fromStorageRow( array $row, $object = null ) {
return call_user_func( $this->fromStorageRow, $row, $object );
}
}
+class CachingObjectMapper implements ObjectMapper {
+
+ protected $toStorageRow;
+
+ protected $fromStorageRow;
+
+ protected $loaded;
+
+ public function __construct( $toStorageRow, $fromStorageRow, array
$primaryKey ) {
+ $this->toStorageRow = $toStorageRow;
+ $this->fromStorageRow = $fromStorageRow;
+ ksort( $primaryKey );
+ $this->primaryKey = $primaryKey;
+ $this->clear();
+ }
+
+ static public function model( $className, array $primaryKey ) {
+ return new self(
+ array( $className, 'toStorageRow' ),
+ array( $className, 'fromStorageRow' ),
+ $primaryKey
+ );
+ }
+
+ public function toStorageRow( $object ) {
+ $row = call_user_func( $this->toStorageRow, $object );
+ $pk = ObjectManager::splitFromRow( $row, $this->primaryKey );
+ if ( $pk === null ) {
+ // new object may not have pk yet, calling code
+ // should call self::fromStorageRow with $object to load
+ // db assigned pk and store obj in $this->loaded
+ } elseif ( !isset( $this->loaded[$pk] ) ) {
+ // first time this id has been seen
+ $this->loaded[$pk] = $object;
+ } elseif ( $this->loaded[$pk] !== $object ) {
+ // loaded object of this id is not same object
+ $class = get_class( $object );
+ $id = json_encode( $pk );
+ throw new \InvalidArgumentException( "Duplicate
'$class' objects for id $id" );
+ }
+ return $row;
+ }
+
+ public function fromStorageRow( array $row, $object = null ) {
+ $pk = ObjectManager::splitFromRow( $row, $this->primaryKey );
+ if ( $pk === null ) {
+ throw new \InvalidArgumentException( 'Storage row has
no pk' );
+ } elseif ( !isset( $this->loaded[$pk] ) ) {
+ // unserialize the object
+ return $this->loaded[$pk] = call_user_func(
$this->fromStorageRow, $row, $object );
+ } elseif ( $object === null ) {
+ // provide previously loaded object
+ return $this->loaded[$pk];
+ } elseif ( $object !== $this->loaded[$pk] ) {
+ // loaded object of this id is not same object
+ $class = get_class( $object );
+ $id = json_encode( $pk );
+ throw new \InvalidArgumentException( "Duplicate
'$class' objects for id $id" );
+ } else {
+ // object was provided, load $row into $object
+ // we already know $this->loaded[$pk] === $object
+ return call_user_func( $this->fromStorageRow, $row,
$object );
+ }
+ }
+
+ /**
+ * @param array|string $pk
+ * @return object
+ * @throws \InvalidArgumentException
+ * @throws \OutOfBoundsException
+ */
+ public function get( $pk ) {
+ $pk = (array)$pk;
+ ksort( $pk );
+ if ( array_keys( $pk ) !== $this->primaryKey ) {
+ throw new \InvalidArgumentException;
+ }
+ return $this->loaded[$pk];
+ }
+
+ public function clear() {
+ $this->loaded = new MultiDimArray;
+ }
+}
+
+
/**
* Base class for all WritableObjectStorage implementers
* which use a database as the backing store.
diff --git a/tests/CachingObjectMapperTest.php
b/tests/CachingObjectMapperTest.php
new file mode 100644
index 0000000..ebdef84
--- /dev/null
+++ b/tests/CachingObjectMapperTest.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace Flow\Tests;
+
+use Flow\Data\CachingObjectMapper;
+
+class CachingObjectManagerTest extends \MediaWikiTestCase {
+
+ public function testReturnsSameObject() {
+ $mapper = $this->createMapper();
+ $object = $mapper->fromStorageRow( array( 'id' => 1 ) );
+ $this->assertSame( $object, $mapper->fromStorageRow( array(
'id' => 1 ) ) );
+ }
+
+ public function testAllowsNullPkOnPut() {
+ $this->createMapper()->toStorageRow( (object)array( 'id' =>
null ) );
+ $this->assertTrue( true );
+ }
+
+ protected function createMapper() {
+ $toStorageRow = function( $object ) { return (array)$object; };
+ $fromStorageRow = function( array $row, $object ) {
+ if ( $object === null ) {
+ return (object)$row;
+ } else {
+ return (object)( $row + (array)$object );
+ }
+ };
+ return new CachingObjectMapper( $toStorageRow, $fromStorageRow,
array( 'id' ) );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/113527
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idc7c496683ecd03ff91d9276374cef738157b094
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Bsitu <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
