Rename Sensor to SensorPlugin

Uff ;)

Started reviewing this before reading the comments, as you said, a lot that IMHO should be done in #2383873: Rename sensor_info: services to sensor / drush to sensor-config (everything drush/services related). Will put that back to needs work with instructions to extract everything related to those topics from this patch. This will be more than big enough without that.

But additionally to that, there are a lot of wrong renames and also a bunch of unrelated things that I noticed in code that got changed.

1. +++ b/README.txt
   @@ -20,7 +20,7 @@
   - * Sensor interface that can be easily implemented to provide custom sensors.
   + * SensorPlugin interface that can be easily implemented to provide custom sensors.
   

   isn't it SensorPluginInterface? And should then say custom sensor plugins.

2. +++ b/README.txt
   @@ -50,6 +50,6 @@
     variable, for example in settings.php. This allows to enforce environment
     specific settings, like disabling a certain sensor.
    
   - $conf['monitoring_sensor_info']['name_of_the_sensor']['settings']['enabled'] = FALSE;
   + $conf['monitoring_sensor']['name_of_the_sensor']['settings']['enabled'] = FALSE;
   

   This whole block needs to be rewritten in 8.x (and mention config overrides instead, not this)

3. +++ b/config/install/views.view.monitoring_sensor_results.yml
   @@ -87,7 +87,7 @@ display:
              admin_label: ''
   -          label: 'Sensor name'
   

   I don't think this rename is correct. This is the UI and I'm pretty sure it refers to an actual sensor.

4. +++ b/config/schema/monitoring.schema.yml
   @@ -2,7 +2,7 @@
      type: config_entity
   -  label: 'Monitoring Sensor'
   +  label: 'Monitoring SensorPlugin'
   

   Same here.

5. +++ b/config/schema/monitoring.schema.yml
   @@ -18,7 +18,7 @@ monitoring.sensor_config.*:
          type: string
   -      label: 'Sensor ID'
   +      label: 'SensorPlugin ID'
   

   This is technically correct, but we should use Sensor plugin in the UI, not as a class name.

6. +++ b/config/schema/monitoring.schema.yml
   @@ -40,7 +40,7 @@ monitoring.sensor_config.*:
   -  label: 'Sensor settings'
   +  label: 'SensorPlugin settings'
   

   Not sure about that, they are settings for the sensor plugin but stored on the sensor config. but again at least not as a class name.

7. +++ b/modules/demo/monitoring_demo.install
   @@ -19,7 +19,7 @@ function monitoring_demo_install() {
      // Enable REST API for monitoring resources.
      $settings = array(
   -    'monitoring-sensor-info' => array(
   +    'monitoring-sensor' => array(
   

   This is unrelated and wrong, we need to rename this as part of the rest plugin rename.

8. +++ b/modules/demo/src/Controller/FrontPage.php
   @@ -61,8 +61,8 @@ class FrontPage extends ControllerBase {
   -            t('Drush integration - open up your console and type in # drush monitoring-info or # drush monitoring-run. See the drush help for more info and commands.'),
   -            t('REST resource for both the info about sensors and running the sensors via the service. Open up your REST client and visit /monitoring-sensor-info/{sensor_name} and /monitoring-sensor-result/{sensor_name}'),
   +            t('Drush integration - open up your console and type in # drush monitoring-sensor or # drush monitoring-run. See the drush help for more info and commands.'),
   +            t('REST resource for both the info about sensors and running the sensors via the service. Open up your REST client and visit /monitoring-sensor/{sensor_name} and /monitoring-sensor-result/{sensor_name}'),
   

   Same, also belongs in the other issue.

9. +++ b/modules/multigraph/src/Form/MultigraphForm.php
   @@ -220,7 +220,7 @@ class MultigraphForm extends EntityForm {
   -      drupal_set_message($this->t('Sensor "@sensor_label" added. You have unsaved changes.', array('@sensor_label' => $sensor_label)), 'warning');
   +      drupal_set_message($this->t('SensorPlugin "@sensor_label" added. You have unsaved changes.', array('@sensor_label' => $sensor_label)), 'warning');
   

   Also wrong.

10. +++ b/modules/multigraph/src/Tests/MultigraphServicesTest.php
    @@ -87,7 +87,7 @@ class MultigraphServicesTest extends RESTTestBase {
         $name = 'multigraph_that_does_not_exist';
    -    $this->doRequest('monitoring-sensor-info/' . $name);
    +    $this->doRequest('monitoring-sensor/' . $name);
    

    Also unrelated.

11. +++ b/monitoring.api.php
    @@ -13,7 +13,7 @@ use Drupal\monitoring\Result\SensorResultInterface;
      * @param \Drupal\monitoring\Entity\SensorConfig $sensor_config
    - *   Sensor config object of a sensor for which links are being altered.
    + *   SensorPlugin config object of a sensor for which links are being altered.
    

    Just no :)

12. +++ b/monitoring.drush.inc
    @@ -64,7 +64,7 @@ function monitoring_drush_command() {
           'output' => 'The output format. Currently "table" and "json" available. Defaults to "table".',
    -      'expand' => 'Relevant only for the json output. Currently "sensor_info" value supported.',
    +      'expand' => 'Relevant only for the json output. Currently "sensor" value supported.',
    

    all drush related renames are likely also unrelated/wrong.

13. +++ b/monitoring.routing.yml
    @@ -25,14 +25,14 @@ monitoring.detail_form:
    -    _controller: '\Drupal\monitoring\Controller\ForceRunController::forceRunAll'
    +    _controller: '\Drupal\monitoring\Controller\SensorForceRunController::forceRunAll'
    ...
    -    _controller: '\Drupal\monitoring\Controller\ForceRunController::forceRunSensor'
    +    _controller: '\Drupal\monitoring\Controller\SensorForceRunController::forceRunSensor'
    

    Not sure if this is really better, the second one now has Sensor twice, but OK ;)

14. +++ b/monitoring.routing.yml
    @@ -63,6 +63,6 @@ monitoring.sensor_add:
    -    _controller: '\Drupal\monitoring\Controller\ConfigAutocompleteController::autocomplete'
    +    _controller: '\Drupal\monitoring\Controller\SensorConfigAutocompleteController::autocomplete'
    

    This however is IMHO wrong. It doesn't autocomplete SensorConfig's, it autocompletes config names and is used by ConfigSensorPlugin.

15. +++ b/monitoring.routing.yml
    index 23c4918..916ece9 100644
    --- a/review.txt
    
    --- a/review.txt
    +++ b/review.txt
    

    We really need to revisit this file and get rid of it, I'm sure it has a lot of very outdated information.

16. +++ b/src/Entity/SensorConfig.php
    @@ -121,11 +122,11 @@ class SensorConfig extends ConfigEntityBase {
    -   * @var string
    +   * @return string
    +   *   Sensor ID.
        */
    -
       public function getName() {
         return $this->id;
       }
    

    getName()? I think that's a weird left-over, we afaik use sensor_id?

17. +++ b/src/Entity/SensorConfig.php
    @@ -383,6 +384,10 @@ class SensorConfig extends ConfigEntityBase {
    +     * @var SensorConfig $a
    +     * @var SensorConfig $b
    

    We should add an interface four our config entity...

18. +++ b/src/Form/SensorForm.php
    @@ -142,7 +143,7 @@ class SensorForm extends EntityForm {
    -      /** @var SensorBase $sensor */
    +      /** @var SensorPluginBase $sensor */
    

    Not sure why this doesn't type hint on the interface. And we'd rather type hint on $sensor_config if that doesn't work already, then getPlugin() should just work?

19. +++ b/src/Plugin/monitoring/SensorPlugin/DisappearedSensorsSensorPlugin.php
    @@ -59,7 +59,7 @@ class DisappearedSensorsSensor extends SensorBase {
             '#type' => 'item',
    -        '#title' => t('Sensor message'),
    +        '#title' => t('SensorPlugin message'),
             '#markup' => $result->getMessage(),
    

    I don't think this is correct?

20. +++ b/src/SensorRunner.php
    @@ -103,7 +103,7 @@ class SensorRunner {
    -   * Runs the defined sensableors.
    +   * Runs the defined sensors.
    

    sensableors. nice :)

I need to check what's left and we should probably discuss that in person.

Right, getName() is a bit weird but correct, was confused.

What confused me was $this->sensor_id, we should rename that as well then, to $this->plugin or $this->plugin_id.

More follow-ups:

- Remove getName(), use id() instead
- Update documentation in README about overrides
- Refactor thresholds stuff, kill interface and base class, move settings form to SensorForm, out of settings. Replace checks for interface (to decide if form should be shown or not for example) with isNumeric() check? form in UI should use #states based on value type. (Not related to this issue, but I keep seeing it)

+++ b/src/Form/SensorForm.php
@@ -200,7 +200,7 @@ class SensorForm extends EntityForm {
-    /** @var SensorInterface $sensor */
+    /** @var SensorPluginInterface $sensor */
     if ($sensor_config->isNew()) {
       $plugin = $form_state->getValue('sensor_id');
       $sensor = monitoring_sensor_manager()->createInstance($plugin, array('sensor_info' => $this->entity));

@@ -220,7 +220,7 @@ class SensorForm extends EntityForm {
     $sensor_config = $this->entity;
-    /** @var SensorInterface $sensor */
+    /** @var SensorPluginInterface $sensor */
     $sensor = $sensor_config->getPlugin();

Rename $sensor to $plugin?

There are other instances too, search for "$sensor ". Some of them are sensor configs, those are less a problem I think, but we shouldn't have any $sensor plugin variables left here..

Two follow ups created:
#2389111: Remove getName( ) use id( ) instead
#2389121: Update documentation in README about overrides