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

[DOP-13016] - add namespace_history table and its api methods #24

Merged
merged 5 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion docs/client/sync.rst
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Reference
.. currentmodule:: horizon.client.sync

.. autoclass:: HorizonClientSync
:members: authorize, ping, whoami, paginate_namespaces, get_namespace, create_namespace, update_namespace, delete_namespace, paginate_hwm, get_hwm, create_hwm, update_hwm, delete_hwm, paginate_hwm_history, retry
:members: authorize, ping, whoami, paginate_namespaces, get_namespace, create_namespace, update_namespace, delete_namespace, paginate_hwm, get_hwm, create_hwm, update_hwm, delete_hwm, paginate_hwm_history, paginate_namespace_history, retry
:member-order: bysource

.. autoclass:: RetryConfig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

Revision ID: 5c7a5c5a193b
Revises: c2d6da81f9ec
Create Date: 2024-02-26 12:05:54.273262
Create Date: 2024-02-27 13:25:07.367475

"""
import sqlalchemy as sa
Expand Down Expand Up @@ -40,7 +40,7 @@
op.execute(
"""
INSERT INTO namespace_history (namespace_id, name, description, action, changed_at, changed_by_user_id)
SELECT id, name, description, 'Deleted', now(), changed_by_user_id
SELECT id, name, description, 'Deleted', changed_at, changed_by_user_id
FROM namespace
WHERE is_deleted = TRUE
""",
Expand All @@ -51,15 +51,17 @@
""",
)
op.drop_column("namespace", "is_deleted")
op.drop_column("user", "is_deleted")
maxim-lixakov marked this conversation as resolved.
Show resolved Hide resolved


def downgrade() -> None:
op.add_column("user", sa.Column("is_deleted", sa.BOOLEAN(), autoincrement=False, nullable=False))
op.add_column(

Check warning on line 59 in horizon/backend/db/migrations/versions/2024-02-27_5c7a5c5a193b_.py

View check run for this annotation

Codecov / codecov/patch

horizon/backend/db/migrations/versions/2024-02-27_5c7a5c5a193b_.py#L58-L59

Added lines #L58 - L59 were not covered by tests
"namespace",
sa.Column("is_deleted", sa.BOOLEAN(), autoincrement=False, nullable=False, server_default="FALSE"),
)

op.execute(

Check warning on line 64 in horizon/backend/db/migrations/versions/2024-02-27_5c7a5c5a193b_.py

View check run for this annotation

Codecov / codecov/patch

horizon/backend/db/migrations/versions/2024-02-27_5c7a5c5a193b_.py#L64

Added line #L64 was not covered by tests
"""
UPDATE namespace SET is_deleted = TRUE
WHERE id IN (
Expand All @@ -67,4 +69,5 @@
)
""",
)
op.drop_index(op.f("ix__namespace_history__namespace_id"), table_name="namespace_history")
op.drop_table("namespace_history")

Check warning on line 73 in horizon/backend/db/migrations/versions/2024-02-27_5c7a5c5a193b_.py

View check run for this annotation

Codecov / codecov/patch

horizon/backend/db/migrations/versions/2024-02-27_5c7a5c5a193b_.py#L72-L73

Added lines #L72 - L73 were not covered by tests
8 changes: 0 additions & 8 deletions horizon/backend/db/mixins/deletable.py

This file was deleted.

3 changes: 1 addition & 2 deletions horizon/backend/db/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
from sqlalchemy import BigInteger, Boolean, String
from sqlalchemy.orm import Mapped, mapped_column

from horizon.backend.db.mixins.deletable import DeletableMixin
from horizon.backend.db.mixins.timestamp import TimestampMixin
from horizon.backend.db.models.base import Base


class User(Base, TimestampMixin, DeletableMixin):
class User(Base, TimestampMixin):
__tablename__ = "user"

id: Mapped[int] = mapped_column(BigInteger, primary_key=True)
Expand Down
6 changes: 2 additions & 4 deletions horizon/backend/db/repositories/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

class UserRepository(Repository[User]):
async def count(self) -> int:
return await self._count(where=[User.is_deleted.is_(False), User.is_active.is_(True)])
return await self._count(where=[User.is_active.is_(True)])

async def get_by_id(self, user_id: int) -> User:
result = await self._get_by_id(user_id, User.is_deleted.is_(False))
result = await self._get_by_id(user_id)
if result is None:
raise EntityNotFoundError("User", "id", user_id)
return result
Expand All @@ -39,6 +39,4 @@ async def get_or_create(
result = await self._get(User.username == username)
if not result:
result = await self.create(username)
elif result.is_deleted:
raise EntityNotFoundError("User", "username", username)
return result
1 change: 0 additions & 1 deletion tests/factories/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ def user_factory(**kwargs):
"id": randint(0, 10000000),
"username": random_string(),
"is_active": True,
"is_deleted": False,
}
data.update(kwargs)
return User(**data)
Expand Down
62 changes: 0 additions & 62 deletions tests/test_backend/test_auth/test_cached_ldap_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ async def test_cached_ldap_auth_get_token_creates_user(
assert created_user.created_at >= current_dt
assert created_user.updated_at >= current_dt
assert created_user.is_active
assert not created_user.is_deleted

# credentials cache is updated
query = select(CredentialsCache).where(CredentialsCache.user_id == user_id)
Expand Down Expand Up @@ -221,7 +220,6 @@ async def test_cached_ldap_auth_get_token_with_lookup_by_custom_attribute(
assert created_user.created_at >= current_dt
assert created_user.updated_at >= current_dt
assert created_user.is_active
assert not created_user.is_deleted

# credentials cache is updated, containing login == user email
query = select(CredentialsCache).where(CredentialsCache.user_id == user_id)
Expand Down Expand Up @@ -489,41 +487,6 @@ async def test_cached_ldap_auth_get_token_for_inactive_user(
assert not cache_item


@pytest.mark.parametrize("user", [{"username": "developer1", "is_deleted": True}], indirect=True)
@pytest.mark.parametrize("settings", [{"auth": {"provider": CACHED_LDAP}}], indirect=True)
async def test_cached_ldap_auth_get_token_for_deleted_user(
test_client: AsyncClient,
async_session: AsyncSession,
user: User,
):
response = await test_client.post(
"v1/auth/token",
data={
"username": user.username,
"password": "password",
},
)
assert response.status_code == 404
assert response.json() == {
"error": {
"code": "not_found",
"message": f"User with username={user.username!r} not found",
"details": {
"entity_type": "User",
"field": "username",
"value": user.username,
},
},
}

# credentials are not cached
query = select(CredentialsCache).where(CredentialsCache.user_id == user.id)
cache = await async_session.scalars(query)
cache_item = cache.one_or_none()

assert not cache_item


@pytest.mark.parametrize("settings", [{"auth": {"provider": CACHED_LDAP}}], indirect=True)
async def test_cached_ldap_auth_get_token_with_malformed_input(
test_client: AsyncClient,
Expand Down Expand Up @@ -1045,31 +1008,6 @@ async def test_cached_ldap_auth_check_missing_user(
}


@pytest.mark.parametrize("user", [{"is_deleted": True}], indirect=True)
@pytest.mark.parametrize("settings", [{"auth": {"provider": CACHED_LDAP}}], indirect=True)
async def test_cached_ldap_auth_check_disabled_user(
test_client: AsyncClient,
access_token: str,
user: User,
):
response = await test_client.get(
"v1/users/me",
headers={"Authorization": f"Bearer {access_token}"},
)
assert response.status_code == 404
assert response.json() == {
"error": {
"code": "not_found",
"message": f"User with id={user.id} not found",
"details": {
"entity_type": "User",
"field": "id",
"value": user.id,
},
},
}


@pytest.mark.parametrize("settings", [{"auth": {"provider": CACHED_LDAP}}], indirect=True)
async def test_cached_ldap_auth_check_invalid_token(
test_client: AsyncClient,
Expand Down
53 changes: 0 additions & 53 deletions tests/test_backend/test_auth/test_dummy_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ async def test_dummy_auth_get_token_creates_user(
assert created_user.created_at >= current_dt
assert created_user.updated_at >= current_dt
assert created_user.is_active
assert not created_user.is_deleted


@pytest.mark.parametrize("settings", [{"auth": {"provider": DUMMY}}], indirect=True)
Expand Down Expand Up @@ -128,33 +127,6 @@ async def test_dummy_auth_get_token_for_inactive_user(
}


@pytest.mark.parametrize("user", [{"is_deleted": True}], indirect=True)
@pytest.mark.parametrize("settings", [{"auth": {"provider": DUMMY}}], indirect=True)
async def test_dummy_auth_get_token_for_deleted_user(
test_client: AsyncClient,
user: User,
):
response = await test_client.post(
"v1/auth/token",
data={
"username": user.username,
"password": secrets.token_hex(16),
},
)
assert response.status_code == 404
assert response.json() == {
"error": {
"code": "not_found",
"message": f"User with username={user.username!r} not found",
"details": {
"entity_type": "User",
"field": "username",
"value": user.username,
},
},
}


@pytest.mark.parametrize("settings", [{"auth": {"provider": DUMMY}}], indirect=True)
async def test_dummy_auth_get_token_with_malformed_input(
test_client: AsyncClient,
Expand Down Expand Up @@ -269,31 +241,6 @@ async def test_dummy_auth_check_missing_user(
}


@pytest.mark.parametrize("user", [{"is_deleted": True}], indirect=True)
@pytest.mark.parametrize("settings", [{"auth": {"provider": DUMMY}}], indirect=True)
async def test_dummy_auth_check_disabled_user(
test_client: AsyncClient,
access_token: str,
user: User,
):
response = await test_client.get(
"v1/users/me",
headers={"Authorization": f"Bearer {access_token}"},
)
assert response.status_code == 404
assert response.json() == {
"error": {
"code": "not_found",
"message": f"User with id={user.id} not found",
"details": {
"entity_type": "User",
"field": "id",
"value": user.id,
},
},
}


@pytest.mark.parametrize("settings", [{"auth": {"provider": DUMMY}}], indirect=True)
async def test_dummy_auth_check_invalid_token(
test_client: AsyncClient,
Expand Down
54 changes: 0 additions & 54 deletions tests/test_backend/test_auth/test_ldap_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ async def test_ldap_auth_get_token_creates_user(
assert created_user.created_at >= current_dt
assert created_user.updated_at >= current_dt
assert created_user.is_active
assert not created_user.is_deleted


@pytest.mark.parametrize("user", [{"username": "developer1"}], indirect=True)
Expand Down Expand Up @@ -191,7 +190,6 @@ async def test_ldap_auth_get_token_with_lookup_by_custom_attribute(
assert created_user.created_at >= current_dt
assert created_user.updated_at >= current_dt
assert created_user.is_active
assert not created_user.is_deleted


@pytest.mark.parametrize("new_user", [{"username": "developer1"}], indirect=True)
Expand Down Expand Up @@ -403,33 +401,6 @@ async def test_ldap_auth_get_token_for_inactive_user(
}


@pytest.mark.parametrize("user", [{"username": "developer1", "is_deleted": True}], indirect=True)
@pytest.mark.parametrize("settings", [{"auth": {"provider": LDAP}}], indirect=True)
async def test_ldap_auth_get_token_for_deleted_user(
test_client: AsyncClient,
user: User,
):
response = await test_client.post(
"v1/auth/token",
data={
"username": user.username,
"password": "password",
},
)
assert response.status_code == 404
assert response.json() == {
"error": {
"code": "not_found",
"message": f"User with username={user.username!r} not found",
"details": {
"entity_type": "User",
"field": "username",
"value": user.username,
},
},
}


@pytest.mark.parametrize("settings", [{"auth": {"provider": LDAP}}], indirect=True)
async def test_ldap_auth_get_token_with_malformed_input(
test_client: AsyncClient,
Expand Down Expand Up @@ -699,31 +670,6 @@ async def test_ldap_auth_check_missing_user(
}


@pytest.mark.parametrize("user", [{"is_deleted": True}], indirect=True)
@pytest.mark.parametrize("settings", [{"auth": {"provider": LDAP}}], indirect=True)
async def test_ldap_auth_check_disabled_user(
test_client: AsyncClient,
access_token: str,
user: User,
):
response = await test_client.get(
"v1/users/me",
headers={"Authorization": f"Bearer {access_token}"},
)
assert response.status_code == 404
assert response.json() == {
"error": {
"code": "not_found",
"message": f"User with id={user.id} not found",
"details": {
"entity_type": "User",
"field": "id",
"value": user.id,
},
},
}


@pytest.mark.parametrize("settings", [{"auth": {"provider": LDAP}}], indirect=True)
async def test_ldap_auth_check_invalid_token(
test_client: AsyncClient,
Expand Down
Loading