Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

986 input validation for directory names #1024

Merged
merged 14 commits into from
Dec 7, 2024
Merged
7 changes: 3 additions & 4 deletions openfecli/commands/quickrun.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pathlib

from openfecli import OFECommandPlugin
from openfecli.parameters.output import ensure_file_does_not_exist
from openfecli.parameters.output import validate_outfile
from openfecli.utils import write, print_duration, configure_logger


Expand All @@ -33,10 +33,9 @@ def _format_exception(exception) -> str:
)
@click.option(
'output', '-o', default=None,
type=click.Path(dir_okay=False, file_okay=True, writable=True,
atravitz marked this conversation as resolved.
Show resolved Hide resolved
path_type=pathlib.Path),
type=click.Path(dir_okay=False, file_okay=True, path_type=pathlib.Path),
help="output file (JSON format) for the final results",
atravitz marked this conversation as resolved.
Show resolved Hide resolved
callback=ensure_file_does_not_exist,
callback=validate_outfile,
)
@print_duration
def quickrun(transformation, work_dir, output):
Expand Down
12 changes: 10 additions & 2 deletions openfecli/parameters/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@ def get_file_and_extension(user_input, context):
return file, ext


def ensure_file_does_not_exist(ctx, param, value):
def ensure_file_does_not_exist(value):
# TODO: I believe we can replace this with click.option(file_okay=False)
if value and value.exists():
raise click.BadParameter(f"File '{value}' already exists.")
return value

def ensure_parent_dir_exists(value):
if value and not value.parent.is_dir():
raise click.BadParameter(f"Cannot write to {value}, parent directory does not exist.")

def validate_outfile(ctx, param, value):
ensure_file_does_not_exist(value)
ensure_parent_dir_exists(value)
return value

OUTPUT_FILE_AND_EXT = Option(
"-o", "--output",
Expand Down
9 changes: 8 additions & 1 deletion openfecli/tests/commands/test_quickrun.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,14 @@ def test_quickrun_output_file_exists(json_file):
assert result.exit_code == 2 # usage error
assert "File 'foo.json' already exists." in result.output


def test_quickrun_output_file_in_nonexistent_directory(json_file):
"""Should catch invalid filepaths up front."""
runner = CliRunner()
outfile = "not_dir/foo.json"
result = runner.invoke(quickrun, [json_file, '-o', outfile])
assert result.exit_code == 2
assert "Cannot write" in result.output

def test_quickrun_unit_error():
with resources.files('openfecli.tests.data') as d:
json_file = str(d / 'bad_transformation.json')
Expand Down
Loading