Make WordPress Core

Opened 10 months ago

Closed 7 months ago

Last modified 6 months ago

#58705 closed enhancement (fixed)

Deprecate WP_Http_Curl and WP_Http_Streams classes

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: desrosj's profile desrosj
Milestone: 6.4 Priority: normal
Severity: normal Version: 4.6
Component: HTTP API Keywords: has-patch needs-dev-note
Focuses: Cc:

Description

Background: #33055, #43414, #52622.

Since the Requests library was introduced in [37428], the WP_Http_Curl and WP_Http_Streams classes are no longer used by WordPress core.

The only two methods referencing these classes are:

  • WP_Http::_get_first_available_transport()
  • WP_Http::_dispatch_request()

::_dispatch_request() was marked as deprecated in favor of WP_Http::request() in [42766] & [44346] / #43414.

To avoid confusion:

  • WP_Http_Curl and WP_Http_Streams classes should be marked as deprecated in favor of WP_Http.
  • WP_Http::_get_first_available_transport() should be marked as deprecated too, its purpose is now served by \WpOrg\Requests\Requests::get_transport_class().

Change History (13)

This ticket was mentioned in PR #5002 on WordPress/wordpress-develop by @rajinsharwar.


8 months ago
#1

  • Keywords has-patch added

Deprecating WP_Http_Curl and WP_Http_Streams classes for 6.4

Trac ticket: https://core.trac.wordpress.org/ticket/58705

@hellofromTonya commented on PR #5002:


8 months ago
#2

LGTM 👍 Thank you @Rajinsharwar for your contribution!

#3 @hellofromTonya
8 months ago

  • Keywords commit added

The patch https://github.com/WordPress/wordpress-develop/pull/5002 LGTM and seems to address each of the TODOs @SergeyBiryukov listed in the description.

Marking for commit.

#4 @desrosj
7 months ago

  • Keywords needs-dev-note added
  • Owner set to desrosj
  • Status changed from new to assigned

I think this looks good to commit as well.

Before I land it, I wanted to call out that there are 77 instances of WP_Http_Stream and 215 instances of WP_Http_Curl.

I don't think any of these are particularly concerning. But this definitely should be mentioned in a miscellaneous changes developer note.

#5 @desrosj
7 months ago

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

In 56655:

HTTP API: Deprecate WP_Http_Curl and WP_Http_Streams classes.

These classes have not been used in WordPress Core since the Requests library was introduced in [37428]. These classes are now deprecated in favor of WP_Http.

There are two remaining spots in Core that reference these classes:

  • The WP_Http::_dispatch_request() method, which was marked as deprecated in favor of WP_Http::request() in [42766]/[44346].
  • The WP_Http::_get_first_available_transport().

That latter is now also marked as deprecated in favor of \WpOrg\Requests\Requests::get_transport_class().

Props SergeyBiryukov, rajinsharwar, hellofromTonya.
Fixes #58705.

#6 follow-up: @desrosj
7 months ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this to consider one final adjustment related to this.

The http_api_transports filter is within WP_Http::_get_first_available_transport() and is only used within that method (nowhere else in Core).

I think it's worth also deprecating this filter.

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


7 months ago

#9 in reply to: ↑ 6 @hellofromTonya
7 months ago

Replying to desrosj:

Reopening this to consider one final adjustment related to this.

The http_api_transports filter is within WP_Http::_get_first_available_transport() and is only used within that method (nowhere else in Core).

I think it's worth also deprecating this filter.

As the public method is now deprecated, yes, I agree with you @desrosj that it makes sense to also deprecate the filter.

Here's a search query of the plugins using that filter https://wpdirectory.net/search/01HB6Y1MTDKR9BTT9JAP3XD3H1.

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


7 months ago

#11 @SergeyBiryukov
7 months ago

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

In 56723:

HTTP API: Deprecate the http_api_transports filter.

The filter is only used within the WP_Http::_get_first_available_transport() method, which has been marked as deprecated in favor of \WpOrg\Requests\Requests::get_transport_class().

Follow-up to [56655]

Props desrosj, hellofromTonya.
Fixes #58705.

#12 @webcommsat
6 months ago

@desrosj from reading this ticket, is this one planned to have a dev note in a cluster of tickets or rather than a single dev note?
Update: comment in the ticket for a call out in Miscellaneous Dev Note.

My understanding of this is that this fix removes references to WP_Http_Curl and WP_Http_Streams classes which are no longer used by WordPress core since the Requests library was introduced in [37428].

Documentation [tracker]https://github.com/WordPress/Documentation-Issue-Tracker/issues/1165. Thanks.

Last edited 6 months ago by webcommsat (previous) (diff)

#13 @webcommsat
6 months ago

This has been added to the Field Guide as single bullet point on deprecating based on the commit message. We will keep the dev note as needed, and be grateful if when ready for review it can be added to the tracker link above. The field guide can be updated post-publication with references to this ticket too. Thanks.

Note: See TracTickets for help on using tickets.