Allow filtering by "Language (with fallback)"

Thanks for reporting the problem, bumping the issue now and providing both a good issue summary and a working patch already!

However, though I appreciate that it might be a good solution for your use case, we cannot change the definition of the search_api_item_language field now. This is guaranteed to be a single-valued string field, and there might be modules out there relying on this behavior. In fact, I know of one: the Database Search module, which is the most popular backend by far, relies on this for keyword-less searches. And god knows what other modules we'd break, in a hard-to-realize way.

That said, I'm pretty sure your use case can also be implemented very well in a small custom module. Instead of modifying the search_api_item_language field, you should just add your own, multi-valued language field (and maybe alter the Views definition, or Pages searches, or just all search queries to use/support it).
Since this could well be practical for others, too, you could also publish that module – in such cases, I always suggest starting a search_api_bonus project for exactly such handy plugins or additions, which are to small to warrant their own module, are too specific to go into the Search API itself but are still useful for general use.

Updated remaining tasks from comment #5.

I feel that adding another language filter with almost identical settings as the current but simply including language fallback functionality would be confusing.

You could instead also just override the existing one with your custom implementation.
But I fear changing this inside the Search API module is pretty much out of the question, since the database backend relies on the item language being a single-valued field. I'm pretty sure your patch would break keyword-less searches with that backend.

In Drupal 8, there is much less of a problem – we have much better built-in language support and there are no single-valued fields. So changing the Language processor in D8 to add multiple values for the default translation would be possible. The question is just whether this is something that everybody will want – I can imagine it might not make sense on a lot of sites.

Since you are, unless I'm mistaken, the only one who ever requested this feature, we should first probably find out if others are even interested in this. I'd be reluctant to add code to the module benefiting only a handful of users.

This patch is critical for us. It should also be critical for any user with the expectation that the content of their search index should match with the same content outside the search index.

From my point of view indexing the content multiple times using the the wrong language is an absolutely wrong approach!

Beside the fact that this will lead into horrible results in search_api_solr_multilingual there's a correct approach:
What you want to achieve here is a working language fallback mechanism. This has to be done mostly at query time, and some minor tweaks at index time.

apachesolr_multilingual 6.x and 7.x already solved that problem in the past.

At index time we added a missing translations field to every item that gets indexed. Its values are all language codes for which no translation exists at the moment. Based on this field you can achieve any complex fallback mechanism you want. Examples as pseudo SQL using 'und' as undefined or default:

• language = 'nl' OR (language = 'und' AND 'nl' IN missing_translations)
• language = 'de_AT' OR (language = 'de' AND 'de_AT' IN missing_translations) OR (language = 'und' AND 'de_AT' IN missing_translations AND 'de' IN missing_translations)

So the minimal patch to be implemented here is about adding the missing_translations field and to track when a new translation is added which has to trigger a re-index of all translations of an entity.

The fallback to 'und'/default translation should be implemented here and ideally configurable per search result page or view.

The more sophisticated fallback mechanisms should be left for contrib modules like search_api_solr_multilingual or search_api_clir.

Of course, at least since #2641392: Review our language/translation support having multiple languages for a single item is out of the question.
The solution from apachesolr_multilingual also seems sensible to me, can't see any problems with that.

However, I also don't see why we'd need to have this integrated into the framework. It should be quite easy to implement this with a processor and (optionally) a backend feature, or just in the backend. The only possible support the framework could give there is automatically marking the other translations as "dirty" for an added/deleted entity, but that's also easy to do with a hook.
So, in any case, I'd say this is definitely no beta blocker. We can at any point decide to either add this as a processor to this module, or to create a new module for this functionality.
The only reason to have this in the framework is to force backends to support it, and I honestly don't think the functionality is vital enough to justify that. And even so, if we add support to the DB and Solr backends, most users will be covered anyways.

from the Solr perspective I'm fine to completely implement that in search_api_solr_multilingual, just like we did it in the past.
But due to the fact that the basic mechanism is similar for the db backend as well, I suggest to implement these hooks in search_api_clir, the module I already created to make some multilingual features available for all backends.
What do you think? Should I move this issue?

Since the generic approach should work for any backend, I think, implementing this in a backend-independent module seems sensible. So if you want to work on this in the near future, then sure, please move the issue.

Since we plan to start that at the dev days I'll move it.

Is this feature now already implemented in Solr? If so, I can’t find it there. If not, then I find this project’s description confusing:

The feature is currently implemented by search_api_solr_multilingual and should be moved into a separate module to be available on all search backends.

(Combined with the Multilingual module now being part of the Solr core module.)

If it’s not implemented in Solr, I might need to take a stab at this for a client, so we’ll finally get support for this – hooray!

There is an existing implementation in apachesolr_multilingual 7.x. Fragments made it into search_api_solr_multilingual 8.x, but not full-featured as it was in apachesolr_multilingual 7.x
Bit I didn't start the task to move it the backend independent parts into this module here and to complete the features in search_api_solr 8.x.

Thanks for your reply, good to know!
Unfortunately, it turned out my client had specific requirements that precluded having the language fallbacks configurable directly on the processor, so that part is still missing (i.e., the configuration form for setting up the fallback order for all languages, plus the code in preprocessSearchQuery() to apply those to the query languages) – but the rest of the code is there and seems to be working fine.

Since this change means we’ll potentially match items of those other languages as well, the processor widens the languages set on the query to include all fallback languages as well. (This is also needed in Solr to use the correct fulltext fields.)

Anyways, sorry I couldn’t provide a complete patch here, but hopefully it will at least help to get this started!
Especially, Markus, you could tell me whether this is what you had in mind?

@drunken monkey: thanks a lot for hacking this down, i hopefully sonn will work on this and it would have taken me aeons to grok the entry points.

That said, i think the code has one flaw. Afaik, every translation has its own row in the search index. So i think like this:
* add a DE node, primary=DE, missing=FR,EN
* translate it to EN, primary=EN, missing=FR
=> what's missing her is: update the first item to primary=DE, missing=FR

How would we implement this?

Public note to myself:
* rename the missing-translations field to language-or-fallback
* use the core fallback api to calculate that field
* (see above note that adding a translation must trigger change of language-or-fallback for other items)

Note that with this we need no complex queries:
* filter by language to get only dedicated translations
* filter by language-or-fallback to get translation-or-fallback

@drunken-monkey: Here is a stab at how i imagine this.

Any idea how we should solve the problem to update other language index items, when a translation is adden *or* deleted?
If i get it right, the alterIndex phase only helps us with added indexes, and how to do it here is not self-ecident to me.

  public function alterIndexedItems(array &$items) {
    // @todo Alter translated items:
    // - when a translation is added, re-index all items languages that
    //   previously fell back to that language.
    // - when a translation is deleted, re-index all item languages that fall
    //   back to that language.
    // - Especially figure out or add a way that we can react on item delete.
  }

Musceled this down. Looks good to me but untested yet.

It came to me in the middle of the night that i messed up fallback and reverse-fallback. Added lots of comments to the rewritten and beautified version. Untested yet.

OK now with views integration (ugly hack!) and manually tested it does what it announces.

When we get this stable we might re-consider moving this to search-api OR naming the module search_api_language_fallback.
Given the simplicity of this, i'd be for search-api. In any case, i can help maintain this plugin.

Updated IS.

Great work, thanks a lot!

As it’s really not a lot of code, and a feature that will surely be useful to a lot of people, I’m definitely open to just adding this to the Search API module directly. Feel free to change the issue metadata and your patch accordingly.

Other notes:

1. Why not just have the processor act like a normal “add properties” processor, that is hidden and only provides the property? (See, e.g., AddURL, RenderedItem, …) That way, people would just add the property instead of enabling the processor.
   I think you can then also get rid of the complete Views hook – the field just gets provided as it normally would by the Search API Views integration and people can just add fields/filters/arguments for it as usual. A allowed_values setting in the property definition should take care of providing the correct options/labels (see attached patch revision). Otherwise, a hook_views_data_alter() would be the better option, I think – just going through all the index’s fields that have a property path of 'search_api_clir_language' (and no datasource) and altering their Views data accordingly.

2. Any idea how we should solve the problem to update other language index items, when a translation is adden *or* deleted?

   Unless I’m mistaken, Search API already takes care of that: whenever an entity is saved (including adding/removing translations – which, I think, would be hard to distinguish as a special case anyways) we track all existing translations as changed, plus add new ones/remove old ones.
   So, I don’t think your hook_entity_update() code is actually necessary. Or have you tested and determined that it is?

3. Finally, at least for the Search API module, this would definitely need to have code coverage before it could be committed. (Not necessarily for Views integration, just processor tests should suffice. (If we know the processor generates the correct field values, we can trust the rest of the framework to do its thing, I think.)

Thanks a lot, that's valuable pointers for me. Hacking on it...

+++ b/src/Plugin/search_api/processor/Clir.php
@@ -0,0 +1,193 @@
+ * Provides cross-language information retrieval (CLIR).

I don't think that this approach meets the definition of CLIR.

But it implements my suggestions from #11, which is good.

OK so as of #31 we move this to search_api. I'm preparing a patch.

Yay! #31 really helped!

Moving to search_api reduced the code complexity a lot.
To do this i split this up as git-am multipatch containing:
* moving common code from the crud hooks to a helper function
* added the processor
* worked around a nasty core issue #2951294: Sort out and fix language fallback inconsistencies which prevented access to full fallback hook info

#31.1: Done. Needed some views data filldling though so we can filter for "current interface language".
#31.2: Current code is quite efficient in not tracking unchanged translations, so we have to fiddle with that too.
#31.3: Tests still todo!

Setting to needs review to make the testbot have a look at this.

Thanks, forgot about that!

Patch flying in with some fixes.

Thanks a lot for the updated patch and keeping the ball rolling on this!

#31.1: Done. Needed some views data filldling though so we can filter for "current interface language".

Hm, but the field handler should work fine with just 'allowed values', or doesn’t it? I think that would be preferable to having the handler hard-coded to language there. (Did you verify that that even works, without the Search API field handler code? I wouldn’t have thought that it does.)

The filter handler adjustment is probably necessary, you’re right. However, I think we can do it in a more elegant, extensible fashion than a hard-coded check on the property path. Please see the attached revision.

#31.2: Current code is quite efficient in not tracking unchanged translations, so we have to fiddle with that too.

I don’t see what you mean by that? As I read the current code of search_api_entity_update(), one of three things happens with all current or previous translations of the updated entity:

1. They are tracked as inserted (if part of current but not of previous translations).
2. They are tracked as deleted (if part of previous but not of current translations).
3. They are tracked as updated (otherwise).

No further checks about actual changes to any translation occur, as far as I can see.
So, in which case exactly would we currently not react correctly?

Further remarks:

1. +++ b/src/Plugin/search_api/processor/LanguageWithFallback.php
   @@ -0,0 +1,156 @@
   +  protected static function getReverseLanguageFallbacks(ContentEntityInterface $entity) {
   

   Why is this static?
   Should be an instance method and use dependency injection.

2. +++ b/src/EntityRepositoryFix.php
   @@ -0,0 +1,61 @@
   +  /**
   +   * Get entity translation with fallbacks.
   +   *
   +   * This is a straight copy from
   +   * @see \Drupal\Core\Entity\EntityRepository::getTranslationFromContext
   +   * that returns NULL when no fallback is defined.
   +   * We need this as the original does not allow to distinguish between
   +   * "current translation is a fallback" and "does not have a fallback".
   +   */
   +  public function getTranslationFromContext(EntityInterface $entity, $langcode = NULL, $context = [], $nullIfNoFallback = FALSE) {
   

   Why do we need a whole adapted entity repository service for that? Wouldn’t a helper method on the processor suffice?
   (Even if a service would make sense, why does it need to extend EntityRepository?)

Please also see the minor changes in the attached patch revision!

@drunken_monkey #41:

[Updated patch]

Thanks a lot for the strongly improved search api integration code!

#31.1 mentioned in #41,

[Add allowed_values]

I still have the advice in mind to add allowed_values, but do not know if this is still necessary if we use the language filter handler, nor did i find an example (and after re-reading i suppose you wanted to attach an interdiff but that got lost).

#31.2:

So, in which case exactly would [current tracking] not react correctly?

Gee, i guess you are right and all that was not needed. Removed that code and let's see if test coverage finds something.

#41.1:

Why is this static?

Yes that being static is a zombie from a prior version where that needed to be. Fixed, thanks.

#41.2:

Why do we need a whole adapted entity repository service for that?

That was a dumb idea of mine and i thought "Let's just copy the code that should go core anyway". But it turned out that issue is way more complex and of course it's better to decouple that issue from this. (But see below for testability.)

tl;dr: Core API does not allow to have a language without fallback. We currently need this. Spent some time to sort this in #2951294: Sort out and fix language fallback inconsistencies.
But fixing this in search API is insane. Made up my mind and think to go this path:
* I'll maintain the proposed core API extension as a decorator in language_fallback_fix.module
* People can install that if they need to index items that do not have a fallback (like we do), but if they do not use that module, core lets everything language fall back to original language version
* The real reason that i made the module (and a patch does not suffice) is that without having it as test dependency, we can not test the fallback-less code paths.

Patch flying in that takes the above into account. And adds tests. They should currently fail as of said core issue.

OK worked more on this and finally it seems to do what it should.

Now the test is locally green (after #3023170-10: Test for compatibility with Drupal 9).

I had to fiddle a bit with the type (type= language makes the processor not show on the add-fields page, also see #3058985: Metatag fields skipped due to unknown type), and maybe there is a better solution to this.

I separated the test dependency patch mentioned above from the patch here so if you want to commit that you can see the testbot go green here too.

Manual test shows: Views filter still targets the language field, not ours.

Fixed that filter issue.
Also added cs fixes (not in interdiff).
For test dep patch see #44.
Now going to bed.

Great job, thanks a lot!
Small revision, but mostly nit-picks. Also should fix all code sniffer complaints.
Other than that, this looks really good! Does maybe someone else want to test/review, too?

Also, one question, just to make sure I understood this correctly: Your language_fallback_fix is only needed to run the tests successfully, since they also cover the case where a language doesn’t have a fallback. So, having that module installed is in no way required to use this new functionality. Is that correct?

+++ b/composer.json
@@ -23,9 +23,6 @@
-  "require-dev": {
-    "drupal/search_api_autocomplete": "@dev"

composer and test dependencies should be the same PHP Fatal error: Class 'Drupal\search_api_autocomplete\Search\SearchPluginBase' not found

#51: Hups, rebase error, thanks!

#50:

Also, one question, just to make sure I understood this correctly: Your language_fallback_fix is only needed to run the tests successfully, since they also cover the case where a language doesn’t have a fallback. So, having that module installed is in no way required to use this new functionality. Is that correct?

Absolutely. It looks like when the heroes of multilingual rolled the translation system, they baked in the assumption that any language has a fallback. Which avoids a lot of complications in the routing system. So extending that assumption means to extend the API. That core issue and the test dependency module is all about that. AND: For the all-languages-have-a-fallback case, neither the core patch nor the language_fallback_fix module is needed.

composer and test dependencies should be the same PHP Fatal error: Class 'Drupal\search_api_autocomplete\Search\SearchPluginBase' not found

OK, thought the test bot would be smart enough to figure out the Composer dependencies based on the test_dependencies. If not, then let’s just keep using Composer for those. Specifying the same dependencies twice just seems nonsensical and error-prone.

@ axel.rutz/#52: Great, thanks for confirming that for me!

Patch on #53 works like a charm!! (tested with Search API + SOLR)

Remember to add the field "Language (with fallback)" to the index under /admin/config/search/search-api/index/index_name/fields and to configure the new filter "Language (with fallback)" instead the normal filter "Item language" in your view.

It even fallbacks to the correct language when changing language weights in /admin/config/regional/language

Great job everyone.

@ axel.rutz:

Fatal error: Class Drupal\language_fallback_fix\EntityRepositoryDecoratorBase contains 4 abstract methods and must therefore be declared abstract or implement the remaining methods (Drupal\Core\Entity\EntityRepositoryInterface::getActive, Drupal\Core\Entity\EntityRepositoryInterface::getActiveMultiple, Drupal\Core\Entity\EntityRepositoryInterface::getCanonical, ...) in /var/www/html/modules/contrib/language_fallback_fix/src/EntityRepositoryDecoratorBase.php on line 8

Seems like an error in your module? Apparently, #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing added some new methods to that interface (for Drupal 8.7.0+), which you still need to implement.

The other fail was our fault, though, the property name was inconsistent. As we don’t prefix our other properties, either, I removed the search_api_ prefix here, too.
Attaching a revised patch, but that will still fail until #3063517: Implement missing methods (for Drupal 8.7) is committed.

I've applied the patch from #56, the files were patched correctly, but the "Language (with fallback)" field does not show up in the "add fields" view. Am I missing something?

Did you clear the cache?

Code looks good from a quick review - Applied #56 to a site where I was having real grief with search api and multilingual and the new filter solved all the duplicate content problems.

RTBC for me!

Patch in #56 seems to have fixed the issues I had with multilingual content as well. Vote for RTBC as well :)

Awesome, great to hear! Thanks for reporting back!
So, finally committed. Thanks a lot again, everyone!

Moving back to D7 in case someone wants to back-port.

I have custom language fallback setup as:

en-in > en > und (Using language hierarchy module)

Most of the content on my site is in 'en'. I want the 'en' content to show up in the results when I am on en-in search.
Applying this patch is not allowing me to have that.

I've added the field "Language (with fallback)" to the index.

I am using simple pages provided by the search_api_page module.

Is there any other config I need to do?

Is there any other config I need to do?

I think you’ll need to use Views to create your search page – I don’t think the Search API Pages module supports the field yet.

Sorry for reviving a four-year-old discussion but I just stumbled on a problem with the solution accepted.

From my point of view indexing the content multiple times using the the wrong language is an absolutely wrong approach!

At index time we added a missing translations field to every item that gets indexed. Its values are all language codes for which no translation exists at the moment. Based on this field you can achieve any complex fallback mechanism you want.

This only works if all the fields in your item come from the same entity or if none of referenced entities are translated.
In my case the index is built on an entity that isn't translated but there are also some fields in the index taking values from other entities (referenced via ER fields) that ARE translated. If we only create one index entry per master entity, it will only contain the referenced fields values in the default language. There is no way to retrieve the master entity by translated values of those referenced fields at query time because they are simply not in the index. I can provide an example if it's not clear.
Now the original idea proposed by jantoine in the very beginning would work in my case because multiple index entries will be created per each master entity for every language, and the fields coming from translated referenced entities will have differently translated values.

@marassa: Thanks for the input!

As the person who determined that direction: Yes. I remember i took that direction with the idea that the approach in code is superior for some use cases, but tbh i can't reconstruct so do not know if that was genius or bollocks. Some time after this was committed, i realized that there are other use cases that can only be done with the originally proposed approach, and felt bad. So if you open a related issue and get the ball for a "missing translations" implementation rolling, i'll support that. (And maybe if that is superior for all cases, we might consider deprecating this implementation.)

@geek-merlin: Thanks for a fast and detailed response!
First of all, I am pretty sure that the currently implemented approach does not deserve to be deprecated because its obvious advantage is a reasonably sized index with no redundant entries. For any use case not involving complex indexes containing possibly translated referenced data (that would probably be the overwhelming majority of use cases) it makes no sense to bloat the index with many identical "fake" translations per each language. It only makes sense when the fake translations are not as identical (or as fake) as it looks at the first sight.
So I guess I am going to start with trying to port patch #7 to D8 and see how it goes. If it works for me and looks halfway decent I will post it as a new issue and let the maintainers decide how (or if) it should be integrated with the current solution.
And of course I would appreciate any alternative ideas of how it could be done! I am very new to Search API and might easily be missing something obvious.

@marassa: Cool, awaiting your issue link. Feel free to PM me if you need something.

Thanks a lot for weighing in, marassa! You‘re right, this use case is not covered by the current implementation, and it’s hard to see how it could be. I also can’t really think of a cleaner way to implement this than your suggested one.
However, one other disadvantage of your proposal, that we should at least be aware of, is that all text analysis (e.g., in Solr) for your fake items would happen in the wrong language. Especially stemming can have highly unpredictable results in this case. So, this is another reason our current approach is much better suited for the general use case.

As it’s confusing for users to have two options for doing more or less the same thing, when they usually don’t have the expertise to choose the more appropriate one for their use case, I feel like we should avoid adding the second solution as a general option.
So, unless several more people turn up for whom the current implementation doesn’t work, I don’t think I’d commit any second way of doing this into our code base. In that case, adding it to a separate contrib module would be the better option. (With the additional advantage that we can still merge it if the usage numbers indicate enough interest.)
But even when committing it, we’ll probably need to somehow “hide” this second option for normal users who’ll be happier with the current solution – which, admittedly, is pretty tricky when we still want to advertise it to those who might need it. But, as said, unless there is more interest, I don’t think I’ll commit it either way.
Just so you’re forewarned that even a patch that works great most likely won’t be committed soon. Still, I very much appreciate the initiative and input, and are happy to give feedback on your patch in a new issue.

Posted a follow-up here: https://www.drupal.org/project/search_api/issues/3126368

Thanks for mentioning this here, and for creating the other issue!
However, it’s unnecessary to reference an issue both ways – since you already added this issue as “related” for the other one, you’ll see the new issue is already listed as “Referenced by” in this one. Adding the reference in both directions just makes the list longer. Therefore, I prefer the link to be present in only one of the issues.

At Drupal 9.1.9 I have an issue with translated items using this fallback approach. View is returning both default and translated content... On processor of search api configuration I ve checked "Fallback language" and in the view I ve added the filter "Language (with fallback) (= Interface text language selected for page)". Is there anything else I forgot?

Updated:
Also I found a warning in watchdog : "entity:node/178:ende">, when I index items with fallback enabled on processor.

@ itzikas: I think your warning got truncated, maybe due to HTML in the message?
Anyways, as this feature was added almost two years ago, please create a new (support request) issue for any problems you encounter. (I’m currently not sure how this worked off the top of my head.) This issue is now just for back-porting that feature to Drupal 7.