From 15f77baf0310b3c5e196922c38b9c47dccd206dd Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Fri, 27 Jan 2023 12:52:40 +1000 Subject: [PATCH 1/6] Changed Response to use body: bytes - Added ActixBytesWrapper to conform to pyclass and MessageBody --- integration_tests/base_routes.py | 26 +++++++-- integration_tests/test_binary_output.py | 17 ++++++ robyn/__init__.py | 16 ++++-- robyn/robyn.pyi | 3 +- robyn/router.py | 19 +++++-- src/types.rs | 71 +++++++++++++++++++++++-- 6 files changed, 135 insertions(+), 17 deletions(-) create mode 100644 integration_tests/test_binary_output.py diff --git a/integration_tests/base_routes.py b/integration_tests/base_routes.py index 48ed13c32..fae42c0bc 100644 --- a/integration_tests/base_routes.py +++ b/integration_tests/base_routes.py @@ -234,7 +234,7 @@ async def redirect_route(request): @app.get("/types/response") def response_type(request): - return Response(status_code=200, headers={}, body="OK") + return Response(status_code=200, headers={}, body=b"OK") @app.get("/types/str") @@ -249,7 +249,7 @@ def int_type(request): @app.get("/async/types/response") async def async_response_type(request): - return Response(status_code=200, headers={}, body="OK") + return Response(status_code=200, headers={}, body=b"OK") @app.get("/async/types/str") @@ -270,12 +270,32 @@ def file_download_sync(): @app.get("/file_download_async") -def file_download_async(): +async def file_download_async(): current_file_path = pathlib.Path(__file__).parent.resolve() file_path = os.path.join(current_file_path, "downloads", "test.txt") return serve_file(file_path) +@app.get("/binary_output_sync") +def binary_output(request): + return b"OK" + # response = Response( + # status_code=200, + # headers={"Content-Type": "application/octet-stream"}, + # body=b"OK", + # ) + + +@app.get("/binary_output_async") +async def binary_output(request): + return b"OK" + # response = Response( + # status_code=200, + # headers={"Content-Type": "application/octet-stream"}, + # body=b"OK", + # ) + + if __name__ == "__main__": app.add_request_header("server", "robyn") current_file_path = pathlib.Path(__file__).parent.resolve() diff --git a/integration_tests/test_binary_output.py b/integration_tests/test_binary_output.py new file mode 100644 index 000000000..e8e02d200 --- /dev/null +++ b/integration_tests/test_binary_output.py @@ -0,0 +1,17 @@ +import requests + +BASE_URL = "http://127.0.0.1:8080" + + +def test_file_download_sync(session): + r = requests.get(f"{BASE_URL}/binary_output_sync") + assert r.status_code == 200 + assert r.headers["Content-Type"] == "application/octet-stream" + assert r.text == "OK" + + +def test_file_download_async(session): + r = requests.get(f"{BASE_URL}/binary_output_async") + assert r.status_code == 200 + assert r.headers["Content-Type"] == "application/octet-stream" + assert r.text == "OK" diff --git a/robyn/__init__.py b/robyn/__init__.py index 351581898..392b1dc3a 100644 --- a/robyn/__init__.py +++ b/robyn/__init__.py @@ -83,7 +83,9 @@ def add_directory( index_file: Optional[str] = None, show_files_listing: bool = False, ): - self.directories.append(Directory(route, directory_path, index_file, show_files_listing)) + self.directories.append( + Directory(route, directory_path, index_file, show_files_listing) + ) def add_request_header(self, key: str, value: str) -> None: self.request_headers.append(Header(key, value)) @@ -115,7 +117,9 @@ def start(self, url: str = "127.0.0.1", port: int = 8080): url = os.getenv("ROBYN_URL", url) port = int(os.getenv("ROBYN_PORT", port)) - logger.info("%sStarting server at %s:%s %s", Colors.OKGREEN, url, port, Colors.ENDC) + logger.info( + "%sStarting server at %s:%s %s", Colors.OKGREEN, url, port, Colors.ENDC + ) def init_processpool(socket): @@ -163,7 +167,9 @@ def init_processpool(socket): process_pool = init_processpool(socket) def terminating_signal_handler(_sig, _frame): - logger.info(f"{Colors.BOLD}{Colors.OKGREEN} Terminating server!! {Colors.ENDC}") + logger.info( + f"{Colors.BOLD}{Colors.OKGREEN} Terminating server!! {Colors.ENDC}" + ) for process in process_pool: process.kill() @@ -176,7 +182,9 @@ def terminating_signal_handler(_sig, _frame): else: event_handler = EventHandler(self.file_path) event_handler.start_server_first_time() - logger.info(f"{Colors.OKBLUE}Dev server initialised with the directory_path : {self.directory_path}{Colors.ENDC}") + logger.info( + f"{Colors.OKBLUE}Dev server initialised with the directory_path : {self.directory_path}{Colors.ENDC}" + ) observer = Observer() observer.schedule(event_handler, path=self.directory_path, recursive=True) observer.start() diff --git a/robyn/robyn.pyi b/robyn/robyn.pyi index e7cd052f2..d721c691b 100644 --- a/robyn/robyn.pyi +++ b/robyn/robyn.pyi @@ -19,12 +19,11 @@ class FunctionInfo: class Response: status_code: int headers: dict[str, str] - body: str + body: bytes def set_file_path(self, file_path: str): pass - class Server: def __init__(self) -> None: pass diff --git a/robyn/router.py b/robyn/router.py index 2b9b11df1..d4ab6d797 100644 --- a/robyn/router.py +++ b/robyn/router.py @@ -28,7 +28,7 @@ def _format_response(self, res): if type(res) == dict: status_code = res.get("status_code", 200) headers = res.get("headers", {"Content-Type": "text/plain"}) - body = res.get("body", "") + body = str(res.get("body", "")).encode("utf-8") if type(status_code) != int: status_code = int(status_code) # status_code can potentially be string @@ -39,12 +39,23 @@ def _format_response(self, res): response.set_file_path(file_path) elif type(res) == Response: response = res + elif type(res) == bytes: + response = Response( + status_code=200, + headers={"Content-Type": "application/octet-stream"}, + body=res, + ) else: - response = Response(status_code=200, headers={"Content-Type": "text/plain"}, body=str(res)) - + response = Response( + status_code=200, + headers={"Content-Type": "text/plain"}, + body=str(res).encode("utf-8"), + ) return response - def add_route(self, route_type: str, endpoint: str, handler: Callable, is_const: bool) -> Union[Callable, CoroutineType]: + def add_route( + self, route_type: str, endpoint: str, handler: Callable, is_const: bool + ) -> Union[Callable, CoroutineType]: @wraps(handler) async def async_inner_handler(*args): response = self._format_response(await handler(*args)) diff --git a/src/types.rs b/src/types.rs index ec6bc07e0..5144163bb 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,13 +1,75 @@ +use core::{mem}; +use std::ops::{Deref, DerefMut}; +use std::{ + convert::Infallible, + task::{Context, Poll}, + pin::Pin, +}; use std::collections::HashMap; use actix_web::web::Bytes; use actix_web::{http::Method, HttpRequest}; +use actix_http::body::MessageBody; +use actix_http::body::BodySize; + use anyhow::Result; use dashmap::DashMap; use pyo3::{exceptions, prelude::*}; +use pyo3::types::PyBytes; use crate::io_helpers::read_file; + +#[derive(Debug, Clone)] +#[pyclass] +pub struct ActixBytesWrapper(Bytes); + +impl ActixBytesWrapper { + pub fn new(bytes: &PyBytes) -> Self { + Self(Bytes::from(bytes.as_bytes().to_vec())) + } +} + +impl Deref for ActixBytesWrapper { + type Target = Bytes; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for ActixBytesWrapper { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl MessageBody for ActixBytesWrapper { + type Error = Infallible; + + #[inline] + fn size(&self) -> BodySize { + BodySize::Sized(self.len() as u64) + } + + #[inline] + fn poll_next( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + ) -> Poll>> { + if self.is_empty() { + Poll::Ready(None) + } else { + Poll::Ready(Some(Ok(mem::take(self.get_mut())))) + } + } + + #[inline] + fn try_into_bytes(self) -> Result { + Ok(self.0) + } +} + #[pyclass] #[derive(Debug, Clone)] pub struct FunctionInfo { @@ -78,20 +140,20 @@ pub struct Response { pub status_code: u16, pub response_type: String, pub headers: HashMap, - pub body: String, + pub body: ActixBytesWrapper, pub file_path: Option, } #[pymethods] impl Response { #[new] - pub fn new(status_code: u16, headers: HashMap, body: String) -> Self { + pub fn new(status_code: u16, headers: HashMap, body: &PyBytes) -> Self { Self { status_code, // we should be handling based on headers but works for now response_type: "text".to_string(), headers, - body, + body: ActixBytesWrapper::new(body), file_path: None, } } @@ -100,10 +162,11 @@ impl Response { // we should be handling based on headers but works for now self.response_type = "static_file".to_string(); self.file_path = Some(file_path.to_string()); - self.body = match read_file(file_path) { + let string = match read_file(file_path) { Ok(b) => b, Err(e) => return Err(exceptions::PyIOError::new_err::(e.to_string())), }; + self.body = ActixBytesWrapper(Bytes::from(string.clone())); Ok(()) } } From 32d328dae50f3f4b0eb4d9222e624e98a236f0f8 Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Mon, 30 Jan 2023 08:33:54 +1000 Subject: [PATCH 2/6] Made suggested improvements to allow both PyString and PyBytes - Reverted unnecessary casting and type changes --- integration_tests/base_routes.py | 33 ++++++++++++++++--------- integration_tests/test_binary_output.py | 18 ++++++++++++-- robyn/router.py | 2 +- src/types.rs | 32 +++++++++++++++++------- 4 files changed, 61 insertions(+), 24 deletions(-) diff --git a/integration_tests/base_routes.py b/integration_tests/base_routes.py index fae42c0bc..775d1c3e8 100644 --- a/integration_tests/base_routes.py +++ b/integration_tests/base_routes.py @@ -234,7 +234,7 @@ async def redirect_route(request): @app.get("/types/response") def response_type(request): - return Response(status_code=200, headers={}, body=b"OK") + return Response(status_code=200, headers={}, body="OK") @app.get("/types/str") @@ -249,7 +249,7 @@ def int_type(request): @app.get("/async/types/response") async def async_response_type(request): - return Response(status_code=200, headers={}, body=b"OK") + return Response(status_code=200, headers={}, body="OK") @app.get("/async/types/str") @@ -279,21 +279,30 @@ async def file_download_async(): @app.get("/binary_output_sync") def binary_output(request): return b"OK" - # response = Response( - # status_code=200, - # headers={"Content-Type": "application/octet-stream"}, - # body=b"OK", - # ) + + +@app.get("/binary_output_response_sync") +def binary_output_response(request): + return Response( + status_code=200, + headers={"Content-Type": "application/octet-stream"}, + body="OK", + ) @app.get("/binary_output_async") async def binary_output(request): return b"OK" - # response = Response( - # status_code=200, - # headers={"Content-Type": "application/octet-stream"}, - # body=b"OK", - # ) + + +@app.get("/binary_output_response_async") +async def binary_output_response(request): + return Response( + status_code=200, + headers={"Content-Type": "application/octet-stream"}, + body="OK", + ) + if __name__ == "__main__": diff --git a/integration_tests/test_binary_output.py b/integration_tests/test_binary_output.py index e8e02d200..54addb334 100644 --- a/integration_tests/test_binary_output.py +++ b/integration_tests/test_binary_output.py @@ -3,15 +3,29 @@ BASE_URL = "http://127.0.0.1:8080" -def test_file_download_sync(session): +def test_binary_output_sync(session): r = requests.get(f"{BASE_URL}/binary_output_sync") assert r.status_code == 200 assert r.headers["Content-Type"] == "application/octet-stream" assert r.text == "OK" -def test_file_download_async(session): +def test_binary_output_response_sync(session): + r = requests.get(f"{BASE_URL}/binary_output_response_sync") + assert r.status_code == 200 + assert r.headers["Content-Type"] == "application/octet-stream" + assert r.text == "OK" + + +def test_binary_output_async(session): r = requests.get(f"{BASE_URL}/binary_output_async") assert r.status_code == 200 assert r.headers["Content-Type"] == "application/octet-stream" assert r.text == "OK" + + +def test_binary_output_response_async(session): + r = requests.get(f"{BASE_URL}/binary_output_response_async") + assert r.status_code == 200 + assert r.headers["Content-Type"] == "application/octet-stream" + assert r.text == "OK" diff --git a/robyn/router.py b/robyn/router.py index d4ab6d797..a2efb18a2 100644 --- a/robyn/router.py +++ b/robyn/router.py @@ -28,7 +28,7 @@ def _format_response(self, res): if type(res) == dict: status_code = res.get("status_code", 200) headers = res.get("headers", {"Content-Type": "text/plain"}) - body = str(res.get("body", "")).encode("utf-8") + body = res.get("body", "") if type(status_code) != int: status_code = int(status_code) # status_code can potentially be string diff --git a/src/types.rs b/src/types.rs index 5144163bb..4303d2eb2 100644 --- a/src/types.rs +++ b/src/types.rs @@ -15,18 +15,32 @@ use actix_http::body::BodySize; use anyhow::Result; use dashmap::DashMap; use pyo3::{exceptions, prelude::*}; -use pyo3::types::PyBytes; +use pyo3::types::{PyBytes, PyString}; +use pyo3::exceptions::PyValueError; use crate::io_helpers::read_file; +fn type_of(_: &T) -> String { + std::any::type_name::().to_string() +} #[derive(Debug, Clone)] #[pyclass] pub struct ActixBytesWrapper(Bytes); +// provides an interface between pyo3::types::{PyString, PyBytes} and actix_web::web::Bytes impl ActixBytesWrapper { - pub fn new(bytes: &PyBytes) -> Self { - Self(Bytes::from(bytes.as_bytes().to_vec())) + pub fn new(raw_body: &PyAny) -> PyResult { + let value = if let Ok(v) = raw_body.downcast::() { + v.to_string().into_bytes() + } else if let Ok(v) = raw_body.downcast::() { + v.as_bytes().to_vec() + } else { + return Err(PyValueError::new_err( + format!("Could not convert {} specified body to bytes", type_of(raw_body)) + )); + }; + Ok(Self(Bytes::from(value))) } } @@ -147,26 +161,26 @@ pub struct Response { #[pymethods] impl Response { #[new] - pub fn new(status_code: u16, headers: HashMap, body: &PyBytes) -> Self { - Self { + pub fn new(status_code: u16, headers: HashMap, body: &PyAny) -> PyResult { + return Ok(Self { status_code, // we should be handling based on headers but works for now response_type: "text".to_string(), headers, - body: ActixBytesWrapper::new(body), + body: ActixBytesWrapper::new(body).unwrap(), file_path: None, - } + }) } pub fn set_file_path(&mut self, file_path: &str) -> PyResult<()> { // we should be handling based on headers but works for now self.response_type = "static_file".to_string(); self.file_path = Some(file_path.to_string()); - let string = match read_file(file_path) { + let response = match read_file(file_path) { Ok(b) => b, Err(e) => return Err(exceptions::PyIOError::new_err::(e.to_string())), }; - self.body = ActixBytesWrapper(Bytes::from(string.clone())); + self.body = ActixBytesWrapper(Bytes::from(response)); Ok(()) } } From da39569f17907288781327daf1251a78bd057d2a Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Mon, 30 Jan 2023 08:34:45 +1000 Subject: [PATCH 3/6] Added test for supplying unsupported type to body - WIP: Issue with async testing --- integration_tests/base_routes.py | 17 +++++++++++++++++ integration_tests/test_unsupported_types.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 integration_tests/test_unsupported_types.py diff --git a/integration_tests/base_routes.py b/integration_tests/base_routes.py index 775d1c3e8..60e6926e4 100644 --- a/integration_tests/base_routes.py +++ b/integration_tests/base_routes.py @@ -304,6 +304,23 @@ async def binary_output_response(request): ) +@app.get("/bad_body_type_error_sync") +def binary_output_response_sync(request): + return Response( + status_code=200, + headers={}, + body=None, + ) + + +# @app.get("/bad_body_type_error_async") +# async def binary_output_response_async(request): +# return Response( +# status_code=200, +# headers={}, +# body=None, +# ) + if __name__ == "__main__": app.add_request_header("server", "robyn") diff --git a/integration_tests/test_unsupported_types.py b/integration_tests/test_unsupported_types.py new file mode 100644 index 000000000..9146dabd5 --- /dev/null +++ b/integration_tests/test_unsupported_types.py @@ -0,0 +1,17 @@ +import pytest +import requests + +BASE_URL = "http://127.0.0.1:8080" + + +from requests.exceptions import ConnectionError + + +def test_bad_body_type_error_sync(session): + with pytest.raises(ConnectionError): + _ = requests.get(f"{BASE_URL}/bad_body_type_error_sync") + + +# def test_bad_body_type_error_async(session): +# with pytest.raises(ConnectionError): +# _ = requests.get(f"{BASE_URL}/bad_body_type_error_async") From 7fb3523587b21831461bccbf87615f75f55e1c35 Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Mon, 30 Jan 2023 20:13:34 +1000 Subject: [PATCH 4/6] Remove unused unsupported type test due to issues with raises --- integration_tests/base_routes.py | 18 ------------------ integration_tests/test_unsupported_types.py | 15 --------------- 2 files changed, 33 deletions(-) delete mode 100644 integration_tests/test_unsupported_types.py diff --git a/integration_tests/base_routes.py b/integration_tests/base_routes.py index eedc4b6f6..e593731bc 100644 --- a/integration_tests/base_routes.py +++ b/integration_tests/base_routes.py @@ -304,24 +304,6 @@ async def binary_output_response_async(request): ) -@app.get("/bad_body_type_error_sync") -def bad_body_type_error_sync(request): - return Response( - status_code=200, - headers={}, - body=None, - ) - - -# @app.get("/bad_body_type_error_async") -# async def binary_output_response_async(request): -# return Response( -# status_code=200, -# headers={}, -# body=None, -# ) - - if __name__ == "__main__": app.add_request_header("server", "robyn") current_file_path = pathlib.Path(__file__).parent.resolve() diff --git a/integration_tests/test_unsupported_types.py b/integration_tests/test_unsupported_types.py deleted file mode 100644 index c776a2827..000000000 --- a/integration_tests/test_unsupported_types.py +++ /dev/null @@ -1,15 +0,0 @@ -import pytest -import requests -from requests.exceptions import ConnectionError - -BASE_URL = "http://127.0.0.1:8080" - - -def test_bad_body_type_error_sync(session): - with pytest.raises(ConnectionError): - _ = requests.get(f"{BASE_URL}/bad_body_type_error_sync") - - -# def test_bad_body_type_error_async(session): -# with pytest.raises(ConnectionError): -# _ = requests.get(f"{BASE_URL}/bad_body_type_error_async") From 76071feeac385dd692b2c38dfd95e0a9aea889cd Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Thu, 2 Feb 2023 11:32:16 +1000 Subject: [PATCH 5/6] Fixed issue where .unwrap causes Panic and instead raises Exception - Added tests for bad and good body types --- integration_tests/test_unsupported_types.py | 41 +++++++++++++++++++++ src/types.rs | 2 +- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 integration_tests/test_unsupported_types.py diff --git a/integration_tests/test_unsupported_types.py b/integration_tests/test_unsupported_types.py new file mode 100644 index 000000000..e39a7750b --- /dev/null +++ b/integration_tests/test_unsupported_types.py @@ -0,0 +1,41 @@ +import pytest + +from robyn.robyn import Response + + +def test_bad_body_types(): + class A: + pass + + bad_bodies = [ + None, + 1, + True, + A, + {"body": "OK"}, + ["OK", b"OK"], + Response( + status_code=200, + headers={}, + body=b"OK", + ), + ] + + for body in bad_bodies: + with pytest.raises(ValueError): + _ = Response( + status_code=200, + headers={}, + body=body, + ) + + +def test_good_body_types(): + good_bodies = ["OK", b"OK"] + + for body in good_bodies: + _ = Response( + status_code=200, + headers={}, + body=body, + ) diff --git a/src/types.rs b/src/types.rs index 4303d2eb2..5aef2027e 100644 --- a/src/types.rs +++ b/src/types.rs @@ -167,7 +167,7 @@ impl Response { // we should be handling based on headers but works for now response_type: "text".to_string(), headers, - body: ActixBytesWrapper::new(body).unwrap(), + body: ActixBytesWrapper::new(body)?, file_path: None, }) } From 4643781809e229c70d4d3cd354293c553e208654 Mon Sep 17 00:00:00 2001 From: Madhava Jay Date: Fri, 3 Feb 2023 12:22:24 +1000 Subject: [PATCH 6/6] Removed unused return - Parameterized body test fixtures --- integration_tests/test_unsupported_types.py | 64 +++++++++++---------- src/types.rs | 2 +- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/integration_tests/test_unsupported_types.py b/integration_tests/test_unsupported_types.py index e39a7750b..6c7206eb7 100644 --- a/integration_tests/test_unsupported_types.py +++ b/integration_tests/test_unsupported_types.py @@ -3,39 +3,41 @@ from robyn.robyn import Response -def test_bad_body_types(): - class A: - pass - - bad_bodies = [ - None, - 1, - True, - A, - {"body": "OK"}, - ["OK", b"OK"], - Response( - status_code=200, - headers={}, - body=b"OK", - ), - ] - - for body in bad_bodies: - with pytest.raises(ValueError): - _ = Response( - status_code=200, - headers={}, - body=body, - ) - - -def test_good_body_types(): - good_bodies = ["OK", b"OK"] - - for body in good_bodies: +class A: + pass + + +bad_bodies = [ + None, + 1, + True, + A, + {"body": "OK"}, + ["OK", b"OK"], + Response( + status_code=200, + headers={}, + body=b"OK", + ), +] + +good_bodies = ["OK", b"OK"] + + +@pytest.mark.parametrize("body", bad_bodies) +def test_bad_body_types(body): + with pytest.raises(ValueError): _ = Response( status_code=200, headers={}, body=body, ) + + +@pytest.mark.parametrize("body", good_bodies) +def test_good_body_types(body): + _ = Response( + status_code=200, + headers={}, + body=body, + ) diff --git a/src/types.rs b/src/types.rs index 5aef2027e..9c9e3ae51 100644 --- a/src/types.rs +++ b/src/types.rs @@ -162,7 +162,7 @@ pub struct Response { impl Response { #[new] pub fn new(status_code: u16, headers: HashMap, body: &PyAny) -> PyResult { - return Ok(Self { + Ok(Self { status_code, // we should be handling based on headers but works for now response_type: "text".to_string(),