Facet source plugins should provide metadata for facet fields

This is a first shot at it, we need to figure out a way to do this for core search still. Just uploading progress for now as it's too late to continue for today.

@fago: is the path in #2 what you wanted to achieve here? I'd like to get approval before I start implementing this in other places.

Rerolled + converted TranslateEntityProcessor. I'm really liking this as this makes the processor so much easier.

Fixes the unit test, which is now also a little bit simpler.

I forgot to commit on my local branch, so the interdiff includes the interdiff in #6.

This makes the change suggested in #7 and adds a kernel test to test basic functionality of the new methods as well.

This is a first stab at an implementation for core search + a small improvement of the ListItemProcessor but not yet an implementation of the new api there (I don't really know how to do that... that processor has become a beast of complexity that we should improve) + extra comments in the kernel test.

So what's next now?

1. Add a test for core search's translate entity or list item processor
2. Fix the implementation in ListItemProcessor to use the new API to make sure that the implementation is sufficent
3. Profit?

Adds more docs on the interface.

Also fixed the list item processor, now all we need to do is make sure that the implementation for core works.

I don't understand .2

Can't get the list item processor to work with the new getters, but I think we can commit this and open a follow-up for the list item processor. #2851845: List item processor does not work with language references is already changing that processor so that sounds like a better place to refactor the whole thing.

Doing this will break the API, so we should do this before we tag the first beta.

FYI once I apply this patch it breaks the core_views_facets module. I get the following error:

Fatal error: Class Drupal\core_views_facets\Plugin\facets\facet_source\CoreViewsContextualFilter contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\facets\FacetSource\FacetSourcePluginInterface::getDataDefinitions) in /var/www/mysite/web/modules/contrib/core_views_facets/src/Plugin/facets/facet_source/CoreViewsContextualFilter.php on line 24

@albertski: Yeah, that's why it's an api break. Sorry about that but we since we're still in alpha we can break API's still and this is an improvement for all other code.

Rebased.

Those 2 fails are unrelated and fixed as a part of #2877989: Module depends on a search api service and it shouldn't

The test passes again.

Rebased again.

This is just an eyeball review. Mostly nitpicks:

1. +++ b/modules/core_search_facets/src/Plugin/facets/facet_source/CoreNodeSearchFacetSource.php
   @@ -21,7 +21,8 @@ use Symfony\Component\HttpFoundation\Request;
   + *   deriver =
   + *   "Drupal\core_search_facets\Plugin\facets\facet_source\CoreNodeSearchFacetSourceDeriver"
   

   I'm not sure annotations like line breaks like that.

2. +++ b/modules/core_search_facets/src/Plugin/facets/facet_source/CoreNodeSearchFacetSource.php
   @@ -240,7 +241,8 @@ class CoreNodeSearchFacetSource extends FacetSourcePluginBase implements CoreSea
   -    $allowed_field_types = $this->moduleHandler->invokeAll('facets_core_allowed_field_types', [$field_types = []]);
   +    $allowed_field_types = \Drupal::moduleHandler()
   +      ->invokeAll('facets_core_allowed_field_types', [$field_types = []]);
   

   This looks like an unrelated change.

3. +++ b/src/FacetSource/FacetSourcePluginInterface.php
   @@ -96,4 +96,32 @@ interface FacetSourcePluginInterface extends PluginFormInterface, DependentPlugi
   +   * For search api this means that we load the index and call it's getFields
   

   Should be 'Search API' for the module name.

   'its' should have no apostrophe.

   Name of a method should have a () after it.

   Though I'm not sure implementation details should go on the interface -- I would move these to inline comments in the implementations.

4. +++ b/src/Plugin/facets/facet_source/SearchApiDisplay.php
   @@ -4,7 +4,6 @@ namespace Drupal\facets\Plugin\facets\facet_source;
   -use Drupal\Core\Url;
   

   Unrelated change.

5. +++ b/tests/src/Unit/Plugin/processor/ListItemProcessorTest.php
   @@ -90,12 +122,14 @@ class ListItemProcessorTest extends UnitTestCase {
   +
   
   @@ -133,12 +167,14 @@ class ListItemProcessorTest extends UnitTestCase {
   +
   

   Unrelated whitespace changes.

   (Yes, I'm being a stickler for these, but when you're doing archaeology in the git log in six month's time to work out when and why something changed, it really helps if commits are as simple as possible!)

#27.3: I know that's an implementation detail, but it's also a great example to show what this method should do. Not sure if we should remove that. I prefer putting such docs in the interface, so we don't have to copy the rest of the docs to the implementation. (As inheritdoc and doc comments can't be mixed.)

The other points are all valid, so we should change those.

> #27.3: I know that's an implementation detail, but it's also a great example to show what this method should do. Not sure if we should remove that.

Fair enough :)

I couldn't find the original changes for .4 and .5. Reroll attached.

Reroll because of removing core search.

This should fix the failing test.

Can we move the getDataDefenition to the Facet instead of the Facet source?

I dont think there is any need in requesting the entire data definition at once (and if so, we can always add an extra method looping over all fields for this).
Working on this

WIP, tests need to be fixed.