[project @ Discovery refactoring: add discovery result, simplify discovery logic]

6 files changed, 166 insertions, 163 deletions

diff --git a/Auth/OpenID/Discover.php b/Auth/OpenID/Discover.php
index c19cf09..b244059 100644
--- a/Auth/OpenID/Discover.php
+++ b/Auth/OpenID/Discover.php
@@ -324,59 +324,53 @@ function Auth_OpenID_discoverWithYadis($uri, &$fetcher)
     // OpenID 1.0 discovery on the same URL will help, so don't bother
     // to catch it.
     $openid_services = array();
-
-    $http_response = null;
-    $response = Services_Yadis_Yadis::discover($uri, $http_response,
-                                               $fetcher);
-
+    $response = Services_Yadis_Yadis::discover($uri, $fetcher);
+    $yadis_url = $response->normalized_uri;
     $yadis_services = array();
-    $identity_url = $uri;
 
-    if ($response) {
-        $identity_url = $response->uri;
+    if ($response->isFailure()) {
+        return array($uri, array());
+    }
 
-        if ($response->xrds) {
-            $yadis_services =
-                $response->xrds->services(
-                                   array('filter_MatchesAnyOpenIDType'));
-        }
+    $xrds =& Services_Yadis_XRDS::parseXRDS($response->response_text);
+
+    if ($xrds) {
+        $yadis_services =
+            $xrds->services(array('filter_MatchesAnyOpenIDType'));
     }
 
     if (!$yadis_services) {
-        if ($response &&
-            Services_Yadis_XRDS::parseXRDS($response->body) !== null) {
-            return @Auth_OpenID_discoverWithoutYadis($uri,
-                                                     $fetcher);
-        } else if (!$response) {
-            return array($uri, array(), null);
+        if ($response->isXRDS()) {
+            return Auth_OpenID_discoverWithoutYadis($uri,
+                                                    $fetcher);
         }
 
         // Try to parse the response as HTML to get OpenID 1.0/1.1
         // <link rel="...">
         $openid_services = Auth_OpenID_ServiceEndpoint::fromHTML(
-                                        $identity_url,
-                                        $response->body);
+                                        $yadis_url,
+                                        $response->response_text);
     } else {
-        $openid_services = Auth_OpenID_makeOpenIDEndpoints($identity_url,
+        $openid_services = Auth_OpenID_makeOpenIDEndpoints($yadis_url,
                                                            $yadis_services);
     }
 
     $openid_services = Auth_OpenID_getOPOrUserServices($openid_services);
-    return array($identity_url, $openid_services, $response);
+    return array($yadis_url, $openid_services);
 }
 
 function _Auth_OpenID_discoverServiceList($uri, &$fetcher)
 {
-    list($url, $services, $resp) = Auth_OpenID_discoverWithYadis($uri,
-                                                                 $fetcher);
+    list($url, $services) = Auth_OpenID_discoverWithYadis($uri,
+                                                          $fetcher);
 
     return $services;
 }
 
 function _Auth_OpenID_discoverXRIServiceList($uri, &$fetcher)
 {
-    list($url, $services, $resp) = _Auth_OpenID_discoverXRI($uri,
-                                                            $fetcher);
+    list($url, $services) = _Auth_OpenID_discoverXRI($uri,
+                                                     $fetcher);
     return $services;
 }
 
@@ -385,7 +379,7 @@ function Auth_OpenID_discoverWithoutYadis($uri, &$fetcher)
     $http_resp = @$fetcher->get($uri);
 
     if ($http_resp->status != 200) {
-        return array(null, array(), $http_resp);
+        return array($uri, array());
     }
 
     $identity_url = $http_resp->final_url;
@@ -396,7 +390,7 @@ function Auth_OpenID_discoverWithoutYadis($uri, &$fetcher)
                                            $identity_url,
                                            $http_resp->body);
 
-    return array($identity_url, $openid_services, $http_resp);
+    return array($identity_url, $openid_services);
 }
 
 function _Auth_OpenID_discoverXRI($iname, &$fetcher)
@@ -418,7 +412,7 @@ function _Auth_OpenID_discoverXRI($iname, &$fetcher)
     }
 
     // FIXME: returned xri should probably be in some normal form
-    return array($iname, $openid_services, null);
+    return array($iname, $openid_services);
 }
 
 function Auth_OpenID_discover($uri, &$fetcher)
diff --git a/Services/Yadis/XRDS.php b/Services/Yadis/XRDS.php
index 72a0d10..f0f5a65 100644
--- a/Services/Yadis/XRDS.php
+++ b/Services/Yadis/XRDS.php
@@ -274,11 +274,11 @@ class Services_Yadis_XRDS {
         $attrs = $parser->attributes($root);
 
         if (array_key_exists('xmlns:xrd', $attrs) &&
-            $attrs['xmlns:xrd'] != 'xri://$xrd*($v*2.0)') {
+            $attrs['xmlns:xrd'] != Services_Yadis_XMLNS_XRDS) {
             return null;
         } else if (array_key_exists('xmlns', $attrs) &&
                    preg_match('/xri/', $attrs['xmlns']) &&
-                   $attrs['xmlns'] != 'xri://$xrd*($v*2.0)') {
+                   $attrs['xmlns'] != Services_Yadis_XMLNS_XRD_2_0) {
             return null;
         }
 
diff --git a/Services/Yadis/Yadis.php b/Services/Yadis/Yadis.php
index d565889..4bb73fd 100644
--- a/Services/Yadis/Yadis.php
+++ b/Services/Yadis/Yadis.php
@@ -31,6 +31,88 @@ require_once "Services/Yadis/ParseHTML.php";
 require_once "Services/Yadis/XRDS.php";
 
 /**
+ * XRDS (yadis) content type
+ */
+define('Services_Yadis_CONTENT_TYPE', 'application/xrds+xml');
+
+/**
+ * Yadis header
+ */
+define('Services_Yadis_HEADER_NAME', 'X-XRDS-Location');
+
+// Contains the result of performing Yadis discovery on a URI
+class Services_Yadis_DiscoveryResult {
+
+    // The URI that was passed to the fetcher
+    var $request_uri = null;
+
+    // The result of following redirects from the request_uri
+    var $normalized_uri = null;
+
+    // The URI from which the response text was returned (set to
+    // None if there was no XRDS document found)
+    var $xrds_uri = null;
+
+    var $xrds = null;
+
+    // The content-type returned with the response_text
+    var $content_type = null;
+
+    // The document returned from the xrds_uri
+    var $response_text = null;
+
+    // Did the discovery fail miserably?
+    var $failed = false;
+
+    function Services_Yadis_DiscoveryResult($request_uri)
+    {
+        // Initialize the state of the object
+        // sets all attributes to None except the request_uri
+        $this->request_uri = $request_uri;
+    }
+
+    function fail()
+    {
+        $this->failed = true;
+    }
+
+    function isFailure()
+    {
+        return $this->failed;
+    }
+
+    /**
+     * Returns the list of service objects as described by the XRDS
+     * document, if this yadis object represents a successful Yadis
+     * discovery.
+     *
+     * @return array $services An array of {@link Services_Yadis_Service}
+     * objects
+     */
+    function services()
+    {
+        if ($this->xrds) {
+            return $this->xrds->services();
+        }
+
+        return null;
+    }
+
+    function usedYadisLocation()
+    {
+        // Was the Yadis protocol's indirection used?
+        return $this->normalized_uri == $this->xrds_uri;
+    }
+
+    function isXRDS()
+    {
+        // Is the response text supposed to be an XRDS document?
+        return ($this->usedYadisLocation() ||
+                $this->content_type == Services_Yadis_CONTENT_TYPE);
+    }
+}
+
+/**
  * This is the core of the PHP Yadis library.  This is the only class
  * a user needs to use to perform Yadis discovery.  This class
  * performs the discovery AND stores the result of the discovery.
@@ -185,129 +267,61 @@ class Services_Yadis_Yadis {
      * Services_Yadis_Yadis, depending on whether the discovery
      * succeeded.
      */
-    function discover($uri, &$http_response, &$fetcher,
+    function discover($uri, &$fetcher,
                       $extra_ns_map = null, $timeout = 20)
     {
-        if (!$uri) {
-            return null;
-        }
+        $result = new Services_Yadis_DiscoveryResult($uri);
 
         $request_uri = $uri;
-        $headers = array("Accept: application/xrds+xml");
+        $headers = array("Accept: " . Services_Yadis_CONTENT_TYPE);
 
         if (!$fetcher) {
             $fetcher = Services_Yadis_Yadis::getHTTPFetcher($timeout);
         }
 
         $response = $fetcher->get($uri, $headers);
-        $http_response = $response;
 
-        if (!$response) {
-            return null;
+        if (!$response || ($response->status != 200)) {
+            $result->fail();
+            return $result;
         }
 
-        if ($response->status != 200) {
-            return null;
-        }
+        $result->normalized_uri = $response->final_url;
+        $result->content_type = Services_Yadis_Yadis::_getHeader($response->headers,
+                                                                 array('content-type'));
 
-        $xrds_uri = $response->final_url;
-        $uri = $response->final_url;
-        $body = $response->body;
+        if ($result->content_type &&
+            (Services_Yadis_Yadis::_getContentType($result->content_type) ==
+             Services_Yadis_CONTENT_TYPE)) {
+            $result->xrds_uri = $result->normalized_uri;
+        } else {
+            $yadis_location = Services_Yadis_Yadis::_getHeader(
+                                                      $response->headers,
+                                                      array(Services_Yadis_HEADER_NAME));
 
-        $xrds_header_uri = Services_Yadis_Yadis::_getHeader(
-                                                    $response->headers,
-                                                    array('x-xrds-location',
-                                                          'x-yadis-location'));
+            if (!$yadis_location) {
+                $parser = new Services_Yadis_ParseHTML();
+                $yadis_location = $parser->getHTTPEquiv($response->body);
+            }
 
-        $content_type = Services_Yadis_Yadis::_getHeader($response->headers,
-                                                         array('content-type'));
+            if ($yadis_location) {
+                $result->xrds_uri = $yadis_location;
 
-        if ($xrds_header_uri) {
-            $xrds_uri = $xrds_header_uri;
-            $response = $fetcher->get($xrds_uri);
-            $http_response = $response;
-            if (!$response) {
-                return null;
-            } else {
-                $body = $response->body;
-                $headers = $response->headers;
-                $content_type = Services_Yadis_Yadis::_getHeader($headers,
-                                                       array('content-type'));
-            }
-        }
+                $response = $fetcher->get($yadis_location);
 
-        if (Services_Yadis_Yadis::_getContentType($content_type) !=
-            'application/xrds+xml') {
-            // Treat the body as HTML and look for a META tag.
-            $parser = new Services_Yadis_ParseHTML();
-            $new_uri = $parser->getHTTPEquiv($body);
-            $xrds_uri = null;
-            if ($new_uri) {
-                $response = $fetcher->get($new_uri);
                 if ($response->status != 200) {
-                    return null;
+                    $result->fail();
+                    return $result;
                 }
-                $http_response = $response;
-                $body = $response->body;
-                $xrds_uri = $new_uri;
-                $content_type = Services_Yadis_Yadis::_getHeader(
+
+                $result->content_type = Services_Yadis_Yadis::_getHeader(
                                                          $response->headers,
                                                          array('content-type'));
             }
         }
 
-        $xrds = Services_Yadis_XRDS::parseXRDS($body, $extra_ns_map);
-
-        $y = new Services_Yadis_Yadis();
-
-        $y->request_uri = $request_uri;
-        $y->xrds = null;
-        $y->xrds_uri = null;
-        $y->uri = $uri;
-        $y->body = $body;
-        $y->content_type = $content_type;
-
-        if ($xrds !== null) {
-            $y->xrds = $xrds;
-            $y->xrds_uri = $xrds_uri;
-        }
-
-        return $y;
-    }
-
-    /**
-     * Instantiates an empty Services_Yadis_Yadis object.  This
-     * constructor should not be used by any user of the library.
-     * This constructor results in a completely useless object which
-     * must be populated with valid discovery information.  Instead of
-     * using this constructor, call
-     * Services_Yadis_Yadis::discover($uri).
-     */
-    function Services_Yadis_Yadis()
-    {
-        $this->request_uri = null;
-        $this->uri = null;
-        $this->xrds = null;
-        $this->xrds_uri = null;
-        $this->body = null;
-        $this->content_type = null;
-    }
-
-    /**
-     * Returns the list of service objects as described by the XRDS
-     * document, if this yadis object represents a successful Yadis
-     * discovery.
-     *
-     * @return array $services An array of {@link Services_Yadis_Service}
-     * objects
-     */
-    function services()
-    {
-        if ($this->xrds) {
-            return $this->xrds->services();
-        }
-
-        return null;
+        $result->response_text = $response->body;
+        return $result;
     }
 }
 
diff --git a/Tests/Auth/OpenID/Discover_OpenID.php b/Tests/Auth/OpenID/Discover_OpenID.php
index c9857cf..4ead0af 100644
--- a/Tests/Auth/OpenID/Discover_OpenID.php
+++ b/Tests/Auth/OpenID/Discover_OpenID.php
@@ -52,9 +52,9 @@ class Tests_Auth_OpenID_DiscoveryFailure extends PHPUnit_TestCase {
             $expected_status = $status;
 
             $result = Auth_OpenID_discover($this->url, $this->fetcher);
-            list($id_url, $svclist, $http_response) = $result;
+            list($id_url, $svclist) = $result;
 
-            $this->assertEquals($http_response->status, $expected_status);
+            $this->assertEquals($svclist, array());
         }
     }
 }
@@ -390,9 +390,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
         $result = Auth_OpenID_discover($this->id_url . '/404',
                                        $this->fetcher);
 
-        list($id_url, $svclist, $http_response) = $result;
+        list($id_url, $svclist) = $result;
 
-        $this->assertTrue($http_response === null);
         $this->assertTrue($svclist == array());
         $this->assertTrue($id_url == $this->id_url . '/404');
     }
@@ -403,7 +402,7 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
 
         $this->documents[$this->id_url] = array('text/html', $__openid_html);
 
-        list($id_url, $services, $http_response) =
+        list($id_url, $services) =
             Auth_OpenID_discover($this->id_url,
                                  $this->fetcher);
 
@@ -480,8 +479,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
         $this->fetcher->documents = array(
                           $this->id_url => array('text/plain', "junk"));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($this->id_url, $id_url);
 
@@ -496,8 +495,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
                  DISCOVERYBASE_ID_URL => array('application/xrds+xml',
                                                $__yadis_2entries));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($this->id_url, $id_url);
 
@@ -525,8 +524,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
         $this->fetcher->documents = array(
                              $this->id_url => array('text/html', $__openid_html));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($expected_final_url, $id_url);
 
@@ -553,8 +552,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
                                  $this->id_url =>
                                  array('application/xrds+xml', $__yadis_0entries));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($this->id_url, $id_url);
 
@@ -570,8 +569,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
             $this->id_url => array('text/html', $__openid_and_yadis_html),
             $this->id_url . 'xrds' => array('application/xrds+xml', $__yadis_0entries));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($this->id_url, $id_url);
 
@@ -593,8 +592,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
         $this->fetcher->documents = array(
               $this->id_url => array('application/xrds+xml', $__yadis_no_delegate));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($this->id_url, $id_url);
 
@@ -618,8 +617,8 @@ class Tests_Auth_OpenID_Discover_OpenID extends _DiscoveryBase {
                       $this->id_url => array('text/html',
                                              $__openid_html_no_delegate));
 
-        list($id_url, $services, $http) = Auth_OpenID_discover($this->id_url,
-                                                               $this->fetcher);
+        list($id_url, $services) = Auth_OpenID_discover($this->id_url,
+                                                        $this->fetcher);
 
         $this->assertEquals($this->id_url, $id_url);
 
diff --git a/Tests/Services/Yadis/Discover_Yadis.php b/Tests/Services/Yadis/Discover_Yadis.php
index b07747e..4c0d71e 100644
--- a/Tests/Services/Yadis/Discover_Yadis.php
+++ b/Tests/Services/Yadis/Discover_Yadis.php
@@ -110,14 +110,11 @@ class _TestCase extends PHPUnit_TestCase {
     function runTest()
     {
         if ($this->expected === null) {
-            $response = array();
             $this->assertTrue(
-                 Services_Yadis_Yadis::discover($this->input_url, $response,
-                                                $this->fetcher) === null);
+                 Services_Yadis_Yadis::discover($this->input_url,
+                                                $this->fetcher)->isFailure());
         } else {
-            $response = array();
             $result = Services_Yadis_Yadis::discover($this->input_url,
-                                                     $response,
                                                      $this->fetcher);
 
             if ($result === null) {
@@ -128,12 +125,12 @@ class _TestCase extends PHPUnit_TestCase {
             $this->assertEquals($this->input_url, $result->request_uri);
 
             $msg = 'Identity URL mismatch: actual = %s, expected = %s';
-            $msg = sprintf($msg, $result->uri, $this->expected->uri);
-            $this->assertEquals($this->expected->uri, $result->uri, $msg);
+            $msg = sprintf($msg, $result->normalized_uri, $this->expected->uri);
+            $this->assertEquals($this->expected->uri, $result->normalized_uri, $msg);
 
             $msg = 'Content mismatch: actual = %s, expected = %s';
-            $msg = sprintf($msg, $result->body, $this->expected->body);
-            $this->assertEquals($this->expected->body, $result->body, $msg);
+            $msg = sprintf($msg, $result->response_text, $this->expected->body);
+            $this->assertEquals($this->expected->body, $result->response_text, $msg);
 
             $this->assertEquals($this->expected->xrds_uri, $result->xrds_uri);
             $this->assertEquals($this->expected->content_type, $result->content_type);
diff --git a/Tests/Services/Yadis/Yadis.php b/Tests/Services/Yadis/Yadis.php
index 5fbfe4a..8cf50f9 100644
--- a/Tests/Services/Yadis/Yadis.php
+++ b/Tests/Services/Yadis/Yadis.php
@@ -26,23 +26,22 @@ class Tests_Services_Yadis_DiscoveryTest extends PHPUnit_TestCase {
 
     function runTest()
     {
-        $http_response = null;
         $fetcher = Services_Yadis_Yadis::getHTTPFetcher();
         $y = Services_Yadis_Yadis::discover(
-             $this->input_url, $http_response, $fetcher);
+             $this->input_url, $fetcher);
         $this->assertTrue($y !== null);
 
         // Compare parts of returned Yadis object to expected URLs.
-        $this->assertEquals($this->redir_uri, $y->uri);
+        $this->assertEquals($this->redir_uri, $y->normalized_uri);
 
         if ($this->xrds_uri) {
             $this->assertEquals($this->xrds_uri, $y->xrds_uri);
             // Compare contents of actual HTTP GET with that of Yadis
             // response.
             $f = Services_Yadis_Yadis::getHTTPFetcher();
-            $response = $f->get($this->xrds_uri);
+            $http_response = $f->get($this->xrds_uri);
 
-            $this->assertEquals($response->body, $y->body);
+            $this->assertEquals($http_response->body, $y->response_text);
         } else {
             $this->assertTrue($y->xrds_uri === null);
         }