From 6ead2f130d2a68c152b85acfcf835f7eb2f83c90 Mon Sep 17 00:00:00 2001 From: Sergey Natalenko Date: Thu, 9 May 2024 19:44:58 +0300 Subject: [PATCH] Add logging --- docker-compose.yaml | 2 + lms/admin/views/pages/distribution.py | 2 +- lms/db/repositories/product.py | 6 +- lms/db/repositories/reviewer.py | 29 ++++++-- lms/db/repositories/student_product.py | 12 ++-- lms/generals/models/reviewer.py | 18 +++++ lms/logic/distribute_homeworks.py | 99 +++++++++++++------------- lms/rest/api/v1/product/router.py | 2 +- lms/rest/args.py | 2 +- lms/rest/service.py | 47 ++++++++---- lms/utils/distribution/models.py | 8 +++ tests/api/product/test_read_by_id.py | 36 +++++++--- tests/conftest.py | 9 +-- tests/plugins/factories/reviewer.py | 33 +++++++++ tests/plugins/instances/database.py | 10 ++- tests/reviewers/__init__.py | 0 tests/reviewers/test_repository.py | 49 +++++++++++++ 17 files changed, 268 insertions(+), 96 deletions(-) create mode 100644 tests/plugins/factories/reviewer.py create mode 100644 tests/reviewers/__init__.py create mode 100644 tests/reviewers/test_repository.py diff --git a/docker-compose.yaml b/docker-compose.yaml index 3384dac..365c279 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -10,6 +10,8 @@ services: APP_API_ADDRESS: 0.0.0.0 APP_API_PORT: 80 + APP_LOG_FORMAT: plain + APP_API_SECRET_KEY: ${SECRET_KEY} APP_PG_DSN: postgresql+asyncpg://${POSTGRES_USER}:${POSTGRES_PASSWORD}@database:5432/${POSTGRES_DB} diff --git a/lms/admin/views/pages/distribution.py b/lms/admin/views/pages/distribution.py index 27c145c..741bc60 100644 --- a/lms/admin/views/pages/distribution.py +++ b/lms/admin/views/pages/distribution.py @@ -66,7 +66,7 @@ async def _call_create_distribution(self, params: DistributionParams) -> None: async def _get_product_list(self) -> Sequence[Product]: async with self.session_factory() as session: now = datetime.now() - return await ProductRepository(session=session).read_actual_list(dt=now) + return await ProductRepository(session=session).get_actual_list(dt=now) @cached_property def token(self) -> str: diff --git a/lms/db/repositories/product.py b/lms/db/repositories/product.py index 23d5776..8453372 100644 --- a/lms/db/repositories/product.py +++ b/lms/db/repositories/product.py @@ -46,11 +46,11 @@ async def find_product_by_offer(self, offer_id: int) -> Product: except NoResultFound as e: raise ProductNotFoundError from e - async def read_actual_list(self, dt: datetime) -> Sequence[Product]: + async def get_actual_list(self, dt: datetime) -> Sequence[Product]: stmt = ( select(ProductDb) .where(ProductDb.end_date > dt.date()) .order_by(ProductDb.created_at) ) - objs = (await self._session.scalars(stmt)).all() - return [Product.model_validate(obj) for obj in objs] + result = await self._session.scalars(stmt) + return [Product.model_validate(obj) for obj in result] diff --git a/lms/db/repositories/reviewer.py b/lms/db/repositories/reviewer.py index e07b731..df172e5 100644 --- a/lms/db/repositories/reviewer.py +++ b/lms/db/repositories/reviewer.py @@ -1,18 +1,37 @@ from collections.abc import Sequence -from sqlalchemy import select +from sqlalchemy import insert, select from sqlalchemy.ext.asyncio import AsyncSession from lms.db.models import Reviewer as ReviewerDb from lms.db.repositories.base import Repository -from lms.generals.models.reviewer import Reviewer +from lms.generals.models.reviewer import CreateReviewerModel, Reviewer class ReviewerRepository(Repository[ReviewerDb]): def __init__(self, session: AsyncSession) -> None: super().__init__(model=ReviewerDb, session=session) - async def get_by_subject_id( + async def create(self, new_reviewer: CreateReviewerModel) -> Reviewer: + query = ( + insert(ReviewerDb) + .values( + subject_id=new_reviewer.subject_id, + first_name=new_reviewer.first_name, + laste_name=new_reviewer.last_name, + email=new_reviewer.email, + desired=new_reviewer.desired, + max_=new_reviewer.max_, + min_=new_reviewer.min_, + abs_max=new_reviewer.abs_max, + is_active=new_reviewer.is_active, + ) + .returning(ReviewerDb) + ) + result = await self._session.scalars(query) + return Reviewer.model_validate(result.one()) + + async def get_list_by_subject_id( self, subject_id: int, is_acitve: bool = True ) -> Sequence[Reviewer]: query = ( @@ -23,5 +42,5 @@ async def get_by_subject_id( ) .order_by(ReviewerDb.id) ) - objs = (await self._session.scalars(query)).all() - return [Reviewer.model_validate(obj) for obj in objs] + result = await self._session.scalars(query) + return [Reviewer.model_validate(obj) for obj in result] diff --git a/lms/db/repositories/student_product.py b/lms/db/repositories/student_product.py index 4a5f022..17ea0fb 100644 --- a/lms/db/repositories/student_product.py +++ b/lms/db/repositories/student_product.py @@ -1,4 +1,4 @@ -from collections.abc import Iterable, Sequence +from collections.abc import Iterable, Mapping from typing import Any, NamedTuple from sqlalchemy import func, insert, select @@ -110,7 +110,7 @@ async def distribute_data( self, subject_id: int, vk_ids: Iterable[int], - ) -> Sequence[StudentDistributeData]: + ) -> Mapping[int, StudentDistributeData]: query = ( select( StudentDb.vk_id, @@ -127,6 +127,8 @@ async def distribute_data( StudentDb.vk_id.in_(vk_ids), ) ) - return [ - StudentDistributeData(*res) for res in await self._session.execute(query) - ] + student_data_map = {} + for res in await self._session.execute(query): + data = StudentDistributeData(*res) + student_data_map[data.vk_id] = data + return student_data_map diff --git a/lms/generals/models/reviewer.py b/lms/generals/models/reviewer.py index e16d77f..a7d724e 100644 --- a/lms/generals/models/reviewer.py +++ b/lms/generals/models/reviewer.py @@ -1,6 +1,20 @@ from pydantic import BaseModel, ConfigDict +class CreateReviewerModel(BaseModel): + model_config = ConfigDict(from_attributes=True) + + first_name: str + last_name: str + subject_id: int + email: str + desired: int + max_: int + min_: int + abs_max: int + is_active: bool + + class Reviewer(BaseModel): model_config = ConfigDict(from_attributes=True) @@ -14,3 +28,7 @@ class Reviewer(BaseModel): min_: int abs_max: int is_active: bool + + @property + def name(self) -> str: + return f"{self.first_name} {self.last_name}" diff --git a/lms/logic/distribute_homeworks.py b/lms/logic/distribute_homeworks.py index a4f07e0..9b8e34c 100644 --- a/lms/logic/distribute_homeworks.py +++ b/lms/logic/distribute_homeworks.py @@ -1,4 +1,4 @@ -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from dataclasses import dataclass from datetime import datetime @@ -39,13 +39,13 @@ async def make_distribution( homeworks = await self._get_soho_homeworks(params.homework_ids) subject = await self._get_subject(params.product_ids[0]) reviewers = await self._get_reviewers(subject_id=subject.id) - student_data = await self._get_students_data( + student_map = await self._get_student_data_map( subject_id=subject.id, homeworks=homeworks, ) - filtered_homeworks, error_homeworks = await self._filter_homeworks( + filtered_homeworks, error_homeworks = await _filter_homeworks( homeworks=homeworks, - student_data=student_data, + student_map=student_map, ) distribution = Distribution( created_at=created_at, @@ -90,64 +90,20 @@ async def _get_subject(self, product_id: int) -> Subject: return await self.uow.subject.read_by_id(product.subject_id) async def _get_reviewers(self, subject_id: int) -> Sequence[DistributionReviewer]: - reviewers = await self.uow.reviewer.get_by_subject_id(subject_id) + reviewers = await self.uow.reviewer.get_list_by_subject_id(subject_id) return [DistributionReviewer(**r.model_dump()) for r in reviewers] - async def _get_students_data( + async def _get_student_data_map( self, subject_id: int, homeworks: Sequence[SohoHomework], - ) -> Sequence[StudentDistributeData]: + ) -> Mapping[int, StudentDistributeData]: vk_ids = set(hw.student_vk_id for hw in homeworks if hw.student_vk_id) return await self.uow.student_product.distribute_data( subject_id=subject_id, vk_ids=vk_ids, ) - async def _filter_homeworks( - self, - student_data: Sequence[StudentDistributeData], - homeworks: Sequence[SohoHomework], - ) -> tuple[Sequence[StudentHomework], Sequence[ErrorHomework]]: - pre_filtered_homeworks: list[StudentHomework] = list() - error_homeworks: list[ErrorHomework] = list() - student_map = {st.vk_id: st for st in student_data} - for hw in homeworks: - if hw.student_vk_id is None: - error_homeworks.append( - ErrorHomework( - homework=hw, - error_message=DistributionErrorMessage.HOMEWORK_WITHOUT_VK_ID, - ) - ) - elif hw.student_vk_id not in student_map: - error_homeworks.append( - ErrorHomework( - homework=hw, - error_message=DistributionErrorMessage.STUDENT_WITH_VK_ID_NOT_FOUND, - ) - ) - elif student_map[hw.student_vk_id].is_expulsed: - error_homeworks.append( - ErrorHomework( - homework=hw, - error_message=DistributionErrorMessage.STUDENT_WAS_EXPULSED, - ) - ) - else: - pre_filtered_homeworks.append( - StudentHomework( - student_name=student_map[hw.student_vk_id].name, - student_vk_id=student_map[hw.student_vk_id].vk_id, - student_soho_id=hw.student_soho_id, - submission_url=hw.chat_url, - teacher_product_id=student_map[ - hw.student_vk_id - ].teacher_product_id, - ) - ) - return pre_filtered_homeworks, error_homeworks - async def _add_folder_to_notification( self, subject_id: int, folder_id: str ) -> None: @@ -206,3 +162,44 @@ def _write_data_in_new_sheet( values=distribution.serialize_for_sheet(), major_dimension=SHEET_MAJOR_DIMENSION, ) + + +async def _filter_homeworks( + homeworks: Sequence[SohoHomework], + student_map: Mapping[int, StudentDistributeData], +) -> tuple[Sequence[StudentHomework], Sequence[ErrorHomework]]: + pre_filtered_homeworks: list[StudentHomework] = list() + error_homeworks: list[ErrorHomework] = list() + for hw in homeworks: + if hw.student_vk_id is None: + error_homeworks.append( + ErrorHomework( + homework=hw, + error_message=DistributionErrorMessage.HOMEWORK_WITHOUT_VK_ID, + ) + ) + elif hw.student_vk_id not in student_map: + error_homeworks.append( + ErrorHomework( + homework=hw, + error_message=DistributionErrorMessage.STUDENT_WITH_VK_ID_NOT_FOUND, + ) + ) + elif student_map[hw.student_vk_id].is_expulsed: + error_homeworks.append( + ErrorHomework( + homework=hw, + error_message=DistributionErrorMessage.STUDENT_WAS_EXPULSED, + ) + ) + else: + pre_filtered_homeworks.append( + StudentHomework( + student_name=student_map[hw.student_vk_id].name, + student_vk_id=student_map[hw.student_vk_id].vk_id, + student_soho_id=hw.student_soho_id, + submission_url=hw.chat_url, + teacher_product_id=student_map[hw.student_vk_id].teacher_product_id, + ) + ) + return pre_filtered_homeworks, error_homeworks diff --git a/lms/rest/api/v1/product/router.py b/lms/rest/api/v1/product/router.py index 5f54da9..147f722 100644 --- a/lms/rest/api/v1/product/router.py +++ b/lms/rest/api/v1/product/router.py @@ -61,6 +61,6 @@ async def create_distribution( await distributor.make_distribution(params=params, created_at=now) return StatusResponseSchema( ok=True, - status_code=201, + status_code=HTTPStatus.CREATED, message="The distribution was created", ) diff --git a/lms/rest/args.py b/lms/rest/args.py index c66cd4b..820cb01 100644 --- a/lms/rest/args.py +++ b/lms/rest/args.py @@ -22,7 +22,7 @@ ) group = parser.add_argument_group("Logging options") -group.add_argument("--log-level", default=LogLevel.info, choices=LogLevel.choices()) +group.add_argument("--log-level", choices=LogLevel.choices(), default=LogLevel.info) group.add_argument("--log-format", choices=LogFormat.choices(), default=LogFormat.color) group = parser.add_argument_group("Project options") diff --git a/lms/rest/service.py b/lms/rest/service.py index f5ed29c..66f9b53 100644 --- a/lms/rest/service.py +++ b/lms/rest/service.py @@ -33,6 +33,8 @@ log = logging.getLogger(__name__) +ExceptionHandlersType = tuple[tuple[type[Exception], Callable], ...] + class REST(UvicornService): __required__ = ( @@ -42,7 +44,6 @@ class REST(UvicornService): "project_version", "secret_key", ) - __dependencies__ = ( "autopilot", "soho", @@ -52,6 +53,12 @@ class REST(UvicornService): "enroller", ) + EXCEPTION_HANDLERS: ExceptionHandlersType = ( + (HTTPException, http_exception_handler), + (RequestValidationError, requset_validation_handler), + (LMSError, lms_exception_handler), + ) + session_factory: async_sessionmaker[AsyncSession] autopilot: Autopilot soho: Soho @@ -72,7 +79,27 @@ async def create_application(self) -> UvicornApplication: title=self.project_name, description=self.project_description, version=self.project_version, + openapi_url="/docs/openapi.json", + docs_url="/docs/swagger", + redoc_url="/docs/redoc", + ) + + self._set_middlewares(app=app) + self._set_routes(app=app) + self._set_exceptions(app=app) + self._set_dependency_overrides(app=app) + + configure_admin( + app=app, + session_factory=self.session_factory, + title=self.project_name, + secret_key=self.secret_key, + debug=self.debug, ) + log.info("REST service app configured") + return app + + def _set_middlewares(self, app: FastAPI) -> None: app.add_middleware( CORSMiddleware, allow_origins=["*"], @@ -81,11 +108,14 @@ async def create_application(self) -> UvicornApplication: allow_headers=["*"], ) + def _set_routes(self, app: FastAPI) -> None: app.include_router(api_router) - app.exception_handler(HTTPException)(http_exception_handler) - app.exception_handler(RequestValidationError)(requset_validation_handler) - app.exception_handler(LMSError)(lms_exception_handler) + def _set_exceptions(self, app: FastAPI) -> None: + for exception, handler in self.EXCEPTION_HANDLERS: + app.add_exception_handler(exception, handler) + + def _set_dependency_overrides(self, app: FastAPI) -> None: app.dependency_overrides.update( { UnitOfWorkMarker: lambda: UnitOfWork(self.session_factory), @@ -98,12 +128,3 @@ async def create_application(self) -> UvicornApplication: DistributorMarker: self.get_distributor, } ) - configure_admin( - app=app, - session_factory=self.session_factory, - title=self.project_name, - secret_key=self.secret_key, - debug=self.debug, - ) - log.info("REST service app configured") - return app diff --git a/lms/utils/distribution/models.py b/lms/utils/distribution/models.py index 053e3ea..7b905e3 100644 --- a/lms/utils/distribution/models.py +++ b/lms/utils/distribution/models.py @@ -87,6 +87,7 @@ async def _distribute_minimum(self) -> None: "Finish min distribution. Left %d homeworks", len(self.filtered_homeworks), ) + self._log_reviewers("minimum") async def _distribute_premium(self) -> None: pass @@ -94,6 +95,7 @@ async def _distribute_premium(self) -> None: async def _distribute_main(self) -> None: self._calculate_actual() self._filled_main() + self._log_reviewers("main") async def _distribute_rechecks(self) -> None: pass @@ -219,3 +221,9 @@ def serialize_for_sheet(self) -> Sequence[Sequence[Any]]: error_message.append(e_hw.error_message) data.extend([[], error_identify, error_name, error_message]) return data + + def _log_reviewers(self, step: str) -> None: + log_str = [f"Reviewers on step `{step}`:"] + for r in self.reviewers: + log_str.append(f"{r.name}: {len(r.student_homeworks)},") + log.info(" ".join(log_str)) diff --git a/tests/api/product/test_read_by_id.py b/tests/api/product/test_read_by_id.py index 934789a..b79cdd1 100644 --- a/tests/api/product/test_read_by_id.py +++ b/tests/api/product/test_read_by_id.py @@ -3,9 +3,17 @@ from aiohttp.test_utils import TestClient -async def test_unauthorized_user(api_client: TestClient) -> None: - response = await api_client.get("/v1/products/1/") +def api_url(product_id: int) -> str: + return f"/v1/products/{product_id}/" + + +async def test_unauthorized_user_status(api_client: TestClient) -> None: + response = await api_client.get(api_url(1)) assert response.status == HTTPStatus.UNAUTHORIZED + + +async def test_unauthorized_user_format(api_client: TestClient) -> None: + response = await api_client.get(api_url(1)) assert await response.json() == { "ok": False, "status_code": HTTPStatus.UNAUTHORIZED, @@ -13,9 +21,13 @@ async def test_unauthorized_user(api_client: TestClient) -> None: } -async def test_invalid_token(api_client: TestClient) -> None: - response = await api_client.get("/v1/products/1/", params={"token": "something"}) +async def test_invalid_token_status(api_client: TestClient) -> None: + response = await api_client.get(api_url(1), params={"token": "something"}) assert response.status == HTTPStatus.FORBIDDEN + + +async def test_invalid_token_format(api_client: TestClient) -> None: + response = await api_client.get(api_url(1), params={"token": "something"}) assert await response.json() == { "ok": False, "status_code": HTTPStatus.FORBIDDEN, @@ -23,9 +35,13 @@ async def test_invalid_token(api_client: TestClient) -> None: } -async def test_subject_not_found(api_client: TestClient, token: str) -> None: - response = await api_client.get("/v1/products/0/", params={"token": token}) +async def test_product_not_found_status(api_client: TestClient, token: str) -> None: + response = await api_client.get(api_url(0), params={"token": token}) assert response.status == HTTPStatus.NOT_FOUND + + +async def test_product_not_found_format(api_client: TestClient, token: str) -> None: + response = await api_client.get(api_url(0), params={"token": token}) assert await response.json() == { "ok": False, "status_code": HTTPStatus.NOT_FOUND, @@ -33,27 +49,27 @@ async def test_subject_not_found(api_client: TestClient, token: str) -> None: } -async def test_succesful_status_ok( +async def test_success_status( api_client: TestClient, token: str, create_product, ): product = await create_product() response = await api_client.get( - f"/v1/products/{product.id}/", + api_url(product_id=product.id), params={"token": token}, ) assert response.status == HTTPStatus.OK -async def test_successful_format_ok( +async def test_success_format( api_client: TestClient, token: str, create_product, ) -> None: product = await create_product() response = await api_client.get( - f"/v1/products/{product.id}/", + api_url(product_id=product.id), params={"token": token}, ) assert await response.json() == { diff --git a/tests/conftest.py b/tests/conftest.py index 58cff59..4e59744 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,16 +1,17 @@ pytest_plugins = ( - "tests.plugins.instances.arguments", - "tests.plugins.instances.autopilot", - "tests.plugins.instances.database", - "tests.plugins.instances.distributor", "tests.plugins.factories.flow", "tests.plugins.factories.offer", "tests.plugins.factories.product", + "tests.plugins.factories.reviewer", "tests.plugins.factories.soho", "tests.plugins.factories.student", "tests.plugins.factories.subject", "tests.plugins.factories.teacher_assignment", "tests.plugins.factories.teacher", + "tests.plugins.instances.arguments", + "tests.plugins.instances.autopilot", + "tests.plugins.instances.database", + "tests.plugins.instances.distributor", "tests.plugins.instances.rest", "tests.plugins.instances.soho", "tests.plugins.instances.telegram", diff --git a/tests/plugins/factories/reviewer.py b/tests/plugins/factories/reviewer.py new file mode 100644 index 0000000..ed58e80 --- /dev/null +++ b/tests/plugins/factories/reviewer.py @@ -0,0 +1,33 @@ +import factory +import pytest +from sqlalchemy.ext.asyncio import AsyncSession + +from lms.db.models import Reviewer + + +class ReviewerFactory(factory.Factory): + class Meta: + model = Reviewer + + id = factory.Sequence(lambda n: n + 1) + subject_id = factory.Sequence(lambda n: n + 1) + first_name = "First" + last_name = "Last" + email = "a@a.ru" + desired = 10 + max_ = 20 + min_ = 5 + abs_max = 30 + is_active = True + + +@pytest.fixture +async def create_reviewer(session: AsyncSession): + async def _factory(**kwargs): + reviewer = ReviewerFactory(**kwargs) + session.add(reviewer) + await session.commit() + await session.flush(reviewer) + return reviewer + + return _factory diff --git a/tests/plugins/instances/database.py b/tests/plugins/instances/database.py index 8a56176..a16c775 100644 --- a/tests/plugins/instances/database.py +++ b/tests/plugins/instances/database.py @@ -12,6 +12,7 @@ ) from lms.db.base import Base +from lms.db.repositories.reviewer import ReviewerRepository from lms.db.uow import UnitOfWork from lms.db.utils import make_alembic_config from tests.plugins.factories.factories import factories @@ -88,6 +89,11 @@ async def session( await clear_db(async_engine) -@pytest.fixture() -async def uow(sessionmaker) -> UnitOfWork: +@pytest.fixture +def uow(sessionmaker) -> UnitOfWork: return UnitOfWork(sessionmaker=sessionmaker) + + +@pytest.fixture +def reviewer_repository(session: AsyncSession) -> ReviewerRepository: + return ReviewerRepository(session=session) diff --git a/tests/reviewers/__init__.py b/tests/reviewers/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/reviewers/test_repository.py b/tests/reviewers/test_repository.py new file mode 100644 index 0000000..4c9880a --- /dev/null +++ b/tests/reviewers/test_repository.py @@ -0,0 +1,49 @@ +from dirty_equals import IsListOrTuple + +from lms.db.repositories.reviewer import ReviewerRepository +from lms.db.uow import UnitOfWork +from lms.generals.models.reviewer import Reviewer + + +async def test_get_list_by_subject_id__subject_unknown( + uow: UnitOfWork, +): + async with uow: + result = await uow.reviewer.get_list_by_subject_id(subject_id=1) + assert result == IsListOrTuple(length=0) + + +async def test_get_list_by_subject_id__ok( + reviewer_repository: ReviewerRepository, + create_subject, + create_reviewer, +): + subject = await create_subject() + reviewer = await create_reviewer(subject_id=subject.id) + + result = await reviewer_repository.get_list_by_subject_id(subject_id=subject.id) + assert result == IsListOrTuple(Reviewer.model_validate(reviewer), length=1) + + +async def test_get_list_by_subject_id__filter_inactive( + reviewer_repository: ReviewerRepository, + create_subject, + create_reviewer, +): + subject = await create_subject() + await create_reviewer(subject_id=subject.id, is_active=False) + + result = await reviewer_repository.get_list_by_subject_id(subject_id=subject.id) + assert result == IsListOrTuple(length=0) + + +async def test_get_list_by_subject_id__filter_subject( + reviewer_repository: ReviewerRepository, + create_subject, + create_reviewer, +): + subject = await create_subject() + await create_reviewer(subject_id=subject.id) + + result = await reviewer_repository.get_list_by_subject_id(subject_id=subject.id + 1) + assert result == IsListOrTuple(length=0)