shopdb-flask/tests/test_openapi_spec.py

"""The generated spec has to be usable by a machine, not just readable.

docs/openapi.json is the only machine-readable description of this API, and it
is what the MCP server builds its tools from. It was published with no
`parameters` and no `requestBody` on any operation - invalid OpenAPI 3.1, and
worse in practice: a tool built from an operation with no parameters has nowhere
to put the id its caller supplied, so every argument was dropped in silence. A
call for one asset returned the list.

These tests are the floor. They check the spec's shape, not its prose.
"""

import json
import re
from pathlib import Path

import pytest

REPO = Path(__file__).resolve().parents[1]
SPEC = REPO / 'docs' / 'openapi.json'

pytestmark = pytest.mark.skipif(
    not SPEC.is_file(),
    reason='no generated spec in this checkout')


@pytest.fixture(scope='module')
def spec():
    return json.loads(SPEC.read_text())


def operations(spec):
    for path, item in spec['paths'].items():
        for verb, operation in item.items():
            yield path, verb, operation


def test_every_templated_path_declares_its_parameters(spec):
    """`/api/assets/{id}` with no parameters is not a description of anything."""
    offenders = [
        '%s %s' % (verb.upper(), path)
        for path, verb, operation in operations(spec)
        if '{' in path and not operation.get('parameters')
    ]
    assert not offenders, (
        'These operations have a templated path and no parameters, so a '
        'generated client cannot fill it in:\n  ' + '\n  '.join(offenders[:15]))


def test_declared_parameters_match_the_template(spec):
    """A parameter that is not in the path, or a placeholder with no parameter,
    is the same defect wearing different clothes."""
    offenders = []
    for path, verb, operation in operations(spec):
        intemplate = set(re.findall(r'\{([^}]+)\}', path))
        declared = {p['name'] for p in operation.get('parameters', [])
                    if p.get('in') == 'path'}
        if intemplate != declared:
            offenders.append('%s %s: template %s, declared %s'
                             % (verb.upper(), path, sorted(intemplate), sorted(declared)))
    assert not offenders, '\n  '.join(offenders[:15])


def test_write_verbs_can_carry_a_body(spec):
    offenders = [
        '%s %s' % (verb.upper(), path)
        for path, verb, operation in operations(spec)
        if verb in ('post', 'put') and 'requestBody' not in operation
    ]
    assert not offenders, (
        'These write operations declare no request body, so a client built '
        'from this spec cannot send one:\n  ' + '\n  '.join(offenders[:15]))


def documented_auth(operation):
    """The auth string the generator wrote into this operation's description.

    The spec is the only thing this module reads, and the description is where
    the human-readable auth lands ('**Auth:** jwt-optional (...)'). Comparing
    the published `security` against it is how a lie gets caught: the two come
    from the same inventory entry and must agree.
    """
    match = re.search(r'\*\*Auth:\*\*\s*(.+)', operation.get('description') or '')
    return match.group(1).strip().lower() if match else ''


def test_optional_auth_is_expressed_as_optional(spec):
    """A route whose auth is optional is published as optional, either way round.

    WHY THE COUNT IS GONE. This asserted `len(optional) > 50`, on the premise
    that optional auth was the majority case here - it said so in as many
    words. It is not any more. The guest role and the permission model moved
    the read surface to `permission:*`, which publishes as bearer-required and
    is correct, and the optional count fell to 14. The assertion went red
    without a single route being wrong.

    A count was never the property anyone cares about. It goes red when the
    codebase legitimately changes shape, and it stays green while an individual
    optional route is published as bearer-required, which is the actual defect.
    Lowering the number to 14 would only buy time until the next legitimate
    move. So the premise is dropped and the per-route property is checked
    instead, whatever the total: an operation whose auth says optional carries
    BOTH the empty requirement and a scheme, and an operation that publishes
    the empty requirement says optional in its auth.
    """
    offenders = []
    for path, verb, operation in operations(spec):
        auth = documented_auth(operation)
        security = operation.get('security') or []
        openly = any(item == {} for item in security)
        scheme = any(item for item in security)
        if 'optional' in auth:
            if not openly:
                offenders.append(
                    '%s %s: auth says optional, published as required (%r)'
                    % (verb.upper(), path, security))
            elif not scheme:
                offenders.append(
                    '%s %s: auth says optional, published with no scheme at '
                    'all, so a caller is never told what the credential is'
                    % (verb.upper(), path))
        elif openly:
            offenders.append(
                '%s %s: published as optional, but its auth says %r'
                % (verb.upper(), path, auth))
    assert not offenders, (
        'Optional auth means "works without a credential, does more with one", '
        'and OpenAPI writes it as the empty requirement ALONGSIDE the '
        'scheme:\n  ' + '\n  '.join(offenders[:15]))


def test_an_operation_needing_no_credential_says_so_in_its_auth(spec):
    """`security: []` is a machine-readable claim that no credential is needed.

    Nine operations published it while their handler required one, including
    the two that hand a bootstrapping PC its agent package - `security: []` and
    a 404 for anyone who does not present the single-use token. The generator
    reached that by substring: the auth string started with the word 'none', in
    a sentence that went on to name the token. This is the gate on that class
    of defect from the spec side, where a reader or a generated client sees it.
    """
    offenders = [
        '%s %s: %r' % (verb.upper(), path, documented_auth(operation))
        for path, verb, operation in operations(spec)
        if not operation.get('security')
        and 'none' not in documented_auth(operation)
        and 'public' not in documented_auth(operation)
    ]
    assert not offenders, (
        'These operations publish "no credentials needed" while their auth '
        'names a credential:\n  ' + '\n  '.join(offenders[:15]))


def test_an_endpoint_that_needs_no_token_cannot_fail_with_401(spec):
    offenders = [
        '%s %s' % (verb.upper(), path)
        for path, verb, operation in operations(spec)
        if not operation.get('security') and '401' in operation.get('responses', {})
    ]
    assert not offenders, '\n  '.join(offenders[:15])


def test_every_operation_documents_failure(spec):
    offenders = [
        '%s %s' % (verb.upper(), path)
        for path, verb, operation in operations(spec)
        if 'default' not in operation.get('responses', {})
    ]
    assert not offenders, (
        'These operations document only success:\n  ' + '\n  '.join(offenders[:15]))


def test_the_envelopes_are_defined_and_referenced(spec):
    schemas = spec.get('components', {}).get('schemas', {})
    assert 'SuccessEnvelope' in schemas and 'ErrorEnvelope' in schemas
    # The error shape is the one people get wrong: nested under data.
    error = schemas['ErrorEnvelope']['properties']['data']['properties']['error']
    assert set(error['required']) == {'code', 'message'}


def test_summaries_are_not_cut_mid_word(spec):
    """A truncated summary is what a tool picker shows as the whole description
    of a call."""
    offenders = []
    for path, verb, operation in operations(spec):
        summary = operation.get('summary', '')
        if summary.endswith('...'):
            body = summary[:-3]
            if body and not body[-1].isalnum() and body[-1] not in ')]"\'':
                offenders.append('%s %s: %r' % (verb.upper(), path, summary[-40:]))
    assert not offenders, '\n  '.join(offenders[:15])


def test_every_parameter_is_well_formed(spec):
    """A parameter with no name, no location or no schema is not a parameter.

    Query parameters are derived from each handler's AST now rather than left
    out, which is 87 operations that could not be filtered, searched or paged
    from this document before. Derived means a code change can produce a
    malformed one, so the shape is checked here rather than assumed.
    """
    offenders = []
    for path, verb, operation in operations(spec):
        for parameter in operation.get('parameters', []):
            where = '%s %s: %r' % (verb.upper(), path, parameter.get('name'))
            if not parameter.get('name'):
                offenders.append('%s %s: a parameter with no name' % (verb.upper(), path))
            if parameter.get('in') not in ('path', 'query', 'header', 'cookie'):
                offenders.append(where + ' has no valid `in`')
            if not isinstance(parameter.get('required'), bool):
                offenders.append(where + ' does not say whether it is required')
            if not parameter.get('schema'):
                offenders.append(where + ' has no schema')
            if parameter.get('in') == 'path' and not parameter.get('required'):
                offenders.append(where + ' is in the path and optional, which '
                                         'OpenAPI does not allow')
    assert not offenders, '\n  '.join(offenders[:15])


def test_no_parameter_is_declared_twice(spec):
    """(name, in) is the identity of a parameter. Two entries for one name is
    ambiguous to a generated client, which picks one and drops the other."""
    offenders = []
    for path, verb, operation in operations(spec):
        seen = set()
        for parameter in operation.get('parameters', []):
            key = (parameter.get('name'), parameter.get('in'))
            if key in seen:
                offenders.append('%s %s: %s in %s'
                                 % (verb.upper(), path, key[0], key[1]))
            seen.add(key)
    assert not offenders, '\n  '.join(offenders[:15])


def test_a_query_parameter_is_not_the_path_argument_under_another_name(spec):
    """`/api/assets/{assetid}?assetid=` is a contradiction: the same value in
    two places, and a generated client has to guess which one the route reads."""
    offenders = []
    for path, verb, operation in operations(spec):
        intemplate = set(re.findall(r'\{([^}]+)\}', path))
        query = {p['name'] for p in operation.get('parameters', [])
                 if p.get('in') == 'query'}
        clash = sorted(intemplate & query)
        if clash:
            offenders.append('%s %s: %s' % (verb.upper(), path, clash))
    assert not offenders, '\n  '.join(offenders[:15])