Skip to content

Commit

Permalink
Merge pull request #974 from syucream/feature/import-limit-on-entries
Browse files Browse the repository at this point in the history
Limit freqnent import jobs for same entity by default
  • Loading branch information
userlocalhost authored Oct 13, 2023
2 parents b7bab12 + 6ba9979 commit cfc944e
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 58 deletions.
4 changes: 4 additions & 0 deletions airone/lib/drf.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ class InvalidValueError(ValidationError):
default_code = "AE-250000"


class FrequentImportError(ValidationError):
default_code = "AE-260000"


def custom_exception_handler(exc, context):
def _convert_error_code(detail):
if isinstance(detail, list):
Expand Down
2 changes: 1 addition & 1 deletion apiclient/typescript-fetch/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@dmm-com/airone-apiclient-typescript-fetch",
"version": "0.0.6",
"version": "0.0.7",
"description": "AirOne APIv2 client in TypeScript",
"main": "src/autogenerated/index.ts",
"scripts": {
Expand Down
38 changes: 33 additions & 5 deletions entry/api_v2/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import re
from datetime import datetime, timedelta
from typing import Optional

from django.db.models import Q
from drf_spectacular.types import OpenApiTypes
Expand All @@ -13,6 +15,7 @@
from airone.lib.acl import ACLType
from airone.lib.drf import (
DuplicatedObjectExistsError,
FrequentImportError,
IncorrectTypeError,
ObjectNotExistsError,
RequiredParameterError,
Expand All @@ -39,7 +42,7 @@
from entry.settings import CONFIG
from entry.settings import CONFIG as ENTRY_CONFIG
from group.models import Group
from job.models import Job
from job.models import Job, JobOperation
from role.models import Role
from user.models import User

Expand Down Expand Up @@ -477,20 +480,45 @@ def get_queryset(self):
raise IncorrectTypeError(f"unsupported attr type: {entity_attr.type}")


@extend_schema(
parameters=[
OpenApiParameter("force", OpenApiTypes.BOOL, OpenApiParameter.QUERY, default=False),
],
)
class EntryImportAPI(generics.GenericAPIView):
parser_classes = [YAMLParser]
serializer_class = serializers.Serializer

def get_queryset(self):
import_data = self.request.data
entity_names = [d["entity"] for d in import_data]
return Entity.objects.filter(name__in=entity_names, is_active=True)

def post(self, request):
import_datas = request.data
user: User = request.user
serializer = EntryImportSerializer(data=import_datas)
serializer.is_valid(raise_exception=True)

job_ids = []
error_list = []
entities = self.get_queryset()

# limit import job to deny accidental frequent import for same entity
if request.query_params.get("force", "") not in ["true", "True"]:
valid_statuses = [Job.STATUS["PREPARING"], Job.STATUS["PROCESSING"], Job.STATUS["DONE"]]
yesterday = datetime.now() - timedelta(days=1)
if Job.objects.filter(
status__in=valid_statuses,
operation=JobOperation.IMPORT_ENTRY_V2.value,
target__in=entities,
created_at__gte=yesterday,
).exists():
raise FrequentImportError("Import job for each entity can apply once a day.")

job_ids: list[int] = []
error_list: list[str] = []
for import_data in import_datas:
entity = Entity.objects.filter(name=import_data["entity"], is_active=True).first()
entity: Optional[Entity] = next(
(e for e in entities if e.name == import_data["entity"]), None
)
if not entity:
error_list.append("%s: Entity does not exists." % import_data["entity"])
continue
Expand Down
28 changes: 24 additions & 4 deletions entry/tests/test_api_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -2490,7 +2490,7 @@ def test_import_update_entry(self):

# Update only some attributes
fp = self.open_fixture_file("import_data_v2_update_some.yaml")
resp = self.client.post("/entry/api/v2/import/", fp.read(), "application/yaml")
resp = self.client.post("/entry/api/v2/import/?force=true", fp.read(), "application/yaml")
fp.close()

self.assertEqual(resp.status_code, 200)
Expand All @@ -2504,7 +2504,7 @@ def test_import_update_entry(self):

# Update remove attribute value
fp = self.open_fixture_file("import_data_v2_update_remove.yaml")
resp = self.client.post("/entry/api/v2/import/", fp.read(), "application/yaml")
resp = self.client.post("/entry/api/v2/import/?force=true", fp.read(), "application/yaml")
fp.close()

self.assertEqual(resp.status_code, 200)
Expand Down Expand Up @@ -2616,9 +2616,10 @@ def test_import_entry_has_referrals_with_entities(self):

with self.assertLogs(logger=Logger, level=logging.WARNING) as warning_log:
fp = self.open_fixture_file("import_data_v2.yaml")
resp = self.client.post("/entry/api/v2/import/", fp.read(), "application/yaml")
resp = self.client.post(
"/entry/api/v2/import/?force=true", fp.read(), "application/yaml"
)
fp.close()
print(warning_log.output)
self.assertEqual(
warning_log.output,
[
Expand Down Expand Up @@ -2844,6 +2845,25 @@ def test_import_cancel(self):
self.assertEqual(job.text, "Now importing... (progress: [ 1/ 1])")
self.assertFalse(Entry.objects.filter(name="test-entry").exists())

@patch("entry.tasks.import_entries_v2.delay", Mock(side_effect=tasks.import_entries_v2))
def test_import_frequent_jobs(self):
fp = self.open_fixture_file("import_data_v2.yaml")
resp = self.client.post("/entry/api/v2/import/", fp.read(), "application/yaml")
fp.close()
self.assertEqual(resp.status_code, 200)

# without force param, it should exceed the limit
fp = self.open_fixture_file("import_data_v2.yaml")
resp = self.client.post("/entry/api/v2/import/?force=false", fp.read(), "application/yaml")
fp.close()
self.assertEqual(resp.status_code, 400)

# with force param, it should ignore the limit
fp = self.open_fixture_file("import_data_v2.yaml")
resp = self.client.post("/entry/api/v2/import/?force=true", fp.read(), "application/yaml")
fp.close()
self.assertEqual(resp.status_code, 200)

def test_advanced_search(self):
entry1: Entry = self.add_entry(
self.user,
Expand Down
49 changes: 24 additions & 25 deletions frontend/src/components/common/ImportForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const ImportForm: FC<Props> = ({ handleImport, handleCancel }) => {
await handleImport(fileReader.result);
history.go(0);
} catch (e) {
// TODO check the error code returned from the backend to get error details
setErrorMessage("ファイルのアップロードに失敗しました。");
enqueueSnackbar("ファイルのアップロードに失敗しました", {
variant: "error",
Expand All @@ -42,32 +43,30 @@ export const ImportForm: FC<Props> = ({ handleImport, handleCancel }) => {
};

return (
<Box>
<Box display="flex" flexDirection="column">
<Input type="file" onChange={onChange} />
<Box display="flex" flexDirection="column">
<Input type="file" onChange={onChange} />

<Typography color="error" variant="caption" my="4px">
{errorMessage}
</Typography>
<Box display="flex" justifyContent="flex-end">
<Button
type="submit"
variant="contained"
color="secondary"
onClick={onClick}
sx={{ m: "4px" }}
>
インポート
</Button>
<Button
variant="contained"
color="info"
onClick={handleCancel}
sx={{ m: "4px" }}
>
キャンセル
</Button>
</Box>
<Typography color="error" variant="caption" my="4px">
{errorMessage}
</Typography>
<Box display="flex" justifyContent="flex-end">
<Button
type="submit"
variant="contained"
color="secondary"
onClick={onClick}
sx={{ m: "4px" }}
>
インポート
</Button>
<Button
variant="contained"
color="info"
onClick={handleCancel}
sx={{ m: "4px" }}
>
キャンセル
</Button>
</Box>
</Box>
);
Expand Down
29 changes: 21 additions & 8 deletions frontend/src/components/entry/EntryImportModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Box, Modal, Typography } from "@mui/material";
import { Box, Checkbox, Modal, Typography } from "@mui/material";
import { styled } from "@mui/material/styles";
import React, { FC } from "react";
import React, { FC, useState } from "react";

import { ImportForm } from "components/common/ImportForm";
import { aironeApiClientV2 } from "repository/AironeApiClientV2";
Expand Down Expand Up @@ -30,6 +30,8 @@ export const EntryImportModal: FC<Props> = ({
openImportModal,
closeImportModal,
}) => {
const [forceImport, setForceImport] = useState(false);

return (
<StyledModal
aria-labelledby="transition-modal-title"
Expand All @@ -47,12 +49,23 @@ export const EntryImportModal: FC<Props> = ({
<Typography variant={"caption"} my="4px">
※CSV形式のファイルは選択できません。
</Typography>
<ImportForm
handleImport={(data: string | ArrayBuffer) =>
aironeApiClientV2.importEntries(data)
}
handleCancel={closeImportModal}
/>
<Box display="flex" alignItems="center">
<Checkbox
checked={forceImport}
onChange={(event) => setForceImport(event.target.checked)}
/>
<Typography variant={"body2"}>
強制的にインポートする(短期間にインポートを繰り返したい場合に使用してください)
</Typography>
</Box>
<Box my="8px">
<ImportForm
handleImport={(data: string | ArrayBuffer) =>
aironeApiClientV2.importEntries(data, forceImport)
}
handleCancel={closeImportModal}
/>
</Box>
</Paper>
</StyledModal>
);
Expand Down
22 changes: 15 additions & 7 deletions frontend/src/repository/AironeApiClientV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,14 +862,22 @@ class AironeApiClientV2 {
);
}

async importEntries(data: string | ArrayBuffer): Promise<void> {
return await this.entry.entryApiV2ImportCreate({
headers: {
"Content-Type": "application/yaml",
"X-CSRFToken": getCsrfToken(),
async importEntries(
data: string | ArrayBuffer,
force: boolean
): Promise<void> {
return await this.entry.entryApiV2ImportCreate(
{
force,
},
body: new Blob([data]),
});
{
headers: {
"Content-Type": "application/yaml",
"X-CSRFToken": getCsrfToken(),
},
body: new Blob([data]),
}
);
}

async resetPassword(username: string): Promise<void> {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/services/AironeAPIErrorUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const aironeAPIErrors: Record<string, string> = {
"AE-122000": "入力データが大きすぎます",
"AE-210000": "操作に必要な権限が不足しています",
"AE-220000": "入力データが既存のデータと重複しています",
"AE-260000": "短期間に同じターゲットに対してインポートが発生しました",
};

const extractErrorDetail = (errorDetail: ErrorDetail): string =>
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"zod": "^3.20.6"
},
"dependencies": {
"@dmm-com/airone-apiclient-typescript-fetch": "^0.0.6",
"@dmm-com/airone-apiclient-typescript-fetch": "^0.0.7",
"@emotion/react": "^11.10.5",
"@emotion/styled": "^11.10.5",
"@jest/globals": "^27.0.6",
Expand Down

0 comments on commit cfc944e

Please sign in to comment.