Skip to content

[ENG-1272] Only use register_uri() for multi-URL registration #11

@cslzchen

Description

@cslzchen

Here is the piece of code for register_uri() and register_json_uri() as shown below.

def register_uri(self, method, uri, **options):
    if any(x.get('params') for x in options.get('responses', [])):
        raise ValueError('Cannot specify params in responses, call register multiple times.')
    params = options.pop('params', {})
    url = ImmutableFurl(uri, params=params)
    self.registry[(method, url)] = options.get('responses', options)

def register_json_uri(self, method, uri, **options):
    body = json.dumps(options.pop('body', None)).encode('utf-8')
    headers = {'Content-Type': 'application/json'}
    headers.update(options.pop('headers', {}))
    self.register_uri(method, uri, body=body, headers=headers, **options)

When multiple URLs are registered using the responses param, the extra header headers = {'Content-Type': 'application/json'} in register_json_uri() is not used by register_uri() at all. In fact, the body param is not used either and only the responses param takes effect. This sometimes confuses the developers when they think that register_json_uri() guarantees the Content-Type header to be added while it actually doesn't for the multi-URL registration case. This breaks tests with aiohttp3 where .json() requires a valid Content-Type header in the response all the time. Below is the proposed fix.

Given that multi-URL registration requires and only uses the raw response list, it doesn't make any sense to use register_json_uri(). The two things that register_json_uri()does for us is dumping the body and adding the header, both are redundant when we need to provide the raw responses anyway as shown below in the multi-URL scenario.

aiohttpretty.register_uri(
    'POST',
    create_tree_url,
    **{
        "responses": [
            {
                'body': json.dumps(crud_fixtures_1).encode('utf-8'),
                'status': 201,
                'headers': {'Content-Type': 'application/json'}
            },
            {
                'body': json.dumps(crud_fixtures_2).encode('utf-8'),
                'status': 201,
                'headers': {'Content-Type': 'application/json'}
            },
        ]
    }
)

Please add a docstr / comment that multi-URL registration should only use register_uri() and the developers must provide everything in the responses list. In addition, throw an error when register_json_uri() is called with responses param (i.e. the multi-URL registration case).

Here are the two related commits in the PR: Throw GitHub into the Oath Pit where the issue is discovered.

  • Explicitly add the Content-Type header: e1ee3e
  • Use register_uri() instead of register_json_uri(): 851578

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