Skip to content

Reco counts download used text/csv and LF instead of text/plain and CRLF #3363

Description

@adamtheturtle

Found by probing the endpoint against real Vuforia with a throwaway database, which #3359 and #3360 were waiting to be able to do. Both are fixed locally on adamtheturtle/create-vws-web-tools-issue, but are recorded here so that the defects and their evidence are not lost in a branch.

The bugs

The mock served a generated reco counts report with the wrong content type and the wrong line ending.

Mock beforeReal Vuforia
Content-Typetext/csvtext/plain
Header rowtarget_id,reco_count\ntarget_id,reco_count\r\n

Both matter to a caller. Code which branches on the content type, or which splits on \r\n, or which compares a downloaded body byte for byte, passes against the mock and behaves differently against real Vuforia.

Evidence

Requesting a report for the current month and fetching the returned presigned URL:

status: 200
headers: {..., 'Content-Type': 'text/plain', 'Content-Length': '22',
'Server': 'AmazonS3', 'x-amz-server-side-encryption': 'AES256', ...}
body: b'target_id,reco_count\r\n'

Note Content-Length: 22, which is the 20 characters of the header row plus a carriage return and a line feed.

Real Vuforia serves the report from S3 rather than from the VWS API, which is why the content type is S3's default rather than a CSV type. The bucket in this observation was guacamole-targetstore-production-targets.s3.us-west-1.amazonaws.com.

The report contained only the header row, which confirms the mock's shape for a database with no recognitions. The database was created seconds earlier and had no targets, so an empty report is expected. Whether a row for a target with recognitions also uses \r\n is untested, and is part of #3356.

The fix

  • src/mock_vws/reco_counts.py: the header row now ends \r\n.
  • src/mock_vws/_reco_counts_web_api.py: the download response now has a text/plain content type.
  • tests/mock_vws/test_reco_counts_report.py: TestDownloadReport asserts both.

Why the mock got this wrong

The endpoint was added in #3357 without being able to reach real Vuforia, because a request needs a database ID that the test credentials did not carry. text/csv and \n were the natural guesses for a file described in the documentation as a CSV report. differences-to-vws.rst did flag the download response headers and the CSV content as unverified, so the guesses were labelled, but a labelled guess is still a wrong answer for anyone who trusted it.

Acceptance criteria

  • The fixes above are merged.
  • differences-to-vws.rst no longer lists the download content type or the CSV content as unverified, since both have now been observed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions