DX: D8 Plugin Type writing

I'd like to build a plugin type using plugin system in drupal 8 for field validation validator.
I have build a plugin type for field validation validator in Drupal 7 using ctools, it works great.

Copying from http://drupal.org/node/1497366#comment-6209878 after examining #1675048: DX: D8 Plugin writing and #1672776: Convert Aggregator Fetchers to Plugins:

- While it makes sense to me that I should somehow "declare" a plugin type, the naming and usage of the PluginType classes in the two issues
made me a bit confused and uneasy. Unsure how the problem should be addressed.

Here's what I saw:

  $messenger_factory = new Messenger();
  foreach ($messenger_factory->getPluginDefinitions() as $plugin_id => $definition) { // something... }

from the first issue, and from the second:

$fetcher_lister = new FetcherType();
foreach ($fetcher_lister->getDefinitions() as $id => $definition) { // something... }
// and in another place
$fetcher_factory = new FetcherType();
$success = $fetcher_factory->createInstance($fetcher)->fetch($feed);

"Messenger" makes me think it's the actual plugin (and the patch in this issue did the same with "Effect').
"FetcherType" also throws me off track, making me think there are multiple types of fetchers, instead of telling me "this is the god object that proxies to both discovery and factory.".
And the variables they are assigned to are always slightly imprecise, since they attempt to describe it as a discovery or a factory mechanism, not both.
Earl had an idea in #1672776: Convert Aggregator Fetchers to Plugins to name the "god object" -> Manager or Arbiter, which would bring clarity but require some reachitecturing.

Perhaps the solution is to just be more verbose:

$plugin_type = new FetcherPluginType();
foreach ($plugin_type->getDefinitions() as $id => definition) {}
// and for message
$plugin_type = new MessagePluginType();
foreach ($plugin_type->getDefinitions() as $id => definition) {}

This brings the question of do we want to mention "plugin" at all in the end user API.
And of course there's always the idea of just splitting the PluginType into two objects (as effulgentsia suggested in #154).
I'm not sure what other logic might live in the PluginType classes, so that might or might not be practical.

Thanks for the review, bojanz. I'd like to see a few more here before thinking about solutions, but you're hitting on exactly the kind of stuff that I was hoping would surface from this issue.

For those who haven't provided feedback yet, in case it helps, I'm attaching the relevant bit from #1497366-176: Introduce Plugin System to Core. The example module in the issue summary is a great way to onboard to the proposed plugin system, while this patch is a way to see how an existing module implementing a plugin-like system already (aggregator fetchers) needs to change in order to use the standardized plugin system being proposed. As in #2, feel free to post feedback about either the example module or this patch or both.

This patch just deals with the "plugin type writing" part. The corresponding "plugin writing" part is in #1675048-12: DX: D8 Plugin writing.

I have build a plugin type for field validation. It works, The code could be download at http://drupal.org/node/1677498.
I have a problem, the last argument of construct function in FieldValidationValidator is passing by reference, In Drupal 7, I use ctools, I could get the the class name, then I could call it directly using this code:

$validator = new $class($entity_type, $entity, $field, $instance, $langcode, $items, $delta, $item, $value, $rule, $errors);

$errors which pass by reference works correctly.

When in Drupal 8, I have to using this code:

 $configuration = array(
            'entity_type' => $entity_type,
            'entity' => $entity,
            'field' => $field,
            'instance' => $instance,
            'langcode' => $langcode,
            'items' => $items,
            'delta' => $delta,
            'item' => $item,
            'value' => $value,
            'rule' => $rule,
            'errors' => &$errors,
		  );

		  $validator_factory = new Validator();
		  $validator = $validator_factory->createPluginInstance($rule->validator,$configuration);

$errors should pass by reference, but it does not work.

Is there any way that I could use my old method instead of using "->createPluginInstance()".

Here is my code:

validator plugin:

namespace Drupal\field_validation\Plugins\field_validation\validator;

use Drupal\Component\Plugin\PluginAbstract;
use Drupal\Component\Plugin\Plugin;
use Drupal\field_validation\Plugins\field_validation\validator\FieldValidationValidator;

class FieldValidationEmailValidator extends FieldValidationValidator {

  /**
   * Validate field. 
   */
  public function validate() {
    //drupal_set_message('abcdefg');
    if ($this->value != '' && (!valid_email_address($this->value))) {
	//drupal_set_message('abcdefg123');
      $this->setError();
    }
  }

}

abstract class:

namespace Drupal\field_validation\Plugins\field_validation\validator;

use Drupal\Component\Plugin\PluginAbstract;
use Drupal\Component\Plugin\Plugin;

abstract class FieldValidationValidator extends PluginAbstract {
  // Variables associated with validation.
  protected $entity_type;
  protected $entity;
  protected $field;
  protected $instance;
  protected $langcode;
  protected $items;
  protected $delta;
  protected $item;
  protected $value;
  protected $rule;
  protected $errors;

  /**
   * Save arguments locally.
   */
  function __construct($entity_type = 'node', $entity = NULL, $field = '', $instance = NULL, $langcode = 'und', $items = array(), $delta = 0, $item = array(), $value = '', $rule = NULL, &$errors = array()) {
    $this->entity_type = $entity_type;
    $this->entity = $entity;
    $this->field = $field;
    $this->instance = $instance;
    $this->langcode = $langcode;
    $this->items = $items;
    $this->delta = $delta;
    $this->item = $item;
    $this->value = $value;
    $this->rule = $rule;
    $this->errors = &$errors;
  }

  /**
   * Validate field. 
   */
  public function validate() {}

  /**
   * Provide settings option
   */
  function settingsForm(&$form, &$form_state) {
    $default_settings = $this->getDefaultSettings($form, $form_state);
    //print debug($default_settings);
    $form['settings']['errors'] = array(
      '#title' => t('Set errors using field API'),
      '#description' => t("There are two methods to set error: using form_set_error provided by form api, using errors provided by field api. form_set_error does not work correctly when a sub form embed into another form. errors does not work correctly when current field does not support hook_field_widget_error."),
      '#type' => 'checkbox',
      '#default_value' => isset($default_settings['errors']) ? $default_settings['errors'] : FALSE,
    );
  }
  /**
   * Return error message string for the validation rule.
   */
  public function getErrorMessage() {
    $error_message = $this->rule->error_message;
    return $error_message;
  }
  
  /**
   * Return error element for the validation rule.
   */
  public function getErrorElement() {
    $error_element = $this->rule->field_name . '][' . $this->langcode . '][' . $this->delta . '][' . $this->rule->col;
    return  $error_element;
  }
  
  /**
   * Return default settingsfor the validator.
   */
  public function getDefaultSettings(&$form, &$form_state) {
    $rule = isset($form_state['item']) ? $form_state['item'] : new stdClass();
    $default_settings = isset($rule->settings) ? $rule->settings : array();
    $default_settings = isset($form_state['values']['settings']) ? $form_state['values']['settings'] : $default_settings;
    return  $default_settings;
  }
  
  /**
   * Set error message.
   */
  public function setError() {
    $error_element = $this->getErrorElement();
    $error_message = t($this->getErrorMessage());
	//drupal_set_message('123ac:'.$error_element.':'.$error_message.':'.$this->rule->settings['errors']);
	//form_set_error($error_element,  check_plain($error_message));
    //We support two methods to set error: using form_set_error provided by form api, using errors provided by field api.
    //form_set_error does not work correctly when a sub form embed into another form; 
    //errors does not work correctly when current field does not support hook_field_widget_error.
    if (empty($this->rule->settings['errors'])) {
      form_set_error($error_element,  check_plain($error_message));
	 // drupal_set_message('123ac:123');
    }
    else{
	  drupal_set_message('123ac:123456');
      $this->errors[$this->rule->field_name][$this->langcode][$this->delta][] = array(
        'error' => $this->rule->col,
        'message' => $error_message,
      );
    }
  }

}

plugin type:

namespace Drupal\field_validation\Plugins\Type;

use Drupal\Component\Plugin\PluginType;
use Drupal\Core\Plugin\Discovery\HookDiscovery;
use Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator;
use Drupal\Component\Plugin\Factory\DefaultFactory;

class Validator extends PluginType {
  public function __construct() {
    $this->discovery = new DerivativeDiscoveryDecorator(new HookDiscovery('validator_info'));
    $this->factory = new DefaultFactory($this);
  }
}

Code from field validation module:

use Drupal\field_validation\Plugins\Type\Validator;


/**
 * Implements hook_field_attach_validate().
 */
function field_validation_field_attach_validate($entity_type, $entity, &$errors) {
  list($id, $vid, $bundle) = entity_extract_ids($entity_type, $entity);
  
  $sql = 'SELECT * FROM {field_validation_rule} WHERE entity_type = :entity_type AND bundle = :bundle';
  $rules = db_query($sql, array(':entity_type' => $entity_type, ':bundle' => $bundle))->fetchAll();
  if ($rules) {
    foreach ($rules as $rule) {
      $rule->settings = unserialize($rule->settings);

	  //drupal_set_message('123456');
	  //debug($rule);
      $field_name = $rule->field_name;
      $field = field_info_field($field_name);
      $instance = field_info_instance($entity_type, $field_name, $bundle);
      $languages = field_available_languages($entity_type, $field);
      foreach ($languages as $langcode) {
        //debug($errors);
        $items = isset($entity->{$field_name}[$langcode]) ? $entity->{$field_name}[$langcode] : array();
        //print debug($rule);
    
        foreach ($items as $delta => $item) {
          $value = $item[$rule->col];
          //$validator = new $class($entity_type, $entity, $field, $instance, $langcode, $items, $delta, $item, $value, $rule, $errors);
		  $configuration = array(
            'entity_type' => $entity_type,
            'entity' => $entity,
            'field' => $field,
            'instance' => $instance,
            'langcode' => $langcode,
            'items' => $items,
            'delta' => $delta,
            'item' => $item,
            'value' => $value,
            'rule' => $rule,
            'errors' => &$errors,
		  );

		  $validator_factory = new Validator();
		  $validator = $validator_factory->createPluginInstance($rule->validator,$configuration);
		  //$validator_definition = $validator_factory->getPluginDefinition($rule->validator);
		  //debug($validator);
          $break = $validator->validate();
		  //debug($errors);
          if (!empty($break)) {
            break;
          }
        }
      }
    }
  }
}

/**
 * Implements hook_messenger_info().
 */
function field_validation_validator_info() {
  return array(
    'email' => array(
      'label' => t('Email'),
      'description' => t('Verifies that user-entered data is a valid email address.'),
      'class' => 'Drupal\field_validation\Plugins\field_validation\validator\FieldValidationEmailValidator',
    ),
    'regex' => array(
      'label' => t('Regular expression'),
      'description' => t('Validates user-entered text against a specified regular expression.'),
      'class' => 'Drupal\field_validation\Plugins\field_validation\validator\FieldValidationRegexValidator',
    ),
  );
}

+++ b/core/modules/aggregator/aggregator.admin.incundefined
@@ -442,16 +443,11 @@ function aggregator_admin_form($form, $form_state) {
+  $fetcher_lister = new FetcherType();

new FetcherType() results in a $fetcher_lister.

+++ b/core/modules/aggregator/aggregator.api.phpundefined
@@ -11,58 +11,29 @@
+      'description' => t('Downloads data from a URL using Drupal\'s HTTP request handler.'),

Nitpick: s/Downloads/Fetches/?

+++ b/core/modules/aggregator/aggregator.moduleundefined
@@ -620,9 +621,16 @@ function aggregator_refresh($feed) {
+  $fetcher_factory = new FetcherType();

new FetcherType() results in a $fetcher_factory. Yet it's an identical instantiation. Very confusing. I'm wondering if there's a better name than "PluginType". I looked at Qt's API design for inspiration, but I could only find explicit "QSomethingFactory" classes. The closest match would be "QSomethingManager" classes. So what about "PluginTypeManager"? You ask the manager to find plugins of a certain type. You also ask it to create an instance.
However, I personally think that it does not make too much sense to explicitly write and use something like FetcherType. To me, it feels more natural to have a single (dare I say, singleton?) PluginManager, which you can use to find plugins of a certain type, or rather that implement a certain interface ($pluginManager = new PluginManager(); $fetcher_plugins = $pluginManager->find($interface = Drupal\aggregator\Plugin\Type\FetcherInterface);). You can also use it to instantiate plug-ins: ($pluginManager = new PluginManager(); $fetcher_plugin = $pluginManager->create($plugin = Drupal\aggregator\Plugin\aggregator\fetcher\DefaultFetcher).
In my very humble opinion, it then becomes much easier to work with plug-ins. There's less things that you need to know. You only need to know about PluginManager and the type/interface of the plug-in you're looking for.

I reviewed this without reading previous reviews. Turns out I made the exact same comment as bojan :)

Sorry about that.

If we went with the approach I think you just outlined, we still have to essentially know that in order to pass it through a singleton in order to get what would likely just be an instance of the plugin type class back again. Half a dozen of one, six of the other imo. Better to just cut to the chase and be really blatantly obvious about what we're doing.

I see what you're saying. I think the biggest win would come from calling it something "manager-ish" anyway, so in that regard I'm fine with what you said.
*However*, always having a single entry point into the plug-in system gives developers a familiar (over time) way to use plug-ins. I think purely from a DX POV, my point still stands. But you're absolutely right that my point about "having to know less things" is not actually true.

I also get an error message when i test annotation:

Error message
 ReflectionException: Class Drupal\msg_plugin/Plugins/msg_plugin/messenger/Roses does not exist in ReflectionClass->__construct() (line 50 of D:\xampp\htdocs\drupalplugin\core\lib\Drupal\Core\Plugin\Discovery\DirectoryDiscovery.php

I download code from:

git clone --recursive --branch plugins-annotations http://git.drupal.org/sandbox/eclipsegc/1441840.git drupal
git clone --recursive --branch 8.x-1.x http://git.drupal.org/sandbox/eclipsegc/1663586.git msg_plugin

Then I use the code from :

git clone --recursive --branch info_hooks http://git.drupal.org/sandbox/eclipsegc/1663586.git msg_plugin

It works.

I prefer the "annotation based discovery" method and expect that you could commit it to Core.

One question of "annotation based discovery" :
How could we provide some metadata information in Plugin? In Ctools it is in this way:

$plugin = array(
  'title' => 'my title',
  'description' => 'my  description',
  'class' => 'my_class',
...
);

I understand it now, it is in the annotation:

/**
 * @Plugin(
 *   plugin_id = "roses",
 *   title = "Roses",
 *   description = "Roses are red."
 * )
 */

My OS is windows7. Maybe it works at apple OS/ Linux OS.

After i change the code from:

$reflectionClass = new ReflectionClass(str_replace(DIRECTORY_SEPARATOR, '\\', "$prefix/". $fileinfo->getBasename('.php')));

to

$reflectionClass = new ReflectionClass('Drupal\msg_plugin\Plugins\msg_plugin\messenger\Roses');

The this error mesaage disappears. But I get another error message:

 Drupal\Component\Plugin\PluginException: The plugin did not specify an instance class. in Drupal\Component\Plugin\Factory\DefaultFactory->getPluginClass() (line 126 of D:\xampp\htdocs\drupalplugin\core\lib\Drupal\Component\Plugin\Factory\DefaultFactory.php

I have started a sandbox (http://drupal.org/sandbox/damiankloip/1682648) where I am currently converting the core filter module to use the plugin system for it's filters. It's still very much a work in progress, but worth mentioning.

adding a checkout for a specific sha1 to guarantee compatilibity

Marking these as fixed. We're still iterating over many aspects and are improving them. Check the plugin system component for related issues.