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

Input validation to handle 502 Errors #87

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion imap_data_access/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
"l1cb",
"l1d",
"l2",
"l2pre",
"l3",
"l3a",
"l3b",
Expand Down
40 changes: 40 additions & 0 deletions imap_data_access/file_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,46 @@ def is_valid_date(input_date: str) -> bool:
except ValueError:
return False

@staticmethod
def is_valid_version(input_version: str) -> bool:
"""Check input version string is in valid format 'vXXX' or 'latest'.

Parameters
----------
input_version : str
Version to be checked.

Returns
-------
bool
Whether input version is valid or not.
"""
if input_version != "latest" and (
input_version[0] != "v" and len(input_version) != 4
):
return False
else:
return True
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved

@staticmethod
def is_valid_repointing(input_repointing: str) -> bool:
"""Check input version string is in valid format 'vXXX' or 'latest'.
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved

Parameters
----------
input_repointing : str
Repointing to be checked.

Returns
-------
bool
Whether input repointing is valid or not.
"""
if not re.fullmatch(r"repoint\d{5}", str(input_repointing)):
return False
else:
return True
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved

def construct_path(self) -> Path:
"""Construct valid path from class variables and data_dir.

Expand Down
49 changes: 49 additions & 0 deletions imap_data_access/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from urllib.parse import urlencode

import imap_data_access
from imap_data_access import file_validation

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -99,6 +100,8 @@ def download(file_path: Union[Path, str]) -> Path:
return destination


# Too many branches error
# ruff: noqa: PLR0912
def query(
*,
instrument: Optional[str] = None,
Expand Down Expand Up @@ -159,6 +162,52 @@ def query(
"At least one query parameter must be provided. "
"Run 'query -h' for more information."
)
# Check instrument name
if instrument is not None:
if instrument not in imap_data_access.VALID_INSTRUMENTS:
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(
"Not a valid instrument, please choose from "
+ ", ".join(imap_data_access.VALID_INSTRUMENTS)
)

# Check data-level
# do an if statement that checks that data_level was passed in,
# then check it against all options, l0, l1a, l1b, l2, l3 etc.
if data_level is not None:
if data_level not in imap_data_access.VALID_DATALEVELS:
raise ValueError(
"Not a valid data level, choose from "
+ ", ".join(imap_data_access.VALID_DATALEVELS)
)

# Check start-date
if start_date is not None:
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
if not file_validation.ScienceFilePath.is_valid_date(start_date):
raise ValueError("Not a valid start date, use format 'YYYYMMDD'.")

# Check end-date
if end_date is not None:
if not file_validation.ScienceFilePath.is_valid_date(end_date):
raise ValueError("Not a valid end date, use format 'YYYYMMDD'.")

# Check version make sure to include 'latest'
if version is not None:
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
if not file_validation.ScienceFilePath.is_valid_version(version):
raise ValueError("Not a valid version, use format 'vXXX'.")

# check repointing follows 'repoint00000' format
if repointing is not None:
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
if not file_validation.ScienceFilePath.is_valid_repointing(repointing):
raise ValueError(
"Not a valid repointing, use format repointing<num>,"
" where <num> is a 5 digit integer."
)

# check extension
if extension is not None:
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
if extension not in imap_data_access.VALID_FILE_EXTENSION:
raise ValueError("Not a valid extension, choose from ('pkts', 'cdf').")

url = f"{imap_data_access.config['DATA_ACCESS_URL']}"
url += f"/query?{urlencode(query_params)}"

Expand Down
60 changes: 58 additions & 2 deletions tests/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import json
import os
import re
import unittest
from io import BytesIO
from pathlib import Path
Expand Down Expand Up @@ -158,8 +159,8 @@ def test_download_already_exists(mock_urlopen: unittest.mock.MagicMock):
"descriptor": "test-description",
"start_date": "20100101",
"end_date": "20100102",
"repointing": "00001",
"version": "000",
"repointing": "repoint00001",
"version": "v000",
"extension": "pkts",
},
# Make sure not all query params are sent if they are missing
Expand Down Expand Up @@ -218,6 +219,61 @@ def test_query_bad_params(mock_urlopen: unittest.mock.MagicMock):
assert mock_urlopen.call_count == 0


@pytest.mark.parametrize(
("query_flag", "query_input", "expected_output"),
[
# All parameters should not send query
(
"instrument",
"badInput",
"Not a valid instrument, please choose from "
+ ", ".join(imap_data_access.VALID_INSTRUMENTS),
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
),
(
"data_level",
"badInput",
"Not a valid data level, choose from "
+ ", ".join(imap_data_access.VALID_DATALEVELS),
),
("start_date", "badInput", "Not a valid start date, use format 'YYYYMMDD'."),
("end_date", "badInput", "Not a valid end date, use format 'YYYYMMDD'."),
(
"repointing",
"badInput",
"Not a valid repointing, use format repointing<num>, "
"where <num> is a 5 digit integer.",
),
("version", "badInput", "Not a valid version, use format 'vXXX'."),
(
"extension",
"badInput",
re.escape("Not a valid extension, choose from ('pkts', 'cdf')."),
),
],
)
def test_bad_query_input(query_flag, query_input, expected_output):
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved
"""Test a function call to query with correct params but bad values.

Ensures correct error message is returned.

Parameters
----------
query_flag : str
correct query flag.
query_input : str
incorrect query input.
expected_output : str
Output error expected to be given.
"""
kwargs = {query_flag: query_input}

# Check if the ValueError is raised and contains the correct message
with pytest.raises(ValueError, match=expected_output):
imap_data_access.query(**kwargs)

# assert str(excinfo.value) == expected_output
daralynnrhode marked this conversation as resolved.
Show resolved Hide resolved


def test_upload_no_file(mock_urlopen: unittest.mock.MagicMock):
"""Test a call to the upload API that has no filename supplied.

Expand Down
Loading