Encapsulation / separate of concern

[popy-dev]

In the current implementation, the nodes classes have multiple responsibilities :

• [ ] abstracting the request body
• [ ] serializing the request body into a multipart string
• [ ] transforming said string into a Psr7 stream
• [ ] injecting things into the request

So here's a possible séparation :

• [ ] abtraction value objects
• [ ] encoders that encode fields and files as a stream (probably delegating the stream stuff to stream factories)
• [ ] manager which takes the formdata abstraction an request, encodes it using encoders, updates and returns the request object

While having encoders returning streams may seem counter intuitive, that would allows to send files as non buffered streams, saving memory, and the formdata encoder could even compose a chunked stream with the streams returned by sub encoders

[Lewiscowles1986]

It's a lot to agree to up-front. I'd suggest breaking it down into multiple PR's. I definitely think having stream factories is over-engineering.

[Lewiscowles1986]

please also see https://github.com/Lewiscowles1986/rfc1867/blob/521db9ff01d8c19e4a43bef69068d8bf33a8d9ca/README.md for stated goals.

[popy-dev]

It's a lot to agree to up-front. I'd suggest breaking it down into multiple PR's

I agree, and i would have posted multiple issues if i had any ideas on how to split it. I'll try multiple progressives PR

I definitely think having stream factories is over-engineering

You already use guzzle stream factory :3

It planned to open another issue and pr on this subject (as it is indeed a separate issue)

I've seen the goals, and stream factories are the only solution i see to decouple from guzzle.

[popy-dev]

Wow, "epic" tag :D

[Lewiscowles1986]

Btw I've been through the code now.

• Although having separate serializers might be an idea (I don't like them coupled, but it makes everything less elegant having data structure objects & serializers for this in PHP).
• providing an API to enable direct passing of Attachment and FormInput to the FormBody
• accepting serializers as part of Attachment
• changing FormBody->submit(RequestInterface $request) to more accurately reflect what it does (adds to a PSR-7 compatible RequestInterface) FormBody->addToRequest(RequestInterface $request)

I don't think as a library this is poorly designed or arduous for someone to get their head around.

You already use guzzle stream factory :3

Yes a class that only pulls in other things and cannot be extended already has a coupling to an externally maintained perfectly good, tested stream factory, which complies with PSR-7 StreamInterface (works were done this AM to bind to StreamInterface instead of Stream).

I'm still open to seeing what you come up with, but try to keep it simple, lightweight.

[Lewiscowles1986]

Please see https://github.com/Lewiscowles1986/rfc1867/blob/master/diagram/Another-Class-Relationship-Diagram.png

and

https://raw.githubusercontent.com/Lewiscowles1986/rfc1867/master/diagram/class-relationship.png

[popy-dev]

I don't think as a library this is poorly designed or arduous for someone to get their head around.

Don't get me wrong, i've never said that : the library is easy to understand and use, and will work perfectly in most of use cases.

My only point was to present a different class diagram to better separate class responsibilities (while trying to keep it simple to use), decouple it from guzzle, and remove implicit non-injected dependencies

And, obviously, that is only an idea, only my opinion, and will be only few pr proposals, and in the end, only your choice to merge or not. I don't plan to start a whole community drama to justify a fork project :D

[popy-dev]

Here's a first draft, where I kept as much original code as I could, focusing on providing a class skeletton.

I encountered few problem writing this :

• The Content-Type of the Http Request must contain the boundary, which is known (for now) only by the Envelope
• The nesting logic is way more complicated than I initially thought

That basically means that I may not be able to provide a second small PR to show my ideas, as I'll propably have to rewrite the nesting/disposition logic, the boundary generation AND the encoder chain in the same run. But if you think I'm heading into an interresting direction I should be able to provide this tomorrow.

[popy-dev]

Not so big finally.