[wp-trac] [WordPress Trac] #14786: WP_Http Transport test order refactoring

[WordPress Trac]

#14786: WP_Http Transport test order refactoring
--------------------------+-------------------------------------------------
 Reporter:  hakre         |       Owner:                        
     Type:  defect (bug)  |      Status:  new                   
 Priority:  normal        |   Milestone:  Awaiting Review       
Component:  HTTP          |     Version:  3.0.1                 
 Severity:  normal        |    Keywords:  has-patch dev-feedback
--------------------------+-------------------------------------------------

Comment(by hakre):

 Replying to [comment:16 jacobsantos]:
 >
  {{{
  if ( !call_user_func( array( $class, 'test' ), $args ) )
          continue;
  }}}
 >
 > Should be:
 >
  {{{
  if( ! $class::test($args) )
         continue;
  }}}
 >
 > for performance. The syntax should work fine, but I haven't tested it.
 AFAIK the later is PHP 5.3. So for static calls on a variable class name I
 choosed the first method: ''As of PHP 5.3.0, it's possible to reference
 the class using a variable.''
 ([http://php.net/manual/en/language.oop5.static.php From the PHP Manual])

 > Also, the Method check in Fopen is wrong. The problem is not the method
 but sending with a body as part of the message. It is possible to send a
 body with GET requests.

 Okay, that needs to be added in {{{..._fopen::test()}}} then. I'm not that
 firm with the body concept. Anyway, on instatiating WP_Http, args will
 ever be an empty array. So this might be just the design flaw?

 ----

 Replying to [comment:17 jacobsantos]:
 > Replying to [comment:15 hakre]:
 > >  * The design limitations as discussed in #11613 are not dealt with so
 far. But The changes so far should make it more easily to add such a
 functionality.
 >
 > I believe the solution in that ticket, should fix the problems. I think
 too much is being relied on with the test() method, which should be
 revisited to not be as used as much as it is now. The test() method was
 only ever supposed to be used as to whether the transport was supported by
 the PHP version and whether the extension was installed.

 So this should only be necessary to run once per request regardless of
 which args etc. . That would explain why for the &_post... function there
 was a different test order then in the &_get... variant. Just hardcoding.

 I then suggest to only test() on a first initialisation done once per
 request (not per class instantiation), to leave WP_Http with an array of
 available transports that can be used later on.

 > Using it as a means to whether the transport supports SSL is wrong. They
 should all support SSL provided openSSL extension is installed and barring
 any PHP specific bugs.
 I'm not that firm with SSL but as far as my PHP experience goes on hosts,
 it's like you describe it. It's there or it isn't, but the type of
 transport/streamwrapper does not make a difference then.

 > Also, the fear with some of the checks, for example nonblocking, is that
 so few of the transport supports nonblocking that you limit which
 transport will be used.

 Are there even non-blocking usage examples in Wordpress?

 > Also, given that also some transports like cURL aren't written to
 support nonblocking correctly it adds additional complexity to adding it.

 Again, where are these non-blocking requests used in a shared-nothing,
 single request application architecture? Is there some fetching multiple
 RSS feeds at once or so?

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/14786#comment:18>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software