EBernhardson (WMF) has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/78186


Change subject: various updates
......................................................................

various updates

Rename AbstractIndex -> FeatureIndex
Rename UniqueIndex -> UniqueFeatureIndex
Rename SecondaryIndex -> TopKIndex

Initial work pulling in visualeditor JS
Add render method to Block interface
Remove some now-irrelivant commenting
ObjectStorage interface now extends from IteratorAggregate, various arn't fully
implementing it yet and throw exceptions when called.
More consistent error results,  use null instead of false for consistency

Rename lifecycle events, onPostLoad -> onAfterLoad and all similar, to
prevent confusion between Post's to a topic and events that happen after
something

Change-Id: If85dbc1a9c8558e4ec0f5b8c2030f3e08b5dc399
---
M Flow.php
M container.php
M includes/Block/Block.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
M includes/Data/MultiDimArray.php
M includes/Data/ObjectManager.php
M includes/Data/RevisionStorage.php
M includes/Model/AbstractRevision.php
M includes/Model/PostRevision.php
M includes/Model/Summary.php
M includes/Model/Workflow.php
M includes/Repository/SelectQueryBuilder.php
M includes/Templating.php
M includes/UrlGenerator.php
A modules/base/ext.flow.base.js
M special/SpecialFlow.php
M vendor/Pimple.php
18 files changed, 246 insertions(+), 204 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow 
refs/changes/86/78186/1

diff --git a/Flow.php b/Flow.php
index e660276..182b860 100755
--- a/Flow.php
+++ b/Flow.php
@@ -89,13 +89,6 @@
 $wgAutoloadClasses['Flow\Data\PostRevisionStorage'] = $dir . 
'includes/Data/RevisionStorage.php';
 $wgAutoloadClasses['Flow\Data\SummaryRevisionStorage'] = $dir . 
'includes/Data/RevisionStorage.php';
 
-// Classes that control user workflows
-$wgAutoloadClasses['Flow\Controller\Controller'] = $dir . 
'includes/Controller/Controller.php';
-$wgAutoloadClasses['Flow\Controller\DefaultController'] = $dir . 
'includes/Controller/DefaultController.php';
-$wgAutoloadClasses['Flow\Controller\SubscriptionController'] = $dir . 
'includes/Controller/SubscriptionController.php';
-$wgAutoloadClasses['Flow\Controller\DiscussionController'] = $dir . 
'includes/Controller/DiscussionController.php';
-$wgAutoloadClasses['Flow\Controller\TopicController'] = $dir . 
'includes/Controller/TopicController.php';
-
 // The individual workflow pieces
 $wgAutoloadClasses['Flow\Block\Block'] = $dir . 'includes/Block/Block.php';
 $wgAutoloadClasses['Flow\Block\AbstractBlock'] = $dir . 
'includes/Block/Block.php';
@@ -131,6 +124,7 @@
                'styles' => 'base/ext.flow.base.css',
                'scripts' => 'base/ext.flow.base.js',
                'dependencies' => array(
+                       'ext.visualEditor.standalone',
                ),
                'messages' => array(
                ),
diff --git a/container.php b/container.php
index b0252ab..5f15341 100644
--- a/container.php
+++ b/container.php
@@ -49,8 +49,8 @@
 use Flow\Data\BasicDbStorage;
 use Flow\Data\PostRevisionStorage;
 use Flow\Data\SummaryRevisionStorage;
-use Flow\Data\UniqueIndex;
-use Flow\Data\SecondaryIndex;
+use Flow\Data\UniqueFeatureIndex;
+use Flow\Data\TopKIndex;
 use Flow\Data\ObjectMapper;
 use Flow\Data\ObjectManager;
 
@@ -71,8 +71,8 @@
                array( 'definition_id' )
        );
        $indexes = array(
-               new UniqueIndex( $cache, $storage, 'flow_definition:pk', array( 
'definition_id' ) ),
-               new UniqueIndex( $cache, $storage, 'flow_definition:name', 
array( 'definition_wiki', 'definition_name' ) ),
+               new UniqueFeatureIndex( $cache, $storage, 'flow_definition:pk', 
array( 'definition_id' ) ),
+               new UniqueFeatureIndex( $cache, $storage, 
'flow_definition:name', array( 'definition_wiki', 'definition_name' ) ),
        );
 
        return new ObjectManager( $mapper, $storage, $indexes );
@@ -87,56 +87,51 @@
                // pk
                array( 'workflow_id' )
        );
-       $pk = new UniqueIndex( $cache, $storage, 'flow_workflow:pk', array( 
'workflow_id' ) );
+       $pk = new UniqueFeatureIndex( $cache, $storage, 'flow_workflow:pk', 
array( 'workflow_id' ) );
        $indexes = array(
                $pk,
                // This is actually a unique index, but it wants the shallow 
functionality.
-               new SecondaryIndex(
+               new TopKIndex(
                        $cache, $storage, 'flow_workflow:title',
                        array( 'workflow_wiki', 'workflow_page_id', 
'workflow_definition_id' ),
                        array( 'shallow' => $pk, 'limit' => 1, 'sort' => 
'workflow_id' )
                ),
        );
-       // not ready yet
+       //ch /valulds t not ready yet
        $lifecycle = array(); // $c['storage.user_subs.user_index'] );
 
        return new ObjectManager( $mapper, $storage, $indexes, $lifecycle );
 } );
 // Arbitrary bit of revisioned wiki-text attached to a workflow
 $c['storage.summary'] = $c->share( function( $c ) {
+       global $wgFlowExternalStore;
+
        $cache = $c['memcache.buffered'];
        $mapper = BasicObjectMapper::model( 'Flow\\Model\\Summary' );
-       $storage = new SummaryRevisionStorage( $c['db.factory'] );
+       $storage = new SummaryRevisionStorage( $c['db.factory'], 
$wgFlowExternalStore );
+
+       $workflowIndexOptions = array(
+               'sort' => 'rev_id',
+               'order' => 'DESC',
+               //'shallow' => array( 'rev_id' ),
+               'create' => function( array $row ) {
+                       return $row['rev_parent_id'] === null;
+               },
+       );
        $indexes = array(
-               new UniqueIndex(
+               new UniqueFeatureIndex(
                        $cache, $storage,
                        'flow_summary:pk', array( 'rev_id' )
                ),
-               new SecondaryIndex(
+               new TopKIndex(
                        $cache, $storage,
                        'flow_summary:workflow', array( 'summary_workflow_id' ),
-                       array(
-                               'limit'  => 100,
-                               'sort' => 'rev_id',
-                               'order' => 'DESC',
-                               //'shallow' => array( 'rev_id' ),
-                               'create' => function( array $row ) {
-                                       return $row['rev_parent_id'] === null;
-                               },
-                       )
+                       array( 'limit' => 100 ) + $workflowIndexOptions
                ),
-               new SecondaryIndex(
+               new TopKIndex(
                        $cache, $storage,
                        'flow_summary:latest', array( 'summary_workflow_id' ),
-                       array(
-                               'limit'  => 1,
-                               'sort' => 'rev_id',
-                               'order' => 'DESC',
-                               //'shallow' => array( 'rev_id' ),
-                               'create' => function( array $row ) {
-                                       return $row['rev_parent_id'] === null;
-                               },
-                       )
+                       array( 'limit'  => 1 ) + $workflowIndexOptions
                ),
        );
 
@@ -154,12 +149,12 @@
                array( 'topic_list_id', 'topic_id' )
        );
        $indexes = array(
-               new SecondaryIndex(
+               new TopKIndex(
                        $cache, $storage,
                        'flow_topic_list:list', array( 'topic_list_id' ),
                        array( 'sort' => 'topic_id' )
                ),
-               new UniqueIndex(
+               new UniqueFeatureIndex(
                        $cache, $storage,
                        'flow_topic_list:topic', array( 'topic_id' )
                ),
@@ -169,13 +164,14 @@
 } );
 // Individual post within a topic workflow
 $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'], $treeRepo );
+       $storage = new PostRevisionStorage( $c['db.factory'], 
$wgFlowExternalStore, $treeRepo );
        $indexes = array(
-               new UniqueIndex( $cache, $storage, 'flow_revision:pk', array( 
'rev_id' ) ),
-               new SecondaryIndex( $cache, $storage, 
'flow_revision:descendant',
+               new UniqueFeatureIndex( $cache, $storage, 'flow_revision:pk', 
array( 'rev_id' ) ),
+               new TopKIndex( $cache, $storage, 'flow_revision:descendant',
                        array( 'tree_rev_descendant' ),
                        array(
                                'limit' => 100,
@@ -187,7 +183,7 @@
                                        return $row['rev_parent_id'] === null;
                                },
                ) ),
-               new SecondaryIndex( $cache, $storage, 
'flow_revision:latest_post',
+               new TopKIndex( $cache, $storage, 'flow_revision:latest_post',
                        array( 'tree_rev_descendant' ),
                        array(
                                'limit' => 1,
@@ -205,7 +201,6 @@
 // Storage implementation for user subscriptions, separate from 
storage.user_subs so it
 // can be used in storage.user_subs.user_index as well.
 $c['storage.user_subs.backing'] = $c->share( function( $c ) {
-
        return new BasicDbStorage(
                // factory and table
                $c['db.factory'], 'flow_user_sub',
@@ -215,12 +210,12 @@
 } );
 // Needs to be separate from storage.user_subs so it can be attached to 
Workflow updates
 // Limits users to 2000 subscriptions
-// TODO: Can't use SecondaryIndex, needs to be custom. so it stores the right 
data
+// TODO: Can't use TopKIndex, needs to be custom. so it stores the right data
 // TODO: Storage wont work either, it
 $c['storage.user_subs.user_index'] = $c->share( function( $c ) {
        $cache = $c['memcache.buffered'];
        $storage = $c['storage.user_subs.backing'];
-       return new SecondaryIndex(
+       return new TopKIndex(
                $cache, $storage, 'flow_user_sub:user', array( 
'subscrip_user_id' ),
                array( 'limit' => 2000, 'sort' => 'subscrip_last_updated' )
        );
@@ -243,12 +238,16 @@
                array(
                        'Flow\\Model\\Definition' => 'storage.definition',
                        'Definition' => 'storage.definition',
+
                        'Flow\\Model\\Workflow' => 'storage.workflow',
                        'Workflow' => 'storage.workflow',
+
                        'Flow\\Model\\PostRevision' => 'storage.post',
                        'PostRevision' => 'storage.post',
+
                        'Flow\\Model\\TopicListEntry' => 'storage.topic_list',
                        'TopicListEntry' => 'storage.topic_list',
+
                        'Flow\\Model\\Summary' => 'storage.summary',
                        'Summary' => 'storage.summary',
                )
diff --git a/includes/Block/Block.php b/includes/Block/Block.php
index bf8adc0..4605606 100644
--- a/includes/Block/Block.php
+++ b/includes/Block/Block.php
@@ -22,6 +22,12 @@
        function commit();
 
        /**
+        * Load whatever is necessary for rendering an use $templating to
+        * render it.
+        */
+       function render( Templating $templating, array $options );
+
+       /**
         * @return string Unique name among all blocks on an object
         */
        function getName();
@@ -52,7 +58,6 @@
        }
 
        public function onSubmit( $action, User $user, array $data  ) {
-               // Only one action currently
                if ( false === array_search( $action, $this->supportedActions ) 
) {
                        return null;
                }
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index f373aad..9c0f18a 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -196,6 +196,9 @@
                                'history' => $history,
                        ), $return );
                }
+
+               $templating->getOutput()->addModules( 'ext.flow.base' );
+
                return $templating->render( "flow:topic.html.php", array(
                        'block' => $this,
                        'topic' => $this->workflow,
@@ -208,9 +211,6 @@
                        return $this->root;
                }
                return $this->root = $this->rootLoader->get( 
$this->workflow->getId() );
-       }
-
-       protected function loadPostHistory() {
        }
 
        // Somehow the template has to know which post the errors go with
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index e90a538..504523c 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -45,7 +45,8 @@
                }
 
                $topicWorkflow = Workflow::create( $topicDef, $this->user, 
$this->workflow->getArticleTitle() );
-               // Should we really have a top level post for the topic title?
+               // Should we really have a top level post for the topic title?  
Simplifies allowing
+               // a revisioned title.
                $topicPost = PostRevision::create( $topicWorkflow, 
$this->submitted['topic'] );
                $firstPost = $topicPost->reply( $this->user, 
$this->submitted['content'] );
                $topicListEntry = TopicListEntry::create( $this->workflow, 
$topicWorkflow );
@@ -57,23 +58,11 @@
        }
 
        public function render( Templating $templating, array $options ) {
-               $topics = array();
-
                // New workflows cant have content yet
-               if ( !$this->workflow->isNew() ) {
-                       $topicList = $this->storage->find( 'TopicListEntry', 
array(
-                               'topic_list_id' => $this->workflow->getId(),
-                       ) );
-                       if ( $topicList ) {
-                               foreach( $topicList as $entry ) {
-                                       $topicIds[] = $entry->getId();
-                               }
-                               $roots = $this->rootLoader->getMulti( $topicIds 
);
-                               foreach ( $this->storage->getMulti( 'Workflow', 
$topicIds ) as $workflow ) {
-                                       $id = $workflow->getId();
-                                       $topics[$id->getHex()] = new 
TopicBlock( $workflow, $this->storage, $roots[$id->getHex()] );
-                               }
-                       }
+               if ( $this->workflow->isNew() ) {
+                       $topics = array();
+               } else {
+                       $topics = $this->loadAllRelatedTopics();
                }
 
                $templating->render( "flow:topiclist.html.php", array(
@@ -85,5 +74,26 @@
        public function getName() {
                return 'topic_list';
        }
+
+       protected function loadAllRelatedTopics() {
+               $found = $this->storage->find( 'TopicListEntry', array(
+                       'topic_list_id' => $this->workflow->getId(),
+               ) );
+               if ( !$found ) {
+                       return array();
+               }
+
+               $topics = array();
+               foreach( $found as $entry ) {
+                       $topicIds[] = $entry->getId();
+               }
+               $roots = $this->rootLoader->getMulti( $topicIds );
+               foreach ( $this->storage->getMulti( 'Workflow', $topicIds ) as 
$workflow ) {
+                       $hexId = $workflow->getId()->getHex();
+                       $topics[$hexId] = new TopicBlock( $workflow, 
$this->storage, $roots[$hexId] );
+               }
+
+               return $topics;
+       }
 }
 
diff --git a/includes/Data/MultiDimArray.php b/includes/Data/MultiDimArray.php
index 936ebb3..bde44e7 100644
--- a/includes/Data/MultiDimArray.php
+++ b/includes/Data/MultiDimArray.php
@@ -81,7 +81,7 @@
        // Maps from the query array to its position in the query array
        protected $queryKeys;
        protected $queryMap;
-       protected $queries;
+       protected $queries = array();
        protected $result;
 
        public function __construct( array $queryKeys, $dimensions ) {
@@ -94,7 +94,16 @@
 
        // Add a query and its position.  Positions must be unique.
        public function add( $query, $position ) {
+               $dim = count( (array) $position );
+               if ( $dim !== $this->dimensions ) {
+                       throw new \InvalidArgumentException( "Expection 
position with {$this->dimensions} dimensions, received $dim" );
+               }
                $query = ObjectManager::splitFromRow( $query, $this->queryKeys 
);
+               if ( $query === null ) {
+                       // the queryKeys are either unset or null, and not 
indexable
+                       // TODO: what should happen here?
+                       return;
+               }
                $this->desiredOrder[$position] = $query;
                if ( !isset( $this->queryMap[$query] ) ) {
                        $this->queries[] = $query;
@@ -105,6 +114,11 @@
        // merge a query into the result set
        public function merge( array $query, array $result ) {
                $query = ObjectManager::splitFromRow( $query, $this->queryKeys 
);
+               if ( $query === null ) {
+                       // the queryKeys are either unset or null, and not 
indexable
+                       // TODO: what should happen here?
+                       return;
+               }
                $this->result[$query] = $result;
        }
 
@@ -124,8 +138,6 @@
                        if ( $dimensions > 1 ) {
                                $final[$position] = self::sortResult( $query, 
$result, $dimensions - 1 );
                        } elseif ( !isset( $result[$query] ) ) {
-                               throw new \Exception( 'Missing result' );
-                       } else {
                                $final[$position] = $result[$query];
                        }
                }
diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php
index c6f7242..76efa71 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -15,28 +15,25 @@
 // - But we want to be able to replace that cache with a buffered cache that 
flushes
 //   on db commit.
 // - Perhaps one buffered cache could wrap both redis and memcache? seems odd 
though
-// Different indexes on the same ObjectManager will always use the same storage
 
-// TODO: This interface is too tied to the ObjectManager,  it should
-// be possible to use things implementing LifecycleHandler without
-// an ObjectManager instances(to keep things simpler).
 interface LifecycleHandler {
-       function onPostLoad( $object, array $old );
-       function onPostInsert( $object, array $new );
-       function onPostUpdate( $object, array $old, array $new );
-       function onPostRemove( $object, array $old );
+       function onAfterLoad( $object, array $old );
+       function onAfterInsert( $object, array $new );
+       function onAfterUpdate( $object, array $old, array $new );
+       function onAfterRemove( $object, array $old );
 }
 
 // Some denormalized data doesnt accept writes, it merely triggers cache 
updates
 // when something else does the write. Indexes are the primary use case.
-interface ObjectStorage {
+// IteratorAggregate rather than traversable to simplify nested implementations
+interface ObjectStorage extends \IteratorAggregate {
        function find( array $attributes, array $options = array() );
        /**
         * The BagOStuff interface returns with keys matching the key, 
unfortunatly
-        * we deal with composite keys which makes that awkward. Instead all 
findMulti
-        * implementations must return their result as if it was arrap_map( 
array( $obj, 'find' ), $queries ).
-        * This is necessary so result sets stay ordered
-        *
+        * we deal with composite keys which makes that awkward. Instead all
+        * findMulti implementations return their data with the same array 
indexes
+        * as the query in $queries. The order the requested queries are 
returned in
+        * is arbitrary.
         *
         * @param array $queries list of queries to perform
         * @param array $options Options to use for all queries
@@ -44,6 +41,9 @@
         */
        function findMulti( array $queries, array $options = array() );
        function getPrimaryKeyColumns();
+       // Clear any information stored about loaded objects
+       // This interface is used by the frontend (ObjectLocator) and the 
backend (BasicDbStorage, etc)
+       //function clear();
 }
 
 // Backing stores, typically in SQL
@@ -53,7 +53,6 @@
        function remove( array $row );
 }
 
-// Perhaps better names, because this isnt php serialization, its domain model 
<-> db row
 interface ObjectMapper {
        /**
         * Convert $object from the domain model to its db row
@@ -132,7 +131,6 @@
  * Denormalized indexes that are query-only.  The indexes used here must
  * be provided to some ObjectManager as a lifecycleHandler to receive
  * update events.
- *
  * Error handling is all wrong, but simplifies prototyping.
  */
 class ObjectLocator implements ObjectStorage {
@@ -153,6 +151,9 @@
                return $result ? reset( $result ) : null;
        }
 
+       public function getIterator() {
+               throw new \Exception( 'Not Implemented' );
+       }
        /**
         * All queries must be against the same index. Results are equivilent to
         * array_map, maintaining order and key relationship between input 
$queries
@@ -175,8 +176,8 @@
                        $limit = false;
                }
 
-               $res = $this->getIndexFor( $keys, $options )->findMulti( 
$queries, $options );
-               if ( !$res ) {
+               $res = $this->getIndexFor( $keys, $options )->findMulti( 
$queries );
+               if ( $res === null ) {
                        return null;
                }
                $retval = array();
@@ -217,7 +218,7 @@
                // to be consistent. undo that for a flat result array
                $res = $this->findMulti( $queries );
                if ( !$res ) {
-                       return false;
+                       return null;
                }
                $retval = array();
                foreach ( $res as $row ) {
@@ -226,12 +227,16 @@
                return $retval;
        }
 
+       public function clear() {
+               // nop, we dont store anything
+       }
+
        /**
         * Only use from maintenance.  Updates not implemented(yet), needs
         * handling for transactions.
         */
        public function visitAll( $callback ) {
-               // storage is IteratorAggregate returning EchoBatchRowIterator 
of
+               // storage is commonly IteratorAggregate returning 
EchoBatchRowIterator of
                // 500 rows at a time by default.
                foreach ( $this->storage as $rows ) {
                        foreach ( $rows as $row ) {
@@ -268,7 +273,7 @@
        protected function load( $row ) {
                $object = $this->mapper->fromStorageRow( $row );
                foreach ( $this->lifecycleHandlers as $handler ) {
-                       $handler->onPostLoad( $object, $row );
+                       $handler->onAfterLoad( $object, $row );
                }
                return $object;
        }
@@ -279,7 +284,7 @@
  * Cant implement WritableObjectStorage because 'update' method signature 
differs
  */
 class ObjectManager extends ObjectLocator {
-       // @var SplObjectHash $loaded If the object exists then an 'update' is 
issued, otherwise 'insert'
+       // @var SplObjectStorage $loaded If the object exists then an 'update' 
is issued, otherwise 'insert'
        protected $loaded;
 
        public function __construct( ObjectMapper $mapper, 
WritableObjectStorage $storage, array $indexes = array(), array 
$lifecycleHandlers = array() ) {
@@ -314,7 +319,7 @@
                        $new = $this->mapper->toStorageRow( $object );
                        $this->storage->insert( $new );
                        foreach ( $this->lifecycleHandlers as $handler ) {
-                               $handler->onPostInsert( $object, $new );
+                               $handler->onAfterInsert( $object, $new );
                        }
                        $this->loaded[$object] = $new;
                } catch ( \Exception $e ) {
@@ -331,7 +336,7 @@
                        }
                        $this->storage->update( $old, $new );
                        foreach ( $this->lifecycleHandlers as $handler ) {
-                               $handler->onPostUpdate( $object, $old, $new );
+                               $handler->onAfterUpdate( $object, $old, $new );
                        }
                        $this->loaded[$object] = $new;
                } catch ( \Exception $e ) {
@@ -340,17 +345,8 @@
        }
 
        static public function arrayEquals( array $old, array $new ) {
-               foreach ( $old as $key => $value ) {
-                       if ( !isset( $new[$key] ) || $new[$key] !== $value ) {
-                               return false;
-                       }
-                       unset( $new[$key] );
-               }
-               if ( !empty( $new ) ) {
-                       return false;
-               }
-               // All keys in old match new and there are no left-over keys
-               return true;
+               return array_diff_assoc( $old, $new ) === array()
+                       && array_diff_assoc( $new, $old ) === array();
        }
 
        static public function makeArray( $input ) {
@@ -366,7 +362,7 @@
                        $old = $this->loaded[$object];
                        $this->storage->remove( $old );
                        foreach ( $this->lifecycleHandlers as $handler ) {
-                               $handler->onPostRemove( $object, $old );
+                               $handler->onAfterRemove( $object, $old );
                        }
                        unset( $this->loaded[$object] );
                } catch ( \Exception $e ) {
@@ -375,7 +371,7 @@
        }
 
        public function clear() {
-               $this->loaded = new SplObjectHash;
+               $this->loaded = new SplObjectStorage;
        }
 
        protected function load( $row ) {
@@ -399,7 +395,6 @@
                                return null;
                        }
                        $split[$key] = $row[$key];
-                       unset( $row[$key] );
                }
 
                return $split;
@@ -419,7 +414,7 @@
 /**
  * $userMapper = new BasicObjectMapper(
  *     array( 'User', 'toStorageRow' ),
- *     array( 'User', 'newFromRow' ),
+ *     array( 'User', 'fromStorageRow' ),
  * );
  */
 class BasicObjectMapper implements ObjectMapper {
@@ -441,9 +436,9 @@
 }
 
 // Doesn't support updating primary key value yet
-class BasicDbStorage implements WritableObjectStorage, \IteratorAggregate {
+class BasicDbStorage implements WritableObjectStorage {
        public function __construct( DbFactory $dbFactory, $table, array 
$primaryKey ) {
-               if ( empty( $primaryKey ) ) {
+               if ( !$primaryKey ) {
                        throw new \Exception( 'PK required' );
                }
                $this->dbFactory = $dbFactory;
@@ -466,14 +461,14 @@
                        $missing = array_diff( $this->primaryKey, array_keys( 
$old ) );
                        throw new PersistenceException( 'Row has null primary 
key: ' . implode( $missing ) );
                }
+               $updates = $this->calcUpdates( $old, $new );
+               if ( !$updates ) {
+                       return true; // nothing to change, success
+               }
                $dbw = $this->dbFactory->getDB( DB_MASTER );
-               $res = $dbw->update(
-                       $this->table,
-                       $this->calcUpdates( $old, $new ),
-                       UUID::convertUUIDs( $pk ),
-                       __METHOD__
-               );
                // update returns boolean true/false as $res
+               $res = $dbw->update( $this->table, $updates, 
UUID::convertUUIDs( $pk ), __METHOD__ );
+               // $dbw->update returns boolean true/false as $res
                // we also want to check that $pk actually selected a row to 
update
                return $res && $dbw->affectedRows();
        }
@@ -484,12 +479,11 @@
                        if ( !array_key_exists( $key, $old ) || $old[$key] !== 
$new[$key] ) {
                                $updates[$key] = $new[$key];
                        }
+                       unset( $old[$key] );
                }
-               if ( !$updates ) {
-                       echo '<pre>';
-                       var_dump( $old );
-                       var_dump( $new );
-                       die();
+               // These keys dont exist in $new
+               foreach ( array_keys( $old ) as $key ) {
+                       $updates[$key] = null;
                }
                return $updates;
        }
@@ -571,18 +565,10 @@
        }
 }
 
-class NullLifecycleHandler implements LifecycleHandler {
-       function onPostInsert( $object, array $new ) {
-       }
-       function onPostUpdate( $object, array $old, array $new ) {
-       }
-       function onPostLoad( $object, array $old ) {
-       }
-       function onPostRemove( $object, array $old ) {
-       }
-}
-
-abstract class AbstractIndex extends NullLifecycleHandler implements Index {
+/**
+ * Index objects with similar features into the same buckets.
+ */
+abstract class FeatureIndex implements Index {
 
        abstract public function getLimit();
        abstract public function queryOptions();
@@ -590,12 +576,18 @@
        abstract protected function addToIndex( array $indexed, array $row );
        abstract protected function removeFromIndex( array $indexed, array $row 
);
 
+       /**
+        * @param BufferedCache $cache
+        * @param ObjectStorage $storage
+        * @param string        $prefix
+        * @param array         $indexedColumns List of columns to index,
+        */
        public function __construct( BufferedCache $cache, ObjectStorage 
$storage, $prefix, array $indexedColumns ) {
                $this->cache = $cache;
                $this->storage = $storage;
                $this->prefix = $prefix;
                $this->indexed = $indexedColumns;
-               // sort this and ksort is self::cacheKey to always have cache 
key
+               // sort this and ksort in self::cacheKey to always have cache 
key
                // fields in same order
                sort( $indexedColumns );
                $this->indexedOrdered = $indexedColumns;
@@ -626,22 +618,37 @@
                return true;
        }
 
-       public function onPostInsert( $object, array $new) {
+       public function onAfterInsert( $object, array $new) {
                $indexed = ObjectManager::splitFromRow( $new , $this->indexed );
-               $this->addToIndex( $indexed, $this->compactRow( $new ) );
+               // is un-indexable a bail-worthy occasion? Probably not
+               if ( $indexed ) {
+                       $this->addToIndex( $indexed, $this->compactRow( $new ) 
);
+               }
        }
 
-       public function onPostUpdate( $object, array $old, array $new ) {
+       public function onAfterUpdate( $object, array $old, array $new ) {
                $oldIndexed = ObjectManager::splitFromRow( $old, $this->indexed 
);
                $newIndexed = ObjectManager::splitFromRow( $new, $this->indexed 
);
                // TODO: optimize $oldIndexed === $newIndexed, which should be 
common
-               $this->removeFromIndex( $oldIndexed, $this->compactRow( $old ) 
);
-               $this->addToIndex( $newIndexed, $this->compactRow( $new ) );
+               // TODO: optimize out occurances where data changed but not the 
values
+               //       indexed or stored in the index(for shallow indexes)
+               if ( $oldIndexed ) {
+                       $this->removeFromIndex( $oldIndexed, $this->compactRow( 
$old ) );
+               }
+               if ( $newIndexed ) {
+                       $this->addToIndex( $newIndexed, $this->compactRow( $new 
) );
+               }
        }
 
-       public function onPostRemove( $object, array $old ) {
+       public function onAfterRemove( $object, array $old ) {
                $indexed = ObjectManager::splitFromRow( $old, $this->indexed );
-               $this->removeFromIndex( $indexed, $this->compactRow( $old ) );
+               if ( $indexed ) {
+                       $this->removeFromIndex( $indexed, $this->compactRow( 
$old ) );
+               }
+       }
+
+       public function onAfterLoad( $object, array $old ) {
+               // nothing to do
        }
 
        public function find( array $attributes ) {
@@ -664,11 +671,9 @@
                                );
                        }
                        $key = $this->cacheKey( $query );
+                       // allow for duplicate queries
                        $keyToIdx[$key][] = $idx;
-                       if ( isset( $keyToQuery[$key] ) ) {
-                               // duplicate query
-                               unset( $queries[$idx] );
-                       } else {
+                       if ( !isset( $keyToQuery[$key] ) ) {
                                $idxToKey[$idx] = $key;
                                $keyToQuery[$key] = $query;
                        }
@@ -709,6 +714,13 @@
 
 
        protected function cacheKey( array $attributes ) {
+               global $wgFlowDefaultWikiDb; // meh
+               if ( $wgFlowDefaultWikiDb === false ) {
+                       $db = wfWikiId();
+               } else {
+                       $db = $wgFlowDefaultWikiDb;
+               }
+
                foreach( $attributes as $key => $attr ) {
                        if ( $attr instanceof \Flow\Model\UUID ) {
                                $attributes[$key] = $attr->getHex();
@@ -718,7 +730,7 @@
                        }
                }
 
-               return wfForeignMemcKey( 'flow', '', $this->prefix, implode( 
':', $attributes ) );
+               return wfForeignMemcKey( $db, '', $this->prefix, implode( ':', 
$attributes ) );
        }
 
        /**
@@ -755,7 +767,11 @@
        }
 }
 
-class UniqueIndex extends AbstractIndex {
+/**
+ * Offers direct lookup of an object via a unique feature(set of properties)
+ * on the object.
+ */
+class UniqueFeatureIndex extends FeatureIndex {
 
        public function getLimit() {
                return 1;
@@ -781,13 +797,16 @@
        }
 }
 
-class SecondaryIndex extends AbstractIndex {
+/**
+ * Holds the top k items matching
+ */
+class TopKIndex extends FeatureIndex {
        public function __construct( BufferedCache $cache, ObjectStorage 
$storage, $prefix, array $indexed, array $options = array() ) {
                if ( empty( $options['sort'] ) ) {
-                       throw new \InvalidArgumentException( 'SecondaryIndex 
must be sorted' );
+                       throw new \InvalidArgumentException( 'TopKIndex must be 
sorted' );
                }
-               if ( isset( $options['shallow'] ) && !$options['shallow'] 
instanceof UniqueIndex ) {
-                       throw new \InvalidArgumentException( "The 'shallow' 
option must be either null or UniqueIndex." );
+               if ( isset( $options['shallow'] ) && !$options['shallow'] 
instanceof UniqueFeatureIndex ) {
+                       throw new \InvalidArgumentException( "The 'shallow' 
option must be either null or UniqueFeatureIndex." );
                }
 
                parent::__construct( $cache, $storage, $prefix, $indexed );
@@ -896,16 +915,13 @@
 
        protected function expandCacheResult( array $cached, array $keyToQuery 
) {
                $cached = parent::expandCacheResult( $cached, $keyToQuery );
-               if ( $this->options['shallow'] ) {
+               if ( $cached && $this->options['shallow'] ) {
                        return $this->expandShallowResult( $cached );
                }
                return $cached;
        }
 
        protected function expandShallowResult( array $results ) {
-               if ( !$results ) {
-                       return array();
-               }
                // Allows us to flatten $results into a single $query array, 
then
                // rebuild final return value in same structure and order as 
$results.
                $duplicator = new ResultDuplicator( 
$this->options['shallow']->getPrimaryKeyColumns(), 2 );
@@ -1050,6 +1066,7 @@
        public function getMulti( array $keys ) {
                return $this->cache->getMulti( $keys );
        }
+
        public function add( $key, $value, $exptime = 0 ) {
                if ( $this->buffer === null ) {
                        $this->cache->add( $key, $value, $exptime );
diff --git a/includes/Data/RevisionStorage.php 
b/includes/Data/RevisionStorage.php
index b954286..22a65fb 100644
--- a/includes/Data/RevisionStorage.php
+++ b/includes/Data/RevisionStorage.php
@@ -12,6 +12,7 @@
 abstract class RevisionStorage implements WritableObjectStorage {
        static protected $allowedUpdateColumns = array( 'rev_flags' );
        protected $dbFactory;
+       protected $externalStores;
 
        abstract protected function joinTable();
        abstract protected function relatedPk();
@@ -21,19 +22,18 @@
        abstract protected function updateRelated( array $rev, array $related );
        abstract protected function removeRelated( array $row );
 
-       public function __construct( DbFactory $dbFactory ) {
+       public function __construct( DbFactory $dbFactory, $externalStore ) {
                $this->dbFactory = $dbFactory;
+               $this->externalStore = $externalStore;
        }
 
        // Find one by specific attributes
        public function find( array $attributes, array $options = array() ) {
-               global $wgFlowExternalStore;
-
                $res = $this->findInternal( $attributes, $options );
                if ( $res === false ) {
                        return false;
                }
-               if ( $wgFlowExternalStore ) {
+               if ( $this->externalStore ) {
                        $res = Merger::merge(
                                $res,
                                'text_content',
@@ -67,14 +67,12 @@
        }
 
        public function findMulti( array $queries, array $options = array() ) {
-               global $wgFlowExternalStore;
-
                if ( count( $queries ) < 3 ) {
                        $res = $this->fallbackFindMulti( $queries, $options );
                } else {
                        $res = $this->findMultiInternal( $queries, $options );
                }
-               if ( $wgFlowExternalStore ) {
+               if ( $this->externalStore ) {
                        return Merger::mergeMulti(
                                $res,
                                'text_content',
@@ -234,12 +232,11 @@
        }
 
        protected function insertContent( $data ) {
-               global $wgFlowExternalStore;
                $flags = \Revision::compressRevisionText( $data );
 
-               if ( $wgFlowExternalStore ) {
+               if ( $this-externalStore ) {
                        // Store and get the URL
-                       $data = ExternalStore::insertWithFallback( $data, 
array(), $wgFlowExternalStore );
+                       $data = ExternalStore::insertWithFallback( 
$this->externalStore, $data );
                        if ( !$data ) {
                                throw new \MWException( "Unable to store text 
to external storage" );
                        }
@@ -312,6 +309,10 @@
                return array( 'rev_id' );
        }
 
+       public function getIterator() {
+               throw new \MWException( 'Not Implemented' );
+       }
+
        // Separtes $row into two arrays, one with the rev_ prefix
        // and the other with everything else.  May need to split more
        // specifically if we want > 2 prefixes.
@@ -331,8 +332,8 @@
 
 class PostRevisionStorage extends RevisionStorage {
 
-       public function __construct( DbFactory $dbFactory, TreeRepository 
$treeRepo ) {
-               parent::__construct( $dbFactory );
+       public function __construct( DbFactory $dbFactory, $externalStore, 
TreeRepository $treeRepo ) {
+               parent::__construct( $dbFactory, $externalStore );
                $this->treeRepo = $treeRepo;
        }
 
@@ -428,7 +429,7 @@
 }
 
 /**
- * This assists in performing client-side joins.  It collects the foreign key
+ * This assists in performing client-side 1-to-1 joins.  It collects the 
foreign key
  * from a multi-dimensional array, queries a callable for the foreign key 
values and
  * then returns the source data with related data merged in.
  */
diff --git a/includes/Model/AbstractRevision.php 
b/includes/Model/AbstractRevision.php
index bc84d78..335f303 100644
--- a/includes/Model/AbstractRevision.php
+++ b/includes/Model/AbstractRevision.php
@@ -10,6 +10,8 @@
        protected $userId;
        protected $userText;
        protected $flags = array();
+       // An i18n message key indicating what kind of change this revision is
+       // primary use case is the a revision history list.
        protected $comment;
        protected $prevRevision;
 
@@ -20,6 +22,13 @@
 
        static public function fromStorageRow( array $row ) {
                $obj = new static;
+               if ( $row['rev_type'] !== $obj->getRevisionType() ) {
+                       throw new \MWException( sprintf(
+                               "Wrong revision type, expected '%s' but 
received '%s'",
+                               $obj->getRevisionType(),
+                               $row['rev_type']
+                       ) );
+               }
                $obj->revId = UUID::create( $row['rev_id'] );
                $obj->userId = $row['rev_user_id'];
                $obj->userText = $row['rev_user_text'];
@@ -48,8 +57,8 @@
                        'rev_flags' => implode( ',', $obj->flags ),
                        'rev_parent_id' => $prevRevision,
                        'rev_comment' => $obj->comment,
-
                        'rev_text_id' => $obj->textId,
+                       'rev_type' => $obj->getRevisionType(),
 
                        'text_content' => $obj->content,
                );
@@ -87,11 +96,6 @@
                return $this->content;
        }
 
-       // internal: for lazy loading from external source
-       public function setContent( $content ) {
-               $this->content = $content;
-       }
-
        public function getTextId() {
                return $this->textId; // not available on creation
        }
@@ -109,6 +113,9 @@
                        // already flagged
                        return $this;
                }
+               if ( false !== strpos( ',', $flag ) ) {
+                       throw new \MWException( 'Invalid flag name: contains 
comma' );
+               }
                $updated = $this->newNullRevision( $user );
                $updated->flags[] = $flag;
                $updated->comment = $comment;
diff --git a/includes/Model/PostRevision.php b/includes/Model/PostRevision.php
index 81eb5ac..3be1956 100644
--- a/includes/Model/PostRevision.php
+++ b/includes/Model/PostRevision.php
@@ -30,24 +30,17 @@
                $obj->origCreateTime = wfTimestampNow();
                $obj->replyToId = null; // not a reply to anything
                $obj->prevRevId = null; // no parent revision
+               $obj->comment = 'flow-rev-message-new-post';
                return $obj;
        }
 
        static public function fromStorageRow( array $row ) {
-               if ( $row['rev_type'] !== 'post' ) {
-                       throw new \MWException( "Wrong revision type, expected 
'post' but got : " . $row['rev_type'] );
-               }
                if ( $row['rev_id'] !== $row['tree_rev_id'] ) {
                        throw new \MWException( 'tree revision doesn\'t match 
provided revision' );
                }
                $obj = parent::fromStorageRow( $row );
 
-               if ( $row['tree_parent_id'] ) {
-                       $obj->replyToId = UUID::create( $row['tree_parent_id'] 
);
-               } else {
-                       $obj->replyToId = null;
-               }
-
+               $obj->replyToId = UUID::create( $row['tree_parent_id'] );
                $obj->postId = UUID::create( $row['tree_rev_descendant'] );
                $obj->origCreateTime = $row['tree_orig_create_time'];
                $obj->origUserId = $row['tree_orig_user_id'];
@@ -57,14 +50,8 @@
        }
 
        static public function toStorageRow( $rev ) {
-               $replyToId = null;
-
-               if ( $rev->replyToId ) {
-                       $replyToId = $rev->replyToId->getBinary();
-               }
                return parent::toStorageRow( $rev ) + array(
-                       'rev_type' => 'post',
-                       'tree_parent_id' => $replyToId,
+                       'tree_parent_id' => $rev->replyToId ? 
$rev->replyToId->getBinary() : null,
                        'tree_rev_descendant' => $rev->postId->getBinary(),
                        'tree_rev_id' => $rev->revId->getBinary(),
                        // rest of tree_ is denormalized data about first post 
revision
@@ -83,6 +70,7 @@
                $reply->origCreateTime = wfTimestampNow();
                $reply->content = $content;
                $reply->replyToId = $this->postId;
+               $reply->comment = 'flow-rev-message-reply';
                return $reply;
        }
 
@@ -125,6 +113,10 @@
        public function compareCreateTime( PostRevision $rev ) {
                return strcmp( $rev->postId->getNumber(), 
$this->postId->getNumber() );
        }
+
+       public function getRevisionType() {
+               return 'post';
+       }
 }
 
 
diff --git a/includes/Model/Summary.php b/includes/Model/Summary.php
index 4fc8250..238a874 100644
--- a/includes/Model/Summary.php
+++ b/includes/Model/Summary.php
@@ -19,9 +19,6 @@
        }
 
        static public function fromStorageRow( array $row ) {
-               if ( $row['rev_type'] !== 'summary' ) {
-                       throw new \MWException( 'Wrong revision type: ' . 
$row['rev_type'] );
-               }
                $obj = parent::fromStorageRow( $row );
                $obj->workflowId = UUID::create( $row['summary_workflow_id'] );
                return $obj;
@@ -34,5 +31,9 @@
                        'summary_workflow_id' => $obj->workflowId->getBinary(),
                );
        }
+
+       public function getRevisionType() {
+               return 'summary';
+       }
 }
 
diff --git a/includes/Model/Workflow.php b/includes/Model/Workflow.php
index 180663c..1060dd5 100644
--- a/includes/Model/Workflow.php
+++ b/includes/Model/Workflow.php
@@ -2,6 +2,7 @@
 
 namespace Flow\Model;
 
+use SpecialPage;
 use Title;
 use User;
 
@@ -83,7 +84,7 @@
 
        public function getTitle() {
                $article = $this->getArticleTitle();
-               return Title::newFromText( "Flow/" . 
$article->getPrefixedDBkey(), NS_SPECIAL );
+               return SpecialPage::getTitleFor( 'Flow', 
$article->getPrefixedDBkey() );
        }
 
        public function getArticleTitle() {
@@ -115,12 +116,14 @@
                }
        }
 
+       // these are exceptions currently to make debugging easier
+       // it should return false later on to allow wider use.
        public function matchesTitle( Title $title ) {
                if ( $title->getArticleID() === 0 ) {
                        return false; // non-existant page
                }
                if ( $title->getNamespace() !== $this->namespace ) {
-                       var_dump( $title->getNamespace());
+                       var_dump( $title->getNamespace() );
                        var_dump( $this->namespace );
                        die();
                        throw new \Exception( 'namespace' );
diff --git a/includes/Repository/SelectQueryBuilder.php 
b/includes/Repository/SelectQueryBuilder.php
index bfed3ed..be1be98 100644
--- a/includes/Repository/SelectQueryBuilder.php
+++ b/includes/Repository/SelectQueryBuilder.php
@@ -16,10 +16,6 @@
                return new self;
        }
 
-       public function setCacheKey( $cacheKey ) {
-               $this->cacheKey = $cacheKey;
-       }
-
        public function from( $table ) {
                $this->table = $table;
                return $this;
@@ -69,10 +65,10 @@
                        list( $field, $op, $value ) = $row;
                        $conditions[] = "$field $op     " . $dbr->addQuotes( 
$value );
                }
-               $row = $db->select( $this->table, $this->fields, $conditions, 
$fname, $this->options );
+               $res = $db->select( $this->table, $this->fields, $conditions, 
$fname, $this->options );
                if ( $this->resultHandler ) {
-                       $row = call_user_func( $this->resultHandler, $row );
+                       return call_user_func( $this->resultHandler, $res );
                }
-               return $row;
+               return $res;
        }
 }
diff --git a/includes/Templating.php b/includes/Templating.php
index 3ef4eaf..66243ec 100644
--- a/includes/Templating.php
+++ b/includes/Templating.php
@@ -19,6 +19,10 @@
                $this->globals = $globals;
        }
 
+       public function getOutput() {
+               return $this->output;
+       }
+
        public function addNamespace( $ns, $path ) {
                $this->namespaces[$ns] = rtrim( $path, '/' );
        }
diff --git a/includes/UrlGenerator.php b/includes/UrlGenerator.php
index 6d62061..72fdd81 100644
--- a/includes/UrlGenerator.php
+++ b/includes/UrlGenerator.php
@@ -14,6 +14,7 @@
        public function generateUrl( $workflow, $action = 'view', array $query 
= array() ) {
                if ( ! $workflow instanceof Workflow ) {
                        $workflowId = $workflow;
+                       // Only way to know what title the workflow points at
                        $workflow = $this->storage->get( $workflowId );
                        if ( !$workflow ) {
                                throw new \MWException( "Unknown flow object: 
$workflowId" );
diff --git a/modules/base/ext.flow.base.js b/modules/base/ext.flow.base.js
new file mode 100644
index 0000000..02405dd
--- /dev/null
+++ b/modules/base/ext.flow.base.js
@@ -0,0 +1 @@
+alert( 'foo' );
diff --git a/special/SpecialFlow.php b/special/SpecialFlow.php
index 1272270..47fba3c 100644
--- a/special/SpecialFlow.php
+++ b/special/SpecialFlow.php
@@ -91,8 +91,7 @@
                        ) );
                        if ( $found ) {
                                $definition = reset( $found );
-                       }
-                       if ( empty( $definition ) ) {
+                       } else {
                                throw new MWException( "Unknown flow type 
'$workflowName' requested" );
                        }
                }
diff --git a/vendor/Pimple.php b/vendor/Pimple.php
index 7537cfa..39f8daf 100644
--- a/vendor/Pimple.php
+++ b/vendor/Pimple.php
@@ -25,7 +25,7 @@
  */
 
 /**
- * Pimple main class.
+ * Pimple main class.  http://pimple.sensiolabs.org/
  *
  * @package pimple
  * @author  Fabien Potencier

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If85dbc1a9c8558e4ec0f5b8c2030f3e08b5dc399
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson (WMF) <ebernhard...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to