diff --git a/.bumpversion.cfg b/.bumpversion.cfg index f82b2552..b3c8559f 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.78.0 +current_version = 0.79.0 commit = True tag = False message = chore: Bump version from {current_version} to {new_version} diff --git a/.github/workflows/ci-cd.yaml b/.github/workflows/ci-cd.yaml index 4ca0afc4..774c958e 100644 --- a/.github/workflows/ci-cd.yaml +++ b/.github/workflows/ci-cd.yaml @@ -64,18 +64,18 @@ jobs: # -----END Release Job----- - # -----BEGIN Deploy Job----- - deploy: - name: Deploy + # -----BEGIN Publish Job----- + publish: + name: Publish if: ${{ github.ref == needs.workflow_config.outputs.PRODUCTION_VCS_REF }} needs: - release - workflow_config - uses: ./.github/workflows/deploy.yaml + uses: ./.github/workflows/publish.yaml with: deploy_env: prod artifacts_path: ${{ needs.release.outputs.artifacts_path }} secrets: inherit - # -----END Deploy Job----- + # -----END Publish Job----- diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7cd66e53..4affa2c8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -36,11 +36,11 @@ jobs: steps: - name: Check Out VCS Repository - uses: actions/checkout@v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set Up Python ${{ matrix.python_version }} id: set_up_python - uses: actions/setup-python@v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "${{ matrix.python_version }}" check-latest: true @@ -48,17 +48,24 @@ jobs: - name: Install Pip run: make python-pip-install - - name: Create Python Virtual Environment - run: make python-virtualenv PYTHON_VIRTUALENV_DIR="venv" - - name: Restoring/Saving Cache - uses: actions/cache@v5.0.5 + id: restoring_saving_cache + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: | .tox venv key: py-v1-deps-${{ runner.os }}-${{ steps.set_up_python.outputs.python-version }}-${{ hashFiles('pyproject.toml', 'requirements.txt', 'requirements-dev.txt', 'Makefile', 'make/**.mk') }} + - name: Create Python Virtual Environment + # Only when the cache was not restored. The cache is restored by extracting an archive + # over the workspace, so restoring it over an existing virtual environment merges the Pip + # installation that `python -m venv` bootstraps with the cached one, instead of replacing + # it. That leaves an unusable Pip whenever the two versions lay out their internal modules + # differently. + if: ${{ steps.restoring_saving_cache.outputs.cache-hit != 'true' }} + run: make python-virtualenv PYTHON_VIRTUALENV_DIR="venv" + - name: Install Dependencies run: | source "$PYTHON_VIRTUALENV_ACTIVATE" @@ -97,17 +104,17 @@ jobs: steps: - name: Check Out VCS Repository - uses: actions/checkout@v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set Up Python ${{ matrix.python_version }} id: set_up_python - uses: actions/setup-python@v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "${{ matrix.python_version }}" check-latest: true - name: Restoring/Saving Cache - uses: actions/cache@v5.0.5 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: | .tox @@ -163,7 +170,7 @@ jobs: - name: Store Artifacts if: ${{ always() }} - uses: actions/upload-artifact@v7.0.1 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: test_reports_${{ matrix.python_version }} path: test-reports/ diff --git a/.github/workflows/dependency-review.yaml b/.github/workflows/dependency-review.yaml index 47c505c8..943f43f8 100644 --- a/.github/workflows/dependency-review.yaml +++ b/.github/workflows/dependency-review.yaml @@ -6,7 +6,6 @@ on: - opened - reopened - synchronize - - ready_for_review permissions: contents: read @@ -22,9 +21,9 @@ jobs: steps: - name: Check Out VCS Repository - uses: actions/checkout@v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Dependency Review - uses: actions/dependency-review-action@v5.0.0 + uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 with: fail-on-severity: critical diff --git a/.github/workflows/git-commit-lint.yaml b/.github/workflows/git-commit-lint.yaml index 6e1c4d1b..59aee589 100644 --- a/.github/workflows/git-commit-lint.yaml +++ b/.github/workflows/git-commit-lint.yaml @@ -8,7 +8,6 @@ on: - opened - reopened - synchronize - - ready_for_review permissions: contents: read diff --git a/.github/workflows/deploy.yaml b/.github/workflows/publish.yaml similarity index 78% rename from .github/workflows/deploy.yaml rename to .github/workflows/publish.yaml index da2534c5..cc90ca2c 100644 --- a/.github/workflows/deploy.yaml +++ b/.github/workflows/publish.yaml @@ -1,6 +1,6 @@ -# GitHub Actions Workflow for Deployment +# GitHub Actions Workflow for Publication -name: Deploy +name: Publish on: workflow_call: @@ -18,7 +18,7 @@ permissions: # -----BEGIN Environment Variables----- -# Environment variables required for deployment: +# Environment variables required for publication: # # - PYPI_PASSWORD := PyPI password or API token. # - PYPI_USERNAME := PyPI username. For API tokens, use "__token__". @@ -31,36 +31,36 @@ env: # -----END Environment Variables----- jobs: - deploy: - name: Deploy + publish: + name: Publish runs-on: ubuntu-22.04 environment: ${{ inputs.deploy_env }} steps: - name: Check Out VCS Repository - uses: actions/checkout@v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set Up Python id: set_up_python - uses: actions/setup-python@v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version-file: "pyproject.toml" check-latest: true - name: Restoring/Saving Cache - uses: actions/cache@v5.0.5 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: "venv" key: py-v1-deps-${{ runner.os }}-${{ steps.set_up_python.outputs.python-version }}-${{ hashFiles('pyproject.toml', 'requirements.txt', 'requirements-dev.txt', 'Makefile', 'make/**.mk') }} fail-on-cache-miss: true - name: Restore Artifacts (Release) - uses: actions/download-artifact@v8.0.1 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: release path: ${{ inputs.artifacts_path }}/ - - name: Deploy + - name: Publish run: | source "$PYTHON_VIRTUALENV_ACTIVATE" make deploy \ diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 800be8b1..f87ab4ef 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -35,24 +35,31 @@ jobs: steps: - name: Check Out VCS Repository - uses: actions/checkout@v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set Up Python id: set_up_python - uses: actions/setup-python@v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version-file: "pyproject.toml" check-latest: true - - name: Create Python Virtual Environment - run: make python-virtualenv PYTHON_VIRTUALENV_DIR="venv" - - name: Restoring/Saving Cache - uses: actions/cache@v5.0.5 + id: restoring_saving_cache + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: "venv" key: py-v1-deps-${{ runner.os }}-${{ steps.set_up_python.outputs.python-version }}-${{ hashFiles('pyproject.toml', 'requirements.txt', 'requirements-dev.txt', 'Makefile', 'make/**.mk') }} + - name: Create Python Virtual Environment + # Only when the cache was not restored. The cache is restored by extracting an archive + # over the workspace, so restoring it over an existing virtual environment merges the Pip + # installation that `python -m venv` bootstraps with the cached one, instead of replacing + # it. That leaves an unusable Pip whenever the two versions lay out their internal modules + # differently. + if: ${{ steps.restoring_saving_cache.outputs.cache-hit != 'true' }} + run: make python-virtualenv PYTHON_VIRTUALENV_DIR="venv" + - name: Install Dependencies run: | source "$PYTHON_VIRTUALENV_ACTIVATE" @@ -69,7 +76,7 @@ jobs: make dist - name: Store Artifacts - uses: actions/upload-artifact@v7.0.1 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: release path: ${{ env.ARTIFACTS_PATH }}/ diff --git a/.github/workflows/task-release-and-deploy.yaml b/.github/workflows/task-release-and-deploy.yaml deleted file mode 100644 index 058bc41d..00000000 --- a/.github/workflows/task-release-and-deploy.yaml +++ /dev/null @@ -1,75 +0,0 @@ -# GitHub Actions Workflow for 'Release and Deploy' Task - -name: "Task: Release and Deploy" - -on: - pull_request: - types: - - closed - branches: # Base reference - - develop - -permissions: {} - -jobs: - deploy: - name: Deploy - if: ${{ github.event.pull_request.merged == true && startsWith(github.head_ref, 'release/') }} - runs-on: ubuntu-22.04 - - permissions: - contents: write - pull-requests: write - - env: - CREATE_RELEASE_VCS_REVISION_ID: ${{ github.sha }} # Merge commit in base branch. - PREPARE_RELEASE_GITHUB_PULL_REQUEST_HTML_URL: ${{ github.event.pull_request.html_url }} - PREPARE_RELEASE_GITHUB_PULL_REQUEST_TITLE: ${{ github.event.pull_request.title }} - PREPARE_RELEASE_GITHUB_VCS_REF: ${{ github.event.pull_request.head.ref }} - RELEASE_ASSIGNEE: ${{ github.event.pull_request.assignee.login }} - RELEASE_VCS_REF: refs/heads/master - - steps: - - name: Check Out VCS Repository - uses: actions/checkout@v6.0.3 - with: - ref: ${{ env.CREATE_RELEASE_VCS_REVISION_ID }} - - - name: Prepare Git - run: | - echo 'Adding Git aliases…' - git config alias.publish \ - 'push --set-upstream origin HEAD' - - - name: Prepare Pull Request for Deployment - run: | - create_release_vcs_branch_name="${PREPARE_RELEASE_GITHUB_VCS_REF:?}" - create_release_vcs_branch_name="${create_release_vcs_branch_name/release/deploy}" - echo "Creating release creation VCS branch '$create_release_vcs_branch_name'…" - git checkout -b "${create_release_vcs_branch_name:?}" -- - git publish --verbose - - create_release_vcs_ref="refs/heads/${create_release_vcs_branch_name:?}" - echo "CREATE_RELEASE_VCS_REF=${create_release_vcs_ref:?}" >> "$GITHUB_ENV" - - create_release_github_pull_request_title="${PREPARE_RELEASE_GITHUB_PULL_REQUEST_TITLE:?}" - create_release_github_pull_request_title="deploy ${create_release_github_pull_request_title,,}" - create_release_github_pull_request_title="${create_release_github_pull_request_title@u}" - echo "CREATE_RELEASE_GITHUB_PULL_REQUEST_TITLE=${create_release_github_pull_request_title:?}" >> "$GITHUB_ENV" - - create_release_github_pull_request_description="Ref: ${PREPARE_RELEASE_GITHUB_PULL_REQUEST_HTML_URL:?}" - echo "CREATE_RELEASE_GITHUB_PULL_REQUEST_DESCRIPTION=${create_release_github_pull_request_description:?}" >> "$GITHUB_ENV" - - - name: Create GitHub Pull Request for Deployment - run: | - gh pr create \ - --base "$RELEASE_VCS_REF" \ - --head "$CREATE_RELEASE_VCS_REF" \ - --draft \ - --title "$CREATE_RELEASE_GITHUB_PULL_REQUEST_TITLE" \ - --body "$CREATE_RELEASE_GITHUB_PULL_REQUEST_DESCRIPTION" \ - --assignee "$RELEASE_ASSIGNEE" \ - --label 'task' \ - --label 'kind: deploy' - env: - GH_TOKEN: ${{ github.token }} diff --git a/.github/workflows/task-release-and-publish.yaml b/.github/workflows/task-release-and-publish.yaml new file mode 100644 index 00000000..8f86da78 --- /dev/null +++ b/.github/workflows/task-release-and-publish.yaml @@ -0,0 +1,21 @@ +# GitHub Actions Workflow for 'Release and Publish' Task + +name: "Task: Release and Publish" + +on: + pull_request: + types: + - closed + branches: # Base reference + - develop + +permissions: {} + +jobs: + publish: + name: Publish + if: ${{ github.event.pull_request.merged == true && startsWith(github.head_ref, 'release/') }} + permissions: + contents: write + pull-requests: write + uses: cordada/github-actions-utils/.github/workflows/task-release-and-publish.yaml@master diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec198bc1..a932662a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -154,7 +154,7 @@ Then wait for the CI tests (`develop` branch) to finish and verify they were suc base branch. -### Pull Request for Deployment +### Pull Request for Publication - Check out the **main VCS branch**: @@ -174,7 +174,7 @@ Then wait for the CI tests (`develop` branch) to finish and verify they were suc - Create a **new branch**: ```sh - git checkout -b deploy/vX.Y.Z + git checkout -b publish/vX.Y.Z ``` - Push the branch to `origin`: @@ -185,8 +185,8 @@ Then wait for the CI tests (`develop` branch) to finish and verify they were suc - Create a **pull request**: - *Base reference*: `master` - - *Head reference*: `deploy/vX.Y.Z` - - *Title*: `Deploy release vX.Y.Z` + - *Head reference*: `publish/vX.Y.Z` + - *Title*: `Publish release vX.Y.Z` - *Description*: ```markdown @@ -195,7 +195,7 @@ Then wait for the CI tests (`develop` branch) to finish and verify they were suc - *Labels*: - `task` - - `kind: deploy` + - `kind: publish` - **Assign** the pull request to the appropriate developer (usually yourself). - Wait until the CI checks have finished. - Request pull request **reviews**. diff --git a/HISTORY.md b/HISTORY.md index 45bdd0ab..210a50d6 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,19 @@ # History +## 0.79.0 (2026-09-30) + +- (PR #1044, 2026-06-17) Do not trigger Commit Linter/Dependency Review for draft PRs marked ready +- (PR #1047, 2026-07-01) chore(deps): Bump the github-actions-production group with 3 updates +- (PR #1048, 2026-07-01) deps: Update `signxml` from 4.2.0 to 4.3.1 +- (PR #1060, 2026-09-10) workflows: Use reusable workflow for release and publication task +- (PR #1061, 2026-09-10) workflows: Label a release publication `kind: publish` +- (PR #1062, 2026-09-10) workflows: Restore the dependency cache before creating the virtual environment +- (PR #1063, 2026-09-10) workflows: Restore dependency cache before creating venv in "Release" +- (PR #1064, 2026-09-10) workflows: Rename workflow "Deploy" to "Publish" +- (PR #1052, 2026-09-10) chore(deps): Bump github-actions-production group (2 updates) +- (PR #1065, 2026-09-14) workflows: Pin GitHub Actions to commit hashes +- (PR #1057, 2026-09-24) rtc: Fall back to a lossless encoding when parsing "cesiones periodo" files + ## 0.78.0 (2026-06-08) - (PR #1031, 2026-05-28) chore(deps): Bump build from 1.4.2 to 1.4.4 in the python-development group diff --git a/requirements.in b/requirements.in index fa37817d..1a60cc6a 100644 --- a/requirements.in +++ b/requirements.in @@ -18,5 +18,5 @@ pydantic==2.12.5 pyOpenSSL==26.0.0 pytz==2025.2 setuptools==82.0.1 -signxml==4.2.0 +signxml==4.3.1 typing-extensions==4.15.0 diff --git a/requirements.txt b/requirements.txt index e524e509..8c918727 100644 --- a/requirements.txt +++ b/requirements.txt @@ -64,7 +64,7 @@ rpds-py==0.27.1 # via # jsonschema # referencing -signxml==4.2.0 +signxml==4.3.1 # via -r requirements.in sqlparse==0.5.4 # via django diff --git a/src/cl_sii/__init__.py b/src/cl_sii/__init__.py index ed265c0d..b78d4d2b 100644 --- a/src/cl_sii/__init__.py +++ b/src/cl_sii/__init__.py @@ -4,4 +4,4 @@ """ -__version__ = '0.78.0' +__version__ = '0.79.0' diff --git a/src/cl_sii/libs/charset_utils.py b/src/cl_sii/libs/charset_utils.py index 03b004e1..456b3132 100644 --- a/src/cl_sii/libs/charset_utils.py +++ b/src/cl_sii/libs/charset_utils.py @@ -1,9 +1,46 @@ +import codecs import unicodedata - -import chardet +from typing import Sequence, Tuple _DEFAULT_FILE_ENCODING = 'utf-8' +"""Encoding assumed when there is no data to inspect.""" + +LOSSLESS_FALLBACK_ENCODING = 'latin-1' # ISO-8859-1. +""" +Encoding that decodes any byte sequence without failing. + +Handy as a last resort for reading a file whose encoding could not be determined reliably. +""" + +_CANDIDATE_ENCODINGS: Sequence[str] = ( + 'utf-8', + # note: for text produced on Windows (the usual case for SII files), "cp1252" interprets the + # 0x80-0x9F range correctly, unlike "latin-1". + 'cp1252', + # note: "latin-1" decodes any byte sequence, so it guarantees a result. + LOSSLESS_FALLBACK_ENCODING, +) +""" +Encodings tried, in order, until one of them decodes the data. + +The files handled by this library contain Spanish text (Chilean documents), so this covers every +encoding they are known to come in. For that kind of text "cp1252" and "latin-1" are equivalent +anyway: they share the whole 0xA0-0xFF range, where the accented characters are. +""" + +_BOMS: Sequence[Tuple[bytes, str]] = ( + # warning: order matters, the UTF-32 LE BOM starts with the UTF-16 LE one. + (codecs.BOM_UTF32_LE, 'utf-32'), + (codecs.BOM_UTF32_BE, 'utf-32'), + (codecs.BOM_UTF8, 'utf-8-sig'), + (codecs.BOM_UTF16_LE, 'utf-16'), + (codecs.BOM_UTF16_BE, 'utf-16'), +) +"""Byte order marks and the encoding each one implies.""" + +_DETECTION_SAMPLE_SIZE = 512 * 1024 +"""Maximum number of bytes read from a file in order to detect its encoding.""" def clean_unicode(value: str) -> str: @@ -50,11 +87,84 @@ def clean_unicode(value: str) -> str: def detect_file_encoding(file_path: str) -> str: + """ + Detect the encoding of the text file at ``file_path``. + + Only the first :data:`_DETECTION_SAMPLE_SIZE` bytes of the file are inspected, so the result is + not guaranteed to decode the rest of the file: the caller must be prepared to handle a + :class:`UnicodeDecodeError` while reading. + + .. seealso:: :func:`detect_encoding` + + """ with open(file_path, 'rb') as f: - raw_data = f.read(512 * 1024) # Read up to 512 KB - result = chardet.detect(raw_data) + raw_data = f.read(_DETECTION_SAMPLE_SIZE) + + # note: 'read' returns fewer bytes than requested only at the end of the file. + is_sample = len(raw_data) == _DETECTION_SAMPLE_SIZE + + return detect_encoding(raw_data, is_sample=is_sample) + + +def detect_encoding(raw_data: bytes, is_sample: bool = False) -> str: + """ + Detect the encoding of ``raw_data``. + + Never raises: the last candidate is an encoding that decodes any byte sequence, so the + ``RuntimeError`` at the end of the function can only signal a programming error. + + :param is_sample: + Whether ``raw_data`` is only the beginning of a larger byte sequence. If it is, an + incomplete multi-byte character at the end of it is not considered a decoding error, + because the rest of that character is in the part that was left out. In that case the + returned encoding is guaranteed to decode the whole sequence, but not necessarily + ``raw_data`` on its own. + + The encoding is chosen in layers: + + 1. The byte order mark, if there is one. + 2. The first of :data:`_CANDIDATE_ENCODINGS` that decodes ``raw_data``. Pure ASCII is decoded + by the first candidate, UTF-8. + + .. note:: + UTF-16 and UTF-32 are recognized only by their byte order mark. Without one they are + indistinguishable from other encodings without a statistical guess, which is deliberately + not made here: a wrong guess yields mojibake silently, whereas a file read with the wrong + encoding fails loudly further down, when its CSV headers do not match. + + >>> detect_encoding('Cesión'.encode('utf-8')) + 'utf-8' + + """ + if not raw_data: + return _DEFAULT_FILE_ENCODING + + for bom, bom_encoding in _BOMS: + if raw_data.startswith(bom): + return bom_encoding + + for encoding in _CANDIDATE_ENCODINGS: + if _decodes_as(raw_data, encoding, is_sample): + return encoding + + raise RuntimeError( + "Programming error: none of the candidate encodings decoded the data.", + _CANDIDATE_ENCODINGS, + ) + + +def _decodes_as(raw_data: bytes, encoding: str, is_sample: bool = False) -> bool: + """ + Return whether ``raw_data`` can be decoded with ``encoding``. + + If ``is_sample`` is true, ``raw_data`` is only the beginning of a larger byte sequence, so an + incomplete multi-byte character at the end of it is not considered an error. + + """ + decoder = codecs.getincrementaldecoder(encoding)() + try: + decoder.decode(raw_data, not is_sample) + except UnicodeDecodeError: + return False - encoding = result['encoding'] - if encoding is None or encoding == 'ascii': - encoding = _DEFAULT_FILE_ENCODING - return encoding + return True diff --git a/src/cl_sii/rtc/parse_cesiones_periodo.py b/src/cl_sii/rtc/parse_cesiones_periodo.py index 873e451b..df3f9dec 100644 --- a/src/cl_sii/rtc/parse_cesiones_periodo.py +++ b/src/cl_sii/rtc/parse_cesiones_periodo.py @@ -29,7 +29,7 @@ from cl_sii.base.constants import SII_OFFICIAL_TZ from cl_sii.extras import mm_fields from cl_sii.libs import csv_utils, mm_utils, rows_processing, tz_utils -from cl_sii.libs.charset_utils import detect_file_encoding +from cl_sii.libs.charset_utils import LOSSLESS_FALLBACK_ENCODING, detect_file_encoding from cl_sii.rtc import data_models_cesiones_periodo from cl_sii.rut import Rut @@ -128,7 +128,7 @@ def parse_cesiones_periodo_csv_file( input_csv_row_schema = input_csv_row_schema_class() with _CesionesPeriodoCsvRowSchemaContext(schema_context): - yield from _parse_cesiones_periodo_csv_file( + yield from _parse_cesiones_periodo_csv_file_with_encoding_fallback( input_csv_row_schema, expected_input_field_names, fields_to_remove_names, @@ -160,6 +160,8 @@ def clean_cesiones_periodo_csv_file( """ + output_new_line_str = '\n' + def _clean_line(original: str) -> str: # Merge consecutive fields that look like they are email addresses (e.g. have the '@' char), # setting a ',' as separator. @@ -189,56 +191,74 @@ def _line_needs_cleaning(original_line: str) -> bool: # The line needs to be cleaned if it does not have the expected number of columns. return original_line.count(';') != _CESIONES_PERIODO_CSV_HEADERS_N - 1 - file_encoding = detect_file_encoding(input_file_path) - output_new_line_str = '\n' - - # note: reading text with `newline=None` means line endings will be autodetected - # (both '\n' and '\r\n' work), and the output of `readline()` includes '\n' - # (in Linux at least) at the end. - with open(input_file_path, mode='rt', encoding=file_encoding, newline=None) as input_f: - query_params: Optional[str] = None - # e.g. 'DATOS_CONSULTA; RUT=76389992-6;TIPO_CONSULTA=CEDENTE;DESDE_DDMMAAAA=01062019;HASTA_DDMMAAAA=01072019' # noqa: E501 - query_params_line = input_f.readline().upper().strip() - csv_headers_line = None - - if query_params_line.startswith('DATOS_CONSULTA;'): - query_params = query_params_line.partition('DATOS_CONSULTA;')[2].strip() - elif query_params_line == _CESIONES_PERIODO_CSV_HEADERS_LINE: - csv_headers_line = query_params_line - else: - # TODO: use a new custom exception for this kind of cases. - raise Exception( - "First line is not the query params line ('DATOS_CONSULTA;'...) " - "nor the expected CSV headers line ('%s...'): %s", - _CESIONES_PERIODO_CSV_HEADERS_LINE[:30], - query_params_line, - ) - - if csv_headers_line is None: - csv_headers_line = input_f.readline().upper().strip() - if not csv_headers_line == _CESIONES_PERIODO_CSV_HEADERS_LINE: + def _clean(file_encoding: str) -> Tuple[Optional[str], int]: + # note: reading text with `newline=None` means line endings will be autodetected + # (both '\n' and '\r\n' work), and the output of `readline()` includes '\n' + # (in Linux at least) at the end. + with open(input_file_path, mode='rt', encoding=file_encoding, newline=None) as input_f: + query_params: Optional[str] = None + # e.g. 'DATOS_CONSULTA; RUT=76389992-6;TIPO_CONSULTA=CEDENTE;DESDE_DDMMAAAA=01062019;HASTA_DDMMAAAA=01072019' # noqa: E501 + query_params_line = input_f.readline().upper().strip() + csv_headers_line = None + + if query_params_line.startswith('DATOS_CONSULTA;'): + query_params = query_params_line.partition('DATOS_CONSULTA;')[2].strip() + elif query_params_line == _CESIONES_PERIODO_CSV_HEADERS_LINE: + csv_headers_line = query_params_line + else: # TODO: use a new custom exception for this kind of cases. raise Exception( - "CSV headers line does not match what is expected ('%s...'): %s", + "First line is not the query params line ('DATOS_CONSULTA;'...) " + "nor the expected CSV headers line ('%s...'): %s", _CESIONES_PERIODO_CSV_HEADERS_LINE[:30], - csv_headers_line, + query_params_line, ) - sorted_lines = sorted(input_f) - n_sorted_lines = len(sorted_lines) - sorted_clean_lines = [] - for line in sorted_lines: - sorted_clean_lines.append(_clean_line(line) if _line_needs_cleaning(line) else line) - - with open(output_file_path, mode='wt', encoding=file_encoding, newline=None) as output_f: - output_f.writelines( - [ - csv_headers_line + output_new_line_str, - ] - ) - output_f.writelines(sorted_clean_lines) + if csv_headers_line is None: + csv_headers_line = input_f.readline().upper().strip() + if not csv_headers_line == _CESIONES_PERIODO_CSV_HEADERS_LINE: + # TODO: use a new custom exception for this kind of cases. + raise Exception( + "CSV headers line does not match what is expected ('%s...'): %s", + _CESIONES_PERIODO_CSV_HEADERS_LINE[:30], + csv_headers_line, + ) + + sorted_lines = sorted(input_f) + n_sorted_lines = len(sorted_lines) + sorted_clean_lines = [] + for line in sorted_lines: + sorted_clean_lines.append(_clean_line(line) if _line_needs_cleaning(line) else line) + + with open( + output_file_path, mode='wt', encoding=file_encoding, newline=None + ) as output_f: + output_f.writelines( + [ + csv_headers_line + output_new_line_str, + ] + ) + output_f.writelines(sorted_clean_lines) + + return query_params, n_sorted_lines - return query_params, n_sorted_lines + file_encoding = detect_file_encoding(input_file_path) + + try: + return _clean(file_encoding) + except UnicodeDecodeError: + # note: the encoding is detected from a sample of the file, so it may not be able to decode + # the rest of it. Fall back to an encoding that decodes any byte sequence. + # note: no guard is needed for the case where the detected encoding already is the fallback + # one, because reading with it never raises 'UnicodeDecodeError', so this handler cannot + # run in that case. + logger.warning( + "Failed to decode file with the detected encoding ('%s'); retrying with '%s': %s", + file_encoding, + LOSSLESS_FALLBACK_ENCODING, + input_file_path, + ) + return _clean(LOSSLESS_FALLBACK_ENCODING) ############################################################################### @@ -469,6 +489,81 @@ class _CesionesPeriodoCsvDialect(csv.Dialect): quoting = csv.QUOTE_MINIMAL +def _parse_cesiones_periodo_csv_file_with_encoding_fallback( + input_csv_row_schema: CesionesPeriodoCsvRowSchema, + expected_input_field_names: Sequence[str], + fields_to_remove_names: Sequence[str], + input_file_path: str, + input_file_encoding: str, + n_rows_offset: int, + max_n_rows: Optional[int] = None, +) -> Iterable[ + Tuple[ + Optional[data_models_cesiones_periodo.CesionesPeriodoEntry], + int, + Dict[str, object], + Dict[str, object], + ] +]: + """ + Same as :func:`_parse_cesiones_periodo_csv_file`, but resilient to a wrong file encoding. + + The encoding is detected from a sample of the file, so it may not be able to decode the rest of + it. In that case the file is parsed again with an encoding that decodes any byte sequence, + skipping the entries that were already yielded. + + .. warning:: + The fallback encoding is byte-preserving, not meaning-preserving: it maps every byte to a + character, so nothing is lost, but the non-ASCII characters of the entries yielded by the + retry come back as mojibake if the file really was in another encoding (e.g. + ``'Cesión'`` for a UTF-8 ``'Cesión'``). That is deliberate: decoding with the detected + encoding and ``errors='replace'`` would instead discard the undecodable bytes + irrecoverably. The warning that is logged records the row from which the fallback applies, + so that the affected entries can be identified afterwards. + + """ + n_yielded_rows = 0 + + try: + for item in _parse_cesiones_periodo_csv_file( + input_csv_row_schema, + expected_input_field_names, + fields_to_remove_names, + input_file_path, + input_file_encoding, + n_rows_offset, + max_n_rows, + ): + n_yielded_rows += 1 + yield item + except UnicodeDecodeError: + if input_file_encoding == LOSSLESS_FALLBACK_ENCODING: + raise + logger.warning( + "Failed to decode file with the detected encoding ('%s') after %s row(s);" + " retrying with '%s' from row %s onwards: %s", + input_file_encoding, + n_yielded_rows, + LOSSLESS_FALLBACK_ENCODING, + n_rows_offset + n_yielded_rows + 1, + input_file_path, + ) + + # note: the rows that were already yielded are skipped through 'n_rows_offset', so they are + # not deserialized nor converted a second time (which would, among other things, log + # their conversion failures again). 'max_n_rows' is reduced by the same amount so that + # the window and the 'MaxRowsExceeded' threshold remain the ones the caller asked for. + yield from _parse_cesiones_periodo_csv_file( + input_csv_row_schema, + expected_input_field_names, + fields_to_remove_names, + input_file_path, + LOSSLESS_FALLBACK_ENCODING, + n_rows_offset + n_yielded_rows, + max_n_rows - n_yielded_rows if max_n_rows is not None else None, + ) + + def _parse_cesiones_periodo_csv_file( input_csv_row_schema: CesionesPeriodoCsvRowSchema, expected_input_field_names: Sequence[str], diff --git a/src/tests/test_libs_charset_utils.py b/src/tests/test_libs_charset_utils.py index 5ec2529d..c764571d 100644 --- a/src/tests/test_libs_charset_utils.py +++ b/src/tests/test_libs_charset_utils.py @@ -1,45 +1,300 @@ +import codecs import tempfile import unittest -from cl_sii.libs.charset_utils import clean_unicode, detect_file_encoding +from cl_sii.libs.charset_utils import clean_unicode, detect_encoding, detect_file_encoding + + +# Contents of a fictional file of "cesiones" of a period, modeled after a real one retrieved from +# the SII that used to make 'detect_file_encoding' return 'big5' and thus make the parsing fail +# with a 'UnicodeDecodeError'. All RUTs, company names and emails below are made up. +# The non-ASCII byte is in "N\xfdCLEO DE PRUEBA SPA". +_CESIONES_PERIODO_FILE_WITH_NON_ASCII_BYTES = ( + b'DATOS_CONSULTA; RUT=11111111-1;TIPO_CONSULTA=DEUDOR;DESDE_DDMMAAAA=24032025;' + b'HASTA_DDMMAAAA=30032025\r\n' + b'VENDEDOR;ESTADO_CESION;DEUDOR;MAIL_DEUDOR;TIPO_DOC;NOMBRE_DOC;FOLIO_DOC;' + b'FCH_EMIS_DTE;MNT_TOTAL;CEDENTE;RZ_CEDENTE;MAIL_CEDENTE;CESIONARIO;' + b'RZ_CESIONARIO;MAIL_CESIONARIO;FCH_CESION;MNT_CESION;FCH_VENCIMIENTO\r\n' + b'22222222-2;Cesion Vigente;11111111-1;null;33;Factura Electronica;1001;' + b'2025-03-24;100000;22222222-2;N\xfdCLEO DE PRUEBA SPA;contacto@ejemplo-uno.cl;' + b'33333333-3;Factoring de Prueba S.A;contacto@ejemplo-dos.cl;' + b'2025-03-24 19:23;100000;2025-05-23\r\n' + b'22222222-2;Cesion Vigente;11111111-1;null;33;Factura Electronica;1002;' + b'2025-03-27;200000;22222222-2;N\xfdCLEO DE PRUEBA SPA;contacto@ejemplo-tres.cl;' + b'44444444-4;Otra Factoring de Prueba S.A.;contacto@ejemplo-cuatro.cl;' + b'2025-03-27 17:23;200000;2025-05-26\r\n' +) class FunctionsTest(unittest.TestCase): def test_clean_unicode(self) -> None: + # %% -----Arrange----- + expected = 'É' - self.assertEqual(clean_unicode('É'), expected) - self.assertEqual(clean_unicode('\u00c9'), expected) - self.assertEqual(clean_unicode('\u0045\u0301'), expected) - self.assertEqual(clean_unicode('\N{LATIN CAPITAL LETTER E WITH ACUTE}'), expected) - self.assertEqual( - clean_unicode('\N{LATIN CAPITAL LETTER E}\N{COMBINING ACUTE ACCENT}'), expected - ) + + # %% -----Act----- + + results = [ + clean_unicode('É'), + clean_unicode('É'), + clean_unicode('É'), + clean_unicode('\N{LATIN CAPITAL LETTER E WITH ACUTE}'), + clean_unicode('\N{LATIN CAPITAL LETTER E}\N{COMBINING ACUTE ACCENT}'), + ] + + # %% -----Assert----- + + for result in results: + self.assertEqual(expected, result) + + # %% ----- def test_clean_unicode_empty_string(self) -> None: + # %% -----Arrange----- + empty_string = '' - self.assertEqual(clean_unicode(empty_string), empty_string) + + # %% -----Act----- + + result = clean_unicode(empty_string) + + # %% -----Assert----- + + self.assertEqual(empty_string, result) + + # %% ----- def test_detect_encoding_utf8(self) -> None: + # %% -----Arrange----- + content = 'Este es un texto en español con acentos y ñ'.encode('utf-8') with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file.write(content) temp_file_path = temp_file.name - self.assertEqual(detect_file_encoding(temp_file_path), 'utf-8') + + # %% -----Act----- + + result = detect_file_encoding(temp_file_path) + + # %% -----Assert----- + + self.assertEqual('utf-8', result) + + # %% ----- def test_detect_encoding_ascii(self) -> None: + # %% -----Arrange----- + content = 'This is a simple ASCII text.'.encode('ascii') with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file.write(content) temp_file_path = temp_file.name - self.assertEqual(detect_file_encoding(temp_file_path), 'utf-8') + + # %% -----Act----- + + result = detect_file_encoding(temp_file_path) + + # %% -----Assert----- + + self.assertEqual('utf-8', result) + + # %% ----- def test_detect_encoding_default(self) -> None: """ - The file does not have a valid encoding - so the default encoding should be returned + The file does not match any known encoding, so an encoding that decodes any byte sequence + should be returned. """ + # %% -----Arrange----- + content = b'\x80\x81\x82\x83\x84\x85' with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file.write(content) temp_file_path = temp_file.name - self.assertEqual(detect_file_encoding(temp_file_path), 'utf-8') + + # %% -----Act----- + + result = detect_file_encoding(temp_file_path) + + # %% -----Assert----- + + self.assertEqual('latin-1', result) + + # %% ----- + + def test_detect_encoding_of_empty_data(self) -> None: + # %% -----Arrange----- + + content = b'' + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + self.assertEqual('utf-8', result) + + # %% ----- + + def test_detect_encoding_of_ascii_data(self) -> None: + # %% -----Arrange----- + + content = b'This is a simple ASCII text.' + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + self.assertEqual('utf-8', result) + + # %% ----- + + def test_detect_encoding_of_utf8_data(self) -> None: + # %% -----Arrange----- + + content = 'Este es un texto en español con acentos y ñ'.encode('utf-8') + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + self.assertEqual('utf-8', result) + + # %% ----- + + def test_detect_encoding_of_utf8_sample_truncated_mid_character(self) -> None: + """ + A sample of a larger file may be cut in the middle of a multi-byte character. + """ + # %% -----Arrange----- + + content = ('Este es un texto en español con acentos y ñ' * 100).encode('utf-8') + self.assertTrue(content.endswith(b'\xc3\xb1')) + truncated_content = content[:-1] + + # %% -----Act----- + + result = detect_encoding(truncated_content, is_sample=True) + + # %% -----Assert----- + + self.assertEqual('utf-8', result) + + # %% ----- + + def test_detect_encoding_of_complete_data_truncated_mid_character(self) -> None: + """ + If the data is not a sample, the returned encoding must be able to decode all of it. + """ + # %% -----Arrange----- + + contents = ( + b'\xc3', + ('Este es un texto en español con acentos y ñ' * 100).encode('utf-8')[:-1], + ) + + for content in contents: + with self.subTest(content=content[:20]): + # %% -----Act----- + + detected_encoding = detect_encoding(content) + + # %% -----Assert----- + + # It must not raise. + content.decode(detected_encoding) + + # %% ----- + + def test_detect_encoding_of_legacy_8_bit_data(self) -> None: + # %% -----Arrange----- + + text = 'Cesión Vigente;MI CAÑOPITO SA;Factura Electrónica' * 20 + + for encoding in ('iso-8859-1', 'cp1252'): + with self.subTest(encoding=encoding): + content = text.encode(encoding) + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + # note: an "iso-8859-1" input is decoded as "cp1252", which for this text is + # equivalent: both share the whole 0xA0-0xFF range. + self.assertEqual('cp1252', result) + self.assertEqual(text, content.decode(result)) + + # %% ----- + + def test_detect_encoding_of_data_with_bom(self) -> None: + # %% -----Arrange----- + + text = 'Cesión Vigente' + cases = ( + (b'', 'utf-8-sig', 'utf-8-sig'), + (b'', 'utf-16', 'utf-16'), + (codecs.BOM_UTF16_LE, 'utf-16-le', 'utf-16'), + (codecs.BOM_UTF16_BE, 'utf-16-be', 'utf-16'), + (b'', 'utf-32', 'utf-32'), + ) + + for bom, encoding, expected_encoding in cases: + with self.subTest(encoding=encoding): + content = bom + text.encode(encoding) + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + self.assertEqual(expected_encoding, result) + self.assertEqual(text, content.decode(expected_encoding)) + + # %% ----- + + def test_detect_encoding_of_data_with_a_few_non_ascii_bytes(self) -> None: + """ + Regression test: this data used to be detected as 'big5', which can not decode it. + """ + # %% -----Arrange----- + + content = _CESIONES_PERIODO_FILE_WITH_NON_ASCII_BYTES + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + self.assertEqual('cp1252', result) + # It must not raise. + content.decode(result) + + # %% ----- + + def test_detect_encoding_of_utf16_data_without_bom(self) -> None: + """ + UTF-16 is recognized only by its byte order mark. Without one, the data is decoded by the + first candidate encoding that accepts it, and the caller finds out further down. + """ + # %% -----Arrange----- + + content = ('Cesión Vigente;Factura Electrónica\r\n' * 20).encode('utf-16-le') + + # %% -----Act----- + + result = detect_encoding(content) + + # %% -----Assert----- + + self.assertNotEqual('utf-16', result) + + # %% ----- diff --git a/src/tests/test_rtc_parse_cesiones_periodo.py b/src/tests/test_rtc_parse_cesiones_periodo.py index 6592c5c9..d6e3798c 100644 --- a/src/tests/test_rtc_parse_cesiones_periodo.py +++ b/src/tests/test_rtc_parse_cesiones_periodo.py @@ -2,12 +2,19 @@ import unittest from collections import OrderedDict from datetime import date, datetime +from typing import Tuple +from unittest import mock from cl_sii.base.constants import SII_OFFICIAL_TZ from cl_sii.dte.constants import TipoDte +from cl_sii.libs.charset_utils import _DETECTION_SAMPLE_SIZE, LOSSLESS_FALLBACK_ENCODING from cl_sii.libs.tz_utils import convert_naive_dt_to_tz_aware +from cl_sii.rtc import parse_cesiones_periodo from cl_sii.rtc.data_models_cesiones_periodo import CesionesPeriodoEntry from cl_sii.rtc.parse_cesiones_periodo import ( + _CESIONES_PERIODO_CSV_HEADERS_LINE, + CesionesPeriodoCsvRowSchema, + _parse_cesiones_periodo_csv_file_with_encoding_fallback, clean_cesiones_periodo_csv_file, parse_cesiones_periodo_csv_file, ) @@ -510,3 +517,136 @@ def test_parse_cesiones_periodo_csv_file_fail_1(self) -> None: ), ], ) + + def test_clean_cesiones_periodo_csv_file_with_undecodable_bytes_after_encoding_sample( + self, + ) -> None: + """ + The encoding is detected from a sample of the file, so it may fail to decode the rest of it. + """ + # %% -----Arrange----- + + input_file_content, n_rows = _make_cesiones_periodo_csv_file_content_with_late_bad_byte() + + # note: for the output we need 'w+b' to be able to read and write data. + with tempfile.NamedTemporaryFile(mode='w+b') as output_tmp_file: + with tempfile.NamedTemporaryFile(mode='wb') as input_tmp_file: + input_tmp_file.write(input_file_content) + input_tmp_file.flush() + + # %% -----Act----- + + with self.assertLogs('cl_sii.rtc.parse_cesiones_periodo', level='WARNING'): + query_params, n_cesiones = clean_cesiones_periodo_csv_file( + input_tmp_file.name, output_tmp_file.name + ) + output_tmp_file.seek(0) + output_file_content = output_tmp_file.read() + + # %% -----Assert----- + + self.assertEqual( + query_params, + 'RUT=75320502-0;TIPO_CONSULTA=DEUDOR;DESDE_DDMMAAAA=01032019;HASTA_DDMMAAAA=31032019', + ) + self.assertEqual(n_rows, n_cesiones) + self.assertIn('MI CAMPITO SA', output_file_content.decode('latin-1')) + + # %% ----- + + def test_parse_cesiones_periodo_csv_file_with_undecodable_bytes_after_encoding_sample( + self, + ) -> None: + # %% -----Arrange----- + + input_file_content, n_rows = _make_cesiones_periodo_csv_file_content_with_late_bad_byte( + include_query_params_line=False + ) + + with tempfile.NamedTemporaryFile(mode='wb') as input_tmp_file: + input_tmp_file.write(input_file_content) + input_tmp_file.flush() + + # %% -----Act----- + + with self.assertLogs('cl_sii.rtc.parse_cesiones_periodo', level='WARNING'): + results = list(parse_cesiones_periodo_csv_file(input_tmp_file.name)) + + # %% -----Assert----- + + self.assertEqual(len(results), n_rows) + # Each entry was parsed, and none of them yielded errors. + self.assertTrue(all(entry is not None for entry, _, _, _ in results)) + self.assertTrue(all(not row_errors for _, _, _, row_errors in results)) + + # %% ----- + + def test_parse_cesiones_periodo_csv_file_with_encoding_fallback_does_not_retry_the_fallback( + self, + ) -> None: + """ + A decoding failure with the fallback encoding itself is propagated, not retried. + """ + # %% -----Arrange----- + + def _raise_unicode_decode_error(*args: object, **kwargs: object) -> None: + raise UnicodeDecodeError( + LOSSLESS_FALLBACK_ENCODING, b'\xfd', 0, 1, 'fake decoding failure' + ) + + g = _parse_cesiones_periodo_csv_file_with_encoding_fallback( + CesionesPeriodoCsvRowSchema(), + tuple(_CESIONES_PERIODO_CSV_HEADERS_LINE.split(';')), + (), + 'non-existent-file.csv', + LOSSLESS_FALLBACK_ENCODING, + 0, + ) + + # %% -----Act----- + + with mock.patch.object( + parse_cesiones_periodo, + '_parse_cesiones_periodo_csv_file', + _raise_unicode_decode_error, + ): + # %% -----Assert----- + + with self.assertRaises(UnicodeDecodeError): + list(g) + + # %% ----- + + +def _make_cesiones_periodo_csv_file_content_with_late_bad_byte( + include_query_params_line: bool = True, +) -> Tuple[bytes, int]: + """ + Return the contents of a file whose beginning is valid UTF-8, and a byte that is not, beyond + the sample used for encoding detection, plus the number of data rows. + """ + row_template = ( + b'51532520-4;Cesion Vigente;75320502-0;null;33;Factura Electronica;%d;' + b'2019-02-15;96628;51532520-4;MI CAMPITO SA;mi@campito.cl;96667560-8;' + b'POBRES SERVICIOS FINANCIEROS S.A.;ejecutivo@pobres.cl;' + b'2019-03-07 13:31;96628;2019-04-16\r\n' + ) + # note: the rows before the last one must add up to more than the detection sample, so + # that the undecodable byte is beyond it. + n_rows = _DETECTION_SAMPLE_SIZE // len(row_template % 1) + 2 + # note: the folio must be greater than zero. + rows = [row_template % (row_ix + 1) for row_ix in range(n_rows)] + # The last row is the only one that can not be decoded as UTF-8. + rows[-1] = rows[-1].replace(b'MI CAMPITO SA', b'MI CAMPITO S\xfd') + assert len(b''.join(rows[:-1])) > _DETECTION_SAMPLE_SIZE + + lines = [] + if include_query_params_line: + lines.append( + b'DATOS_CONSULTA; RUT=75320502-0;TIPO_CONSULTA=DEUDOR;DESDE_DDMMAAAA=01032019;' + b'HASTA_DDMMAAAA=31032019\r\n' + ) + lines.append(_CESIONES_PERIODO_CSV_HEADERS_LINE.encode('ascii') + b'\r\n') + lines.extend(rows) + + return b''.join(lines), n_rows