Merge CommentAccess and NodeAccess into ContentAccess, provide tests

Great job!
The submitConfigurationForm() code is definitely wrong, there should be a cleaner way to do this. But for now, I guess we can leave it with the @todo.

However, the preprocessSearchQuery() doesn't really seem to be updated to D8, and the _search_api_query_add_node_access() was just copied in without also copying the function.

I don't have the time currently to fix this completely, but please use the attached patch as a starting point to properly fix preprocessSearchQuery(). You just need to set the same conditions as before, but the field(s) you set the conditions on have also a datasource prefix. So, loop over all datasources and for each node (or comment) datasource add a OR new filter: either the $datasource_id . IndexInterface::DATASOURCE_ID_SEPARATOR . 'search_api_access_node' field matches your node access criteria, or search_api_datasource is another datasource ID.

+++ b/src/Plugin/SearchApi/Processor/NodeAccess.php
@@ -61,27 +68,31 @@ class NodeAccess extends ProcessorPluginBase {
-      // @todo Only process nodes (check '#datasource' key).
+      if (!in_array($this->index->getDatasource($item['#datasource'])->getEntityTypeId(), array('node', 'comment'))) {
+        continue;
+      }

This should only check for the specific entity type for this processor – i.e., static::$entityTypeId.

However, seeing that, I think it might now make sense to just combine the two processors into one new one that's able to handle both comments and nodes (maybe making it configurable for which datasources it will work). Maybe you can try that, or just try to get these two separate ones to work (or just node access, for now, without the subclass problems).

+++ b/src/Plugin/SearchApi/Processor/NodeAccess.php
@@ -61,27 +68,31 @@ class NodeAccess extends ProcessorPluginBase {
+          $items[$id]['search_api_access_node'][] = "node_access_{$grant->realm}:{$grant->gid}";
         }
       }
       else {
         // Add the generic view grant if we are not using node access or the
         // node is viewable by anonymous users.

As said, you have to use $items[$id]['search_api_access_node']['value'] here instead (which should become an array).

I now had the time to look at this and fixed a few style issues, but generally this seems to work fine.

The tests are too large in scope, though, testing even the Database backend instead of just the core functionality of the processor. I.e., are the conditions correctly set for the query and for indexed items – real indexing / searching isn't necessary, just the preprocessing. If you need a server, you should use a dummy backend class.
The processor tests base class is also very hack-y, installing the schemas without the modules, etc. I can't believe it works – but, I guess, as long as it does, it's not top priority to refactor this. Even though it would certainly be easier to do it right the first time, having something working is also worth something (and especially if it's the tests, it makes it a lot easier to later refactor).

Therefore, I'd say this is fixed now.

The processor tests base class is also very hack-y, installing the schemas without the modules, etc. I can't believe it works – but, I guess, as long as it does, it's not top priority to refactor this.

Not sure what exactly you are referring to, but that's how DrupalUnitTestBase works, you can enable modules and the test does that, but you then need to explicitly specify which tables you want to have installed, but you can only install tables of modules that are "installed". It's a mix of a web test and a unit test, creating a fake environment that only exists in a memory with the exception of things that you explicitly install/create.

Real unit tests have problems too, unless you know exactly what you are doing when you write unit tests, you just write a test that makes sure that your code is passing, like we "successfully" did by having passing tests for various completely broken processors :)

When doing something as complicated as node access query altering, I think makes a lot of sense to do it in a real environment that actually executes that query.

Not sure what exactly you are referring to, but that's how DrupalUnitTestBase works, you can enable modules and the test does that, but you then need to explicitly specify which tables you want to have installed, but you can only install tables of modules that are "installed". It's a mix of a web test and a unit test, creating a fake environment that only exists in a memory with the exception of things that you explicitly install/create.

I was referring to the whole \Drupal\search_api\Tests\SearchApiProcessorTestBase::setUp() method, which looked like a huge hack to me. But if that's how it's done for Drupal Unit tests, it's of course OK and I've learned another thing about those. I guess it makes sense, too.

Real unit tests have problems too, unless you know exactly what you are doing when you write unit tests, you just write a test that makes sure that your code is passing, like we "successfully" did by having passing tests for various completely broken processors :)

That's a problem with all sorts of tests, not really specific to unit tests. Though I admit that in this case, seeing the intended result on the large scale is much easier than seeing what the individual methods should do exactly.

When doing something as complicated as node access query altering, I think makes a lot of sense to do it in a real environment that actually executes that query.

Hm, as long as we can't completely rely on the Node Access API (i.e., Core RC), you might be right there. At least it's good to test that the "complete picture" is working before changing to test the individual methods (as you said, to prevent us from just testing that the processor works as it works).
However, I don't think that using the database backend for that is the right way. It violates the principle of having tests as self-contained as possible – when we are testing a processor, we don't want to test a backend implementation along with it. I think the correct way to do this would be to write a crude test backend that just stores the items as serialized PHP and supports just the few kinds of conditions/queries it has to for the tests to work.
If we had such a backend, I wouldn't be against having (and keeping) both kinds of tests for processors which need it (i.e., which do any query altering) – unit tests to just test the method effects, and larger tests that also test the complete cycle with a working backend.