From 841b72ef6290e07fa7eb144c488d9813ca41c070 Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 23 Feb 2024 18:42:48 +0900 Subject: [PATCH 01/14] Support celery task for entity create on APIv2 --- entity/api_v2/serializers.py | 14 ++++++++++---- entity/api_v2/views.py | 15 ++++++++++++++- entity/tasks.py | 22 ++++++++++++++++++++++ frontend/src/repository/AironeApiClient.ts | 4 ++-- job/models.py | 13 +++++++++++++ 5 files changed, 61 insertions(+), 7 deletions(-) diff --git a/entity/api_v2/serializers.py b/entity/api_v2/serializers.py index 083c115f8..848e921f0 100644 --- a/entity/api_v2/serializers.py +++ b/entity/api_v2/serializers.py @@ -204,7 +204,7 @@ def _update_or_create( entity: Entity entity, is_created_entity = Entity.objects.get_or_create( - id=entity_id, defaults={**validated_data} + id=entity_id, created_user=user, defaults={**validated_data} ) if not is_created_entity: # record history for specific fields on update @@ -324,11 +324,10 @@ class EntityCreateSerializer(EntitySerializer): child=EntityAttrCreateSerializer(), write_only=True, required=False, default=[] ) webhooks = WebhookCreateUpdateSerializer(many=True, write_only=True, required=False, default=[]) - created_user = serializers.HiddenField(default=serializers.CurrentUserDefault()) class Meta: model = Entity - fields = ["id", "name", "note", "is_toplevel", "attrs", "webhooks", "created_user"] + fields = ["id", "name", "note", "is_toplevel", "attrs", "webhooks"] extra_kwargs = {"note": {"write_only": True}} def validate_name(self, name): @@ -353,7 +352,14 @@ def validate_webhooks(self, webhooks: list[WebhookCreateUpdateSerializer]): return webhooks def create(self, validated_data: EntityCreateData): - user: User = self.context["request"].user + user: User | None = None + if "request" in self.context: + user = self.context["request"].user + if "_user" in self.context: + user = self.context["_user"] + + if user is None: + raise RequiredParameterError("user is required") if custom_view.is_custom("before_create_entity_V2"): validated_data = custom_view.call_custom( diff --git a/entity/api_v2/views.py b/entity/api_v2/views.py index 6c381d324..c1ea81134 100644 --- a/entity/api_v2/views.py +++ b/entity/api_v2/views.py @@ -29,6 +29,7 @@ from entity.models import Entity, EntityAttr from entry.api_v2.serializers import EntryBaseSerializer, EntryCreateSerializer from entry.models import Entry +from job.models import Job from user.models import History, User @@ -110,7 +111,7 @@ class EntityAPI(viewsets.ModelViewSet): def get_serializer_class(self): serializer = { "list": EntityListSerializer, - "create": EntityCreateSerializer, + "create": serializers.Serializer, "update": EntityUpdateSerializer, } return serializer.get(self.action, EntityDetailSerializer) @@ -129,6 +130,18 @@ def get_queryset(self): return Entity.objects.filter(**filter_condition).exclude(**exclude_condition) + @extend_schema(request=EntityCreateSerializer) + def create(self, request, *args, **kwargs): + user: User = request.user + + serializer = EntityCreateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + job = Job.new_create_entity_v2(user, None, params=serializer.validated_data) + job.run() + + return Response(status=status.HTTP_202_ACCEPTED) + def destroy(self, request, pk): entity: Entity = self.get_object() user: User = request.user diff --git a/entity/tasks.py b/entity/tasks.py index 770c80946..f8d6aa637 100644 --- a/entity/tasks.py +++ b/entity/tasks.py @@ -2,6 +2,7 @@ from airone.celery import app from airone.lib.types import AttrTypeValue +from entity.api_v2.serializers import EntityCreateSerializer from entity.models import Entity, EntityAttr from job.models import Job from user.models import User @@ -237,3 +238,24 @@ def delete_entity(self, job_id): # update job status and save it job.update(Job.STATUS["DONE"]) + + +@app.task(bind=True) +def create_entity_v2(self, job_id: int): + job = Job.objects.get(id=job_id) + + if not job.proceed_if_ready(): + return + + # At the first time, update job status to prevent executing this job duplicately + job.update(Job.STATUS["PROCESSING"]) + + serializer = EntityCreateSerializer(data=json.loads(job.params), context={"_user": job.user}) + if not serializer.is_valid(): + job.update(Job.STATUS["ERROR"]) + return + + serializer.create(serializer.validated_data) + + # update job status and save it + job.update(Job.STATUS["DONE"]) diff --git a/frontend/src/repository/AironeApiClient.ts b/frontend/src/repository/AironeApiClient.ts index bf58e6bc1..a0c6202c1 100644 --- a/frontend/src/repository/AironeApiClient.ts +++ b/frontend/src/repository/AironeApiClient.ts @@ -240,8 +240,8 @@ class AironeApiClient { isToplevel: boolean, attrs: Array, webhooks: Array - ): Promise { - return await this.entity.entityApiV2Create( + ): Promise { + await this.entity.entityApiV2Create( { entityCreate: { id: -1, diff --git a/job/models.py b/job/models.py index aa849fe81..875d8dee7 100644 --- a/job/models.py +++ b/job/models.py @@ -50,6 +50,7 @@ class JobOperation(Enum): UPDATE_DOCUMENT = 21 EXPORT_SEARCH_RESULT_V2 = 22 MAY_INVOKE_TRIGGER = 23 + CREATE_ENTITY_V2 = 24 class Job(models.Model): @@ -338,6 +339,7 @@ def method_table(kls): JobOperation.UPDATE_DOCUMENT.value: entry_task.update_es_documents, JobOperation.EXPORT_SEARCH_RESULT_V2.value: entry_task.export_search_result_v2, JobOperation.MAY_INVOKE_TRIGGER.value: trigger_task.may_invoke_trigger, + JobOperation.CREATE_ENTITY_V2.value: entity_task.create_entity_v2, } return kls._METHOD_TABLE @@ -540,6 +542,17 @@ def new_invoke_trigger(kls, user, target_entry, recv_attrs={}, dependent_job=Non dependent_job, ) + @classmethod + def new_create_entity_v2(kls, user, target, text="", params={}): + print(params) + return kls._create_new_job( + user, + target, + JobOperation.CREATE_ENTITY_V2.value, + text, + json.dumps(params, default=_support_time_default, sort_keys=True), + ) + def set_cache(self, value): with default_storage.open("job_%d" % self.id, "wb") as fp: pickle.dump(value, fp) From b963c8e8fb6e40f152ec4fc8097fc87614495283 Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Sat, 2 Mar 2024 21:39:02 +0900 Subject: [PATCH 02/14] Fix test for entity create --- acl/tests/test_api_v2.py | 9 +++++++++ airone/lib/drf.py | 15 +++++++++++++++ entity/api_v2/serializers.py | 4 +++- entity/api_v2/views.py | 4 ++-- entity/tests/test_api_v2.py | 34 +++++++++++++++++++++------------- job/models.py | 1 - 6 files changed, 50 insertions(+), 17 deletions(-) diff --git a/acl/tests/test_api_v2.py b/acl/tests/test_api_v2.py index 3fac8663d..ab90ee79c 100644 --- a/acl/tests/test_api_v2.py +++ b/acl/tests/test_api_v2.py @@ -1,9 +1,12 @@ import json +from mock import mock + from acl.models import ACLBase from airone.lib.acl import ACLType from airone.lib.test import AironeViewTest from airone.lib.types import AttrTypeValue +from entity import tasks from entity.models import Entity, EntityAttr from role.models import Role @@ -474,6 +477,9 @@ def _put_acl(): self.assertEqual(resp.status_code, 200) self.assertTrue(role.is_permitted(acl, ACLType.Full)) + @mock.patch( + "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) + ) def test_list_history(self): self.initialization_for_retrieve_test() self.client.post("/entity/api/v2/", json.dumps({"name": "test"}), "application/json") @@ -594,6 +600,9 @@ def test_list_history_with_role(self): ], ) + @mock.patch( + "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) + ) def test_list_history_with_entity_attr(self): self.initialization_for_retrieve_test() diff --git a/airone/lib/drf.py b/airone/lib/drf.py index 1f6fa4aef..b2e56c3e6 100644 --- a/airone/lib/drf.py +++ b/airone/lib/drf.py @@ -2,6 +2,7 @@ import yaml from django.conf import settings +from rest_framework import serializers from rest_framework.exceptions import APIException, ParseError, ValidationError from rest_framework.parsers import BaseParser from rest_framework.renderers import BaseRenderer @@ -150,3 +151,17 @@ def _convert_error_code(detail): response.data = _convert_error_code(response.data) return response + + +class AironeUserDefault(serializers.CurrentUserDefault): + """ + It enables to get user from the custom field in the context. + The original CurrentUserDefault fetches it from request context, + so it fails if the context doesn't have request. + """ + + def __call__(self, serializer_field): + if "_user" in serializer_field.context: + return serializer_field.context["_user"] + + return super().__call__(serializer_field) diff --git a/entity/api_v2/serializers.py b/entity/api_v2/serializers.py index 848e921f0..2304e4cf3 100644 --- a/entity/api_v2/serializers.py +++ b/entity/api_v2/serializers.py @@ -11,6 +11,7 @@ from rest_framework.exceptions import PermissionDenied, ValidationError import custom_view +from airone.lib import drf from airone.lib.acl import ACLType from airone.lib.drf import DuplicatedObjectExistsError, ObjectNotExistsError, RequiredParameterError from airone.lib.log import Logger @@ -69,7 +70,7 @@ def validate(self, webhook): class EntityAttrCreateSerializer(serializers.ModelSerializer): - created_user = serializers.HiddenField(default=serializers.CurrentUserDefault()) + created_user = serializers.HiddenField(default=drf.AironeUserDefault()) class Meta: model = EntityAttr @@ -361,6 +362,7 @@ def create(self, validated_data: EntityCreateData): if user is None: raise RequiredParameterError("user is required") + validated_data["created_user"] = user if custom_view.is_custom("before_create_entity_V2"): validated_data = custom_view.call_custom( "before_create_entity_v2", None, user, validated_data diff --git a/entity/api_v2/views.py b/entity/api_v2/views.py index c1ea81134..fe4c85f7c 100644 --- a/entity/api_v2/views.py +++ b/entity/api_v2/views.py @@ -134,10 +134,10 @@ def get_queryset(self): def create(self, request, *args, **kwargs): user: User = request.user - serializer = EntityCreateSerializer(data=request.data) + serializer = EntityCreateSerializer(data=request.data, context={"_user": user}) serializer.is_valid(raise_exception=True) - job = Job.new_create_entity_v2(user, None, params=serializer.validated_data) + job = Job.new_create_entity_v2(user, None, params=request.data) job.run() return Response(status=status.HTTP_202_ACCEPTED) diff --git a/entity/tests/test_api_v2.py b/entity/tests/test_api_v2.py index 233246432..77410e3f1 100644 --- a/entity/tests/test_api_v2.py +++ b/entity/tests/test_api_v2.py @@ -6,6 +6,7 @@ import yaml from django.conf import settings from django.urls import reverse +from rest_framework import status from rest_framework.exceptions import ValidationError from acl.models import ACLBase @@ -465,6 +466,9 @@ def test_list_entity_without_permission(self): self.assertEqual(resp.status_code, 200) self.assertEqual(resp.json()["count"], 2) + @mock.patch( + "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) + ) def test_create_entity(self): params = { "name": "entity1", @@ -493,16 +497,9 @@ def test_create_entity(self): } resp = self.client.post("/entity/api/v2/", json.dumps(params), "application/json") - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) - entity: Entity = Entity.objects.get(id=resp.json()["id"]) - self.assertEqual( - resp.json(), - { - "id": entity.id, - "name": "entity1", - }, - ) + entity: Entity = Entity.objects.get(name=params["name"]) self.assertEqual(entity.name, "entity1") self.assertEqual(entity.note, "hoge") self.assertEqual(entity.status, Entity.STATUS_TOP_LEVEL) @@ -1246,6 +1243,9 @@ def test_create_entity_with_invalid_param_webhooks(self): }, ) + @mock.patch( + "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) + ) def test_create_entity_with_attrs_referral(self): params = { "name": "entity1", @@ -1261,25 +1261,33 @@ def test_create_entity_with_attrs_referral(self): } resp = self.client.post("/entity/api/v2/", json.dumps(params), "application/json") + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) - entity: Entity = Entity.objects.get(id=resp.json()["id"]) + entity: Entity = Entity.objects.get(name=params["name"]) for entity_attr in entity.attrs.all(): if entity_attr.type & AttrTypeValue["object"]: self.assertEqual([x.id for x in entity_attr.referral.all()], [self.ref_entity.id]) else: self.assertEqual([x.id for x in entity_attr.referral.all()], []) + @mock.patch( + "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) + ) def test_create_entity_with_webhook_is_verified(self): params = { "name": "entity1", "webhooks": [{"url": "http://example.net/"}, {"url": "http://hoge.hoge/"}], } resp = self.client.post("/entity/api/v2/", json.dumps(params), "application/json") - entity: Entity = Entity.objects.get(id=resp.json()["id"]) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + entity: Entity = Entity.objects.get(name=params["name"]) self.assertEqual([x.is_verified for x in entity.webhooks.all()], [True, False]) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") + @mock.patch( + "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) + ) def test_create_entity_with_customview(self, mock_call_custom): params = {"name": "hoge"} @@ -1297,7 +1305,7 @@ def side_effect(handler_name, entity_name, user, *args): self.assertEqual(user, self.user) if handler_name == "before_create_entity_v2": - self.assertEqual( + self.assertDictEqual( args[0], { "name": "hoge", @@ -1315,7 +1323,7 @@ def side_effect(handler_name, entity_name, user, *args): mock_call_custom.side_effect = side_effect resp = self.client.post("/entity/api/v2/", json.dumps(params), "application/json") - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertTrue(mock_call_custom.called) def test_create_entity_with_webhook_is_disabled(self): diff --git a/job/models.py b/job/models.py index 875d8dee7..372b35d8e 100644 --- a/job/models.py +++ b/job/models.py @@ -544,7 +544,6 @@ def new_invoke_trigger(kls, user, target_entry, recv_attrs={}, dependent_job=Non @classmethod def new_create_entity_v2(kls, user, target, text="", params={}): - print(params) return kls._create_new_job( user, target, From ddc23c5140079d09a7bb936e729ed661964f077c Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Sat, 2 Mar 2024 22:25:42 +0900 Subject: [PATCH 03/14] Support celery task for entity edit on APIv2 --- entity/api_v2/serializers.py | 9 ++++++- entity/api_v2/views.py | 17 +++++++++++- entity/tasks.py | 30 +++++++++++++++++++++- frontend/src/repository/AironeApiClient.ts | 4 +-- job/models.py | 17 +++++++++++- 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/entity/api_v2/serializers.py b/entity/api_v2/serializers.py index 2304e4cf3..314bdc42c 100644 --- a/entity/api_v2/serializers.py +++ b/entity/api_v2/serializers.py @@ -423,7 +423,14 @@ def validate_webhooks(self, webhooks: list[WebhookCreateUpdateSerializer]): return webhooks def update(self, entity: Entity, validated_data: EntityUpdateData): - user: User = self.context["request"].user + user: User | None = None + if "request" in self.context: + user = self.context["request"].user + if "_user" in self.context: + user = self.context["_user"] + + if user is None: + raise RequiredParameterError("user is required") if custom_view.is_custom("before_update_entity_v2"): validated_data = custom_view.call_custom( diff --git a/entity/api_v2/views.py b/entity/api_v2/views.py index fe4c85f7c..df5a4932c 100644 --- a/entity/api_v2/views.py +++ b/entity/api_v2/views.py @@ -112,7 +112,7 @@ def get_serializer_class(self): serializer = { "list": EntityListSerializer, "create": serializers.Serializer, - "update": EntityUpdateSerializer, + "update": serializers.Serializer, } return serializer.get(self.action, EntityDetailSerializer) @@ -142,6 +142,21 @@ def create(self, request, *args, **kwargs): return Response(status=status.HTTP_202_ACCEPTED) + @extend_schema(request=EntityUpdateSerializer) + def update(self, request, *args, **kwargs): + user: User = request.user + entity: Entity = self.get_object() + + serializer = EntityUpdateSerializer( + instance=entity, data=request.data, context={"_user": user} + ) + serializer.is_valid(raise_exception=True) + + job = Job.new_edit_entity_v2(user, entity, params=request.data) + job.run() + + return Response(status=status.HTTP_202_ACCEPTED) + def destroy(self, request, pk): entity: Entity = self.get_object() user: User = request.user diff --git a/entity/tasks.py b/entity/tasks.py index f8d6aa637..08813969d 100644 --- a/entity/tasks.py +++ b/entity/tasks.py @@ -2,7 +2,7 @@ from airone.celery import app from airone.lib.types import AttrTypeValue -from entity.api_v2.serializers import EntityCreateSerializer +from entity.api_v2.serializers import EntityCreateSerializer, EntityUpdateSerializer from entity.models import Entity, EntityAttr from job.models import Job from user.models import User @@ -259,3 +259,31 @@ def create_entity_v2(self, job_id: int): # update job status and save it job.update(Job.STATUS["DONE"]) + + +@app.task(bind=True) +def edit_entity_v2(self, job_id: int): + job = Job.objects.get(id=job_id) + + if not job.proceed_if_ready(): + return + + # At the first time, update job status to prevent executing this job duplicately + job.update(Job.STATUS["PROCESSING"]) + + entity: Entity | None = Entity.objects.filter(id=job.target.id, is_active=True).first() + if not entity: + job.update(Job.STATUS["ERROR"]) + return + + serializer = EntityUpdateSerializer( + instance=entity, data=json.loads(job.params), context={"_user": job.user} + ) + if not serializer.is_valid(): + job.update(Job.STATUS["ERROR"]) + return + + serializer.update(entity, serializer.validated_data) + + # update job status and save it + job.update(Job.STATUS["DONE"]) diff --git a/frontend/src/repository/AironeApiClient.ts b/frontend/src/repository/AironeApiClient.ts index a0c6202c1..a375a66a7 100644 --- a/frontend/src/repository/AironeApiClient.ts +++ b/frontend/src/repository/AironeApiClient.ts @@ -268,8 +268,8 @@ class AironeApiClient { isToplevel: boolean, attrs: Array, webhooks: Array - ): Promise { - return await this.entity.entityApiV2Update( + ): Promise { + await this.entity.entityApiV2Update( { id: id, entityUpdate: { diff --git a/job/models.py b/job/models.py index 372b35d8e..ec8857554 100644 --- a/job/models.py +++ b/job/models.py @@ -4,6 +4,7 @@ from datetime import date, datetime, timedelta from enum import Enum from importlib import import_module +from typing import Any import pytz from django.conf import settings @@ -51,6 +52,7 @@ class JobOperation(Enum): EXPORT_SEARCH_RESULT_V2 = 22 MAY_INVOKE_TRIGGER = 23 CREATE_ENTITY_V2 = 24 + EDIT_ENTITY_V2 = 25 class Job(models.Model): @@ -263,7 +265,9 @@ def run(self, will_delay=True): return method(self.id) @classmethod - def _create_new_job(kls, user, target, operation, text, params, depend_on=None) -> "Job": + def _create_new_job( + kls, user, target: Entity | Entry | Any, operation, text, params, depend_on=None + ) -> "Job": t_type = kls.TARGET_UNKNOWN if isinstance(target, Entry): t_type = kls.TARGET_ENTRY @@ -340,6 +344,7 @@ def method_table(kls): JobOperation.EXPORT_SEARCH_RESULT_V2.value: entry_task.export_search_result_v2, JobOperation.MAY_INVOKE_TRIGGER.value: trigger_task.may_invoke_trigger, JobOperation.CREATE_ENTITY_V2.value: entity_task.create_entity_v2, + JobOperation.EDIT_ENTITY_V2.value: entity_task.edit_entity_v2, } return kls._METHOD_TABLE @@ -552,6 +557,16 @@ def new_create_entity_v2(kls, user, target, text="", params={}): json.dumps(params, default=_support_time_default, sort_keys=True), ) + @classmethod + def new_edit_entity_v2(kls, user, target: Entity, text="", params={}): + return kls._create_new_job( + user, + target, + JobOperation.EDIT_ENTITY_V2.value, + text, + json.dumps(params, default=_support_time_default, sort_keys=True), + ) + def set_cache(self, value): with default_storage.open("job_%d" % self.id, "wb") as fp: pickle.dump(value, fp) From 8a9734e1815dc7713e60bbe091b002acaeb68b9f Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Sun, 3 Mar 2024 00:39:30 +0900 Subject: [PATCH 04/14] Fix test for entity edit --- entity/api_v2/serializers.py | 2 +- entity/tasks.py | 12 ++++++++-- entity/tests/test_api_v2.py | 46 ++++++++++++++++++------------------ 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/entity/api_v2/serializers.py b/entity/api_v2/serializers.py index 314bdc42c..d9f63e0f6 100644 --- a/entity/api_v2/serializers.py +++ b/entity/api_v2/serializers.py @@ -142,7 +142,7 @@ def validate(self, attr): if "type" in attr and attr["type"] != entity_attr.type: raise ValidationError("type cannot be changed") - user: User = self.context["request"].user + user: User = self.context.get("_user") or self.context["request"].user if attr["is_deleted"] and not user.has_permission(entity_attr, ACLType.Full): raise PermissionDenied("Does not have permission to delete") if not attr["is_deleted"] and not user.has_permission(entity_attr, ACLType.Writable): diff --git a/entity/tasks.py b/entity/tasks.py index 08813969d..a3dc77ae9 100644 --- a/entity/tasks.py +++ b/entity/tasks.py @@ -255,7 +255,11 @@ def create_entity_v2(self, job_id: int): job.update(Job.STATUS["ERROR"]) return - serializer.create(serializer.validated_data) + try: + serializer.create(serializer.validated_data) + except Exception: + job.update(Job.STATUS["ERROR"]) + return # update job status and save it job.update(Job.STATUS["DONE"]) @@ -283,7 +287,11 @@ def edit_entity_v2(self, job_id: int): job.update(Job.STATUS["ERROR"]) return - serializer.update(entity, serializer.validated_data) + try: + serializer.update(entity, serializer.validated_data) + except Exception: + job.update(Job.STATUS["ERROR"]) + return # update job status and save it job.update(Job.STATUS["DONE"]) diff --git a/entity/tests/test_api_v2.py b/entity/tests/test_api_v2.py index 77410e3f1..3521175ed 100644 --- a/entity/tests/test_api_v2.py +++ b/entity/tests/test_api_v2.py @@ -1283,11 +1283,11 @@ def test_create_entity_with_webhook_is_verified(self): entity: Entity = Entity.objects.get(name=params["name"]) self.assertEqual([x.is_verified for x in entity.webhooks.all()], [True, False]) - @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) - @mock.patch("custom_view.call_custom") @mock.patch( "entity.tasks.create_entity_v2.delay", mock.Mock(side_effect=tasks.create_entity_v2) ) + @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) + @mock.patch("custom_view.call_custom") def test_create_entity_with_customview(self, mock_call_custom): params = {"name": "hoge"} @@ -1296,8 +1296,8 @@ def side_effect(handler_name, entity_name, user, *args): mock_call_custom.side_effect = side_effect resp = self.client.post("/entity/api/v2/", json.dumps(params), "application/json") - self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json(), [{"code": "AE-121000", "message": "create error"}]) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, *args): # Check specified parameters are expected @@ -1351,6 +1351,7 @@ def test_create_entity_with_webhook_is_disabled(self): finally: settings.AIRONE_FLAGS = {"WEBHOOK": True} + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) def test_update_entity(self): entity: Entity = self.create_entity( **{ @@ -1403,15 +1404,8 @@ def test_update_entity(self): resp = self.client.put( "/entity/api/v2/%d/" % entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) - self.assertEqual( - resp.json(), - { - "id": entity.id, - "name": "change-entity1", - }, - ) entity.refresh_from_db() self.assertEqual(entity.name, "change-entity1") self.assertEqual(entity.note, "change-hoge") @@ -1567,6 +1561,7 @@ def test_update_entity_with_invalid_param(self): {"is_toplevel": [{"code": "AE-121000", "message": "Must be a valid boolean."}]}, ) + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) def test_update_entity_with_invalid_param_attrs(self): params = { "attrs": "hoge", @@ -2088,7 +2083,7 @@ def test_update_entity_with_invalid_param_attrs(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertTrue( all(["hoge" not in x.name for x in self.entity.attrs.filter(is_active=True)]) ) @@ -2133,7 +2128,7 @@ def test_update_entity_with_invalid_param_attrs(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) def test_update_entity_with_invalid_param_webhooks(self): params = { @@ -2429,6 +2424,7 @@ def test_update_entity_with_invalid_param_webhooks(self): }, ) + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) def test_update_entity_with_attrs_referral(self): self.entity.attrs.all().delete() params = { @@ -2446,7 +2442,7 @@ def test_update_entity_with_attrs_referral(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) for entity_attr in self.entity.attrs.all(): if entity_attr.type & AttrTypeValue["object"]: @@ -2454,6 +2450,7 @@ def test_update_entity_with_attrs_referral(self): else: self.assertEqual([x.id for x in entity_attr.referral.all()], []) + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) def test_update_entity_with_webhook_is_verified(self): self.entity.webhooks.all().delete() params = { @@ -2463,10 +2460,11 @@ def test_update_entity_with_webhook_is_verified(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertEqual([x.is_verified for x in self.entity.webhooks.all()], [True, False]) + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") def test_update_entity_with_customview(self, mock_call_custom): @@ -2479,8 +2477,8 @@ def side_effect(handler_name, entity_name, user, *args): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json(), [{"code": "AE-121000", "message": "update error"}]) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, *args): # Check specified parameters are expected @@ -2506,9 +2504,10 @@ def side_effect(handler_name, entity_name, user, *args): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertTrue(mock_call_custom.called) + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) def test_update_entry_with_specified_only_param(self): self.entity.note = "hoge" self.entity.status = Entity.STATUS_TOP_LEVEL @@ -2525,7 +2524,7 @@ def test_update_entry_with_specified_only_param(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.entity.refresh_from_db() changed_attr_count = self.entity.attrs.filter(is_active=True).count() @@ -2536,6 +2535,7 @@ def test_update_entry_with_specified_only_param(self): self.assertEqual(attr_count, changed_attr_count) self.assertEqual(webhook_count, changed_webhook_count) + @mock.patch("entity.tasks.edit_entity_v2.delay", mock.Mock(side_effect=tasks.edit_entity_v2)) def test_update_entity_without_permission(self): self.role.users.add(self.user) @@ -2574,7 +2574,7 @@ def test_update_entity_without_permission(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(paramas), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) # permission nothing EntityAttr update self.entity.is_public = True @@ -2608,7 +2608,7 @@ def test_update_entity_without_permission(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) # permission writable EntityAttr delete params = {"attrs": [{"id": entity_attr.id, "is_deleted": True}]} @@ -2626,7 +2626,7 @@ def test_update_entity_without_permission(self): resp = self.client.put( "/entity/api/v2/%d/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) def test_update_entity_with_webhook_is_disabled(self): entity: Entity = self.create_entity( From f1aeae4edb8d6b42014723441fd326a13aaf04a2 Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Sun, 3 Mar 2024 09:32:57 +0900 Subject: [PATCH 05/14] Support celery task for entity delete on APIv2 --- entity/api_v2/views.py | 25 +++++---------------- entity/tasks.py | 43 ++++++++++++++++++++++++++++++++++++- entity/tests/test_api_v2.py | 19 ++++++++++------ job/models.py | 12 +++++++++++ 4 files changed, 72 insertions(+), 27 deletions(-) diff --git a/entity/api_v2/views.py b/entity/api_v2/views.py index df5a4932c..20b0b1410 100644 --- a/entity/api_v2/views.py +++ b/entity/api_v2/views.py @@ -13,7 +13,6 @@ from rest_framework.response import Response from rest_framework.serializers import Serializer -import custom_view from airone.lib.acl import ACLType, get_permitted_objects from airone.lib.drf import ObjectNotExistsError, YAMLParser, YAMLRenderer from airone.lib.http import http_get @@ -157,9 +156,9 @@ def update(self, request, *args, **kwargs): return Response(status=status.HTTP_202_ACCEPTED) - def destroy(self, request, pk): - entity: Entity = self.get_object() + def destroy(self, request, *args, **kwargs): user: User = request.user + entity: Entity = self.get_object() if not entity.is_active: raise ObjectNotExistsError("specified entity has already been deleted") @@ -169,24 +168,10 @@ def destroy(self, request, pk): "cannot delete Entity because one or more Entries are not deleted" ) - if custom_view.is_custom("before_delete_entity_v2"): - custom_view.call_custom("before_delete_entity_v2", None, user, entity) - - # register operation History for deleting entity - history: History = user.seth_entity_del(entity) - - entity.delete() - - # Delete all attributes which target Entity have - entity_attr: EntityAttr - for entity_attr in entity.attrs.filter(is_active=True): - history.del_attr(entity_attr) - entity_attr.delete() - - if custom_view.is_custom("after_delete_entity_v2"): - custom_view.call_custom("after_delete_entity_v2", None, user, entity) + job = Job.new_delete_entity_v2(user, entity, params=request.data) + job.run() - return Response(status=status.HTTP_204_NO_CONTENT) + return Response(status=status.HTTP_202_ACCEPTED) class EntityEntryAPI(viewsets.ModelViewSet): diff --git a/entity/tasks.py b/entity/tasks.py index a3dc77ae9..cac47586f 100644 --- a/entity/tasks.py +++ b/entity/tasks.py @@ -1,11 +1,12 @@ import json +import custom_view from airone.celery import app from airone.lib.types import AttrTypeValue from entity.api_v2.serializers import EntityCreateSerializer, EntityUpdateSerializer from entity.models import Entity, EntityAttr from job.models import Job -from user.models import User +from user.models import History, User @app.task(bind=True) @@ -295,3 +296,43 @@ def edit_entity_v2(self, job_id: int): # update job status and save it job.update(Job.STATUS["DONE"]) + + +@app.task(bind=True) +def delete_entity_v2(self, job_id: int): + job = Job.objects.get(id=job_id) + + if not job.proceed_if_ready(): + return + + # At the first time, update job status to prevent executing this job duplicately + job.update(Job.STATUS["PROCESSING"]) + + entity: Entity | None = Entity.objects.filter(id=job.target.id, is_active=True).first() + if not entity: + job.update(Job.STATUS["ERROR"]) + return + + try: + if custom_view.is_custom("before_delete_entity_v2"): + custom_view.call_custom("before_delete_entity_v2", None, job.user, entity) + + # register operation History for deleting entity + history: History = job.user.seth_entity_del(entity) + + entity.delete() + + # Delete all attributes which target Entity have + entity_attr: EntityAttr + for entity_attr in entity.attrs.filter(is_active=True): + history.del_attr(entity_attr) + entity_attr.delete() + + if custom_view.is_custom("after_delete_entity_v2"): + custom_view.call_custom("after_delete_entity_v2", None, job.user, entity) + except Exception: + job.update(Job.STATUS["ERROR"]) + return + + # update job status and save it + job.update(Job.STATUS["DONE"]) diff --git a/entity/tests/test_api_v2.py b/entity/tests/test_api_v2.py index 3521175ed..706465f62 100644 --- a/entity/tests/test_api_v2.py +++ b/entity/tests/test_api_v2.py @@ -2663,9 +2663,12 @@ def test_update_entity_with_webhook_is_disabled(self): finally: settings.AIRONE_FLAGS = {"WEBHOOK": True} + @mock.patch( + "entity.tasks.delete_entity_v2.delay", mock.Mock(side_effect=tasks.delete_entity_v2) + ) def test_delete_entity(self): - resp = self.client.delete("/entity/api/v2/%d/" % self.entity.id, None, "application/json") - self.assertEqual(resp.status_code, 204) + resp = self.client.delete("/entity/api/v2/%d/" % self.entity.id) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.entity.refresh_from_db() self.assertFalse(self.entity.is_active) @@ -2676,6 +2679,9 @@ def test_delete_entity(self): self.assertEqual(history.details.count(), self.entity.attrs.count()) self.assertEqual(history.details.first().target_obj, self.entity.attrs.first().aclbase_ptr) + @mock.patch( + "entity.tasks.delete_entity_v2.delay", mock.Mock(side_effect=tasks.delete_entity_v2) + ) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") def test_delete_entity_with_customview(self, mock_call_custom): @@ -2683,8 +2689,9 @@ def side_effect(handler_name, entity_name, user, entity): raise ValidationError("delete error") mock_call_custom.side_effect = side_effect - resp = self.client.delete("/entity/api/v2/%d/" % self.entity.id, None, "application/json") - self.assertEqual(resp.status_code, 400) + resp = self.client.delete("/entity/api/v2/%d/" % self.entity.id) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, entity): # Check specified parameters are expected @@ -2694,8 +2701,8 @@ def side_effect(handler_name, entity_name, user, entity): self.assertEqual(entity, self.entity) mock_call_custom.side_effect = side_effect - resp = self.client.delete("/entity/api/v2/%d/" % self.entity.id, None, "application/json") - self.assertEqual(resp.status_code, 204) + resp = self.client.delete("/entity/api/v2/%d/" % self.entity.id) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertTrue(mock_call_custom.called) def test_delete_entity_with_invalid_param(self): diff --git a/job/models.py b/job/models.py index ec8857554..b01cc5c6d 100644 --- a/job/models.py +++ b/job/models.py @@ -53,6 +53,7 @@ class JobOperation(Enum): MAY_INVOKE_TRIGGER = 23 CREATE_ENTITY_V2 = 24 EDIT_ENTITY_V2 = 25 + DELETE_ENTITY_V2 = 26 class Job(models.Model): @@ -345,6 +346,7 @@ def method_table(kls): JobOperation.MAY_INVOKE_TRIGGER.value: trigger_task.may_invoke_trigger, JobOperation.CREATE_ENTITY_V2.value: entity_task.create_entity_v2, JobOperation.EDIT_ENTITY_V2.value: entity_task.edit_entity_v2, + JobOperation.DELETE_ENTITY_V2.value: entity_task.delete_entity_v2, } return kls._METHOD_TABLE @@ -567,6 +569,16 @@ def new_edit_entity_v2(kls, user, target: Entity, text="", params={}): json.dumps(params, default=_support_time_default, sort_keys=True), ) + @classmethod + def new_delete_entity_v2(kls, user, target: Entity, text="", params={}): + return kls._create_new_job( + user, + target, + JobOperation.DELETE_ENTITY_V2.value, + text, + json.dumps(params, default=_support_time_default, sort_keys=True), + ) + def set_cache(self, value): with default_storage.open("job_%d" % self.id, "wb") as fp: pickle.dump(value, fp) From 39d0beedc350853e7ee397b2aa6282cde5ba042c Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 11:14:29 +0900 Subject: [PATCH 06/14] Support celery task for entry create on APIv2 --- entity/api_v2/views.py | 13 +++++++-- entry/api_v2/serializers.py | 20 +++++++++++-- entry/tasks.py | 15 ++++++++++ frontend/src/repository/AironeApiClient.ts | 4 +-- job/models.py | 34 ++++++++++++++++++++++ 5 files changed, 79 insertions(+), 7 deletions(-) diff --git a/entity/api_v2/views.py b/entity/api_v2/views.py index 20b0b1410..923890f24 100644 --- a/entity/api_v2/views.py +++ b/entity/api_v2/views.py @@ -185,7 +185,7 @@ class EntityEntryAPI(viewsets.ModelViewSet): def get_serializer_class(self): serializer = { - "create": EntryCreateSerializer, + "create": serializers.Serializer, } return serializer.get(self.action, EntryBaseSerializer) @@ -195,9 +195,18 @@ def get_queryset(self): raise Http404 return self.queryset.filter(schema=entity) + @extend_schema(request=EntryCreateSerializer) def create(self, request, entity_id): + user: User = request.user request.data["schema"] = entity_id - return super().create(request) + + serializer = EntryCreateSerializer(data=request.data, context={"_user": user}) + serializer.is_valid(raise_exception=True) + + job = Job.new_create_entry_v2(user, None, params=request.data) + job.run() + + return Response(status=status.HTTP_202_ACCEPTED) class EntityHistoryAPI(viewsets.ReadOnlyModelViewSet): diff --git a/entry/api_v2/serializers.py b/entry/api_v2/serializers.py index 7553367af..c146e2d54 100644 --- a/entry/api_v2/serializers.py +++ b/entry/api_v2/serializers.py @@ -7,6 +7,7 @@ import custom_view from acl.models import ACLBase +from airone.lib import drf from airone.lib.acl import ACLType from airone.lib.drf import ( DuplicatedObjectExistsError, @@ -205,7 +206,14 @@ def validate_name(self, name: str): def _validate(self, schema: Entity, attrs: list[dict[str, Any]]): # In create case, check attrs mandatory attribute if not self.instance: - user: User = self.context["request"].user + user: User | None = None + if "request" in self.context: + user = self.context["request"].user + if "_user" in self.context: + user = self.context["_user"] + if user is None: + raise RequiredParameterError("user is required") + for mandatory_attr in schema.attrs.filter(is_mandatory=True, is_active=True): if not user.has_permission(mandatory_attr, ACLType.Writable): raise PermissionDenied( @@ -256,7 +264,7 @@ class EntryCreateSerializer(EntryBaseSerializer): queryset=Entity.objects.all(), write_only=True, required=True ) attrs = serializers.ListField(child=AttributeDataSerializer(), write_only=True, required=False) - created_user = serializers.HiddenField(default=serializers.CurrentUserDefault()) + created_user = serializers.HiddenField(default=drf.AironeUserDefault()) class Meta: model = Entry @@ -267,7 +275,13 @@ def validate(self, params): return params def create(self, validated_data: EntryCreateData): - user: User = self.context["request"].user + user: User | None = None + if "request" in self.context: + user = self.context["request"].user + if "_user" in self.context: + user = self.context["_user"] + if user is None: + raise RequiredParameterError("user is required") entity_name = validated_data["schema"].name if custom_view.is_custom("before_create_entry_v2", entity_name): diff --git a/entry/tasks.py b/entry/tasks.py index 70da43d47..117f344e8 100644 --- a/entry/tasks.py +++ b/entry/tasks.py @@ -905,3 +905,18 @@ def notify_update_entry(self, job): @may_schedule_until_job_is_ready def notify_delete_entry(self, job): return _notify_event(notify_entry_delete, job.target.id, job.user) + + +@app.task(bind=True) +@may_schedule_until_job_is_ready +def create_entry_v2(self, job: Job) -> int: + serializer = EntryCreateSerializer(data=json.loads(job.params), context={"_user": job.user}) + if not serializer.is_valid(): + return Job.STATUS["ERROR"] + + try: + serializer.create(serializer.validated_data) + except Exception: + return Job.STATUS["ERROR"] + + return Job.STATUS["DONE"] diff --git a/frontend/src/repository/AironeApiClient.ts b/frontend/src/repository/AironeApiClient.ts index a375a66a7..682a0dcd7 100644 --- a/frontend/src/repository/AironeApiClient.ts +++ b/frontend/src/repository/AironeApiClient.ts @@ -361,8 +361,8 @@ class AironeApiClient { entityId: number, name: string, attrs: AttributeData[] - ): Promise { - return await this.entity.entityApiV2EntriesCreate( + ): Promise { + await this.entity.entityApiV2EntriesCreate( { entityId, entryCreate: { id: -1, name, attrs } }, { headers: { diff --git a/job/models.py b/job/models.py index da4ed6f21..1ad92d5ee 100644 --- a/job/models.py +++ b/job/models.py @@ -54,6 +54,9 @@ class JobOperation(Enum): CREATE_ENTITY_V2 = 24 EDIT_ENTITY_V2 = 25 DELETE_ENTITY_V2 = 26 + CREATE_ENTRY_V2 = 27 + EDIT_ENTRY_V2 = 28 + DELETE_ENTRY_V2 = 29 class JobTarget(Enum): @@ -354,6 +357,7 @@ def method_table(kls): JobOperation.CREATE_ENTITY_V2.value: entity_task.create_entity_v2, JobOperation.EDIT_ENTITY_V2.value: entity_task.edit_entity_v2, JobOperation.DELETE_ENTITY_V2.value: entity_task.delete_entity_v2, + JobOperation.CREATE_ENTRY_V2.value: entry_task.create_entry_v2, } return kls._METHOD_TABLE @@ -586,6 +590,36 @@ def new_delete_entity_v2(kls, user, target: Entity, text="", params={}): json.dumps(params, default=_support_time_default, sort_keys=True), ) + @classmethod + def new_create_entry_v2(kls, user, target, text="", params={}): + return kls._create_new_job( + user, + target, + JobOperation.CREATE_ENTRY_V2.value, + text, + json.dumps(params, default=_support_time_default, sort_keys=True), + ) + + @classmethod + def new_edit_entry_v2(kls, user, target: Entry, text="", params={}): + return kls._create_new_job( + user, + target, + JobOperation.EDIT_ENTRY_V2.value, + text, + json.dumps(params, default=_support_time_default, sort_keys=True), + ) + + @classmethod + def new_delete_entry_v2(kls, user, target: Entry, text="", params={}): + return kls._create_new_job( + user, + target, + JobOperation.DELETE_ENTRY_V2.value, + text, + json.dumps(params, default=_support_time_default, sort_keys=True), + ) + def set_cache(self, value): with default_storage.open("job_%d" % self.id, "wb") as fp: pickle.dump(value, fp) From 6c81f494472a51ca6b7cb4d62c2af9cd5a9bda6e Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 11:45:19 +0900 Subject: [PATCH 07/14] Support celery task for entry update on APIv2 --- entry/api_v2/serializers.py | 9 ++++++++- entry/api_v2/views.py | 17 ++++++++++++++++- entry/tasks.py | 21 +++++++++++++++++++++ frontend/src/repository/AironeApiClient.ts | 4 ++-- job/models.py | 1 + 5 files changed, 48 insertions(+), 4 deletions(-) diff --git a/entry/api_v2/serializers.py b/entry/api_v2/serializers.py index c146e2d54..f23742790 100644 --- a/entry/api_v2/serializers.py +++ b/entry/api_v2/serializers.py @@ -362,7 +362,14 @@ def validate(self, params): def update(self, entry: Entry, validated_data: EntryUpdateData): entry.set_status(Entry.STATUS_EDITING) - user: User = self.context["request"].user + + user: User | None = None + if "request" in self.context: + user = self.context["request"].user + if "_user" in self.context: + user = self.context["_user"] + if user is None: + raise RequiredParameterError("user is required") # for history record entry._history_user = user diff --git a/entry/api_v2/views.py b/entry/api_v2/views.py index eca9be6e6..5df02be03 100644 --- a/entry/api_v2/views.py +++ b/entry/api_v2/views.py @@ -97,12 +97,27 @@ class EntryAPI(viewsets.ModelViewSet): def get_serializer_class(self): serializer = { "retrieve": EntryRetrieveSerializer, - "update": EntryUpdateSerializer, + "update": serializers.Serializer, "copy": EntryCopySerializer, "list": EntryHistoryAttributeValueSerializer, } return serializer.get(self.action, EntryBaseSerializer) + @extend_schema(request=EntryUpdateSerializer) + def update(self, request, *args, **kwargs): + user: User = request.user + entry: Entry = self.get_object() + + serializer = EntryUpdateSerializer( + instance=entry, data=request.data, context={"_user": user} + ) + serializer.is_valid(raise_exception=True) + + job = Job.new_edit_entry_v2(user, entry, params=request.data) + job.run() + + return Response(status=status.HTTP_202_ACCEPTED) + def destroy(self, request, pk): entry: Entry = self.get_object() if not entry.is_active: diff --git a/entry/tasks.py b/entry/tasks.py index 117f344e8..5fd5cee61 100644 --- a/entry/tasks.py +++ b/entry/tasks.py @@ -920,3 +920,24 @@ def create_entry_v2(self, job: Job) -> int: return Job.STATUS["ERROR"] return Job.STATUS["DONE"] + + +@app.task(bind=True) +@may_schedule_until_job_is_ready +def edit_entry_v2(self, job: Job) -> int: + entry: Entry | None = Entry.objects.filter(id=job.target.id, is_active=True).first() + if not entry: + return Job.STATUS["ERROR"] + + serializer = EntryUpdateSerializer( + instance=entry, data=json.loads(job.params), context={"_user": job.user} + ) + if not serializer.is_valid(): + return Job.STATUS["ERROR"] + + try: + serializer.update(entry, serializer.validated_data) + except Exception: + return Job.STATUS["ERROR"] + + return Job.STATUS["DONE"] diff --git a/frontend/src/repository/AironeApiClient.ts b/frontend/src/repository/AironeApiClient.ts index 682a0dcd7..9c0eac1c9 100644 --- a/frontend/src/repository/AironeApiClient.ts +++ b/frontend/src/repository/AironeApiClient.ts @@ -377,8 +377,8 @@ class AironeApiClient { id: number, name: string, attrs: AttributeData[] - ): Promise { - return await this.entry.entryApiV2Update( + ): Promise { + await this.entry.entryApiV2Update( { id, entryUpdate: { id: id, name, attrs } }, { headers: { diff --git a/job/models.py b/job/models.py index 1ad92d5ee..4808966c5 100644 --- a/job/models.py +++ b/job/models.py @@ -358,6 +358,7 @@ def method_table(kls): JobOperation.EDIT_ENTITY_V2.value: entity_task.edit_entity_v2, JobOperation.DELETE_ENTITY_V2.value: entity_task.delete_entity_v2, JobOperation.CREATE_ENTRY_V2.value: entry_task.create_entry_v2, + JobOperation.EDIT_ENTRY_V2.value: entry_task.edit_entry_v2, } return kls._METHOD_TABLE From d9fc3dc9be07224914216ed6cd03026a56a26cfc Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 12:00:24 +0900 Subject: [PATCH 08/14] Support celery task for entry delete on APIv2 --- entry/api_v2/views.py | 18 +++++------------- entry/tasks.py | 23 +++++++++++++++++++++++ job/models.py | 1 + 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/entry/api_v2/views.py b/entry/api_v2/views.py index 5df02be03..cb99c4863 100644 --- a/entry/api_v2/views.py +++ b/entry/api_v2/views.py @@ -47,24 +47,16 @@ from user.models import User -def delete_entry_with_notifucation(user, entry): +def delete_entry_with_notifucation(user: User, entry: Entry): """ This implements whole processing related with Entry's deletion such as - - running custom_view processing + - invoking job about deletion - invoking job about notification """ - if custom_view.is_custom("before_delete_entry_v2", entry.schema.name): - custom_view.call_custom("before_delete_entry_v2", entry.schema.name, user, entry) - - # register operation History for deleting entry - user.seth_entry_del(entry) - - # delete entry - entry.delete(deleted_user=user) - - if custom_view.is_custom("after_delete_entry_v2", entry.schema.name): - custom_view.call_custom("after_delete_entry_v2", entry.schema.name, user, entry) + # delete an entry + job: Job = Job.new_delete_entry_v2(user, entry) + job.run() # Send notification to the webhook URL job_notify: Job = Job.new_notify_delete_entry(user, entry) diff --git a/entry/tasks.py b/entry/tasks.py index 5fd5cee61..f21266414 100644 --- a/entry/tasks.py +++ b/entry/tasks.py @@ -941,3 +941,26 @@ def edit_entry_v2(self, job: Job) -> int: return Job.STATUS["ERROR"] return Job.STATUS["DONE"] + + +@app.task(bind=True) +@may_schedule_until_job_is_ready +def delete_entry_v2(self, job: Job) -> int: + entry: Entry | None = Entry.objects.filter(id=job.target.id, is_active=True).first() + if not entry: + return Job.STATUS["ERROR"] + + if custom_view.is_custom("before_delete_entry_v2", entry.schema.name): + custom_view.call_custom("before_delete_entry_v2", entry.schema.name, job.user, entry) + + try: + # register operation History for deleting entry + job.user.seth_entry_del(entry) + entry.delete() + except Exception: + return Job.STATUS["ERROR"] + + if custom_view.is_custom("after_delete_entry_v2", entry.schema.name): + custom_view.call_custom("after_delete_entry_v2", entry.schema.name, job.user, entry) + + return Job.STATUS["DONE"] diff --git a/job/models.py b/job/models.py index 4808966c5..74e970cf7 100644 --- a/job/models.py +++ b/job/models.py @@ -359,6 +359,7 @@ def method_table(kls): JobOperation.DELETE_ENTITY_V2.value: entity_task.delete_entity_v2, JobOperation.CREATE_ENTRY_V2.value: entry_task.create_entry_v2, JobOperation.EDIT_ENTRY_V2.value: entry_task.edit_entry_v2, + JobOperation.DELETE_ENTRY_V2.value: entry_task.delete_entry_v2, } return kls._METHOD_TABLE From 369f06de2f1463985332e39c7cdb666f0b1b985e Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 12:34:04 +0900 Subject: [PATCH 09/14] Fix test for entry create --- entity/tests/test_api_v2.py | 39 ++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/entity/tests/test_api_v2.py b/entity/tests/test_api_v2.py index 706465f62..752d05a27 100644 --- a/entity/tests/test_api_v2.py +++ b/entity/tests/test_api_v2.py @@ -17,6 +17,7 @@ from entity import tasks from entity.models import Entity, EntityAttr from entry.models import Entry +from entry.tasks import create_entry_v2 from group.models import Group from role.models import Role from trigger import tasks as trigger_tasks @@ -2834,6 +2835,7 @@ def test_list_entry_with_invalid_param(self): self.assertEqual(resp.status_code, 404) self.assertEqual(resp.json(), {"code": "AE-230000", "message": "Not found."}) + @mock.patch("entry.tasks.create_entry_v2.delay", mock.Mock(side_effect=create_entry_v2)) def test_create_entry(self): attr = {} for attr_name in [x["name"] for x in self.ALL_TYPED_ATTR_PARAMS_FOR_CREATING_ENTITY]: @@ -2860,16 +2862,9 @@ def test_create_entry(self): resp = self.client.post( "/entity/api/v2/%s/entries/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) - entry: Entry = Entry.objects.get(id=resp.json()["id"], is_active=True) - self.assertEqual( - resp.json(), - { - "id": entry.id, - "name": "entry1", - }, - ) + entry: Entry = Entry.objects.get(name=params["name"], is_active=True) self.assertEqual(entry.schema, self.entity) self.assertEqual(entry.created_user, self.user) self.assertEqual(entry.status, 0) @@ -2937,8 +2932,9 @@ def test_create_entry_without_permission_entity(self): resp = self.client.post( "/entity/api/v2/%s/entries/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + @mock.patch("entry.tasks.create_entry_v2.delay", mock.Mock(side_effect=create_entry_v2)) def test_create_entry_without_permission_entity_attr(self): attr = {} for attr_name in [x["name"] for x in self.ALL_TYPED_ATTR_PARAMS_FOR_CREATING_ENTITY]: @@ -2958,9 +2954,9 @@ def test_create_entry_without_permission_entity_attr(self): json.dumps({**params, "name": "entry1"}), "application/json", ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) - entry: Entry = Entry.objects.get(id=resp.json()["id"], is_active=True) + entry: Entry = Entry.objects.get(name="entry1", is_active=True) self.assertEqual(entry.attrs.get(schema=attr["val"]).get_latest_value().get_value(), "hoge") self.assertEqual(entry.attrs.get(schema=attr["vals"]).get_latest_value().get_value(), []) @@ -3027,7 +3023,7 @@ def test_create_entry_with_invalid_param_name(self): json.dumps({"name": "a" * (Entry._meta.get_field("name").max_length)}), "application/json", ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) entry = self.add_entry(self.user, "hoge", self.entity) resp = self.client.post( @@ -3047,7 +3043,7 @@ def test_create_entry_with_invalid_param_name(self): json.dumps({"name": "hoge"}), "application/json", ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) def test_create_entry_with_invalid_param_attrs(self): attr = {} @@ -3188,8 +3184,9 @@ def test_create_entry_with_invalid_param_attrs(self): resp = self.client.post( "/entity/api/v2/%s/entries/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + @mock.patch("entry.tasks.create_entry_v2.delay", mock.Mock(side_effect=create_entry_v2)) @mock.patch("entry.tasks.notify_create_entry.delay") def test_create_entry_notify(self, mock_task): self.client.post( @@ -3200,6 +3197,7 @@ def test_create_entry_notify(self, mock_task): self.assertTrue(mock_task.called) + @mock.patch("entry.tasks.create_entry_v2.delay", mock.Mock(side_effect=create_entry_v2)) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") def test_create_entry_with_customview(self, mock_call_custom): @@ -3220,8 +3218,8 @@ def side_effect(handler_name, entity_name, user, *args): resp = self.client.post( "/entity/api/v2/%s/entries/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json(), [{"code": "AE-121000", "message": "create error"}]) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, *args): # Check specified parameters are expected @@ -3242,7 +3240,7 @@ def side_effect(handler_name, entity_name, user, *args): resp = self.client.post( "/entity/api/v2/%s/entries/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertTrue(mock_call_custom.called) @mock.patch("entity.tasks.create_entity.delay", mock.Mock(side_effect=tasks.create_entity)) @@ -3480,6 +3478,7 @@ def test_get_entity_attr_names(self): resp = self.client.get("/entity/api/v2/attrs?entity_ids=9999") self.assertEqual(resp.status_code, 400) + @mock.patch("entry.tasks.create_entry_v2.delay", mock.Mock(side_effect=create_entry_v2)) @mock.patch( "trigger.tasks.may_invoke_trigger.delay", mock.Mock(side_effect=trigger_tasks.may_invoke_trigger), @@ -3510,10 +3509,10 @@ def test_create_entry_when_trigger_is_set(self): resp = self.client.post( "/entity/api/v2/%s/entries/" % self.entity.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 201) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) # check Attribute "vals", which is specified by TriggerCondition, was changed as expected - entry: Entry = Entry.objects.get(id=resp.json()["id"], is_active=True) + entry: Entry = Entry.objects.get(name=params["name"], is_active=True) self.assertEqual(entry.get_attrv("text").value, "hogefuga") self.assertEqual( [x.value for x in entry.get_attrv("vals").data_array.all()], ["fuga", "piyo"] From 31a693ae440a7b5cfd1e096946de4e49fbcf4ba6 Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 23:16:47 +0900 Subject: [PATCH 10/14] Fix test for entry update/delete --- entry/api_v2/views.py | 22 +---- entry/tasks.py | 16 ++-- entry/tests/test_api_v2.py | 186 +++++++++++++++++++------------------ 3 files changed, 109 insertions(+), 115 deletions(-) diff --git a/entry/api_v2/views.py b/entry/api_v2/views.py index cb99c4863..4e0decc82 100644 --- a/entry/api_v2/views.py +++ b/entry/api_v2/views.py @@ -47,22 +47,6 @@ from user.models import User -def delete_entry_with_notifucation(user: User, entry: Entry): - """ - This implements whole processing related with Entry's deletion such as - - invoking job about deletion - - invoking job about notification - """ - - # delete an entry - job: Job = Job.new_delete_entry_v2(user, entry) - job.run() - - # Send notification to the webhook URL - job_notify: Job = Job.new_notify_delete_entry(user, entry) - job_notify.run() - - class EntryPermission(BasePermission): def has_object_permission(self, request, view, obj): user: User = request.user @@ -117,7 +101,8 @@ def destroy(self, request, pk): user: User = request.user - delete_entry_with_notifucation(user, entry) + job: Job = Job.new_delete_entry_v2(user, entry) + job.run() return Response(status=status.HTTP_204_NO_CONTENT) @@ -583,6 +568,7 @@ def delete(self, request, *args, **kwargs): raise PermissionDenied("deleting some entries is not allowed") for entry in entries: - delete_entry_with_notifucation(user, entry) + job: Job = Job.new_delete_entry_v2(user, entry) + job.run() return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/entry/tasks.py b/entry/tasks.py index f21266414..3dc552828 100644 --- a/entry/tasks.py +++ b/entry/tasks.py @@ -950,17 +950,21 @@ def delete_entry_v2(self, job: Job) -> int: if not entry: return Job.STATUS["ERROR"] - if custom_view.is_custom("before_delete_entry_v2", entry.schema.name): - custom_view.call_custom("before_delete_entry_v2", entry.schema.name, job.user, entry) - try: + if custom_view.is_custom("before_delete_entry_v2", entry.schema.name): + custom_view.call_custom("before_delete_entry_v2", entry.schema.name, job.user, entry) + # register operation History for deleting entry job.user.seth_entry_del(entry) - entry.delete() + entry.delete(deleted_user=job.user) + + if custom_view.is_custom("after_delete_entry_v2", entry.schema.name): + custom_view.call_custom("after_delete_entry_v2", entry.schema.name, job.user, entry) except Exception: return Job.STATUS["ERROR"] - if custom_view.is_custom("after_delete_entry_v2", entry.schema.name): - custom_view.call_custom("after_delete_entry_v2", entry.schema.name, job.user, entry) + # Send notification to the webhook URL + job_notify: Job = Job.new_notify_delete_entry(job.user, entry) + job_notify.run() return Job.STATUS["DONE"] diff --git a/entry/tests/test_api_v2.py b/entry/tests/test_api_v2.py index 623d85b6c..73c5b6b92 100644 --- a/entry/tests/test_api_v2.py +++ b/entry/tests/test_api_v2.py @@ -7,6 +7,7 @@ from unittest.mock import Mock, patch import yaml +from rest_framework import status from rest_framework.exceptions import ValidationError from airone.lib.log import Logger @@ -670,6 +671,7 @@ def test_retrieve_entry_with_deleted_referrals(self): }, ) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) def test_update_entry(self): entry: Entry = self.add_entry(self.user, "entry", self.entity) @@ -698,16 +700,8 @@ def test_update_entry(self): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) - self.assertEqual( - resp.json(), - { - "id": entry.id, - "name": "entry-change", - "delay_trigger": True, - }, - ) self.assertEqual(entry.status, 0) self.assertEqual( { @@ -774,7 +768,7 @@ def test_update_entry_without_permission(self): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) # permission nothing entry entry.is_public = False @@ -810,8 +804,9 @@ def test_update_entry_without_permission(self): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) def test_update_entry_without_permission_attr(self): entry: Entry = self.add_entry(self.user, "entry", self.entity) entity_attr = {} @@ -832,7 +827,7 @@ def test_update_entry_without_permission_attr(self): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertEqual(attr["val"].get_latest_value().get_value(), "hoge") self.assertEqual(attr["vals"].get_latest_value().get_value(), []) @@ -850,7 +845,7 @@ def test_update_entry_without_permission_attr(self): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertEqual(attr["val"].get_latest_value().get_value(), "fuga") self.assertEqual(attr["vals"].get_latest_value().get_value(), []) @@ -891,7 +886,7 @@ def test_update_entry_with_invalid_param_name(self): json.dumps({"name": "a" * (Entry._meta.get_field("name").max_length)}), "application/json", ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) hoge_entry: Entry = self.add_entry(self.user, "hoge", self.entity) resp = self.client.put( @@ -925,7 +920,7 @@ def test_update_entry_with_invalid_param_name(self): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps({"name": "hoge"}), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) def test_update_entry_with_invalid_param_attrs(self): entry: Entry = self.add_entry(self.user, "entry", self.entity) @@ -1025,6 +1020,7 @@ def test_update_entry_with_invalid_param_attrs(self): self.assertEqual(resp.status_code, 400) self.assertEqual(resp.json(), test_value["error_msg"]) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) @mock.patch("entry.tasks.notify_update_entry.delay") def test_update_entry_notify(self, mock_task): entry: Entry = self.add_entry(self.user, "entry", self.entity) @@ -1034,6 +1030,7 @@ def test_update_entry_notify(self, mock_task): self.assertTrue(mock_task.called) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") def test_update_entry_with_customview(self, mock_call_custom): @@ -1056,8 +1053,8 @@ def side_effect(handler_name, entity_name, user, *args): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json(), [{"code": "AE-121000", "message": "update error"}]) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, *args): self.assertEqual(entity_name, self.entity.name) @@ -1075,7 +1072,7 @@ def side_effect(handler_name, entity_name, user, *args): resp = self.client.put( "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) self.assertTrue(mock_call_custom.called) @mock.patch("entry.tasks.notify_update_entry.delay") @@ -1112,6 +1109,7 @@ def test_update_entry_with_no_update(self, mock_task): @mock.patch( "entry.tasks.register_referrals.delay", mock.Mock(side_effect=tasks.register_referrals) ) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) def test_update_entry_with_referral(self): self.add_entry( self.user, @@ -1137,8 +1135,49 @@ def test_update_entry_without_attrs(self): resp = self.client.put( "/entry/api/v2/%s/" % self.ref_entry.id, json.dumps({}), "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) + @mock.patch( + "trigger.tasks.may_invoke_trigger.delay", + mock.Mock(side_effect=trigger_tasks.may_invoke_trigger), + ) + def test_update_entry_when_trigger_is_set(self): + # create Entry to be updated in this test + entry: Entry = self.add_entry(self.user, "entry", self.entity) + + attr = {} + for attr_name in [x["name"] for x in self.ALL_TYPED_ATTR_PARAMS_FOR_CREATING_ENTITY]: + attr[attr_name] = self.entity.attrs.get(name=attr_name) + + # register Trigger and Action that specify "fuga" at text attribute + # when value "hoge" is set to the Attribute "val". + TriggerCondition.register( + self.entity, + [ + {"attr_id": self.entity.attrs.get(name="val").id, "cond": "hoge"}, + ], + [ + {"attr_id": self.entity.attrs.get(name="text").id, "value": "fuga"}, + ], + ) + + # send request to update Entry + params = { + "name": "entry-change", + "attrs": [ + {"id": attr["val"].id, "value": "hoge"}, + ], + } + resp = self.client.put( + "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" + ) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + + # check Attribute "text", which is specified by TriggerCondition, was changed to "fuga" + self.assertEqual(entry.get_attrv("text").value, "fuga") + + @patch("entry.tasks.delete_entry_v2.delay", Mock(side_effect=tasks.delete_entry_v2)) def test_destroy_entry(self): entry: Entry = self.add_entry(self.user, "entry", self.entity) @@ -1233,6 +1272,7 @@ def test_destory_entry_with_invalid_param(self): self.assertEqual(resp.status_code, 404) self.assertEqual(resp.json(), {"code": "AE-230000", "message": "Not found."}) + @patch("entry.tasks.delete_entry_v2.delay", Mock(side_effect=tasks.delete_entry_v2)) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") def test_destroy_entry_with_custom_view(self, mock_call_custom): @@ -1243,8 +1283,8 @@ def side_effect(handler_name, entity_name, user, entry): mock_call_custom.side_effect = side_effect resp = self.client.delete("/entry/api/v2/%s/" % entry.id, None, "application/json") - self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json(), [{"code": "AE-121000", "message": "delete error"}]) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, entry): self.assertTrue(handler_name in ["before_delete_entry_v2", "after_delete_entry_v2"]) @@ -1254,7 +1294,7 @@ def side_effect(handler_name, entity_name, user, entry): mock_call_custom.side_effect = side_effect resp = self.client.delete("/entry/api/v2/%s/" % entry.id, None, "application/json") - self.assertEqual(resp.status_code, 204) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) self.assertTrue(mock_call_custom.called) @mock.patch("entry.tasks.notify_delete_entry.delay") @@ -1402,6 +1442,34 @@ def test_restore_entry_notify(self, mock_task): self.assertTrue(mock_task.called) + @patch("entry.tasks.edit_entry_v2.delay", Mock(side_effect=tasks.edit_entry_v2)) + def test_restore_entry_attribute_value(self): + entry: Entry = self.add_entry(self.user, "entry", self.entity) + + attr = self.entity.attrs.get(name="val") + prev_attrv = entry.attrs.get(schema=attr).get_latest_value() + + # update + params = { + "name": "entry-change", + "attrs": [ + {"id": attr.id, "value": "updated"}, + ], + } + resp = self.client.put( + "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" + ) + self.assertEqual(resp.status_code, status.HTTP_202_ACCEPTED) + + # restore to previous value + resp = self.client.patch( + "/entry/api/v2/%s/attrv_restore/" % prev_attrv.id, json.dumps({}), "application/json" + ) + self.assertEqual(resp.status_code, 200) + + attrv = entry.attrs.get(schema=attr).get_latest_value() + self.assertEqual(attrv.value, prev_attrv.value) + @mock.patch("entry.tasks.copy_entry.delay", mock.Mock(side_effect=tasks.copy_entry)) def test_copy_entry(self): entry: Entry = self.add_entry(self.user, "entry", self.entity) @@ -3105,33 +3173,6 @@ def test_advanced_search_with_too_long_keyword(self): ) self.assertEqual(resp.status_code, 400) - def test_restore_entry_attribute_value(self): - entry: Entry = self.add_entry(self.user, "entry", self.entity) - - attr = self.entity.attrs.get(name="val") - prev_attrv = entry.attrs.get(schema=attr).get_latest_value() - - # update - params = { - "name": "entry-change", - "attrs": [ - {"id": attr.id, "value": "updated"}, - ], - } - resp = self.client.put( - "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" - ) - self.assertEqual(resp.status_code, 200) - - # restore to previous value - resp = self.client.patch( - "/entry/api/v2/%s/attrv_restore/" % prev_attrv.id, json.dumps({}), "application/json" - ) - self.assertEqual(resp.status_code, 200) - - attrv = entry.attrs.get(schema=attr).get_latest_value() - self.assertEqual(attrv.value, prev_attrv.value) - @patch( "entry.tasks.export_search_result_v2.delay", Mock(side_effect=tasks.export_search_result_v2) ) @@ -4411,6 +4452,7 @@ def test_entry_history_without_permission(self): ) ) + @patch("entry.tasks.delete_entry_v2.delay", Mock(side_effect=tasks.delete_entry_v2)) def test_destroy_entries(self): entry1: Entry = self.add_entry(self.user, "entry1", self.entity) entry2: Entry = self.add_entry(self.user, "entry2", self.entity) @@ -4502,6 +4544,7 @@ def test_destory_entries_with_invalid_param(self): ) self.assertEqual(resp.status_code, 404) + @patch("entry.tasks.delete_entry_v2.delay", Mock(side_effect=tasks.delete_entry_v2)) @mock.patch("custom_view.is_custom", mock.Mock(return_value=True)) @mock.patch("custom_view.call_custom") def test_destroy_entries_with_custom_view(self, mock_call_custom): @@ -4514,8 +4557,8 @@ def side_effect(handler_name, entity_name, user, entry): resp = self.client.delete( "/entry/api/v2/bulk_delete/?ids=%s" % entry.id, None, "application/json" ) - self.assertEqual(resp.status_code, 400) - self.assertEqual(resp.json(), [{"code": "AE-121000", "message": "delete error"}]) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + self.assertTrue(mock_call_custom.called) def side_effect(handler_name, entity_name, user, entry): self.assertTrue(handler_name in ["before_delete_entry_v2", "after_delete_entry_v2"]) @@ -4527,7 +4570,7 @@ def side_effect(handler_name, entity_name, user, entry): resp = self.client.delete( "/entry/api/v2/bulk_delete/?ids=%s" % entry.id, None, "application/json" ) - self.assertEqual(resp.status_code, 204) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) self.assertTrue(mock_call_custom.called) @mock.patch("entry.tasks.notify_delete_entry.delay") @@ -4536,42 +4579,3 @@ def test_destroy_entries_notify(self, mock_task): self.client.delete("/entry/api/v2/bulk_delete/?ids=%s" % entry.id, None, "application/json") self.assertTrue(mock_task.called) - - @mock.patch( - "trigger.tasks.may_invoke_trigger.delay", - mock.Mock(side_effect=trigger_tasks.may_invoke_trigger), - ) - def test_update_entry_when_trigger_is_set(self): - # create Entry to be updated in this test - entry: Entry = self.add_entry(self.user, "entry", self.entity) - - attr = {} - for attr_name in [x["name"] for x in self.ALL_TYPED_ATTR_PARAMS_FOR_CREATING_ENTITY]: - attr[attr_name] = self.entity.attrs.get(name=attr_name) - - # register Trigger and Action that specify "fuga" at text attribute - # when value "hoge" is set to the Attribute "val". - TriggerCondition.register( - self.entity, - [ - {"attr_id": self.entity.attrs.get(name="val").id, "cond": "hoge"}, - ], - [ - {"attr_id": self.entity.attrs.get(name="text").id, "value": "fuga"}, - ], - ) - - # send request to update Entry - params = { - "name": "entry-change", - "attrs": [ - {"id": attr["val"].id, "value": "hoge"}, - ], - } - resp = self.client.put( - "/entry/api/v2/%s/" % entry.id, json.dumps(params), "application/json" - ) - self.assertEqual(resp.status_code, 200) - - # check Attribute "text", which is specified by TriggerCondition, was changed to "fuga" - self.assertEqual(entry.get_attrv("text").value, "fuga") From 3eb9f43e94863d98e65f03921351a71ce83021cd Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 23:32:31 +0900 Subject: [PATCH 11/14] Simplify tasks --- airone/lib/job.py | 7 +++++-- entry/tasks.py | 29 ++++++++++------------------- entry/tests/test_api_v2.py | 2 ++ 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/airone/lib/job.py b/airone/lib/job.py index 8ce7f855f..7fa040cac 100644 --- a/airone/lib/job.py +++ b/airone/lib/job.py @@ -11,8 +11,11 @@ def wrapper(kls, job_id): # update Job status from PREPARING to PROCEEDING job.update(Job.STATUS["PROCESSING"]) - # running Job processing - ret = func(kls, job) + try: + # running Job processing + ret: int | tuple = func(kls, job) + except Exception: + ret = Job.STATUS["ERROR"] # update Job status after finishing Job processing if isinstance(ret, int): diff --git a/entry/tasks.py b/entry/tasks.py index 3dc552828..d2bfc7b15 100644 --- a/entry/tasks.py +++ b/entry/tasks.py @@ -914,10 +914,7 @@ def create_entry_v2(self, job: Job) -> int: if not serializer.is_valid(): return Job.STATUS["ERROR"] - try: - serializer.create(serializer.validated_data) - except Exception: - return Job.STATUS["ERROR"] + serializer.create(serializer.validated_data) return Job.STATUS["DONE"] @@ -935,10 +932,7 @@ def edit_entry_v2(self, job: Job) -> int: if not serializer.is_valid(): return Job.STATUS["ERROR"] - try: - serializer.update(entry, serializer.validated_data) - except Exception: - return Job.STATUS["ERROR"] + serializer.update(entry, serializer.validated_data) return Job.STATUS["DONE"] @@ -950,21 +944,18 @@ def delete_entry_v2(self, job: Job) -> int: if not entry: return Job.STATUS["ERROR"] - try: - if custom_view.is_custom("before_delete_entry_v2", entry.schema.name): - custom_view.call_custom("before_delete_entry_v2", entry.schema.name, job.user, entry) - - # register operation History for deleting entry - job.user.seth_entry_del(entry) - entry.delete(deleted_user=job.user) + if custom_view.is_custom("before_delete_entry_v2", entry.schema.name): + custom_view.call_custom("before_delete_entry_v2", entry.schema.name, job.user, entry) - if custom_view.is_custom("after_delete_entry_v2", entry.schema.name): - custom_view.call_custom("after_delete_entry_v2", entry.schema.name, job.user, entry) - except Exception: - return Job.STATUS["ERROR"] + # register operation History for deleting entry + job.user.seth_entry_del(entry) + entry.delete(deleted_user=job.user) # Send notification to the webhook URL job_notify: Job = Job.new_notify_delete_entry(job.user, entry) job_notify.run() + if custom_view.is_custom("after_delete_entry_v2", entry.schema.name): + custom_view.call_custom("after_delete_entry_v2", entry.schema.name, job.user, entry) + return Job.STATUS["DONE"] diff --git a/entry/tests/test_api_v2.py b/entry/tests/test_api_v2.py index 73c5b6b92..47e61f6bf 100644 --- a/entry/tests/test_api_v2.py +++ b/entry/tests/test_api_v2.py @@ -1297,6 +1297,7 @@ def side_effect(handler_name, entity_name, user, entry): self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) self.assertTrue(mock_call_custom.called) + @patch("entry.tasks.delete_entry_v2.delay", Mock(side_effect=tasks.delete_entry_v2)) @mock.patch("entry.tasks.notify_delete_entry.delay") def test_destroy_entry_notify(self, mock_task): entry: Entry = self.add_entry(self.user, "entry", self.entity) @@ -4573,6 +4574,7 @@ def side_effect(handler_name, entity_name, user, entry): self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) self.assertTrue(mock_call_custom.called) + @patch("entry.tasks.delete_entry_v2.delay", Mock(side_effect=tasks.delete_entry_v2)) @mock.patch("entry.tasks.notify_delete_entry.delay") def test_destroy_entries_notify(self, mock_task): entry: Entry = self.add_entry(self.user, "entry", self.entity) From d78d37fac1953d7ec82c6e0c2a5fe81bcc0beb1e Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 23:37:39 +0900 Subject: [PATCH 12/14] Simplify tasks on entity v2 --- entity/tasks.py | 92 +++++++++++++++---------------------------------- 1 file changed, 27 insertions(+), 65 deletions(-) diff --git a/entity/tasks.py b/entity/tasks.py index cac47586f..e114a131d 100644 --- a/entity/tasks.py +++ b/entity/tasks.py @@ -2,6 +2,7 @@ import custom_view from airone.celery import app +from airone.lib.job import may_schedule_until_job_is_ready from airone.lib.types import AttrTypeValue from entity.api_v2.serializers import EntityCreateSerializer, EntityUpdateSerializer from entity.models import Entity, EntityAttr @@ -242,40 +243,21 @@ def delete_entity(self, job_id): @app.task(bind=True) -def create_entity_v2(self, job_id: int): - job = Job.objects.get(id=job_id) - - if not job.proceed_if_ready(): - return - - # At the first time, update job status to prevent executing this job duplicately - job.update(Job.STATUS["PROCESSING"]) - +@may_schedule_until_job_is_ready +def create_entity_v2(self, job: Job): serializer = EntityCreateSerializer(data=json.loads(job.params), context={"_user": job.user}) if not serializer.is_valid(): - job.update(Job.STATUS["ERROR"]) - return + return Job.STATUS["ERROR"] - try: - serializer.create(serializer.validated_data) - except Exception: - job.update(Job.STATUS["ERROR"]) - return + serializer.create(serializer.validated_data) # update job status and save it - job.update(Job.STATUS["DONE"]) + return Job.STATUS["DONE"] @app.task(bind=True) -def edit_entity_v2(self, job_id: int): - job = Job.objects.get(id=job_id) - - if not job.proceed_if_ready(): - return - - # At the first time, update job status to prevent executing this job duplicately - job.update(Job.STATUS["PROCESSING"]) - +@may_schedule_until_job_is_ready +def edit_entity_v2(self, job: Job): entity: Entity | None = Entity.objects.filter(id=job.target.id, is_active=True).first() if not entity: job.update(Job.STATUS["ERROR"]) @@ -285,54 +267,34 @@ def edit_entity_v2(self, job_id: int): instance=entity, data=json.loads(job.params), context={"_user": job.user} ) if not serializer.is_valid(): - job.update(Job.STATUS["ERROR"]) - return + return Job.STATUS["ERROR"] - try: - serializer.update(entity, serializer.validated_data) - except Exception: - job.update(Job.STATUS["ERROR"]) - return + serializer.update(entity, serializer.validated_data) - # update job status and save it - job.update(Job.STATUS["DONE"]) + return Job.STATUS["DONE"] @app.task(bind=True) -def delete_entity_v2(self, job_id: int): - job = Job.objects.get(id=job_id) - - if not job.proceed_if_ready(): - return - - # At the first time, update job status to prevent executing this job duplicately - job.update(Job.STATUS["PROCESSING"]) - +@may_schedule_until_job_is_ready +def delete_entity_v2(self, job: Job): entity: Entity | None = Entity.objects.filter(id=job.target.id, is_active=True).first() if not entity: - job.update(Job.STATUS["ERROR"]) - return + return Job.STATUS["ERROR"] - try: - if custom_view.is_custom("before_delete_entity_v2"): - custom_view.call_custom("before_delete_entity_v2", None, job.user, entity) + if custom_view.is_custom("before_delete_entity_v2"): + custom_view.call_custom("before_delete_entity_v2", None, job.user, entity) - # register operation History for deleting entity - history: History = job.user.seth_entity_del(entity) + # register operation History for deleting entity + history: History = job.user.seth_entity_del(entity) + entity.delete() - entity.delete() + # Delete all attributes which target Entity have + entity_attr: EntityAttr + for entity_attr in entity.attrs.filter(is_active=True): + history.del_attr(entity_attr) + entity_attr.delete() - # Delete all attributes which target Entity have - entity_attr: EntityAttr - for entity_attr in entity.attrs.filter(is_active=True): - history.del_attr(entity_attr) - entity_attr.delete() - - if custom_view.is_custom("after_delete_entity_v2"): - custom_view.call_custom("after_delete_entity_v2", None, job.user, entity) - except Exception: - job.update(Job.STATUS["ERROR"]) - return + if custom_view.is_custom("after_delete_entity_v2"): + custom_view.call_custom("after_delete_entity_v2", None, job.user, entity) - # update job status and save it - job.update(Job.STATUS["DONE"]) + return Job.STATUS["DONE"] From 3773dd741886ada795f704e41cbb807cd8a59560 Mon Sep 17 00:00:00 2001 From: Ryo Okubo Date: Fri, 8 Mar 2024 23:50:38 +0900 Subject: [PATCH 13/14] Update apiclient --- apiclient/typescript-fetch/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apiclient/typescript-fetch/package.json b/apiclient/typescript-fetch/package.json index 539ef60a0..ea2382a85 100644 --- a/apiclient/typescript-fetch/package.json +++ b/apiclient/typescript-fetch/package.json @@ -1,6 +1,6 @@ { "name": "@dmm-com/airone-apiclient-typescript-fetch", - "version": "0.0.9", + "version": "0.0.10", "description": "AirOne APIv2 client in TypeScript", "main": "src/autogenerated/index.ts", "scripts": { From 7f0b4065e6f7e9b292859a74837981dc6b482f80 Mon Sep 17 00:00:00 2001 From: hinashi Date: Tue, 12 Mar 2024 10:47:19 +0900 Subject: [PATCH 14/14] Updated version of airone api client package --- frontend/src/repository/AironeApiClient.ts | 4 ---- package-lock.json | 14 +++++++------- package.json | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/frontend/src/repository/AironeApiClient.ts b/frontend/src/repository/AironeApiClient.ts index 9c0eac1c9..b66c62db7 100644 --- a/frontend/src/repository/AironeApiClient.ts +++ b/frontend/src/repository/AironeApiClient.ts @@ -12,15 +12,11 @@ import { EntityApiV2ListRequest, EntityAttrCreate, EntityAttrUpdate, - EntityCreate, EntityDetail, - EntityUpdate, EntryApi, EntryBase, EntryCopy, - EntryCreate, EntryRetrieve, - EntryUpdate, GetEntryAttrReferral, Group, GroupApi, diff --git a/package-lock.json b/package-lock.json index c31f17dd9..c42d350cb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "1.0.0", "license": "GPLv2", "dependencies": { - "@dmm-com/airone-apiclient-typescript-fetch": "^0.0.9", + "@dmm-com/airone-apiclient-typescript-fetch": "^0.0.10", "@emotion/react": "^11.10.5", "@emotion/styled": "^11.10.5", "@jest/globals": "^27.0.6", @@ -2012,9 +2012,9 @@ } }, "node_modules/@dmm-com/airone-apiclient-typescript-fetch": { - "version": "0.0.9", - "resolved": "https://npm.pkg.github.com/download/@dmm-com/airone-apiclient-typescript-fetch/0.0.9/d75af5b5d7bb85a562280afb804f1ce9f4abbbb2", - "integrity": "sha512-7fjbLKA7ugKkb4E4uCAQFj6hR527pySpsGEb0SbF4McTFTQMg58ql64wMySBwFDXvrBb2oLo2Q+ddmS5K6EzIQ==", + "version": "0.0.10", + "resolved": "https://npm.pkg.github.com/download/@dmm-com/airone-apiclient-typescript-fetch/0.0.10/358ffaca2f588e67752f276d4136a9d20851b469", + "integrity": "sha512-bc92q7+jdg0Nl2TmKpfM46oaKuDPOvMVyH5Yk4+0hn4u/0sBftKzC0xc4YecdRzCOt48VWDE0aoY10UVhIr+Fg==", "license": "ISC" }, "node_modules/@emotion/babel-plugin": { @@ -18870,9 +18870,9 @@ "dev": true }, "@dmm-com/airone-apiclient-typescript-fetch": { - "version": "0.0.9", - "resolved": "https://npm.pkg.github.com/download/@dmm-com/airone-apiclient-typescript-fetch/0.0.9/d75af5b5d7bb85a562280afb804f1ce9f4abbbb2", - "integrity": "sha512-7fjbLKA7ugKkb4E4uCAQFj6hR527pySpsGEb0SbF4McTFTQMg58ql64wMySBwFDXvrBb2oLo2Q+ddmS5K6EzIQ==" + "version": "0.0.10", + "resolved": "https://npm.pkg.github.com/download/@dmm-com/airone-apiclient-typescript-fetch/0.0.10/358ffaca2f588e67752f276d4136a9d20851b469", + "integrity": "sha512-bc92q7+jdg0Nl2TmKpfM46oaKuDPOvMVyH5Yk4+0hn4u/0sBftKzC0xc4YecdRzCOt48VWDE0aoY10UVhIr+Fg==" }, "@emotion/babel-plugin": { "version": "11.11.0", diff --git a/package.json b/package.json index 9104e2973..a9b5a5728 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,7 @@ "zod": "^3.22.4" }, "dependencies": { - "@dmm-com/airone-apiclient-typescript-fetch": "^0.0.9", + "@dmm-com/airone-apiclient-typescript-fetch": "^0.0.10", "@emotion/react": "^11.10.5", "@emotion/styled": "^11.10.5", "@jest/globals": "^27.0.6",