Skip to content

Commit e85f185

Browse files
authored
Merge pull request #289 from clue-labs/container-getobject
Refactor and simplify `App` with new `Container::getObject()` helper
2 parents e0ca633 + 620de5a commit e85f185

File tree

4 files changed

+126
-135
lines changed

4 files changed

+126
-135
lines changed

src/App.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,8 @@ public function __construct(...$middleware)
5353
$needsErrorHandlerNext = false;
5454
foreach ($middleware as $handler) {
5555
// load AccessLogHandler and ErrorHandler instance from last Container
56-
if ($handler === AccessLogHandler::class) {
57-
$handler = $container->getAccessLogHandler();
58-
} elseif ($handler === ErrorHandler::class) {
59-
$handler = $container->getErrorHandler();
56+
if ($handler === AccessLogHandler::class || $handler === ErrorHandler::class) {
57+
$handler = $container->getObject($handler);
6058
}
6159

6260
// ensure AccessLogHandler is always followed by ErrorHandler
@@ -99,12 +97,12 @@ public function __construct(...$middleware)
9997

10098
// add default ErrorHandler as first handler unless it is already added explicitly
10199
if ($needsErrorHandler instanceof Container) {
102-
\array_unshift($handlers, $needsErrorHandler->getErrorHandler());
100+
\array_unshift($handlers, $needsErrorHandler->getObject(ErrorHandler::class));
103101
}
104102

105103
// only log for built-in webserver and PHP development webserver by default, others have their own access log
106104
if ($needsAccessLog instanceof Container) {
107-
$handler = $needsAccessLog->getAccessLogHandler();
105+
$handler = $needsAccessLog->getObject(AccessLogHandler::class);
108106
if (!$handler->isDevNull()) {
109107
\array_unshift($handlers, $handler);
110108
}

src/Container.php

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -135,45 +135,39 @@ public function getEnv(string $name): ?string
135135
}
136136

137137
/**
138+
* [Internal] Get an object of given class from container
139+
*
140+
* @template T of object
141+
* @param class-string<T> $class
142+
* @return object returns an instance of given $class or throws if it can not be instantiated
143+
* @phpstan-return T
138144
* @throws \TypeError if container config or factory returns an unexpected type
145+
* @throws \Error if object of type $class can not be loaded
139146
* @throws \Throwable if container factory function throws unexpected exception
140147
* @internal
141148
*/
142-
public function getAccessLogHandler(): AccessLogHandler
149+
public function getObject(string $class) /*: object (PHP 7.2+) */
143150
{
144-
if ($this->container instanceof ContainerInterface) {
145-
if ($this->container->has(AccessLogHandler::class)) {
146-
// @phpstan-ignore-next-line method return type will ensure correct type or throw `TypeError`
147-
return $this->container->get(AccessLogHandler::class);
148-
} else {
149-
return new AccessLogHandler();
151+
if ($this->container instanceof ContainerInterface && $this->container->has($class)) {
152+
$value = $this->container->get($class);
153+
if (!$value instanceof $class) {
154+
throw new \TypeError(
155+
'Return value of ' . \explode("\0", \get_class($this->container))[0] . '::get() for ' . $class . ' must be of type ' . $class . ', ' . $this->gettype($value) . ' returned'
156+
);
150157
}
158+
return $value;
159+
} elseif ($this->container instanceof ContainerInterface) {
160+
return new $class();
151161
}
152-
return $this->loadObject(AccessLogHandler::class);
153-
}
154162

155-
/**
156-
* @throws \TypeError if container config or factory returns an unexpected type
157-
* @throws \Throwable if container factory function throws unexpected exception
158-
* @internal
159-
*/
160-
public function getErrorHandler(): ErrorHandler
161-
{
162-
if ($this->container instanceof ContainerInterface) {
163-
if ($this->container->has(ErrorHandler::class)) {
164-
// @phpstan-ignore-next-line method return type will ensure correct type or throw `TypeError`
165-
return $this->container->get(ErrorHandler::class);
166-
} else {
167-
return new ErrorHandler();
168-
}
169-
}
170-
return $this->loadObject(ErrorHandler::class);
163+
return $this->loadObject($class);
171164
}
172165

173166
/**
174167
* @template T of object
175168
* @param class-string<T> $name
176-
* @return T
169+
* @return object returns an instance of given class $name or throws if it can not be instantiated
170+
* @phpstan-return T
177171
* @throws \TypeError if container config or factory returns an unexpected type
178172
* @throws \Error if object of type $name can not be loaded
179173
* @throws \Throwable if container factory function throws unexpected exception

tests/AppTest.php

Lines changed: 59 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,10 @@ public function testConstructWithContainerAssignsDefaultHandlersAndContainerForR
6868
$errorHandler = new ErrorHandler();
6969

7070
$container = $this->createMock(Container::class);
71-
$container->expects($this->once())->method('getAccessLogHandler')->willReturn($accessLogHandler);
72-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
71+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
72+
[AccessLogHandler::class, $accessLogHandler],
73+
[ErrorHandler::class, $errorHandler],
74+
]);
7375

7476
assert($container instanceof Container);
7577
$app = new App($container);
@@ -105,8 +107,15 @@ public function testConstructWithContainerAndMiddlewareClassNameAssignsCallableF
105107
{
106108
$middleware = function (ServerRequestInterface $request, callable $next) { };
107109

110+
$accessLogHandler = new AccessLogHandler();
111+
$errorHandler = new ErrorHandler();
112+
108113
$container = $this->createMock(Container::class);
109114
$container->expects($this->once())->method('callable')->with('stdClass')->willReturn($middleware);
115+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
116+
[AccessLogHandler::class, $accessLogHandler],
117+
[ErrorHandler::class, $errorHandler],
118+
]);
110119

111120
assert($container instanceof Container);
112121
$app = new App($container, \stdClass::class);
@@ -126,8 +135,8 @@ public function testConstructWithContainerAndMiddlewareClassNameAssignsCallableF
126135
assert(is_array($handlers));
127136

128137
$this->assertCount(4, $handlers);
129-
$this->assertInstanceOf(AccessLogHandler::class, $handlers[0]);
130-
$this->assertInstanceOf(ErrorHandler::class, $handlers[1]);
138+
$this->assertSame($accessLogHandler, $handlers[0]);
139+
$this->assertSame($errorHandler, $handlers[1]);
131140
$this->assertSame($middleware, $handlers[2]);
132141
$this->assertInstanceOf(RouteHandler::class, $handlers[3]);
133142

@@ -217,10 +226,14 @@ public function testConstructWithContainerAndErrorHandlerAssignsErrorHandlerAfte
217226

218227
public function testConstructWithContainerAndErrorHandlerClassAssignsErrorHandlerFromContainerAfterDefaultAccessLogHandler(): void
219228
{
229+
$accessLogHandler = new AccessLogHandler();
220230
$errorHandler = new ErrorHandler();
221231

222232
$container = $this->createMock(Container::class);
223-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
233+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
234+
[AccessLogHandler::class, $accessLogHandler],
235+
[ErrorHandler::class, $errorHandler],
236+
]);
224237

225238
assert($container instanceof Container);
226239
$app = new App($container, ErrorHandler::class);
@@ -240,20 +253,24 @@ public function testConstructWithContainerAndErrorHandlerClassAssignsErrorHandle
240253
assert(is_array($handlers));
241254

242255
$this->assertCount(3, $handlers);
243-
$this->assertInstanceOf(AccessLogHandler::class, $handlers[0]);
256+
$this->assertSame($accessLogHandler, $handlers[0]);
244257
$this->assertSame($errorHandler, $handlers[1]);
245258
$this->assertInstanceOf(RouteHandler::class, $handlers[2]);
246259
}
247260

248261
public function testConstructWithMultipleContainersAndErrorHandlerClassAssignsErrorHandlerFromLastContainerBeforeErrorHandlerAfterDefaultAccessLogHandler(): void
249262
{
263+
$accessLogHandler = new AccessLogHandler();
250264
$errorHandler = new ErrorHandler();
251265

252266
$unused = $this->createMock(Container::class);
253-
$unused->expects($this->never())->method('getErrorHandler');
267+
$unused->expects($this->never())->method('getObject');
254268

255269
$container = $this->createMock(Container::class);
256-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
270+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
271+
[AccessLogHandler::class, $accessLogHandler],
272+
[ErrorHandler::class, $errorHandler],
273+
]);
257274

258275
assert($unused instanceof Container);
259276
assert($container instanceof Container);
@@ -274,21 +291,25 @@ public function testConstructWithMultipleContainersAndErrorHandlerClassAssignsEr
274291
assert(is_array($handlers));
275292

276293
$this->assertCount(3, $handlers);
277-
$this->assertInstanceOf(AccessLogHandler::class, $handlers[0]);
294+
$this->assertSame($accessLogHandler, $handlers[0]);
278295
$this->assertSame($errorHandler, $handlers[1]);
279296
$this->assertInstanceOf(RouteHandler::class, $handlers[2]);
280297
}
281298

282299
public function testConstructWithMultipleContainersAndMiddlewareAssignsErrorHandlerFromLastContainerBeforeMiddlewareAfterDefaultAccessLogHandler(): void
283300
{
284301
$middleware = function (ServerRequestInterface $request, callable $next) { };
302+
$accessLogHandler = new AccessLogHandler();
285303
$errorHandler = new ErrorHandler();
286304

287305
$unused = $this->createMock(Container::class);
288-
$unused->expects($this->never())->method('getErrorHandler');
306+
$unused->expects($this->never())->method('getObject');
289307

290308
$container = $this->createMock(Container::class);
291-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
309+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
310+
[AccessLogHandler::class, $accessLogHandler],
311+
[ErrorHandler::class, $errorHandler],
312+
]);
292313

293314
assert($unused instanceof Container);
294315
assert($container instanceof Container);
@@ -309,7 +330,7 @@ public function testConstructWithMultipleContainersAndMiddlewareAssignsErrorHand
309330
assert(is_array($handlers));
310331

311332
$this->assertCount(4, $handlers);
312-
$this->assertInstanceOf(AccessLogHandler::class, $handlers[0]);
333+
$this->assertSame($accessLogHandler, $handlers[0]);
313334
$this->assertSame($errorHandler, $handlers[1]);
314335
$this->assertSame($middleware, $handlers[2]);
315336
$this->assertInstanceOf(RouteHandler::class, $handlers[3]);
@@ -350,15 +371,19 @@ public function testConstructWithMultipleContainersAndMiddlewareAndErrorHandlerC
350371
$middleware = function (ServerRequestInterface $request, callable $next) { };
351372

352373
$unused = $this->createMock(Container::class);
353-
$unused->expects($this->never())->method('getErrorHandler');
374+
$unused->expects($this->never())->method('getObject');
354375

376+
$accessLogHandler = new AccessLogHandler();
355377
$errorHandler1 = new ErrorHandler();
356378
$container1 = $this->createMock(Container::class);
357-
$container1->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler1);
379+
$container1->expects($this->exactly(2))->method('getObject')->willReturnMap([
380+
[AccessLogHandler::class, $accessLogHandler],
381+
[ErrorHandler::class, $errorHandler1],
382+
]);
358383

359384
$errorHandler2 = new ErrorHandler();
360385
$container2 = $this->createMock(Container::class);
361-
$container2->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler2);
386+
$container2->expects($this->exactly(1))->method('getObject')->with(ErrorHandler::class)->willReturn($errorHandler2);
362387

363388
assert($unused instanceof Container);
364389
assert($container1 instanceof Container);
@@ -380,7 +405,7 @@ public function testConstructWithMultipleContainersAndMiddlewareAndErrorHandlerC
380405
assert(is_array($handlers));
381406

382407
$this->assertCount(5, $handlers);
383-
$this->assertInstanceOf(AccessLogHandler::class, $handlers[0]);
408+
$this->assertSame($accessLogHandler, $handlers[0]);
384409
$this->assertSame($errorHandler1, $handlers[1]);
385410
$this->assertSame($middleware, $handlers[2]);
386411
$this->assertSame($errorHandler2, $handlers[3]);
@@ -473,8 +498,10 @@ public function testConstructWithContainerAndAccessLogHandlerClassAndErrorHandle
473498
$errorHandler = new ErrorHandler();
474499

475500
$container = $this->createMock(Container::class);
476-
$container->expects($this->once())->method('getAccessLogHandler')->willReturn($accessLogHandler);
477-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
501+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
502+
[AccessLogHandler::class, $accessLogHandler],
503+
[ErrorHandler::class, $errorHandler],
504+
]);
478505

479506
assert($container instanceof Container);
480507
$app = new App($container, AccessLogHandler::class, ErrorHandler::class);
@@ -507,8 +534,10 @@ public function testConstructWithContainerAndAccessLogHandlerClassAndErrorHandle
507534
$errorHandler = new ErrorHandler();
508535

509536
$container = $this->createMock(Container::class);
510-
$container->expects($this->once())->method('getAccessLogHandler')->willReturn($accessLogHandler);
511-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
537+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
538+
[AccessLogHandler::class, $accessLogHandler],
539+
[ErrorHandler::class, $errorHandler],
540+
]);
512541

513542
assert($container instanceof Container);
514543
$app = new App($container, AccessLogHandler::class, ErrorHandler::class);
@@ -569,11 +598,13 @@ public function testConstructWithMultipleContainersAndAccessLogHandlerClassAndEr
569598
$errorHandler = new ErrorHandler();
570599

571600
$unused = $this->createMock(Container::class);
572-
$unused->expects($this->never())->method('getErrorHandler');
601+
$unused->expects($this->never())->method('getObject');
573602

574603
$container = $this->createMock(Container::class);
575-
$container->expects($this->once())->method('getAccessLogHandler')->willReturn($accessLogHandler);
576-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
604+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
605+
[AccessLogHandler::class, $accessLogHandler],
606+
[ErrorHandler::class, $errorHandler],
607+
]);
577608

578609
assert($unused instanceof Container);
579610
assert($container instanceof Container);
@@ -608,12 +639,13 @@ public function testConstructWithMultipleContainersAndMiddlewareAssignsDefaultHa
608639
$errorHandler = new ErrorHandler();
609640

610641
$unused = $this->createMock(Container::class);
611-
$unused->expects($this->never())->method('getAccessLogHandler');
612-
$unused->expects($this->never())->method('getErrorHandler');
642+
$unused->expects($this->never())->method('getObject');
613643

614644
$container = $this->createMock(Container::class);
615-
$container->expects($this->once())->method('getAccessLogHandler')->willReturn($accessLogHandler);
616-
$container->expects($this->once())->method('getErrorHandler')->willReturn($errorHandler);
645+
$container->expects($this->exactly(2))->method('getObject')->willReturnMap([
646+
[AccessLogHandler::class, $accessLogHandler],
647+
[ErrorHandler::class, $errorHandler],
648+
]);
617649

618650
assert($unused instanceof Container);
619651
assert($container instanceof Container);

0 commit comments

Comments
 (0)