Deprecate NodeAccessControlHandlerInterface::writeGrants() for removal in Drupal 9.0.x

I agree, and it certainly doesn't hurt to deprecate it. At least it will serve to tell anyone that uses it that they shouldn't (which is already the case, if the documentation is right).

Slightly improved version with some documentation clean-up.

Now the question seems to be if the same warning to not use NodeGrantStorageInterface::write() directly also means that we should deprecate it or remove the warning. That function isn't a simple wrapper like this one, so I'd be strongly against deprecating it (and besides, what's the benefit of a storage interface if you can't write to it??).

#2473041: Deprecate node_access_grants() and move its functionality to NodeAccessControlHandler is basically postponed on this issue.

Retesting to see if it still doesn't explode.

+++ b/core/modules/node/src/NodeGrantDatabaseStorageInterface.php
@@ -59,7 +59,7 @@ public function alterQuery($query, array $tables, $op, AccountInterface $account
    * Note: Don't call this method directly from a contributed module. Call
-   * node_access_write_grants() instead.
+   * \Drupal\node\NodeAccessControlHandlerInterface::acquireGrants() instead.

Not really in scope here, but if contrib shouldn't call this, then it shouldn't be part of the interface.

+++ b/core/modules/node/src/Entity/Node.php
@@ -111,7 +111,10 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
     if ($this->isDefaultRevision()) {
-      \Drupal::entityManager()->getAccessControlHandler('node')->writeGrants($this, $update);
+      /** @var \Drupal\node\NodeAccessControlHandlerInterface $access_control_handler */
+      $access_control_handler = \Drupal::entityManager()->getAccessControlHandler('node');
+      $grants = $access_control_handler->acquireGrants($this);
+      \Drupal::service('node.grant_storage')->write($this, $grants, NULL, $update);
     }

Is there really no place these services are injected into the entity?

$ git apply deprecate-2473021-5.patch 
error: patch failed: core/modules/node/src/NodeAccessControlHandlerInterface.php:34
error: core/modules/node/src/NodeAccessControlHandlerInterface.php: patch does not apply

Reroll of #5.

Retesting, as #9 is correct.

As to the comments in #6:
1. It's only correcting the comment to point to the right place. Moving this method out of the interface is as referred, out of scope.
2. No. These services are not injected. There's one use each of these services and they both use the same code. The only thing I found, two steps above in the class hierarchy, is that we could replace \Drupal::entityManager() with $this->entityManager() (which in turn would call \Drupal::entityManager(), thus adding one extra stack call).

Requesting some triage on this. It's mostly documentation, but it also contains some preparation for removing the code in writeGrants, by copying it's logic to the places where it's used.

And the tag.

Should it be deprecated or @internal?

Patch looks good to be in.

Committed/pushed to 8.1.x, thanks!