thephpleague / omnipay-common

Core components for the Omnipay PHP payment processing library
MIT License
330 stars 244 forks source link

AbstractResponse.php inconsistencies #140

Open wimpog opened 7 years ago

wimpog commented 7 years ago

Hi Barry,

in AbstractResponse.php I noticed the following inconsistencies (the master branch):

  1. incorrect return type in

    /**
     * @return HttpRedirectResponse
     */
    public function getRedirectResponse()

    expected use Symfony\Component\HttpFoundation\RedirectResponse as HttpRedirectResponse;, actual: return HttpResponse::create($output); which is: Symfony\Component\HttpFoundation\Response

  2. the class implements a wrong interface (Omnipay\Common\Message\ResponseInterface):

    * @see ResponseInterface
    */
    abstract class AbstractResponse implements ResponseInterface

    methods getRedirectMethod, getRedirectUrl, getRedirectData aren't found in this interface; instead they're found in Omnipay\Common\Message\RedirectResponseInterface

Could you please explain these inconsistencies, and whether or not they should be corrected? Thank you!

barryvdh commented 7 years ago

Yes it onlye return a RedirectResponse in case of a GET; https://github.com/thephpleague/omnipay-common/blob/b1440bd7c0b4fb28ba8250d93695c61ac887685d/src/Omnipay/Common/Message/AbstractResponse.php#L186 The RedirectResponse extends the regular response, so the docblock should be just the normal Symfony\Component\HttpFoundation\Response You can submit a PR to correct this.

The RedirectResponseInterface should only be implemented when it's an actual redirect. Not all responses are redirects. So when a class implements a redirect, it should add the interface to the implementation. So that doesn't need to change.