Merge pull request #54 from penpot/bugfix/html-temp-file-writing

Bugfix/html temp file writing
penpot · Jun 6, 2024 · 0d741d0 · 0d741d0
2 parents ddee295 + 93e1554
commit 0d741d0
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 42 deletions.
diff --git a/src/penai/models.py b/src/penai/models.py
@@ -1,6 +1,6 @@
 from copy import deepcopy
 from dataclasses import dataclass, field
-from functools import cache, cached_property
+from functools import cached_property
 from pathlib import Path
 from typing import Generic, Self, TypeVar
 from uuid import UUID
@@ -100,7 +100,6 @@ class PenpotComponentDict(dict[str, PenpotComponent]):
     def get_component_names(self) -> list[str]:
         return [component.name for component in self.values()]
 
-    @cache
     def get_by_name(self, name: str) -> PenpotComponent:
         # This can definitely be implemented more efficiently but since the number
         # of components per file is typically very small, this shouldn't become

diff --git a/src/penai/render.py b/src/penai/render.py
@@ -1,6 +1,5 @@
 import abc
 import io
-import time
 from collections.abc import Callable, Generator
 from contextlib import contextmanager
 from functools import wraps
@@ -11,7 +10,7 @@
 from PIL import Image
 from selenium.webdriver.remote.webdriver import WebDriver
 
-from penai.svg import SVG, BoundingBox
+from penai.svg import SVG
 from penai.types import PathLike
 from penai.utils.svg import image_from_bytes, temp_file_for_content
 from penai.utils.web_drivers import create_chrome_web_driver
@@ -93,35 +92,8 @@ def create_chrome_renderer(
         with create_chrome_web_driver() as driver:
             yield cls(driver, **kwargs)
 
-    def _open_svg(self, svg_path: str) -> None:
-        self.web_driver.get(svg_path)
-
-        # TODO: Wait until content is displayed instead of a fixed time
-        # See for instance https://www.selenium.dev/documentation/webdriver/waits/
-
-        if self.wait_time:
-            time.sleep(self.wait_time)
-
-        # Determine the size of the SVG element and set the window size accordingly
-        bbox = BoundingBox.from_dom_rect(
-            self.web_driver.execute_script(
-                "return document.querySelector('svg').getBoundingClientRect();",
-            ),
-        )
-
-        assert (
-            bbox.x >= 0 and bbox.y >= 0
-        ), f"Bounding box origin should be non-negative, got ({bbox.x}, {bbox.y})"
-
-        self.web_driver.set_window_size(bbox.x + bbox.width, bbox.y + bbox.height)
-
-        if self.wait_time:
-            time.sleep(self.wait_time)
-
-        self.web_driver.get(svg_path)
-
     def _render_svg(self, svg_path: str) -> Image.Image:
-        self._open_svg(svg_path)
+        self.web_driver.get(svg_path)
 
         buffer = io.BytesIO(self.web_driver.get_screenshot_as_png())
         buffer.seek(0)
@@ -148,7 +120,8 @@ def render_svg_string(
         # See https://html.spec.whatwg.org/multipage/syntax.html#syntax-tag-omission
         content = '<body style="margin: 0;">' + svg_string + "</body>"
 
-        with temp_file_for_content(content, extension=".html") as path:
+        with temp_file_for_content(content, extension=".html", delete=False) as path:
+            print(f"Saving html to: {path.absolute().as_uri()}")
             return self._render_svg(path.absolute().as_uri())
 
     def render_svg(self, svg: SVG) -> Image.Image:
@@ -169,7 +142,7 @@ def render_svg_file(
         :param width: The width of the rendered image. Currently not supported.
         :param height: The height of the rendered image. Currently not supported.
         """
-        return self.render_svg_string(Path(svg_path).read_text())
+        return self.render_svg(SVG.from_file(svg_path))
 
 
 class ResvgRenderer(BaseSVGRenderer):

diff --git a/src/penai/svg.py b/src/penai/svg.py
@@ -386,7 +386,7 @@ def set_default_view_box(
 
         if web_driver is None:
             raise ValueError(
-                "since bbox was not provided, a renderer must be provided to derive the default view box "
+                "since bbox was not provided, a web_driver must be provided to derive the default view box "
                 "from the dom.",
             )
         self._default_view_box = self.to_svg(view_box=None).retrieve_default_view_box(web_driver)
@@ -438,7 +438,7 @@ def get_default_view_box(
 
         if web_driver is None:
             raise ValueError(
-                "Default view box was not yet set, a renderer must be provided to derive the default view box.",
+                "Default view box was not yet set, a web_driver must be provided to derive the default view box.",
             )
         self.set_default_view_box(web_driver=web_driver)
         return cast(BoundingBox, self._default_view_box)
@@ -628,11 +628,9 @@ def _get_shapes_by_attr(
             )
         return matched_shapes[0]
 
-    @cache
     def get_shape_by_name(self, name: str) -> PenpotShapeElement:
         return self._get_shapes_by_attr("name", name, should_be_unique=True)
 
-    @cache
     def get_shape_by_id(self, shape_id: str) -> PenpotShapeElement:
         return self._get_shapes_by_attr("shape_id", shape_id, should_be_unique=True)
 

diff --git a/src/penai/utils/svg.py b/src/penai/utils/svg.py
@@ -54,7 +54,11 @@ def validate_uri(x: Any) -> bool:
 
 
 @contextmanager
-def temp_file_for_content(content: str | bytes, extension: str) -> Generator[Path, Any, Any]:
+def temp_file_for_content(
+    content: str | bytes,
+    extension: str,
+    delete: bool = False,
+) -> Generator[Path, Any, Any]:
     """Create a temporary file for a given file content."""
     if extension and not extension.startswith("."):
         raise ValueError("Extension should start with a dot")
@@ -65,9 +69,15 @@ def temp_file_for_content(content: str | bytes, extension: str) -> Generator[Pat
         assert isinstance(content, bytes)
         mode = "wb"
 
+    # Note: (just for the curious, not actually needed to know)
+    # buffering=0 is very important if you want to yield inside
+    # Since we don't use the delete option here (we delete manually below)
+    # we yield outside of this context
+    # The code below is essentially equivalent to `with open()...write`
     with NamedTemporaryFile(prefix="penai_", suffix=extension, mode=mode, delete=False) as file:
         file.write(content)
         path = Path(file.name)
-        yield path
+    yield path
 
-    path.unlink()
+    if delete:
+        path.unlink()