Move language node access logic into NodeAccessController

Moved the check from node_access to NodeAccessController::viewAccess.

+++ b/core/modules/node/lib/Drupal/node/NodeAccessController.phpundefined
@@ -21,6 +21,23 @@ class NodeAccessController extends EntityAccessController {
+    if (empty($langcode)) {

Is there a way to be empty as it has default LANGUAGE_DEFAULT?

Is there a way to be empty as it has default LANGUAGE_DEFAULT?

Yes, if the value passed is explicitly NULL (which happens in this context of the call that we do from the method where this chunk of code was prior to this patch).

Is there a reason to not move this to a place where it's generic for all entities? Might have to wait until after the access controller refactoring that is happening here: #1947892: Improve DX with EntityAccessControllerInterface. or it needs to be duplicated quite a bit.

Also, Entity:access() and all other entity-related implementations of Accessible:access() need to be updated to default $language to NULL or this code won't get triggered because node_access() is basically just a deprecated wrapper for that and will go away. See #1947880: Replace node_access() by $entity->access(), among other issues.

Something like this.

Also changed the default value to NULL, node_access() is just a BC layer and will go away. Therefore this also isn't a minor nice to have task but needs to happen.

The problem is that core/lib now depends on language.module, which is not good. Not sure what to do about that? But it certainly makes no sense to keep this as node specific code as we want to support all languages equally in terms of language support.

#8: language-negotation-entity-access-1953892-8.patch queued for re-testing.

Re-rolling...

Let's rerole and while we are here make it unit testable and provide one.

Nice work on those tests ;)

I fear that NG is going to make this harder to mock/work against. Maybe we can still mock it by mocking the interfaces and not the class (e.g. for User, I'm changing the access stuff to type hint against UserInterface in the userNG issue).

You're using the base Entity class as mock but are calling the EntityTestAccessController of entity_test.module, which is the access controller for the EntityTest (NG) entity types. It works now because we just check permissions but this might break once we actually do something with $entity there...

Can you maybe do the same by mocking the base EntityAccessController directly?

Merge for #1620010: Move LANGUAGE constants to the Language class

clean-up and case fix

Thanks for the cleanup @andypost

Good idea berdir.
I started with using the actual class, realized that it's not usuable, went with the test controller and then even realized that I need some mocking.

#16: language-negotation-entity-access-1953892-16.patch queued for re-testing.

Patch is green and straightforward so +1 to RTBC

+++ b/core/lib/Drupal/Core/Entity/EntityAccessController.phpundefined
@@ -23,9 +26,61 @@ class EntityAccessController implements EntityAccessControllerInterface {
+      // If the Language module is enabled, try to use the language from content
+      // negotiation.
+      // @todo: Relying on a module in core/lib is not cool.
+      if ($this->moduleHandler->moduleExists('language')) {
+        // Load languages the node exists in.
+        $node_translations = $entity->getTranslationLanguages();
+        // Load the language from content negotiation.
+        $content_negotiation_langcode = $this->languageManager->getLanguage(Language::TYPE_CONTENT)->langcode;
+        // If there is a translation available, use it.
+        if (isset($node_translations[$content_negotiation_langcode])) {
+          $langcode = $content_negotiation_langcode;
+        }
+      }

@berdir asked me to look at this if the language module check is necessary. It does not seem to me that it would be. The negotiated languages are present via bootstrap and entities support getting translations natively. If language module is not enabled, Drupal will understand that the site is in language_default() and even then actively assign language codes from that to things.

merged with latest 8.x status, rerolled and condition removed.

This has two blockers here: https://drupal.org/node/1856976 and here: https://drupal.org/node/1827038 as phpunit will fail. Otherwise unit tests are passing.

+++ b/core/lib/Drupal/Core/Entity/EntityAccessController.php
@@ -23,11 +26,60 @@ class EntityAccessController implements EntityAccessControllerInterface {
+    // If no language code was provided, default to the entity's langcode.
+    if (empty($langcode)) {
+      $langcode = $entity->language()->langcode;
+      // Load languages the node exists in.
+      $node_translations = $entity->getTranslationLanguages();
+      // Load the language from content negotiation.
+      $content_negotiation_langcode = $this->languageManager->getLanguage(Language::TYPE_CONTENT)->langcode;
+      // If there is a translation available, use it.
+      if (isset($node_translations[$content_negotiation_langcode])) {
+        $langcode = $content_negotiation_langcode;
+      }
+    }

It should be safe to just use the entity language, see #1810370: Entity Translation API improvements. It would be good to remove the langcode parameter altogether.

#21: language-negotation-entity-access-1953892-17.patch queued for re-testing.

This means it is unsafe to call $node->access() right now, that's a huge problem.
It also blocks #1947880: Replace node_access() by $entity->access() (where I took a different, hackier approach).

Patch has been rerolled. Submitting for review.

Updating patch with cleanup. Correctly named patch this time!

Per #24, postponing this on #2072945: Remove the $langcode parameter in EntityAccessControllerInterface::access() and friends, unless someone can think of a way to move forward on this without that, in which case, please unpostpone.