These are chat archives for ppi/framework

18th
Nov 2015
Paul Dragoonis
@dragoonis
Nov 18 2015 10:33
@KorvinSzanto interesting feedback, thank you!
dannym87
@dannym87
Nov 18 2015 10:34
yes, good to know. thanks.
Paul Dragoonis
@dragoonis
Nov 18 2015 10:35
@KorvinSzanto is moveTo() once, a Sf2 implementation problem ?
Andrew Carter
@AndrewCarterUK
Nov 18 2015 11:41
@KorvinSzanto @dannym87 @dragoonis - Which direction would the bridging take place? Is the default object going to be Http Foundation or PSR-7?
Paul Dragoonis
@dragoonis
Nov 18 2015 11:41
I believe Foundation would incur less of a rewrite, since a few other areas (such as Router Interop layer) are betting on foundation request. but at the developer level, would always advocate using PSR-7 methods.
Having said that. I know the SF Bridge, let's you create foundation object from PSR-7 object, and vice versa, right? If that's the case then we could have PSR-7 object by default and convert when necessary.
Thoughts?
Andrew Carter
@AndrewCarterUK
Nov 18 2015 11:43
In that case @KorvinSzanto - is there any issue bridging a HttpFoundation request to a PSR-7 one?
I'm not too familiar with the layout/design of the framework, you could construct both request objects separately event
dannym87
@dannym87
Nov 18 2015 11:45
and it should be consistent with the response object. makes no sense having a response object psr-7 by default and the request object foundation
Andrew Carter
@AndrewCarterUK
Nov 18 2015 11:46
I would say that it might be useful to think about how you want to provide PSR-7 and http foundation support to the users and work backwards. How will the framework know what controllers/actions/middleware are used and what type of objects they want to use?
Paul Dragoonis
@dragoonis
Nov 18 2015 11:47
@AndrewCarterUK the design is fairly simple, i think :), whenever you ask for $container->get(‘Request’) it hits this RequestFactory - https://github.com/ppi/framework/blob/master/src/ServiceManager/Factory/RequestFactory.php

@AndrewCarterUK I’m going to go PSR-7 fully for all users, and foundation will just be converted internally, within factories of other bits that need it. For example the RouterFactory can do a convert from $container->get(‘Request’) to Foundation.

Thoughts?

dannym87
@dannym87
Nov 18 2015 11:50
yes, that will give you the initial request object. however, doing $request->withAttribute(‘foo’, ‘bar’) for example, will give you a completely new request object causing $container->get(‘Request’) to not have the foo attribute
Paul Dragoonis
@dragoonis
Nov 18 2015 11:54
Isn’t it common expectation to re-persist it back into the container then? $container->set(‘Request’, $request->withAttribute(..))
Andrew Carter
@AndrewCarterUK
Nov 18 2015 11:54
request and response objects shouldnt be in a container, it's not particularly reusable
the container is application scope
it might want to be used in commands and services not in the request scope
dannym87
@dannym87
Nov 18 2015 12:03
that was my thinking. doesnt seem right having to override the request object in the container
Paul Dragoonis
@dragoonis
Nov 18 2015 14:00
Based on all this input, I’m trying to conslidate a development plan for this, but first… @dannym87 where’s the PR for the work you’ve done thus far. i.e: when you were showing me controller examples on making your response stream and returning it from controller
dannym87
@dannym87
Nov 18 2015 14:03
ppi/framework#136
Korvin Szanto
@KorvinSzanto
Nov 18 2015 17:17
@dannym87 / @dragoonis the issue is with psr-7 using streams
PSR-7's UploadedFileInterface uses streams, but has a convenience method for writing to disk
it's UploadedFileInterface::moveTo
but moveTo implies that you're moving a single instance of the uploaded file
and since PSR-7 uses streams, that likely doesn't exist
so the first time anything calls moveTo, the stream is written to disk at a path
the second time, it can't actually move the file because it doesn't know where it is anymore
so it throws a runtime exception
PSR-7 specifically requires it only work once because otherwise we'd be creating orphans
so the issue is from PSR-7 request to HTTP Foundation
when we take a psr-7 request and make it a foundation request, the symfony bridge moves the file on disk so that it can have a real path
so you can only EVER translate a PSR-7 request to a http foundation request one time
and because PSR-7 is immutable, that's kind of a blocker
Korvin Szanto
@KorvinSzanto
Nov 18 2015 17:24
Another reason why you can only run moveTo once is the stream might not be seekable
I think that's probably the REAL reason for it
Now, It's been two weeks since I opened the issue for it and I haven't gotten real feedback
so there might be a way around this
but at this point, I can't think of one
Paul Dragoonis
@dragoonis
Nov 18 2015 17:31
@KorvinSzanto reading this in a second :)
Paul Dragoonis
@dragoonis
Nov 18 2015 17:47

@KorvinSzanto read it - i understand.

In terms of implmentation then, should we update our RequestFactory and ResponseFactory to call

DiactorosFactory::createRequest() and DiactorosFactory::createResponse()

Korvin Szanto
@KorvinSzanto
Nov 18 2015 17:53
nah, even that doesn't work because the UploadedFile on the existing PSR-7 request is already move
moved*
you'd have to recreate an entirely new version of a request based on the http foundation request
Paul Dragoonis
@dragoonis
Nov 18 2015 17:56
What do you recommend then? PPI\App class, we spin up request, routing kicks in, dispatch occurs and we send Request / Response objects to the controller.
typical front controller MVC workflow.
Korvin Szanto
@KorvinSzanto
Nov 18 2015 17:56
@dragoonis I don't know. I'm at that same spot with concrete5's implementation
I started at the top with psr-7 requests and slowly moved the conversion to http foundation down the stack
Paul Dragoonis
@dragoonis
Nov 18 2015 17:57
Interesting.
Korvin Szanto
@KorvinSzanto
Nov 18 2015 17:57
once I got to the spot where it might need to support both for BC, I ran into issue
issues*
Paul Dragoonis
@dragoonis
Nov 18 2015 17:57
You see, our Request object foundation with additional PSR-7 methods.
hybrid object - had this for months now.
@noisebleed added it
Korvin Szanto
@KorvinSzanto
Nov 18 2015 17:58
weir
weird*
how'd you implement stream stuff?
I guess I can look at the code...
oh, this isn't serverrequestinterface
so you have psr-7 support up to the point of retrieving attributes and uploaded files
Paul Dragoonis
@dragoonis
Nov 18 2015 18:01
yep, attrs and such
Korvin Szanto
@KorvinSzanto
Nov 18 2015 18:01
So I think you still have the same problem, because the requests work just fine as long as you don't upload a file
my concrete5 implementation works until you upload a file
Paul Dragoonis
@dragoonis
Nov 18 2015 18:02
This code is in production right now, and we’re uploading files. but accessing symfony part of it :)
$request->files
Korvin Szanto
@KorvinSzanto
Nov 18 2015 18:02
right
if you had the uploadedfileinterface implemented you could probably weasel your way around it
does ServerRequestInterface collide with httpfoundation?
Paul Dragoonis
@dragoonis
Nov 18 2015 18:03
bridge? or non-bridge
Korvin Szanto
@KorvinSzanto
Nov 18 2015 18:04
well I guess my real question is why did you go with implementing RequestInterface and not ServerRequestInterface on https://github.com/ppi/framework/blob/master/src/Http/Request.php
had you done the latter, there might have been a way around it since you could bridge to your own implementation and not to http foundations base request
Paul Dragoonis
@dragoonis
Nov 18 2015 18:06
/cc @noisebleed you around, to answer ?
Korvin Szanto
@KorvinSzanto
Nov 18 2015 18:33
I just wanted to triple double check: https://gist.github.com/KorvinSzanto/3cb099644fb64b8cf68a
try that with a project that has the symfony bridge and diactoros
you get Fatal error: Uncaught exception 'RuntimeException' with message 'Error occurred while moving uploaded file' in ./vendor/zendframework/zend-diactoros/src/UploadedFile.php on line 166
Thinking about it, I guess this /could/ be an issue with diactoros
but I don't think you'd ever want to allow running moveTo twice
Korvin Szanto
@KorvinSzanto
Nov 18 2015 18:40
In my example, if that worked you'd end up with two orphan uploaded files in the tmp
Paul Dragoonis
@dragoonis
Nov 18 2015 19:22
Yes. I’m wondering what approach we should take.
Korvin Szanto
@KorvinSzanto
Nov 18 2015 20:28
Yeah I think that's the million dollar question
I'm interested in any decision you guys make, and since I'm in almost the same boat I'm very willing to help discuss it
so feel free to ping me in here if you want input