Issue #1776494 by tim.plunkett: Fix failures exposed by plugin instantiation test.

lib/Drupal/views/Plugin/Type/ViewsPluginManager.php

@@ -11,7 +11,6 @@ use Drupal\Component\Plugin\PluginManagerBase;
 use Drupal\Component\Plugin\Factory\DefaultFactory;
 use Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery;
 use Drupal\Core\Plugin\Discovery\CacheDecorator;
-use Drupal\Component\Plugin\Exception\PluginException;
 
 class ViewsPluginManager extends PluginManagerBase {
 
@@ -36,7 +35,7 @@ class ViewsPluginManager extends PluginManagerBase {
     $this->type = $type;
 
     $this->discovery = new CacheDecorator(new AnnotatedClassDiscovery('views', $this->type), 'views:' . $this->type, 'views');
-    $this->factory = new DefaultFactory($this->discovery);
+    $this->factory = new DefaultFactory($this);
     $this->coreModules = views_core_modules();
   }
 
@@ -71,28 +70,8 @@ class ViewsPluginManager extends PluginManagerBase {
       'theme path' => $theme_path,
       'theme file' => $theme_file,
       'parent' => 'parent',
+      'plugin_type' => $this->type,
     );
   }
 
-  /**
-   * Creates a plugin from an ID.
-   *
-   * @param string $plugin_id
-   *   The plugin ID.
-   * @param array|false $definition
-   *   Either the definition array, or FALSE if it needs to be loaded.
-   *
-   * @return Drupal\views\Plugin\views\PluginBase
-   *   The plugin instance.
-   */
-  public function createPluginFromId($plugin_id, $definition = FALSE) {
-    if (!$definition) {
-      $definition = $this->getDefinition($plugin_id);
-    }
-    if (!empty($definition)) {
-      $instance = $this->createInstance($plugin_id, array('type' => $this->type, 'definition' => $definition));
-      return $instance;
-    }
-  }
-
 }

lib/Drupal/views/Plugin/views/HandlerBase.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\views\Plugin\views;
 
+use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
 use Drupal\views\Plugin\views\PluginBase;
 use Drupal\views\View;
 
@@ -72,8 +73,8 @@ abstract class HandlerBase extends PluginBase {
   /**
    * Constructs a Handler object.
    */
-  public function __construct(array $configuration, $plugin_id) {
-    parent::__construct($configuration, $plugin_id);
+  public function __construct(array $configuration, $plugin_id, DiscoveryInterface $discovery) {
+    parent::__construct($configuration, $plugin_id, $discovery);
     $this->is_handler = TRUE;
   }
 
@@ -87,6 +88,7 @@ abstract class HandlerBase extends PluginBase {
    *   based upon the type of handler.
    */
   public function init(&$view, &$options) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $display_id = $this->view->current_display;
     // Check to see if this handler type is defaulted. Note that

lib/Drupal/views/Plugin/views/PluginBase.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\views\Plugin\views;
 
+use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
 use Drupal\Component\Plugin\PluginBase as ComponentPluginBase;
 
 abstract class PluginBase extends ComponentPluginBase {
@@ -56,14 +57,19 @@ abstract class PluginBase extends ComponentPluginBase {
   /**
    * Constructs a Plugin object.
    */
-  public function __construct(array $configuration, $plugin_id) {
-    $this->configuration = $configuration;
-    $this->plugin_id = $plugin_id;
-    $this->plugin_type = $configuration['type'];
-    $this->definition = $configuration['definition'];
+  public function __construct(array $configuration, $plugin_id, DiscoveryInterface $discovery) {
+    parent::__construct($configuration, $plugin_id, $discovery);
+
+    $this->definition = $this->discovery->getDefinition($plugin_id) + $configuration;
+
+    // @todo Change calls to $this->plugin_type to use the definition directly.
+    $this->plugin_type = $this->definition['plugin_type'];
+
+    // @todo Change calls to $this->real_field to use the definition directly.
     if (isset($this->definition['field'])) {
       $this->real_field = $this->definition['field'];
     }
+
     $this->construct();
   }
 
@@ -97,7 +103,6 @@ abstract class PluginBase extends ComponentPluginBase {
    * easily construct them with variable arguments.
    */
   public function construct() {
-    $this->setOptionDefaults($this->options, $this->defineOptions());
   }
 
   protected function setOptionDefaults(&$storage, $options, $level = 0) {

lib/Drupal/views/Plugin/views/access/AccessPluginBase.php

@@ -31,6 +31,7 @@ abstract class AccessPluginBase extends PluginBase {
    *   The display handler.
    */
   public function init(&$view, &$display) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Plugin/views/area/AreaPluginBase.php

@@ -31,6 +31,7 @@ abstract class AreaPluginBase extends HandlerBase {
    * is empty.
    */
   public function init(&$view, &$options) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     parent::init($view, $options);
     if ($this->handler_type == 'empty') {
       $this->options['empty'] = TRUE;

lib/Drupal/views/Plugin/views/argument_default/ArgumentDefaultPluginBase.php

@@ -34,6 +34,7 @@ abstract class ArgumentDefaultPluginBase extends PluginBase {
    * it is linked to.
    */
   public function init(&$view, &$argument, $options) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->argument = &$argument;
 

lib/Drupal/views/Plugin/views/argument_validator/ArgumentValidatorPluginBase.php

@@ -27,6 +27,7 @@ abstract class ArgumentValidatorPluginBase extends PluginBase {
    * it is linked to.
    */
   public function init(&$view, &$argument, $options) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->argument = &$argument;
 

lib/Drupal/views/Plugin/views/cache/CachePluginBase.php

@@ -60,6 +60,7 @@ abstract class CachePluginBase extends PluginBase {
    *   The display handler.
    */
   public function init(&$view, &$display) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php

@@ -86,6 +86,7 @@ abstract class DisplayPluginBase extends PluginBase {
   protected $usesAttachments = FALSE;
 
   public function init(&$view, &$display, $options = NULL) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Plugin/views/display_extender/DisplayExtenderPluginBase.php

@@ -18,6 +18,7 @@ use Drupal\Core\Annotation\Translation;
 abstract class DisplayExtenderPluginBase extends PluginBase {
 
   public function init(&$view, &$display) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = $view;
     $this->display = $display;
   }

lib/Drupal/views/Plugin/views/exposed_form/ExposedFormPluginBase.php

@@ -38,6 +38,7 @@ abstract class ExposedFormPluginBase extends PluginBase {
    *   The display handler.
    */
   public function init(&$view, &$display, $options = array()) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Plugin/views/field/Markup.php

@@ -26,11 +26,8 @@ use Drupal\Core\Annotation\Plugin;
  */
 class Markup extends FieldPluginBase {
 
-  /**
-   * Constructor; calls to base object constructor.
-   */
-  public function construct() {
-    parent::construct();
+  public function init(&$view, &$options) {
+    parent::init($view, $options);
 
     $this->format = $this->definition['format'];
 

lib/Drupal/views/Plugin/views/filter/Broken.php

@@ -24,6 +24,7 @@ class Broken extends FilterPluginBase {
     return t('Broken/missing handler');
   }
 
+  public function init(&$view, &$options) { }
   public function defineOptions() { return array(); }
   public function ensureMyTable() { /* No table to ensure! */ }
   public function query($group_by = FALSE) { /* No query to run */ }

lib/Drupal/views/Plugin/views/localization/LocalizationPluginBase.php

@@ -34,6 +34,7 @@ abstract class LocalizationPluginBase extends PluginBase {
    *   The view object.
    */
   public function init(&$view) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
   }
 

lib/Drupal/views/Plugin/views/pager/PagerPluginBase.php

@@ -40,6 +40,7 @@ abstract class PagerPluginBase extends PluginBase {
    *   The display handler.
    */
   public function init(&$view, &$display, $options = array()) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Plugin/views/query/QueryPluginBase.php

@@ -25,6 +25,7 @@ abstract class QueryPluginBase extends PluginBase implements QueryInterface {
    * Constructor; Create the basic query object and fill with default values.
    */
   public function init($base_table, $base_field, $options) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->base_table = $base_table;
     $this->base_field = $base_field;
     $this->unpackOptions($this->options, $options);

lib/Drupal/views/Plugin/views/relationship/RelationshipPluginBase.php

@@ -48,6 +48,7 @@ abstract class RelationshipPluginBase extends HandlerBase {
    * the table they operate on.
    */
   public function init(&$view, &$options) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     parent::init($view, $options);
     if (isset($this->definition['relationship table'])) {
       $this->table = $this->definition['relationship table'];

lib/Drupal/views/Plugin/views/row/RowPluginBase.php

@@ -42,6 +42,7 @@ abstract class RowPluginBase extends PluginBase {
    * Initialize the row plugin.
    */
   public function init(&$view, &$display, $options = NULL) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Plugin/views/style/StylePluginBase.php

@@ -90,6 +90,7 @@ abstract class StylePluginBase extends PluginBase {
    *   from at least two locations. If it's not included, look on the display.
    */
   public function init(&$view, &$display, $options = NULL) {
+    $this->setOptionDefaults($this->options, $this->defineOptions());
     $this->view = &$view;
     $this->display = &$display;
 

lib/Drupal/views/Tests/HandlersTest.php

@@ -67,7 +67,7 @@ class HandlersTest extends ViewTestBase {
     $this->assertEqual($empty_stdclass, views_break_phrase_string('', $null));
 
     $handler = views_get_handler('node', 'title', 'argument');
-    $this->assertEqual($handler, views_break_phrase_string('', $handler));
+    $this->assertEqual($handler, views_break_phrase_string('', $handler), 'The views_break_phrase_string() works correctly.');
 
     // test ors
     $handler = views_break_phrase_string('word1 word2+word');
@@ -122,7 +122,7 @@ class HandlersTest extends ViewTestBase {
     $this->assertEqual($empty_stdclass, views_break_phrase('', $null));
 
     $handler = views_get_handler('node', 'title', 'argument');
-    $this->assertEqual($handler, views_break_phrase('', $handler));
+    $this->assertEqual($handler, views_break_phrase('', $handler), 'The views_break_phrase() function works correctly.');
 
     // Generate three random numbers which can be used below;
     $n1 = rand(0, 100);

lib/Drupal/views/Tests/ViewTestBase.php

@@ -61,6 +61,9 @@ abstract class ViewTestBase extends WebTestBase {
     }
     $query->execute();
     $this->checkPermissions(array(), TRUE);
+
+    // Reset the test view, in case it was dependent on the test data module.
+    $this->view = $this->getBasicView();
   }
 
   /**
@@ -403,7 +406,10 @@ abstract class ViewTestBase extends WebTestBase {
    *   A View instance.
    */
   protected function createViewFromConfig($view_name) {
-    module_enable(array('views_test_config'));
+    if (!module_exists('views_test_config')) {
+      module_enable(array('views_test_config'));
+    }
+
     $data = config("views.view.$view_name")->get();
 
     $view = entity_create('view', $data);

views.module

@@ -12,8 +12,8 @@
 use Drupal\Core\Database\Query\AlterableInterface;
 use Drupal\views\TempStore\UserTempStore;
 use Drupal\views\View;
-use Drupal\Component\Plugin\PluginManagerInterface;
 use Drupal\views\Plugin\Type\ViewsPluginManager;
+use Drupal\Component\Plugin\Exception\PluginException;
 
 /**
  * Views API version string.
@@ -1264,7 +1264,7 @@ function views_include_handlers($reset = FALSE) {
  *   The name of the table this handler is from.
  * @param $field
  *   The name of the field this handler is from.
- * @param $key
+ * @param $type
  *   The type of handler. i.e, sort, field, argument, filter, relationship
  * @param $override
  *   Override the actual handler object with this class. Used for
@@ -1274,7 +1274,7 @@ function views_include_handlers($reset = FALSE) {
  * @return views_handler
  *   An instance of a handler object. May be views_handler_broken.
  */
-function views_get_handler($table, $field, $key, $override = NULL) {
+function views_get_handler($table, $field, $type, $override = NULL) {
   static $recursion_protection = array();
 
   $data = views_fetch_data($table, FALSE);
@@ -1292,11 +1292,11 @@ function views_get_handler($table, $field, $key, $override = NULL) {
     $moved = $data[$field]['moved to'];
   }
   // Support conversion on handler level.
-  if (isset($data[$field][$key]['moved to'])) {
-    $moved = $data[$field][$key]['moved to'];
+  if (isset($data[$field][$type]['moved to'])) {
+    $moved = $data[$field][$type]['moved to'];
   }
 
-  if (isset($data[$field][$key]) || !empty($moved)) {
+  if (isset($data[$field][$type]) || !empty($moved)) {
     if (!empty($moved)) {
       list($moved_table, $moved_field) = $moved;
       if (!empty($recursion_protection[$moved_table][$moved_field])) {
@@ -1305,7 +1305,7 @@ function views_get_handler($table, $field, $key, $override = NULL) {
       }
 
       $recursion_protection[$moved_table][$moved_field] = TRUE;
-      $handler = views_get_handler($moved_table, $moved_field, $key, $override);
+      $handler = views_get_handler($moved_table, $moved_field, $type, $override);
       $recursion_protection = array();
       if ($handler) {
         // store these values so we know what we were originally called.
@@ -1321,16 +1321,15 @@ function views_get_handler($table, $field, $key, $override = NULL) {
 
     // @fixme: temporary.
     // Set up a default handler, if both handler and id is not specified.
-    if (empty($data[$field][$key]['handler']) && empty($data[$field][$key]['id'])) {
-      $data[$field][$key]['id'] = 'standard';
+    if (empty($data[$field][$type]['handler']) && empty($data[$field][$type]['id'])) {
+      $data[$field][$type]['id'] = 'standard';
     }
 
     if ($override) {
-      $data[$field][$key]['override handler'] = $override;
+      $data[$field][$type]['override handler'] = $override;
     }
 
-    $definition = $data[$field][$key];
-    $type = $key;
+    $definition = $data[$field][$type];
     foreach (array('group', 'title', 'title short', 'help', 'real field') as $key) {
       if (!isset($definition[$key])) {
         // First check the field level
@@ -1347,32 +1346,24 @@ function views_get_handler($table, $field, $key, $override = NULL) {
     // @todo This is crazy. Find a way to remove the override functionality.
     $manager = new ViewsPluginManager($type);
     $plugin_id = !empty($definition['override handler']) ? $definition['override handler'] : $definition['id'];
+    // Try to use the overridden handler.
     try {
-      return $manager->createPluginFromId($plugin_id, $definition);
+      return $manager->createInstance($plugin_id, $definition);
     }
     catch (PluginException $e) {
-      return $manager->createPluginFromId($definition['id'], $definition);
+      // If that fails, use the original handler.
+      try {
+        return $manager->createInstance($definition['id'], $definition);
+      }
+      catch (PluginException $e) {
+        // Deliberately empty, this case is handled generically below.
+      }
     }
   }
 
-  if ($handler) {
-    return $handler;
-  }
-
-  // DEBUG -- identify missing handlers
-  debug(t("Missing handler: @table @field @key", array('@table' => $table, '@field' => $field, '@key' => $key)));
-  $broken = array(
-    'title' => t('Broken handler @table.@field', array('@table' => $table, '@field' => $field)),
-    'id' => 'broken',
-    'table' => $table,
-    'field' => $field,
-  );
-
-  // In case the manager was already initialized above, try to reuse it.
-  if (!isset($manager)) {
-    $manager = new ViewsPluginManager($key);
-  }
-  return $manager->createPluginFromId('broken');
+  // Finally, use the 'broken' handler.
+  debug(t("Missing handler: @table @field @type", array('@table' => $table, '@field' => $field, '@type' => $type)));
+  return views_get_plugin($type, 'broken');
 }
 
 /**
@@ -1442,7 +1433,7 @@ function views_fetch_plugin_names($type, $key = NULL, $base = array()) {
  */
 function views_get_plugin($type, $plugin_id) {
   $manager = new ViewsPluginManager($type);
-  return $manager->createPluginFromId($plugin_id);
+  return $manager->createInstance($plugin_id);
 }
 
 /**