diff --git a/.secrets.baseline b/.secrets.baseline index 05921baa..24361804 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2019-12-06T21:22:07Z", + "generated_at": "2019-12-13T20:38:57Z", "plugins_used": [ { "base64_limit": 4.5, @@ -170,7 +170,7 @@ "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", "is_secret": false, "is_verified": false, - "line_number": 656, + "line_number": 659, "type": "Hex High Entropy String" } ] diff --git a/alembic/versions/c487d91f1a26_add_application_name_and_portfolio_id_.py b/alembic/versions/c487d91f1a26_add_application_name_and_portfolio_id_.py new file mode 100644 index 00000000..a0c32c2c --- /dev/null +++ b/alembic/versions/c487d91f1a26_add_application_name_and_portfolio_id_.py @@ -0,0 +1,28 @@ +"""add application name and portfolio_id unique constraint + +Revision ID: c487d91f1a26 +Revises: 3bd8552f1c57 +Create Date: 2019-12-13 14:33:23.952450 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c487d91f1a26' # pragma: allowlist secret +down_revision = '3bd8552f1c57' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint('name_portfolio_id_unique_constraint', 'applications', ['name', 'portfolio_id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('name_portfolio_id_unique_constraint', 'applications', type_='unique') + # ### end Alembic commands ### diff --git a/atst/domain/applications.py b/atst/domain/applications.py index 21e8782b..39cf7251 100644 --- a/atst/domain/applications.py +++ b/atst/domain/applications.py @@ -1,4 +1,7 @@ +from sqlalchemy.exc import IntegrityError + from . import BaseDomainClass +from .exceptions import AlreadyExistsError from flask import g from atst.database import db from atst.domain.application_roles import ApplicationRoles @@ -28,7 +31,12 @@ class Applications(BaseDomainClass): if environment_names: Environments.create_many(user, application, environment_names) - db.session.commit() + try: + db.session.commit() + except IntegrityError: + db.session.rollback() + raise AlreadyExistsError("application") + return application @classmethod @@ -54,7 +62,12 @@ class Applications(BaseDomainClass): g.current_user, application, new_data["environment_names"] ) db.session.add(application) - db.session.commit() + + try: + db.session.commit() + except IntegrityError: + db.session.rollback() + raise AlreadyExistsError("application") return application diff --git a/atst/models/application.py b/atst/models/application.py index b6cff6bd..415e5694 100644 --- a/atst/models/application.py +++ b/atst/models/application.py @@ -1,4 +1,4 @@ -from sqlalchemy import and_, Column, ForeignKey, String +from sqlalchemy import and_, Column, ForeignKey, String, UniqueConstraint from sqlalchemy.orm import relationship, synonym from atst.models.base import Base @@ -34,6 +34,11 @@ class Application( ), ) members = synonym("roles") + __table_args__ = ( + UniqueConstraint( + "name", "portfolio_id", name="name_portfolio_id_unique_constraint" + ), + ) @property def users(self): diff --git a/tests/domain/test_applications.py b/tests/domain/test_applications.py index 66f4ee01..50ae8038 100644 --- a/tests/domain/test_applications.py +++ b/tests/domain/test_applications.py @@ -5,7 +5,7 @@ from atst.models import CSPRole, ApplicationRoleStatus from atst.domain.application_roles import ApplicationRoles from atst.domain.applications import Applications from atst.domain.environment_roles import EnvironmentRoles -from atst.domain.exceptions import NotFoundError +from atst.domain.exceptions import AlreadyExistsError, NotFoundError from atst.domain.permission_sets import PermissionSets from tests.factories import ( @@ -177,3 +177,22 @@ def test_invite_to_nonexistent_environment(): {"environment_id": uuid4(), "role": CSPRole.BASIC_ACCESS.value}, ], ) + + +def test_create_does_not_duplicate_names_within_portfolio(): + portfolio = PortfolioFactory.create() + name = "An Awesome Application" + + assert Applications.create(portfolio.owner, portfolio, name, "") + with pytest.raises(AlreadyExistsError): + Applications.create(portfolio.owner, portfolio, name, "") + + +def test_update_does_not_duplicate_names_within_portfolio(): + portfolio = PortfolioFactory.create() + name = "An Awesome Application" + application = ApplicationFactory.create(portfolio=portfolio, name=name) + dupe_application = ApplicationFactory.create(portfolio=portfolio) + + with pytest.raises(AlreadyExistsError): + Applications.update(dupe_application, {"name": name}) diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index 41e3fc81..0390f69f 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -1,4 +1,5 @@ import pytest +import random from uuid import uuid4 from atst.domain.exceptions import NotFoundError, UnauthorizedError @@ -97,7 +98,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_admin( Applications.create( portfolio.owner, portfolio, - "My Application", + "My Application %s" % (random.randrange(1, 1000)), "My application", ["dev", "staging", "prod"], ) @@ -120,7 +121,7 @@ def test_scoped_portfolio_returns_all_applications_for_portfolio_owner( Applications.create( portfolio.owner, portfolio, - "My Application", + "My Application %s" % (random.randrange(1, 1000)), "My application", ["dev", "staging", "prod"], ) diff --git a/tests/test_access.py b/tests/test_access.py index 19aaf749..ad4bd5be 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -1,6 +1,7 @@ from unittest.mock import Mock import pytest +import random from flask import url_for, Response @@ -264,26 +265,28 @@ def test_applications_post_application_step_1(post_url_assert_status): rando = user_with() portfolio = PortfolioFactory.create(owner=owner) application = ApplicationFactory.create(portfolio=portfolio) - step_1_form_data = { - "name": "Test Application", - "description": "This is only a test", - } + + def _form_data(): + return { + "name": "Test Application %s" % (random.randrange(1, 1000)), + "description": "This is only a test", + } url = url_for( "applications.create_new_application_step_1", portfolio_id=portfolio.id ) - post_url_assert_status(ccpo, url, 302, data=step_1_form_data) - post_url_assert_status(owner, url, 302, data=step_1_form_data) - post_url_assert_status(rando, url, 404, data=step_1_form_data) + post_url_assert_status(ccpo, url, 302, data=_form_data()) + post_url_assert_status(owner, url, 302, data=_form_data()) + post_url_assert_status(rando, url, 404, data=_form_data()) url = url_for( "applications.update_new_application_step_1", portfolio_id=portfolio.id, application_id=application.id, ) - post_url_assert_status(ccpo, url, 302, data=step_1_form_data) - post_url_assert_status(owner, url, 302, data=step_1_form_data) - post_url_assert_status(rando, url, 404, data=step_1_form_data) + post_url_assert_status(ccpo, url, 302, data=_form_data()) + post_url_assert_status(owner, url, 302, data=_form_data()) + post_url_assert_status(rando, url, 404, data=_form_data()) # applications.view_new_application_step_2