Skip to content

Commit

Permalink
Return HTTP 400 when attempting to post an event with an unregistered…
Browse files Browse the repository at this point in the history
… schema (#1463)

Co-authored-by: Zachary Sailer <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 4, 2024
1 parent d3a6c60 commit 432a9cc
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 21 deletions.
27 changes: 20 additions & 7 deletions jupyter_server/services/events/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,25 @@ def on_close(self):
self.event_logger.remove_listener(listener=self.event_listener)


def validate_model(data: dict[str, Any]) -> None:
"""Validates for required fields in the JSON request body"""
def validate_model(
data: dict[str, Any], registry: jupyter_events.schema_registry.SchemaRegistry
) -> None:
"""Validates for required fields in the JSON request body and verifies that
a registered schema/version exists"""
required_keys = {"schema_id", "version", "data"}
for key in required_keys:
if key not in data:
raise web.HTTPError(400, f"Missing `{key}` in the JSON request body.")
message = f"Missing `{key}` in the JSON request body."
raise Exception(message)
schema_id = cast(str, data.get("schema_id"))
# The case where a given schema_id isn't found,
# jupyter_events raises a useful error, so there's no need to
# handle that case here.
schema = registry.get(schema_id)
version = int(cast(int, data.get("version")))
if schema.version != version:
message = f"Unregistered version: {version}{schema.version} for `{schema_id}`"
raise Exception(message)


def get_timestamp(data: dict[str, Any]) -> Optional[datetime]:
Expand Down Expand Up @@ -111,18 +124,18 @@ async def post(self):
raise web.HTTPError(400, "No JSON data provided")

try:
validate_model(payload)
validate_model(payload, self.event_logger.schemas)
self.event_logger.emit(
schema_id=cast(str, payload.get("schema_id")),
data=cast("Dict[str, Any]", payload.get("data")),
timestamp_override=get_timestamp(payload),
)
self.set_status(204)
self.finish()
except web.HTTPError:
raise
except Exception as e:
raise web.HTTPError(500, str(e)) from e
# All known exceptions are raised by bad requests, e.g., bad
# version, unregistered schema, invalid emission data payload, etc.
raise web.HTTPError(400, str(e)) from e


default_handlers = [
Expand Down
32 changes: 18 additions & 14 deletions tests/services/events/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,17 @@ async def test_post_event(jp_fetch, event_logger_sink, payload):
}
"""


@pytest.mark.parametrize("payload", [payload_3, payload_4, payload_5, payload_6])
async def test_post_event_400(jp_fetch, event_logger, payload):
with pytest.raises(tornado.httpclient.HTTPClientError) as e:
await jp_fetch("api", "events", method="POST", body=payload)

assert expected_http_error(e, 400)


payload_7 = """\
{
"schema_id": "http://event.mock.jupyter.org/UNREGISTERED-SCHEMA",
"version": 1,
"data": {
"event_message": "Hello, world!"
}
}
"""

payload_8 = """\
{
"schema_id": "http://event.mock.jupyter.org/message",
"version": 1,
Expand All @@ -136,20 +137,23 @@ async def test_post_event_400(jp_fetch, event_logger, payload):
}
"""

payload_8 = """\
payload_9 = """\
{
"schema_id": "http://event.mock.jupyter.org/message",
"version": 2,
"data": {
"message": "Hello, world!"
"event_message": "Hello, world!"
}
}
"""


@pytest.mark.parametrize("payload", [payload_7, payload_8])
async def test_post_event_500(jp_fetch, event_logger, payload):
@pytest.mark.parametrize(
"payload",
[payload_3, payload_4, payload_5, payload_6, payload_7, payload_8, payload_9],
)
async def test_post_event_400(jp_fetch, event_logger, payload):
with pytest.raises(tornado.httpclient.HTTPClientError) as e:
await jp_fetch("api", "events", method="POST", body=payload)

assert expected_http_error(e, 500)
assert expected_http_error(e, 400)

0 comments on commit 432a9cc

Please sign in to comment.