✨(backend) allow delete documents by tags (#45)
* ✨(backend) allow delete documents by tags the assistant does not know the ids. It needs to be able to detele documents by tags. * ♻️(backend) refactor I refactor to have a better code. * 🚨(backend) various fixes I am fixing things for review.
This commit is contained in:
committed by
GitHub
parent
f8b87cc1c2
commit
05aebf564a
@@ -32,6 +32,7 @@ and this project adheres to
|
||||
## Changed
|
||||
|
||||
- 🏗️(backend) switch Python dependency management to uv
|
||||
- ✨(backend) allow deletion by tags
|
||||
|
||||
## Fixed
|
||||
|
||||
|
||||
@@ -123,4 +123,14 @@ class DeleteDocumentsSchema(BaseModel):
|
||||
"""Schema for validating the delete documents request"""
|
||||
|
||||
service: str = Field(max_length=300)
|
||||
document_ids: List[str] = Field(min_length=1)
|
||||
document_ids: Optional[List[str]] = Field(default=None)
|
||||
tags: Optional[List[str]] = Field(default=None)
|
||||
|
||||
@model_validator(mode="after")
|
||||
def check_at_least_one_filter(self):
|
||||
"""Ensure at least one of document_ids or tags is provided"""
|
||||
if not self.document_ids and not self.tags:
|
||||
raise ValueError(
|
||||
"At least one of 'document_ids' or 'tags' must be provided"
|
||||
)
|
||||
return self
|
||||
|
||||
@@ -154,7 +154,7 @@ def test_api_documents_delete_invalid_params(settings):
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
service = factories.ServiceFactory()
|
||||
|
||||
# Missing document_ids
|
||||
# Missing both document_ids and tags
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/delete/",
|
||||
{
|
||||
@@ -165,8 +165,12 @@ def test_api_documents_delete_invalid_params(settings):
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert (
|
||||
response.json()[0]["msg"]
|
||||
== "Value error, At least one of 'document_ids' or 'tags' must be provided"
|
||||
)
|
||||
|
||||
# Empty document_ids
|
||||
# Empty document_ids and no tags
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/delete/",
|
||||
{"service": service.name, "document_ids": []},
|
||||
@@ -175,6 +179,24 @@ def test_api_documents_delete_invalid_params(settings):
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert (
|
||||
response.json()[0]["msg"]
|
||||
== "Value error, At least one of 'document_ids' or 'tags' must be provided"
|
||||
)
|
||||
|
||||
# Both empty
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/delete/",
|
||||
{"service": service.name, "document_ids": [], "tags": []},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert (
|
||||
response.json()[0]["msg"]
|
||||
== "Value error, At least one of 'document_ids' or 'tags' must be provided"
|
||||
)
|
||||
|
||||
# Missing service
|
||||
response = APIClient().post(
|
||||
@@ -208,3 +230,155 @@ def test_api_documents_delete_nonexistent_documents(settings):
|
||||
assert response.status_code == 200
|
||||
assert response.json()["nb-deleted-documents"] == 0
|
||||
assert response.json()["undeleted-document-ids"] == ["non-existent-id"]
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_delete_by_single_tag(settings):
|
||||
"""Users should be able to delete documents by tags."""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
service = factories.ServiceFactory()
|
||||
|
||||
document_to_deletes = [
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag", "keep-tag-1"]
|
||||
),
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag", "keep-tag-2"]
|
||||
),
|
||||
]
|
||||
document_to_keep = [
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["keep-tag-1", "keep-tag-2"]
|
||||
),
|
||||
factories.DocumentSchemaFactory.build(users=["user_sub"], tags=["keep-tag-1"]),
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["other_user_sub"], tags=["delete-tag"]
|
||||
),
|
||||
]
|
||||
prepare_index(service.index_name, document_to_deletes + document_to_keep)
|
||||
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/delete/",
|
||||
{"service": service.name, "tags": ["delete-tag"]},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["nb-deleted-documents"] == 2
|
||||
assert response.json()["undeleted-document-ids"] == []
|
||||
|
||||
opensearch_client_ = opensearch_client()
|
||||
for document in document_to_deletes:
|
||||
with pytest.raises(opensearchpy.exceptions.NotFoundError):
|
||||
opensearch_client_.get(index=service.index_name, id=document["id"])
|
||||
|
||||
for document in document_to_keep:
|
||||
doc = opensearch_client_.get(index=service.index_name, id=document["id"])
|
||||
assert doc["found"]
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_delete_by_multiple_tags(settings):
|
||||
"""Users should be able to delete documents matching any of multiple tags."""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
service = factories.ServiceFactory()
|
||||
|
||||
document_to_deletes = [
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag-1", "keep-tag-1"]
|
||||
),
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag-1", "delete-tag-2"]
|
||||
),
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag-2"]
|
||||
),
|
||||
]
|
||||
document_to_keep = [
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["keep-tag-1", "keep-tag-2"]
|
||||
),
|
||||
factories.DocumentSchemaFactory.build(users=["user_sub"], tags=["keep-tag-1"]),
|
||||
factories.DocumentSchemaFactory.build(
|
||||
users=["other_user_sub"], tags=["delete-tag-1"]
|
||||
),
|
||||
]
|
||||
prepare_index(service.index_name, document_to_deletes + document_to_keep)
|
||||
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/delete/",
|
||||
{"service": service.name, "tags": ["delete-tag-1", "delete-tag-2"]},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["nb-deleted-documents"] == 3
|
||||
assert response.json()["undeleted-document-ids"] == []
|
||||
|
||||
opensearch_client_ = opensearch_client()
|
||||
for document in document_to_deletes:
|
||||
with pytest.raises(opensearchpy.exceptions.NotFoundError):
|
||||
opensearch_client_.get(index=service.index_name, id=document["id"])
|
||||
|
||||
for document in document_to_keep:
|
||||
doc = opensearch_client_.get(index=service.index_name, id=document["id"])
|
||||
assert doc["found"]
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_delete_by_ids_and_tags(settings):
|
||||
"""Users should be able to delete documents by both IDs and tags (AND logic)."""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
service = factories.ServiceFactory()
|
||||
|
||||
document_delete_by_tag_and_id = factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag"]
|
||||
)
|
||||
document_delete_by_tag_keep_by_id = factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"], tags=["delete-tag"]
|
||||
)
|
||||
document_keep_by_tag_delete_by_id = factories.DocumentSchemaFactory.build(
|
||||
users=["user_sub"]
|
||||
)
|
||||
|
||||
prepare_index(
|
||||
service.index_name,
|
||||
[
|
||||
document_delete_by_tag_and_id,
|
||||
document_delete_by_tag_keep_by_id,
|
||||
document_keep_by_tag_delete_by_id,
|
||||
],
|
||||
)
|
||||
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/delete/",
|
||||
{
|
||||
"service": service.name,
|
||||
"document_ids": [
|
||||
document_delete_by_tag_and_id["id"],
|
||||
document_keep_by_tag_delete_by_id["id"],
|
||||
],
|
||||
"tags": ["delete-tag"],
|
||||
},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["nb-deleted-documents"] == 1
|
||||
assert response.json()["undeleted-document-ids"] == [
|
||||
document_keep_by_tag_delete_by_id["id"]
|
||||
]
|
||||
|
||||
opensearch_client_ = opensearch_client()
|
||||
with pytest.raises(opensearchpy.exceptions.NotFoundError):
|
||||
opensearch_client_.get(
|
||||
index=service.index_name, id=document_delete_by_tag_and_id["id"]
|
||||
)
|
||||
|
||||
doc = opensearch_client_.get(
|
||||
index=service.index_name, id=document_delete_by_tag_keep_by_id["id"]
|
||||
)
|
||||
assert doc["found"]
|
||||
|
||||
+43
-14
@@ -203,13 +203,22 @@ class DeleteDocumentsView(ResourceServerMixin, views.APIView):
|
||||
---------------
|
||||
service: str
|
||||
service name to determine the index from which to delete documents.
|
||||
document_ids : List[str]
|
||||
document_ids : List[str], optional
|
||||
A list of document IDs to delete from the index.
|
||||
tags : List[str], optional
|
||||
A list of tags to filter documents for deletion.
|
||||
|
||||
At least one of document_ids or tags must be provided.
|
||||
The list of ids and the list of tags are combined with AND logic.
|
||||
|
||||
Returns:
|
||||
--------
|
||||
Response : rest_framework.response.Response
|
||||
- 200 OK: Returns the number of documents deleted and list of undeleted document IDs.
|
||||
- 200 OK: returns a JSON object with the following keys:
|
||||
- nb-deleted-documents: Number of documents deleted.
|
||||
- undeleted-document-ids: sublist of param.document_ids that were not deleted.
|
||||
Deletion may be prevented because the document does not exist,
|
||||
because the user is not authorized to delete it or because a tag filter was used.
|
||||
- 400 Bad Request: If parameters are invalid or missing.
|
||||
"""
|
||||
params = schemas.DeleteDocumentsSchema(**request.data)
|
||||
@@ -225,26 +234,27 @@ class DeleteDocumentsView(ResourceServerMixin, views.APIView):
|
||||
)
|
||||
|
||||
logger.info(
|
||||
"Deleting %d documents from index %s", len(params.document_ids), index_name
|
||||
"Deleting documents from index %s with filters: document_ids=%s, tags=%s",
|
||||
index_name,
|
||||
params.document_ids,
|
||||
params.tags,
|
||||
)
|
||||
|
||||
deletable_matches = opensearch_client().search(
|
||||
client = opensearch_client()
|
||||
deletable_matches = client.search(
|
||||
index=index_name,
|
||||
body={
|
||||
"query": {
|
||||
"bool": {
|
||||
"must": [
|
||||
{"ids": {"values": params.document_ids}},
|
||||
{"term": {"users": self.request.user.sub}},
|
||||
]
|
||||
}
|
||||
}
|
||||
"query": self._build_query(
|
||||
self.request.user.sub,
|
||||
document_ids=params.document_ids,
|
||||
tags=params.tags,
|
||||
)
|
||||
},
|
||||
)
|
||||
deletable_ids = [hit["_id"] for hit in deletable_matches["hits"]["hits"]]
|
||||
|
||||
if deletable_ids:
|
||||
response = opensearch_client().delete_by_query(
|
||||
response = client.delete_by_query(
|
||||
index=index_name,
|
||||
body={"query": {"ids": {"values": deletable_ids}}},
|
||||
)
|
||||
@@ -257,13 +267,32 @@ class DeleteDocumentsView(ResourceServerMixin, views.APIView):
|
||||
"nb-deleted-documents": nb_deleted,
|
||||
"undeleted-document-ids": [
|
||||
document_id
|
||||
for document_id in params.document_ids
|
||||
for document_id in params.document_ids or []
|
||||
if document_id not in deletable_ids
|
||||
],
|
||||
},
|
||||
status=status.HTTP_200_OK,
|
||||
)
|
||||
|
||||
def _build_query(self, user_sub, document_ids=None, tags=None):
|
||||
"""
|
||||
Build OpenSearch query for document deletion.
|
||||
|
||||
Args:
|
||||
user_sub: User subject identifier for authorization.
|
||||
document_ids: Optional list of document IDs to filter.
|
||||
tags: Optional list of tags to filter.
|
||||
|
||||
Returns:
|
||||
Deletion OpenSearch query.
|
||||
"""
|
||||
filters = [{"term": {"users": user_sub}}]
|
||||
if document_ids:
|
||||
filters.append({"ids": {"values": document_ids}})
|
||||
if tags:
|
||||
filters.append({"terms": {"tags": tags}})
|
||||
return {"bool": {"must": filters}}
|
||||
|
||||
|
||||
class SearchDocumentView(ResourceServerMixin, views.APIView):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user