Issue #2283929 by tim.plunkett: Clean up condition plugins to have schema support and to allow them to be optional.

core/config/schema/core.data_types.schema.yml

@@ -292,3 +292,14 @@ block_settings:
     provider:
       type: string
       label: 'Provider'
+
+condition.plugin:
+  type: mapping
+  label: 'Condition'
+  mapping:
+    id:
+      type: string
+      label: 'ID'
+    negate:
+      type: boolean
+      label: 'Negate'

core/lib/Drupal/Core/Condition/ConditionPluginBase.php

@@ -37,7 +37,7 @@ abstract class ConditionPluginBase extends ExecutablePluginBase implements Condi
     $form['negate'] = array(
       '#type' => 'checkbox',
       '#title' => $this->t('Negate the condition.'),
-      '#default_value' => isset($this->configuration['negate']) ? $this->configuration['negate'] : FALSE,
+      '#default_value' => $this->configuration['negate'],
     );
     return $form;
   }
@@ -83,7 +83,9 @@ abstract class ConditionPluginBase extends ExecutablePluginBase implements Condi
    * {@inheritdoc}
    */
   public function defaultConfiguration() {
-    return array();
+    return array(
+      'negate' => FALSE,
+    );
   }
 
   /**

core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php

@@ -11,6 +11,7 @@ use Drupal\Component\Plugin\ContextAwarePluginBase as ComponentContextAwarePlugi
 use Drupal\Core\Plugin\Context\Context;
 use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Drupal specific class for plugins that use context.
@@ -22,6 +23,16 @@ use Drupal\Core\StringTranslation\StringTranslationTrait;
 abstract class ContextAwarePluginBase extends ComponentContextAwarePluginBase {
   use StringTranslationTrait;
 
+  /**
+   * An array of service IDs keyed by property name used for serialization.
+   *
+   * @todo Remove when Drupal\Core\DependencyInjection\DependencySerialization
+   *   is converted to a trait in https://drupal.org/node/2208115.
+   *
+   * @var array
+   */
+  protected $_serviceIds = array();
+
   /**
    * Override of \Drupal\Component\Plugin\ContextAwarePluginBase::__construct().
    */
@@ -50,4 +61,45 @@ abstract class ContextAwarePluginBase extends ComponentContextAwarePluginBase {
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   *
+   * @todo Remove when Drupal\Core\DependencyInjection\DependencySerialization
+   *   is converted to a trait in https://drupal.org/node/2208115.
+   */
+  public function __sleep() {
+    $this->_serviceIds = array();
+    $vars = get_object_vars($this);
+    foreach ($vars as $key => $value) {
+      if (is_object($value) && isset($value->_serviceId)) {
+        // If a class member was instantiated by the dependency injection
+        // container, only store its ID so it can be used to get a fresh object
+        // on unserialization.
+        $this->_serviceIds[$key] = $value->_serviceId;
+        unset($vars[$key]);
+      }
+      // Special case the container, which might not have a service ID.
+      elseif ($value instanceof ContainerInterface) {
+        $this->_serviceIds[$key] = 'service_container';
+        unset($vars[$key]);
+      }
+    }
+
+    return array_keys($vars);
+  }
+
+  /**
+   * {@inheritdoc}
+   *
+   * @todo Remove when Drupal\Core\DependencyInjection\DependencySerialization
+   *   is converted to a trait in https://drupal.org/node/2208115.
+   */
+  public function __wakeup() {
+    $container = \Drupal::getContainer();
+    foreach ($this->_serviceIds as $key => $service_id) {
+      $this->$key = $container->get($service_id);
+    }
+    unset($this->_serviceIds);
+  }
+
 }

core/modules/language/config/schema/language.schema.yml

@@ -123,3 +123,11 @@ language.settings:
                         language_show:
                           type: boolean
                           label: 'Show language selector on create and edit pages'
+
+condition.plugin.language:
+  type: condition.plugin
+  mapping:
+    langcodes:
+      type: sequence
+      sequence:
+        - type: string

core/modules/language/src/Plugin/Condition/Language.php

@@ -8,7 +8,7 @@
 namespace Drupal\language\Plugin\Condition;
 
 use Drupal\Core\Condition\ConditionPluginBase;
-use Drupal\Core\Language\Language as Lang;
+use Drupal\Core\Language\LanguageInterface;
 
 /**
  * Provides a 'Language' condition.
@@ -32,23 +32,23 @@ class Language extends ConditionPluginBase {
     $form = parent::buildConfigurationForm($form, $form_state);
     if (\Drupal::languageManager()->isMultilingual()) {
       // Fetch languages.
-      $languages = language_list(Lang::STATE_ALL);
+      $languages = language_list(LanguageInterface::STATE_ALL);
       $langcodes_options = array();
       foreach ($languages as $language) {
-        $langcodes_options[$language->id] = $language->label();
+        $langcodes_options[$language->id] = $language->getName();
       }
       $form['langcodes'] = array(
         '#type' => 'checkboxes',
         '#title' => t('Language selection'),
-        '#default_value' => !empty($this->configuration['langcodes']) ? $this->configuration['langcodes'] : array(),
+        '#default_value' => $this->configuration['langcodes'],
         '#options' => $langcodes_options,
         '#description' => t('Select languages to enforce. If none are selected, all languages will be allowed.'),
       );
     }
     else {
-      $form['language']['langcodes'] = array(
+      $form['langcodes'] = array(
         '#type' => 'value',
-        '#value' => !empty($this->configuration['langcodes']) ? $this->configuration['langcodes'] : array()
+        '#value' => $this->configuration['langcodes'],
       );
     }
     return $form;
@@ -66,7 +66,7 @@ class Language extends ConditionPluginBase {
    * {@inheritdoc}
    */
   public function summary() {
-    $language_list = language_list(Lang::STATE_ALL);
+    $language_list = language_list(LanguageInterface::STATE_ALL);
     $selected = $this->configuration['langcodes'];
     // Reduce the language list to an array of language names.
     $language_names = array_reduce($language_list, function(&$result, $item) use ($selected) {
@@ -96,12 +96,20 @@ class Language extends ConditionPluginBase {
    * {@inheritdoc}
    */
   public function evaluate() {
+    if (empty($this->configuration['langcodes']) && !$this->isNegated()) {
+      return TRUE;
+    }
+
     $language = $this->getContextValue('language');
     // Language visibility settings.
-    if (!empty($this->configuration['langcodes'])) {
-      return !empty($this->configuration['langcodes'][$language->id]);
-    }
-    return TRUE;
+    return !empty($this->configuration['langcodes'][$language->id]);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function defaultConfiguration() {
+    return array('langcodes' => array()) + parent::defaultConfiguration();
   }
 
 }

core/modules/node/config/schema/node.schema.yml

@@ -137,3 +137,11 @@ block.settings.node_syndicate_block:
     block_count:
       type: integer
       label: 'Block count'
+
+condition.plugin.node_type:
+  type: condition.plugin
+  mapping:
+    bundles:
+      type: sequence
+      sequence:
+        - type: string

core/modules/node/src/Plugin/Condition/NodeType.php

@@ -37,23 +37,11 @@ class NodeType extends ConditionPluginBase {
       '#title' => t('Node types'),
       '#type' => 'checkboxes',
       '#options' => $options,
-      '#required' => TRUE,
-      '#default_value' => isset($this->configuration['bundles']) ? $this->configuration['bundles'] : array(),
+      '#default_value' => $this->configuration['bundles'],
     );
     return $form;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function validateConfigurationForm(array &$form, array &$form_state) {
-    foreach ($form_state['values']['bundles'] as $bundle) {
-      if (!in_array($bundle, array_keys(node_type_get_types()))) {
-        form_set_error('bundles', $form_state, t('You have chosen an invalid node bundle, please check your selection and try again.'));
-      }
-    }
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -80,8 +68,18 @@ class NodeType extends ConditionPluginBase {
    * {@inheritdoc}
    */
   public function evaluate() {
+    if (empty($this->configuration['bundles']) && !$this->isNegated()) {
+      return TRUE;
+    }
     $node = $this->getContextValue('node');
     return !empty($this->configuration['bundles'][$node->getType()]);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function defaultConfiguration() {
+    return array('bundles' => array()) + parent::defaultConfiguration();
+  }
+
 }

core/modules/system/config/schema/system.schema.yml

@@ -367,3 +367,9 @@ block.settings.system_branding_block:
     use_site_slogan:
       type: boolean
       label: 'Use site slogan'
+
+condition.plugin.request_path:
+  type: condition.plugin
+  mapping:
+    pages:
+      type: string

core/modules/system/src/Plugin/Condition/RequestPath.php

@@ -87,7 +87,7 @@ class RequestPath extends ConditionPluginBase implements ContainerFactoryPluginI
    * {@inheritdoc}
    */
   public function defaultConfiguration() {
-    return array('pages' => '');
+    return array('pages' => '') + parent::defaultConfiguration();
   }
 
   /**
@@ -134,6 +134,9 @@ class RequestPath extends ConditionPluginBase implements ContainerFactoryPluginI
     // Convert path to lowercase. This allows comparison of the same path
     // with different case. Ex: /Page, /page, /PAGE.
     $pages = Unicode::strtolower($this->configuration['pages']);
+    if (!$pages) {
+      return TRUE;
+    }
 
     $request = $this->requestStack->getCurrentRequest();
     // Compare the lowercase path alias (if any) and internal path.
@@ -142,4 +145,5 @@ class RequestPath extends ConditionPluginBase implements ContainerFactoryPluginI
 
     return $this->pathMatcher->matchPath($path_alias, $pages) || (($path != $path_alias) && $this->pathMatcher->matchPath($path, $pages));
   }
+
 }

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

@@ -162,3 +162,11 @@ search.plugin.user_search:
   label: 'User search'
   sequence:
     - type: undefined
+
+condition.plugin.user_role:
+  type: condition.plugin
+  mapping:
+    roles:
+      type: sequence
+      sequence:
+        - type: string

core/modules/user/src/Plugin/Condition/UserRole.php

@@ -35,7 +35,6 @@ class UserRole extends ConditionPluginBase {
       '#default_value' => $this->configuration['roles'],
       '#options' => array_map('\Drupal\Component\Utility\String::checkPlain', user_role_names()),
       '#description' => $this->t('If you select no roles, the condition will evaluate to TRUE for all users.'),
-      '#required' => TRUE,
     );
     return $form;
   }
@@ -46,7 +45,7 @@ class UserRole extends ConditionPluginBase {
   public function defaultConfiguration() {
     return array(
       'roles' => array(),
-    );
+    ) + parent::defaultConfiguration();
   }
 
   /**
@@ -81,6 +80,9 @@ class UserRole extends ConditionPluginBase {
    * {@inheritdoc}
    */
   public function evaluate() {
+    if (empty($this->configuration['roles']) && !$this->isNegated()) {
+      return TRUE;
+    }
     $user = $this->getContextValue('user');
     return (bool) array_intersect($this->configuration['roles'], $user->getRoles());
   }