Http-Factory #27

danopz · 2017-06-20T21:48:22Z

I just tried to implement the PSR-17 interfaces for Slim-Http addressing #1. I copied some of the existing static methods, such as Request::createFromGlobals(). Maybe we could even call them inside the factories - else not we should remove them.
This is still not done as the classes needs some tests and there still is a todo in the ServerRequestFactory.
I just wanted to leave this here to get a review before I waste additional hours, and to start a discussion wheter we should implement them here or in an additional package.

Write some tests
call static methods or remove them
fix the todo in ServerRequestFactory
...

coveralls · 2017-06-20T21:52:17Z

Coverage decreased (-8.0%) to 90.241% when pulling f1e4d0e on copynpaste:psr17 into 7ad2514 on slimphp:master.

akrabat · 2017-06-27T19:21:28Z

I'm in favour of supporting PSR-17

coveralls · 2017-07-05T18:43:30Z

Coverage decreased (-18.0007%) to 80.211% when pulling ac8ec09 on danopz:psr17 into 960af6e on slimphp:master.

coveralls · 2017-07-05T19:29:24Z

Coverage decreased (-0.7%) to 97.493% when pulling 2062d61 on danopz:psr17 into 960af6e on slimphp:master.

danopz · 2017-07-05T20:07:38Z

I pushed my last commits.

updated factories
moved Uri::createFromGlobals to UriFactory with @internal flag
removed static factories from implementations, where PSR factories exists
added http-interop/http-factory-tests for factory tests
moved some tests to factory tests

Don't know how we should handle UploadedFile::createFromGlobals since the PSR doesn't handle this case we are using right now - maybe we should add a middleware for this?
Also don't know why the test suddenly fails on PHP7+?

Note: I just renamed using a more appropriate name 😉

coveralls · 2017-08-27T00:33:47Z

Coverage decreased (-0.7%) to 97.493% when pulling d7f72d4 on danopz:psr17 into 960af6e on slimphp:master.

coveralls · 2017-10-08T19:47:55Z

Coverage decreased (-0.7%) to 97.493% when pulling 47dd163 on danopz:psr17 into 37e80cc on slimphp:master.

coveralls · 2017-10-08T20:09:43Z

Coverage decreased (-0.4%) to 97.391% when pulling a21eb30 on danopz:psr17 into 4c76c96 on slimphp:master.

danopz · 2017-10-08T20:11:00Z

Rebased to current master / fixed tests

akrabat · 2017-12-23T10:43:53Z

This is great work, but I'm not prepared to put it in until PSR-17 is ratified and published in the psr namespace.

Let's keep this around as a WIP until then.

MarcHagen · 2018-06-19T19:13:27Z

Maybe starting a discussion but, is it a idea to separate the both? So Slim-Http, Slim-HttpFactory ?
Both can have there respected (dev) requirements and then can even be used seperately.
Let me know :)

danopz · 2018-07-30T21:27:23Z

After thinking about this I'm also in favor of creating a dedicated factory repository.
So they get their own dependencies and they will respect PHP versions 5/7 (PSR7 is PHP 5+, PSR17 is PHP 7+).

danopz · 2018-07-31T20:12:44Z

Updated to psr/http-factory 1.0.0.
Tests are failing because of an increased PHP version + #54

danopz · 2018-08-23T17:40:52Z

@akrabat should we create an extra package to keep dependencies clean?

akrabat · 2018-08-24T12:27:25Z

Maybe we make Slim\Http a PHP 7+ library?

akrabat · 2018-09-16T10:35:17Z

This needs rebasing to pick up the changes that make Slim-Http PHP 7+

…p-interop/http-factory-tests#27

…obals as @internal to UriFactory, update http-factory-tests to master, fix cs

…mock

…re strict types, php 7.0

danopz · 2018-09-16T17:46:01Z

Rebased 👍

ghost · 2018-09-22T12:08:25Z

I'm not in favour of splitting this to a separate package. It's all directly related, i.e. you can't use the factories without the messages.

ghost · 2018-09-22T12:14:05Z

Also, can we not make a single class for RequestFactory and ServerRequestFactory implementing both interfaces? It doesn't need to be separate as it's final output is the same class; it makes sense to me to have a single factory for a single class type.

ghost

In general as I've commented, I feel that Factory classes are injectable dependencies, not inline newables. We should follow that method IMO.

ghost · 2018-09-22T12:34:23Z

src/Factory/RequestFactory.php

+    public function createRequest(string $method, $uri): RequestInterface
+    {
+        if (is_string($uri)) {
+            $uri = (new UriFactory())->createUri($uri);


I'm not comfortable with this. A Factory class is an injectable dependency, not a newable class. Should this not be injected via a constructor?

Yeah I agree, so one could inject some other factory and must not live with ours.

ghost · 2018-09-22T12:35:02Z

src/Factory/RequestFactory.php

+            throw new \InvalidArgumentException('URI must either be string or instance of ' . UriInterface::class);
+        }
+
+        $body = (new StreamFactory())->createStream();


Again as with UriFactory.

ghost · 2018-09-22T12:35:56Z

src/Factory/ServerRequestFactory.php

+    public function createServerRequest(string $method, $uri, array $serverParams = []): ServerRequestInterface
+    {
+        if (is_string($uri)) {
+            $uri = (new UriFactory())->createUri($uri);


Again, an injectable dependency IMO.

ghost · 2018-09-22T12:36:04Z

src/Factory/ServerRequestFactory.php

+            throw new \InvalidArgumentException('URI must either be string or instance of ' . UriInterface::class);
+        }
+
+        $body = (new StreamFactory())->createStream();


Again, an injectable dependency IMO.

ghost · 2018-09-22T12:41:03Z

src/Factory/UriFactory.php

+            throw new \InvalidArgumentException('URI cannot be parsed');
+        }
+
+        $scheme = isset($parts['scheme']) ? $parts['scheme'] : '';


If we're using PHP 7, could we do something like:

$scheme = $parts['scheme'] ?? '';

akrabat · 2018-09-22T18:04:18Z

Also, can we not make a single class for RequestFactory and ServerRequestFactory implementing both interfaces? It doesn't need to be separate as it's final output is the same class; it makes sense to me to have a single factory for a single class type.

Should we have a Request and ServerRequest in Slim-Http?

…ct, using null coalescing operator for UriFactory

danopz · 2018-09-22T18:46:56Z

Now we can inject a StreamFactoryInterface and UriFactoryInterface in both RequestFactories defaulting to null and so to our factories.
I'm not sure about merging RequestFactory and ServerRequestFactory.

akrabat · 2018-10-30T17:15:44Z

This needs to be against Slim-Psr7 now.

slimphp/Slim-Psr7#14 tracks it.

danopz force-pushed the psr17 branch from f1e4d0e to 8475b17 Compare June 29, 2017 22:24

danopz changed the title ~~[WIP] Http-Factory~~ Http-Factory Jul 6, 2017

danopz force-pushed the psr17 branch from d7f72d4 to 47dd163 Compare October 8, 2017 19:44

akrabat changed the title ~~Http-Factory~~ Http-Factory (WIP - waiting for PSR-17) Dec 23, 2017

danopz force-pushed the psr17 branch from 856ee0f to f3717dd Compare February 5, 2018 22:03

danopz force-pushed the psr17 branch from f3717dd to c7ab833 Compare July 22, 2018 22:03

danopz changed the title ~~Http-Factory (WIP - waiting for PSR-17)~~ Http-Factory Aug 23, 2018

This was referenced Sep 7, 2018

Update minimum php version to 7.0.0 #59

Merged

Support PSR-17 #57

Closed

akrabat mentioned this pull request Sep 16, 2018

Provide built-in HTTP response factories, support PSR-17 slimphp/Slim#2056

Closed

danopz added 4 commits September 16, 2018 19:04

psr17 proposal - slimphp#1

f949dbc

add http-interop/http-factory-tests - requires phpunit namespaces htt…

419506a

…p-interop/http-factory-tests#27

move factory methods to their factory classes, move Uri::createFromGl…

4c411aa

…obals as @internal to UriFactory, update http-factory-tests to master, fix cs

moved some tests to factory classes, fix return type of Environment::…

a14bb2b

…mock

danopz added 4 commits September 16, 2018 19:35

fixed failing unit test

5374848

rebased with current master

70a4e4c

fix unit test where cookie gets overridden

66dbe0f

update to psr/http-factory 1.0.0, copyright 2018 for new files, decla…

4a898f0

…re strict types, php 7.0

danopz force-pushed the psr17 branch from bff5204 to 4a898f0 Compare September 16, 2018 17:37

fix line length of unit test for phpcs

468defb

ghost suggested changes Sep 22, 2018

View reviewed changes

possibility to inject required factories in request factories constru…

a21eb30

…ct, using null coalescing operator for UriFactory

akrabat mentioned this pull request Oct 30, 2018

Add Http-Factory slimphp/Slim-Psr7#14

Closed

akrabat closed this Oct 30, 2018

danopz added a commit to danopz/Slim-Psr7 that referenced this pull request Nov 1, 2018

psr17 http-factory, port of slimphp/Slim-Http#27 - resolves slimphp#14

d30e6f6

danopz mentioned this pull request Nov 1, 2018

Http Factory PSR-17 slimphp/Slim-Psr7#17

Merged

danopz deleted the psr17 branch November 1, 2018 22:45

akrabat pushed a commit to danopz/Slim-Psr7 that referenced this pull request Nov 2, 2018

psr17 http-factory, port of slimphp/Slim-Http#27 - resolves slimphp#14

8fb8863

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Http-Factory #27

Http-Factory #27

danopz commented Jun 20, 2017 •

edited

Loading

coveralls commented Jun 20, 2017

akrabat commented Jun 27, 2017

coveralls commented Jul 5, 2017

coveralls commented Jul 5, 2017

danopz commented Jul 5, 2017 •

edited

Loading

coveralls commented Aug 27, 2017

coveralls commented Oct 8, 2017

coveralls commented Oct 8, 2017 •

edited

Loading

danopz commented Oct 8, 2017

akrabat commented Dec 23, 2017

MarcHagen commented Jun 19, 2018

danopz commented Jul 30, 2018

danopz commented Jul 31, 2018

danopz commented Aug 23, 2018

akrabat commented Aug 24, 2018

akrabat commented Sep 16, 2018

danopz commented Sep 16, 2018

ghost commented Sep 22, 2018

ghost commented Sep 22, 2018

ghost left a comment

ghost Sep 22, 2018

danopz Sep 22, 2018

ghost Sep 22, 2018

ghost Sep 22, 2018

ghost Sep 22, 2018

ghost Sep 22, 2018

akrabat commented Sep 22, 2018

danopz commented Sep 22, 2018

akrabat commented Oct 30, 2018

Http-Factory #27

Http-Factory #27

Conversation

danopz commented Jun 20, 2017 • edited Loading

coveralls commented Jun 20, 2017

akrabat commented Jun 27, 2017

coveralls commented Jul 5, 2017

coveralls commented Jul 5, 2017

danopz commented Jul 5, 2017 • edited Loading

coveralls commented Aug 27, 2017

coveralls commented Oct 8, 2017

coveralls commented Oct 8, 2017 • edited Loading

danopz commented Oct 8, 2017

akrabat commented Dec 23, 2017

MarcHagen commented Jun 19, 2018

danopz commented Jul 30, 2018

danopz commented Jul 31, 2018

danopz commented Aug 23, 2018

akrabat commented Aug 24, 2018

akrabat commented Sep 16, 2018

danopz commented Sep 16, 2018

ghost commented Sep 22, 2018

ghost commented Sep 22, 2018

ghost left a comment

Choose a reason for hiding this comment

ghost Sep 22, 2018

Choose a reason for hiding this comment

danopz Sep 22, 2018

Choose a reason for hiding this comment

ghost Sep 22, 2018

Choose a reason for hiding this comment

ghost Sep 22, 2018

Choose a reason for hiding this comment

ghost Sep 22, 2018

Choose a reason for hiding this comment

ghost Sep 22, 2018

Choose a reason for hiding this comment

akrabat commented Sep 22, 2018

danopz commented Sep 22, 2018

akrabat commented Oct 30, 2018

danopz commented Jun 20, 2017 •

edited

Loading

danopz commented Jul 5, 2017 •

edited

Loading

coveralls commented Oct 8, 2017 •

edited

Loading