Don't fail on libxml errors if the RSD URL can still be found #35
Conversation
| @@ -79,15 +79,30 @@ public static function newFromApiEndpoint( $apiEndpoint ) { | |||
| * @throws RsdException If the RSD URL could not be found in the page's HTML. | |||
| */ | |||
| public static function newFromPage( $url ) { | |||
There was a problem hiding this comment.
This method is starting to get a little bit big.
There was a problem hiding this comment.
RSD could conceivably be done in its own package, then here we'd just have to do something like
$rsd = new Rsd($url);
if (!$rsd->hasApi('MediaWiki')) {
throw new RsdException( "Unable to find RSD URL in page: $url" );
}
return self::newFromApiEndpoint( $rsd->getApi('MediaWiki')->getApiLink() );But I reckon methods that can be seen whole on one screen are okay. Although I know some people say 20 lines is about the max.
Up to you; I'm happy to rework. (I don't think splitting bits into other methods in this class is much better than this though.)
|
@samwilson I'm guessing this could go in a bugfix release? |
Are we following semver? If so, I'd say this is a backwards-compatible bug fix that doesn't introduce any new features, so yeah, a patch release is fine. :-) |
|
@samwilson Looks like this is good to go once rebased! IMO |
Various things, such as duplicate element IDs or repeated attributes, break DOMDocument::loadHTML() and so this turns off the direct reporting (E_ERROR) of these and istead adds them to the RsdException if the RSD URL really can't be determined from the HTML. In most cases, the URL can be found correctly and the errors can be disregarded. A test is added for this as well, although it does *not* test for the case when the RSD URL can't be found and there are libxml errors (because we need to serve up a broken HTML file, and the mediawiki-api-base test system can only interact with MediaWiki via the API, which makes it hard to produce broken HTML that doesn't also have the correct LINK element for the RSD URL). A new TestEnvironment::savePage method is added, for easier creation of test wiki pages. Bug: https://phabricator.wikimedia.org/T163527
samwilson
force-pushed
the
api-url-from-page
branch
from
5453e45 to
9da1b47
Compare
|
Done |
Various things, such as duplicate element IDs or repeated
attributes, break DOMDocument::loadHTML() and so this turns off
the direct reporting (E_ERROR) of these and istead adds them
to the RsdException if the RSD URL really can't be determined
from the HTML. In most cases, the URL can be found correctly
and the errors can be disregarded.
A test is added for this as well, although it does not test
for the case when the RSD URL can't be found and there are
libxml errors (because we need to serve up a broken HTML file,
and the mediawiki-api-base test system can only interact with
MediaWiki via the API, which makes it hard to produce broken
HTML that doesn't also have the correct LINK element for the
RSD URL).
A new TestEnvironment::savePage method is added, for easier
creation of test wiki pages.
Bug: https://phabricator.wikimedia.org/T163527