Skip to content

Commit

Permalink
Merge pull request #153 from opensafely-core/optimise-partial-render
Browse files Browse the repository at this point in the history 
Optimise htmx partial rendering
  • Loading branch information
@bloodearnest
bloodearnest authored Mar 11, 2024
2 parents e415c3b + 8ba0333 commit ae8dfc6
Show file tree
Hide file tree
Showing 10 changed files with 291 additions and 105 deletions.
73 changes: 64 additions & 9 deletions airlock/file_browser_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class PathType(Enum):
"""Types of PathItems in a tree."""

FILE = "file"
DIR = "dir"
DIR = "directory"
WORKSPACE = "workspace"
REQUEST = "request"
FILEGROUP = "filegroup"
Expand Down Expand Up @@ -143,7 +143,7 @@ def html_classes(self):
distinguish file/dirs, and maybe even file types, in the UI, in case we
need to.
"""
classes = [self.type.name.lower()]
classes = [self.type.value.lower()]

if self.type == PathType.FILE:
classes.append(self.file_type())
Expand Down Expand Up @@ -208,13 +208,32 @@ def build_string(node, indent):
return "\n".join(build_string(self, ""))


def get_workspace_tree(workspace, selected_path=ROOT_PATH):
"""Recursively build a workspace tree from files on disk."""
def get_workspace_tree(workspace, selected_path=ROOT_PATH, selected_only=False):
"""Recursively build workspace tree from the root dir.
If selected_only==True, we do not build entire tree, as that can be
expensive if we just want to partially render one node.
Instead, we build just the tree down to the selected path, and then all its
immediate children, if it has any. We include children so that if
selected_path is a directory, its contents can be partially rendered.
"""

selected_path = UrlPath(selected_path)
root = workspace.root()
# list all files in one go is much faster than walking the tree
pathlist = [p.relative_to(root) for p in root.glob("**/*")]

if selected_only:
pathlist = [selected_path]

# if directory, we also need to also load children to display in the content area
abspath = workspace.abspath(selected_path)
if abspath.is_dir():
pathlist.extend(child.relative_to(root) for child in abspath.iterdir())

else:
# listing all files in one go is much faster than walking the tree
pathlist = [p.relative_to(root) for p in root.glob("**/*")]

root_node = PathItem(
container=workspace,
relpath=ROOT_PATH,
Expand All @@ -230,11 +249,14 @@ def get_workspace_tree(workspace, selected_path=ROOT_PATH):
return root_node


def get_request_tree(release_request, selected_path=ROOT_PATH):
def get_request_tree(release_request, selected_path=ROOT_PATH, selected_only=False):
"""Build a tree recursively for a ReleaseRequest
For each group, we create a node for that group, and then build a sub-tree
for its file groups.
If selected_only=True, we avoid building the entire tree. Instead, we just
build part of the tree on the selected_path, and its immediate children.
"""
# ensure selected_path is UrlPath
selected_path = UrlPath(selected_path)
Expand All @@ -261,9 +283,26 @@ def get_request_tree(release_request, selected_path=ROOT_PATH):
expanded=selected or expanded,
)

group_paths = [f.relpath for f in group.files]

if selected_only:
if expanded:
if group_path == selected_path:
# we just need the group's immediate child paths
pathlist = [UrlPath(p.parts[0]) for p in group_paths]
else:
# filter for just the selected path and any immediate children
selected_subpath = selected_path.relative_to(group_path)
pathlist = list(filter_files(selected_subpath, group_paths))
else:
# we don't want any children for unselected groups
pathlist = []
else:
pathlist = group_paths

group_node.children = get_path_tree(
release_request,
pathlist=[f.relpath for f in group.files],
pathlist=pathlist,
parent=group_node,
selected_path=selected_path,
expanded=expanded,
Expand All @@ -274,7 +313,23 @@ def get_request_tree(release_request, selected_path=ROOT_PATH):
return root_node


def get_path_tree(container, pathlist, parent, selected_path=ROOT_PATH, expanded=False):
def filter_files(selected, files):
"""Filter the list of file paths for the selected file and any immediate children."""
n = len(selected.parts)
for f in files:
head, tail = f.parts[:n], f.parts[n:]
if head == selected.parts and len(tail) <= 1:
yield f


def get_path_tree(
container,
pathlist,
parent,
selected_path=ROOT_PATH,
expanded=False,
selected_only=False,
):
"""Walk a flat list of paths and create a tree from them."""

def build_path_tree(path_parts, parent):
Expand Down
89 changes: 89 additions & 0 deletions airlock/templates/file_browser/contents.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<div id="selected-contents">

{% if path_item.is_directory %}

{% #card title=path_item.name %}
{% #list_group %}
{% if not path_item.children %}
{% list_group_empty icon=True title="Empty Directory" %}
{% else %}
{% for entry in path_item.children %}
{% #list_group_item href=entry.url %}
{{ entry.name}}
{% if entry.is_directory %}
{% icon_folder_outline class="h-6 w-6 text-slate-600 inline" %}
{% endif %}
{% /list_group_item %}
{% endfor %}
{% endif %}
{% /list_group %}
{% /card %}

{% else %}
{% fragment as add_button %}
{% if context == "workspace" %}
{% if form %}
{% #modal id="addRequestFile" button_text="Add File to Request" variant="success" %}
{% #card container=True title="Add a file" %}
<form action="{{ request_file_url }}" method="POST" aria-label="add-file-form">
{% csrf_token %}
{% form_select class="w-full max-w-lg mx-auto" label="Select a file group" field=form.filegroup choices=form.filegroup.field.choices %}
{% form_input class="w-full max-w-lg mx-auto" label="Or create a new file group" field=form.new_filegroup %}
<input type=hidden name="path" value="{{ path_item.relpath }}"/>
<div class="mt-2">
{% #button type="submit" variant="success" id="add-file-button" %}Add File to Request{% /button %}
{% #button variant="danger" type="cancel" %}Cancel{% /button %}
</div>
</form>
{% /card %}
{% /modal %}
{% elif file_in_request %}
{% #button type="button" disabled=True tooltip="This file has already been added to the current request" id="add-file-modal-button-disabled" %}
Add File to Request
{% /button %}
{% else %}
{% #button type="button" disabled=True tooltip="You do not have permission to add this file to a request" id="add-file-modal-button-disabled" %}
Add File to Request
{% /button %}
{% endif %}
{% elif is_author %}
<form action="" method="POST">
{% csrf_token %}
{% #button type="submit" tooltip="Remove this file from this request" variant="warning" %}Remove File from Request{% /button %}
</form>
{% elif is_output_checker %}
{% #button variant="primary" type="link" href=path_item.download_url id="download-button" %}Download file{% /button %}
{% endif %}
{% endfragment %}

{% #card title=path_item.name container=True custom_button=add_button %}

<div class="content">
{% if path_item.display_type == "iframe" %}
<iframe
src="{{ path_item.contents_url }}"
title="{{ path_item.relpath }}"
frameborder=0
height=1000
style="width: 100%;"
>
</iframe>
{% elif path_item.display_type == "image" %}
<div class="content-scroller">
<img src="{{ path_item.contents_url }}">
</div>
{% elif path_item.display_type == "table" %}
<div class="content-scroller">
{% include "file_browser/csv.html" with contents=path_item.contents %}
</div>
{% else %}
<div class="content-scroller">
<pre>{{ path_item.contents }}</pre>
</div>
{% endif %}
</div>
{% /card %}

{% endif %}

</div>
92 changes: 2 additions & 90 deletions airlock/templates/file_browser/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
background-color: lightgrey;
}

.tree a.dir {
.tree a.directory {
background-image: url("{% static "folder.png" %}");
background-repeat: no-repeat;
background-size: 1.4rem;
Expand Down Expand Up @@ -132,95 +132,7 @@
</div>

<div style="flex-basis: 75%; max-width: 75%">
<div id="selected-contents">

{% if path_item.is_directory %}

{% #card title=path_item.name %}
{% #list_group %}
{% if not path_item.children %}
{% list_group_empty icon=True title="Empty Directory" %}
{% else %}
{% for entry in path_item.children %}
{% #list_group_item href=entry.url %}
{{ entry.name}}
{% if entry.is_directory %}
{% icon_folder_outline class="h-6 w-6 text-slate-600 inline" %}
{% endif %}
{% /list_group_item %}
{% endfor %}
{% endif %}
{% /list_group %}
{% /card %}

{% else %}
{% fragment as add_button %}
{% if context == "workspace" %}
{% if form %}
{% #modal id="addRequestFile" button_text="Add File to Request" variant="success" %}
{% #card container=True title="Add a file" %}
<form action="{{ request_file_url }}" method="POST" aria-label="add-file-form">
{% csrf_token %}
{% form_select class="w-full max-w-lg mx-auto" label="Select a file group" field=form.filegroup choices=form.filegroup.field.choices %}
{% form_input class="w-full max-w-lg mx-auto" label="Or create a new file group" field=form.new_filegroup %}
<input type=hidden name="path" value="{{ path_item.relpath }}"/>
<div class="mt-2">
{% #button type="submit" variant="success" id="add-file-button" %}Add File to Request{% /button %}
{% #button variant="danger" type="cancel" %}Cancel{% /button %}
</div>
</form>
{% /card %}
{% /modal %}
{% elif file_in_request %}
{% #button type="button" disabled=True tooltip="This file has already been added to the current request" id="add-file-modal-button-disabled" %}
Add File to Request
{% /button %}
{% else %}
{% #button type="button" disabled=True tooltip="You do not have permission to add this file to a request" id="add-file-modal-button-disabled" %}
Add File to Request
{% /button %}
{% endif %}
{% elif is_author %}
<form action="" method="POST">
{% csrf_token %}
{% #button type="submit" tooltip="Remove this file from this request" variant="warning" %}Remove File from Request{% /button %}
</form>
{% elif is_output_checker %}
{% #button variant="primary" type="link" href=path_item.download_url id="download-button" %}Download file{% /button %}
{% endif %}
{% endfragment %}

{% #card title=path_item.name container=True custom_button=add_button %}

<div class="content">
{% if path_item.display_type == "iframe" %}
<iframe
src="{{ path_item.contents_url }}"
title="{{ path_item.relpath }}"
frameborder=0
height=1000
style="width: 100%;"
>
</iframe>
{% elif path_item.display_type == "image" %}
<div class="content-scroller">
<img src="{{ path_item.contents_url }}">
</div>
{% elif path_item.display_type == "table" %}
<div class="content-scroller">
{% include "file_browser/csv.html" with contents=path_item.contents %}
</div>
{% else %}
<div class="content-scroller">
<pre>{{ path_item.contents }}</pre>
</div>
{% endif %}
</div>
{% /card %}

{% endif %}

</div>
{% include "file_browser/contents.html" %}
</div>
</div>

Expand Down
15 changes: 13 additions & 2 deletions airlock/views/request.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.template.response import TemplateResponse
from django.urls import reverse
from django.views.decorators.http import require_http_methods
from django.views.decorators.vary import vary_on_headers

from airlock.api import Status
from airlock.file_browser_api import get_request_tree
Expand Down Expand Up @@ -35,10 +36,20 @@ def request_index(request):
)


# we return different content if it is a HTMX request.
@vary_on_headers("HX-Request")
def request_view(request, request_id: str, path: str = ""):
release_request = validate_release_request(request.user, request_id)

tree = get_request_tree(release_request, path)
template = "file_browser/index.html"
selected_only = False

if request.htmx:
template = "file_browser/contents.html"
selected_only = True

tree = get_request_tree(release_request, path, selected_only)

try:
path_item = tree.get_path(path)
except tree.PathNotFound:
Expand Down Expand Up @@ -81,7 +92,7 @@ def request_view(request, request_id: str, path: str = ""):
"release_files_url": release_files_url,
}

return TemplateResponse(request, "file_browser/index.html", context)
return TemplateResponse(request, template, context)


@require_http_methods(["GET"])
Expand Down
14 changes: 12 additions & 2 deletions airlock/views/workspace.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.template.response import TemplateResponse
from django.urls import reverse
from django.views.decorators.http import require_http_methods
from django.views.decorators.vary import vary_on_headers

from airlock.api import UrlPath
from airlock.file_browser_api import get_workspace_tree
Expand All @@ -21,10 +22,19 @@ def workspace_index(request):
return TemplateResponse(request, "workspaces.html", {"workspaces": workspaces})


# we return different content if it is a HTMX request.
@vary_on_headers("HX-Request")
def workspace_view(request, workspace_name: str, path: str = ""):
workspace = validate_workspace(request.user, workspace_name)

tree = get_workspace_tree(workspace, path)
template = "file_browser/index.html"
selected_only = False

if request.htmx:
template = "file_browser/contents.html"
selected_only = True

tree = get_workspace_tree(workspace, path, selected_only)

try:
path_item = tree.get_path(path)
Expand Down Expand Up @@ -58,7 +68,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):

return TemplateResponse(
request,
"file_browser/index.html",
template,
{
"workspace": workspace,
"root": tree,
Expand Down
Loading

0 comments on commit ae8dfc6

Please sign in to comment.