Naming convention issue with post update hooks

It took a bit of digging. I believe this has the following cause but I'm unsure about a correct fix:

In DbUpdateController::selection both update and post_update functions are loaded one after another (in that order). For post_update functions the function UpdateRegistry::getPendingUpdateInformation() function is called which makes a call to UpdateRegistry::getAvailableUpdateFunctions() . That function will return an array of functions based on the pattern /^(?<module>.+)_' . $this->updateType . '_(?<name>.+)$/ with $this->updateType == 'post_update' . This causes the now loaded function social_post_update_N to register as an available update for the social module.

A way to fix this would be to add some inspection using the ReflectionFunction class to check if the update function is from the correct file (hook_update_N from module.install and hook_post_update_N from module.post_update.php).

This architectural flaw affect all new Opensocial versions. I recommend to rename module social_post to other name. It's a relatively easier way to avoid problem.

Moved from stale core issue to open social itself.

This remains an issue as of 10.0.2

@basvredeling

As the issue block any update that request a database update this can become critical and @ carlos-miranda-levy indicate that the issue remains as of 10.0.2, is there a workaround ?
Can we by-pass this update 8801 ?

Thanks.

See temp fix in: https://www.drupal.org/project/social/issues/3177521

The issue remains as of 10.0.3.

We keep referring to closed issues.

Can the op of the patch on 3177521 please post it here for review and so that some action can take place on this matter.

The isue still remains.
Every fresh install returns the same error when trying to update.

Since there is no review or verification of the github posted patch, it's not being tested or validated, probably not even considered for inclusion -- there is no one assigned to this issue if you check.

It is still not clear what would the best course of action to take now:

1. Rename the conflicting function social_post_update to social_post__update on modules/social_features/social_post/social_post.install?
2. Replace all instances of social_post_update with social_post__update on modules/social_features/social_post/social_post.install?
3. Apply the "massive" patch posted on github for modules/social_features/social_post/social_post.install and modules/social_features/social_post/social_post.post_update.php, already a couple of minor releases behind?
   
   https://github.com/vredeling/open_social/commit/d95b83412f21ae2aabe6fd9a...

Any advice on pros-cons?

I'm currently dealing with it after install or update and before running drush updb, otherwise it fails:

cd /var/www/vhosts/social10/html/profiles/contrib/social/modules/social_features/social_post
sed -i 's/social_post_update/social_post__update/g' social_post.install

Thanks.

This issue remains on 10.0.4
Still no best recommendation on how to deal with it.
The existing fixes are of course erased when updating the distribution code and the problem returns.

I'm currently dealing with it by running the following after install or update and before running drush updb, otherwise it fails:

cd /var/www/vhosts/social10/html/profiles/contrib/social/modules/social_features/social_post
sed -i 's/social_post_update/social_post__update/g' social_post.install

But I'm still not sure if this is the best course of action.

This issue renders the distro unreliable and unsafe for anyone not willing to edit the code with an adventurous spirit.

Can someone provide a sound, trusting advice on how to deal with it until we get a fix?

As a workaround I created new sub-module that contains the social_post update hooks.

https://github.com/goalgorilla/open_social/pull/2281

Patch: https://github.com/goalgorilla/open_social/commit/a50e06b30109ac9e56399d...

#13 seems to work for now and at least up to 10.0.5.

cd html/profiles/contrib/social
wget https://github.com/goalgorilla/open_social/commit/a50e06b30109ac9e56399d8e668c14a94795f048.patch
patch -p1 --dry-run < a50e06b30109ac9e56399d8e668c14a94795f048.patch
patch -p1  < a50e06b30109ac9e56399d8e668c14a94795f048.patch

cd ../../../sites/sitedirectory/
drush pm:enable social_post_updates

I'm concerned, as the patch proposed #13 seems to leave updates out.

I just did a fresh install of 10.0.5, followed by composer update.

The Status Dashboard reported that there were database updates available, so I ran drush updb and got this output:

drush updb

 -------- -------------- ------------- ----------------------------------------
  Module   Update ID      Type          Description
 -------- -------------- ------------- ----------------------------------------
  social   8801           post-update   Set the view mode to use when shown in
                                        activities.
  social   8802           post-update   Give access to creating posts of
                                        specific types.
  social   8804           post-update   Create "Featured" view modedisplay for
                                        post.
  social   8901           post-update   Update likes in post activity and
                                        comment view modes.
  social   dependencies   post-update   Implements hook_update_dependencies().
 -------- -------------- ------------- ----------------------------------------


 Do you wish to run the specified pending updates? (yes/no) [yes]:
 > yes

>  [warning] Post update function social_post_update_8801 not found in file social.post_update.php
>  [error]  Update failed: social_post_update_8801
 [error]  Update aborted by: social_post_update_8801
 [error]  Finished performing updates.

I restored the database from the backup I made previous to attempting the update, applied the patch from #13 and ran drush updb again, only to be greeted with the news that there were no updates available!!!

drush updb
 [success] No pending updates.

So were 8804, 8901 and the other updates not valid/necessary updates?

Thanks very much @Harlor for the patch in #13. It seemed to work well for me. However, I ran it on 10.0.7 and hoping this won't cause any issues.

This was an update of social 7 to social 8. I have not tried it on social 10.x
https://www.drupal.org/project/social/issues/3177521#comment-14138201

Hello all.... I could not get the updates to happen from the command line using drush.

They did work in the UI http://site/update.php did work

First of all thank you for all the work and input.
We are trying to still get this in to core, it's not an Open Social architectural flaw, it is an issue with the brittle way these hooks are being used and named.

You can see the work in progress in core here: https://www.drupal.org/project/drupal/issues/3236316

Our aim here is to make Drupal more flexible first, if it does not get accepted in there we will likely come up with our own way.
Creating a new module to deal with these update hooks is not going to be something we can work with, maintaining separate modules for this is not the way to go. It is also very prone to issues where the configuration and schemas are actually not part of that module but social_post itself still.

If we could get some reviews for the issue in core, to get some traction there that would be great!
You can find the merge request in there https://git.drupalcode.org/project/drupal/-/merge_requests/1215 where all the latest code, tests and fixes have been added.

https://github.com/goalgorilla/open_social/pull/2508 you can find the PR in OS here :).

Thanks for the writeup: @ronaldtebrake

After clicking through to the issue, I am left with a few questions.

I understand that you want to introduce a way in core for core to help determine if there is a conflict. I am all for it.

but there is a conflict now between open social social_post and drupal.org social post. How would what you outlined above deal with that.

Wouldn't the solution above just send an error message that there is a conflict? Is there an automated way to fix it?

For Open Social could all elements be prefixed with "os_" or just specifically social_post? So social_post would become os_social_post or it could be os_post.

Thanks for all that you do for the Open Social community.

No problem, thanks for your input @SocialNicheGuru, let me try to answer your questions.

I understand that you want to introduce a way in core for core to help determine if there is a conflict. I am all for it.
but there is a conflict now between open social social_post and drupal.org social post. How would what you outlined above deal with that.

Wouldn't the solution above just send an error message that there is a conflict? Is there an automated way to fix it?

It wouldn't unfortunately, that is something the related issue #3167625: Deprecate/replace hook_update_N() in favor of an object-oriented approach similar to Laravel migrations would be able to deal with. Having it's own namespaces instead of using module naming / file naming conventions is a bigger architectural approach to this and would likely be taking a while to implement. My solution, in a nutshell, just says, post_update hooks should live in a post_update.php file, regardless of how you have named your module.

For Open Social could all elements be prefixed with "os_" or just specifically social_post? So social_post would become os_social_post or it could be os_post.

We have been thinking about this for a while now, but it isn't as straightforward as it sounds. Migrating that is very BC breaking, as people might have used things like: $module_handler->moduleExists('social_post'), all the alter hooks need to be rewritten, dependencies will break etc.

And it's very brittle right, we can't be sure somebody won't start creating an os_post or os_social_group module either. Even if is very edge case that would likely happen, Drupal allows it. So it feels like we are chasing something that will never be fully fixed, if we don't fix it in Drupal Core itself.

See f.e. https://www.drupal.org/project/os_base which will conflict if we are going for a os_base module.

The applied patch will land in Open Social10.3.x and above.

The issue is not fixed in 10.3.1
I am still getting it. I understand it will be instead fixed in drupal core

Thanks