Go through all of the Configurables code, add/update documentation, and clean-up code.

lib/Drupal/views/Tests/ViewStorageTest.php

@@ -34,6 +34,20 @@ class ViewStorageTest extends WebTestBase {
     'display',
   );
 
+  /**
+   * The Configurable information from entity_get_info().
+   *
+   * @var array
+   */
+  protected $info;
+
+  /**
+   * The Configurable controller.
+   *
+   * @var Drupal\views\ViewStorageController
+   */
+  protected $controller;
+
   /**
    * Modules to enable.
    *
@@ -58,7 +72,7 @@ class ViewStorageTest extends WebTestBase {
     $this->controller = entity_get_controller('view');
 
     // Confirm that an info array has been returned.
-    $this->assertTrue(!empty($this->info) && is_array($this->info), 'The View  info array is loaded.');
+    $this->assertTrue(!empty($this->info) && is_array($this->info), 'The View info array is loaded.');
 
     // Confirm we have the correct controller class.
     $this->assertTrue($this->controller instanceof ViewStorageController, 'The correct controller is loaded.');

lib/Drupal/views/ViewDisplay.php

@@ -15,13 +15,27 @@ namespace Drupal\views;
  */
 class ViewDisplay {
 
+  /**
+   * The display plugin ID.
+   *
+   * @var string
+   */
+  public $display_plugin;
+
   /**
    * The display handler itself, which has all the methods.
    *
-   * @var views_plugin_display
+   * @var Drupal\views\Plugin\views\display\DisplayPluginBase
    */
   public $handler;
 
+  /**
+   * The machine name of this display.
+   *
+   * @var string
+   */
+  public $id;
+
   /**
    * Stores all options of the display, like fields, filters etc.
    *
@@ -29,6 +43,13 @@ class ViewDisplay {
    */
   public $display_options;
 
+  /**
+   * The human-readable name of this display.
+   *
+   * @var string
+   */
+  public $display_title;
+
   /**
    * Constructs a ViewDisplay object.
    *

lib/Drupal/views/ViewStorage.php

@@ -43,17 +43,20 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Add a new display handler to the view, automatically creating an id.
+   * Adds a new display handler to the view, automatically creating an ID.
    *
-   * @param $plugin_id
+   * @param string $plugin_id
-   *   The plugin type from the views plugin data. Defaults to 'page'.
+   *   (optional) The plugin type from the Views plugin annotation. Defaults to
-   * @param $title
+   *   'page'.
-   *   The title of the display; optional, may be filled in from default.
+   * @param string $title
-   * @param $id
+   *   (optional) The title of the display. Defaults to NULL.
-   *   The id to use.
+   * @param string $id
-   * @return
+   *   (optional) The ID to use, e.g., 'default', 'page_1', 'block_2'. Defaults
-   *   The key to the display in $view->display, so that the new display
+   *   to NULL.
-   *   can be easily located.
+   *
+   * @return string|false
+   *   The key to the display in $view->display, or FALSE if no plugin ID was
+   *   provided.
    */
   function add_display($plugin_id = 'page', $title = NULL, $id = NULL) {
     if (empty($plugin_id)) {
@@ -65,13 +68,11 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
       $plugin['title'] = t('Broken');
     }
 
-
     if (empty($id)) {
       $id = $this->generate_display_id($plugin_id);
 
-      // Generate a unique human readable name.
+      // Generate a unique human-readable name by inspecting the counter at the
-      // Therefore find out how often the display_plugin already got used,
+      // end of the previous display ID, e.g., 'page_1'.
-      // which is stored at the end of the $id, for example page_1.
       if ($id !== 'default') {
         preg_match("/[0-9]+/", $id, $count);
         $count = $count[0];
@@ -81,13 +82,11 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
       }
 
       if (empty($title)) {
-        // If we had more then one instance already attach the count,
+        // If there is no title provided, use the plugin title, and if there are
-        // so you end up with "Page" and "Page 2" for example.
+        // multiple displays, append the count.
+        $title = $plugin['title'];
         if ($count > 1) {
-          $title = $plugin['title'] . ' ' . $count;
+          $title .= ' ' . $count;
-        }
-        else {
-          $title = $plugin['title'];
         }
       }
     }
@@ -107,44 +106,46 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Generate a display id of a certain plugin type.
+   * Generates a display ID of a certain plugin type.
    *
-   * @param $type
+   * @param string $plugin_id
-   *   Which plugin should be used for the new display id.
+   *   Which plugin should be used for the new display ID.
    */
-  function generate_display_id($type) {
+  function generate_display_id($plugin_id) {
     // 'default' is singular and is unique, so just go with 'default'
     // for it. For all others, start counting.
-    if ($type == 'default') {
+    if ($plugin_id == 'default') {
       return 'default';
     }
-    // Initial id.
+    // Initial ID.
-    $id = $type . '_1';
+    $id = $plugin_id . '_1';
     $count = 1;
 
     // Loop through IDs based upon our style plugin name until
     // we find one that is unused.
     while (!empty($this->display[$id])) {
-      $id = $type . '_' . ++$count;
+      $id = $plugin_id . '_' . ++$count;
     }
 
     return $id;
   }
 
   /**
-   * Generates a unique ID for an item.
+   * Generates a unique ID for an handler instance.
    *
-   * These items are typically fields, filters, sort criteria, or arguments.
+   * These handler instances are typically fields, filters, sort criteria, or
+   * arguments.
    *
-   * @param $requested_id
+   * @param string $requested_id
-   *   The requested ID for the item.
+   *   The requested ID for the handler instance.
-   * @param $existing_items
+   * @param array $existing_items
-   *   An array of existing items, keyed by their IDs.
+   *   An array of existing handler instancess, keyed by their IDs.
    *
-   * @return
+   * @return string
-   *   A unique ID. This will be equal to $requested_id if no item with that ID
+   *   A unique ID. This will be equal to $requested_id if no handler instance
-   *   already exists. Otherwise, it will be appended with an integer to make
+   *   with that ID already exists. Otherwise, it will be appended with an
-   *   it unique, e.g. "{$requested_id}_1", "{$requested_id}_2", etc.
+   *   integer to make it unique, e.g., "{$requested_id}_1",
+   *   "{$requested_id}_2", etc.
    */
   public static function generate_item_id($requested_id, $existing_items) {
     $count = 0;
@@ -156,20 +157,24 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Create a new display and a display handler for it.
+   * Creates a new display and a display handler for it.
-   * @param $type
+   *
-   *   The plugin type from the views plugin data. Defaults to 'page'.
+   * @param string $plugin_id
-   * @param $title
+   *   (optional) The plugin type from the Views plugin annotation. Defaults to
-   *   The title of the display; optional, may be filled in from default.
+   *   'page'.
-   * @param $id
+   * @param string $title
-   *   The id to use.
+   *   (optional) The title of the display. Defaults to NULL.
-   * @return views_plugin_display
+   * @param string $id
+   *   (optional) The ID to use, e.g., 'default', 'page_1', 'block_2'. Defaults
+   *   to NULL.
+   *
+   * @return Drupal\views\Plugin\views\display\DisplayPluginBase
    *   A reference to the new handler object.
    */
-  function &new_display($type = 'page', $title = NULL, $id = NULL) {
+  function &new_display($plugin_id = 'page', $title = NULL, $id = NULL) {
-    $id = $this->add_display($type, $title, $id);
+    $id = $this->add_display($plugin_id, $title, $id);
 
-    // Create a handler
+    // Create a handler.
     $this->display[$id]->handler = views_get_plugin('display', $this->display[$id]->display_plugin);
     if (empty($this->display[$id]->handler)) {
       // provide a 'default' handler as an emergency. This won't work well but
@@ -190,9 +195,26 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Add an item with a handler to the view.
+   * Adds an instance of a handler to the view.
    *
-   * These items may be fields, filters, sort criteria, or arguments.
+   * Items may be fields, filters, sort criteria, or arguments.
+   *
+   * @param string $display_id
+   *   The machine name of the display.
+   * @param string $type
+   *   The type of handler being added.
+   * @param string $table
+   *   The name of the table this handler is from.
+   * @param string $field
+   *   The name of the field this handler is from.
+   * @param array $options
+   *   (optional) Extra options for this instance. Defaults to an empty array.
+   * @param string $id
+   *   (optional) A unique ID for this handler instance. Defaults to NULL, in
+   *   which case one will be generated.
+   *
+   * @return string
+   *   The unique ID for this handler instance.
    */
   function add_item($display_id, $type, $table, $field, $options = array(), $id = NULL) {
     $types = View::views_object_types();
@@ -204,29 +226,34 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
       $id = $this->generate_item_id($field, $fields);
     }
 
-    $new_item = array(
+    // If the desired type is not found, use the original value directly.
+    $handler_type = !empty($types[$type]['type']) ? $types[$type]['type'] : $type;
+
+    // @todo This variable is never used.
+    $handler = views_get_handler($table, $field, $handler_type);
+
+    $fields[$id] = array(
       'id' => $id,
       'table' => $table,
       'field' => $field,
     ) + $options;
 
-    if (!empty($types[$type]['type'])) {
-      $handler_type = $types[$type]['type'];
-    }
-    else {
-      $handler_type = $type;
-    }
-
-    $handler = views_get_handler($table, $field, $handler_type);
-
-    $fields[$id] = $new_item;
     $this->display[$display_id]->handler->set_option($types[$type]['plural'], $fields);
 
     return $id;
   }
 
   /**
-   * Get an array of items for the current display.
+   * Gets an array of handler instances for the current display.
+   *
+   * @param string $type
+   *   The type of handlers to retrieve.
+   * @param string $display_id
+   *   (optional) A specific display machine name to use. If NULL, the current
+   *   display will be used.
+   *
+   * @return array
+   *   An array of handler instances of a given type for this display.
    */
   function get_items($type, $display_id = NULL) {
     $this->set_display($display_id);
@@ -241,8 +268,18 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Get the configuration of an item (field/sort/filter/etc) on a given
+   * Gets the configuration of a handler instance on a given display.
-   * display.
+   *
+   * @param string $display_id
+   *   The machine name of the display.
+   * @param string $type
+   *   The type of handler to retrieve.
+   * @param string $id
+   *   The ID of the handler to retrieve.
+   *
+   * @return array|null
+   *   Either the handler instance's configuration, or NULL if the handler is
+   *   not used on the display.
    */
   function get_item($display_id, $type, $id) {
     // Get info about the types so we can get the right data.
@@ -257,18 +294,26 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Set the configuration of an item (field/sort/filter/etc) on a given
+   * Sets the configuration of a handler instance on a given display.
-   * display.
    *
-   * Pass in NULL for the $item to remove an item.
+   * @param string $display_id
+   *   The machine name of the display.
+   * @param string $type
+   *   The type of handler being set.
+   * @param string $id
+   *   The ID of the handler being set.
+   * @param array|null $item
+   *   An array of configuration for a handler, or NULL to remove this instance.
+   *
+   * @see set_item_option()
    */
   function set_item($display_id, $type, $id, $item) {
     // Get info about the types so we can get the right data.
     $types = View::views_object_types();
-    // Initialize the display
+    // Initialize the display.
     $this->set_display($display_id);
 
-    // Get the existing configuration
+    // Get the existing configuration.
     $fields = $this->display[$display_id]->handler->get_option($types[$type]['plural']);
     if (isset($item)) {
       $fields[$id] = $item;
@@ -282,11 +327,24 @@ class ViewStorage extends ConfigurableBase implements ViewStorageInterface {
   }
 
   /**
-   * Set an option on an item.
+   * Sets an option on a handler instance.
    *
-   * Use this only if you have just 1 or 2 options to set; if you have
+   * Use this only if you have just 1 or 2 options to set; if you have many,
-   * many, consider getting the item, adding the options and doing
+   * consider getting the handler instance, adding the options and using
-   * set_item yourself.
+   * set_item() directly.
+   *
+   * @param string $display_id
+   *   The machine name of the display.
+   * @param string $type
+   *   The type of handler being set.
+   * @param string $id
+   *   The ID of the handler being set.
+   * @param string $option
+   *   The configuration key for the value being set.
+   * @param mixed $value
+   *   The value being set.
+   *
+   * @see set_item()
    */
   function set_item_option($display_id, $type, $id, $option, $value) {
     $item = $this->get_item($display_id, $type, $id);

lib/Drupal/views/ViewStorageController.php

@@ -18,11 +18,11 @@ class ViewStorageController extends ConfigStorageController {
   /**
    * Overrides Drupal\config\ConfigStorageController::attachLoad();
    */
-  protected function attachLoad(&$queried_entities, $revision_id = FALSE) {
+  protected function attachLoad(&$queried_objects, $revision_id = FALSE) {
-    foreach ($queried_entities as $id => $entity) {
+    foreach ($queried_objects as $id => $configurable) {
       // @todo This property is left in for CTools export UI.
-      $entity->type = t('Normal');
+      $configurable->type = t('Normal');
-      $this->attachDisplays($entity);
+      $this->attachDisplays($configurable);
     }
   }
 
@@ -30,31 +30,31 @@ class ViewStorageController extends ConfigStorageController {
    * Overrides Drupal\config\ConfigStorageController::save().
    *
    * This currently replaces the reflection code with a static array of
-   * properties to be set on the config object. This can be removed
+   * properties to be set on the config object. This can be removed when the
-   * when the view storage is isolated so the ReflectionClass can work.
+   * view storage is isolated so the ReflectionClass can work.
    */
-  public function save(StorableInterface $entity) {
+  public function save(StorableInterface $configurable) {
     $prefix = $this->entityInfo['config prefix'] . '.';
 
-    // Load the stored entity, if any.
+    // Load the stored configurable, if any, and rename it.
-    if ($entity->getOriginalID()) {
+    if ($configurable->getOriginalID()) {
-      $id = $entity->getOriginalID();
+      $id = $configurable->getOriginalID();
     }
     else {
-      $id = $entity->id();
+      $id = $configurable->id();
     }
     $config = config($prefix . $id);
-    $config->setName($prefix . $entity->id());
+    $config->setName($prefix . $configurable->id());
 
-    if (!$config->isNew() && !isset($entity->original)) {
+    if (!$config->isNew() && !isset($configurable->original)) {
-      $entity->original = entity_load_unchanged($this->entityType, $id);
+      $configurable->original = entity_load_unchanged($this->entityType, $id);
     }
 
-    $this->preSave($entity);
+    $this->preSave($configurable);
-    $this->invokeHook('presave', $entity);
+    $this->invokeHook('presave', $configurable);
 
-    // @todo: This temp measure will be removed once we have a better way or
+    // @todo This temp measure will be removed once we have a better way or
-    // separation of storage and the executed view.
+    //   separation of storage and the executed view.
     $config_properties = array (
       'disabled',
       'api_version',
@@ -70,7 +70,7 @@ class ViewStorageController extends ConfigStorageController {
     foreach ($config_properties as $property) {
       if ($property == 'display') {
         $displays = array();
-        foreach ($entity->display as $key => $display) {
+        foreach ($configurable->display as $key => $display) {
           $displays[$key] = array(
             'display_options' => $display->display_options,
             'display_plugin' => $display->display_plugin,
@@ -82,28 +82,28 @@ class ViewStorageController extends ConfigStorageController {
         $config->set('display', $displays);
       }
       else {
-        $config->set($property, $entity->$property);
+        $config->set($property, $configurable->$property);
       }
     }
 
     if (!$config->isNew()) {
       $return = SAVED_NEW;
       $config->save();
-      $this->postSave($entity, TRUE);
+      $this->postSave($configurable, TRUE);
-      $this->invokeHook('update', $entity);
+      $this->invokeHook('update', $configurable);
     }
     else {
       $return = SAVED_UPDATED;
       $config->save();
-      $entity->enforceIsNew(FALSE);
+      $configurable->enforceIsNew(FALSE);
-      $this->postSave($entity, FALSE);
+      $this->postSave($configurable, FALSE);
-      $this->invokeHook('insert', $entity);
+      $this->invokeHook('insert', $configurable);
     }
 
     // Clear caches.
     views_invalidate_cache();
 
-    unset($entity->original);
+    unset($configurable->original);
 
     return $return;
   }
@@ -124,22 +124,22 @@ class ViewStorageController extends ConfigStorageController {
       )
     );
 
-    $entity = parent::create($values);
+    $configurable = parent::create($values);
 
-    $this->attachDisplays($entity);
+    $this->attachDisplays($configurable);
-    return $entity;
+    return $configurable;
   }
 
   /**
    * Attaches an array of ViewDisplay objects to the view display property.
    *
-   * @param Drupal\entity\StorableInterface $entity
+   * @param Drupal\entity\StorableInterface $configurable
    */
-  protected function attachDisplays(StorableInterface $entity) {
+  protected function attachDisplays(StorableInterface $configurable) {
-    if (isset($entity->display) && is_array($entity->display)) {
+    if (isset($configurable->display) && is_array($configurable->display)) {
       $displays = array();
 
-      foreach ($entity->get('display') as $key => $options) {
+      foreach ($configurable->get('display') as $key => $options) {
         $options += array(
           'display_options' => array(),
           'display_plugin' => NULL,
@@ -151,7 +151,7 @@ class ViewStorageController extends ConfigStorageController {
         $displays[$key] = new ViewDisplay($options);
       }
 
-      $entity->set('display', $displays);
+      $configurable->set('display', $displays);
     }
   }
 

views.module

@@ -1178,7 +1178,7 @@ function views_module_include($api, $reset = FALSE) {
     }
   }
 
-  // @todo replace with http://drupal.org/node/1741154.
+  // @todo Replace with http://drupal.org/node/1760284.
   ctools_include('plugins');
   return ctools_plugin_api_include('views', $api, views_api_minimum_version(), views_api_version());
 }
@@ -1194,7 +1194,7 @@ function views_get_module_apis($api = 'views', $reset = FALSE) {
     }
   }
 
-  // @todo replace with http://drupal.org/node/1741154.
+  // @todo Replace with http://drupal.org/node/1760284.
   ctools_include('plugins');
   return ctools_plugin_api_info('views', $api, views_api_minimum_version(), views_api_version());
 }
@@ -1507,9 +1507,7 @@ function views_get_all_templates() {
  *   it can be successfully saved.
  */
 function views_new_view() {
-  $view = entity_create('view', array());
+  return entity_create('view', array());
-
-  return $view;
 }
 
 /**
@@ -1569,9 +1567,7 @@ function views_get_applicable_views($type) {
  *   If TRUE, reset the static cache forcing views to be reloaded.
  */
 function views_get_all_views($reset = FALSE) {
-  // @todo replace with http://drupal.org/node/1741154.
+  return entity_get_controller('view')->load();
-  $controller = entity_get_controller('view');
-  return $controller->load();
 }
 
 /**
@@ -1584,8 +1580,7 @@ function views_get_all_views($reset = FALSE) {
  *   The view which is loaded.
  */
 function views_storage_load($id) {
-  $controller = entity_get_controller('view');
+  $result = entity_get_controller('view')->load(array($id));
-  $result = $controller->load(array($id));
   return reset($result);
 }
 
@@ -1600,8 +1595,7 @@ function views_storage_load($id) {
  *   performed.
  */
 function views_storage_save(View $view) {
-  $controller = entity_get_controller('view');
+  return entity_get_controller('view')->save($view);
-  return $controller->save($view);
 }
 
 /**