Browse Source

code review

Falko Schindler 1 year ago
parent
commit
dc5f5cbc04

+ 1 - 1
README.md

@@ -43,7 +43,7 @@ NiceGUI is available as [PyPI package](https://pypi.org/project/nicegui/), [Dock
 - straight-forward data binding and refreshable functions to write even less code
 - straight-forward data binding and refreshable functions to write even less code
 - notifications, dialogs and menus to provide state of the art user interaction
 - notifications, dialogs and menus to provide state of the art user interaction
 - shared and individual web pages
 - shared and individual web pages
-- easy to use per-user and general persistence
+- easy-to-use per-user and general persistence
 - ability to add custom routes and data responses
 - ability to add custom routes and data responses
 - capture keyboard input for global shortcuts etc.
 - capture keyboard input for global shortcuts etc.
 - customize look by defining primary, secondary and accent colors
 - customize look by defining primary, secondary and accent colors

+ 1 - 1
examples/authentication/main.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
 #!/usr/bin/env python3
-"""This is a just very simple authentication example.
+"""This is just a very simple authentication example.
 
 
 Please see the `OAuth2 example at FastAPI <https://fastapi.tiangolo.com/tutorial/security/simple-oauth2/>`_  or
 Please see the `OAuth2 example at FastAPI <https://fastapi.tiangolo.com/tutorial/security/simple-oauth2/>`_  or
 use the great `Authlib package <https://docs.authlib.org/en/v0.13/client/starlette.html#using-fastapi>`_ to implement a classing real authentication system.
 use the great `Authlib package <https://docs.authlib.org/en/v0.13/client/starlette.html#using-fastapi>`_ to implement a classing real authentication system.

+ 1 - 1
main.py

@@ -352,7 +352,7 @@ async def documentation_page_more(name: str, client: Client) -> None:
     with side_menu() as menu:
     with side_menu() as menu:
         ui.markdown(f'[← back](/documentation#{create_anchor_name(back_link_target)})').classes('bold-links')
         ui.markdown(f'[← back](/documentation#{create_anchor_name(back_link_target)})').classes('bold-links')
     with ui.column().classes('w-full p-8 lg:p-16 max-w-[1250px] mx-auto'):
     with ui.column().classes('w-full p-8 lg:p-16 max-w-[1250px] mx-auto'):
-        section_heading('Documentation', f'ui.*{name}*')
+        section_heading('Documentation', f'ui.*{name}*' if hasattr(ui, name) else f'*{name.title()}*')
         with menu:
         with menu:
             ui.markdown('**Demos**' if more else '**Demo**').classes('mt-4')
             ui.markdown('**Demos**' if more else '**Demo**').classes('mt-4')
         element_demo(api)(getattr(module, 'main_demo'))
         element_demo(api)(getattr(module, 'main_demo'))

+ 1 - 1
nicegui/element.py

@@ -230,7 +230,7 @@ class Element(Visibility):
                 throttle=throttle,
                 throttle=throttle,
                 leading_events=leading_events,
                 leading_events=leading_events,
                 trailing_events=trailing_events,
                 trailing_events=trailing_events,
-                request=storage.request_contextvar.get()
+                request=storage.request_contextvar.get(),
             )
             )
             self._event_listeners[listener.id] = listener
             self._event_listeners[listener.id] = listener
             self.update()
             self.update()

+ 1 - 1
nicegui/event_listener.py

@@ -17,7 +17,7 @@ class EventListener:
     throttle: float
     throttle: float
     leading_events: bool
     leading_events: bool
     trailing_events: bool
     trailing_events: bool
-    request: Request
+    request: Optional[Request]
 
 
     def __post_init__(self) -> None:
     def __post_init__(self) -> None:
         self.id = str(uuid.uuid4())
         self.id = str(uuid.uuid4())

+ 57 - 39
nicegui/storage.py

@@ -3,76 +3,85 @@ import contextvars
 import json
 import json
 import threading
 import threading
 import uuid
 import uuid
+from collections.abc import MutableMapping
 from pathlib import Path
 from pathlib import Path
-from typing import Dict
+from typing import Any, Dict, Iterator
 
 
 import aiofiles
 import aiofiles
 from fastapi import Request
 from fastapi import Request
-from starlette.middleware.base import BaseHTTPMiddleware
+from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
+from starlette.responses import Response
+
+from . import globals
 
 
 request_contextvar = contextvars.ContextVar('request_var', default=None)
 request_contextvar = contextvars.ContextVar('request_var', default=None)
 
 
 
 
-class ReadOnlyDict:
-    def __init__(self, data: Dict, write_error_message: str = 'Read-only dict'):
-        self._data = data
-        self._write_error_message = write_error_message
+class ReadOnlyDict(MutableMapping):
+
+    def __init__(self, data: Dict[Any, Any], write_error_message: str = 'Read-only dict') -> None:
+        self._data: Dict[Any, Any] = data
+        self._write_error_message: str = write_error_message
 
 
-    def __getitem__(self, item):
+    def __getitem__(self, item: Any) -> Any:
         return self._data[item]
         return self._data[item]
 
 
-    def __iter__(self):
+    def __setitem__(self, key: Any, value: Any) -> None:
+        raise TypeError(self._write_error_message)
+
+    def __delitem__(self, key: Any) -> None:
+        raise TypeError(self._write_error_message)
+
+    def __iter__(self) -> Iterator:
         return iter(self._data)
         return iter(self._data)
 
 
-    def __len__(self):
+    def __len__(self) -> int:
         return len(self._data)
         return len(self._data)
 
 
-    def __setitem__(self, key, value):
-        raise TypeError(self._write_error_message)
-
 
 
 class PersistentDict(dict):
 class PersistentDict(dict):
-    def __init__(self, filename: Path, *arg, **kw):
-        self.filename = filename
+
+    def __init__(self, filepath: Path, *args: Any, **kwargs: Any) -> None:
+        self.filepath = filepath
         self.lock = threading.Lock()
         self.lock = threading.Lock()
         self.load()
         self.load()
-        self.update(*arg, **kw)
-        self.modified = bool(arg or kw)
+        self.update(*args, **kwargs)
+        self.modified = bool(args or kwargs)
 
 
-    def load(self):
+    def clear(self) -> None:
         with self.lock:
         with self.lock:
-            if self.filename.exists():
-                with open(self.filename, 'r') as f:
-                    try:
-                        self.update(json.load(f))
-                    except json.JSONDecodeError:
-                        pass
+            super().clear()
+            self.modified = True
 
 
-    def __setitem__(self, key, value):
+    def __setitem__(self, key: Any, value: Any) -> None:
         with self.lock:
         with self.lock:
             super().__setitem__(key, value)
             super().__setitem__(key, value)
             self.modified = True
             self.modified = True
 
 
-    def __delitem__(self, key):
+    def __delitem__(self, key: Any) -> None:
         with self.lock:
         with self.lock:
             super().__delitem__(key)
             super().__delitem__(key)
             self.modified = True
             self.modified = True
 
 
-    def clear(self):
+    def load(self) -> None:
         with self.lock:
         with self.lock:
-            super().clear()
-            self.modified = True
+            if self.filepath.exists():
+                with open(self.filepath, 'r') as f:
+                    try:
+                        self.update(json.load(f))
+                    except json.JSONDecodeError:
+                        pass
 
 
-    async def backup(self):
+    async def backup(self) -> None:
         data = dict(self)
         data = dict(self)
         if self.modified:
         if self.modified:
-            async with aiofiles.open(self.filename, 'w') as f:
+            async with aiofiles.open(self.filepath, 'w') as f:
                 await f.write(json.dumps(data))
                 await f.write(json.dumps(data))
 
 
 
 
 class RequestTrackingMiddleware(BaseHTTPMiddleware):
 class RequestTrackingMiddleware(BaseHTTPMiddleware):
 
 
-    async def dispatch(self, request: Request, call_next):
+    async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -> Response:
         request_contextvar.set(request)
         request_contextvar.set(request)
         if 'id' not in request.session:
         if 'id' not in request.session:
             request.session['id'] = str(uuid.uuid4())
             request.session['id'] = str(uuid.uuid4())
@@ -84,7 +93,7 @@ class RequestTrackingMiddleware(BaseHTTPMiddleware):
 
 
 class Storage:
 class Storage:
 
 
-    def __init__(self):
+    def __init__(self) -> None:
         self.storage_dir = Path('.nicegui')
         self.storage_dir = Path('.nicegui')
         self.storage_dir.mkdir(exist_ok=True)
         self.storage_dir.mkdir(exist_ok=True)
         self._general = PersistentDict(self.storage_dir / 'storage_general.json')
         self._general = PersistentDict(self.storage_dir / 'storage_general.json')
@@ -94,15 +103,20 @@ class Storage:
     def browser(self) -> Dict:
     def browser(self) -> Dict:
         """Small storage that is saved directly within the user's browser (encrypted cookie).
         """Small storage that is saved directly within the user's browser (encrypted cookie).
 
 
-        The data is shared between all browser tab and can only be modified before the initial request has been submitted.
-        It is normally better to use `app.storage.user` instead to reduce payload, gain improved security and have larger storage capacity)."""
+        The data is shared between all browser tabs and can only be modified before the initial request has been submitted.
+        It is normally better to use `app.storage.user` instead to reduce payload, gain improved security and have larger storage capacity.
+        """
         request: Request = request_contextvar.get()
         request: Request = request_contextvar.get()
         if request is None:
         if request is None:
-            raise RuntimeError('storage.browser needs a storage_secret passed in ui.run()')
+            if globals.get_client() == globals.index_client:
+                raise RuntimeError('app.storage.browser can only be used with page builder functions '
+                                   '(https://nicegui.io/documentation/page)')
+            else:
+                raise RuntimeError('app.storage.browser needs a storage_secret passed in ui.run()')
         if request.state.responded:
         if request.state.responded:
             return ReadOnlyDict(
             return ReadOnlyDict(
                 request.session,
                 request.session,
-                'the response to the browser has already been built so modifications cannot be sent back anymore'
+                'the response to the browser has already been built, so modifications cannot be sent back anymore'
             )
             )
         return request.session
         return request.session
 
 
@@ -111,11 +125,15 @@ class Storage:
         """Individual user storage that is persisted on the server (where NiceGUI is executed).
         """Individual user storage that is persisted on the server (where NiceGUI is executed).
 
 
         The data is stored in a file on the server.
         The data is stored in a file on the server.
-        It is shared between all browser tabs by identifying the user via session cookie id.
+        It is shared between all browser tabs by identifying the user via session cookie ID.
         """
         """
         request: Request = request_contextvar.get()
         request: Request = request_contextvar.get()
         if request is None:
         if request is None:
-            raise RuntimeError('app.storage.user needs a storage_secret passed in ui.run()')
+            if globals.get_client() == globals.index_client:
+                raise RuntimeError('app.storage.user can only be used with page builder functions '
+                                   '(https://nicegui.io/documentation/page)')
+            else:
+                raise RuntimeError('app.storage.user needs a storage_secret passed in ui.run()')
         if request.session['id'] not in self._users:
         if request.session['id'] not in self._users:
             self._users[request.session['id']] = {}
             self._users[request.session['id']] = {}
         return self._users[request.session['id']]
         return self._users[request.session['id']]
@@ -132,4 +150,4 @@ class Storage:
     async def _loop(self) -> None:
     async def _loop(self) -> None:
         while True:
         while True:
             await self.backup()
             await self.backup()
-            await asyncio.sleep(10)
+            await asyncio.sleep(1.0)

+ 0 - 1
tests/conftest.py

@@ -43,7 +43,6 @@ def reset_globals() -> Generator[None, None, None]:
     # NOTE favicon routes must be removed separately because they are not "pages"
     # NOTE favicon routes must be removed separately because they are not "pages"
     [globals.app.routes.remove(r) for r in globals.app.routes if r.path.endswith('/favicon.ico')]
     [globals.app.routes.remove(r) for r in globals.app.routes if r.path.endswith('/favicon.ico')]
     importlib.reload(globals)
     importlib.reload(globals)
-    # importlib.reload(nicegui)
     globals.app.storage.general.clear()
     globals.app.storage.general.clear()
     globals.app.storage._users.clear()
     globals.app.storage._users.clear()
     globals.index_client = Client(page('/'), shared=True).__enter__()
     globals.index_client = Client(page('/'), shared=True).__enter__()

+ 3 - 3
tests/test_storage.py

@@ -86,7 +86,7 @@ async def test_access_user_storage_on_interaction(screen: Screen):
     screen.click('switch')
     screen.click('switch')
     screen.wait(1)
     screen.wait(1)
     await app.storage.backup()
     await app.storage.backup()
-    assert '{"test_switch": true}' in app.storage._users.filename.read_text()
+    assert '{"test_switch": true}' in app.storage._users.filepath.read_text()
 
 
 
 
 def test_access_user_storage_from_button_click_handler(screen: Screen):
 def test_access_user_storage_from_button_click_handler(screen: Screen):
@@ -102,7 +102,7 @@ def test_access_user_storage_from_button_click_handler(screen: Screen):
     screen.open('/')
     screen.open('/')
     screen.click('test')
     screen.click('test')
     screen.wait(1)
     screen.wait(1)
-    assert '{"inner_function": "works"}' in app.storage._users.filename.read_text()
+    assert '{"inner_function": "works"}' in app.storage._users.filepath.read_text()
 
 
 
 
 async def test_access_user_storage_from_background_task(screen: Screen):
 async def test_access_user_storage_from_background_task(screen: Screen):
@@ -116,7 +116,7 @@ async def test_access_user_storage_from_background_task(screen: Screen):
 
 
     screen.ui_run_kwargs['storage_secret'] = 'just a test'
     screen.ui_run_kwargs['storage_secret'] = 'just a test'
     screen.open('/')
     screen.open('/')
-    assert '{"subtask": "works"}' in app.storage._users.filename.read_text()
+    assert '{"subtask": "works"}' in app.storage._users.filepath.read_text()
 
 
 
 
 def test_user_and_general_storage_is_persisted(screen: Screen):
 def test_user_and_general_storage_is_persisted(screen: Screen):

+ 5 - 5
website/documentation.py

@@ -416,11 +416,11 @@ def create_full() -> None:
     load_demo('storage')
     load_demo('storage')
 
 
     @text_demo('Parameter injection', '''
     @text_demo('Parameter injection', '''
-        Thanks to FastAPI, a page function accepts optional parameters to 
-        provide [path parameters](https://fastapi.tiangolo.com/tutorial/path-params/), 
-        [query parameters](https://fastapi.tiangolo.com/tutorial/query-params/) or
-        the [the full incoming request](https://fastapi.tiangolo.com/advanced/using-request-directly/?h=request) for
-        access to the body payload, headers, cookies and more.
+        Thanks to FastAPI, a page function accepts optional parameters to provide
+        [path parameters](https://fastapi.tiangolo.com/tutorial/path-params/), 
+        [query parameters](https://fastapi.tiangolo.com/tutorial/query-params/) or the whole incoming
+        [request](https://fastapi.tiangolo.com/advanced/using-request-directly/) for accessing
+        the body payload, headers, cookies and more.
     ''')
     ''')
     def parameter_demo():
     def parameter_demo():
         @ui.page('/icon/{icon}')
         @ui.page('/icon/{icon}')

+ 2 - 1
website/documentation_tools.py

@@ -96,7 +96,8 @@ class element_demo:
         doc = self.element_class.__doc__ or self.element_class.__init__.__doc__
         doc = self.element_class.__doc__ or self.element_class.__init__.__doc__
         title, documentation = doc.split('\n', 1)
         title, documentation = doc.split('\n', 1)
         with ui.column().classes('w-full mb-8 gap-2'):
         with ui.column().classes('w-full mb-8 gap-2'):
-            subheading(title, more_link=more_link)
+            if more_link:
+                subheading(title, more_link=more_link)
             render_docstring(documentation, with_params=more_link is None)
             render_docstring(documentation, with_params=more_link is None)
             result = demo(f)
             result = demo(f)
             if more_link:
             if more_link:

+ 10 - 4
website/more_documentation/storage_documentation.py

@@ -8,12 +8,18 @@ from ..documentation_tools import text_demo
 
 
 def main_demo() -> None:
 def main_demo() -> None:
     """Storage
     """Storage
+
     NiceGUI offers a straightforward method for data persistence within your application. 
     NiceGUI offers a straightforward method for data persistence within your application. 
     It features three built-in storage types:
     It features three built-in storage types:
 
 
-    - `app.storage.user`: Stored server-side, each dictionary is associated with a unique identifier held in a browser session cookie. Unique to each user, this storage is accessible across all their browser tabs.
-    - app.storage.general`: Also stored server-side, this dictionary provides a shared storage space accessible to all users.
-    - `app.storage.browser`: Unlike the previous types, this dictionary is stored directly as the browser session cookie, shared among all browser tabs for the same user. However, `app.storage.user` is generally preferred due to its advantages in reducing data payload, enhancing security, and offering larger storage capacity.
+    - `app.storage.user`:
+        Stored server-side, each dictionary is associated with a unique identifier held in a browser session cookie.
+        Unique to each user, this storage is accessible across all their browser tabs.
+    - `app.storage.general`:
+        Also stored server-side, this dictionary provides a shared storage space accessible to all users.
+    - `app.storage.browser`:
+        Unlike the previous types, this dictionary is stored directly as the browser session cookie, shared among all browser tabs for the same user.
+        However, `app.storage.user` is generally preferred due to its advantages in reducing data payload, enhancing security, and offering larger storage capacity.
 
 
     To use the user or browser storage, you must pass a `storage_secret` to `ui.run()`. 
     To use the user or browser storage, you must pass a `storage_secret` to `ui.run()`. 
     This is a private key used to encrypt the browser session cookie.
     This is a private key used to encrypt the browser session cookie.
@@ -41,7 +47,7 @@ start = datetime.now().strftime('%H:%M, %d %B %Y')
 
 
 def more() -> None:
 def more() -> None:
     @text_demo('Counting page visits', '''
     @text_demo('Counting page visits', '''
-        Here we are using the automatically available browser stored session id to count the number of unique page visits.
+        Here we are using the automatically available browser-stored session ID to count the number of unique page visits.
     ''')
     ''')
     def page_visits():
     def page_visits():
         from collections import Counter
         from collections import Counter