Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #2880361: Add core patch for correct checking if post_update is hook_post_update_NAME() or hook_update_N(). #2508

Merged
merged 1 commit into from Sep 27, 2021

Conversation

ronaldtebrake
Copy link
Contributor

@ronaldtebrake ronaldtebrake commented Sep 19, 2021

Problem

As mentioned: https://www.drupal.org/project/drupal/issues/548470

The underscore separator in the hook naming is very brittle. If a module invents the foo_bar hook and another invents the bar hook and you also have two modules called something_foo and something then all hell breaks lose. Also, it's impossible to see from a function name whether it's a hook implementation or not (and unless every non-hook implementation function name is carefully prefixed with an underscore, just with enabling another module suddenly it might become one).

We have found an issue with exactly this problem.

It has to do with the fact we have an install profile called social and a module called social_post
This means any regular update hooks (f.e. hook_update_N() or hook_update_dependencies()) used are considered as social module's hook_post_update_NAME(). instead of social_post module's hook_update_%

The same can be done with the standard install profile for example, by creating a standard_post module or any other module with the module_post name.

Solution

For now we propose to at least fix this by adding some inspection in DbUpdateController.php to check if the hook_post_update_NAME function is coming from the correct file module.post_update.php
This aligns with what is indicated when the post update hook can't be found.

For example in our case:
> [warning] Post update function social_post_update_8801 not found in file social.post_update.php

Issue tracker

In Open Social -
https://www.drupal.org/project/social/issues/2880361

In Drupal -
https://www.drupal.org/project/drupal/issues/3236316

How to test

  • See that update.php/selection does not show any social_post related update hooks.
  • See that update path from an older version of Open Social is still accepted

Release notes

Social post update hooks should now correctly not show up anymore when trying to update.

…o see if it works with our social_post update hook issues
@ronaldtebrake ronaldtebrake added the status: needs review This pull request is waiting for a requested review label Sep 19, 2021
@ronaldtebrake ronaldtebrake added this to the 10.2.6 milestone Sep 19, 2021
@navneet0693 navneet0693 merged commit a807771 into main Sep 27, 2021
navneet0693 added a commit that referenced this pull request Sep 27, 2021
…ing-convention

Issue #2880361: Add core patch for correct checking if post_update is hook_post_update_NAME() or hook_update_N().
@navneet0693 navneet0693 removed the status: needs review This pull request is waiting for a requested review label Sep 27, 2021
@navneet0693 navneet0693 modified the milestones: 10.2.6, 10.3.0 Sep 27, 2021
@navneet0693 navneet0693 deleted the issue/2880361-post-hook-naming-convention branch September 27, 2021 08:21
@navneet0693
Copy link
Collaborator

🍒 -picked to 10.3.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants