Integrate Address with Search API

AddressPlainFormatter is a good example of getting both the codes and the full names.

1. +++ b/src/Plugin/search_api/processor/FullAdministrativeArea.php
   @@ -0,0 +1,143 @@
   +
   +
   

   Should be only one line.

2. +++ b/src/Plugin/search_api/processor/FullAdministrativeArea.php
   @@ -0,0 +1,143 @@
   + *   id = "full_administrative_area",
   

   Not sure about this id, let's prefix this with address field as well.

3. +++ b/src/Plugin/search_api/processor/FullAdministrativeArea.php
   @@ -0,0 +1,143 @@
   + *   label = @Translation("Address Field : Full Administrative Area"),
   + *   description = @Translation("Processor for appending address field full text value of administrative_area (state) to its abbreviation i.e., IL => IL Illinois."),
   

   Let's mention in the label or description that this is only useful for US states.

4. +++ b/src/Plugin/search_api/processor/FullAdministrativeArea.php
   @@ -0,0 +1,143 @@
   +    $this->subdivisionRepository = \Drupal::service('address.subdivision_repository');
   

   We don't do this, we usually extend the contructor and implemt a create method to set those in.

5. +++ b/src/Plugin/search_api/processor/FullAdministrativeArea.php
   @@ -0,0 +1,143 @@
   +        continue;
   +      }
   +      if ($this->testType($field->getType())) {
   

   I don't think we need to do it like that, can be refactored to be shorter.

   $admin_area_field = (strpos($field->getPropertyPath(), ':administrative_area') === FALSE)
   if ($this->testType($field->getType() && $admin_area_field) {
   

This will also need at least a unit test, maybe integration as well (even though that'd need to have address as a test-dependency - so not sure about that.

Oh wait, this is in the address queue - not in search api.
The fourth issue I posted is probably not as relevant as for the search api queue.

This makes the integration test also a lot less viable, so that's probably not needed. Also not sure how strict @bojanz is about unit tests.

If no tests are needed, this looks rtbc to me.

+  /**
+   * Stored Administrative Areas indexed by country_country.
+   *
+   * Used as a cache.
+   *
+   * @var array
+   */
+  protected $heapAdministrativeAreas = array();
+
+  /**
+   * {@inheritdoc}
+   */
+  public function __construct(array $configuration, $plugin_id, array $plugin_definition) {
+    parent::__construct($configuration, $plugin_id, $plugin_definition);
+    $this->addressFormatRepository = \Drupal::service('address.address_format_repository');
+    $this->subdivisionRepository = \Drupal::service('address.subdivision_repository');
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildConfigurationForm(array $form, FormStateInterface $form_state) {
+    $fields = $this->index->getFields();
+    $field_options = array();
+    $default_fields = array();

1) We never use array() in Drupal anymore, only short array syntax: [].
2) The address format and subdivision repositories can be injected, you don't need to use \Drupal::service()?
3) I've never seen the convention of using "heap" as a prefix for static caches? Probably unneeded?

+    // Get the subdivisions for country.
+    $states = $this->subdivisionRepository->getAll([$country_code]);
+    foreach ($states as $state) {
+      $state_code = $state->getCode();
+      $administrative_areas[$state_code] = $state_code . ' - ' . $state->getName();
+    }

In the same block of code you use subdivisions, administrative areas, states. Only the first one is precise (state is just one label that an administrative area could have).

Architectural issues:
1) I don't see this code accounting for a country not having any subdivisions defined? In which case the static cache will always be empty and you will call subdivisionRepository->getAll() for each item? The library optimizes this away, but for clarity's sake it would make sense to acknowledge in the code that it's a possibility.

2) Some countries (Russia, China, Japan, etc) also have a local code and a local name (cyrlic, chinese version, etc). You would want to add those as well. The local code and the local name could be identical, in which case you'd only add the local code.

3) We would ideally register this handler for the locality and dependent_locality fields as well, load the right values using the parents. Brazil has predefined localities, China has both predefined levels. On those two levels the name could be the same as the code (cause there's no actual abbreviation to use), in which case you could skip adding it.

This patch does not work correctly for me but I might be misunderstanding the desired functionality.
Instead of displaying the full country name (instead of the code) it displays the administrative subdivision.
So in this case the result is:
Country = wv - West Vlaanderen
subdevision = wv

EDIT
Never mind, I did misunderstand the purpose..
I was able to make it work for the country name as well.

Anyway, out of the box search_api integration would be a very nice to have.

I won't comment on general code style issues, etc., just on the Search API side of things:

+        $field->setValues($new_data);
+        if ($this->testField($name, $field)) {
+          $this->processField($field);
+        }

Largest problem: you mustn't use setValues() to set unprocessed values on a field, as the documentation notes. Use addValue() instead. (Cf. #2764385: Aggregated text plugin setting field value without datatype.)

Also, if you don't override/implement process() (or any of the other process*() methods) anyways, there's no need to call processField() from here.

Finally, I agree with Joris, the plugin ID should definitely be prefixed with the module name – as everywhere in Drupal. Only the module defining the plugin type (the Search API, in this case) can be exempt, as that sets the standard, which more or less guarantees there won't be any clashes (provided we're careful with new plugins now that 1.0 is out). For other modules, this can't be guaranteed, so the prefix is needed.
Again, it's the same for all other plugin types in Drupal.

Hi!

Thanks for all the work on this patch.

I tried this today and I got an error when the data is re-indexing if I set the type as "full text" in the fields page.

Fatal error: Call to a member function getTokens() on string in /vagrant/web/modules/contrib/search_api/modules/search_api_db/src/Plugin/search_api/backend/Database.php on line 1387

I tried this today and I got an error when the data is re-indexing if I set the type as "full text" in the fields page.

Yes, that's exactly what will happen if setValues() is used instead of addValue().

So, this needs work, yes?

Thanks!

This can implemented cleaner by extending the property definitions of the address field items with computed fields for the full names of the sub-properties.
With the full names as field item properties:
* Search API automatically finds them.
* They can be added to the Search API index fields as needed.
* Concatenation can be accomplished with Search API's "Aggregated fields" with the code property and the equivalent new full name property.

The attached patch adds:

• Plugin/Field/FieldType/AddressItem: Added full name computed properties added for: country_code, administrative_area, locality, dependent_locality.
• Plugin/Field/FieldType/CountryItem: Added full name computed properties added for: value.
• Created TypedData classes for SubdivisionName and CountryName that act similar to the views field handlers.

@recrit I can't add the address fields using the patch above to facets unfortunately... I get this:
Error: Call to undefined method Drupal\Core\TypedData\DataDefinition::getPropertyDefinitions() in Drupal\facets\Plugin\facets\processor\BooleanItemProcessor->supportsFacet() (line 92 of /var/www/vhosts/example.dev/web/modules/contrib/facets/src/Plugin/facets/processor/BooleanItemProcessor.php) #0

#30 is a bug in facets, see #2917323: Critical error thrown when editing a facet based on an aggregated field - please confirm if the patch there fixes your problem.

@borisson_ thanks for the link. This did indeed fix the issue. Just to clarify - I have a content type with an address field and search api installed using database (for now may up to using solar before the project is finished). Instead of using the patch by chriscalip which appears to add a processor option to the index I went for the latest patch by recrit which adds the full country name to the list of fields I can add to the index.

I have created a view based on the index and added 4 of the required fields into a table display.

I then added some facets for taxonomy terms which worked great. Then I got the error mentioned above for country name but the patch you referenced has enabled me to fully configure the facet and place all the blocks required for testing. So for that - thanks very much!

Quick question though as I prefer to make sure I'm doing things correctly: In your opinion, which patch should I be using here? The one by chriscalip which adds a processor plugin specifically for address fields or recrit which allows you to add the country field with nothing extra turned on?

Hi,

I applied the patch in #28 thinking it would fix my issue. Sadly doesn't seem to be working for me. I may have missed the point totally here. Here is what I have - a content type with an address field. Got Search API installed using database for the index.

Can add the address field to the agregated content.

For the field I can add additional_name, locality_name, address_line1 and so on. However it does report one field skipped, field_item:address (see attached screenshot).

Ignoring this I add the fields, perform a re-index, and do a search for something I know is in one of the fields. Location name, postcodes, etc. It does find most of them. The only one that doesn't seem to work is "Company" which is what I assume is Organization.

Hi Bojan Živanović, Michael Vanetta.

Why wasn't https://www.drupal.org/files/issues/address-search-api-full-name-properties-2812659-28.patch added to the module? Except for comment https://www.drupal.org/project/address/issues/2812659#comment-12563243, it works clear and I think is the best suggestion. However, it requires some views filter for the Search API like Drupal\address\Plugin\views\filter\AdministrativeArea. Can you check this issue again and maybe update main issue goals? E.g. if somebody wants to finish this issue.

address 8.x-1.4 "require": "commerceguys/addressing": "^1.0.0" , Addressing\LocaleHelper -> Addressing\Locale

addressing-1.0.0-beta5 is LocaleHelper.php

revise

use CommerceGuys\Addressing\Locale;

$use_local_name = Locale::match($parent->getLocale(), $subdivision->getLocale());

@radiumhe Thanks for locale change!
Patch re-rolled for the latest 8.x-1.x as of #a29a522.

updated status

Hi guys,
Patch seems to be working for me.
When can we make this RTBC?

I have used #37 successfully for a while now, I believe when multiple users has tested the patch with success we can mark this RTBC.

#28 and later are "won't fix" as far as I'm concerned. We are not going to modify the field type itself to support a contrib module, especially since the original approach seemed to be valid (adding search-api specific plugins).

Rewriting the patch mid-issue after it has already been reviewed is a good way to ensure an issue never lands.

Is there any way to use the entire address as a single field to build a facet? For example, I'd like a facet block of checkboxes with the entire address as the checkbox label.

Edit: I've realized what I'm asking for would not be practical for usability anyways. Nevermind!

Hello,

I modified patch #23 to use addValue(). Now I can use full text search field and it works. I also tried to use Search API Autocomplete with Address field, but it doesn't work. I created another issue for this: #3126474: Integrate Address with Search API Autocomplete.

Also, I see notice "Notice: Undefined offset: 1 in Drupal\address\Plugin\search_api\processor\AddressAbbreviationsToFullForm->buildConfigurationForm() (line 69 of modules/contrib/address/src/Plugin/search_api/processor/AddressAbbreviationsToFullForm.php)." in some cases. I fixed it.

Checked on working site and fixed several other errors.

Fixed the typo.

Hello,
After Applying patch #48:

1. In Facets: I get duplicated items (Country code and Country name)

PE (1)
Peru (1)
United States (1)
US (1)

2. Views using Content indexed: When 'Content datasource: address » The two-letter country code (indexed field)' field is added Country code is displayed.

Minor fix for when the address is empty.

This patch replaces the field value instead of adding it, so @langelhc it can probably fix your problem (but the facet case hasn't been tested by me yet).

The patch gives me the following error:

ResponseText: Error: Call to a member function setText() on string in Drupal\address\Plugin\search_api\processor\AddressAbbreviationsToFullForm->preprocessIndexItems() (line 167 of /modules/contrib/address/src/Plugin/search_api/processor/AddressAbbreviationsToFullForm.php).

It works only if "Fulltext" field type selected.

And another error I got when tried to apply the previous patch (#52):
Drupal\search_api_solr\SearchApiSolrException while indexing item entity:node/6:en: Solr endpoint http://solr:8983/ bad request (400). ERROR: [doc=674e8e-search_api_solr_index-entity:node/6:en] multiple values encountered for non multiValued field ss_country_code: [UA, Ukraine] in Drupal\search_api_solr\SolrConnector\SolrConnectorPluginBase->handleHttpException() (line 997 of /modules/contrib/search_api_solr/src/SolrConnector/SolrConnectorPluginBase.php).

Added fix for scalar values.

Added support for fields on referenced entities.

For example, when you want to use the processor on fields from a user profile (https://www.drupal.org/project/profile).
Then the property path is something like this user_profiles:entity:field_address:country_code

Fixed - warnings about undefined offset.

// [$field_name, $component, $reference_field_name, $reference_component] = explode(':', $field->getPropertyPath());
[$field_name, $component, $reference_field_name, $reference_component] = array_pad(explode(':', $field->getPropertyPath()), 4, '');

// $target = $reference_component ?? $component;
$target = $reference_component ?: $component;

All of the "fixes" for the Search API approach patch - facet support, any index field type, any level of nesting of entity references - were already handled by just adding the computed properties to the address field type in the patch #37.
The Search API Processor Plugin that is post processing an existing indexed field is going to have these issues since you are trying to reproduce what Drupal Core already provides to you with the Field API system. The admin UX also will have the issue where the admin / developer must know that the index type selected on the Search API "Fields" page must be acceptable by the new Search API Processor plugin selected on the Search API "Processors" page.

Adding the computed properties to the field type solves all of these issues for the Search API indexing while exposing the properties for any non Search API usage.
Example: Get the administrative area name.

  // With new computed properties:
  $admin_area_name = $node->field_address->first()->administrative_area_name;

  // vs without computed properties.
  $admin_area_code = $node->field_address->first()->administrative_area;
  $country_code = $node->field_address->first()->country_code;
  if ($admin_area_code && $country_code) {
    $subdivision = \Drupal::service('address.subdivision_repository')->get($admin_area_code, [$country_code]);
    if ($subdivision) {
      $admin_area_name = $subdivision->getName();
    }
  }

Maintainers - any feedback on @recrit's comment #61?

Quick reminder - please set the status to "needs review" after you upload a patch so we can see what the testbot thinks of it.

Copying some thoughts over from Slack, let's start by attempting to make the Search API field processor plugin work. If that system was designed to do what this issue desires, I'd second bojanz's earlier desire to make use of it vs. working around it with a pattern unique to this module. (In other words, I'd rather we do what other modules do, if that's the case with the processor, vs. present our own workaround for Address.)

I believe the latest patch to work from is #59, but I haven't tested it myself.

i am getting the following error in drupal 9.3.7

PHP Fatal error: Cannot declare class Drupal\address\Plugin\search_api\processor\AddressAbbreviationsToFullForm, because the name is already in use in /var/www/html/*/web/modules/contrib/search_api/src/Plugin/search_api/processor/AddressAbbreviationsToFullForm.php on line 29

is there a fix? cheers kevin

@ pyxio: Seems like you maybe applied the patch to both the Search API and the Address modules, leading to two copies of the same class in your installation? The patch should only be applied to Address, not to Search API.
Please delete the one in web/modules/contrib/search_api/src/Plugin/search_api/processor and make sure there is also one in web/modules/contrib/address/src/Plugin/search_api/processor.

@drunken monkey thanks for the reply. I do not believe I tried to patch search api but I can't confirm that now as I have updated the code base.

Now I have applied the patch successfully and there are no errors. I now have the processor Address Field : Convert Abbreviations into Full Text Form. available and have enabled it. Unfortunately, views field and facet still show abbreviation. I have tried both fulltext and string in the search api fields config. I can't find any other settings.

What else is needed to configure other than enable the processor and reindex? many thanks.

Something does not seem correct to me, I can't select the country code for indexing and therefore I can't apply this preprocess to my address field. To get the country code in the facet list, I need to use the bare field, without any additional "target" or subpath.

Because the county_code is actually the main property of this field, it can't be referred to as a component like others. So it was needed to adjust the logic.

For sites that might be using it, this provides a fix for patch #37 due to changes in the addressing library.

so is #69 or #70 the correct patch?

#69 is the preferred path forward, #70 was a fix of #37 necessary for a site that was built using the previous approach where we didn't have time to switch (we'll work on the switch shortly).

Added support for country field type.

Country field stores the country code in the default 'value' property instead of 'country_code'.

Patch #74 works well (as did patch #69) with an address field.
I've also tested with a Country field type and the preprocessor worked as expected.

I vote for this to be RTBC.

This looks great, I'm not sure how viable it is to add integration tests for this and a unit test doesn't add much value, because it's only the integration with search api that matters.

RTBC based on that.

After applying patch address-integrate_address_searchapi-2812659-74.patch, I started to notice the logs being littered with the following warning:

Warning: Undefined array key "value" in Drupal\address\Plugin\search_api\processor\AddressAbbreviationsToFullForm->triangulateCodeToFulltext() (line 198...

Cause:
This is caused by the new code in patch 74 to account for the country field:

$current = $result = $field_value[$target] ?? $field_value['value'];

An address field can have optional properties so $field_value[$target] may actually be NULL for an address field (not a country field).

Fix:
The attached patch changes the line to the following to account for NULL values.

$current = $result = $field_value[$target] ?? ($field_value['value'] ?? NULL);
if (!isset($result)) {
  return $result;
 }

The patch also updates the "preprocessIndexField" method to account for NULL to avoid string operations on a NULL value which causes deprecation notices in PHP 8.1

Does 2.0 work with search api?

Changing version to 2.x-dev, this will have to go into the 2.1.0 release, 8.x-1.x is now closed.

address 2.0.1
#77 works

Steps to test

1. Go to your search index's field form admin/config/search/search-api/index/{index_name}/fields
2. Click "add fields"
3. Go to your address field and click "expand"
4. Select one of the supported abbreviation --> to label fields (e.g. country_code, administrative_area, locality, and dependent_locality)
5. Click "Done" in the modal
6. Click "save changes"
7. Go to your search index's processor form, /admin/config/search/search-api/index/{index_name}/processors
8. Check "Address Field : Convert Abbreviations into Full Text Form"
9. Scroll to the bottom to the process settings sections
10. Select "Enable" for your given fields
11. Click "save"
12. All items should be queued for index. Now you have to reindex them /admin/config/search/search-api/index/{index_name}
13. Add the necessary facet(s) to your search api view
14. you should now see the field labels

I tested on
"drupal/search_api": "1.40.0",
"drupal/address": "2.0.4",
"drupal/facets": "3.0.2",

This works! See screenshots.

Based on #76, remove the need for tests tag. I would help to get this in if the patch would be converted to a MR.

Country code is missing for some reason.

Actually 'country_code' is represented as 'address' which is quite misleading. It also would index the code and not the country name (I suspect most would want that option) which I could only get with a Custom Value index item and token.

MR is created based on patch #77