Skip to content
This repository has been archived by the owner on Feb 21, 2025. It is now read-only.

Commit

Permalink
fix: properly handle HEAD and OPTIONS
Browse files Browse the repository at this point in the history
  • Loading branch information
lsfratel committed Nov 25, 2024
1 parent 5f6c47e commit f15b2d4
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 89 deletions.
2 changes: 1 addition & 1 deletion src/restcraft/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.1.1"
__version__ = "0.1.2"
16 changes: 6 additions & 10 deletions src/restcraft/http/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,9 @@ def _setup_head_handler(self, node: Node):
return

handler = node.handlers["GET"]["handler"]
metadata = node.handlers["GET"]["metadata"]
metadata = node.handlers["GET"]["metadata"].copy()
metadata["methods"].append("HEAD")

if "OPTIONS" not in metadata["methods"]:
metadata["methods"].append("OPTIONS")

node.handlers["HEAD"] = {
"handler": handler,
"metadata": metadata,
Expand Down Expand Up @@ -137,12 +134,11 @@ def _match_part(self, node: Node, part: str, params: dict):
return None

def _register_view_handlers(self, node: Node, view: object):
for verb, meta in extract_metadata(view):
for method_name, method_meta in meta.items():
node.handlers[verb] = {
"handler": getattr(view, method_name),
"metadata": method_meta,
}
for verb, handler, metadata in extract_metadata(view):
node.handlers[verb] = {
"handler": handler,
"metadata": metadata.copy(),
}
node.view = view

def _merge_nodes(self, current_node: Node, other_node: Node):
Expand Down
4 changes: 2 additions & 2 deletions src/restcraft/utils/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def _get_funcs(cls: object, attr="__metadata__"):


def extract_metadata(cls: object):
for name, func in _get_funcs(cls):
for _, func in _get_funcs(cls):
func_metadata = getattr(func, "__metadata__", {}).copy()

http_verbs = func_metadata.get("methods", None)
Expand All @@ -19,4 +19,4 @@ def extract_metadata(cls: object):
continue

for verb in http_verbs:
yield verb, {name: func_metadata}
yield verb, func, func_metadata
139 changes: 63 additions & 76 deletions tests/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,23 @@
from restcraft.views import metadata


def test_router_add_route_find():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}


class AnotherDummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

@metadata(methods=["PUT"])
def put(self):
return {"message": "PUT response"}


def test_router_add_route_find():
router = Router()
router.add_route(r"/test", DummyView())

Expand All @@ -20,11 +31,6 @@ def get(self):


def test_router_dynamic_route():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

router = Router()
router.add_route(r"/users/<user_id:\d+>", DummyView())
node, params = router.find("/users/42")
Expand All @@ -37,16 +43,6 @@ def get(self):


def test_router_merge_static_and_dynamic_routes():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

class AnotherDummyView:
@metadata(methods=["PUT"])
def put(self):
return {"message": "PUT response"}

router = Router()

router.add_route("/static", DummyView())
Expand All @@ -65,16 +61,6 @@ def put(self):


def test_router_merge():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

class AnotherDummyView:
@metadata(methods=["PUT"])
def put(self):
return {"message": "PUT response"}

router1 = Router()
router2 = Router()

Expand Down Expand Up @@ -103,11 +89,6 @@ def wrapper(*args, **kwargs):

return wrapper

class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

router = Router()
router.add_route("/test", DummyView())

Expand All @@ -131,11 +112,6 @@ def test_router_not_found():


def test_router_method_not_allowed():
class AnotherDummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

router = Router()

router.add_route("/test", AnotherDummyView())
Expand All @@ -148,16 +124,6 @@ def get(self):


def test_router_conflicting_routes():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

class AnotherDummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

router = Router()

router.add_route("/test", DummyView())
Expand Down Expand Up @@ -226,11 +192,6 @@ def get(self):


def test_router_auto_generate_head_handler():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

router = Router()
router.add_route("/head_test", DummyView())
node, _ = router.find("/head_test")
Expand All @@ -241,15 +202,9 @@ def get(self):


def test_router_auto_generate_options_handler():
class MyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

view = MyView()
router = Router()

router.add_route("/options_test", view)
router.add_route("/options_test", DummyView())
node, _ = router.find("/options_test")

assert node is not None
Expand All @@ -275,24 +230,56 @@ def get(self):


def test_router_conflicting_dynamic_routes():
class DummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

class AnotherDummyView:
@metadata(methods=["GET"])
def get(self):
return {"message": "GET response"}

@metadata(methods=["PUT"])
def put(self):
return {"message": "PUT response"}

router = Router()

router.add_route("/test/<id>", DummyView())
try:
router.add_route("/test/<name>", AnotherDummyView())
except ValueError as e:
assert str(e) == "Conflicting views found during merge"


def test_router_auto_generate_head_and_options_metadata():
router = Router()
router.add_route("/test", DummyView())
node, _ = router.find("/test")

assert node is not None
assert "HEAD" in node.handlers
assert "OPTIONS" in node.handlers

head_metadata = node.handlers["HEAD"]["metadata"]
get_metadata = node.handlers["GET"]["metadata"]

options_metadata = node.handlers["OPTIONS"]["metadata"]
methods = list(node.handlers.keys())

assert head_metadata == {**get_metadata, "methods": ["GET", "HEAD"]}
assert options_metadata == {"methods": methods, "plugins": ["..."]}


def test_router_head_and_options_isolation():
router = Router()
router.add_route("/test", DummyView())

node, _ = router.find("/test")
assert node is not None

head_handler = node.handlers["HEAD"]["handler"]
options_handler = node.handlers["OPTIONS"]["handler"]

assert head_handler is node.handlers["GET"]["handler"]
assert options_handler == router._handler_options


def test_router_dynamic_route_with_head_and_options():
router = Router()
router.add_route("/users/<user_id>", DummyView())
node, _ = router.find("/users/42")

assert node is not None
assert "HEAD" in node.handlers
assert "OPTIONS" in node.handlers
assert node.handlers["HEAD"]["handler"]() == {"message": "GET response"}
response = node.handlers["OPTIONS"]["handler"]()
assert response.status == "204 No Content"

0 comments on commit f15b2d4

Please sign in to comment.