Merge pull request #575 from fukatani/issue/571

[breaking consistency] Unsupport --overwrite option and abort download if file already exists.

onlinejudge/_implementation/command/download.py

@@ -2,9 +2,10 @@
 import json
 import os
 import pathlib
-import sys
 from typing import *
 
+import requests.exceptions
+
 import onlinejudge
 import onlinejudge._implementation.download_history
 import onlinejudge._implementation.format_utils as format_utils
@@ -29,7 +30,7 @@ def download(args: 'argparse.Namespace') -> None:
     # prepare values
     problem = onlinejudge.dispatch.problem_from_url(args.url)
     if problem is None:
-        sys.exit(1)
+        raise requests.exceptions.InvalidURL('The contest "%s" is not supported' % args.url)
     is_default_format = args.format is None and args.directory is None  # must be here since args.directory and args.format are overwritten
     if args.directory is None:
         args.directory = pathlib.Path('test')
@@ -70,10 +71,7 @@ def download(args: 'argparse.Namespace') -> None:
             if args.dry_run:
                 continue
             if path.exists():
-                log.warning('file already exists: %s', path)
-                if not args.overwrite:
-                    log.warning('skipped')
-                    continue
+                raise FileExistsError('Failed to download since file already exists: ' + str(path))
             path.parent.mkdir(parents=True, exist_ok=True)
             with path.open('wb') as fh:
                 fh.write(data)

onlinejudge/_implementation/main.py

@@ -68,7 +68,6 @@ def get_parser() -> argparse.ArgumentParser:
     subparser.add_argument('url')
     subparser.add_argument('-f', '--format', help='a format string to specify paths of cases (default: "sample-%%i.%%e" if not --system)')  # default must be None for --system
     subparser.add_argument('-d', '--directory', type=pathlib.Path, help='a directory name for test cases (default: test/)')  # default must be None for guessing in submit command
-    subparser.add_argument('--overwrite', action='store_true')
     subparser.add_argument('-n', '--dry-run', action='store_true', help='don\'t write to files')
     subparser.add_argument('-a', '--system', action='store_true', help='download system testcases')
     subparser.add_argument('-s', '--silent', action='store_true')
@@ -308,9 +307,15 @@ def main(args: Optional[List[str]] = None) -> None:
         log.error(str(e))
         log.debug(traceback.format_exc())
         sys.exit(1)
+    except requests.exceptions.InvalidURL as e:
+        log.error(str(e))
+        sys.exit(1)
     except onlinejudge.type.SampleParseError:
         log.error('Failed to parse sample.')
         sys.exit(1)
+    except FileExistsError as e:
+        log.error(str(e))
+        sys.exit(1)
 
 
 if __name__ == '__main__':

tests/command_download.py

@@ -48,3 +48,36 @@ def snippet_call_download_raises(self, expected_exception, url, is_system=False,
     args = get_parser().parse_args(args=args)
     with self.assertRaises(expected_exception):
         download(args)
+
+
+def snippet_call_download_twice(self, url1, url2, files, is_system=False, is_silent=False, type='files'):
+    assert type in 'files' or 'json'
+    if type == 'json':
+        files = get_files_from_json(files)
+
+    with tests.utils.sandbox([]):
+        args = ['download', url1]
+        if is_system:
+            args += ['--system']
+        if is_silent:
+            args += ['--silent']
+        args = get_parser().parse_args(args=args)
+        download(args)
+
+        args = ['download', url2]
+        if is_system:
+            args += ['--system']
+        if is_silent:
+            args += ['--silent']
+        args = get_parser().parse_args(args=args)
+        # download from url2 should be aborted.
+        with self.assertRaises(FileExistsError):
+            download(args)
+
+        # check download from url1 is not overwritten
+        result = {}
+        if os.path.exists('test'):
+            for name in os.listdir('test'):
+                with open(os.path.join('test', name)) as fh:
+                    result[name] = hashlib.md5(fh.buffer.read()).hexdigest()
+        self.assertEqual(files, result)

tests/command_download_invalid.py

@@ -0,0 +1,31 @@
+import unittest
+
+import requests.exceptions
+import tests.command_download
+
+
+class DownloadInvalid(unittest.TestCase):
+    def snippet_call_download_raises(self, *args, **kwargs):
+        tests.command_download.snippet_call_download_raises(self, *args, **kwargs)
+
+    def snippet_call_download_twice(self, *args, **kwargs):
+        tests.command_download.snippet_call_download_twice(self, *args, **kwargs)
+
+    def test_call_download_invalid(self):
+        self.snippet_call_download_raises(requests.exceptions.InvalidURL, 'https://not_exist_contest.jp/tasks/001_a')
+
+    def test_call_download_twice(self):
+        self.snippet_call_download_twice('https://atcoder.jp/contests/abc114/tasks/abc114_c', 'https://atcoder.jp/contests/abc003/tasks/abc003_4', [
+            {
+                "input": "575\n",
+                "output": "4\n"
+            },
+            {
+                "input": "3600\n",
+                "output": "13\n"
+            },
+            {
+                "input": "999999999\n",
+                "output": "26484\n"
+            },
+        ], type='json')