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

Race conditions e.g. on post creation: Post not "ready" when added to outbox. #1269

Open
Menrath opened this issue Feb 4, 2025 · 11 comments
Labels

Comments

@Menrath
Copy link
Contributor

Menrath commented Feb 4, 2025

Quick summary

Sometimes there is a race condition:

  • Post creation/update
  • The Post Scheduler gets called immediately
  • In the Scheduler add_to_outbox gets called immediately
  • In add_to_outbox the transformer gets called directly: sometimes even before the object that should get transformed is fully ready (e.g. post-meta is not set yet)

Steps to reproduce

Use the plugin "The Events Calendar". Create a new event post with a venue. The post scheduler gets called which calls add_to_outbox. At this stage the venue is not yet linked to the event post: \tribe_has_venue( $post_id ) returns false. As are most or all other meta values.

If a workaround is available, please outline it here.

Use scheduling within the add_to_outbox function.

@Menrath Menrath added [Type] Bug Something isn't working Needs triage labels Feb 4, 2025
@Menrath Menrath changed the title Use scheduling within the function \Activitypub\add_to_outbox Race conditions e.g. on post creation: .Use scheduling within the function \Activitypub\add_to_outbox to make sure objects are ready. Feb 4, 2025
@Menrath Menrath changed the title Race conditions e.g. on post creation: .Use scheduling within the function \Activitypub\add_to_outbox to make sure objects are ready. Race conditions e.g. on post creation: Post not "ready" when added to outbox. Feb 5, 2025
Menrath added a commit to Menrath/wordpress-activitypub that referenced this issue Feb 5, 2025
Menrath added a commit to Menrath/wordpress-activitypub that referenced this issue Feb 5, 2025
Menrath added a commit to Menrath/wordpress-activitypub that referenced this issue Feb 5, 2025
Menrath added a commit to Menrath/wordpress-activitypub that referenced this issue Feb 5, 2025
@obenland
Copy link
Member

obenland commented Feb 5, 2025

We had to add a callback to make post meta available earlier for posts that get saved through the Rest API. Is that what you're running into here? Or is it something else?

@Menrath
Copy link
Contributor Author

Menrath commented Feb 5, 2025

@obenland This could indeed by the same problem. But maybe there are further issues, e.g. when on the request where a post is created also other related posts or terms are created (venue post/term of an event post that is created on the fly).

@obenland
Copy link
Member

obenland commented Feb 5, 2025

For related posts, my assumption would be that they get added to the outbox through existing hooks. Is terms support something you're working on? Would a callback like the one we used help in that case?

@Menrath
Copy link
Contributor Author

Menrath commented Feb 5, 2025

Not all post types are also scheduled.

But lets take the "The Events Calendar" plugin as an example, it is easier to understand that way and that plugin is highly popular:

  • Venues are stored in a custom post type (not-public). Events are too, of course (public).
  • Venues can be created independently but I'd say most of the time they are created in the same fly on the event edit page.
  • Venues are transformed to as:Place within the as:location of an as:Event. The venue information is also used for the as:Events as:summary.
  • Currently in the wild there is currently (might change) need to send/schedule venues independently. Their current purpose is federated caching, and to be able to reliably map them.
  • With the current status quo they can be queried by ID/URL though, because the custom transformer that is applied and has it's own is_post_disabled check.

However, regarding "The Events Calendar" the problem could be reduced to the post_meta race condition because it seems that wp_insert_post for the venue post is getting called before the post gets inserted/updated. I have to double check that again.

Other example:

Some other event plugins use a custom term for the venues/organizers. I had no time yet to check if there are more race conditions.

Next steps?
Depending on which solutions we might figure out I could adopt my unit tests to also check what's is the outbox, which I would prefer not to have to do. I think as a third party my main focus should be on just hooking into the Transformer Factory and testing that the transformed object is O.K.

@obenland
Copy link
Member

obenland commented Feb 5, 2025

as a third party my main focus should be on just hooking into the Transformer Factory and testing the the transformed object is O.K.

Agreed, in general :)
In this case it's a Core peculiarity that I'm not sure the plugin should be expected to conceal for third-parties.

Could you try that rest_pre_insert_* route and see if that works for you?

@pfefferle
Copy link
Member

While I understand the issue and that this is a quite straight forward fix, it feels wild to have a schedule to add it to a schedule 🫣

It's getting quite complex now!

@pfefferle
Copy link
Member

@obenland the problem is, that this is done by a third party plugin, that @Menrath is also "only" bridging... I am not sure if it is possible to patch so much from the outside...

@Menrath
Copy link
Contributor Author

Menrath commented Feb 5, 2025

@pfefferle And do not forget that batch processing that will follow! Another schedule of the schedule of the schedule. 🫠

Complexity... hmm. Depends on whether there is another less complex option. I feel like having lots of filter and hooks are also not non-complex :)

Could you try that rest_pre_insert_* route and see if that works for you?

@obenland I will do so now.

@obenland
Copy link
Member

obenland commented Feb 5, 2025

I am not sure if it is possible to patch so much from the outside...

I don't know. If we were to remove the _activitypub_ check and just straight set any post meta that comes in, that might already solve it?

Now if that's a good idea or not… 🤷

@pfefferle
Copy link
Member

But in the case of @Menrath it is not necessarily the meta, but the missing taxonomies!

@Menrath
Copy link
Contributor Author

Menrath commented Feb 5, 2025

@obenland I cannot test this without a lot of effort generically. But I ran some manual tests: Using the normal /wp-admin/posts.php to insert a new event "The Events Calendar" first publishes the event post and then creates a venue post. So using a similar filter like this would not work straight-forward. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants