Repository: huge-success/sanic
Pull request #1379 (2018-10-24 12:11:30)

Created methods to append and finish body content on Request
Replaces #1310 

## original description

Hi! I just created two methods to append and finish the body content "feed" on request.py so the underlying body instance can have certain flexibility (example: one could use `BytesIO` instead of a `list` for storing the body data if a custom Request-like class is provided to the `Sanic` constructor).

I also modified server.py to reflect the changes on the `Request` class.

If any documentation is required regarding this, please let me know. Also, I'm not sure if this change is the best approach to the `Request.body` attribute so I'm open to suggestions :wink:

codecov-io: # [Codecov](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=h1) Report
> Merging [#1379](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=desc) into [master](https://codecov.io/gh/huge-success/sanic/commit/905c51bef03818c629173938a1183b38dd0ebc65?src=pr&el=desc) will **increase** coverage by `0.04%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/huge-success/sanic/pull/1379/graphs/tree.svg?width=650&token=NqLbJHUqoR&height=150&src=pr)](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1379      +/-   ##
==========================================
+ Coverage    81.5%   81.54%   +0.04%     
==========================================
  Files          17       17              
  Lines        1692     1696       +4     
  Branches      321      321              
==========================================
+ Hits         1379     1383       +4     
  Misses        246      246              
  Partials       67       67
```


| [Impacted Files](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sanic/app.py](https://codecov.io/gh/huge-success/sanic/pull/1379/diff?src=pr&el=tree#diff-c2FuaWMvYXBwLnB5) | `80.88% <ø> (ø)` | :arrow_up: |
| [sanic/cookies.py](https://codecov.io/gh/huge-success/sanic/pull/1379/diff?src=pr&el=tree#diff-c2FuaWMvY29va2llcy5weQ==) | `82.6% <ø> (ø)` | :arrow_up: |
| [sanic/request.py](https://codecov.io/gh/huge-success/sanic/pull/1379/diff?src=pr&el=tree#diff-c2FuaWMvcmVxdWVzdC5weQ==) | `76.16% <100%> (+0.5%)` | :arrow_up: |
| [sanic/testing.py](https://codecov.io/gh/huge-success/sanic/pull/1379/diff?src=pr&el=tree#diff-c2FuaWMvdGVzdGluZy5weQ==) | `94.8% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=footer). Last update [905c51b...898ed7c](https://codecov.io/gh/huge-success/sanic/pull/1379?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

vltr: @sjsadowski would you mind passing over the Project and Milestone from PR #1310 to this one? I really appreciate it, sorry for the mess I made.
vltr: Yeap, I had this idea previously.

I'm 900km away from my workstation atm, but I think I'll tackle this with
one idea or another in the docs and also in tests, probably next weekend.

Em dom, 28 de out de 2018 04:26, Adam Hopkins <notifications@github.com>
escreveu:

> *@ahopkins* commented on this pull request.
> ------------------------------
>
> In sanic/request.py
> <https://github.com/huge-success/sanic/pull/1379#discussion_r228737204>:
>
> > @@ -106,6 +106,12 @@ def __bool__(self):
>              return True
>          return False
>
> +    def body_append(self, data):
>
> I really like this approach because of the flexibility it provides. But, I
> am not keen on the naming of this method. The word *append* is naturally
> tied to *lists* in Python. If the goal is to allow for any data
> structure, I would suggest making a change to abstract the initialization
> of the body, and provide a more generic term for body_append.
>
> def __init__(self, ...):
>     self.body_init()
>
> def body_init(self):
>     self.body = []
>
> def body_push(self, data):
>     self.body.append(data)
>
>     def body_finish(self):
>         self.body = b"".join(self.body)
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/huge-success/sanic/pull/1379#pullrequestreview-169081109>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABeAsyZ7HHeowrcEA_9_hSVPL9loNTfPks5upVwKgaJpZM4X3xrH>
> .
>

sjsadowski: I'm holding off on this until @vltr returns and updates the method name.

Patch:
diff --git a/docs/sanic/logging.md b/docs/sanic/logging.md
index 49805d0e..e1231eb9 100644
--- a/docs/sanic/logging.md
+++ b/docs/sanic/logging.md
@@ -9,17 +9,32 @@ A simple example using default settings would be like this:
 
 ```python
 from sanic import Sanic
+from sanic.log import logger
+from sanic.response import text
 
 app = Sanic('test')
 
 @app.route('/')
 async def test(request):
-    return response.text('Hello World!')
+    logger.info('Here is your log')
+    return text('Hello World!')
 
 if __name__ == "__main__":
   app.run(debug=True, access_log=True)
 ```
 
+After the server is running, you can see some messages looks like:
+```
+[2018-11-06 21:16:53 +0800] [24622] [INFO] Goin' Fast @ http://127.0.0.1:8000
+[2018-11-06 21:16:53 +0800] [24667] [INFO] Starting worker [24667]
+```
+
+You can send a request to server and it will print the log messages:
+```
+[2018-11-06 21:18:53 +0800] [25685] [INFO] Here is your log
+[2018-11-06 21:18:53 +0800] - (sanic.access)[INFO][127.0.0.1:57038]: GET http://localhost:8000/  200 12
+```
+
 To use your own logging config, simply use `logging.config.dictConfig`, or
 pass `log_config` when you initialize `Sanic` app:
 
@@ -49,7 +64,7 @@ By default, log_config parameter is set to use sanic.log.LOGGING_CONFIG_DEFAULTS
 
 There are three `loggers` used in sanic, and **must be defined if you want to create your own logging configuration**:
 
-- root:<br>
+- sanic.root:<br>
   Used to log internal messages.
 
 - sanic.error:<br>
diff --git a/sanic/request.py b/sanic/request.py
index 013a27ea..e775596a 100644
--- a/sanic/request.py
+++ b/sanic/request.py
@@ -85,7 +85,7 @@ class Request(dict):
         self.transport = transport
 
         # Init but do not inhale
-        self.body = []
+        self.body_init()
         self.parsed_json = None
         self.parsed_form = None
         self.parsed_files = None
@@ -106,6 +106,15 @@ class Request(dict):
             return True
         return False
 
+    def body_init(self):
+        self.body = []
+
+    def body_push(self, data):
+        self.body.append(data)
+
+    def body_finish(self):
+        self.body = b"".join(self.body)
+
     @property
     def json(self):
         if self.parsed_json is None:
diff --git a/sanic/server.py b/sanic/server.py
index 0d0bf29c..dc197941 100644
--- a/sanic/server.py
+++ b/sanic/server.py
@@ -300,7 +300,7 @@ class HttpProtocol(asyncio.Protocol):
                 self.request.stream.put(body)
             )
             return
-        self.request.body.append(body)
+        self.request.body_push(body)
 
     def on_message_complete(self):
         # Entire request (headers and whole body) is received.
@@ -313,7 +313,7 @@ class HttpProtocol(asyncio.Protocol):
                 self.request.stream.put(None)
             )
             return
-        self.request.body = b"".join(self.request.body)
+        self.request.body_finish()
         self.execute_request_handler()
 
     def execute_request_handler(self):


Test patch:
diff --git a/tests/test_custom_request.py b/tests/test_custom_request.py
new file mode 100644
index 00000000..d0ae48e7
--- /dev/null
+++ b/tests/test_custom_request.py
@@ -0,0 +1,53 @@
+from io import BytesIO
+
+from sanic import Sanic
+from sanic.request import Request
+from sanic.response import json_dumps, text
+
+
+class CustomRequest(Request):
+    __slots__ = ("body_buffer",)
+
+    def body_init(self):
+        self.body_buffer = BytesIO()
+
+    def body_push(self, data):
+        self.body_buffer.write(data)
+
+    def body_finish(self):
+        self.body = self.body_buffer.getvalue()
+        self.body_buffer.close()
+
+
+def test_custom_request():
+    app = Sanic(request_class=CustomRequest)
+
+    @app.route("/post", methods=["POST"])
+    async def post_handler(request):
+        return text("OK")
+
+    @app.route("/get")
+    async def get_handler(request):
+        return text("OK")
+
+    payload = {"test": "OK"}
+    headers = {"content-type": "application/json"}
+
+    request, response = app.test_client.post(
+        "/post", data=json_dumps(payload), headers=headers
+    )
+
+    assert isinstance(request.body_buffer, BytesIO)
+    assert request.body_buffer.closed
+    assert request.body == b'{"test":"OK"}'
+    assert request.json.get("test") == "OK"
+    assert response.text == "OK"
+    assert response.status == 200
+
+    request, response = app.test_client.get("/get")
+
+    assert isinstance(request.body_buffer, BytesIO)
+    assert request.body_buffer.closed
+    assert request.body == b""
+    assert response.text == "OK"
+    assert response.status == 200
diff --git a/tests/test_logging.py b/tests/test_logging.py
index 95c55de0..d2b43c1a 100644
--- a/tests/test_logging.py
+++ b/tests/test_logging.py
@@ -11,6 +11,7 @@ import sanic
 from sanic.response import text
 from sanic.log import LOGGING_CONFIG_DEFAULTS
 from sanic import Sanic
+from sanic.log import logger
 
 
 logging_format = '''module: %(module)s; \
@@ -46,7 +47,7 @@ def test_log(app):
 
 
 def test_logging_defaults():
-    reset_logging()
+    # reset_logging()
     app = Sanic("test_logging")
 
     for fmt in [h.formatter for h in logging.getLogger('sanic.root').handlers]:
@@ -60,7 +61,7 @@ def test_logging_defaults():
 
 
 def test_logging_pass_customer_logconfig():
-    reset_logging()
+    # reset_logging()
 
     modified_config = LOGGING_CONFIG_DEFAULTS
     modified_config['formatters']['generic']['format'] = '%(asctime)s - (%(name)s)[%(levelname)s]: %(message)s'
@@ -104,6 +105,25 @@ def test_log_connection_lost(app, debug, monkeypatch):
         assert 'Connection lost before response written @' not in log
 
 
+def test_logger(caplog):
+    rand_string = str(uuid.uuid4())
+
+    app = Sanic()
+
+    @app.get('/')
+    def log_info(request):
+        logger.info(rand_string)
+        return text('hello')
+
+    with caplog.at_level(logging.INFO):
+        request, response = app.test_client.get('/')
+
+    assert caplog.record_tuples[0] == ('sanic.root', logging.INFO, 'Goin\' Fast @ http://127.0.0.1:42101')
+    assert caplog.record_tuples[1] == ('sanic.root', logging.INFO, 'http://127.0.0.1:42101/')
+    assert caplog.record_tuples[2] == ('sanic.root', logging.INFO, rand_string)
+    assert caplog.record_tuples[-1] == ('sanic.root', logging.INFO, 'Server Stopped')
+
+
 def test_logging_modified_root_logger_config():
     reset_logging()
 
@@ -113,4 +133,3 @@ def test_logging_modified_root_logger_config():
     app = Sanic("test_logging", log_config=modified_config)
 
     assert logging.getLogger('sanic.root').getEffectiveLevel() == logging.DEBUG
-