From d6fbe51ef59ed87d65b3e8d4c8e2d1fb8c068259 Mon Sep 17 00:00:00 2001 From: RandomJo Date: Wed, 30 Jan 2019 15:58:18 -0600 Subject: [PATCH 1/7] Fix rmtree method of S3BotoStorageMixin The listdir function returns two values, one for directory names and one for file names. These are lists of strings and thus do not have a delete method. Instead of trying to delete each array, we loop through them to build keys that we can delete. --- filebrowser_safe/storage.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/filebrowser_safe/storage.py b/filebrowser_safe/storage.py index 88d1d15..3609355 100644 --- a/filebrowser_safe/storage.py +++ b/filebrowser_safe/storage.py @@ -112,9 +112,13 @@ def makedirs(self, name): def rmtree(self, name): name = self._normalize_name(self._clean_name(name)) - dirlist = self.listdir(self._encode_name(name)) - for item in dirlist: - item.delete() + directories, files = self.listdir(self._encode_name(name)) + + for key in files: + self.delete('/'.join([name, key])) + + for dirname in directories: + self.rmtree('/'.join([name, dirname])) class GoogleStorageMixin(StorageMixin): From 6f23b88f7348d06878198bb63c2392266e7b9fc5 Mon Sep 17 00:00:00 2001 From: Tirso Rodriguez Date: Wed, 6 Mar 2019 14:19:28 +0000 Subject: [PATCH 2/7] Fixes issue with old methods in gcs support --- .gitignore | 1 + filebrowser_safe/storage.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 4d8f39b..994aaf8 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,5 @@ *.pyc *.eggs/ *.egg-info +.idea mezzanine-git/ diff --git a/filebrowser_safe/storage.py b/filebrowser_safe/storage.py index 88d1d15..adc18b8 100644 --- a/filebrowser_safe/storage.py +++ b/filebrowser_safe/storage.py @@ -4,6 +4,7 @@ # PYTHON IMPORTS import os import shutil +import posixpath # DJANGO IMPORTS from django.core.files.move import file_move_safe @@ -133,7 +134,7 @@ def isdir(self, name): return False name = self._normalize_name(self._clean_name(name)) - dirlist = self.bucket.list(self._encode_name(name)) + dirlist = self.listdir(self._encode_name(name)) # Check whether the iterator is empty for item in dirlist: @@ -163,6 +164,32 @@ def makedirs(self, name): def rmtree(self, name): name = self._normalize_name(self._clean_name(name)) - dirlist = self.bucket.list(self._encode_name(name)) + dirlist = self.listdir(self._encode_name(name)) for item in dirlist: item.delete() + + def _clean_name(self, name): + """ + Cleans the name so that Windows style paths work + """ + return clean_name(name) + + +def clean_name(name): + """ + Cleans the name so that Windows style paths work + """ + # Normalize Windows style paths + clean_name = posixpath.normpath(name).replace('\\', '/') + + # os.path.normpath() can strip trailing slashes so we implement + # a workaround here. + if name.endswith('/') and not clean_name.endswith('/'): + # Add a trailing slash as it was stripped. + clean_name = clean_name + '/' + + # Given an empty string, os.path.normpath() will return ., which we don't want + if clean_name == '.': + clean_name = '' + + return clean_name From 3b86e2868b3e0f2e198b7c128ba5ace9058f0038 Mon Sep 17 00:00:00 2001 From: Tirso Rodriguez Date: Sun, 24 Mar 2019 16:03:11 +0000 Subject: [PATCH 3/7] Fix cs specification that is breaking compilemessages command --- filebrowser_safe/locale/cs/LC_MESSAGES/django.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filebrowser_safe/locale/cs/LC_MESSAGES/django.po b/filebrowser_safe/locale/cs/LC_MESSAGES/django.po index ca8b3fc..fb66196 100644 --- a/filebrowser_safe/locale/cs/LC_MESSAGES/django.po +++ b/filebrowser_safe/locale/cs/LC_MESSAGES/django.po @@ -322,7 +322,7 @@ msgstr[1] "%(counter) výsledky" #: templates/filebrowser/include/toolbar.html:9 #, python-format msgid "%(full_result_count)s total" -msgstr "%(full_result_count) celkem" +msgstr "%(full_result_count)s celkem" #: templates/filebrowser/include/search.html:5 msgid "Clear Restrictions" From 6ec34335e124b6ea788378251ce5e35d953729da Mon Sep 17 00:00:00 2001 From: Jesse London Date: Mon, 6 May 2019 22:43:40 -0500 Subject: [PATCH 4/7] improved detection of files and directories in GoogleStorageMixin filebrowser_safe performed oddly against Google Cloud Storage, inappropriately considering files to be directories and vice-versa. the changeset handles edge cases in ``isfile()`` and correctly iterates the file+dir listing returned by ``listdir()``. --- filebrowser_safe/storage.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/filebrowser_safe/storage.py b/filebrowser_safe/storage.py index aa5b790..8391cd3 100644 --- a/filebrowser_safe/storage.py +++ b/filebrowser_safe/storage.py @@ -2,6 +2,7 @@ # coding: utf-8 # PYTHON IMPORTS +import itertools import os import shutil import posixpath @@ -125,7 +126,7 @@ def rmtree(self, name): class GoogleStorageMixin(StorageMixin): def isfile(self, name): - return self.exists(name) + return bool(name) and self.exists(self._clean_name(name).rstrip('/')) def isdir(self, name): # That's some inefficient implementation... @@ -134,13 +135,11 @@ def isdir(self, name): if not name: # Empty name is a directory return True - if self.isfile(name): - return False - name = self._normalize_name(self._clean_name(name)) - dirlist = self.listdir(self._encode_name(name)) + (dirs, files) = self.listdir(self._encode_name(name)) + dirlist = itertools.chain(dirs, files) - # Check whether the iterator is empty + # Check for contents for item in dirlist: return True return False From 9f6c2f63092720f10c753f79dfcb9cff01ab65aa Mon Sep 17 00:00:00 2001 From: Jesse London Date: Fri, 10 May 2019 13:27:14 -0500 Subject: [PATCH 5/7] further refined isfile & isdir checks in GoogleStorageMixin --- filebrowser_safe/storage.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/filebrowser_safe/storage.py b/filebrowser_safe/storage.py index 8391cd3..74757cd 100644 --- a/filebrowser_safe/storage.py +++ b/filebrowser_safe/storage.py @@ -126,7 +126,7 @@ def rmtree(self, name): class GoogleStorageMixin(StorageMixin): def isfile(self, name): - return bool(name) and self.exists(self._clean_name(name).rstrip('/')) + return bool(name) and self.exists(name) def isdir(self, name): # That's some inefficient implementation... @@ -135,8 +135,7 @@ def isdir(self, name): if not name: # Empty name is a directory return True - name = self._normalize_name(self._clean_name(name)) - (dirs, files) = self.listdir(self._encode_name(name)) + (dirs, files) = self.listdir(name) dirlist = itertools.chain(dirs, files) # Check for contents From 7f2c2330cae478f38e900dc57dcdb1ba54539a62 Mon Sep 17 00:00:00 2001 From: Jesse London Date: Fri, 10 May 2019 13:41:18 -0500 Subject: [PATCH 6/7] reinstitute `isfile` check within `isdir` check --- filebrowser_safe/storage.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/filebrowser_safe/storage.py b/filebrowser_safe/storage.py index 74757cd..7bb6e3f 100644 --- a/filebrowser_safe/storage.py +++ b/filebrowser_safe/storage.py @@ -135,6 +135,9 @@ def isdir(self, name): if not name: # Empty name is a directory return True + if self.isfile(name): + return False + (dirs, files) = self.listdir(name) dirlist = itertools.chain(dirs, files) From 896710ab6c444ccac5d09f5ee147694210ecadd4 Mon Sep 17 00:00:00 2001 From: Jesse London Date: Sat, 11 May 2019 18:36:25 -0500 Subject: [PATCH 7/7] improve file/dir detection and improve browsing performance (Google Cloud Storage) --- filebrowser_safe/base.py | 2 +- filebrowser_safe/storage.py | 35 +++++++++++++++++++++++++++++------ filebrowser_safe/views.py | 6 ++++-- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/filebrowser_safe/base.py b/filebrowser_safe/base.py index 6955f91..d24936a 100644 --- a/filebrowser_safe/base.py +++ b/filebrowser_safe/base.py @@ -47,7 +47,7 @@ def __len__(self): @cached_property def filetype(self): - if self.is_folder: + if not default_storage.type_checks_slow and self.is_folder: return 'Folder' return get_file_type(self.filename) diff --git a/filebrowser_safe/storage.py b/filebrowser_safe/storage.py index 7bb6e3f..87873b2 100644 --- a/filebrowser_safe/storage.py +++ b/filebrowser_safe/storage.py @@ -16,6 +16,7 @@ class StorageMixin(object): """ Adds some useful methods to the Storage class. """ + type_checks_slow = False def isdir(self, name): """ @@ -125,8 +126,21 @@ def rmtree(self, name): class GoogleStorageMixin(StorageMixin): + type_checks_slow = True # indicate that isfile/isdir should be avoided, + # for performance reasons, as appropriate + def isfile(self, name): - return bool(name) and self.exists(name) + # Because GCS does (semi-arbitrarily) create empty blobs for + # "folders," it's not enough to check whether the path exists; + # and, there's not (yet) any good way to differentiate these from + # proper files. + # + # It is POSSIBLE that an actual file name endswith / ... + # HOWEVER, it is unlikely, (and kind of evil). + # + # (Until then), just exclude paths out-of-hand if they're empty + # (i.e. the bucket root) OR if they end in /: + return bool(name) and not name.endswith('/') and self.exists(name) def isdir(self, name): # That's some inefficient implementation... @@ -138,16 +152,25 @@ def isdir(self, name): if self.isfile(name): return False - (dirs, files) = self.listdir(name) - dirlist = itertools.chain(dirs, files) + # rather than ``listdir()``, which retrieves all results, retrieve + # blob iterator directly, and return as soon as ANY retrieved + name = self._normalize_name(clean_name(name)) + # For bucket.list_blobs and logic below name needs to end in / + if not name.endswith('/'): + name += '/' + + iterator = self.bucket.list_blobs(prefix=self._encode_name(name), delimiter='/') + dirlist = itertools.chain(iterator, iterator.prefixes) # Check for contents - for item in dirlist: + try: + next(dirlist) + except StopIteration: + return False + else: return True - return False def move(self, old_file_name, new_file_name, allow_overwrite=False): - if self.exists(new_file_name): if allow_overwrite: self.delete(new_file_name) diff --git a/filebrowser_safe/views.py b/filebrowser_safe/views.py index 69bf1d1..b59b231 100644 --- a/filebrowser_safe/views.py +++ b/filebrowser_safe/views.py @@ -98,7 +98,9 @@ def browse(request): raise ImproperlyConfigured(_("Error finding Upload-Folder. Maybe it does not exist?")) redirect_url = reverse("fb_browse") + query_helper(query, "", "dir") return HttpResponseRedirect(redirect_url) - abs_path = os.path.join(get_directory(), path) + + base_dir = get_directory() + abs_path = os.path.join(base_dir, path) # INITIAL VARIABLES results_var = {'results_total': 0, 'results_current': 0, 'delete_total': 0, 'images_total': 0, 'select_total': 0} @@ -121,7 +123,7 @@ def browse(request): # CREATE FILEOBJECT url_path = "/".join([s.strip("/") for s in - [get_directory(), path.replace("\\", "/"), file] if s.strip("/")]) + [base_dir, path.replace("\\", "/"), file] if s.strip("/")]) fileobject = FileObject(url_path)