Make adding new properties/fields more a first-class operation of processors

Should we postpone this issue on #2574969: Add a Views-like UI for adding fields? I think we should.

I quickly went trough the latest interdiff - I did see one thing. I like that there are less random fails now that we removed the container rebuilds.

+++ b/src/Entity/Index.php
@@ -912,6 +912,7 @@ public function indexItems($limit = '-1', $datasource_id = NULL) {
+          debug($e->getMessage());

We should remove this debug statement.

I have more time to look at this later today/tomorrow.

Overall, I think this patch looks great, this makes the processors that add fields much simpler.

1. +++ b/src/Plugin/search_api/processor/ContentAccess.php
   @@ -129,40 +120,54 @@ public function preprocessIndexItems(array &$items) {
   +    if (!$node) {
   +      // Apparently we were active for a wrong item.
   +      return;
   +    }
   

   Can this happen? I know it was in the previous processor as well but I still don't understand it.

2. +++ b/src/Plugin/search_api/processor/ContentAccess.php
   @@ -129,40 +120,54 @@ public function preprocessIndexItems(array &$items) {
   +        $result = Database::getConnection()
   +          ->query('SELECT * FROM {node_access} WHERE (nid = 0 OR nid = :nid) AND grant_view = 1', array(':nid' => $node->id()));
   

   Since we're refactoring this processor anyway, can we inject the database service instead? We could open a followup as well I guess.

3. +++ b/src/Plugin/search_api/processor/ContentAccess.php
   @@ -197,7 +202,7 @@ public function preprocessSearchQuery(QueryInterface $query) {
   -      if (is_object($account)) {
   +      if ($account instanceof AccountInterface) {
   

   Nice, this makes sense!

> And, as said, incorporating the "Aggregated fields" processor into this plan would of course be quite a challenge

I was thinking that you could have a new plugin type for index fields, and OOTB have 'normal' and 'aggregated'.

This patch has become really hard to review, does it make the patch smaller to use git diff -M?

I did find 2 things in the tests, but overall from what I can see, this is looking great. Thanks for the answers in #16, I understand that now.

1. +++ b/src/Plugin/search_api/processor/RenderedItem.php
   @@ -315,6 +248,7 @@ public function calculateDependencies() {
   +    # @todo
   

   What exactly still needs to happen here? Can we link to an issue or should this be done in this patch?

2. +++ b/src/Tests/IntegrationTest.php
   @@ -618,9 +621,10 @@ protected function checkFieldLabels() {
   +    # @todo add "rendered item" field and test there
   +//    $this->drupalGet($this->getIndexPath('processors'));
   +//    $this->assertHtmlEscaped($content_type_name);
   +//    $this->assertHtmlEscaped($field_name);
   

   We should fix this in this issue as well?

I spent some time sitting in the car waiting, so I looked trough the patch again.

1. +++ b/config/schema/search_api.processor.schema.yml
   @@ -15,46 +22,17 @@ plugin.plugin_configuration.search_api_processor.add_url:
   +# Default for any processor without specific configuration
   +
   +plugin.plugin_configuration.search_api_processor.*:
   +  type: search_api.default_processor_configuration
   

   Should we remove this blank line? That makes it clearer that the comment belongs to the definition.

2. +++ b/config/schema/search_api.processor.schema.yml
   @@ -15,46 +22,17 @@ plugin.plugin_configuration.search_api_processor.add_url:
    plugin.plugin_configuration.search_api_processor.highlight:
   -  type: mapping
   +  type: search_api.default_processor_configuration
   

   I like this, makes it clearer!

3. +++ b/config/schema/search_api.processor.schema.yml
   @@ -72,15 +50,9 @@ plugin.plugin_configuration.search_api_processor.highlight:
    plugin.plugin_configuration.search_api_processor.html_filter:
   -  type: mapping
   +  type: search_api.default_processor_configuration
   

   can we make this extend field_processors_configuration instead? Less lines of code!

4. +++ b/config/schema/search_api.processor.schema.yml
   @@ -280,35 +137,46 @@ plugin.plugin_configuration.search_api_processor.tokenizer:
   +# Definitions for property configuration
   +
   +search_api.property_configuration.*:
   

   Let's remove this blank line as well.

5. +++ b/src/Entity/Index.php
   @@ -1126,12 +1137,33 @@ public function query(array $options = array()) {
   +    // Merge in default options.
   +    $config = \Drupal::config('search_api.settings');
   ...
   +    // Merge in default options.
   +    $config = \Drupal::config('search_api.settings');
   

   Do we really need to do this twice? I'd think that this is only needed in the preSave? Not sure though.

woo.

This breaks existing search indexes -- or at least I assume it's this:

> Drupal\search_api\SearchApiException: Could not retrieve data definition for field 'Foo - aggregated' on index 'My index'. in Drupal\search_api\Item\Field->getDataDefinition() (line 369 of modules/search_api/src/Item/Field.php).

I know this is still in alpha, so it's my problem :) but tips on upgrading would be useful.

Big improvement to the UI for aggregated fields BTW :)

Ok so got it working. Your existing search index configuration will need hacking by hand in the exported YML file then reimporting:

1. Find the settings for the aggregated_field processor, eg:

processor_settings:
  aggregated_field:
    plugin_id: aggregated_field
    settings:
      fields:
        search_api_aggregation_1:
          label: 'Topics - aggregated'
          type: union
          fields:
            - 'entity:node/field_topics'
            - 'entity:resource/field_topics'

2. Remove the whole of the 'fields' property from this, but pasting it to another file.
3. For each aggregated field, find it in the main fields list:

field_settings:
  search_api_aggregation_1:
    label: 'Topics - aggregated'
    datasource_id: null
    property_path: search_api_aggregation_1
    type: integer
    indexed_locked: true

4. Change the property_path to be 'aggregated_field'
5. Remove the indexed_locked property
6. Add a configuration property
7. Paste into the configuration property the type and fields properties from the stuff you removed from the processor in step 2.

End result should be like:

  search_api_aggregation_1:
    label: 'Topics - aggregated'
    datasource_id: null
    property_path: aggregated_field
    type: integer
    configuration:
      type: union
      fields:
        - 'entity:node/field_topics'
        - 'entity:resource/field_topics'

There are a few other changes to the config array, but saving the Processors form will take care of those.

thanks @joachim!

Could someone update the documentation on https://www.drupal.org/node/2715959?

Sorry, someone can update the documentation or explain here what the new procedure to add custom fields?