From 442e136a4b4cf4496fbbe83a2d52dfd78476bf13 Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Tue, 18 Dec 2018 10:18:51 -0500 Subject: [PATCH 1/6] Don't return filename from upload The uploader only needs to return the object_name of the uploaded object. The filename is read directly from the input, so there's no need to return it as well. --- atst/models/attachment.py | 4 ++-- atst/uploader.py | 2 +- tests/models/test_attachment.py | 1 + tests/test_uploader.py | 3 +-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/atst/models/attachment.py b/atst/models/attachment.py index 89dfe7f8..30dee524 100644 --- a/atst/models/attachment.py +++ b/atst/models/attachment.py @@ -25,12 +25,12 @@ class Attachment(Base, mixins.TimestampsMixin): @classmethod def attach(cls, fyle, resource=None, resource_id=None): try: - filename, object_name = app.uploader.upload(fyle) + object_name = app.uploader.upload(fyle) except UploadError as e: raise AttachmentError("Could not add attachment. " + str(e)) attachment = Attachment( - filename=filename, + filename=fyle.filename, object_name=object_name, resource=resource, resource_id=resource_id, diff --git a/atst/uploader.py b/atst/uploader.py index 03bc2460..5e44e2ee 100644 --- a/atst/uploader.py +++ b/atst/uploader.py @@ -35,7 +35,7 @@ class Uploader: object_name=object_name, extra={"acl": "private"}, ) - return (fyle.filename, object_name) + return object_name def download_stream(self, object_name): obj = self.container.get_object(object_name=object_name) diff --git a/tests/models/test_attachment.py b/tests/models/test_attachment.py index 00c54b88..5ddeda34 100644 --- a/tests/models/test_attachment.py +++ b/tests/models/test_attachment.py @@ -9,6 +9,7 @@ from tests.mocks import PDF_FILENAME def test_attach(pdf_upload): attachment = Attachment.attach(pdf_upload) assert attachment.filename == PDF_FILENAME + assert attachment.object_name is not None def test_attach_raises(): diff --git a/tests/test_uploader.py b/tests/test_uploader.py index 131277f5..1e8b86ba 100644 --- a/tests/test_uploader.py +++ b/tests/test_uploader.py @@ -21,8 +21,7 @@ NONPDF_FILENAME = "tests/fixtures/disa-pki.html" def test_upload(uploader, upload_dir, pdf_upload): - filename, object_name = uploader.upload(pdf_upload) - assert filename == PDF_FILENAME + object_name = uploader.upload(pdf_upload) assert os.path.isfile(os.path.join(upload_dir, object_name)) From d6ff3406ef2eedfeaa18d4a919bf17869270a92e Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Tue, 18 Dec 2018 11:00:59 -0500 Subject: [PATCH 2/6] Initial attempt at modularizing CSP integration --- atst/app.py | 14 ++------------ atst/domain/csp/__init__.py | 11 +++++++++++ atst/domain/csp/files.py | 30 ++++++++++++++++++++++++++++++ atst/models/attachment.py | 2 +- atst/routes/requests/approval.py | 2 +- 5 files changed, 45 insertions(+), 14 deletions(-) create mode 100644 atst/domain/csp/__init__.py create mode 100644 atst/domain/csp/files.py diff --git a/atst/app.py b/atst/app.py index 44c904ea..444f7032 100644 --- a/atst/app.py +++ b/atst/app.py @@ -21,9 +21,9 @@ from atst.routes.errors import make_error_pages from atst.domain.authnid.crl import CRLCache from atst.domain.auth import apply_authentication from atst.domain.authz import Authorization +from atst.domain.csp import make_csp_provider from atst.models.permissions import Permissions from atst.eda_client import MockEDAClient -from atst.uploader import Uploader from atst.utils import mailer from atst.utils.form_cache import FormCache from atst.queue import queue @@ -52,7 +52,7 @@ def make_app(config): make_crl_validator(app) register_filters(app) make_eda_client(app) - make_upload_storage(app) + make_csp_provider(app) make_mailer(app) queue.init_app(app) @@ -191,16 +191,6 @@ def make_eda_client(app): app.eda_client = MockEDAClient() -def make_upload_storage(app): - uploader = Uploader( - provider=app.config.get("STORAGE_PROVIDER"), - container=app.config.get("STORAGE_CONTAINER"), - key=app.config.get("STORAGE_KEY"), - secret=app.config.get("STORAGE_SECRET"), - ) - app.uploader = uploader - - def make_mailer(app): if app.config["DEBUG"]: mailer_connection = mailer.RedisConnection(app.redis) diff --git a/atst/domain/csp/__init__.py b/atst/domain/csp/__init__.py new file mode 100644 index 00000000..1e296d1a --- /dev/null +++ b/atst/domain/csp/__init__.py @@ -0,0 +1,11 @@ +from .files import RackspaceFiles + + +class MockCSP: + + def __init__(self, file_provider): + self.files = file_provider + + +def make_csp_provider(app): + app.csp = MockCSP(RackspaceFiles(app)) diff --git a/atst/domain/csp/files.py b/atst/domain/csp/files.py new file mode 100644 index 00000000..9f0c06d6 --- /dev/null +++ b/atst/domain/csp/files.py @@ -0,0 +1,30 @@ +from atst.uploader import Uploader + + +class Files: + def upload(self, fyle): # pragma: no cover + """Store the file object `fyle` in the CSP. This method returns the + object name that can be used to later look up the file.""" + raise NotImplementedError() + + def download(self, object_name): # pragma: no cover + """Retrieve the stored file represented by `object_name`. Returns a + file object. + """ + raise NotImplementedError() + + +class RackspaceFiles(Files): + def __init__(self, app): + self.uploader = Uploader( + provider=app.config.get("STORAGE_PROVIDER"), + container=app.config.get("STORAGE_CONTAINER"), + key=app.config.get("STORAGE_KEY"), + secret=app.config.get("STORAGE_SECRET"), + ) + + def upload(self, fyle): + return self.uploader.upload(fyle) + + def download(self, object_name): + return self.uploader.download_stream(object_name) diff --git a/atst/models/attachment.py b/atst/models/attachment.py index 30dee524..2dee056f 100644 --- a/atst/models/attachment.py +++ b/atst/models/attachment.py @@ -25,7 +25,7 @@ class Attachment(Base, mixins.TimestampsMixin): @classmethod def attach(cls, fyle, resource=None, resource_id=None): try: - object_name = app.uploader.upload(fyle) + object_name = app.csp.files.upload(fyle) except UploadError as e: raise AttachmentError("Could not add attachment. " + str(e)) diff --git a/atst/routes/requests/approval.py b/atst/routes/requests/approval.py index 338fa1c5..9ec31bc9 100644 --- a/atst/routes/requests/approval.py +++ b/atst/routes/requests/approval.py @@ -71,7 +71,7 @@ def task_order_pdf_download(request_id): request = Requests.get(g.current_user, request_id) if request.legacy_task_order and request.legacy_task_order.pdf: pdf = request.legacy_task_order.pdf - generator = app.uploader.download_stream(pdf.object_name) + generator = app.csp.files.download(pdf.object_name) return Response( generator, headers={ From d07cb11b78060031f7d7afd38842e6f22926058b Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 19 Dec 2018 10:10:21 -0500 Subject: [PATCH 3/6] Rename files interface class --- atst/domain/csp/__init__.py | 4 ++-- atst/domain/csp/files.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/atst/domain/csp/__init__.py b/atst/domain/csp/__init__.py index 1e296d1a..58434a8f 100644 --- a/atst/domain/csp/__init__.py +++ b/atst/domain/csp/__init__.py @@ -1,4 +1,4 @@ -from .files import RackspaceFiles +from .files import RackspaceFileProvider class MockCSP: @@ -8,4 +8,4 @@ class MockCSP: def make_csp_provider(app): - app.csp = MockCSP(RackspaceFiles(app)) + app.csp = MockCSP(RackspaceFileProvider(app)) diff --git a/atst/domain/csp/files.py b/atst/domain/csp/files.py index 9f0c06d6..13c7bf41 100644 --- a/atst/domain/csp/files.py +++ b/atst/domain/csp/files.py @@ -1,7 +1,7 @@ from atst.uploader import Uploader -class Files: +class FileProviderInterface: def upload(self, fyle): # pragma: no cover """Store the file object `fyle` in the CSP. This method returns the object name that can be used to later look up the file.""" @@ -14,7 +14,7 @@ class Files: raise NotImplementedError() -class RackspaceFiles(Files): +class RackspaceFileProvider(FileProviderInterface): def __init__(self, app): self.uploader = Uploader( provider=app.config.get("STORAGE_PROVIDER"), From cd920373a841e6c46b6254d778bec56b3bdef626 Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 19 Dec 2018 10:11:05 -0500 Subject: [PATCH 4/6] Refactor reporting interface with CSP --- atst/domain/csp/__init__.py | 9 +- atst/domain/csp/files.py | 4 +- atst/domain/csp/reports.py | 305 ++++++++++++++++++++++++++++++++++++ atst/domain/reports.py | 242 +--------------------------- 4 files changed, 317 insertions(+), 243 deletions(-) create mode 100644 atst/domain/csp/reports.py diff --git a/atst/domain/csp/__init__.py b/atst/domain/csp/__init__.py index 58434a8f..d2db2b2a 100644 --- a/atst/domain/csp/__init__.py +++ b/atst/domain/csp/__init__.py @@ -1,11 +1,12 @@ from .files import RackspaceFileProvider +from .reports import MockReportingProvider class MockCSP: - - def __init__(self, file_provider): - self.files = file_provider + def __init__(self, app): + self.files = RackspaceFileProvider(app) + self.reports = MockReportingProvider() def make_csp_provider(app): - app.csp = MockCSP(RackspaceFileProvider(app)) + app.csp = MockCSP(app) diff --git a/atst/domain/csp/files.py b/atst/domain/csp/files.py index 13c7bf41..d1fa07ae 100644 --- a/atst/domain/csp/files.py +++ b/atst/domain/csp/files.py @@ -2,12 +2,12 @@ from atst.uploader import Uploader class FileProviderInterface: - def upload(self, fyle): # pragma: no cover + def upload(self, fyle): # pragma: no cover """Store the file object `fyle` in the CSP. This method returns the object name that can be used to later look up the file.""" raise NotImplementedError() - def download(self, object_name): # pragma: no cover + def download(self, object_name): # pragma: no cover """Retrieve the stored file represented by `object_name`. Returns a file object. """ diff --git a/atst/domain/csp/reports.py b/atst/domain/csp/reports.py new file mode 100644 index 00000000..c052377b --- /dev/null +++ b/atst/domain/csp/reports.py @@ -0,0 +1,305 @@ +from itertools import groupby +from collections import OrderedDict +import pendulum + + +class ReportingInterface: + def monthly_totals_for_environment(environment): + """Return the monthly totals for the specified environment. + + Data should be in the format of a dictionary with the month as the key + and the spend in that month as the value. For example: + + { "01/2018": 79.85, "02/2018": 86.54 } + + """ + raise NotImplementedError() + + +class MockEnvironment: + def __init__(self, id_, env_name): + self.id = id_ + self.name = env_name + + +class MockProject: + def __init__(self, project_name, envs): + def make_env(name): + return MockEnvironment("{}_{}".format(project_name, name), name) + + self.name = project_name + self.environments = [make_env(env_name) for env_name in envs] + + +class MockReportingProvider(ReportingInterface): + MONTHLY_SPEND_BY_ENVIRONMENT = { + "LC04_Integ": { + "02/2018": 284, + "03/2018": 1210, + "04/2018": 1430, + "05/2018": 1366, + "06/2018": 1169, + "07/2018": 991, + "08/2018": 978, + "09/2018": 737, + }, + "LC04_PreProd": { + "02/2018": 812, + "03/2018": 1389, + "04/2018": 1425, + "05/2018": 1306, + "06/2018": 1112, + "07/2018": 936, + "08/2018": 921, + "09/2018": 694, + }, + "LC04_Prod": { + "02/2018": 1742, + "03/2018": 1716, + "04/2018": 1866, + "05/2018": 1809, + "06/2018": 1839, + "07/2018": 1633, + "08/2018": 1654, + "09/2018": 1103, + }, + "SF18_Integ": { + "04/2018": 1498, + "05/2018": 1400, + "06/2018": 1394, + "07/2018": 1171, + "08/2018": 1200, + "09/2018": 963, + }, + "SF18_PreProd": { + "04/2018": 1780, + "05/2018": 1667, + "06/2018": 1703, + "07/2018": 1474, + "08/2018": 1441, + "09/2018": 933, + }, + "SF18_Prod": { + "04/2018": 1686, + "05/2018": 1779, + "06/2018": 1792, + "07/2018": 1570, + "08/2018": 1539, + "09/2018": 986, + }, + "Canton_Prod": { + "05/2018": 28699, + "06/2018": 26766, + "07/2018": 22619, + "08/2018": 24090, + "09/2018": 16719, + }, + "BD04_Integ": {}, + "BD04_PreProd": { + "02/2018": 7019, + "03/2018": 3004, + "04/2018": 2691, + "05/2018": 2901, + "06/2018": 3463, + "07/2018": 3314, + "08/2018": 3432, + "09/2018": 723, + }, + "SCV18_Dev": {"05/2019": 9797}, + "Crown_CR Portal Dev": { + "03/2018": 208, + "04/2018": 457, + "05/2018": 671, + "06/2018": 136, + "07/2018": 1524, + "08/2018": 2077, + "09/2018": 1858, + }, + "Crown_CR Staging": { + "03/2018": 208, + "04/2018": 457, + "05/2018": 671, + "06/2018": 136, + "07/2018": 1524, + "08/2018": 2077, + "09/2018": 1858, + }, + "Crown_CR Portal Test 1": {"07/2018": 806, "08/2018": 1966, "09/2018": 2597}, + "Crown_Jewels Prod": {"07/2018": 806, "08/2018": 1966, "09/2018": 2597}, + "Crown_Jewels Dev": { + "03/2018": 145, + "04/2018": 719, + "05/2018": 1243, + "06/2018": 2214, + "07/2018": 2959, + "08/2018": 4151, + "09/2018": 4260, + }, + "NP02_Integ": {"08/2018": 284, "09/2018": 1210}, + "NP02_PreProd": {"08/2018": 812, "09/2018": 1389}, + "NP02_Prod": {"08/2018": 3742, "09/2018": 4716}, + "FM_Integ": {"08/2018": 1498}, + "FM_Prod": {"09/2018": 5686}, + } + + CUMULATIVE_BUDGET_AARDVARK = { + "02/2018": {"spend": 9857, "cumulative": 9857}, + "03/2018": {"spend": 7881, "cumulative": 17738}, + "04/2018": {"spend": 14010, "cumulative": 31748}, + "05/2018": {"spend": 43510, "cumulative": 75259}, + "06/2018": {"spend": 41725, "cumulative": 116_984}, + "07/2018": {"spend": 41328, "cumulative": 158_312}, + "08/2018": {"spend": 47491, "cumulative": 205_803}, + "09/2018": {"spend": 36028, "cumulative": 241_831}, + } + + CUMULATIVE_BUDGET_BELUGA = { + "08/2018": {"spend": 4838, "cumulative": 4838}, + "09/2018": {"spend": 14500, "cumulative": 19338}, + } + + REPORT_FIXTURE_MAP = { + "Aardvark": { + "cumulative": CUMULATIVE_BUDGET_AARDVARK, + "projects": [ + MockProject("LC04", ["Integ", "PreProd", "Prod"]), + MockProject("SF18", ["Integ", "PreProd", "Prod"]), + MockProject("Canton", ["Prod"]), + MockProject("BD04", ["Integ", "PreProd"]), + MockProject("SCV18", ["Dev"]), + MockProject( + "Crown", + [ + "CR Portal Dev", + "CR Staging", + "CR Portal Test 1", + "Jewels Prod", + "Jewels Dev", + ], + ), + ], + "budget": 500_000, + }, + "Beluga": { + "cumulative": CUMULATIVE_BUDGET_BELUGA, + "projects": [ + MockProject("NP02", ["Integ", "PreProd", "NP02_Prod"]), + MockProject("FM", ["Integ", "Prod"]), + ], + "budget": 70000, + }, + } + + def _sum_monthly_spend(self, data): + return sum( + [ + spend + for project in data + for env in project.environments + for spend in self.MONTHLY_SPEND_BY_ENVIRONMENT[env.id].values() + ] + ) + + def get_budget(self, workspace): + if workspace.name in self.REPORT_FIXTURE_MAP: + return self.REPORT_FIXTURE_MAP[workspace.name]["budget"] + elif workspace.request and workspace.legacy_task_order: + return workspace.legacy_task_order.budget + return 0 + + def get_total_spending(self, workspace): + if workspace.name in self.REPORT_FIXTURE_MAP: + return self._sum_monthly_spend( + self.REPORT_FIXTURE_MAP[workspace.name]["projects"] + ) + return 0 + + def _rollup_project_totals(self, data): + project_totals = {} + for project, environments in data.items(): + project_spend = [ + (month, spend) + for env in environments.values() + if env + for month, spend in env.items() + ] + project_totals[project] = { + month: sum([spend[1] for spend in spends]) + for month, spends in groupby(sorted(project_spend), lambda x: x[0]) + } + + return project_totals + + def _rollup_workspace_totals(self, project_totals): + monthly_spend = [ + (month, spend) + for project in project_totals.values() + for month, spend in project.items() + ] + workspace_totals = {} + for month, spends in groupby(sorted(monthly_spend), lambda m: m[0]): + workspace_totals[month] = sum([spend[1] for spend in spends]) + + return workspace_totals + + def monthly_totals_for_environment(self, environment_id): + """Return the monthly totals for the specified environment. + + Data should be in the format of a dictionary with the month as the key + and the spend in that month as the value. For example: + + { "01/2018": 79.85, "02/2018": 86.54 } + + """ + return self.MONTHLY_SPEND_BY_ENVIRONMENT.get(environment_id, {}) + + def monthly_totals(self, workspace): + """Return month totals rolled up by environment, project, and workspace. + + Data should returned with three top level keys, "workspace", "projects", + and "environments". + The "projects" key will have budget data per month for each project, + The "environments" key will have budget data for each environment. + The "workspace" key will be total monthly spending for the workspace. + For example: + + { + "environments": { "X-Wing": { "Prod": { "01/2018": 75.42 } } }, + "projects": { "X-Wing": { "01/2018": 75.42 } }, + "workspace": { "01/2018": 75.42 }, + } + + """ + projects = workspace.projects + if workspace.name in self.REPORT_FIXTURE_MAP: + projects = self.REPORT_FIXTURE_MAP[workspace.name]["projects"] + environments = { + project.name: { + env.name: self.monthly_totals_for_environment(env.id) + for env in project.environments + } + for project in projects + } + + project_totals = self._rollup_project_totals(environments) + workspace_totals = self._rollup_workspace_totals(project_totals) + + return { + "environments": environments, + "projects": project_totals, + "workspace": workspace_totals, + } + + def cumulative_budget(self, workspace): + if workspace.name in self.REPORT_FIXTURE_MAP: + budget_months = self.REPORT_FIXTURE_MAP[workspace.name]["cumulative"] + else: + budget_months = {} + + this_year = pendulum.now().year + all_months = OrderedDict() + for m in range(1, 13): + month_str = "{month:02d}/{year}".format(month=m, year=this_year) + all_months[month_str] = budget_months.get(month_str, None) + + return {"months": all_months} diff --git a/atst/domain/reports.py b/atst/domain/reports.py index 85858a23..47f558fb 100644 --- a/atst/domain/reports.py +++ b/atst/domain/reports.py @@ -1,249 +1,17 @@ -from itertools import groupby -from collections import OrderedDict -import pendulum - - -MONTHLY_SPEND_AARDVARK = { - "LC04": { - "Integ": { - "02/2018": 284, - "03/2018": 1210, - "04/2018": 1430, - "05/2018": 1366, - "06/2018": 1169, - "07/2018": 991, - "08/2018": 978, - "09/2018": 737, - }, - "PreProd": { - "02/2018": 812, - "03/2018": 1389, - "04/2018": 1425, - "05/2018": 1306, - "06/2018": 1112, - "07/2018": 936, - "08/2018": 921, - "09/2018": 694, - }, - "Prod": { - "02/2018": 1742, - "03/2018": 1716, - "04/2018": 1866, - "05/2018": 1809, - "06/2018": 1839, - "07/2018": 1633, - "08/2018": 1654, - "09/2018": 1103, - }, - }, - "SF18": { - "Integ": { - "04/2018": 1498, - "05/2018": 1400, - "06/2018": 1394, - "07/2018": 1171, - "08/2018": 1200, - "09/2018": 963, - }, - "PreProd": { - "04/2018": 1780, - "05/2018": 1667, - "06/2018": 1703, - "07/2018": 1474, - "08/2018": 1441, - "09/2018": 933, - }, - "Prod": { - "04/2018": 1686, - "05/2018": 1779, - "06/2018": 1792, - "07/2018": 1570, - "08/2018": 1539, - "09/2018": 986, - }, - }, - "Canton": { - "Prod": { - "05/2018": 28699, - "06/2018": 26766, - "07/2018": 22619, - "08/2018": 24090, - "09/2018": 16719, - } - }, - "BD04": { - "Integ": {}, - "PreProd": { - "02/2018": 7019, - "03/2018": 3004, - "04/2018": 2691, - "05/2018": 2901, - "06/2018": 3463, - "07/2018": 3314, - "08/2018": 3432, - "09/2018": 723, - }, - }, - "SCV18": {"Dev": {"05/2019": 9797}}, - "Crown": { - "CR Portal Dev": { - "03/2018": 208, - "04/2018": 457, - "05/2018": 671, - "06/2018": 136, - "07/2018": 1524, - "08/2018": 2077, - "09/2018": 1858, - }, - "CR Staging": { - "03/2018": 208, - "04/2018": 457, - "05/2018": 671, - "06/2018": 136, - "07/2018": 1524, - "08/2018": 2077, - "09/2018": 1858, - }, - "CR Portal Test 1": {"07/2018": 806, "08/2018": 1966, "09/2018": 2597}, - "Jewels Prod": {"07/2018": 806, "08/2018": 1966, "09/2018": 2597}, - "Jewels Dev": { - "03/2018": 145, - "04/2018": 719, - "05/2018": 1243, - "06/2018": 2214, - "07/2018": 2959, - "08/2018": 4151, - "09/2018": 4260, - }, - }, -} - -CUMULATIVE_BUDGET_AARDVARK = { - "02/2018": {"spend": 9857, "cumulative": 9857}, - "03/2018": {"spend": 7881, "cumulative": 17738}, - "04/2018": {"spend": 14010, "cumulative": 31748}, - "05/2018": {"spend": 43510, "cumulative": 75259}, - "06/2018": {"spend": 41725, "cumulative": 116_984}, - "07/2018": {"spend": 41328, "cumulative": 158_312}, - "08/2018": {"spend": 47491, "cumulative": 205_803}, - "09/2018": {"spend": 36028, "cumulative": 241_831}, -} - -MONTHLY_SPEND_BELUGA = { - "NP02": { - "Integ": {"08/2018": 284, "09/2018": 1210}, - "PreProd": {"08/2018": 812, "09/2018": 1389}, - "Prod": {"08/2018": 3742, "09/2018": 4716}, - }, - "FM": {"Integ": {"08/2018": 1498}, "Prod": {"09/2018": 5686}}, -} - -CUMULATIVE_BUDGET_BELUGA = { - "08/2018": {"spend": 4838, "cumulative": 4838}, - "09/2018": {"spend": 14500, "cumulative": 19338}, -} - -REPORT_FIXTURE_MAP = { - "Aardvark": { - "cumulative": CUMULATIVE_BUDGET_AARDVARK, - "monthly": MONTHLY_SPEND_AARDVARK, - "budget": 500_000, - }, - "Beluga": { - "cumulative": CUMULATIVE_BUDGET_BELUGA, - "monthly": MONTHLY_SPEND_BELUGA, - "budget": 70000, - }, -} - - -def _sum_monthly_spend(data): - return sum( - [ - spend - for project in data.values() - for env in project.values() - for spend in env.values() - ] - ) - - -def _derive_project_totals(data): - project_totals = {} - for project, environments in data.items(): - project_spend = [ - (month, spend) - for env in environments.values() - for month, spend in env.items() - ] - project_totals[project] = { - month: sum([spend[1] for spend in spends]) - for month, spends in groupby(sorted(project_spend), lambda x: x[0]) - } - - return project_totals - - -def _derive_workspace_totals(project_totals): - monthly_spend = [ - (month, spend) - for project in project_totals.values() - for month, spend in project.items() - ] - workspace_totals = {} - for month, spends in groupby(sorted(monthly_spend), lambda m: m[0]): - workspace_totals[month] = sum([spend[1] for spend in spends]) - - return workspace_totals +from flask import current_app class Reports: @classmethod def workspace_totals(cls, workspace): - if workspace.name in REPORT_FIXTURE_MAP: - budget = REPORT_FIXTURE_MAP[workspace.name]["budget"] - spent = _sum_monthly_spend(REPORT_FIXTURE_MAP[workspace.name]["monthly"]) - elif workspace.request and workspace.request.legacy_task_order: - ws_to = workspace.request.legacy_task_order - budget = ws_to.budget - # spent will be derived from CSP data - spent = 0 - else: - budget = 0 - spent = 0 - + budget = current_app.csp.reports.get_budget(workspace) + spent = current_app.csp.reports.get_total_spending(workspace) return {"budget": budget, "spent": spent} @classmethod def monthly_totals(cls, workspace): - if workspace.name in REPORT_FIXTURE_MAP: - environments = REPORT_FIXTURE_MAP[workspace.name]["monthly"] - else: - environments = { - project.name: {env.name: {} for env in project.environments} - for project in workspace.projects - } - - project_totals = _derive_project_totals(environments) - workspace_totals = _derive_workspace_totals(project_totals) - - return { - "environments": environments, - "projects": project_totals, - "workspace": workspace_totals, - } + return current_app.csp.reports.monthly_totals(workspace) @classmethod def cumulative_budget(cls, workspace): - if workspace.name in REPORT_FIXTURE_MAP: - budget_months = REPORT_FIXTURE_MAP[workspace.name]["cumulative"] - else: - budget_months = {} - - this_year = pendulum.now().year - all_months = OrderedDict() - for m in range(1, 13): - month_str = "{month:02d}/{year}".format(month=m, year=this_year) - all_months[month_str] = budget_months.get(month_str, None) - - return {"months": all_months} + return current_app.csp.reports.cumulative_budget(workspace) From e432da0d508660155c77ffc53a003f7ec5a1b99b Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Wed, 2 Jan 2019 17:12:55 -0500 Subject: [PATCH 5/6] Refactor to remove Uploader in favor of RackspaceFileProvider --- atst/domain/csp/files.py | 49 ++++++++++++++++-- atst/domain/exceptions.py | 4 ++ atst/models/attachment.py | 3 +- atst/uploader.py | 50 ------------------- config/test.ini | 1 + .../csp/test_files.py} | 20 ++++---- 6 files changed, 60 insertions(+), 67 deletions(-) rename tests/{test_uploader.py => domain/csp/test_files.py} (70%) diff --git a/atst/domain/csp/files.py b/atst/domain/csp/files.py index d1fa07ae..9fbb545e 100644 --- a/atst/domain/csp/files.py +++ b/atst/domain/csp/files.py @@ -1,7 +1,27 @@ -from atst.uploader import Uploader +from tempfile import NamedTemporaryFile +from uuid import uuid4 + +from libcloud.storage.types import Provider +from libcloud.storage.providers import get_driver + +from atst.domain.exceptions import UploadError class FileProviderInterface: + _PERMITTED_MIMETYPES = ["application/pdf"] + + def _enforce_mimetype(self, fyle): + # TODO: for hardening, we should probably use a better library for + # determining mimetype and not rely on FileUpload's determination + # TODO: we should set MAX_CONTENT_LENGTH in the config to prevent large + # uploads + if not fyle.mimetype in self._PERMITTED_MIMETYPES: + raise UploadError( + "could not upload {} with mimetype {}".format( + fyle.filename, fyle.mimetype + ) + ) + def upload(self, fyle): # pragma: no cover """Store the file object `fyle` in the CSP. This method returns the object name that can be used to later look up the file.""" @@ -16,15 +36,36 @@ class FileProviderInterface: class RackspaceFileProvider(FileProviderInterface): def __init__(self, app): - self.uploader = Uploader( + self.container = self._get_container( provider=app.config.get("STORAGE_PROVIDER"), container=app.config.get("STORAGE_CONTAINER"), key=app.config.get("STORAGE_KEY"), secret=app.config.get("STORAGE_SECRET"), ) + def _get_container(self, provider, container=None, key=None, secret=None): + if provider == "LOCAL": # pragma: no branch + key = container + container = "" + + driver = get_driver(getattr(Provider, provider))(key=key, secret=secret) + return driver.get_container(container) + def upload(self, fyle): - return self.uploader.upload(fyle) + self._enforce_mimetype(fyle) + + object_name = uuid4().hex + with NamedTemporaryFile() as tempfile: + tempfile.write(fyle.stream.read()) + self.container.upload_object( + file_path=tempfile.name, + object_name=object_name, + extra={"acl": "private"}, + ) + return object_name def download(self, object_name): - return self.uploader.download_stream(object_name) + obj = self.container.get_object(object_name=object_name) + with NamedTemporaryFile() as tempfile: + obj.download(tempfile.name, overwrite_existing=True) + return open(tempfile.name, "rb") diff --git a/atst/domain/exceptions.py b/atst/domain/exceptions.py index ec574232..bad0d4b3 100644 --- a/atst/domain/exceptions.py +++ b/atst/domain/exceptions.py @@ -30,3 +30,7 @@ class UnauthenticatedError(Exception): @property def message(self): return str(self) + + +class UploadError(Exception): + pass diff --git a/atst/models/attachment.py b/atst/models/attachment.py index 2dee056f..e4a9d6c2 100644 --- a/atst/models/attachment.py +++ b/atst/models/attachment.py @@ -5,8 +5,7 @@ from flask import current_app as app from atst.models import Base, types, mixins from atst.database import db -from atst.uploader import UploadError -from atst.domain.exceptions import NotFoundError +from atst.domain.exceptions import NotFoundError, UploadError class AttachmentError(Exception): diff --git a/atst/uploader.py b/atst/uploader.py index 5e44e2ee..139597f9 100644 --- a/atst/uploader.py +++ b/atst/uploader.py @@ -1,52 +1,2 @@ -from tempfile import NamedTemporaryFile -from uuid import uuid4 - -from libcloud.storage.types import Provider -from libcloud.storage.providers import get_driver -class UploadError(Exception): - pass - - -class Uploader: - _PERMITTED_MIMETYPES = ["application/pdf"] - - def __init__(self, provider, container=None, key=None, secret=None): - self.container = self._get_container(provider, container, key, secret) - - def upload(self, fyle): - # TODO: for hardening, we should probably use a better library for - # determining mimetype and not rely on FileUpload's determination - # TODO: we should set MAX_CONTENT_LENGTH in the config to prevent large - # uploads - if not fyle.mimetype in self._PERMITTED_MIMETYPES: - raise UploadError( - "could not upload {} with mimetype {}".format( - fyle.filename, fyle.mimetype - ) - ) - - object_name = uuid4().hex - with NamedTemporaryFile() as tempfile: - tempfile.write(fyle.stream.read()) - self.container.upload_object( - file_path=tempfile.name, - object_name=object_name, - extra={"acl": "private"}, - ) - return object_name - - def download_stream(self, object_name): - obj = self.container.get_object(object_name=object_name) - with NamedTemporaryFile() as tempfile: - obj.download(tempfile.name, overwrite_existing=True) - return open(tempfile.name, "rb") - - def _get_container(self, provider, container, key, secret): - if provider == "LOCAL": - key = container - container = "" - - driver = get_driver(getattr(Provider, provider))(key=key, secret=secret) - return driver.get_container(container) diff --git a/config/test.ini b/config/test.ini index fcb43e48..3da77886 100644 --- a/config/test.ini +++ b/config/test.ini @@ -3,3 +3,4 @@ ENVIRONMENT = test PGDATABASE = atat_test CRL_DIRECTORY = tests/fixtures/crl WTF_CSRF_ENABLED = false +STORAGE_PROVIDER=LOCAL diff --git a/tests/test_uploader.py b/tests/domain/csp/test_files.py similarity index 70% rename from tests/test_uploader.py rename to tests/domain/csp/test_files.py index 1e8b86ba..0f50cd11 100644 --- a/tests/test_uploader.py +++ b/tests/domain/csp/test_files.py @@ -2,26 +2,23 @@ import os import pytest from werkzeug.datastructures import FileStorage -from atst.uploader import Uploader, UploadError +from atst.domain.csp.files import RackspaceFileProvider +from atst.domain.exceptions import UploadError from tests.mocks import PDF_FILENAME -@pytest.fixture(scope="function") -def upload_dir(tmpdir): - return tmpdir.mkdir("uploads") - - @pytest.fixture -def uploader(upload_dir): - return Uploader("LOCAL", container=upload_dir) +def uploader(app): + return RackspaceFileProvider(app) NONPDF_FILENAME = "tests/fixtures/disa-pki.html" -def test_upload(uploader, upload_dir, pdf_upload): +def test_upload(app, uploader, pdf_upload): object_name = uploader.upload(pdf_upload) + upload_dir = app.config["STORAGE_CONTAINER"] assert os.path.isfile(os.path.join(upload_dir, object_name)) @@ -32,17 +29,18 @@ def test_upload_fails_for_non_pdfs(uploader): uploader.upload(fs) -def test_download_stream(upload_dir, uploader, pdf_upload): +def test_download(app, uploader, pdf_upload): # write pdf content to upload file storage and make sure it is flushed to # disk pdf_upload.seek(0) pdf_content = pdf_upload.read() pdf_upload.close() + upload_dir = app.config["STORAGE_CONTAINER"] full_path = os.path.join(upload_dir, "abc") with open(full_path, "wb") as output_file: output_file.write(pdf_content) output_file.flush() - stream = uploader.download_stream("abc") + stream = uploader.download("abc") stream_content = b"".join([b for b in stream]) assert pdf_content == stream_content From 1517de922d60932236d297af2b8a393c26cf4906 Mon Sep 17 00:00:00 2001 From: Patrick Smith Date: Thu, 3 Jan 2019 09:43:37 -0500 Subject: [PATCH 6/6] Remove unnecessary empty file --- atst/uploader.py | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 atst/uploader.py diff --git a/atst/uploader.py b/atst/uploader.py deleted file mode 100644 index 139597f9..00000000 --- a/atst/uploader.py +++ /dev/null @@ -1,2 +0,0 @@ - -