The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
In issue #611772: Node access hooks don't quite make sense... and identified again in #1025928: [meta] Remove the last $op params from our hooks, it was suggested to remove the $op from the function, because of the functions already established in hook_node_access_records.
Proposed resolution
Patch is attached.
| Comment | File | Size | Author |
|---|---|---|---|
| Remove_op-1469758.patch | 2.99 KB | chertzog |
Comments
Comment #1
jhodgdonFixing component, and presumably you wanted this patch reviewed?
Comment #3
jody lynnThere are implementations of this hook in node tests that also need to be changed.
Comment #4
matt2000 commentedThis would remove real functionality, so I'd request careful consideration before taking away a tool that may possibly be in use. Remember, changes to node_access records are costly, and can result in downtime on large sites.
The existence of the $op parameter allows a module developer to further subdivide access groups without cluttering the node_access table.
For example, If I usually grant all permissions to a my_realm::1 but I want to grant the few users who have some profile field property set to some value ("magic_users") the ability to only view my_realm::1 nodes, I can do that with a hook_node_grants, without having to also implement a hook_node_access_records, which will require rebuilding the node_access table (at least in part) and add additional rows to it for every single node in my_realm.
Put another way, the operations allowed in hook_node_access_records can only be changed when the node is saves. (Barring explicitly calling node_access_acquire_grants().) The operations allowed in hook_node_grant can be changed dynamically, anytime.
If my_realm is large, and magic_users is small, using $op makes sense. Don't take that choice away from developers lightly.
The other way to reduce the perceived duplication would be to remove the operations from hook_node_access_records, but these need to be present in the database to support fast bulk operations on nodes. We can take our time calculating what the user can do, because we're only dealing with one at a time, but we want to be able to deal with lots of nodes at a time.
Comment #5
xjm(Merging "node system" and "node.module" components for 8.x; disregard.)
Comment #14
catchThe $op argument is still there, this is a task (if we want to do it at all).
Comment #20
acbramley commentedIs this still something we want to do? As per #4 it seems like functionality that might be used, I'm struggling to find examples in gitlab though.
The hooks are fired from the
node_access_grantsfunction, that function is called in several places with a hardcoded view operation, but NodeAccesControlHandler does pass in the $operation to NodeGrantDatabaseStorage::access which is passed to node_access_grants.Comment #21
acbramley commentedClosing out due to lack of activity.
Comment #22
xjmSubsystem maintainer here. I think that the future in this problem space was not so much to remove the op as to refactor away from it being a single hook into dedicated hooks. I also agree that it is not a bug and way down the list of anything we want to try to do with the node access API. (API changes to node access should be limited to those required for forward compatibility or to replace it with a generic entity access API.) For now, reclassifying as wontfix. Thanks!