I have a case where I need to append my own checks into the User entity's access callback, but unfortunately I can't set my own 'access callback' as the Entity module uses 'hook_module_implements_alter()' to ensure it has the last say and overwrites my own changes.

I can't even use 'hook_module_implements_alter()' myself because mine runs earlier. While I could change the weight of my module it is less than ideal.

Instead, it seems to be that the Entity module should check if certain values have been already set before setting them itself, or at least implementing a pass-through method (take the old callback and if it's own checks return FALSE, pass through to the previous callback to let it have the last say) or a drupal_alter() approach in all of it's callbacks.

CommentFileSizeAuthor
#13 entity-info-metadata-alter-2269989-13-D7.patch8.92 KBBird-Kid
#4 entity-2269989-4-entity_info_metadata_alter.patch8.11 KBles lim
#1 entity_info_metadata_alter-2269989-1.patch439 bytesdeciphered

Comments

deciphered’s picture

deciphered CreditAttribution: deciphered commented
Status: Active » Needs review
StatusFileSize
new entity_info_metadata_alter-2269989-1.patch439 bytes

This was the easiest solution for the issue, simply adding an alter to the _entity_info_add_metadata() function. I'm happy to revisit if a better solution is suggested.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

an alter for an alter seems wrong to me. I'd ok with setting the values in a way you can enforce others tough, e.g. we could be just doing $info += $array_of_additions; ?

deciphered’s picture

deciphered CreditAttribution: deciphered commented

I agree that it seems wrong, which is why it wasn't my first attempt at a fix.

I did try adding all the metadata additions in the exact manner described, as well as using array_merge_recursive(), but neither of those resulted in a satisfactory result:
- += ignored any nested items if the parent was present, which it always is.
- array_merge_recursive() doubles up on nested items if present.

My initial feeling is that each callback should be iterated over and shift the existing item into an secondary array so that it can be passed through in the metadata callback;

eg., if $entity_info['user']['access callback'] is set, array_unshift it into $entity_info['user']['access callbacks'], then during the metadata access callback if the result is not TRUE, pull in the $entity_info values and array_shift() the callbacks (if there are any) and pass return from that callback.

The reason I didn't start with this is two fold:
- The $entity_info object isn't passed to the callback.
- A lot of work would be required to implement this change.

les lim’s picture

les lim
he/they
Primary language English
CreditAttribution: les lim commented
StatusFileSize
new entity-2269989-4-entity_info_metadata_alter.patch8.11 KB

I just ran into the same problem, trying to use a custom callback for $entity_info['comment']['access callback'].

Did you try drupal_array_merge_deep()? That should prevent _entity_info_add_metadata() from overwriting previously set properties or doubling nested items.

alberto56’s picture

alberto56 CreditAttribution: alberto56 at Dcycle commented
Status: Needs review » Reviewed & tested by the community

The code is straightforward, and I successfully tested this on my site.

checker’s picture

checker CreditAttribution: checker commented

Looks good.
Is "if (!module_exists('file_entity')) { .. }" still necessary?

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
CreditAttribution: nancydru commented

See also #2398129: "Add new" taxonomy entity reference requires elevated permissions which needs this patch before that issue can be resolved.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
CreditAttribution: nancydru commented

Applies with a 5 line offset to the current Git checkout.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
CreditAttribution: nancydru commented

Seems to work for me.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago at drunomics commented
Status: Reviewed & tested by the community » Needs work

While that seems reasonable, I fear it might create issues in subtle situations where modules already provide a key that clashes with the one of entity.module. Thus, I'd rather opt to not commit a risky change like that.

I think having to change module implements order or module weights is fine to achieve the alter of the alter. I'd be happy though to commit a change to make that easier, e.g. we could move the module implements alter of entity module to the top?

drupalfan2’s picture

drupalfan2 CreditAttribution: drupalfan2 as a volunteer commented

Is this patch still necessary for latest Drupal 7 version?

drupalfan2’s picture

drupalfan2 CreditAttribution: drupalfan2 as a volunteer commented
Bird-Kid’s picture

Bird-Kid CreditAttribution: Bird-Kid commented
StatusFileSize
new entity-info-metadata-alter-2269989-13-D7.patch8.92 KB

Unfortunately this issue is still active.

We are using custom entity classes for core entities such as node and taxonomy_term in production. Inconveniently, entity.module is setting a creation callback for nodes. That callback ignores the entity controller and creates a new, class-less object instead. As mentioned by Deciphered, when your own module is invoked before the entity module, you cannot even use an implementation hook_module_implements_alter, because entity module is implementing the very same hook to ensure its implementation of entity_info_alter is run last.

I have looked into both patches provided. While they do provide solutions to the issue at hand, I also somewhat agree with fago's concerns:

  1. Patch #1, as mentioned by fago and Deciphered, is essentially an alter within an alter, implementing a second hook to modify not just the new entity info metadata, but all of entity info. This might make figuring out which hook to use and when a little ambiguous.
  2. Patch #2, again as mentioned by fago, may result in some entity metadata info being lost due to accidentally key collisions with other modules. (Note that in this patch, definitions set previously in entity info take precedence over the new info metadata, hence the potential for collisions affecting the entity module.)
  3. Additionally, the solution Patch #2 is not ideal when, instead of specify your own definition, you want to just disable a definition set in _entity_info_add_metadata(). Doing so with this patch would require setting something like
    $entity_info['node']['creation callback'] = NULL;

    in an implementation of hook_entity_info or hook_entity_info_alter, which IMO is not very declarative.

Attached here is a patch which I'd argue merits the best of both previous patches, while trying to address the aforementioned concerns:

  1. While in the order of execution we are technically still calling drupal_alter() from within an implementation of hook_entity_info_alter, it is not exposing the same information twice. Here, only the newly defined entity info metadata is being passed for altering.
  2. In this patch, entity info metadata does take precedence over previously declared entity info definitions. Any potential key collisions in order modules be present here as much as they would be without this patch.
  3. Simply unsetting an entity info metadata definition is a little more straightforward, but declaring
    unset($entity_info_metadata['node']['creation callback']);

    from inside an implementation of hook_entity_info_metadata_alter.

We are already using this patch in production without any issues. Hope this helps!

Bird-Kid’s picture

Bird-Kid CreditAttribution: Bird-Kid commented
Status: Needs work » Needs review