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.

Sign up for GitHub

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

Added ability to use suffixes with a controller that has been already registered without suffixes. #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added ability to use suffixes with a controller that has been already registered without suffixes. #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01e1913
Added ability to use suffixes with a controller that has been already…
ebensonwwg Jun 26, 2020
346f65b
Renamed resource=pet_resource
ebensonwwg Jul 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 51 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class PetSchema(Schema):
# Create Falcon web app
app = falcon.API()

class RandomPetResource:
class PetResource:
def on_get(self, req, resp):
"""A cute furry animal endpoint.
---
Expand All @@ -56,10 +56,29 @@ class RandomPetResource:
pet = get_random_pet() # returns JSON
resp.media = pet

def on_get_one(self, req, resp, pet):
"""A cute furry animal endpoint.
---
description: Get a random pet
parameters:
- in: path
name: pet
required: true
schema:
type: string
responses:
200:
description: A pet to be returned
schema: PetSchema
"""
pet = get_pet(pet) # returns JSON
resp.media = pet

# create instance of resource
random_pet_resource = RandomPetResource()
pet_resource = PetResource()
# pass into `add_route` for Falcon
app.add_route("/random", random_pet_resource)
app.add_route("/random", pet_resource)
app.add_route("/{pet}", pet_resource, suffix="one")


# Create an APISpec
Expand All @@ -77,7 +96,10 @@ spec = APISpec(
spec.components.schema('Category', schema=CategorySchema)
spec.components.schema('Pet', schema=PetSchema)
# pass created resource into `path` for APISpec
spec.path(resource=random_pet_resource)
# should be passed twice so the suffix is registered
# path should be called n + 1 times where n is the number of suffixes
Comment on lines +99 to +100
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless people read this README, they are not going to know about this convention.

Is there a better way to register resources? Would prefer to do it once versus n+1 times. Also, is there is a reason it's n+1?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Unfortunately not, due to how the ApiSpec plugin works. It calls each resource once, meaning if you use a controller twice only one path will be chosen.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alysivji Any updates?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling something n+1 times is a sign of bad design. What if y'all add a new suffix but forgot to add the line? Having a manual process is a recipe for disaster.

There has to be a way of fixing it I would need to look into how the path_helper works... why can't we loop n+1 times inside there to get the information you need?

I have commitments until August 4th. I can definitely help out after then.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alysivji Status?

spec.path(resource=pet_resource)
spec.path(resource=pet_resource)
```

### Generated OpenAPI Spec
Expand All @@ -101,7 +123,30 @@ spec.to_dict()
# },
# "description": "A pet to be returned"
# }
# },
# }
# }
# },
# "/v1/client/{team}/{client_uuid:uuid}": {
# "get": {
# "description": "A cute furry animal endpoint.",
# "parameters": [
# {
# "in": "path",
# "name": "pet",
# "required": true,
# "schema": {
# "type": "string"
# }
# }
# ],
# "responses": {
# "200": {
# "schema": {
# "$ref": "#/definitions/Pet"
# },
# "description": "A pet to be returned"
# }
# }
# }
# }
# },
Expand Down Expand Up @@ -133,7 +178,7 @@ spec.to_dict()
# }
# }
# }
# },
# }
# }

spec.to_yaml()
Expand Down
30 changes: 23 additions & 7 deletions falcon_apispec/falcon_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,43 @@ class FalconPlugin(BasePlugin):
def __init__(self, app):
super(FalconPlugin, self).__init__()
self._app = app
self.uris_returned = []

@staticmethod
def _generate_resource_uri_mapping(app):
def _generate_resource_uri_mapping(self, app):
routes_to_check = copy.copy(app._router._roots)

if len(routes_to_check) == 0:
return {}

mapping = {}
for route in routes_to_check:
uri = route.uri_template
resource = route.resource
if route.uri_template is not None and route.uri_template not in self.uris_returned:
uri = route.uri_template
resource = route.resource
method_map = route.method_map
elif route.uri_template is None and len(route.children) != 0:
routes_to_check.extend(route.children)
continue
else:
for child in route.children:
if child.uri_template not in self.uris_returned:
uri = child.uri_template
resource = child.resource
method_map = child.method_map
break

mapping[resource] = {
"uri": uri,
"methods": {}
}

if route.method_map:
for method_name, method_handler in route.method_map.items():
if method_map:
for method_name, method_handler in method_map.items():
if method_handler.__dict__.get("__module__") == "falcon.responders":
continue
mapping[resource]["methods"][method_name.lower()] = method_handler

routes_to_check.extend(route.children)
self.uris_returned.append(uri)
return mapping

def path_helper(self, operations, resource, base_path=None, **kwargs):
Expand Down
42 changes: 42 additions & 0 deletions tests/falcon_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,48 @@ def on_get(self):

assert spec._paths["/hi"]["get"] == expected

def test_path_with_and_without_suffix(self, app, spec_factory):
class HelloResource:
def on_get_hello(self):
"""A greeting endpoint.
---
description: get a greeting
responses:
200:
description: said hello
"""
return "dummy"

def on_get(self):
"""A valid method.
---
description: this should pass
responses:
200:
description: said hi
"""
return "on get"

on_get_suffix_expected = {
"description": "get a greeting",
"responses": {"200": {"description": "said hello"}},
}

on_get_expected = {
"description": "this should pass",
"responses": {"200": {"description": "said hi"}},
}

hello_resource_with_suffix = HelloResource()
app.add_route("/hi", hello_resource_with_suffix)
app.add_route("/hi/{hello}", hello_resource_with_suffix, suffix="hello")
spec = spec_factory(app)
spec.path(resource=hello_resource_with_suffix)
spec.path(resource=hello_resource_with_suffix)

assert spec._paths["/hi/{hello}"]["get"] == on_get_suffix_expected
assert spec._paths["/hi"]["get"] == on_get_expected

def test_resource_without_endpoint(self, app, spec_factory):
class HelloResource:
def on_get(self, req, resp):
Expand Down