From 15c4314e36358ef9a569ac0102808bdba5003586 Mon Sep 17 00:00:00 2001 From: Hans-Joachim Krauch Date: Fri, 15 Mar 2024 17:22:14 -0300 Subject: [PATCH 1/5] apply spec updates to c++ server implementation --- .../include/foxglove/websocket/common.hpp | 16 ++++- .../foxglove/websocket/serialization.hpp | 2 + cpp/foxglove-websocket/src/serialization.cpp | 65 +++++++++++++++++-- 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp b/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp index 29ae852d..e5c56a34 100644 --- a/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp +++ b/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp @@ -111,11 +111,23 @@ struct ClientMessage { } }; +struct ServiceRequestDefinition { + std::string encoding; + std::string schemaName; + std::string schemaEncoding; + std::string schema; +}; + +using ServiceResponseDefinition = ServiceRequestDefinition; + struct ServiceWithoutId { std::string name; std::string type; - std::string requestSchema; - std::string responseSchema; + std::optional request; + std::optional response; + + std::optional requestSchema; // Prefer request instead + std::optional responseSchema; // Prefer response instead }; struct Service : ServiceWithoutId { diff --git a/cpp/foxglove-websocket/include/foxglove/websocket/serialization.hpp b/cpp/foxglove-websocket/include/foxglove/websocket/serialization.hpp index 87893239..0712c7ac 100644 --- a/cpp/foxglove-websocket/include/foxglove/websocket/serialization.hpp +++ b/cpp/foxglove-websocket/include/foxglove/websocket/serialization.hpp @@ -52,5 +52,7 @@ void to_json(nlohmann::json& j, const Parameter& p); void from_json(const nlohmann::json& j, Parameter& p); void to_json(nlohmann::json& j, const Service& p); void from_json(const nlohmann::json& j, Service& p); +void to_json(nlohmann::json& j, const ServiceRequestDefinition& p); +void from_json(const nlohmann::json& j, ServiceRequestDefinition& p); } // namespace foxglove diff --git a/cpp/foxglove-websocket/src/serialization.cpp b/cpp/foxglove-websocket/src/serialization.cpp index 1058b278..1b78cee8 100644 --- a/cpp/foxglove-websocket/src/serialization.cpp +++ b/cpp/foxglove-websocket/src/serialization.cpp @@ -99,21 +99,78 @@ void from_json(const nlohmann::json& j, Parameter& p) { } void to_json(nlohmann::json& j, const Service& service) { + if (!service.request.has_value() && !service.requestSchema.has_value()) { + throw std::runtime_error("Invalid service definition: Either `request` or `requestSchema` must be defined"); + } + if (!service.response.has_value() && !service.responseSchema.has_value()) { + throw std::runtime_error("Invalid service definition: Either `response` or `responseSchema` must be defined"); + } + j = { {"id", service.id}, {"name", service.name}, {"type", service.type}, - {"requestSchema", service.requestSchema}, - {"responseSchema", service.responseSchema}, }; + + if (service.request.has_value()) { + j["request"] = service.request.value(); + } + if (service.response.has_value()) { + j["response"] = service.response.value(); + } + if (service.requestSchema.has_value()) { + j["requestSchema"] = service.requestSchema.value(); + } + if (service.responseSchema.has_value()) { + j["responseSchema"] = service.responseSchema.value(); + } } void from_json(const nlohmann::json& j, Service& p) { p.id = j["id"].get(); p.name = j["name"].get(); p.type = j["type"].get(); - p.requestSchema = j["requestSchema"].get(); - p.responseSchema = j["responseSchema"].get(); + +{ + const auto it = j.find("request"); + p.request = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + +} +{ + const auto it = j.find("response"); + p.response = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + +} +{ + const auto it = j.find("requestSchema"); + p.requestSchema = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + +} +{ + const auto it = j.find("responseSchema"); + p.responseSchema = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + +} +} + +void to_json(nlohmann::json& j, const ServiceRequestDefinition& r) { + j = { + {"encoding", r.encoding}, + {"schemaName", r.schemaName}, + {"schemaEncoding", r.schemaEncoding}, + {"schema", r.schema}, + }; +} + +void from_json(const nlohmann::json& j, ServiceRequestDefinition& r) { + r.encoding = j["encoding"].get(); + r.schemaName = j["schemaName"].get(); + r.schemaEncoding = j["schemaEncoding"].get(); + r.schema = j["schema"].get(); } void ServiceResponse::read(const uint8_t* data, size_t dataLength) { From 8177eba1e3b41e38961cd924c0ab4b375f182ccb Mon Sep 17 00:00:00 2001 From: Hans-Joachim Krauch Date: Fri, 15 Mar 2024 17:27:57 -0300 Subject: [PATCH 2/5] lint fixes --- .../include/foxglove/websocket/common.hpp | 2 +- cpp/foxglove-websocket/src/serialization.cpp | 52 +++++++++---------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp b/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp index e5c56a34..3af5801e 100644 --- a/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp +++ b/cpp/foxglove-websocket/include/foxglove/websocket/common.hpp @@ -126,7 +126,7 @@ struct ServiceWithoutId { std::optional request; std::optional response; - std::optional requestSchema; // Prefer request instead + std::optional requestSchema; // Prefer request instead std::optional responseSchema; // Prefer response instead }; diff --git a/cpp/foxglove-websocket/src/serialization.cpp b/cpp/foxglove-websocket/src/serialization.cpp index 1b78cee8..a213402f 100644 --- a/cpp/foxglove-websocket/src/serialization.cpp +++ b/cpp/foxglove-websocket/src/serialization.cpp @@ -100,10 +100,12 @@ void from_json(const nlohmann::json& j, Parameter& p) { void to_json(nlohmann::json& j, const Service& service) { if (!service.request.has_value() && !service.requestSchema.has_value()) { - throw std::runtime_error("Invalid service definition: Either `request` or `requestSchema` must be defined"); + throw std::runtime_error( + "Invalid service definition: Either `request` or `requestSchema` must be defined"); } if (!service.response.has_value() && !service.responseSchema.has_value()) { - throw std::runtime_error("Invalid service definition: Either `response` or `responseSchema` must be defined"); + throw std::runtime_error( + "Invalid service definition: Either `response` or `responseSchema` must be defined"); } j = { @@ -131,35 +133,31 @@ void from_json(const nlohmann::json& j, Service& p) { p.name = j["name"].get(); p.type = j["type"].get(); -{ - const auto it = j.find("request"); - p.request = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); - -} -{ - const auto it = j.find("response"); - p.response = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); - -} -{ - const auto it = j.find("requestSchema"); - p.requestSchema = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); - -} -{ - const auto it = j.find("responseSchema"); - p.responseSchema = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); - -} + { + const auto it = j.find("request"); + p.request = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + } + { + const auto it = j.find("response"); + p.response = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + } + { + const auto it = j.find("requestSchema"); + p.requestSchema = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + } + { + const auto it = j.find("responseSchema"); + p.responseSchema = it == j.end() ? std::optional(std::nullopt) + : std::make_optional(it->get()); + } } void to_json(nlohmann::json& j, const ServiceRequestDefinition& r) { j = { - {"encoding", r.encoding}, + {"encoding", r.encoding}, {"schemaName", r.schemaName}, {"schemaEncoding", r.schemaEncoding}, {"schema", r.schema}, From be45d634e71a72fd72cd5cd2b24646f8f52acabc Mon Sep 17 00:00:00 2001 From: Hans-Joachim Krauch Date: Mon, 18 Mar 2024 10:09:08 -0300 Subject: [PATCH 3/5] move service sanity check to server --- .../include/foxglove/websocket/websocket_server.hpp | 9 +++++++++ cpp/foxglove-websocket/src/serialization.cpp | 9 --------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cpp/foxglove-websocket/include/foxglove/websocket/websocket_server.hpp b/cpp/foxglove-websocket/include/foxglove/websocket/websocket_server.hpp index 959656ce..b1eb8169 100644 --- a/cpp/foxglove-websocket/include/foxglove/websocket/websocket_server.hpp +++ b/cpp/foxglove-websocket/include/foxglove/websocket/websocket_server.hpp @@ -906,6 +906,15 @@ inline std::vector Server::addServices( std::vector serviceIds; json newServices; for (const auto& service : services) { + if (!service.request.has_value() && !service.requestSchema.has_value()) { + throw std::runtime_error( + "Invalid service definition: Either `request` or `requestSchema` must be defined"); + } + if (!service.response.has_value() && !service.responseSchema.has_value()) { + throw std::runtime_error( + "Invalid service definition: Either `response` or `responseSchema` must be defined"); + } + const ServiceId serviceId = ++_nextServiceId; _services.emplace(serviceId, service); serviceIds.push_back(serviceId); diff --git a/cpp/foxglove-websocket/src/serialization.cpp b/cpp/foxglove-websocket/src/serialization.cpp index a213402f..aff5d492 100644 --- a/cpp/foxglove-websocket/src/serialization.cpp +++ b/cpp/foxglove-websocket/src/serialization.cpp @@ -99,15 +99,6 @@ void from_json(const nlohmann::json& j, Parameter& p) { } void to_json(nlohmann::json& j, const Service& service) { - if (!service.request.has_value() && !service.requestSchema.has_value()) { - throw std::runtime_error( - "Invalid service definition: Either `request` or `requestSchema` must be defined"); - } - if (!service.response.has_value() && !service.responseSchema.has_value()) { - throw std::runtime_error( - "Invalid service definition: Either `response` or `responseSchema` must be defined"); - } - j = { {"id", service.id}, {"name", service.name}, From 3d79d972af86dcd5823146e5269bd7736866277e Mon Sep 17 00:00:00 2001 From: Hans-Joachim Krauch Date: Mon, 18 Mar 2024 11:06:40 -0300 Subject: [PATCH 4/5] update python implementation --- .../examples/json_server.py | 46 ++++++++++++------- .../src/foxglove_websocket/server/__init__.py | 9 ++++ python/src/foxglove_websocket/types.py | 17 ++++++- python/tests/test_server.py | 44 +++++++++++------- 4 files changed, 81 insertions(+), 35 deletions(-) diff --git a/python/src/foxglove_websocket/examples/json_server.py b/python/src/foxglove_websocket/examples/json_server.py index 42f0244b..98baf026 100644 --- a/python/src/foxglove_websocket/examples/json_server.py +++ b/python/src/foxglove_websocket/examples/json_server.py @@ -87,23 +87,35 @@ async def on_service_request( await server.add_service( { "name": "example_set_bool", - "requestSchema": json.dumps( - { - "type": "object", - "properties": { - "data": {"type": "boolean"}, - }, - } - ), - "responseSchema": json.dumps( - { - "type": "object", - "properties": { - "success": {"type": "boolean"}, - "message": {"type": "string"}, - }, - } - ), + "request": { + "encoding": "json", + "schemaName": "requestSchema", + "schemaEncoding": "jsonschema", + "schema": json.dumps( + { + "type": "object", + "properties": { + "data": {"type": "boolean"}, + }, + } + ), + }, + "response": { + "encoding": "json", + "schemaName": "responseSchema", + "schemaEncoding": "jsonschema", + "schema": json.dumps( + { + "type": "object", + "properties": { + "success": {"type": "boolean"}, + "message": {"type": "string"}, + }, + } + ), + }, + "requestSchema": None, + "responseSchema": None, "type": "example_set_bool", } ) diff --git a/python/src/foxglove_websocket/server/__init__.py b/python/src/foxglove_websocket/server/__init__.py index f0c3219c..696359d2 100644 --- a/python/src/foxglove_websocket/server/__init__.py +++ b/python/src/foxglove_websocket/server/__init__.py @@ -271,6 +271,15 @@ async def remove_channel(self, chan_id: ChannelId): ) async def add_service(self, service: ServiceWithoutId) -> ServiceId: + if "request" not in service.keys() and "requestSchema" not in service.keys(): + raise ValueError( + f"Invalid service definition: Either 'request' or 'requestSchema' must be defined" + ) + if "response" not in service.keys() and "responseSchema" not in service.keys(): + raise ValueError( + f"Invalid service definition: Either 'response' or 'responseSchema' must be defined" + ) + new_id = self._next_service_id self._next_service_id = ServiceId(new_id + 1) new_service = Service(id=new_id, **service) diff --git a/python/src/foxglove_websocket/types.py b/python/src/foxglove_websocket/types.py index 4d01e901..7f1a96ca 100644 --- a/python/src/foxglove_websocket/types.py +++ b/python/src/foxglove_websocket/types.py @@ -142,11 +142,24 @@ class Channel(ChannelWithoutId): id: ChannelId +class ServiceRequestDefinition(TypedDict): + encoding: str + schemaName: str + schemaEncoding: str + schema: str + + +class ServiceResponseDefinition(ServiceRequestDefinition): + pass + + class ServiceWithoutId(TypedDict): name: str type: str - requestSchema: str - responseSchema: str + request: Optional[ServiceRequestDefinition] + response: Optional[ServiceResponseDefinition] + requestSchema: Optional[str] + responseSchema: Optional[str] class Service(ServiceWithoutId): diff --git a/python/tests/test_server.py b/python/tests/test_server.py index 296bae02..1dcb46d6 100644 --- a/python/tests/test_server.py +++ b/python/tests/test_server.py @@ -311,22 +311,34 @@ async def on_service_request( service: ServiceWithoutId = { "name": "set_bool", "type": "set_bool", - "requestSchema": json.dumps( - { - "type": "object", - "properties": { - "data": {"type": "boolean"}, - }, - } - ), - "responseSchema": json.dumps( - { - "type": "object", - "properties": { - "success": {"type": "boolean"}, - }, - } - ), + "request": { + "encoding": "json", + "schemaName": "requestSchema", + "schemaEncoding": "jsonschema", + "schema": json.dumps( + { + "type": "object", + "properties": { + "data": {"type": "boolean"}, + }, + } + ), + }, + "response": { + "encoding": "json", + "schemaName": "responseSchema", + "schemaEncoding": "jsonschema", + "schema": json.dumps( + { + "type": "object", + "properties": { + "success": {"type": "boolean"}, + }, + } + ), + }, + "requestSchema": None, + "responseSchema": None, } service_id = await server.add_service(service) From dc5f38146c2ad269dfc8fdb9c61d5fba31dd6669 Mon Sep 17 00:00:00 2001 From: Hans-Joachim Krauch Date: Wed, 20 Mar 2024 21:43:50 -0300 Subject: [PATCH 5/5] address comments --- cpp/foxglove-websocket/src/serialization.cpp | 43 +++++++++----------- python/src/foxglove_websocket/types.py | 4 +- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/cpp/foxglove-websocket/src/serialization.cpp b/cpp/foxglove-websocket/src/serialization.cpp index aff5d492..29d21b52 100644 --- a/cpp/foxglove-websocket/src/serialization.cpp +++ b/cpp/foxglove-websocket/src/serialization.cpp @@ -105,17 +105,17 @@ void to_json(nlohmann::json& j, const Service& service) { {"type", service.type}, }; - if (service.request.has_value()) { - j["request"] = service.request.value(); + if (service.request) { + j["request"] = *service.request; } - if (service.response.has_value()) { - j["response"] = service.response.value(); + if (service.response) { + j["response"] = *service.response; } - if (service.requestSchema.has_value()) { - j["requestSchema"] = service.requestSchema.value(); + if (service.requestSchema) { + j["requestSchema"] = *service.requestSchema; } - if (service.responseSchema.has_value()) { - j["responseSchema"] = service.responseSchema.value(); + if (service.responseSchema) { + j["responseSchema"] = *service.responseSchema; } } @@ -124,25 +124,20 @@ void from_json(const nlohmann::json& j, Service& p) { p.name = j["name"].get(); p.type = j["type"].get(); - { - const auto it = j.find("request"); - p.request = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); + if (const auto it = j.find("request"); it != j.end()) { + p.request = it->get(); } - { - const auto it = j.find("response"); - p.response = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); + + if (const auto it = j.find("response"); it != j.end()) { + p.response = it->get(); } - { - const auto it = j.find("requestSchema"); - p.requestSchema = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); + + if (const auto it = j.find("requestSchema"); it != j.end()) { + p.requestSchema = it->get(); } - { - const auto it = j.find("responseSchema"); - p.responseSchema = it == j.end() ? std::optional(std::nullopt) - : std::make_optional(it->get()); + + if (const auto it = j.find("responseSchema"); it != j.end()) { + p.responseSchema = it->get(); } } diff --git a/python/src/foxglove_websocket/types.py b/python/src/foxglove_websocket/types.py index 7f1a96ca..0f5cf683 100644 --- a/python/src/foxglove_websocket/types.py +++ b/python/src/foxglove_websocket/types.py @@ -158,8 +158,8 @@ class ServiceWithoutId(TypedDict): type: str request: Optional[ServiceRequestDefinition] response: Optional[ServiceResponseDefinition] - requestSchema: Optional[str] - responseSchema: Optional[str] + requestSchema: Optional[str] # Prefer request instead + responseSchema: Optional[str] # Prefer response instead class Service(ServiceWithoutId):