-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Payload refactoring #1664
Payload refactoring #1664
Changes from all commits
d69838a
22462a4
ca1e650
7e9a381
0e765a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,3 +129,20 @@ def update_cookies(self, cookies, response_url=None): | |
@abstractmethod | ||
def filter_cookies(self, request_url): | ||
"""Return the jar's cookies filtered by their attributes.""" | ||
|
||
|
||
class AbstractPayloadWriter(ABC): | ||
|
||
@abstractmethod | ||
def write(self, chunk): | ||
"""Write chunk into stream""" | ||
|
||
@asyncio.coroutine # pragma: no branch | ||
@abstractmethod | ||
def write_eof(self, chunk=b''): | ||
"""Write last chunk""" | ||
|
||
@asyncio.coroutine # pragma: no branch | ||
@asyncio.coroutine | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Twice coroutite? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
def drain(self): | ||
"""Flush the write buffer.""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
import asyncio | ||
import io | ||
import json | ||
import mimetypes | ||
import os | ||
import sys | ||
import traceback | ||
import warnings | ||
|
@@ -13,11 +11,11 @@ | |
|
||
import aiohttp | ||
|
||
from . import hdrs, helpers, http, streams | ||
from . import hdrs, helpers, http, payload | ||
from .formdata import FormData | ||
from .helpers import PY_35, HeadersMixin, SimpleCookie, _TimeServiceTimeoutNoop | ||
from .http import HttpMessage | ||
from .log import client_logger | ||
from .multipart import MultipartWriter | ||
from .streams import FlowControlStreamReader | ||
|
||
try: | ||
|
@@ -219,86 +217,54 @@ def update_auth(self, auth): | |
|
||
self.headers[hdrs.AUTHORIZATION] = auth.encode() | ||
|
||
def update_body_from_data(self, data, skip_auto_headers): | ||
if not data: | ||
def update_body_from_data(self, body, skip_auto_headers): | ||
if not body: | ||
return | ||
|
||
if isinstance(data, str): | ||
data = data.encode(self.encoding) | ||
|
||
if isinstance(data, (bytes, bytearray)): | ||
self.body = data | ||
if (hdrs.CONTENT_TYPE not in self.headers and | ||
hdrs.CONTENT_TYPE not in skip_auto_headers): | ||
self.headers[hdrs.CONTENT_TYPE] = 'application/octet-stream' | ||
if hdrs.CONTENT_LENGTH not in self.headers and not self.chunked: | ||
self.headers[hdrs.CONTENT_LENGTH] = str(len(self.body)) | ||
|
||
elif isinstance(data, (asyncio.StreamReader, streams.StreamReader, | ||
streams.DataQueue)): | ||
self.body = data | ||
if asyncio.iscoroutine(body): | ||
warnings.warn( | ||
'coroutine as data object is deprecated, ' | ||
'use aiohttp.streamer #1664', | ||
DeprecationWarning, stacklevel=2) | ||
|
||
elif asyncio.iscoroutine(data): | ||
self.body = data | ||
self.body = body | ||
if (hdrs.CONTENT_LENGTH not in self.headers and | ||
self.chunked is None): | ||
self.chunked = True | ||
|
||
elif isinstance(data, io.IOBase): | ||
assert not isinstance(data, io.StringIO), \ | ||
'attempt to send text data instead of binary' | ||
self.body = data | ||
if not self.chunked and isinstance(data, io.BytesIO): | ||
# Not chunking if content-length can be determined | ||
size = len(data.getbuffer()) | ||
self.headers[hdrs.CONTENT_LENGTH] = str(size) | ||
self.chunked = False | ||
elif (not self.chunked and | ||
isinstance(data, (io.BufferedReader, io.BufferedRandom))): | ||
# Not chunking if content-length can be determined | ||
try: | ||
size = os.fstat(data.fileno()).st_size - data.tell() | ||
self.headers[hdrs.CONTENT_LENGTH] = str(size) | ||
self.chunked = False | ||
except OSError: | ||
# data.fileno() is not supported, e.g. | ||
# io.BufferedReader(io.BytesIO(b'data')) | ||
self.chunked = True | ||
else: | ||
self.chunked = True | ||
return | ||
|
||
if hasattr(data, 'mode'): | ||
if data.mode == 'r': | ||
raise ValueError('file {!r} should be open in binary mode' | ||
''.format(data)) | ||
if (hdrs.CONTENT_TYPE not in self.headers and | ||
hdrs.CONTENT_TYPE not in skip_auto_headers and | ||
hasattr(data, 'name')): | ||
mime = mimetypes.guess_type(data.name)[0] | ||
mime = 'application/octet-stream' if mime is None else mime | ||
self.headers[hdrs.CONTENT_TYPE] = mime | ||
|
||
elif isinstance(data, MultipartWriter): | ||
self.body = data.serialize() | ||
self.headers.update(data.headers) | ||
self.chunked = True | ||
# FormData | ||
if isinstance(body, FormData): | ||
body = body(self.encoding) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't be better to handle there any callable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? I dont understand There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean that what if body could be an arbitrary callable object? FormData uses that interface to be processed. We can reuse that way for everything else. Though not sure if this would be helpful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can add this later, right now I do not see value in this. Developer should be able to prepare spendable data before calling client. Case with custom data generation from coroutine already covered with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. Thanks! |
||
|
||
else: | ||
if not isinstance(data, helpers.FormData): | ||
data = helpers.FormData(data) | ||
try: | ||
body = payload.PAYLOAD_REGISTRY.get(body) | ||
except payload.LookupError: | ||
body = FormData(body)(self.encoding) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not to send data as is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. body could be dict or list or whatever. I am fine with changing FormData api There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I was though to just throw an "encode me" error in this case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. first i try to convert There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I should be more clear. I mean that just try to send data as is without implicit transformations and fail if that's not possible (dict, list, custom object that didn't implement required interface). Implicit transformation may play a bad joke. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to be explicit, it may be not clear if it fails on actual write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. |
||
|
||
self.body = data(self.encoding) | ||
self.body = body | ||
|
||
if (hdrs.CONTENT_TYPE not in self.headers and | ||
hdrs.CONTENT_TYPE not in skip_auto_headers): | ||
self.headers[hdrs.CONTENT_TYPE] = data.content_type | ||
# enable chunked encoding if needed | ||
if not self.chunked: | ||
if hdrs.CONTENT_LENGTH not in self.headers: | ||
size = body.size | ||
if size is None: | ||
self.chunked = True | ||
else: | ||
if hdrs.CONTENT_LENGTH not in self.headers: | ||
self.headers[hdrs.CONTENT_LENGTH] = str(size) | ||
|
||
if data.is_multipart: | ||
self.chunked = True | ||
else: | ||
if (hdrs.CONTENT_LENGTH not in self.headers and | ||
not self.chunked): | ||
self.headers[hdrs.CONTENT_LENGTH] = str(len(self.body)) | ||
# set content-type | ||
if (hdrs.CONTENT_TYPE not in self.headers and | ||
hdrs.CONTENT_TYPE not in skip_auto_headers): | ||
self.headers[hdrs.CONTENT_TYPE] = body.content_type | ||
|
||
# copy payload headers | ||
if body.headers: | ||
for (key, value) in body.headers.items(): | ||
if key not in self.headers: | ||
self.headers[key] = value | ||
|
||
def update_transfer_encoding(self): | ||
"""Analyze transfer-encoding header.""" | ||
|
@@ -344,7 +310,10 @@ def write_bytes(self, request, conn): | |
yield from self._continue | ||
|
||
try: | ||
if asyncio.iscoroutine(self.body): | ||
if isinstance(self.body, payload.Payload): | ||
yield from self.body.write(request) | ||
|
||
elif asyncio.iscoroutine(self.body): | ||
exc = None | ||
value = None | ||
stream = self.body | ||
|
@@ -377,29 +346,6 @@ def write_bytes(self, request, conn): | |
raise ValueError( | ||
'Bytes object is expected, got: %s.' % | ||
type(result)) | ||
|
||
elif isinstance(self.body, (asyncio.StreamReader, | ||
streams.StreamReader)): | ||
chunk = yield from self.body.read(streams.DEFAULT_LIMIT) | ||
while chunk: | ||
yield from request.write(chunk, drain=True) | ||
chunk = yield from self.body.read(streams.DEFAULT_LIMIT) | ||
|
||
elif isinstance(self.body, streams.DataQueue): | ||
while True: | ||
try: | ||
chunk = yield from self.body.read() | ||
if not chunk: | ||
break | ||
yield from request.write(chunk) | ||
except streams.EofStream: | ||
break | ||
|
||
elif isinstance(self.body, io.IOBase): | ||
chunk = self.body.read(streams.DEFAULT_LIMIT) | ||
while chunk: | ||
request.write(chunk) | ||
chunk = self.body.read(self.chunked) | ||
else: | ||
if isinstance(self.body, (bytes, bytearray)): | ||
self.body = (self.body,) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
import io | ||
from urllib.parse import urlencode | ||
|
||
from multidict import MultiDict, MultiDictProxy | ||
|
||
from . import hdrs, multipart, payload | ||
from .helpers import guess_filename | ||
|
||
__all__ = ('FormData',) | ||
|
||
|
||
class FormData: | ||
"""Helper class for multipart/form-data and | ||
application/x-www-form-urlencoded body generation.""" | ||
|
||
def __init__(self, fields=(), quote_fields=True): | ||
self._writer = multipart.MultipartWriter('form-data') | ||
self._fields = [] | ||
self._is_multipart = False | ||
self._quote_fields = quote_fields | ||
|
||
if isinstance(fields, dict): | ||
fields = list(fields.items()) | ||
elif not isinstance(fields, (list, tuple)): | ||
fields = (fields,) | ||
self.add_fields(*fields) | ||
|
||
def add_field(self, name, value, *, content_type=None, filename=None, | ||
content_transfer_encoding=None): | ||
|
||
if isinstance(value, io.IOBase): | ||
self._is_multipart = True | ||
elif isinstance(value, (bytes, bytearray, memoryview)): | ||
if filename is None and content_transfer_encoding is None: | ||
filename = name | ||
|
||
type_options = MultiDict({'name': name}) | ||
if filename is not None and not isinstance(filename, str): | ||
raise TypeError('filename must be an instance of str. ' | ||
'Got: %s' % filename) | ||
if filename is None and isinstance(value, io.IOBase): | ||
filename = guess_filename(value, name) | ||
if filename is not None: | ||
type_options['filename'] = filename | ||
self._is_multipart = True | ||
|
||
headers = {} | ||
if content_type is not None: | ||
if not isinstance(content_type, str): | ||
raise TypeError('content_type must be an instance of str. ' | ||
'Got: %s' % content_type) | ||
headers[hdrs.CONTENT_TYPE] = content_type | ||
self._is_multipart = True | ||
if content_transfer_encoding is not None: | ||
if not isinstance(content_transfer_encoding, str): | ||
raise TypeError('content_transfer_encoding must be an instance' | ||
' of str. Got: %s' % content_transfer_encoding) | ||
headers[hdrs.CONTENT_TRANSFER_ENCODING] = content_transfer_encoding | ||
self._is_multipart = True | ||
|
||
self._fields.append((type_options, headers, value)) | ||
|
||
def add_fields(self, *fields): | ||
to_add = list(fields) | ||
|
||
while to_add: | ||
rec = to_add.pop(0) | ||
|
||
if isinstance(rec, io.IOBase): | ||
k = guess_filename(rec, 'unknown') | ||
self.add_field(k, rec) | ||
|
||
elif isinstance(rec, (MultiDictProxy, MultiDict)): | ||
to_add.extend(rec.items()) | ||
|
||
elif isinstance(rec, (list, tuple)) and len(rec) == 2: | ||
k, fp = rec | ||
self.add_field(k, fp) | ||
|
||
else: | ||
raise TypeError('Only io.IOBase, multidict and (name, file) ' | ||
'pairs allowed, use .add_field() for passing ' | ||
'more complex parameters, got {!r}' | ||
.format(rec)) | ||
|
||
def _gen_form_urlencoded(self, encoding): | ||
# form data (x-www-form-urlencoded) | ||
data = [] | ||
for type_options, _, value in self._fields: | ||
data.append((type_options['name'], value)) | ||
|
||
return payload.BytesPayload( | ||
urlencode(data, doseq=True).encode(encoding), | ||
content_type='application/x-www-form-urlencoded') | ||
|
||
def _gen_form_data(self, encoding): | ||
"""Encode a list of fields using the multipart/form-data MIME format""" | ||
for dispparams, headers, value in self._fields: | ||
if hdrs.CONTENT_TYPE in headers: | ||
part = payload.get_payload( | ||
value, content_type=headers[hdrs.CONTENT_TYPE], | ||
headers=headers, encoding=encoding) | ||
else: | ||
part = payload.get_payload( | ||
value, headers=headers, encoding=encoding) | ||
if dispparams: | ||
part.set_content_disposition( | ||
'form-data', quote_fields=self._quote_fields, **dispparams | ||
) | ||
# FIXME cgi.FieldStorage doesn't likes body parts with | ||
# Content-Length which were sent via chunked transfer encoding | ||
part.headers.pop(hdrs.CONTENT_LENGTH, None) | ||
|
||
self._writer.append_payload(part) | ||
|
||
return self._writer | ||
|
||
def __call__(self, encoding): | ||
if self._is_multipart: | ||
return self._gen_form_data(encoding) | ||
else: | ||
return self._gen_form_urlencoded(encoding) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can exclude abstract methods from coverage:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good