Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sending MultiPart in http client Fix #1005 #1876

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

WebFreak001
Copy link
Contributor

this is also testing the server implementation which made me find #1875

Copy link
Member

@s-ludwig s-ludwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While reviewing the changes, I came to the conclusion that this needs a deeper API change to work efficiently with the non-class streams of vibe-core. What I would propose is to drop the MultiPart(BodyPart)' class(es) completely and change the signature of writeParttovoid writePart(InputStream)(InputStream content, in ref InetHeaderMap headers = emptyHeaderMap) if (isInputStream!InputStream)`. That would avoid the need for a proxy that does dynamic dispatch and destroys all inlining possibilities.

Of course there can still be the convenience overloads that take other kinds of types for content.

http/vibe/http/common.d Outdated Show resolved Hide resolved
http/vibe/http/common.d Outdated Show resolved Hide resolved
http/vibe/http/common.d Outdated Show resolved Hide resolved
http/vibe/http/common.d Outdated Show resolved Hide resolved
http/vibe/http/common.d Outdated Show resolved Hide resolved
http/vibe/http/common.d Show resolved Hide resolved
http/vibe/http/common.d Outdated Show resolved Hide resolved
@WebFreak001
Copy link
Contributor Author

@s-ludwig ping

@WebFreak001
Copy link
Contributor Author

@s-ludwig pong

@WebFreak001
Copy link
Contributor Author

@s-ludwig bump... I need this yet again...

@WebFreak001

This comment has been minimized.

@WebFreak001
Copy link
Contributor Author

ping

@vitalka200
Copy link
Contributor

Any plans on merging this? @s-ludwig

@WebFreak001
Copy link
Contributor Author

WebFreak001 commented Apr 18, 2020

bump @s-ludwig

  • improved API
  • documented everything!
  • moved into vibe.inet (because this is a MIME RFC which also applies to mails)
  • no longer using my emplace hack which probably broke everything before just because I was too lazy to understand the new vibe-core code, but now instead using proper code
  • no internal proxy reading hacks anymore, instead stores the length along with MultiPart
  • fixed unittest to actually run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants