-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: takes into account when statics not in host_url root #22
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,3 +81,6 @@ coverage.xml | |
*.pot | ||
|
||
# End of https://www.gitignore.io/api/python,osx | ||
|
||
env/ | ||
.vscode/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
import json | ||
import os | ||
|
||
from urllib.parse import urljoin | ||
|
||
from urllib.parse import urljoin, urlparse | ||
from flask import url_for as flask_url_for | ||
|
||
|
||
|
@@ -25,7 +24,8 @@ def init_app(self, app): | |
app.config.setdefault("FLASK_STATIC_DIGEST_GZIP_FILES", True) | ||
app.config.setdefault("FLASK_STATIC_DIGEST_HOST_URL", None) | ||
|
||
self.host_url = app.config.get("FLASK_STATIC_DIGEST_HOST_URL") | ||
self.host_url, self.prefix = parse_digest_host_url( | ||
app.config.get("FLASK_STATIC_DIGEST_HOST_URL")) | ||
self.static_url_path = app.static_url_path | ||
|
||
self.manifest_path = os.path.join(app.static_folder, | ||
|
@@ -46,8 +46,10 @@ def _load_manifest(self, app): | |
return manifest_dict | ||
|
||
def _prepend_host_url(self, host, filename): | ||
return urljoin(self.host_url, | ||
"/".join([self.static_url_path, filename])) | ||
parts = list( # removing empty strings | ||
filter(None, [self.prefix, self.static_url_path, filename])) | ||
|
||
return urljoin(self.host_url, "/".join(parts)) | ||
|
||
def static_url_for(self, endpoint, **values): | ||
""" | ||
|
@@ -84,3 +86,28 @@ def static_url_for(self, endpoint, **values): | |
merged_values.get("filename")) | ||
else: | ||
return flask_url_for(endpoint, **merged_values) | ||
|
||
|
||
def parse_digest_host_url(host_url_prefix): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you rename this function to |
||
""" | ||
Detect if host_url_prefix contains a path element and returns a tuple with | ||
the elements (host_part, path_part), for example for | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you re-word this line to be:
|
||
|
||
host_url_prefix = "https://cdn.example.com/myapp/some/path/" | ||
|
||
This function should return: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you change this line to:
|
||
('https://cdn.example.com', "/myapp/some/path/") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you change this line into these lines:
|
||
|
||
:param host_url_prefix: CDN like URL prefix | ||
:type host_url_prefix: str | ||
:return: tuple with the elements (host_part, path_part). | ||
""" | ||
scheme, netloc, path, _, _, _ = urlparse(host_url_prefix) | ||
|
||
if path and path.startswith("/"): | ||
path = path[1:] | ||
|
||
if path and path.endswith("/"): | ||
path = path[:-1] | ||
|
||
return (f"{scheme}://{netloc}", path) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
from flask_static_digest import parse_digest_host_url | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on replicating all of these tests without using pytest, following the same patterns used in the existing test suite? |
||
|
||
import pytest as pt | ||
|
||
|
||
@pt.mark.parametrize( | ||
"digest_host_url, expected", [ | ||
( | ||
"https://cdn.example.com", | ||
("https://cdn.example.com", "") | ||
), | ||
( | ||
"https://cdn.example.com/", | ||
("https://cdn.example.com", "") | ||
), | ||
( | ||
"https://cdn.example.com/myapp", | ||
("https://cdn.example.com", "myapp") | ||
), | ||
( | ||
"https://cdn.example.com/myapp/", | ||
("https://cdn.example.com", "myapp") | ||
), | ||
( | ||
"https://cdn.example.com/myapp/anotherdir", | ||
("https://cdn.example.com", "myapp/anotherdir") | ||
), | ||
( | ||
"https://cdn.example.com/myapp/anotherdir/", | ||
("https://cdn.example.com", "myapp/anotherdir") | ||
), | ||
] | ||
) | ||
def test_parse_function(digest_host_url, expected): | ||
assert parse_digest_host_url(digest_host_url) == expected | ||
|
||
|
||
def test_joined_urls(): | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
from flask import Flask | ||
from flask_static_digest import FlaskStaticDigest | ||
import pytest as pt | ||
import json | ||
|
||
|
||
fake_manifest = { | ||
"hey.png": "hey-HASH.png", | ||
"images/hey.png": "images/hey-HASH.png", | ||
"images/card/hey.png": "images/card/hey-HASH.png" | ||
} | ||
|
||
|
||
def make_app(tmp_path, host_url, static_url_path): | ||
appdir = tmp_path / 'fakeappdir' | ||
appdir.mkdir() | ||
|
||
staticdir = appdir / 'static' | ||
staticdir.mkdir() | ||
|
||
manifest_f = staticdir / 'cache_manifest.json' | ||
manifest_f.write_text(json.dumps(fake_manifest)) | ||
|
||
app = Flask( | ||
__name__, root_path=appdir, static_folder=staticdir, | ||
static_url_path=static_url_path) | ||
app.config['TESTING'] = True | ||
app.config['FLASK_STATIC_DIGEST_HOST_URL'] = host_url | ||
|
||
with app.app_context(): | ||
return app | ||
|
||
|
||
@pt.mark.parametrize( | ||
"host_url, static_url_path, file, expected", [ | ||
( | ||
"https://cdn.example.com", None, "hey.png", | ||
"https://cdn.example.com/static/hey-HASH.png" | ||
), | ||
( | ||
"https://cdn.example.com", None, "images/hey.png", | ||
"https://cdn.example.com/static/images/hey-HASH.png" | ||
), | ||
( | ||
"https://cdn.example.com", "/static/something/", "hey.png", | ||
"https://cdn.example.com/static/something/hey-HASH.png" | ||
), | ||
( | ||
pt.param( | ||
"https://cdn.example.com", "static/something/", "hey.png", | ||
"https://cdn.example.com/static/something/hey-HASH.png", | ||
marks=pt.mark.xfail(raises=ValueError) | ||
) | ||
), | ||
( | ||
"https://cdn.example.com/myapp", None, "images/hey.png", | ||
"https://cdn.example.com/myapp/static/images/hey-HASH.png" | ||
), | ||
( | ||
"https://cdn.example.com/myapp", '/static', "images/hey.png", | ||
"https://cdn.example.com/myapp/static/images/hey-HASH.png" | ||
), | ||
( | ||
"https://cdn.example.com/myapp", "/static/something/", "hey.png", | ||
"https://cdn.example.com/myapp/static/something/hey-HASH.png" | ||
), | ||
( | ||
"https://cdn.example.com/myapp/anotherdir", None, "images/hey.png", | ||
"https://cdn.example.com/myapp/anotherdir" | ||
"/static/images/hey-HASH.png" | ||
), | ||
(None, None, "hey.png", "/static/hey-HASH.png"), | ||
(None, '/mystatic/url', "hey.png", "/mystatic/url/hey-HASH.png"), | ||
nickjj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
] | ||
) | ||
def test_get_url(tmp_path, host_url, static_url_path, file, expected): | ||
app = make_app(tmp_path, host_url, static_url_path) | ||
ext = FlaskStaticDigest(app=app) | ||
|
||
ret = ext.static_url_for('static', filename=file) | ||
|
||
assert ret == expected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance we can write rewrite this and the line below it to be a more clear on what it does?