Merge pull request #5836 from cakephp/3.0-table-registry-alias

The plugin alias should not be ignored

src/Datasource/EntityTrait.php

@@ -112,7 +112,7 @@ trait EntityTrait
      *
      * @var string
      */
-    protected $_repositoryAlias;
+    protected $_registryAlias;
 
     /**
      * Magic getter to access properties that have been set in this entity
@@ -812,9 +812,9 @@ trait EntityTrait
     public function source($alias = null)
     {
         if ($alias === null) {
-            return $this->_repositoryAlias;
+            return $this->_registryAlias;
         }
-        $this->_repositoryAlias = $alias;
+        $this->_registryAlias = $alias;
     }
 
     /**
@@ -843,7 +843,7 @@ trait EntityTrait
             'original' => $this->_original,
             'virtual' => $this->_virtual,
             'errors' => $this->_errors,
-            'repository' => $this->_repositoryAlias
+            'repository' => $this->_registryAlias
         ];
     }
 }

src/Datasource/ModelAwareTrait.php

@@ -83,11 +83,11 @@ trait ModelAwareTrait
             $modelClass = $this->modelClass;
         }
 
-        if (isset($this->{$modelClass})) {
-            return $this->{$modelClass};
-        }
+        list($plugin, $alias) = pluginSplit($modelClass, true);
 
-        list($plugin, $modelClass) = pluginSplit($modelClass, true);
+        if (isset($this->{$alias})) {
+            return $this->{$alias};
+        }
 
         if (!isset($this->_modelFactories[$type])) {
             throw new InvalidArgumentException(sprintf(
@@ -96,11 +96,11 @@ trait ModelAwareTrait
             ));
         }
         $factory = $this->_modelFactories[$type];
-        $this->{$modelClass} = $factory($plugin . $modelClass);
-        if (!$this->{$modelClass}) {
+        $this->{$alias} = $factory($modelClass);
+        if (!$this->{$alias}) {
             throw new MissingModelException([$modelClass, $type]);
         }
-        return $this->{$modelClass};
+        return $this->{$alias};
     }
 
     /**

src/ORM/Association.php

@@ -269,13 +269,18 @@ abstract class Association
             return $this->_targetTable = $table;
         }
 
-        if ($table === null) {
-            $config = [];
-            if (!TableRegistry::exists($this->_name)) {
-                $config = ['className' => $this->_className];
-            }
-            $this->_targetTable = TableRegistry::get($this->_name, $config);
+        if ($this->_className && strpos($this->_className, '\\') === false) {
+            $tableAlias = $this->_className;
+        } else {
+            $tableAlias = $this->_name;
         }
+
+        $config = [];
+        if (!TableRegistry::exists($tableAlias)) {
+            $config = ['className' => $this->_className];
+        }
+        $this->_targetTable = TableRegistry::get($tableAlias, $config);
+
         return $this->_targetTable;
     }
 

src/ORM/Marshaller.php

@@ -107,7 +107,7 @@ class Marshaller
         $schema = $this->_table->schema();
         $entityClass = $this->_table->entityClass();
         $entity = new $entityClass();
-        $entity->source($this->_table->alias());
+        $entity->source($this->_table->registryAlias());
 
         if (isset($options['accessibleFields'])) {
             foreach ((array)$options['accessibleFields'] as $key => $value) {

src/ORM/Table.php

@@ -192,6 +192,13 @@ class Table implements RepositoryInterface, EventListenerInterface
     protected $_entityClass;
 
     /**
+     * Registry key used to create this table object
+     *
+     * @var string
+     */
+    protected $_registryAlias;
+
+    /**
      * A list of validation objects indexed by name
      *
      * @var array
@@ -228,6 +235,9 @@ class Table implements RepositoryInterface, EventListenerInterface
      */
     public function __construct(array $config = [])
     {
+        if (!empty($config['registryAlias'])) {
+            $this->registryAlias($config['registryAlias']);
+        }
         if (!empty($config['table'])) {
             $this->table($config['table']);
         }
@@ -349,6 +359,23 @@ class Table implements RepositoryInterface, EventListenerInterface
     }
 
     /**
+     * Returns the table registry key used to create this table instance
+     *
+     * @param string|null $registryAlias the key used to access this object
+     * @return string
+     */
+    public function registryAlias($registryAlias = null)
+    {
+        if ($registryAlias !== null) {
+            $this->_registryAlias = $registryAlias;
+        }
+        if ($this->_registryAlias === null) {
+            $this->_registryAlias = $this->alias();
+        }
+        return $this->_registryAlias;
+    }
+
+    /**
      * Returns the connection instance or sets a new one
      *
      * @param \Cake\Database\Connection|null $conn The new connection instance
@@ -1379,7 +1406,7 @@ class Table implements RepositoryInterface, EventListenerInterface
                 $entity->clean();
                 $this->dispatchEvent('Model.afterSave', compact('entity', 'options'));
                 $entity->isNew(false);
-                $entity->source($this->alias());
+                $entity->source($this->registryAlias());
                 $success = true;
             }
         }
@@ -1845,8 +1872,7 @@ class Table implements RepositoryInterface, EventListenerInterface
     {
         if ($data === null) {
             $class = $this->entityClass();
-            $entity = new $class;
-            $entity->source($this->alias());
+            $entity = new $class(['source' => $this->registryAlias()]);
             return $entity;
         }
         if (!isset($options['associated'])) {
@@ -2007,7 +2033,11 @@ class Table implements RepositoryInterface, EventListenerInterface
         }
         $entity = new Entity(
             $options['data'],
-            ['useSetters' => false, 'markNew' => $options['newRecord']]
+            [
+                'useSetters' => false,
+                'markNew' => $options['newRecord'],
+                'source' => $this->registryAlias()
+            ]
         );
         $fields = array_merge(
             [$options['field']],

src/ORM/TableRegistry.php

@@ -101,7 +101,6 @@ class TableRegistry
      */
     public static function config($alias = null, $options = null)
     {
-        list(, $alias) = pluginSplit($alias);
         if ($alias === null) {
             return static::$_config;
         }
@@ -154,27 +153,24 @@ class TableRegistry
      * @return \Cake\ORM\Table
      * @throws RuntimeException When you try to configure an alias that already exists.
      */
-    public static function get($name, array $options = [])
+    public static function get($alias, array $options = [])
     {
-        list(, $alias) = pluginSplit($name);
-        $exists = isset(static::$_instances[$alias]);
-
-        if ($exists && !empty($options)) {
-            if (static::$_options[$alias] !== $options) {
+        if (isset(static::$_instances[$alias])) {
+            if (!empty($options) && static::$_options[$alias] !== $options) {
                 throw new RuntimeException(sprintf(
                     'You cannot configure "%s", it already exists in the registry.',
                     $alias
                 ));
             }
-        }
-        if ($exists) {
             return static::$_instances[$alias];
         }
+
         static::$_options[$alias] = $options;
-        $options = ['alias' => $alias] + $options;
+        list(, $classAlias) = pluginSplit($alias);
+        $options = ['alias' => $classAlias] + $options;
 
         if (empty($options['className'])) {
-            $options['className'] = Inflector::camelize($name);
+            $options['className'] = Inflector::camelize($alias);
         }
         $className = App::className($options['className'], 'Model/Table', 'Table');
         $options['className'] = $className ?: 'Cake\ORM\Table';
@@ -187,6 +183,7 @@ class TableRegistry
             $options['connection'] = ConnectionManager::get($connectionName);
         }
 
+        $options['registryAlias'] = $alias;
         static::$_instances[$alias] = new $options['className']($options);
 
         if ($options['className'] === 'Cake\ORM\Table') {
@@ -199,15 +196,11 @@ class TableRegistry
     /**
      * Check to see if an instance exists in the registry.
      *
-     * Plugin names will be trimmed off of aliases as instances
-     * stored in the registry will be without the plugin name as well.
-     *
      * @param string $alias The alias to check for.
      * @return bool
      */
     public static function exists($alias)
     {
-        list(, $alias) = pluginSplit($alias);
         return isset(static::$_instances[$alias]);
     }
 
@@ -220,7 +213,6 @@ class TableRegistry
      */
     public static function set($alias, Table $object)
     {
-        list(, $alias) = pluginSplit($alias);
         return static::$_instances[$alias] = $object;
     }
 
@@ -252,16 +244,11 @@ class TableRegistry
     /**
      * Removes an instance from the registry.
      *
-     * Plugin name will be trimmed off of aliases as instances
-     * stored in the registry will be without the plugin name as well.
-     *
      * @param string $alias The alias to remove.
      * @return void
      */
     public static function remove($alias)
     {
-        list(, $alias) = pluginSplit($alias);
-
         unset(
             static::$_instances[$alias],
             static::$_config[$alias],

tests/TestCase/Datasource/ModelAwareTraitTest.php

@@ -71,6 +71,29 @@ class ModelAwareTraitTest extends TestCase
     }
 
     /**
+     * test loadModel() with plugin prefixed models
+     *
+     * Load model should not be called with Foo.Model Bar.Model Model
+     * But if it is, the first call wins.
+     *
+     * @return void
+     */
+    public function testLoadModelPlugin()
+    {
+        $stub = new Stub();
+        $stub->setProps('Articles');
+        $stub->modelFactory('Table', ['\Cake\ORM\TableRegistry', 'get']);
+
+        $result = $stub->loadModel('TestPlugin.Comments');
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $result);
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $stub->Comments);
+
+        $result = $stub->loadModel('Comments');
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $result);
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $stub->Comments);
+    }
+
+    /**
      * test alternate model factories.
      *
      * @return void

tests/TestCase/ORM/AssociationTest.php

@@ -14,6 +14,7 @@
  */
 namespace Cake\Test\TestCase\ORM;
 
+use Cake\Core\Plugin;
 use Cake\ORM\Association;
 use Cake\ORM\Table;
 use Cake\ORM\TableRegistry;
@@ -170,6 +171,41 @@ class AssociationTest extends TestCase
     }
 
     /**
+     * Tests that target() returns the correct Table object for plugins
+     *
+     * @return void
+     */
+    public function testTargetPlugin()
+    {
+        Plugin::load('TestPlugin');
+
+        $config = [
+            'className' => 'TestPlugin.Comments',
+            'foreignKey' => 'a_key',
+            'conditions' => ['field' => 'value'],
+            'dependent' => true,
+            'sourceTable' => $this->source,
+            'joinType' => 'INNER'
+        ];
+
+        $this->association = $this->getMock(
+            '\Cake\ORM\Association',
+            ['type', 'eagerLoader', 'cascadeDelete', 'isOwningSide', 'saveAssociated'],
+            ['ThisAssociationName', $config]
+        );
+
+        $table = $this->association->target();
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $table);
+
+        $this->assertTrue(TableRegistry::exists('TestPlugin.Comments'));
+        $this->assertFalse(TableRegistry::exists('Comments'));
+        $this->assertFalse(TableRegistry::exists('ThisAssociationName'));
+
+        $plugin = TableRegistry::get('TestPlugin.Comments');
+        $this->assertSame($table, $plugin, 'Should be the same TestPlugin.Comments object');
+    }
+
+    /**
      * Tests that source() returns the correct Table object
      *
      * @return void
@@ -264,10 +300,7 @@ class AssociationTest extends TestCase
         ];
         $assoc = $this->getMock(
             '\Cake\ORM\Association',
-            [
-                '_options', 'attachTo', '_joinCondition', 'cascadeDelete', 'isOwningSide',
-                'saveAssociated', 'eagerLoader', 'type'
-            ],
+            ['type', 'eagerLoader', 'cascadeDelete', 'isOwningSide', 'saveAssociated'],
             ['Foo', $config]
         );
         $this->assertEquals('published', $assoc->finder());

tests/TestCase/ORM/TableRegistryTest.php

@@ -102,10 +102,6 @@ class TableRegistryTest extends TestCase
 
         $result = TableRegistry::config('TestPlugin.TestPluginComments', $data);
         $this->assertEquals($data, $result, 'Returns config data.');
-
-        $result = TableRegistry::config();
-        $expected = ['TestPluginComments' => $data];
-        $this->assertEquals($expected, $result);
     }
 
     /**
@@ -138,6 +134,25 @@ class TableRegistryTest extends TestCase
     }
 
     /**
+     * Test the exists() method with plugin-prefixed models.
+     *
+     * @return void
+     */
+    public function testExistsPlugin()
+    {
+        $this->assertFalse(TableRegistry::exists('Comments'));
+        $this->assertFalse(TableRegistry::exists('TestPlugin.Comments'));
+
+        TableRegistry::config('TestPlugin.Comments', ['table' => 'comments']);
+        $this->assertFalse(TableRegistry::exists('Comments'), 'The Comments key should not be populated');
+        $this->assertFalse(TableRegistry::exists('TestPlugin.Comments'), 'The plugin.alias key should not be populated');
+
+        TableRegistry::get('TestPlugin.Comments', ['table' => 'comments']);
+        $this->assertFalse(TableRegistry::exists('Comments'), 'The Comments key should not be populated');
+        $this->assertTrue(TableRegistry::exists('TestPlugin.Comments'), 'The plugin.alias key should now be populated');
+    }
+
+    /**
      * Test getting instances from the registry.
      *
      * @return void
@@ -224,11 +239,10 @@ class TableRegistryTest extends TestCase
         Plugin::load('TestPlugin');
         $table = TableRegistry::get('TestPlugin.TestPluginComments', ['connection' => 'test']);
 
-        $class = 'TestPlugin\Model\Table\TestPluginCommentsTable';
-        $this->assertInstanceOf($class, $table);
-        $this->assertTrue(
+        $this->assertInstanceOf('TestPlugin\Model\Table\TestPluginCommentsTable', $table);
+        $this->assertFalse(
             TableRegistry::exists('TestPluginComments'),
-            'Short form should exist'
+            'Short form should NOT exist'
         );
         $this->assertTrue(
             TableRegistry::exists('TestPlugin.TestPluginComments'),
@@ -237,9 +251,35 @@ class TableRegistryTest extends TestCase
 
         $second = TableRegistry::get('TestPlugin.TestPluginComments');
         $this->assertSame($table, $second, 'Can fetch long form');
+    }
 
-        $second = TableRegistry::get('TestPluginComments');
-        $this->assertSame($table, $second);
+    /**
+     * Test get() with same-alias models in different plugins
+     *
+     * There should be no internal cache-confusion
+     *
+     * @return void
+     */
+    public function testGetMultiplePlugins()
+    {
+        Plugin::load('TestPlugin');
+        Plugin::load('TestPluginTwo');
+
+        $app = TableRegistry::get('Comments');
+        $plugin1 = TableRegistry::get('TestPlugin.Comments');
+        $plugin2 = TableRegistry::get('TestPluginTwo.Comments');
+
+        $this->assertInstanceOf('Cake\ORM\Table', $app, 'Should be an app table instance');
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $plugin1, 'Should be a plugin 1 table instance');
+        $this->assertInstanceOf('TestPluginTwo\Model\Table\CommentsTable', $plugin2, 'Should be a plugin 2 table instance');
+
+        $plugin2 = TableRegistry::get('TestPluginTwo.Comments');
+        $plugin1 = TableRegistry::get('TestPlugin.Comments');
+        $app = TableRegistry::get('Comments');
+
+        $this->assertInstanceOf('Cake\ORM\Table', $app, 'Should still be an app table instance');
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $plugin1, 'Should still be a plugin 1 table instance');
+        $this->assertInstanceOf('TestPluginTwo\Model\Table\CommentsTable', $plugin2, 'Should still be a plugin 2 table instance');
     }
 
     /**
@@ -257,6 +297,7 @@ class TableRegistryTest extends TestCase
         $class = 'TestPlugin\Model\Table\TestPluginCommentsTable';
         $this->assertInstanceOf($class, $table);
         $this->assertFalse(TableRegistry::exists('TestPluginComments'), 'Class name should not exist');
+        $this->assertFalse(TableRegistry::exists('TestPlugin.TestPluginComments'), 'Full class alias should not exist');
         $this->assertTrue(TableRegistry::exists('Comments'), 'Class name should exist');
 
         $second = TableRegistry::get('Comments');
@@ -278,6 +319,7 @@ class TableRegistryTest extends TestCase
         ]);
         $this->assertInstanceOf($class, $table);
         $this->assertFalse(TableRegistry::exists('TestPluginComments'), 'Class name should not exist');
+        $this->assertFalse(TableRegistry::exists('TestPlugin.TestPluginComments'), 'Full class alias should not exist');
         $this->assertTrue(TableRegistry::exists('Comments'), 'Class name should exist');
     }
 
@@ -389,7 +431,6 @@ class TableRegistryTest extends TestCase
 
         $this->assertSame($mock, TableRegistry::set('TestPlugin.Comments', $mock));
         $this->assertSame($mock, TableRegistry::get('TestPlugin.Comments'));
-        $this->assertSame($mock, TableRegistry::get('Comments'));
     }
 
     /**
@@ -413,19 +454,62 @@ class TableRegistryTest extends TestCase
      */
     public function testRemove()
     {
+        $first = TableRegistry::get('Comments');
+
+        $this->assertTrue(TableRegistry::exists('Comments'));
+
+        TableRegistry::remove('Comments');
+        $this->assertFalse(TableRegistry::exists('Comments'));
+
+        $second = TableRegistry::get('Comments');
+
+        $this->assertNotSame($first, $second, 'Should be different objects, as the reference to the first was destroyed');
+        $this->assertTrue(TableRegistry::exists('Comments'));
+    }
+
+    /**
+     * testRemovePlugin
+     *
+     * Removing a plugin-prefixed model should not affect any other
+     * plugin-prefixed model, or app model.
+     * Removing an app model should not affect any other
+     * plugin-prefixed model.
+     *
+     * @return void
+     */
+    public function testRemovePlugin()
+    {
         Plugin::load('TestPlugin');
+        Plugin::load('TestPluginTwo');
 
-        $pluginTable = TableRegistry::get('TestPlugin.Comments');
-        $cachedTable = TableRegistry::get('Comments');
+        $app = TableRegistry::get('Comments');
+        TableRegistry::get('TestPlugin.Comments');
+        $plugin = TableRegistry::get('TestPluginTwo.Comments');
 
         $this->assertTrue(TableRegistry::exists('Comments'));
-        $this->assertSame($pluginTable, $cachedTable);
+        $this->assertTrue(TableRegistry::exists('TestPlugin.Comments'));
+        $this->assertTrue(TableRegistry::exists('TestPluginTwo.Comments'));
+
+        TableRegistry::remove('TestPlugin.Comments');
+
+        $this->assertTrue(TableRegistry::exists('Comments'));
+        $this->assertFalse(TableRegistry::exists('TestPlugin.Comments'));
+        $this->assertTrue(TableRegistry::exists('TestPluginTwo.Comments'));
+
+        $app2 = TableRegistry::get('Comments');
+        $plugin2 = TableRegistry::get('TestPluginTwo.Comments');
+
+        $this->assertSame($app, $app2, 'Should be the same Comments object');
+        $this->assertSame($plugin, $plugin2, 'Should be the same TestPluginTwo.Comments object');
 
         TableRegistry::remove('Comments');
+
         $this->assertFalse(TableRegistry::exists('Comments'));
+        $this->assertFalse(TableRegistry::exists('TestPlugin.Comments'));
+        $this->assertTrue(TableRegistry::exists('TestPluginTwo.Comments'));
 
-        $appTable = TableRegistry::get('Comments');
-        $this->assertTrue(TableRegistry::exists('Comments'));
-        $this->assertNotSame($pluginTable, $appTable);
+        $plugin3 = TableRegistry::get('TestPluginTwo.Comments');
+
+        $this->assertSame($plugin, $plugin3, 'Should be the same TestPluginTwo.Comments object');
     }
 }

tests/TestCase/ORM/TableTest.php

@@ -16,6 +16,7 @@ namespace Cake\Test\TestCase\ORM;
 
 use ArrayObject;
 use Cake\Core\Configure;
+use Cake\Core\Plugin;
 use Cake\Database\Expression\OrderByExpression;
 use Cake\Database\Expression\QueryExpression;
 use Cake\Database\TypeMap;
@@ -520,6 +521,41 @@ class TableTest extends TestCase
     }
 
     /**
+     * Ensure associations use the plugin-prefixed model
+     *
+     * @return void
+     */
+    public function testHasManyPluginOverlap()
+    {
+        TableRegistry::get('Comments');
+        Plugin::load('TestPlugin');
+
+        $table = new Table(['table' => 'authors']);
+
+        $table->hasMany('TestPlugin.Comments');
+        $comments = $table->Comments->target();
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $comments);
+    }
+
+    /**
+     * Ensure associations use the plugin-prefixed model
+     * even if specified with config
+     *
+     * @return void
+     */
+    public function testHasManyPluginOverlapConfig()
+    {
+        TableRegistry::get('Comments');
+        Plugin::load('TestPlugin');
+
+        $table = new Table(['table' => 'authors']);
+
+        $table->hasMany('Comments', ['className' => 'TestPlugin.Comments']);
+        $comments = $table->Comments->target();
+        $this->assertInstanceOf('TestPlugin\Model\Table\CommentsTable', $comments);
+    }
+
+    /**
      * Tests that BelongsToMany() creates and configures correctly the association
      *
      * @return void

tests/test_app/Plugin/TestPluginTwo/src/Model/Table/CommentsTable.php

@@ -0,0 +1,25 @@
+<?php
+/**
+ * CakePHP :  Rapid Development Framework (http://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakefoundation.org/projects/info/cakephp CakePHP Project
+ * @since         3.0.0
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace TestPluginTwo\Model\Table;
+
+use Cake\ORM\Table;
+
+/**
+ * Class CommentsTable
+ */
+class CommentsTable extends Table
+{
+
+}