WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#35301 closed defect (bug) (fixed)

list_plugin_updates() performs an API call per plugin item

Reported by: dd32 Owned by: dd32
Milestone: 4.5 Priority: normal
Severity: normal Version: 2.9
Component: Upgrade/Install Keywords:
Focuses: Cc:

Description

Currently list_plugin_updates() performs an uncached API query for each row of the table on every page load.
We should stop doing that, it slows the page down and causes many unnecessary requests to the WordPress.org servers.

The call is to fetch the tested and compatible information for each plugin, api.WordPress.org has been updated to return these pieces of information alongside the update response for WordPress 4.5+ as implemented in the attached patch.

It currently returns objects / forces them to objects in WordPress, the response format from the API may change as needed.

Attachments (2)

35301.diff (6.8 KB) - added by dd32 4 years ago.
35301.2.diff (6.8 KB) - added by dd32 4 years ago.
35301.diff with less debug output & a simpler convert-to-object routine.

Download all attachments as: .zip

Change History (21)

@dd32
4 years ago

#1 @dd32
4 years ago

@nacin I know you've had fun experiences dealing with the API arrays/objects not being standard, do you have any opinion here?

@dd32
4 years ago

35301.diff with less debug output & a simpler convert-to-object routine.

#2 @dd32
4 years ago

In 36182:

Updates: Don't perform an API call to WordPress.org for every plugin update displayed. The API has been updated to return this information with the update response.

See #35301

#3 @dd32
4 years ago

  • Component changed from Plugins to Upgrade/Install
  • Keywords has-patch needs-testing removed
  • Version set to 2.9

Core has had this extra call in there since [12157]. Leaving this open for now while testing it.

#4 follow-up: @ctalkington
4 years ago

I think there maybe implications for non-wporg updaters here since the plugin_information action wouldn't be called anymore.

I think combining the request is great but may be good to add a make/core post due to change in expected data inUpdate API result.

Version 0, edited 4 years ago by ctalkington (next)

#5 in reply to: ↑ 4 @mordauk
4 years ago

Replying to ctalkington:

I think there maybe implications for non-wporg updaters here since the plugins_api plugin_information filter wouldn't be called anymore.

I think combining the request is great but may be good to add a make/core post due to change in expected data inUpdate API result.

I'm almost certain this will break nearly every custom plugin updater but I need to test to confirm.

#6 @SergeyBiryukov
4 years ago

Related: #20002, #meta1304.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#7 follow-up: @dd32
4 years ago

I'm almost certain this will break nearly every custom plugin updater but I need to test to confirm.

That's perfectly acceptable in this case. We can't support plugins which hook into the update process through these methods forever. The plugin check API is also going to change at some point which will more than likely also break all existing custom update providers (although we'd attempt not to..)

#8 in reply to: ↑ 7 @mordauk
4 years ago

Replying to dd32:

I'm almost certain this will break nearly every custom plugin updater but I need to test to confirm.

That's perfectly acceptable in this case. We can't support plugins which hook into the update process through these methods forever. The plugin check API is also going to change at some point which will more than likely also break all existing custom update providers (although we'd attempt not to..)

Really? Sorry to be blunt, but are you serious? You're telling me that it is perfectly acceptable to break the update notifications for thousands of commercial plugins. I'm sorry, but in my opinion that is 100% unacceptable without exhausting other means first.

Of course it's not realistic to support custom integrations forever, but it is WordPress's responsibility to maintain backwards compatibility as best as it can. The plugins_api filter has always been the recommended and best way to hook into the update process for custom plugin updaters. To suddenly say "we don't care, find another way" leaves a really bad taste in my mouth. It would be one thing if this was a ticket that had been opened 3+ years ago, but this is a two week old ticket. To say this kind of change would come at an extreme surprise to a lot of plugin developers would be an strong understatement.

I fully support improving this page and reducing the number of API queries that need to happen here, but it must happen gradually if it's something that will break a lot of plugins.

Now, all that said, luckily this does not appear to break plugins that use the plugins_api, so everything is fine. I do firmly believe it is still important that such flippant responses to potentially wide-spread breakage not be dismissed so easily.

#9 @dd32
4 years ago

In this case where it's only for loading in a small amount of data for plugin compatibility which doesn't affect the overall update ability, breaking how an existing plugin has been hooking into that isn't a big deal.

Completely breaking the ability for a third party plugin to hook in would be an issue.
However, WordPress specifically does not have any supported method for a third-party update component to hook into WordPress at present.
Filtering the update transient is not an API, expecting that format to remain 100% exact for years to come is simply not possible, although hopefully future iterations will build on top of it to extend it, rather than replace completely.

#10 @mordauk
4 years ago

Agree 100%. I thought you were implying it was okay to break custom plugin updaters 100%, hence my not-exactly-nice reply :)

I would love to see a true API in the future, but that's a separate issue.

#11 @ctalkington
4 years ago

Upon digging through the updater library that I'm familiar with, it seems the updates would still work. It indeed will have less details than previously available but it won't outright break which is good.

It seems like moving a few fields between transient and plugins_api handlers in such library won't be a big undertaking.

I'm again all for this sensible reduction in API calls, I just want to make sure that there's some notifications made to the developer community who may have to update their libraries or dependencies.

#12 @toddlahman
4 years ago

A careful migration, with an effort to maintain backward compatibility, even if it is a parallel method, to ease the transition for custom plugin/theme updaters, would be much appreciated. A real API would be nice too.

#13 follow-up: @jorbin
4 years ago

Have any plugins with custom updaters reported breakage with this change? If not, I think we can mark it fixed and @dd32 can continue any work in future work on the plugins update API in future tickets.

cc/ @mordauk @carlhancock as I know that both of them have custom updaters.

#14 @jorbin
4 years ago

  • Owner set to dd32
  • Status changed from new to assigned

#15 in reply to: ↑ 13 @toddlahman
4 years ago

Replying to jorbin:

Have any plugins with custom updaters reported breakage with this change? If not, I think we can mark it fixed and @dd32 can continue any work in future work on the plugins update API in future tickets.

cc/ @mordauk @carlhancock as I know that both of them have custom updaters.

@jorbin,

@toddlahman Also has a custom updater.

#16 @dd32
4 years ago

There's been no breakage reported yet, although anyone who was filtering the update responses to add the compat info to the plugins info endpoint will have broken (I'm not aware of anyone bothering to do that).

This may be reverted from 4.5 yet, pending api.w.org work next week.

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


4 years ago

#19 @dd32
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

This may be reverted from 4.5 yet, pending api.w.org work next week.

The API work needed for this was enabled last week, and no breakage was detected. The update responses for 4.4.2 should include the compatibility fields now too (That was enabled for load testing).

Note: See TracTickets for help on using tickets.