Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused RedirectException and add some PHPDocs #1497

Merged
merged 1 commit into from
Nov 29, 2018
Merged

Remove unused RedirectException and add some PHPDocs #1497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions system/HTTP/Message.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ public function getHeaderLine(string $name): string
/**
* Sets a header and it's value.
*
* @param string $name
* @param string $value
* @param string $name
* @param array|null|string $value
*
* @return Message|Response
*/
Expand Down
53 changes: 32 additions & 21 deletions system/HTTP/Response.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@
use Config\Format;
use CodeIgniter\HTTP\Exceptions\HTTPException;

/**
* Redirect exception
*/
class RedirectException extends \Exception
{

}

/**
* Representation of an outgoing, getServer-side response.
*
Expand Down Expand Up @@ -310,8 +302,8 @@ public function getStatusCode(): int
* provided status code; if none is provided, will
* default to the IANA name.
*
* @return self
* @throws \InvalidArgumentException For invalid status code arguments.
* @return $this
* @throws \CodeIgniter\HTTP\Exceptions\HTTPException For invalid status code arguments.
*/
public function setStatusCode(int $code, string $reason = '')
{
Expand Down Expand Up @@ -411,7 +403,7 @@ public function setContentType(string $mime, string $charset = 'UTF-8')
/**
* Converts the $body into JSON and sets the Content Type header.
*
* @param $body
* @param array|string $body
*
* @return $this
*/
Expand All @@ -428,13 +420,18 @@ public function setJSON($body)
* Returns the current body, converted to JSON is it isn't already.
*
* @return mixed|string
*
* @throws \InvalidArgumentException If the body property is not array.
*/
public function getJSON()
{
$body = $this->body;

if ($this->bodyFormat !== 'json')
{
/**
* @var Format $config
*/
$config = config(Format::class);
$formatter = $config->getFormatter('application/json');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file, in the line below, $formatter->format() is typed to expect an array.

CodeIgniter4/system/Format/FormatterInterface.php

Line 49 in cd8eba1

public function format(array $data);

Removing the type hint in the FormatterInterface allow more flexibility to work with objects, like pass an \CodeIgniter\Entity directly.

The conversion could be done inside the method, if necessary:

CodeIgniter4/system/Format/XMLFormatter.php

Line 61 in cd8eba1

$this->arrayToXML($data, $output);

JSONFormatter does not needs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea, and I'm ok with that. You could easily check if it's an Entity and call it's toArray() method to get the values.

Expand All @@ -449,7 +446,7 @@ public function getJSON()
/**
* Converts $body into XML, and sets the correct Content-Type.
*
* @param $body
* @param array|string $body
*
* @return $this
*/
Expand All @@ -466,13 +463,17 @@ public function setXML($body)
* Retrieves the current body into XML and returns it.
*
* @return mixed|string
* @throws \InvalidArgumentException If the body property is not array.
*/
public function getXML()
{
$body = $this->body;

if ($this->bodyFormat !== 'xml')
{
/**
* @var Format $config
*/
$config = config(Format::class);
$formatter = $config->getFormatter('application/xml');

Expand All @@ -488,10 +489,11 @@ public function getXML()
* Handles conversion of the of the data into the appropriate format,
* and sets the correct Content-Type header for our response.
*
* @param $body
* @param string $format Valid: json, xml
* @param string|array $body
* @param string $format Valid: json, xml
*
* @return mixed
* @throws \InvalidArgumentException If the body property is not string or array.
*/
protected function formatBody($body, string $format)
{
Expand All @@ -502,6 +504,9 @@ protected function formatBody($body, string $format)
// Nothing much to do for a string...
if (! is_string($body))
{
/**
* @var Format $config
*/
$config = config(Format::class);
$formatter = $config->getFormatter($mime);

Expand Down Expand Up @@ -718,7 +723,7 @@ public function getBody()
* @param integer $code The type of redirection, defaults to 302
*
* @return $this
* @throws \CodeIgniter\HTTP\RedirectException
* @throws \CodeIgniter\HTTP\Exceptions\HTTPException For invalid status code.
*/
public function redirect(string $uri, string $method = 'auto', int $code = null)
{
Expand Down Expand Up @@ -775,6 +780,8 @@ public function redirect(string $uri, string $method = 'auto', int $code = null)
* @param string $prefix Cookie name prefix
* @param boolean|false $secure Whether to only transfer cookies via SSL
* @param boolean|false $httponly Whether only make the cookie accessible via HTTP (no javascript)
*
* @return $this
*/
public function setCookie(
$name,
Expand Down Expand Up @@ -851,9 +858,9 @@ public function setCookie(
/**
* Checks to see if the Response has a specified cookie or not.
*
* @param string $name
* @param null $value
* @param string $prefix
* @param string $name
* @param string|null $value
* @param string $prefix
*
* @return boolean
*/
Expand Down Expand Up @@ -887,8 +894,8 @@ public function hasCookie(string $name, $value = null, string $prefix = '')
/**
* Returns the cookie
*
* @param string $name
* @param string $prefix
* @param string|null $name
* @param string $prefix
*
* @return mixed
*/
Expand Down Expand Up @@ -924,12 +931,14 @@ public function getCookie(string $name = null, string $prefix = '')
* @param string $domain
* @param string $path
* @param string $prefix
*
* @return $this
*/
public function deleteCookie($name = '', string $domain = '', string $path = '/', string $prefix = '')
{
if (empty($name))
{
return;
return $this;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow fluent method continues to work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the $name is empty. Should not delete all? - removing the default ''

}

if ($prefix === '' && $this->cookiePrefix !== '')
Expand Down Expand Up @@ -989,6 +998,8 @@ protected function sendCookies()
* @param string $filename The path to the file to send
* @param string $data The data to be downloaded
* @param boolean $setMime Whether to try and send the actual MIME type
*
* @return \CodeIgniter\HTTP\DownloadResponse|null
*/
public function download(string $filename = '', $data = '', bool $setMime = false)
{
Expand Down