Skip to content

Commit

Permalink
fix(RHINENG-15180): Fix CSV data export with unicode chars (#2178)
Browse files Browse the repository at this point in the history
* Encode export_service POSTs in utf-8; make try/catch more specific; test

* Add type hint

* Update test_handle_create_export_unicode

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED

---------

Co-authored-by: Frantisek Stavela <[email protected]>
  • Loading branch information
kruai and fstavela authored Jan 17, 2025
1 parent 1f927fa commit 0621d61
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 6 deletions.
11 changes: 7 additions & 4 deletions app/queue/export_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@
from app.auth.identity import Identity
from app.auth.identity import from_auth_header
from app.config import Config
from app.exceptions import InventoryException
from app.logging import get_logger
from lib import metrics
from lib.middleware import get_rbac_filter
from utils.json_to_csv import json_arr_to_csv

logger = get_logger(__name__)

HEADER_CONTENT_TYPE = {"json": "application/json", "csv": "text/csv"}
HEADER_CONTENT_TYPE = {"json": "application/json; charset=utf-8", "csv": "text/csv; charset=utf-8"}


def extract_export_svc_data(export_svc_data: dict) -> tuple[str, UUID, str, str, str]:
Expand Down Expand Up @@ -133,7 +134,9 @@ def create_export(
f"{len(host_data)} hosts will be exported (format: {exportFormat}) for org_id {identity.org_id}"
)
response = session.post(
url=request_url, headers=request_headers, data=_format_export_data(host_data, exportFormat)
url=request_url,
headers=request_headers,
data=_format_export_data(host_data, exportFormat).encode("utf-8"),
)
_handle_export_response(response, exportUUID, exportFormat)
export_created = True
Expand All @@ -149,7 +152,7 @@ def create_export(
)
_handle_export_response(response, exportUUID, exportFormat)
export_created = False
except Exception as e:
except InventoryException as e:
request_url = _build_export_request_url(
export_service_endpoint, exportUUID, applicationName, resourceUUID, "error"
)
Expand Down Expand Up @@ -188,7 +191,7 @@ def _handle_export_error(
# This function is used by create_export, needs improvement
def _handle_export_response(response: Response, exportUUID: UUID, exportFormat: str):
if response.status_code != HTTPStatus.ACCEPTED:
raise Exception(response.text)
raise InventoryException(response.text)
elif response.text != "":
logger.info(f"{response.text} for export ID {str(exportUUID)} in {exportFormat.upper()} format")

Expand Down
13 changes: 13 additions & 0 deletions tests/helpers/api_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
import math
from base64 import b64encode
from datetime import timedelta
from http import HTTPStatus
from itertools import product
from struct import unpack
from typing import Any
from urllib.parse import parse_qs
from urllib.parse import quote_plus as url_quote
from urllib.parse import urlencode
from urllib.parse import urlsplit
from urllib.parse import urlunsplit

import dateutil.parser
from requests import Response

from app.auth.identity import IdentityType
from tests.helpers.test_utils import now
Expand Down Expand Up @@ -610,3 +613,13 @@ def assert_resource_types_pagination(
assert links["next"] is None

assert links["last"] == f"{expected_path_base}?per_page={expected_per_page}&page={expected_number_of_pages}"


def mocked_export_post(_self: Any, url: str, *, data: bytes, **_: Any) -> Response:
# This will raise UnicodeDecodeError if not correctly encoded or AttributeError if data is str
data.decode("utf-8")
response = Response()
response.url = url
response.status_code = HTTPStatus.ACCEPTED
response._content = b"Export successful"
return response
4 changes: 2 additions & 2 deletions tests/helpers/export_service_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
]


def create_export_message_mock():
def create_export_message_mock(format: str = "json") -> str:
return json.dumps(
{
"id": "b4228e37-8ae8-4c67-81d5-d03f39bbe309",
Expand All @@ -85,7 +85,7 @@ def create_export_message_mock():
"productId": "RHEL",
"startDate": "2024-01-01T00:00:00Z",
},
"format": "json",
"format": format,
"resource": "urn:redhat:application:inventory:export:systems",
"uuid": "2844f3a1-e047-45b1-b0ce-fb9812ad6a6f",
"x-rh-identity": (
Expand Down
17 changes: 17 additions & 0 deletions tests/test_export_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@
from app.culling import Timestamps
from app.culling import _Config as CullingConfig
from app.queue.export_service import _format_export_data
from app.queue.export_service import create_export
from app.queue.export_service import get_host_list
from app.queue.export_service_mq import handle_export_message
from app.queue.export_service_mq import parse_export_service_message
from app.serialization import _EXPORT_SERVICE_FIELDS
from app.serialization import serialize_host_for_export_svc
from tests.helpers import export_service_utils as es_utils
from tests.helpers.api_utils import HOST_READ_ALLOWED_RBAC_RESPONSE_FILES
from tests.helpers.api_utils import HOST_READ_PROHIBITED_RBAC_RESPONSE_FILES
from tests.helpers.api_utils import create_mock_rbac_response
from tests.helpers.api_utils import mocked_export_post
from tests.helpers.db_utils import db_host
from tests.helpers.test_utils import USER_IDENTITY

Expand All @@ -34,6 +37,20 @@ def test_handle_create_export_happy_path(mock_post, db_create_host, flask_app, i
assert resp is True


@pytest.mark.parametrize("format", ("json", "csv"))
@mock.patch("requests.Session.post", new=mocked_export_post)
def test_handle_create_export_unicode(db_create_host, flask_app, inventory_config, format):
with flask_app.app.app_context():
host_to_create = db_host()
host_to_create.display_name = "“quotetest”"
db_create_host(host=host_to_create)

validated_msg = parse_export_service_message(es_utils.create_export_message_mock(format=format))
base64_x_rh_identity = validated_msg["data"]["resource_request"]["x_rh_identity"]

assert create_export(validated_msg, base64_x_rh_identity, inventory_config)


@mock.patch("requests.Session.post", autospec=True)
@mock.patch("app.queue.export_service.get_hosts_to_export", return_value=iter(es_utils.EXPORT_DATA))
@mock.patch("app.queue.export_service.create_export", return_value=True)
Expand Down

0 comments on commit 0621d61

Please sign in to comment.