Skip to content

Commit

Permalink
fixup! split rbac tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@gagantrivedi
gagantrivedi committed Aug 22, 2024
1 parent 2d99cad commit 4a3e2ce
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 48 deletions.
63 changes: 61 additions & 2 deletions api/tests/unit/environments/test_unit_environments_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import pytest
from core.constants import STRING
from django.conf import settings
from django.contrib.contenttypes.models import ContentType
from django.urls import reverse
from flag_engine.segments.constants import EQUAL
Expand Down Expand Up @@ -552,7 +553,65 @@ def test_create_environment_without_required_metadata_returns_400(
assert "Missing required metadata field" in response.json()["metadata"][0]


def test_view_environment_with_staff__query_count_is_expected(
@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is True,
reason="Skip this test if RBAC is installed",
)
def test_view_environment_with_staff__query_count_is_expected_without_rbac(
staff_client: APIClient,
environment: Environment,
with_environment_permissions: WithEnvironmentPermissionsCallable,
project: Project,
django_assert_num_queries: DjangoAssertNumQueries,
environment_metadata_a: Metadata,
environment_metadata_b: Metadata,
required_a_environment_metadata_field: MetadataModelField,
environment_content_type: ContentType,
) -> None:
_assert_view_environment_with_staff__query_count(
staff_client,
environment,
with_environment_permissions,
project,
django_assert_num_queries,
environment_metadata_a,
environment_metadata_b,
required_a_environment_metadata_field,
environment_content_type,
expected_query_count=9,
)


@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is False,
reason="Skip this test if RBAC is not installed",
)
def test_view_environment_with_staff__query_count_is_expected_with_rbac(
staff_client: APIClient,
environment: Environment,
with_environment_permissions: WithEnvironmentPermissionsCallable,
project: Project,
django_assert_num_queries: DjangoAssertNumQueries,
environment_metadata_a: Metadata,
environment_metadata_b: Metadata,
required_a_environment_metadata_field: MetadataModelField,
environment_content_type: ContentType,
) -> None: # pragma: no cover
_assert_view_environment_with_staff__query_count(
staff_client,
environment,
with_environment_permissions,
project,
django_assert_num_queries,
environment_metadata_a,
environment_metadata_b,
required_a_environment_metadata_field,
environment_content_type,
expected_query_count=10,
)


def _assert_view_environment_with_staff__query_count(
staff_client: APIClient,
environment: Environment,
with_environment_permissions: WithEnvironmentPermissionsCallable,
Expand All @@ -562,14 +621,14 @@ def test_view_environment_with_staff__query_count_is_expected(
environment_metadata_b: Metadata,
required_a_environment_metadata_field: MetadataModelField,
environment_content_type: ContentType,
expected_query_count: int,
) -> None:
# Given
with_environment_permissions([VIEW_ENVIRONMENT])

url = reverse("api-v1:environments:environment-list")
data = {"project": project.id}

expected_query_count = 9
# When
with django_assert_num_queries(expected_query_count):
response = staff_client.get(url, data=data, content_type="application/json")
Expand Down
99 changes: 74 additions & 25 deletions api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,24 @@
from users.models import FFAdminUser


@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is True,
reason="Skip this test if RBAC is installed",
)
@pytest.mark.parametrize(
"client,is_admin_master_api_key_client, num_queries",
"client, num_queries",
[
(
lazy_fixture("admin_client"),
False,
6,
), # 1 for paging, 3 for permissions, 1 for result, 1 for getting the current live version
(
lazy_fixture("admin_master_api_key_client"),
True,
4,
), # an extra one for master_api_key
), # one for each for master_api_key
],
)
def test_list_feature_segments(
def test_list_feature_segments_without_rbac(
segment: Segment,
feature: Feature,
environment: Environment,
Expand All @@ -53,16 +55,77 @@ def test_list_feature_segments(
client: APIClient,
feature_segment: FeatureSegment,
num_queries: int,
is_admin_master_api_key_client: bool,
):
) -> None:
# Given
if (
settings.IS_RBAC_INSTALLED and not is_admin_master_api_key_client
): # pragma: no cover
num_queries += 1
base_url = reverse("api-v1:features:feature-segment-list")
url = f"{base_url}?environment={environment.id}&feature={feature.id}"
_list_feature_segment_setup_data(project, environment, feature, segment)

# When
with django_assert_num_queries(num_queries):
response = client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
response_json = response.json()
assert response_json["count"] == 3
for result in response_json["results"]:
assert result["environment"] == environment.id
assert "uuid" in result
assert "segment_name" in result
assert not result["is_feature_specific"]


@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is False,
reason="Skip this test if RBAC is not installed",
)
@pytest.mark.parametrize(
"client, num_queries",
[
(
lazy_fixture("admin_client"),
7,
), # 1 for paging, 4 for permissions, 1 for result, 1 for getting the current live version
(
lazy_fixture("admin_master_api_key_client"),
4,
), # one for each for master_api_key
],
)
def test_list_feature_segments_with_rbac(
segment: Segment,
feature: Feature,
environment: Environment,
project: Project,
django_assert_num_queries: DjangoAssertNumQueries,
client: APIClient,
feature_segment: FeatureSegment,
num_queries: int,
) -> None: # pragma: no cover
# Given
base_url = reverse("api-v1:features:feature-segment-list")
url = f"{base_url}?environment={environment.id}&feature={feature.id}"
_list_feature_segment_setup_data(project, environment, feature, segment)

# When
with django_assert_num_queries(num_queries):
response = client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
response_json = response.json()
assert response_json["count"] == 3
for result in response_json["results"]:
assert result["environment"] == environment.id
assert "uuid" in result
assert "segment_name" in result
assert not result["is_feature_specific"]


def _list_feature_segment_setup_data(
project: Project, environment: Environment, feature: Feature, segment: Segment
) -> None:
environment_2 = Environment.objects.create(
project=project, name="Test environment 2"
)
Expand All @@ -79,20 +142,6 @@ def test_list_feature_segments(
feature=feature, segment=segment, environment=environment_2
)

# When
with django_assert_num_queries(num_queries):
response = client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
response_json = response.json()
assert response_json["count"] == 3
for result in response_json["results"]:
assert result["environment"] == environment.id
assert "uuid" in result
assert "segment_name" in result
assert not result["is_feature_specific"]


@pytest.mark.parametrize(
"client",
Expand Down
120 changes: 102 additions & 18 deletions api/tests/unit/features/test_unit_features_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2676,20 +2676,62 @@ def test_update_segment_override__using_simple_feature_state_viewset__denies_upd
assert response.status_code == status.HTTP_403_FORBIDDEN


def test_list_features_n_plus_1(
@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is True,
reason="Skip this test if RBAC is installed",
)
def test_list_features_n_plus_1_without_rbac(
staff_client: APIClient,
project: Project,
feature: Feature,
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
environment: Environment,
) -> None:
# Given
with_project_permissions([VIEW_PROJECT])
num_queries = 16
_assert_list_feature_n_plus_1(
staff_client,
project,
feature,
with_project_permissions,
django_assert_num_queries,
environment,
num_queries=16,
)


@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is False,
reason="Skip this test if RBAC is not installed",
)
def test_list_features_n_plus_1_with_rbac(
staff_client: APIClient,
project: Project,
feature: Feature,
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
environment: Environment,
) -> None: # pragma: no cover
_assert_list_feature_n_plus_1(
staff_client,
project,
feature,
with_project_permissions,
django_assert_num_queries,
environment,
num_queries=17,
)

if settings.IS_RBAC_INSTALLED: # pragma: no cover
num_queries += 1

def _assert_list_feature_n_plus_1(
staff_client: APIClient,
project: Project,
feature: Feature,
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
environment: Environment,
num_queries: int,
) -> None:
with_project_permissions([VIEW_PROJECT])

base_url = reverse("api-v1:projects:project-features-list", args=[project.id])
url = f"{base_url}?environment={environment.id}"
Expand Down Expand Up @@ -2842,11 +2884,6 @@ def test_list_features_with_feature_state(
) -> None:
# Given
with_project_permissions([VIEW_PROJECT])
num_queries = 16

if settings.IS_RBAC_INSTALLED: # pragma: no cover
num_queries += 1

feature2 = Feature.objects.create(
name="another_feature", project=project, initial_value="initial_value"
)
Expand Down Expand Up @@ -2937,8 +2974,7 @@ def test_list_features_with_feature_state(
url = f"{base_url}?environment={environment.id}"

# When
with django_assert_num_queries(num_queries):
response = staff_client.get(url)
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
Expand Down Expand Up @@ -3187,8 +3223,12 @@ def test_simple_feature_state_returns_only_latest_versions(
assert response_json["count"] == 2


@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is True,
reason="Skip this test if RBAC is installed",
)
@pytest.mark.freeze_time(two_hours_ago)
def test_feature_list_last_modified_values(
def test_feature_list_last_modified_values_without_rbac(
staff_client: APIClient,
staff_user: FFAdminUser,
environment_v2_versioning: Environment,
Expand All @@ -3197,15 +3237,59 @@ def test_feature_list_last_modified_values(
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
) -> None:
_assert_feature_list_last_modified_values(
staff_client,
staff_user,
environment_v2_versioning,
project,
feature,
with_project_permissions,
django_assert_num_queries,
num_queries=18,
)


@pytest.mark.skipif(
settings.IS_RBAC_INSTALLED is False,
reason="Skip this test if RBAC is not installed",
)
@pytest.mark.freeze_time(two_hours_ago)
def test_feature_list_last_modified_values_with_rbac(
staff_client: APIClient,
staff_user: FFAdminUser,
environment_v2_versioning: Environment,
project: Project,
feature: Feature,
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
) -> None: # pragma: no cover
_assert_feature_list_last_modified_values(
staff_client,
staff_user,
environment_v2_versioning,
project,
feature,
with_project_permissions,
django_assert_num_queries,
num_queries=19,
)


def _assert_feature_list_last_modified_values(
staff_client: APIClient,
staff_user: FFAdminUser,
environment_v2_versioning: Environment,
project: Project,
feature: Feature,
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
num_queries: int,
):
# Given
# another v2 versioning environment
environment_v2_versioning_2 = Environment.objects.create(
name="environment 2", project=project, use_v2_feature_versioning=True
)
num_queries = 18

if settings.IS_RBAC_INSTALLED: # pragma: no cover
num_queries += 1

url = "{base_url}?environment={environment_id}".format(
base_url=reverse("api-v1:projects:project-features-list", args=[project.id]),
Expand Down
Loading

0 comments on commit 4a3e2ce

Please sign in to comment.