From 632c4ffd9cb1da273303bcd8005fff216506c795 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 25 Jan 2017 10:13:08 -0500 Subject: [PATCH] Refs #23919 -- Replaced errno checking with PEP 3151 exceptions. --- django/contrib/sessions/backends/file.py | 12 ++++----- django/core/cache/backends/filebased.py | 21 ++++++---------- django/core/files/move.py | 4 +-- django/core/files/storage.py | 32 ++++++++++-------------- django/core/files/uploadedfile.py | 12 ++++----- django/core/management/templates.py | 9 +++---- django/template/backends/dummy.py | 15 +++++------ django/template/loaders/filesystem.py | 8 ++---- tests/cache/tests.py | 2 +- tests/file_storage/tests.py | 17 ++++++------- tests/file_uploads/tests.py | 4 +-- tests/staticfiles_tests/storage.py | 6 ++--- 12 files changed, 55 insertions(+), 87 deletions(-) diff --git a/django/contrib/sessions/backends/file.py b/django/contrib/sessions/backends/file.py index 594973ec91a..83baa35ef29 100644 --- a/django/contrib/sessions/backends/file.py +++ b/django/contrib/sessions/backends/file.py @@ -1,5 +1,4 @@ import datetime -import errno import logging import os import shutil @@ -133,13 +132,12 @@ class SessionStore(SessionBase): flags |= os.O_EXCL | os.O_CREAT fd = os.open(session_file_name, flags) os.close(fd) - - except OSError as e: - if must_create and e.errno == errno.EEXIST: - raise CreateError - if not must_create and e.errno == errno.ENOENT: + except FileNotFoundError: + if not must_create: raise UpdateError - raise + except FileExistsError: + if must_create: + raise CreateError # Write the session file without interfering with other threads # or processes. By writing to an atomically generated temporary diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py index ae5b48c054f..896f798e952 100644 --- a/django/core/cache/backends/filebased.py +++ b/django/core/cache/backends/filebased.py @@ -1,5 +1,4 @@ "File-based cache backend" -import errno import glob import hashlib import os @@ -34,9 +33,8 @@ class FileBasedCache(BaseCache): with open(fname, 'rb') as f: if not self._is_expired(f): return pickle.loads(zlib.decompress(f.read())) - except IOError as e: - if e.errno != errno.ENOENT: - raise + except FileNotFoundError: + pass return default def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): @@ -64,11 +62,9 @@ class FileBasedCache(BaseCache): return try: os.remove(fname) - except OSError as e: - # ENOENT can happen if the cache file is removed (by another - # process) after the os.path.exists check. - if e.errno != errno.ENOENT: - raise + except FileNotFoundError: + # The file may have been removed by another process. + pass def has_key(self, key, version=None): fname = self._key_to_file(key, version) @@ -99,11 +95,8 @@ class FileBasedCache(BaseCache): if not os.path.exists(self._dir): try: os.makedirs(self._dir, 0o700) - except OSError as e: - if e.errno != errno.EEXIST: - raise EnvironmentError( - "Cache directory '%s' does not exist " - "and could not be created'" % self._dir) + except FileExistsError: + pass def _key_to_file(self, key, version=None): """ diff --git a/django/core/files/move.py b/django/core/files/move.py index f3972c5f984..9e6cfe76844 100644 --- a/django/core/files/move.py +++ b/django/core/files/move.py @@ -71,10 +71,10 @@ def file_move_safe(old_file_name, new_file_name, chunk_size=1024 * 64, allow_ove try: os.remove(old_file_name) - except OSError as e: + except PermissionError as e: # Certain operating systems (Cygwin and Windows) # fail when deleting opened files, ignore it. (For the # systems where this happens, temporary files will be auto-deleted # on close anyway.) - if getattr(e, 'winerror', 0) != 32 and getattr(e, 'errno', 0) != 13: + if getattr(e, 'winerror', 0) != 32: raise diff --git a/django/core/files/storage.py b/django/core/files/storage.py index a8814ddc485..df4db9f2e85 100644 --- a/django/core/files/storage.py +++ b/django/core/files/storage.py @@ -1,4 +1,3 @@ -import errno import os from datetime import datetime from urllib.parse import urljoin @@ -224,9 +223,6 @@ class FileSystemStorage(Storage): full_path = self.path(name) # Create any intermediate directories that do not exist. - # Note that there is a race between os.path.exists and os.makedirs: - # if os.makedirs fails with EEXIST, the directory was created - # concurrently, and we can continue normally. Refs #16082. directory = os.path.dirname(full_path) if not os.path.exists(directory): try: @@ -240,9 +236,11 @@ class FileSystemStorage(Storage): os.umask(old_umask) else: os.makedirs(directory) - except OSError as e: - if e.errno != errno.EEXIST: - raise + except FileNotFoundError: + # There's a race between os.path.exists() and os.makedirs(). + # If os.makedirs() fails with FileNotFoundError, the directory + # was created concurrently. + pass if not os.path.isdir(directory): raise IOError("%s exists and is not a directory." % directory) @@ -280,13 +278,10 @@ class FileSystemStorage(Storage): _file.close() else: os.close(fd) - except OSError as e: - if e.errno == errno.EEXIST: - # Ooops, the file exists. We need a new file name. - name = self.get_available_name(name) - full_path = self.path(name) - else: - raise + except FileExistsError: + # A new name is needed if the file exists. + name = self.get_available_name(name) + full_path = self.path(name) else: # OK, the file save worked. Break out of the loop. break @@ -301,13 +296,12 @@ class FileSystemStorage(Storage): assert name, "The name argument is not allowed to be empty." name = self.path(name) # If the file exists, delete it from the filesystem. - # If os.remove() fails with ENOENT, the file may have been removed - # concurrently, and it's safe to continue normally. try: os.remove(name) - except OSError as e: - if e.errno != errno.ENOENT: - raise + except FileNotFoundError: + # If os.remove() fails with FileNotFoundError, the file may have + # been removed concurrently. + pass def exists(self, name): return os.path.exists(self.path(name)) diff --git a/django/core/files/uploadedfile.py b/django/core/files/uploadedfile.py index aa1038ae783..f5ab68f0fc2 100644 --- a/django/core/files/uploadedfile.py +++ b/django/core/files/uploadedfile.py @@ -2,7 +2,6 @@ Classes representing uploaded files. """ -import errno import os from io import BytesIO @@ -71,12 +70,11 @@ class TemporaryUploadedFile(UploadedFile): def close(self): try: return self.file.close() - except OSError as e: - if e.errno != errno.ENOENT: - # Means the file was moved or deleted before the tempfile - # could unlink it. Still sets self.file.close_called and - # calls self.file.file.close() before the exception - raise + except FileNotFoundError: + # The file was moved or deleted before the tempfile could unlink + # it. Still sets self.file.close_called and calls + # self.file.file.close() before the exception. + pass class InMemoryUploadedFile(UploadedFile): diff --git a/django/core/management/templates.py b/django/core/management/templates.py index a53303ae380..772f20b93ff 100644 --- a/django/core/management/templates.py +++ b/django/core/management/templates.py @@ -1,5 +1,4 @@ import cgi -import errno import mimetypes import os import posixpath @@ -76,12 +75,10 @@ class TemplateCommand(BaseCommand): top_dir = path.join(os.getcwd(), name) try: os.makedirs(top_dir) + except FileExistsError: + raise CommandError("'%s' already exists" % top_dir) except OSError as e: - if e.errno == errno.EEXIST: - message = "'%s' already exists" % top_dir - else: - message = e - raise CommandError(message) + raise CommandError(e) else: top_dir = os.path.abspath(path.expanduser(target)) if not os.path.exists(top_dir): diff --git a/django/template/backends/dummy.py b/django/template/backends/dummy.py index ce658ec7644..992465cd02f 100644 --- a/django/template/backends/dummy.py +++ b/django/template/backends/dummy.py @@ -1,4 +1,3 @@ -import errno import string from django.conf import settings @@ -31,14 +30,12 @@ class TemplateStrings(BaseEngine): try: with open(template_file, encoding=settings.FILE_CHARSET) as fp: template_code = fp.read() - except IOError as e: - if e.errno == errno.ENOENT: - tried.append(( - Origin(template_file, template_name, self), - 'Source does not exist', - )) - continue - raise + except FileNotFoundError: + tried.append(( + Origin(template_file, template_name, self), + 'Source does not exist', + )) + continue return Template(template_code) diff --git a/django/template/loaders/filesystem.py b/django/template/loaders/filesystem.py index 658e7021438..e3dda299b3b 100644 --- a/django/template/loaders/filesystem.py +++ b/django/template/loaders/filesystem.py @@ -2,8 +2,6 @@ Wrapper for loading templates from the filesystem. """ -import errno - from django.core.exceptions import SuspiciousFileOperation from django.template import Origin, TemplateDoesNotExist from django.utils._os import safe_join @@ -24,10 +22,8 @@ class Loader(BaseLoader): try: with open(origin.name, encoding=self.engine.file_charset) as fp: return fp.read() - except IOError as e: - if e.errno == errno.ENOENT: - raise TemplateDoesNotExist(origin) - raise + except FileNotFoundError: + raise TemplateDoesNotExist(origin) def get_template_sources(self, template_name): """ diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 79a5120f715..7c61837ab0b 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -1360,7 +1360,7 @@ class FileBasedCacheTests(BaseCacheTests, TestCase): # Returns the default instead of erroring. self.assertEqual(cache.get('foo', 'baz'), 'baz') - def test_get_does_not_ignore_non_enoent_errno_values(self): + def test_get_does_not_ignore_non_filenotfound_exceptions(self): with mock.patch('builtins.open', side_effect=IOError): with self.assertRaises(IOError): cache.get('foo') diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py index d33ca67c337..ba77622067a 100644 --- a/tests/file_storage/tests.py +++ b/tests/file_storage/tests.py @@ -1,4 +1,3 @@ -import errno import os import shutil import sys @@ -416,9 +415,9 @@ class FileStorageTests(SimpleTestCase): real_makedirs(path) elif path == os.path.join(self.temp_dir, 'raced'): real_makedirs(path) - raise OSError(errno.EEXIST, 'simulated EEXIST') + raise FileNotFoundError() elif path == os.path.join(self.temp_dir, 'error'): - raise OSError(errno.EACCES, 'simulated EACCES') + raise FileExistsError() else: self.fail('unexpected argument %r' % path) @@ -433,8 +432,8 @@ class FileStorageTests(SimpleTestCase): with self.storage.open('raced/test.file') as f: self.assertEqual(f.read(), b'saved with race') - # OSErrors aside from EEXIST are still raised. - with self.assertRaises(OSError): + # Exceptions aside from FileNotFoundError are raised. + with self.assertRaises(FileExistsError): self.storage.save('error/test.file', ContentFile('not saved')) finally: os.makedirs = real_makedirs @@ -452,9 +451,9 @@ class FileStorageTests(SimpleTestCase): real_remove(path) elif path == os.path.join(self.temp_dir, 'raced.file'): real_remove(path) - raise OSError(errno.ENOENT, 'simulated ENOENT') + raise FileNotFoundError() elif path == os.path.join(self.temp_dir, 'error.file'): - raise OSError(errno.EACCES, 'simulated EACCES') + raise PermissionError() else: self.fail('unexpected argument %r' % path) @@ -469,9 +468,9 @@ class FileStorageTests(SimpleTestCase): self.storage.delete('raced.file') self.assertFalse(self.storage.exists('normal.file')) - # OSErrors aside from ENOENT are still raised. + # Exceptions aside from FileNotFoundError are raised. self.storage.save('error.file', ContentFile('delete with error')) - with self.assertRaises(OSError): + with self.assertRaises(PermissionError): self.storage.delete('error.file') finally: os.remove = real_remove diff --git a/tests/file_uploads/tests.py b/tests/file_uploads/tests.py index c7315ae1421..827083412e7 100644 --- a/tests/file_uploads/tests.py +++ b/tests/file_uploads/tests.py @@ -1,5 +1,4 @@ import base64 -import errno import hashlib import json import os @@ -564,9 +563,8 @@ class DirectoryCreationTests(SimpleTestCase): """Permission errors are not swallowed""" os.chmod(MEDIA_ROOT, 0o500) self.addCleanup(os.chmod, MEDIA_ROOT, 0o700) - with self.assertRaises(OSError) as cm: + with self.assertRaises(PermissionError): self.obj.testfile.save('foo.txt', SimpleUploadedFile('foo.txt', b'x'), save=False) - self.assertEqual(cm.exception.errno, errno.EACCES) def test_not_a_directory(self): """The correct IOError is raised when the upload directory name exists but isn't a directory""" diff --git a/tests/staticfiles_tests/storage.py b/tests/staticfiles_tests/storage.py index 79e6245f596..7a1f72c1303 100644 --- a/tests/staticfiles_tests/storage.py +++ b/tests/staticfiles_tests/storage.py @@ -1,4 +1,3 @@ -import errno import os from datetime import datetime, timedelta @@ -51,9 +50,8 @@ class PathNotImplementedStorage(storage.Storage): name = self._path(name) try: os.remove(name) - except OSError as e: - if e.errno != errno.ENOENT: - raise + except FileNotFoundError: + pass def path(self, name): raise NotImplementedError