Skip to content

Refactor Format class #3563

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.

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

Merged
merged 6 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
98 changes: 45 additions & 53 deletions app/Config/Format.php
Original file line number Diff line number Diff line change
@@ -1,54 +1,59 @@
<?php namespace Config;
Copy link
Member

Choose a reason for hiding this comment

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

We're currently all over the place on these. Let's see if @lonnieezell wants to make a call on the styling of namespaces before we change them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with this. Currently it's a mix, and we need a definitive style guide for this one.

Copy link
Member

Choose a reason for hiding this comment

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

My personal preference has always been on line 1, but it seems my IDE and lots of other programmers prefer it with a blank space after the opening php tag. I'm fine with either of those options. Not of fan of having the CI docblock above it, though. I think namespace should be the first item in the file.

So do either of you have preferences on first line vs third line?

Copy link
Member

Choose a reason for hiding this comment

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

I've been doing it on first line but I think I picked that up from you ;) I don't use an IDE so I'm not sure how much work it is to fight against one. Are there any standards? Does our CS have options that might indicate a "default"?

Copy link
Member Author

@paulbalandan paulbalandan Aug 31, 2020

Choose a reason for hiding this comment

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

I am following PSR-12.

When the opening <?php tag is on the first line of the file, it MUST be on its own line with no other statements unless it is a file containing markup outside of PHP opening and closing tags.

Copy link
Member

Choose a reason for hiding this comment

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

I'm good with following the PSR on this. I had forgotten 12 got released, actually, so thanks for the reminder.

Copy link
Member

Choose a reason for hiding this comment

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

Oh that's great! Yes we should definitely move towards that then. The consensus is third line for namespace then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Third line for namespace if no file-level doc block. Namespace after doc block if present. This is following PSR12.

I'll take note on this on my upcoming PR on php-cs-fixer fixes.

Copy link
Member

Choose a reason for hiding this comment

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

Bummer - namespace after file docblock is in PSR. Oh well. Can't have it all :)

<?php

namespace Config;

use CodeIgniter\Config\BaseConfig;

class Format extends BaseConfig
{
/*
|--------------------------------------------------------------------------
| Available Response Formats
|--------------------------------------------------------------------------
|
| When you perform content negotiation with the request, these are the
| available formats that your application supports. This is currently
| only used with the API\ResponseTrait. A valid Formatter must exist
| for the specified format.
|
| These formats are only checked when the data passed to the respond()
| method is an array.
|
*/
/**
* --------------------------------------------------------------------------
* Available Response Formats
* --------------------------------------------------------------------------
*
* When you perform content negotiation with the request, these are the
* available formats that your application supports. This is currently
* only used with the API\ResponseTrait. A valid Formatter must exist
* for the specified format.
*
* These formats are only checked when the data passed to the respond()
* method is an array.
*
* @var string[]
*/
public $supportedResponseFormats = [
'application/json',
'application/xml', // machine-readable XML
'text/xml', // human-readable XML
];

/*
|--------------------------------------------------------------------------
| Formatters
|--------------------------------------------------------------------------
|
| Lists the class to use to format responses with of a particular type.
| For each mime type, list the class that should be used. Formatters
| can be retrieved through the getFormatter() method.
|
*/
/**
* --------------------------------------------------------------------------
* Formatters
* --------------------------------------------------------------------------
*
* Lists the class to use to format responses with of a particular type.
* For each mime type, list the class that should be used. Formatters
* can be retrieved through the getFormatter() method.
*
* @var array<string, string>
*/
public $formatters = [
'application/json' => \CodeIgniter\Format\JSONFormatter::class,
'application/xml' => \CodeIgniter\Format\XMLFormatter::class,
'text/xml' => \CodeIgniter\Format\XMLFormatter::class,
'application/json' => 'CodeIgniter\Format\JSONFormatter',
'application/xml' => 'CodeIgniter\Format\XMLFormatter',
'text/xml' => 'CodeIgniter\Format\XMLFormatter',
];

/*
|--------------------------------------------------------------------------
| Formatters Options
|--------------------------------------------------------------------------
|
| Additional Options to adjust default formatters behaviour.
| For each mime type, list the additional options that should be used.
|
*/
/**
* --------------------------------------------------------------------------
* Formatters Options
* --------------------------------------------------------------------------
*
* Additional Options to adjust default formatters behaviour.
* For each mime type, list the additional options that should be used.
*
* @var array<string, int>
*/
public $formatterOptions = [
'application/json' => JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES,
'application/xml' => 0,
Expand All @@ -63,24 +68,11 @@ class Format extends BaseConfig
* @param string $mime
*
* @return \CodeIgniter\Format\FormatterInterface
*
* @deprecated This is an alias of `\CodeIgniter\Format\Format::getFormatter`. Use that instead.
*/
public function getFormatter(string $mime)
{
if (! array_key_exists($mime, $this->formatters))
{
throw new \InvalidArgumentException('No Formatter defined for mime type: ' . $mime);
}

$class = $this->formatters[$mime];

if (! class_exists($class))
{
throw new \BadMethodCallException($class . ' is not a valid Formatter.');
}

return new $class();
return Services::format()->getFormatter($mime);
}

//--------------------------------------------------------------------

}
31 changes: 19 additions & 12 deletions system/API/ResponseTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
namespace CodeIgniter\API;

use CodeIgniter\HTTP\Response;
use Config\Format;
use Config\Services;

/**
* Response trait.
Expand Down Expand Up @@ -102,6 +102,13 @@ trait ResponseTrait
*/
protected $format = 'json';

/**
* Current Formatter instance. This is usually set by ResponseTrait::format
*
* @var \CodeIgniter\Format\FormatterInterface
*/
protected $formatter;

//--------------------------------------------------------------------

/**
Expand All @@ -123,7 +130,8 @@ public function respond($data = null, int $status = null, string $message = '')

// Create the output var here in case of $this->response([]);
$output = null;
} // If data is null but status provided, keep the output empty.
}
// If data is null but status provided, keep the output empty.
elseif ($data === null && is_numeric($status))
{
$output = null;
Expand All @@ -134,8 +142,7 @@ public function respond($data = null, int $status = null, string $message = '')
$output = $this->format($data);
}

return $this->response->setBody($output)
->setStatusCode($status, $message);
return $this->response->setBody($output)->setStatusCode($status, $message);
}

//--------------------------------------------------------------------
Expand All @@ -159,7 +166,7 @@ public function fail($messages, int $status = 400, string $code = null, string $

$response = [
'status' => $status,
'error' => $code === null ? $status : $code,
'error' => $code ?? $status,
'messages' => $messages,
];

Expand Down Expand Up @@ -386,25 +393,25 @@ protected function format($data = null)
return $data;
}

$config = new Format();
$format = "application/$this->format";
$format = Services::format();
$mime = "application/{$this->format}";

// Determine correct response type through content negotiation if not explicitly declared
if (empty($this->format) || ! in_array($this->format, ['json', 'xml']))
if (empty($this->format) || ! in_array($this->format, ['json', 'xml'], true))
{
$format = $this->request->negotiate('media', $config->supportedResponseFormats, false);
$mime = $this->request->negotiate('media', $format->getConfig()->supportedResponseFormats, false);
}

$this->response->setContentType($format);
$this->response->setContentType($mime);

// if we don't have a formatter, make one
if (! isset($this->formatter))
{
// if no formatter, use the default
$this->formatter = $config->getFormatter($format); // @phpstan-ignore-line
$this->formatter = $format->getFormatter($mime);
}

if ($format !== 'application/json')
if ($mime !== 'application/json')
{
// Recursively convert objects into associative arrays
// Conversion not required for JSONFormatter
Expand Down
24 changes: 24 additions & 0 deletions system/Config/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
use CodeIgniter\Encryption\EncrypterInterface;
use CodeIgniter\Encryption\Encryption;
use CodeIgniter\Filters\Filters;
use CodeIgniter\Format\Format;
use CodeIgniter\Honeypot\Honeypot;
use CodeIgniter\HTTP\CLIRequest;
use CodeIgniter\HTTP\CURLRequest;
Expand Down Expand Up @@ -83,6 +84,7 @@
use Config\Email as EmailConfig;
use Config\Encryption as EncryptionConfig;
use Config\Exceptions as ExceptionsConfig;
use Config\Format as FormatConfig;
use Config\Filters as FiltersConfig;
use Config\Honeypot as HoneypotConfig;
use Config\Images;
Expand Down Expand Up @@ -311,6 +313,28 @@ public static function filters(FiltersConfig $config = null, bool $getShared = t

//--------------------------------------------------------------------

/**
* The Format class is a convenient place to create Formatters.
*
* @param \Config\Format|null $config
* @param boolean $getShared
*
* @return \CodeIgniter\Format\Format
*/
public static function format(FormatConfig $config = null, bool $getShared = true)
{
if ($getShared)
{
return static::getSharedInstance('format', $config);
}

$config = $config ?? config('Format');

return new Format($config);
}

//--------------------------------------------------------------------

/**
* The Honeypot provides a secret input on forms that bots should NOT
* fill in, providing an additional safeguard when accepting user input.
Expand Down
86 changes: 82 additions & 4 deletions system/Format/Exceptions/FormatException.php
Original file line number Diff line number Diff line change
@@ -1,22 +1,100 @@
<?php namespace CodeIgniter\Format\Exceptions;
<?php

/**
* CodeIgniter
*
* An open source application development framework for PHP
*
* This content is released under the MIT License (MIT)
*
* Copyright (c) 2014-2019 British Columbia Institute of Technology
* Copyright (c) 2019-2020 CodeIgniter Foundation
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @package CodeIgniter
* @author CodeIgniter Dev Team
* @copyright 2019-2020 CodeIgniter Foundation
* @license https://opensource.org/licenses/MIT MIT License
* @link https://codeigniter.com
* @since Version 4.0.0
* @filesource
*/

namespace CodeIgniter\Format\Exceptions;

use CodeIgniter\Exceptions\ExceptionInterface;
use RuntimeException;

class FormatException extends \RuntimeException implements ExceptionInterface
/**
* FormatException
*/
class FormatException extends RuntimeException implements ExceptionInterface
{
/**
* Thrown when the instantiated class does not exist.
*
* @param string $class
*
* @return FormatException
*/
public static function forInvalidFormatter(string $class)
{
return new static(lang('Format.invalidFormatter', [$class]));
}

/**
* Thrown in JSONFormatter when the json_encode produces
* an error code other than JSON_ERROR_NONE and JSON_ERROR_RECURSION.
*
* @param string $error
*
* @return FormatException
*/
public static function forInvalidJSON(string $error = null)
{
return new static(lang('Format.invalidJSON', [$error]));
}

/**
* This will never be thrown in travis-ci
* Thrown when the supplied MIME type has no
* defined Formatter class.
*
* @param string $mime
*
* @return FormatException
*/
public static function forInvalidMime(string $mime)
{
return new static(lang('Format.invalidMime', [$mime]));
}

/**
* Thrown on XMLFormatter when the `simplexml` extension
* is not installed.
*
* @return FormatException
*
* @codeCoverageIgnore
*/
public static function forMissingExtension()
{
return new static(lang('Format.missingExtension'));
}

}
Loading