Merge 177992c into d5e25f4

hepdata/modules/records/utils/data_files.py

@@ -387,37 +387,41 @@ def _move_files_for_record(rec_id):  # pragma: no cover
 
     os.makedirs(new_path, exist_ok=True)
 
+    is_sandbox = hep_submissions[0].status == 'sandbox'
+
     # Find all data resources
     resources = _find_all_current_dataresources(rec_id)
     for resource in resources:
-        resource_errors = _move_data_resource(resource, old_paths, new_path)
+        resource_errors = _move_data_resource(resource, old_paths, new_path,
+                                              is_sandbox=is_sandbox)
         errors.extend(resource_errors)
 
     # Move rest of files in old_paths
     for old_path in old_paths:
         for dir_name, subdir_list, file_list in os.walk(old_path):
             for filename in file_list:
-                if allowed_file(filename):
-                    full_path = os.path.join(dir_name, filename)
-                    log.debug("Found remaining file: %s" % full_path)
-                    sub_path = full_path.split(old_path + '/', 1)[1]
-                    new_file_path = os.path.join(new_path, sub_path)
-                    log.debug("Moving %s to %s" % (full_path, new_file_path))
-                    try:
-                        os.makedirs(os.path.dirname(new_file_path), exist_ok=True)
-                        shutil.move(full_path, new_file_path)
-                    except Exception as e:
-                        errors.append("Unable to move file from %s to %s\n"
-                                      "Error was: %s"
-                                      % (full_path, new_file_path, str(e)))
-
-        # Remove old directory (along with any unrecognized files that remain)
-        try:
-            shutil.rmtree(old_path)
-        except Exception as e:
-            errors.append("Unable to remove directory %s\n"
-                          "Error was: %s"
-                          % (old_path, str(e)))
+                full_path = os.path.join(dir_name, filename)
+                log.debug("Found remaining file: %s" % full_path)
+                sub_path = full_path.split(old_path + '/', 1)[1]
+                new_file_path = os.path.join(new_path, sub_path)
+                log.debug("Moving %s to %s" % (full_path, new_file_path))
+                try:
+                    os.makedirs(os.path.dirname(new_file_path), exist_ok=True)
+                    shutil.move(full_path, new_file_path)
+                except Exception as e:
+                    errors.append("Unable to move file from %s to %s\n"
+                                  "Error was: %s"
+                                  % (full_path, new_file_path, str(e)))
+
+        # Remove directories, which should be empty
+        for dirpath, _, _ in os.walk(old_path, topdown=False):
+            log.debug("Removing directory %s" % dirpath)
+            try:
+                os.rmdir(dirpath)
+            except Exception as e:
+                errors.append("Unable to remove directory %s\n"
+                              "Error was: %s"
+                              % (dirpath, str(e)))
 
     # If there's a zip file from the migration, move that to the new dir too.
     if inspire_id is not None:
@@ -445,7 +449,7 @@ def _move_files_for_record(rec_id):  # pragma: no cover
                                reply_to_address=current_app.config['ADMIN_EMAIL'])
 
 
-def _move_data_resource(resource, old_paths, new_path):  # pragma: no cover
+def _move_data_resource(resource, old_paths, new_path, is_sandbox=False):  # pragma: no cover
     errors = []
     log.debug("    Checking file %s" % resource.file_location)
 
@@ -475,8 +479,10 @@ def _move_data_resource(resource, old_paths, new_path):  # pragma: no cover
                               "Error was: %s"
                               % (resource.file_location, new_file_path, resource.id, str(e)))
 
-        elif not os.path.exists(new_file_path):
+        elif not os.path.exists(new_file_path) and not is_sandbox:
             # File does not exist in old or new locations - something has gone wrong
+            # If it's a sandbox record then it's not important, but for other records
+            # we'll flag an error.
             errors.append("File for for data resource id %s does not exist at "
                           "either old path (%s) or new path (%s)"
                           % (resource.id, resource.file_location, new_file_path))
@@ -535,19 +541,17 @@ def clean_remaining_files(synchronous=True):  # pragma: no cover
                         for sub_entry in subdir_entries:
                             if not sub_entry.name.isdigit():
                                 unknown_files.append(sub_entry.path)
-                else:
-                    # Is it a deleted sandbox entry?
-                    if len(entry.name) == 10 and \
-                            entry.name.isdigit():
-                        # This is probably a deleted sandbox entry
-                        recognised = True
-                        rec_id = int(entry.name)
-                        submission_count = HEPSubmission.query.filter_by(publication_recid=rec_id).count()
-                        if submission_count == 0:
-                            log.debug("Deleting %s" %entry.path)
-                            shutil.rmtree(entry.path)
-                        else:
-                            log.warning("Sandbox entry %s is still in old file location" % entry.name)
+                elif entry.name.isdigit():
+                    # This is probably either a deleted sandbox entry or a hangover from
+                    # a migration from hepdata.cedar.ac.uk that was later deleted
+                    recognised = True
+                    rec_id = int(entry.name)
+                    submission_count = HEPSubmission.query.filter_by(publication_recid=rec_id).count()
+                    if submission_count == 0:
+                        log.debug("Deleting %s" % entry.path)
+                        shutil.rmtree(entry.path)
+                    else:
+                        log.warning("Record id %s is still in old file location" % entry.name)
 
             if not recognised:
                 unknown_files.append(entry.path)

hepdata/modules/submission/models.py

@@ -264,7 +264,7 @@ def receive_data_resource_after_delete(mapper, connection, target):
                 log.debug('Deleting file %s' % target.file_location)
                 os.remove(target.file_location)
         else:
-            log.error("Could not remove file %s" % target.file_location)
+            log.warning("Could not remove file %s as it is not a valid file." % target.file_location)
 
 
 datareview_messages = db.Table('review_messages',