Skip to content

Commit

Permalink
Fixed #22712 -- Avoided name shadowing of "all" in django.contrib.sta…
Browse files Browse the repository at this point in the history
…ticfiles.finders.

Co-authored-by: Natalia <[email protected]>
  • Loading branch information
avallbona and nessita committed Jun 28, 2024
1 parent dfac15d commit 0fdcf10
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 17 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ answer newbie questions, and generally made Django that much better:
Andreas Mock <[email protected]>
Andreas Pelme <[email protected]>
Andrés Torres Marroquín <[email protected]>
Andreu Vallbona Plazas <[email protected]>
Andrew Brehaut <https://brehaut.net/blog>
Andrew Clark <[email protected]>
Andrew Durdin <[email protected]>
Expand Down
84 changes: 70 additions & 14 deletions django/contrib/staticfiles/finders.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import functools
import os
import warnings

from django.apps import apps
from django.conf import settings
Expand All @@ -8,13 +9,40 @@
from django.core.exceptions import ImproperlyConfigured
from django.core.files.storage import FileSystemStorage, Storage, default_storage
from django.utils._os import safe_join
from django.utils.deprecation import RemovedInDjango60Warning
from django.utils.functional import LazyObject, empty
from django.utils.module_loading import import_string

# To keep track on which directories the finder has searched the static files.
searched_locations = []


# RemovedInDjango60Warning: When the deprecation ends, remove completely.
def _check_deprecated_find_param(class_name="", find_all=False, **kwargs):
method_name = "find" if not class_name else f"{class_name}.find"
if "all" in kwargs:
legacy_all = kwargs.pop("all")
msg = (
"Passing the `all` argument to find() is deprecated. Use `find_all` "
"instead."
)
warnings.warn(msg, RemovedInDjango60Warning, stacklevel=2)

# If both `find_all` and `all` were given, raise TypeError.
if find_all is not False:
raise TypeError(
f"{method_name}() got multiple values for argument 'find_all'"
)

find_all = legacy_all

if kwargs: # any remaining kwargs must be a TypeError
first = list(kwargs.keys()).pop()
raise TypeError(f"{method_name}() got an unexpected keyword argument '{first}'")

return find_all


class BaseFinder:
"""
A base file finder to be used for custom staticfiles finder classes.
Expand All @@ -26,12 +54,20 @@ def check(self, **kwargs):
"configured correctly."
)

def find(self, path, all=False):
# RemovedInDjango60Warning: When the deprecation ends, remove completely.
def _check_deprecated_find_param(self, **kwargs):
return _check_deprecated_find_param(
class_name=self.__class__.__qualname__, **kwargs
)

# RemovedInDjango60Warning: When the deprecation ends, replace with:
# def find(self, path, find_all=False):
def find(self, path, find_all=False, **kwargs):
"""
Given a relative file path, find an absolute file path.
If the ``all`` parameter is False (default) return only the first found
file path; if True, return a list of all found files paths.
If the ``find_all`` parameter is False (default) return only the first
found file path; if True, return a list of all found files paths.
"""
raise NotImplementedError(
"subclasses of BaseFinder must provide a find() method"
Expand Down Expand Up @@ -113,17 +149,22 @@ def check(self, **kwargs):
)
return errors

def find(self, path, all=False):
# RemovedInDjango60Warning: When the deprecation ends, replace with:
# def find(self, path, find_all=False):
def find(self, path, find_all=False, **kwargs):
"""
Look for files in the extra locations as defined in STATICFILES_DIRS.
"""
# RemovedInDjango60Warning.
if kwargs:
find_all = self._check_deprecated_find_param(find_all=find_all, **kwargs)
matches = []
for prefix, root in self.locations:
if root not in searched_locations:
searched_locations.append(root)
matched_path = self.find_location(root, path, prefix)
if matched_path:
if not all:
if not find_all:
return matched_path
matches.append(matched_path)
return matches
Expand Down Expand Up @@ -191,18 +232,23 @@ def list(self, ignore_patterns):
for path in utils.get_files(storage, ignore_patterns):
yield path, storage

def find(self, path, all=False):
# RemovedInDjango60Warning: When the deprecation ends, replace with:
# def find(self, path, find_all=False):
def find(self, path, find_all=False, **kwargs):
"""
Look for files in the app directories.
"""
# RemovedInDjango60Warning.
if kwargs:
find_all = self._check_deprecated_find_param(find_all=find_all, **kwargs)
matches = []
for app in self.apps:
app_location = self.storages[app].location
if app_location not in searched_locations:
searched_locations.append(app_location)
match = self.find_in_app(app, path)
if match:
if not all:
if not find_all:
return match
matches.append(match)
return matches
Expand Down Expand Up @@ -241,10 +287,15 @@ def __init__(self, storage=None, *args, **kwargs):
self.storage = self.storage()
super().__init__(*args, **kwargs)

def find(self, path, all=False):
# RemovedInDjango60Warning: When the deprecation ends, replace with:
# def find(self, path, find_all=False):
def find(self, path, find_all=False, **kwargs):
"""
Look for files in the default file storage, if it's local.
"""
# RemovedInDjango60Warning.
if kwargs:
find_all = self._check_deprecated_find_param(find_all=find_all, **kwargs)
try:
self.storage.path("")
except NotImplementedError:
Expand All @@ -254,7 +305,7 @@ def find(self, path, all=False):
searched_locations.append(self.storage.location)
if self.storage.exists(path):
match = self.storage.path(path)
if all:
if find_all:
match = [match]
return match
return []
Expand Down Expand Up @@ -285,26 +336,31 @@ def __init__(self, *args, **kwargs):
)


def find(path, all=False):
# RemovedInDjango60Warning: When the deprecation ends, replace with:
# def find(path, find_all=False):
def find(path, find_all=False, **kwargs):
"""
Find a static file with the given path using all enabled finders.
If ``all`` is ``False`` (default), return the first matching
If ``find_all`` is ``False`` (default), return the first matching
absolute path (or ``None`` if no match). Otherwise return a list.
"""
# RemovedInDjango60Warning.
if kwargs:
find_all = _check_deprecated_find_param(find_all=find_all, **kwargs)
searched_locations[:] = []
matches = []
for finder in get_finders():
result = finder.find(path, all=all)
if not all and result:
result = finder.find(path, find_all=find_all)
if not find_all and result:
return result
if not isinstance(result, (list, tuple)):
result = [result]
matches.extend(result)
if matches:
return matches
# No match.
return [] if all else None
return [] if find_all else None


def get_finders():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def add_arguments(self, parser):

def handle_label(self, path, **options):
verbosity = options["verbosity"]
result = finders.find(path, all=options["all"])
result = finders.find(path, find_all=options["all"])
if verbosity >= 2:
searched_locations = (
"\nLooking in the following locations:\n %s"
Expand Down
3 changes: 2 additions & 1 deletion docs/releases/5.2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,5 @@ Features deprecated in 5.2
Miscellaneous
-------------

* ...
* The ``all`` argument for the ``django.contrib.staticfiles.finders.find()``
function is deprecated in favor of the ``find_all`` argument.
82 changes: 81 additions & 1 deletion tests/staticfiles_tests/test_finders.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@
from django.contrib.staticfiles import finders, storage
from django.core.exceptions import ImproperlyConfigured
from django.test import SimpleTestCase, override_settings
from django.utils.deprecation import RemovedInDjango60Warning

from .cases import StaticFilesTestCase
from .settings import TEST_ROOT

DEPRECATION_MSG = (
"Passing the `all` argument to find() is deprecated. Use `find_all` instead."
)


class TestFinders:
"""
Expand All @@ -25,11 +30,49 @@ def test_find_first(self):

def test_find_all(self):
src, dst = self.find_all
found = self.finder.find(src, all=True)
found = self.finder.find(src, find_all=True)
found = [os.path.normcase(f) for f in found]
dst = [os.path.normcase(d) for d in dst]
self.assertEqual(found, dst)

def test_find_all_deprecated_param(self):
src, dst = self.find_all
with self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG):
found = self.finder.find(src, all=True)
found = [os.path.normcase(f) for f in found]
dst = [os.path.normcase(d) for d in dst]
self.assertEqual(found, dst)

def test_find_all_conflicting_params(self):
src, dst = self.find_all
msg = (
f"{self.finder.__class__.__qualname__}.find() got multiple values for "
"argument 'find_all'"
)
with (
self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG),
self.assertRaisesMessage(TypeError, msg),
):
self.finder.find(src, find_all=True, all=True)

def test_find_all_unexpected_params(self):
src, dst = self.find_all
msg = (
f"{self.finder.__class__.__qualname__}.find() got an unexpected keyword "
"argument 'wrong'"
)
with (
self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG),
self.assertRaisesMessage(TypeError, msg),
):
self.finder.find(src, all=True, wrong=1)

with self.assertRaisesMessage(TypeError, msg):
self.finder.find(src, find_all=True, wrong=1)

with self.assertRaisesMessage(TypeError, msg):
self.finder.find(src, wrong=1)


class TestFileSystemFinder(TestFinders, StaticFilesTestCase):
"""
Expand Down Expand Up @@ -114,6 +157,43 @@ def test_searched_locations(self):
[os.path.join(TEST_ROOT, "project", "documents")],
)

def test_searched_locations_find_all(self):
finders.find("spam", find_all=True)
self.assertEqual(
finders.searched_locations,
[os.path.join(TEST_ROOT, "project", "documents")],
)

def test_searched_locations_deprecated_all(self):
with self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG):
finders.find("spam", all=True)
self.assertEqual(
finders.searched_locations,
[os.path.join(TEST_ROOT, "project", "documents")],
)

def test_searched_locations_conflicting_params(self):
msg = "find() got multiple values for argument 'find_all'"
with (
self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG),
self.assertRaisesMessage(TypeError, msg),
):
finders.find("spam", find_all=True, all=True)

def test_searched_locations_unexpected_params(self):
msg = "find() got an unexpected keyword argument 'wrong'"
with (
self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG),
self.assertRaisesMessage(TypeError, msg),
):
finders.find("spam", all=True, wrong=1)

with self.assertRaisesMessage(TypeError, msg):
finders.find("spam", find_all=True, wrong=1)

with self.assertRaisesMessage(TypeError, msg):
finders.find("spam", wrong=1)

@override_settings(MEDIA_ROOT="")
def test_location_empty(self):
msg = (
Expand Down

0 comments on commit 0fdcf10

Please sign in to comment.