Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identified regression in bravado_core.spec.Spec if internally_dereference_refs is enabled #275

Closed
macisamuele opened this issue Jun 1, 2018 · 1 comment
Assignees

Comments

@macisamuele
Copy link
Collaborator

macisamuele commented Jun 1, 2018

A regression as been introduced by d23868c.

Affected versions

bravado-core: 4.13.3, 4.13.4, 5.0.0 and 5.0.1

Symptoms

After initialising a bravado.client.SwaggerClient with internally_dereference_refs config enabled you're not able to communicate with the service.
The traceback will look like

  File "/home/maci/pg/test_service/virtualenv_run/lib/python3.6/site-packages/bravado/client.py", line 243, in __call__
    self.operation, request_options, **op_kwargs)
  File "/home/maci/pg/test_service/virtualenv_run/lib/python3.6/site-packages/bravado/client.py", line 271, in construct_request
    url = operation.swagger_spec.api_url.rstrip('/') + operation.path_name
AttributeError: 'NoneType' object has no attribute 'rstrip'

Motivations

While working on #263 I've ensured that resources are rebuilt in case internally_dereference_refs is set.
This was made in order to guarantee that all the references (on the already built resources) were stripped out, so after building the Spec object no references will be available.
Unfortunately, build_resource will save a pointer to the used Spec instance into the resource itself but we're using a temporary instance that was not fully initialised.
The effect of this is that a Spec instance that is fully dereferenced will, internally, point to an other Spec instance bind into Operation instances (the pointed Spec instance will be used to send requests, validate responses, etc).

How to fix this

The results of the regression is that Spec object could raise weird exceptions due to undefined api_url and uses double the memory needed (which is sad too).

To fix the issue will be sufficient to replace build_resources(tmp_spec) with build_resources(swagger_spec) but we need to ensure that at that time _internal_spec_dict has been updated to point toward the dereferenced version of the spec dict.

How to reproduce and test

spec_dict = {
    "swagger": "2.0",
    "info": {"title": "Test", "version": "1.0"},
    "paths": {
        "/endpoint": {
            "get": {
                "operationId": "operation",
                "responses": {"200": {"description": "random response"}},
                "tags": ["tag"],
            }
        }
    },
}
spec = Spec.from_dict(spec_dict, config={'internally_dereference_refs': True})
spec_in_object = spec.resources['tag'].operations['operation'].swagger_spec
assert spec == spec_in_object
@macisamuele
Copy link
Collaborator Author

Fixed on version 5.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant