Skip to content

Commit 9569e32

Browse files
dborsattodborsatto
authored
Merge pull request #13 from contentful/refactor/logging
Change which logs are created
2 parents f13fb67 + 44da016 commit 9569e32

File tree

6 files changed

+70
-87
lines changed

6 files changed

+70
-87
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,17 @@ This project adheres to [Semantic Versioning](http://semver.org/).
55

66
## [Unreleased](https://github.com/contentful/contentful-core.php/compare/1.2.0...HEAD)
77

8+
### Changed
9+
10+
* The client used to create a log entry with a full message by default. Now it will create one regular entry with either level "INFO" or "ERROR" depending on the status code of the response, and one with level "DEBUG" with full dumps of request, response, and exception.
11+
812
## [1.1.0](https://github.com/contentful/contentful-core.php/tree/1.1.0) (2018-06-06)
913

14+
### Fixed
15+
1016
* `Contentful\Core\Api\Location` used to provide a wrongful serialization of the longitude property. Now it correctly serializes to `lon` instead of `long`.
17+
18+
### Changed
1119
* The `Contentful\Core\Log\Timer` has been deprecated and will be removed in version 2.
1220

1321
## [1.0.0](https://github.com/contentful/contentful-core.php/tree/1.0.0) (2018-04-17)

‎composer.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{
22
"name": "contentful/core",
33
"type": "library",
4+
"description": "Foundation library for Contentful PHP SDKs",
45
"require": {
56
"php": "^5.6|^7.0",
67
"guzzlehttp/guzzle": "^6.3",
@@ -10,7 +11,8 @@
1011
},
1112
"require-dev": {
1213
"phpunit/phpunit": "^5.7",
13-
"symfony/finder": "^3.0|^4.0"
14+
"symfony/finder": "^3.0|^4.0",
15+
"monolog/monolog": "^1.23"
1416
},
1517
"license": "MIT",
1618
"autoload": {

‎src/Api/BaseClient.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ public function setIntegration($name, $version = null)
119119
* * body The request body
120120
* * baseUri A string that can be used to override the default client base URI
121121
*
122+
* @throws \Exception
123+
*
122124
* @return array|null
123125
*/
124126
protected function request($method, $path, array $options = [])
@@ -128,10 +130,20 @@ protected function request($method, $path, array $options = [])
128130
$message = $this->callApi($request);
129131
$this->messages[] = $message;
130132

133+
$logMessage = \sprintf(
134+
'%s %s (%.3Fs)',
135+
$request->getMethod(),
136+
(string) $request->getUri(),
137+
$message->getDuration()
138+
);
139+
140+
// This is a "simple" log, for general purpose
131141
$this->logger->log(
132142
$message->getLogLevel(),
133-
$message->asString()
143+
$logMessage
134144
);
145+
// This is a debug log, with all message details
146+
$this->logger->debug($logMessage, $message->jsonSerialize());
135147

136148
if ($message->getException()) {
137149
throw $message->getException();

‎src/Api/BaseQuery.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,8 @@ public function setMimeTypeGroup($group)
261261
*
262262
* @param string $field
263263
* @param string|array|\DateTimeInterface|Location $value
264-
* @param string|null $operator The operator to use for this condition.
265-
* Default is strict equality.
264+
* @param string|null $operator the operator to use for this condition.
265+
* Default is strict equality
266266
*
267267
* @throws \InvalidArgumentException If $operator is not one of the valid values
268268
*

‎src/Api/Message.php‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111

1212
use Psr\Http\Message\RequestInterface;
1313
use Psr\Http\Message\ResponseInterface;
14+
use function GuzzleHttp\json_decode as guzzle_json_decode;
15+
use function GuzzleHttp\json_encode as guzzle_json_encode;
16+
use function GuzzleHttp\Psr7\parse_request as guzzle_parse_request;
17+
use function GuzzleHttp\Psr7\parse_response as guzzle_parse_response;
18+
use function GuzzleHttp\Psr7\str as guzzle_stringify_message;
1419

1520
/**
1621
* Message class.
@@ -84,7 +89,7 @@ public function __construct(
8489
*/
8590
public static function createFromString($json)
8691
{
87-
$data = \GuzzleHttp\json_decode($json, true);
92+
$data = guzzle_json_decode($json, true);
8893

8994
if (
9095
!isset($data['api']) ||
@@ -101,8 +106,8 @@ public static function createFromString($json)
101106
return new self(
102107
$data['api'],
103108
$data['duration'],
104-
\GuzzleHttp\Psr7\parse_request($data['request']),
105-
$data['response'] ? \GuzzleHttp\Psr7\parse_response($data['response']) : null,
109+
guzzle_parse_request($data['request']),
110+
$data['response'] ? guzzle_parse_response($data['response']) : null,
106111
$data['exception'] ? \unserialize($data['exception']) : null
107112
);
108113
}
@@ -175,8 +180,8 @@ private function asSerializableArray()
175180
return [
176181
'api' => $this->api,
177182
'duration' => $this->duration,
178-
'request' => \GuzzleHttp\Psr7\str($this->request),
179-
'response' => null !== $this->response ? \GuzzleHttp\Psr7\str($this->response) : null,
183+
'request' => guzzle_stringify_message($this->request),
184+
'response' => null !== $this->response ? guzzle_stringify_message($this->response) : null,
180185
'exception' => \serialize($this->exception),
181186
];
182187
}
@@ -206,8 +211,8 @@ public function unserialize($serialized)
206211

207212
$this->api = $data['api'];
208213
$this->duration = $data['duration'];
209-
$this->request = \GuzzleHttp\Psr7\parse_request($data['request']);
210-
$this->response = null !== $data['response'] ? \GuzzleHttp\Psr7\parse_response($data['response']) : null;
214+
$this->request = guzzle_parse_request($data['request']);
215+
$this->response = null !== $data['response'] ? guzzle_parse_response($data['response']) : null;
211216
$this->exception = \unserialize($data['exception']);
212217
}
213218

@@ -218,7 +223,7 @@ public function unserialize($serialized)
218223
*/
219224
public function asString()
220225
{
221-
return \GuzzleHttp\json_encode($this);
226+
return guzzle_json_encode($this);
222227
}
223228

224229
/**

‎tests/Unit/Api/BaseClientTest.php‎

Lines changed: 31 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,18 @@
1111

1212
use Contentful\Core\Api\BaseClient;
1313
use Contentful\Core\Api\Exception;
14-
use Contentful\Core\Api\Message;
1514
use Contentful\Tests\Core\TestCase;
1615
use Contentful\Tests\Core\Unit\Api\Exception\BadRequestException;
1716
use GuzzleHttp\Client as HttpClient;
1817
use GuzzleHttp\Exception\ClientException;
1918
use GuzzleHttp\Handler\CurlHandler;
2019
use GuzzleHttp\HandlerStack;
2120
use GuzzleHttp\Psr7\Response;
21+
use Monolog\Handler\TestHandler;
22+
use Monolog\Logger;
2223
use Psr\Http\Message\RequestInterface;
23-
use Psr\Log\LoggerInterface;
24+
use function GuzzleHttp\Psr7\parse_request as guzzle_parse_request;
25+
use function GuzzleHttp\Psr7\parse_response as guzzle_parse_response;
2426

2527
class BaseClientTest extends TestCase
2628
{
@@ -29,11 +31,6 @@ class BaseClientTest extends TestCase
2931
*/
3032
private $requestHandler;
3133

32-
public function setUp()
33-
{
34-
ConcreteLogger::reset();
35-
}
36-
3734
public function createHttpClient()
3835
{
3936
$stack = new HandlerStack();
@@ -51,8 +48,10 @@ public function createHttpClient()
5148

5249
public function testClient()
5350
{
51+
$handler = new TestHandler();
52+
$logger = new Logger('test', [$handler]);
5453
$httpClient = $this->createHttpClient();
55-
$client = new ConcreteClient('b4c0n73n7fu1', 'https://cdn.contentful.com/', new ConcreteLogger(), $httpClient);
54+
$client = new ConcreteClient('b4c0n73n7fu1', 'https://cdn.contentful.com/', $logger, $httpClient);
5655
$client->setApplication('sdk-test-application', '1.0');
5756

5857
$this->assertSame('DELIVERY', $client->getApi());
@@ -61,16 +60,33 @@ public function testClient()
6160
$jsonResponse = $client->request('GET', '/spaces/cfexampleapi');
6261

6362
$this->assertSame('cfexampleapi', $jsonResponse['sys']['id']);
64-
$logs = $client->getLogger()->getLogs('INFO');
65-
$this->assertCount(1, $logs);
63+
$logs = $handler->getRecords();
64+
$this->assertCount(2, $logs);
6665

67-
$message = Message::createFromString($logs[0]);
68-
$this->assertSame(200, $message->getResponse()->getStatusCode());
66+
$this->assertSame('INFO', $logs[0]['level_name']);
67+
$this->assertRegExp('/GET https\:\/\/cdn\.contentful\.com\/spaces\/cfexampleapi \(([0-9]{1,})\.([0-9]{3})s\)/', $logs[0]['message']);
68+
69+
$this->assertSame('DEBUG', $logs[1]['level_name']);
70+
$context = $logs[1]['context'];
71+
$this->assertSame('DELIVERY', $context['api']);
72+
$this->assertInternalType('float', $context['duration']);
73+
$this->assertNull(\unserialize($context['exception']));
74+
75+
try {
76+
$request = guzzle_parse_request($context['request']);
77+
if ($context['response']) {
78+
$response = guzzle_parse_response($context['response']);
79+
$this->assertSame(200, $response->getStatusCode());
80+
}
81+
} catch (\Exception $exception) {
82+
$this->fail('Creating request and response from strings failed');
83+
84+
return;
85+
}
6986

7087
// String representations of HTTP messages have no real way of storing the HTTPS vs HTTP
7188
// information. Because of this, after serialization the protocol is defaulted to HTTP.
7289
// To get the original request, use a Message object retrieved from BaseClient::getMessages().
73-
$request = $message->getRequest();
7490
$this->assertSame('http://cdn.contentful.com/spaces/cfexampleapi', (string) $request->getUri());
7591
$this->assertSame('Bearer b4c0n73n7fu1', $request->getHeaderLine('Authorization'));
7692
$this->assertRegExp(
@@ -88,7 +104,7 @@ public function testClient()
88104
public function testErrorPage()
89105
{
90106
$httpClient = $this->createHttpClient();
91-
$client = new ConcreteClient('b4c0n73n7fu1', 'https://cdn.contentful.com', new ConcreteLogger(), $httpClient);
107+
$client = new ConcreteClient('b4c0n73n7fu1', 'https://cdn.contentful.com', null, $httpClient);
92108

93109
$this->requestHandler = function (RequestInterface $request, array $options) {
94110
$response = new Response(404, [], $this->getFixtureContent('not_found.json'));
@@ -102,7 +118,7 @@ public function testErrorPage()
102118
public function testCustomException()
103119
{
104120
$httpClient = $this->createHttpClient();
105-
$client = new CustomExceptionConcreteClient('b4c0n73n7fu1', 'https://api.contentful.com', new ConcreteLogger(), $httpClient);
121+
$client = new CustomExceptionConcreteClient('b4c0n73n7fu1', 'https://api.contentful.com', null, $httpClient);
106122
$client->setIntegration('sdk-test-integration', '1.0.0-beta');
107123

108124
$this->assertSame('MANAGEMENT', $client->getApi());
@@ -214,63 +230,3 @@ protected function getExceptionNamespace()
214230
return __NAMESPACE__.'\\Exception';
215231
}
216232
}
217-
218-
class ConcreteLogger implements LoggerInterface
219-
{
220-
private static $logs = [
221-
'ERROR' => [],
222-
'INFO' => [],
223-
];
224-
225-
public static function reset()
226-
{
227-
self::$logs = [
228-
'ERROR' => [],
229-
'INFO' => [],
230-
];
231-
}
232-
233-
public function getLogs($level)
234-
{
235-
return self::$logs[$level];
236-
}
237-
238-
public function emergency($message, array $context = [])
239-
{
240-
}
241-
242-
public function alert($message, array $context = [])
243-
{
244-
}
245-
246-
public function critical($message, array $context = [])
247-
{
248-
}
249-
250-
public function error($message, array $context = [])
251-
{
252-
$this->log('ERROR', $message, $context);
253-
}
254-
255-
public function warning($message, array $context = [])
256-
{
257-
}
258-
259-
public function notice($message, array $context = [])
260-
{
261-
}
262-
263-
public function info($message, array $context = [])
264-
{
265-
$this->log('INFO', $message, $context);
266-
}
267-
268-
public function debug($message, array $context = [])
269-
{
270-
}
271-
272-
public function log($level, $message, array $context = [])
273-
{
274-
self::$logs[$level][] = $message;
275-
}
276-
}

0 commit comments

Comments
 (0)