From 042710ca91ed8f87f37e45b1a5de6949665cd1b8 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 18 Aug 2023 13:12:17 -0700 Subject: [PATCH] Handle file uploads with component-local state (#1616) --- integration/test_upload.py | 174 +++++++++++++++++++++++++ reflex/.templates/web/utils/state.js | 10 +- reflex/components/forms/__init__.py | 3 +- reflex/components/forms/upload.py | 12 +- reflex/event.py | 2 + tests/components/forms/test_uploads.py | 4 +- 6 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 integration/test_upload.py diff --git a/integration/test_upload.py b/integration/test_upload.py new file mode 100644 index 000000000..240b4ae47 --- /dev/null +++ b/integration/test_upload.py @@ -0,0 +1,174 @@ +"""Integration tests for file upload.""" +from __future__ import annotations + +import time +from typing import Generator + +import pytest +from selenium.webdriver.common.by import By + +from reflex.testing import AppHarness + + +def UploadFile(): + """App for testing dynamic routes.""" + import reflex as rx + + class UploadState(rx.State): + _file_data: dict[str, str] = {} + + async def handle_upload(self, files: list[rx.UploadFile]): + for file in files: + upload_data = await file.read() + self._file_data[file.filename or ""] = upload_data.decode("utf-8") + + @rx.var + def token(self) -> str: + return self.get_token() + + def index(): + return rx.vstack( + rx.input(value=UploadState.token, is_read_only=True, id="token"), + rx.upload( + rx.vstack( + rx.button("Select File"), + rx.text("Drag and drop files here or click to select files"), + ), + ), + rx.button( + "Upload", + on_click=lambda: UploadState.handle_upload(rx.upload_files()), # type: ignore + id="upload_button", + ), + rx.box( + rx.foreach( + rx.selected_files, + lambda f: rx.text(f), + ), + id="selected_files", + ), + ) + + app = rx.App(state=UploadState) + app.add_page(index) + app.compile() + + +@pytest.fixture(scope="session") +def upload_file(tmp_path_factory) -> Generator[AppHarness, None, None]: + """Start UploadFile app at tmp_path via AppHarness. + + Args: + tmp_path_factory: pytest tmp_path_factory fixture + + Yields: + running AppHarness instance + """ + with AppHarness.create( + root=tmp_path_factory.mktemp("upload_file"), + app_source=UploadFile, # type: ignore + ) as harness: + yield harness + + +@pytest.fixture +def driver(upload_file: AppHarness): + """Get an instance of the browser open to the upload_file app. + + Args: + upload_file: harness for DynamicRoute app + + Yields: + WebDriver instance. + """ + assert upload_file.app_instance is not None, "app is not running" + driver = upload_file.frontend() + try: + assert upload_file.poll_for_clients() + yield driver + finally: + driver.quit() + + +def test_upload_file(tmp_path, upload_file: AppHarness, driver): + """Submit a file upload and check that it arrived on the backend. + + Args: + tmp_path: pytest tmp_path fixture + upload_file: harness for UploadFile app. + driver: WebDriver instance. + """ + assert upload_file.app_instance is not None + token_input = driver.find_element(By.ID, "token") + assert token_input + # wait for the backend connection to send the token + token = upload_file.poll_for_value(token_input) + assert token is not None + + upload_box = driver.find_element(By.XPATH, "//input[@type='file']") + assert upload_box + upload_button = driver.find_element(By.ID, "upload_button") + assert upload_button + + exp_name = "test.txt" + exp_contents = "test file contents!" + target_file = tmp_path / exp_name + target_file.write_text(exp_contents) + + upload_box.send_keys(str(target_file)) + upload_button.click() + + # look up the backend state and assert on uploaded contents + backend_state = upload_file.app_instance.state_manager.states[token] + time.sleep(0.5) + assert backend_state._file_data[exp_name] == exp_contents + + # check that the selected files are displayed + selected_files = driver.find_element(By.ID, "selected_files") + assert selected_files.text == exp_name + + +def test_upload_file_multiple(tmp_path, upload_file: AppHarness, driver): + """Submit several file uploads and check that they arrived on the backend. + + Args: + tmp_path: pytest tmp_path fixture + upload_file: harness for UploadFile app. + driver: WebDriver instance. + """ + assert upload_file.app_instance is not None + token_input = driver.find_element(By.ID, "token") + assert token_input + # wait for the backend connection to send the token + token = upload_file.poll_for_value(token_input) + assert token is not None + + upload_box = driver.find_element(By.XPATH, "//input[@type='file']") + assert upload_box + upload_button = driver.find_element(By.ID, "upload_button") + assert upload_button + + exp_files = { + "test1.txt": "test file contents!", + "test2.txt": "this is test file number 2!", + "reflex.txt": "reflex is awesome!", + } + for exp_name, exp_contents in exp_files.items(): + target_file = tmp_path / exp_name + target_file.write_text(exp_contents) + upload_box.send_keys(str(target_file)) + + time.sleep(0.2) + + # check that the selected files are displayed + selected_files = driver.find_element(By.ID, "selected_files") + assert selected_files.text == "\n".join(exp_files) + + # do the upload + upload_button.click() + + # look up the backend state and assert on uploaded contents + backend_state = upload_file.app_instance.state_manager.states[token] + time.sleep(0.5) + for exp_name, exp_contents in exp_files.items(): + assert backend_state._file_data[exp_name] == exp_contents diff --git a/reflex/.templates/web/utils/state.js b/reflex/.templates/web/utils/state.js index 009cf86c0..5c382a3cf 100644 --- a/reflex/.templates/web/utils/state.js +++ b/reflex/.templates/web/utils/state.js @@ -193,10 +193,10 @@ export const applyEvent = async (event, socket) => { * * @returns Whether the event was sent. */ -export const applyRestEvent = async (event, state) => { +export const applyRestEvent = async (event) => { let eventSent = false; if (event.handler == "uploadFiles") { - eventSent = await uploadFiles(state, event.name); + eventSent = await uploadFiles(event.name, event.payload.files); } return eventSent; }; @@ -232,7 +232,7 @@ export const processEvent = async ( let eventSent = false // Process events with handlers via REST and all others via websockets. if (event.handler) { - eventSent = await applyRestEvent(event, currentState); + eventSent = await applyRestEvent(event); } else { eventSent = await applyEvent(event, socket); } @@ -298,9 +298,7 @@ export const connect = async ( * * @returns Whether the files were uploaded. */ -export const uploadFiles = async (state, handler) => { - const files = state.files; - +export const uploadFiles = async (handler, files) => { // return if there's no file to upload if (files.length == 0) { return false; diff --git a/reflex/components/forms/__init__.py b/reflex/components/forms/__init__.py index 15c038e0e..ad816a46e 100644 --- a/reflex/components/forms/__init__.py +++ b/reflex/components/forms/__init__.py @@ -46,10 +46,11 @@ from .select import Option, Select from .slider import Slider, SliderFilledTrack, SliderMark, SliderThumb, SliderTrack from .switch import Switch from .textarea import TextArea -from .upload import Upload +from .upload import Upload, selected_files helpers = [ "color_mode_cond", + "selected_files", ] __all__ = [f for f in dir() if f[0].isupper()] + helpers # type: ignore diff --git a/reflex/components/forms/upload.py b/reflex/components/forms/upload.py index d98959dec..f923a71a1 100644 --- a/reflex/components/forms/upload.py +++ b/reflex/components/forms/upload.py @@ -1,4 +1,5 @@ """A file upload component.""" +from __future__ import annotations from typing import Dict, List, Optional @@ -8,7 +9,11 @@ from reflex.components.layout.box import Box from reflex.event import EventChain from reflex.vars import BaseVar, Var -upload_file = BaseVar(name="e => File(e)", type_=EventChain) +files_state = "const [files, setFiles] = useState([]);" +upload_file = BaseVar(name="e => setFiles((files) => e)", type_=EventChain) + +# Use this var along with the Upload component to render the list of selected files. +selected_files = BaseVar(name="files.map((f) => f.name)", type_=List[str]) class Upload(Component): @@ -73,7 +78,7 @@ class Upload(Component): zone = Box.create( upload, *children, - **{k: v for k, v in props.items() if k not in supported_props} + **{k: v for k, v in props.items() if k not in supported_props}, ) zone.special_props = {BaseVar(name="{...getRootProps()}", type_=None)} @@ -94,3 +99,6 @@ class Upload(Component): out = super()._render() out.args = ("getRootProps", "getInputProps") return out + + def _get_hooks(self) -> str | None: + return (super()._get_hooks() or "") + files_state diff --git a/reflex/event.py b/reflex/event.py index d5954a8ae..a71521c62 100644 --- a/reflex/event.py +++ b/reflex/event.py @@ -64,6 +64,8 @@ class EventHandler(Base): return EventSpec( handler=self, client_handler_name="uploadFiles", + # `files` is defined in the Upload component's _use_hooks + args=((Var.create_safe("files"), Var.create_safe("files")),), ) # Otherwise, convert to JSON. diff --git a/tests/components/forms/test_uploads.py b/tests/components/forms/test_uploads.py index 74134276f..d59d760bd 100644 --- a/tests/components/forms/test_uploads.py +++ b/tests/components/forms/test_uploads.py @@ -53,7 +53,7 @@ def test_upload_component_render(upload_component): assert upload["name"] == "ReactDropzone" assert upload["props"] == [ "multiple={true}", - "onDrop={e => File(e)}", + "onDrop={e => setFiles((files) => e)}", ] assert upload["args"] == ("getRootProps", "getInputProps") @@ -92,5 +92,5 @@ def test_upload_component_with_props_render(upload_component_with_props): "maxFiles={2}", "multiple={true}", "noDrag={true}", - "onDrop={e => File(e)}", + "onDrop={e => setFiles((files) => e)}", ]