Issue #2248977 by Berdir: Complete support for multi-value base fields in...

Issue #2248977 by Berdir: Complete support for multi-value base fields in ContentEntitySchemaHandler and use it for the user.roles field

core/lib/Drupal/Core/Entity/Query/Sql/Tables.php

@@ -8,11 +8,9 @@
 namespace Drupal\Core\Entity\Query\Sql;
 
 use Drupal\Core\Database\Query\SelectInterface;
-use Drupal\Core\Entity\ContentEntityTypeInterface;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\Query\QueryException;
 use Drupal\Core\Entity\Sql\SqlEntityStorageInterface;
-use Drupal\field\FieldStorageConfigInterface;
 
 /**
  * Adds tables and fields to the SQL entity query.
@@ -80,12 +78,7 @@ public function addField($field, $type, $langcode) {
     $propertyDefinitions = array();
     $entity_type = $this->entityManager->getDefinition($entity_type_id);
 
-    $field_storage_definitions = array();
-    // @todo Needed for menu links, make this implementation content entity
-    //   specific after https://drupal.org/node/2256521.
-    if ($entity_type instanceof ContentEntityTypeInterface) {
     $field_storage_definitions = $this->entityManager->getFieldStorageDefinitions($entity_type_id);
-    }
     for ($key = 0; $key <= $count; $key ++) {
       // If there is revision support and only the current revision is being
       // queried then use the revision id. Otherwise, the entity id will do.
@@ -110,13 +103,15 @@ public function addField($field, $type, $langcode) {
       else {
         $field_storage = FALSE;
       }
-      // If we managed to retrieve a configurable field, process it.
-      if ($field_storage instanceof FieldStorageConfigInterface) {
-        // Find the field column.
-        $column = $field_storage->getMainPropertyName();
+
       /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
       $table_mapping = $this->entityManager->getStorage($entity_type_id)->getTableMapping();
 
+      // Check whether this field is stored in a dedicated table.
+      if ($field_storage && $table_mapping->requiresDedicatedTableStorage($field_storage)) {
+        // Find the field column.
+        $column = $field_storage->getMainPropertyName();
+
         if ($key < $count) {
           $next = $specifiers[$key + 1];
           // Is this a field column?
@@ -145,7 +140,7 @@ public function addField($field, $type, $langcode) {
         $table = $this->ensureFieldTable($index_prefix, $field_storage, $type, $langcode, $base_table, $entity_id_field, $field_id_field);
         $sql_column = $table_mapping->getFieldColumnName($field_storage, $column);
       }
-      // This is an entity base field (non-configurable field).
+      // The field is stored in a shared table.
       else {
         // ensureEntityTable() decides whether an entity property will be
         // queried from the data table or the base table based on where it

core/lib/Drupal/Core/Session/SessionHandler.php

@@ -92,7 +92,7 @@ public function read($sid) {
     // active user.
     if ($values && $values['uid'] > 0 && $values['status'] == 1) {
       // Add roles element to $user.
-      $rids = $this->connection->query("SELECT ur.rid FROM {users_roles} ur WHERE ur.uid = :uid", array(
+      $rids = $this->connection->query("SELECT ur.roles_target_id as rid FROM {user__roles} ur WHERE ur.entity_id = :uid", array(
         ':uid' => $values['uid'],
       ))->fetchCol();
       $values['roles'] = array_merge(array(DRUPAL_AUTHENTICATED_RID), $rids);

core/lib/Drupal/Core/Session/UserSession.php

@@ -126,7 +126,7 @@ public function getRoles($exclude_locked_roles = FALSE) {
     $roles = $this->roles;
 
     if ($exclude_locked_roles) {
-      $roles = array_diff($roles, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID));
+      $roles = array_values(array_diff($roles, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID)));
     }
 
     return $roles;

core/modules/filter/src/Tests/FilterFormatAccessTest.php

@@ -184,7 +184,7 @@ function testFormatPermissions() {
    */
   function testFormatRoles() {
     // Get the role ID assigned to the regular user.
-    $roles = $this->web_user->getRoles();
+    $roles = $this->web_user->getRoles(TRUE);
     $rid = $roles[0];
 
     // Check that this role appears in the list of roles that have access to an

core/modules/search/src/Tests/SearchCommentTest.php

@@ -143,7 +143,7 @@ function testSearchResultsComment() {
   function testSearchResultsCommentAccess() {
     $comment_body = 'Test comment body';
     $this->comment_subject = 'Test comment subject';
-    $roles = $this->admin_user->getRoles();
+    $roles = $this->admin_user->getRoles(TRUE);
     $this->admin_role = $roles[0];
 
     // Create a node.

core/modules/system/src/Tests/Entity/EntityUnitTestBase.php

@@ -75,8 +75,7 @@ protected function setUp() {
    * @param array $values
    *   (optional) The values used to create the entity.
    * @param array $permissions
-   *   (optional) Array of permission names to assign to user. The
-   *   users_roles tables must be installed before this can be used.
+   *   (optional) Array of permission names to assign to user.
    *
    * @return \Drupal\user\Entity\User
    *   The created user entity.

core/modules/user/config/install/views.view.user_admin_people.yml

@@ -294,10 +294,10 @@ display:
           type_custom_false: ''
           not: '0'
           plugin_id: boolean
-        rid:
-          id: rid
-          table: users_roles
-          field: rid
+        roles_target_id:
+          id: roles_target_id
+          table: user__roles
+          field: roles_target_id
           relationship: none
           group_type: group
           admin_label: ''
@@ -552,7 +552,7 @@ display:
             user_bulk_form: '0'
             name: '0'
             status: '0'
-            rid: '0'
+            roles_target_id: '0'
             created: '0'
             access: '0'
           destination: true
@@ -649,10 +649,10 @@ display:
             name: name
             mail: mail
           plugin_id: combine
-        rid:
-          id: rid
-          table: users_roles
-          field: rid
+        roles_target_id:
+          id: roles_target_id
+          table: user__roles
+          field: roles_target_id
           relationship: none
           group_type: group
           admin_label: ''
@@ -661,11 +661,11 @@ display:
           group: 1
           exposed: true
           expose:
-            operator_id: rid_op
+            operator_id: roles_target_id_op
             label: Role
             description: ''
             use_operator: false
-            operator: rid_op
+            operator: roles_target_id_op
             identifier: role
             required: false
             remember: false
@@ -691,7 +691,7 @@ display:
           plugin_id: user_roles
         permission:
           id: permission
-          table: users_roles
+          table: user__roles
           field: permission
           relationship: none
           group_type: group

core/modules/user/config/schema/user.views.schema.yml

@@ -23,7 +23,7 @@ views.argument.user_uid:
   type: views.argument.numeric
   label: 'User ID'
 
-views.argument.users_roles_rid:
+views.argument.user__roles_rid:
   type: views.argument.many_to_one
   label: 'Role ID'
 

core/modules/user/src/Entity/User.php

@@ -70,22 +70,19 @@ public function isNew() {
     return !empty($this->enforceIsNew) || $this->id() === NULL;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  static function preCreate(EntityStorageInterface $storage, array &$values) {
-    parent::preCreate($storage, $values);
-
-    // Users always have the authenticated user role.
-    $values['roles'][] = DRUPAL_AUTHENTICATED_RID;
-  }
-
   /**
    * {@inheritdoc}
    */
   public function preSave(EntityStorageInterface $storage) {
     parent::preSave($storage);
 
+    // Make sure that the authenticated/anonymous roles are not persisted.
+    foreach ($this->get('roles') as $index => $item) {
+      if (in_array($item->target_id, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID))) {
+        $this->get('roles')->offsetUnset($index);
+      }
+    }
+
     // Update the user password if it has changed.
     if ($this->isNew() || ($this->pass->value && $this->pass->value != $this->original->pass->value)) {
       // Allow alternate password hashing schemes.
@@ -129,12 +126,6 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
         }
       }
 
-      // Update user roles if changed.
-      if ($this->getRoles() != $this->original->getRoles()) {
-        $storage->deleteUserRoles(array($this->id()));
-        $storage->saveRoles($this);
-      }
-
       // If the user was blocked, delete the user's sessions to force a logout.
       if ($this->original->status->value != $this->status->value && $this->status->value == 0) {
         $session_manager->delete($this->id());
@@ -147,12 +138,6 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
         _user_mail_notify($op, $this);
       }
     }
-    else {
-      // Save user roles.
-      if (count($this->getRoles()) > 1) {
-        $storage->saveRoles($this);
-      }
-    }
   }
 
   /**
@@ -163,7 +148,6 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
 
     $uids = array_keys($entities);
     \Drupal::service('user.data')->delete(NULL, $uids);
-    $storage->deleteUserRoles($uids);
   }
 
   /**
@@ -172,8 +156,18 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
   public function getRoles($exclude_locked_roles = FALSE) {
     $roles = array();
 
+    // Users with an ID always have the authenticated user role.
+    if (!$exclude_locked_roles) {
+      if ($this->isAuthenticated()) {
+        $roles[] = DRUPAL_AUTHENTICATED_RID;
+      }
+      else {
+        $roles[] = DRUPAL_ANONYMOUS_RID;
+      }
+    }
+
     foreach ($this->get('roles') as $role) {
-      if (!($exclude_locked_roles && in_array($role->target_id, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID)))) {
+      if ($role->target_id) {
         $roles[] = $role->target_id;
       }
     }
@@ -223,7 +217,12 @@ public function hasRole($rid) {
    * {@inheritdoc}
    */
   public function addRole($rid) {
-    $roles = $this->getRoles();
+
+    if (in_array($rid, [DRUPAL_AUTHENTICATED_RID, DRUPAL_ANONYMOUS_RID])) {
+      throw new \InvalidArgumentException('Anonymous or authenticated role ID must not be assigned manually.');
+    }
+
+    $roles = $this->getRoles(TRUE);
     $roles[] = $rid;
     $this->set('roles', array_unique($roles));
   }
@@ -232,7 +231,7 @@ public function addRole($rid) {
    * {@inheritdoc}
    */
   public function removeRole($rid) {
-    $this->set('roles', array_diff($this->getRoles(), array($rid)));
+    $this->set('roles', array_diff($this->getRoles(TRUE), array($rid)));
   }
 
   /**
@@ -531,7 +530,6 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
       ->setDefaultValue('');
 
     $fields['roles'] = BaseFieldDefinition::create('entity_reference')
-      ->setCustomStorage(TRUE)
       ->setLabel(t('Roles'))
       ->setCardinality(BaseFieldDefinition::CARDINALITY_UNLIMITED)
       ->setDescription(t('The roles the user has.'))

core/modules/user/src/Plugin/entity_reference/selection/UserSelection.php

@@ -88,6 +88,12 @@ public function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') {
       $query->condition('name', $match, $match_operator);
     }
 
+    // Filter by role.
+    $handler_settings = $this->fieldDefinition->getSetting('handler_settings');
+    if (!empty($handler_settings['filter']['role'])) {
+      $query->condition('roles', $handler_settings['filter']['role'], 'IN');
+    }
+
     // Adding the permission check is sadly insufficient for users: core
     // requires us to also know about the concept of 'blocked' and 'active'.
     if (!\Drupal::currentUser()->hasPermission('administer users')) {
@@ -131,16 +137,5 @@ public function entityQueryAlter(SelectInterface $query) {
         }
       }
     }
-
-    // Add the filter by role option.
-    if (!empty($this->fieldDefinition->getSetting('handler_settings')['filter'])) {
-      $filter_settings = $this->fieldDefinition->getSetting('handler_settings')['filter'];
-      if ($filter_settings['type'] == 'role') {
-        $tables = $query->getTables();
-        $base_table = $tables['base_table']['alias'];
-        $query->join('users_roles', 'ur', $base_table . '.uid = ur.uid');
-        $query->condition('ur.rid', $filter_settings['role']);
-      }
-    }
   }
 }

core/modules/user/src/Plugin/views/argument/RolesRid.php

@@ -17,7 +17,7 @@
  *
  * @ingroup views_argument_handlers
  *
- * @ViewsArgument("users_roles_rid")
+ * @ViewsArgument("user__roles_rid")
  */
 class RolesRid extends ManyToOne {
 

core/modules/user/src/Plugin/views/field/Roles.php

@@ -79,7 +79,7 @@ public function preRender(&$values) {
 
     if ($uids) {
       $roles = user_roles();
-      $result = $this->database->query('SELECT u.uid, u.rid FROM {users_roles} u WHERE u.uid IN (:uids) AND u.rid IN (:rids)', array(':uids' => $uids, ':rids' => array_keys($roles)));
+      $result = $this->database->query('SELECT u.entity_id as uid, u.roles_target_id as rid FROM {user__roles} u WHERE u.entity_id IN (:uids) AND u.roles_target_id IN (:rids)', array(':uids' => $uids, ':rids' => array_keys($roles)));
       foreach ($result as $role) {
         $this->items[$role->uid][$role->rid]['role'] = String::checkPlain($roles[$role->rid]->label());
         $this->items[$role->uid][$role->rid]['rid'] = $role->rid;

core/modules/user/src/RoleStorage.php

@@ -34,8 +34,8 @@ public function isPermissionInRoles($permission, array $rids) {
    */
   public function deleteRoleReferences(array $rids) {
     // Remove the role from all users.
-    db_delete('users_roles')
-      ->condition('rid', $rids)
+    db_delete('user__roles')
+      ->condition('target_id', $rids)
       ->execute();
   }
 

core/modules/user/src/Tests/UserDeleteTest.php

@@ -28,10 +28,10 @@ function testUserDeleteMultiple() {
     $uids = array($user_a->id(), $user_b->id(), $user_c->id());
 
     // These users should have a role
-    $query = db_select('users_roles', 'r');
+    $query = db_select('user__roles', 'r');
     $roles_created = $query
-      ->fields('r', array('uid'))
-      ->condition('uid', $uids)
+      ->fields('r', array('entity_id'))
+      ->condition('entity_id', $uids)
       ->countQuery()
       ->execute()
       ->fetchField();
@@ -42,10 +42,10 @@ function testUserDeleteMultiple() {
     // Delete the users.
     user_delete_multiple($uids);
     // Test if the roles assignments are deleted.
-    $query = db_select('users_roles', 'r');
+    $query = db_select('user__roles', 'r');
     $roles_after_deletion = $query
-      ->fields('r', array('uid'))
-      ->condition('uid', $uids)
+      ->fields('r', array('entity_id'))
+      ->condition('entity_id', $uids)
       ->countQuery()
       ->execute()
       ->fetchField();

core/modules/user/src/Tests/UserEntityReferenceTest.php

@@ -78,6 +78,7 @@ function testUserSelectionByRole() {
     $user3->addRole($this->role2->id());
     $user3->save();
 
+
     /** @var \Drupal\entity_reference\EntityReferenceAutocomplete $autocomplete */
     $autocomplete = \Drupal::service('entity_reference.autocomplete');
 

core/modules/user/src/Tests/UserEntityTest.php

@@ -39,32 +39,35 @@ public function testUserMethods() {
     $role_storage->create(array('id' => 'test_role_two'))->save();
     $role_storage->create(array('id' => 'test_role_three'))->save();
 
-    $values = array('roles' => array(LanguageInterface::LANGCODE_DEFAULT => array('test_role_one')));
-    $user = new User($values, 'user');
+    $values = array(
+      'uid' => 1,
+      'roles' => array('test_role_one'),
+    );
+    $user = User::create($values);
 
     $this->assertTrue($user->hasRole('test_role_one'));
     $this->assertFalse($user->hasRole('test_role_two'));
-    $this->assertEqual(array('test_role_one'), $user->getRoles());
+    $this->assertEqual(array(DRUPAL_AUTHENTICATED_RID, 'test_role_one'), $user->getRoles());
 
     $user->addRole('test_role_one');
     $this->assertTrue($user->hasRole('test_role_one'));
     $this->assertFalse($user->hasRole('test_role_two'));
-    $this->assertEqual(array('test_role_one'), $user->getRoles());
+    $this->assertEqual(array(DRUPAL_AUTHENTICATED_RID, 'test_role_one'), $user->getRoles());
 
     $user->addRole('test_role_two');
     $this->assertTrue($user->hasRole('test_role_one'));
     $this->assertTrue($user->hasRole('test_role_two'));
-    $this->assertEqual(array('test_role_one', 'test_role_two'), $user->getRoles());
+    $this->assertEqual(array(DRUPAL_AUTHENTICATED_RID, 'test_role_one', 'test_role_two'), $user->getRoles());
 
     $user->removeRole('test_role_three');
     $this->assertTrue($user->hasRole('test_role_one'));
     $this->assertTrue($user->hasRole('test_role_two'));
-    $this->assertEqual(array('test_role_one', 'test_role_two'), $user->getRoles());
+    $this->assertEqual(array(DRUPAL_AUTHENTICATED_RID, 'test_role_one', 'test_role_two'), $user->getRoles());
 
     $user->removeRole('test_role_one');
     $this->assertFalse($user->hasRole('test_role_one'));
     $this->assertTrue($user->hasRole('test_role_two'));
-    $this->assertEqual(array('test_role_two'), $user->getRoles());
+    $this->assertEqual(array(DRUPAL_AUTHENTICATED_RID, 'test_role_two'), $user->getRoles());
   }
 
 }

core/modules/user/src/Tests/UserRolesAssignmentTest.php

@@ -88,10 +88,10 @@ function testCreateUserWithRole() {
   private function userLoadAndCheckRoleAssigned($account, $rid, $is_assigned = TRUE) {
     $account = user_load($account->id(), TRUE);
     if ($is_assigned) {
-      $this->assertTrue(array_search($rid, $account->getRoles()), 'The role is present in the user object.');
+      $this->assertFalse(array_search($rid, $account->getRoles()) === FALSE, 'The role is present in the user object.');
     }
     else {
-      $this->assertFalse(array_search($rid, $account->getRoles()), 'The role is not present in the user object.');
+      $this->assertTrue(array_search($rid, $account->getRoles()) === FALSE, 'The role is not present in the user object.');
     }
   }
 }

core/modules/user/src/Tests/Views/HandlerFieldRoleTest.php

@@ -41,11 +41,13 @@ public function testRole() {
     $user->addRole($rolename_b);
     $user->save();
 
+    debug(db_query('SELECT * FROM {user__roles}')->fetchAll());
+
     $view = Views::getView('test_views_handler_field_role');
     $this->executeView($view);
     // The role field is populated during preRender.
-    $view->field['rid']->preRender($view->result);
-    $render = $view->field['rid']->advancedRender($view->result[0]);
+    $view->field['roles_target_id']->preRender($view->result);
+    $render = $view->field['roles_target_id']->advancedRender($view->result[0]);
 
     $this->assertEqual($rolename_b . $rolename_a, $render, 'View test_views_handler_field_role renders role assigned to user in the correct order.');
     $this->assertFalse(strpos($render, $rolename_not_assigned), 'View test_views_handler_field_role does not render a role not assigned to a user.');

core/modules/user/src/UserStorage.php

@@ -65,25 +65,6 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
     );
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  function mapFromStorageRecords(array $records) {
-    foreach ($records as $record) {
-      $record->roles = array();
-      if ($record->uid) {
-        $record->roles[] = DRUPAL_AUTHENTICATED_RID;
-      }
-      else {
-        $record->roles[] = DRUPAL_ANONYMOUS_RID;
-      }
-    }
-
-    // Add any additional roles from the database.
-    $this->addRoles($records);
-    return parent::mapFromStorageRecords($records);
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -105,43 +86,6 @@ protected function isColumnSerial($table_name, $schema_name) {
     return $table_name == $this->revisionTable && $schema_name == $this->revisionKey;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function saveRoles(UserInterface $account) {
-    $query = $this->database->insert('users_roles')->fields(array('uid', 'rid'));
-    foreach ($account->getRoles() as $rid) {
-      if (!in_array($rid, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID))) {
-        $query->values(array(
-          'uid' => $account->id(),
-          'rid' => $rid,
-        ));
-      }
-    }
-    $query->execute();
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function addRoles(array $users) {
-    if ($users) {
-      $result = $this->database->query('SELECT rid, uid FROM {users_roles} WHERE uid IN (:uids)', array(':uids' => array_keys($users)));
-      foreach ($result as $record) {
-        $users[$record->uid]->roles[] = $record->rid;
-      }
-    }
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function deleteUserRoles(array $uids) {
-    $this->database->delete('users_roles')
-      ->condition('uid', $uids)
-      ->execute();
-  }
-
   /**
    * {@inheritdoc}
    */

core/modules/user/src/UserStorageInterface.php

@@ -15,27 +15,6 @@
  */
 interface UserStorageInterface extends EntityStorageInterface{
 
-  /**
-   * Add any roles from the storage to the user.
-   *
-   * @param array $users
-   */
-  public function addRoles(array $users);
-
-  /**
-   * Save the user's roles.
-   *
-   * @param \Drupal\user\UserInterface $account
-   */
-  public function saveRoles(UserInterface $account);
-
-  /**
-   * Remove the roles of a user.
-   *
-   * @param array $uids
-   */
-  public function deleteUserRoles(array $uids);
-
   /**
    * Update the last login timestamp of the user.
    *

core/modules/user/src/UserStorageSchema.php

@@ -26,35 +26,6 @@ protected function getEntitySchema(ContentEntityTypeInterface $entity_type, $res
       'user__name' => array('name', 'langcode'),
     );
 
-    $schema['users_roles'] = array(
-      'description' => 'Maps users to roles.',
-      'fields' => array(
-        'uid' => array(
-          'type' => 'int',
-          'unsigned' => TRUE,
-          'not null' => TRUE,
-          'default' => 0,
-          'description' => 'Primary Key: {users}.uid for user.',
-        ),
-        'rid' => array(
-          'type' => 'varchar',
-          'length' => 64,
-          'not null' => TRUE,
-          'description' => 'Primary Key: ID for the role.',
-        ),
-      ),
-      'primary key' => array('uid', 'rid'),
-      'indexes' => array(
-        'rid' => array('rid'),
-      ),
-      'foreign keys' => array(
-        'user' => array(
-          'table' => 'users',
-          'columns' => array('uid' => 'uid'),
-        ),
-      ),
-    );
-
     return $schema;
   }
 

core/modules/user/src/UserViewsData.php

@@ -277,16 +277,16 @@ public function getViewsData() {
       ),
     );
 
-    $data['users_roles']['table']['group']  = t('User');
+    $data['user__roles']['table']['group']  = t('User');
 
-    $data['users_roles']['table']['join'] = array(
+    $data['user__roles']['table']['join'] = array(
       'users' => array(
         'left_field' => 'uid',
-        'field' => 'uid',
+        'field' => 'entity_id',
       ),
     );
 
-    $data['users_roles'