From 03d5d900bc935dbfaa352c092ebdb9463a8118b5 Mon Sep 17 00:00:00 2001
From: Kirit Thadaka <kirit.thadaka@gmail.com>
Date: Tue, 5 Dec 2017 16:18:06 +0530
Subject: [PATCH] Changed order of checks in delete_file

---
 notebook/services/contents/filemanager.py       | 17 ++++++++---------
 .../contents/tests/test_contents_api.py         |  3 +--
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py
index 19cdc8f4f..7d0c7be5d 100644
--- a/notebook/services/contents/filemanager.py
+++ b/notebook/services/contents/filemanager.py
@@ -493,15 +493,6 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
             raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path)
 
         if self.delete_to_trash:
-            if os.path.isdir(os_path):
-                listing = os.listdir(os_path)
-                # Don't send non-empty directories to trash.
-                # A directory containing only leftover checkpoints is
-                # considered empty.
-                cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None)
-                for entry in listing:
-                    if entry != cp_dir:
-                        raise web.HTTPError(400, u'Directory %s not empty' % os_path)
             self.log.debug("Sending %s to trash", os_path)
             # Looking at the code in send2trash, I don't think the errors it
             # raises let us distinguish permission errors from other errors in
@@ -510,6 +501,14 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
             return
 
         if os.path.isdir(os_path):
+            listing = os.listdir(os_path)
+            # Don't permanently delete non-empty directories.
+            # A directory containing only leftover checkpoints is
+            # considered empty.
+            cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None)
+            for entry in listing:
+                if entry != cp_dir:
+                    raise web.HTTPError(400, u'Directory %s not empty' % os_path)
             self.log.debug("Removing directory %s", os_path)
             with self.perm_to_403():
                 shutil.rmtree(os_path)
diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py
index 9b880f539..990b6ca51 100644
--- a/notebook/services/contents/tests/test_contents_api.py
+++ b/notebook/services/contents/tests/test_contents_api.py
@@ -518,8 +518,7 @@ class APITest(NotebookTestBase):
         for name in sorted(self.dirs + ['/'], key=len, reverse=True):
             listing = self.api.list(name).json()['content']
             for model in listing:
-                if os.path.exists(model['path']):
-                    self.api.delete(model['path'])
+                self.api.delete(model['path'])
         listing = self.api.list('/').json()['content']
         self.assertEqual(listing, [])