From 7595c7daa6213ba73a116550039e0b3cbca1e567 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Wed, 31 Jul 2024 11:40:02 +0200 Subject: [PATCH 01/12] Server refactoring + bug fixes. --- src/brayns/core/Launcher.cpp | 3 +- src/brayns/core/jsonrpc/Errors.cpp | 5 + src/brayns/core/jsonrpc/Errors.h | 1 + src/brayns/core/jsonrpc/Messages.h | 8 +- src/brayns/core/jsonrpc/Parser.cpp | 25 ++-- src/brayns/core/jsonrpc/Parser.h | 5 +- src/brayns/core/service/Service.cpp | 125 ++++++++---------- src/brayns/core/service/Service.h | 8 +- src/brayns/core/websocket/RequestQueue.cpp | 4 +- src/brayns/core/websocket/RequestQueue.h | 17 ++- .../core/websocket/WebSocketHandler.cpp | 53 ++++---- src/brayns/core/websocket/WebSocketHandler.h | 7 +- src/brayns/core/websocket/WebSocketServer.cpp | 74 +++++------ src/brayns/core/websocket/WebSocketServer.h | 2 +- 14 files changed, 166 insertions(+), 171 deletions(-) diff --git a/src/brayns/core/Launcher.cpp b/src/brayns/core/Launcher.cpp index a577e1a04..ba34b2f1e 100644 --- a/src/brayns/core/Launcher.cpp +++ b/src/brayns/core/Launcher.cpp @@ -28,6 +28,7 @@ #include #include #include +#include namespace { @@ -95,7 +96,7 @@ void startServerAndRunService(const ServiceSettings &settings, Logger &logger) logger.info("Websocket server started"); logger.info("Service running"); - runService(server, api, token, logger); + runService([&] { return server.waitForRequests(); }, api, token, logger); } } diff --git a/src/brayns/core/jsonrpc/Errors.cpp b/src/brayns/core/jsonrpc/Errors.cpp index 6b2b4bd24..ee8197e00 100644 --- a/src/brayns/core/jsonrpc/Errors.cpp +++ b/src/brayns/core/jsonrpc/Errors.cpp @@ -91,4 +91,9 @@ InternalError::InternalError(const std::string &message): JsonRpcException(-32603, message) { } + +InternalError::InternalError(const std::exception &e): + JsonRpcException(-32603, e.what()) +{ +} } diff --git a/src/brayns/core/jsonrpc/Errors.h b/src/brayns/core/jsonrpc/Errors.h index b6727611b..44f4c4e20 100644 --- a/src/brayns/core/jsonrpc/Errors.h +++ b/src/brayns/core/jsonrpc/Errors.h @@ -70,5 +70,6 @@ class InternalError : public JsonRpcException { public: explicit InternalError(const std::string &message); + explicit InternalError(const std::exception &e); }; } diff --git a/src/brayns/core/jsonrpc/Messages.h b/src/brayns/core/jsonrpc/Messages.h index 75911b3c0..b675b9484 100644 --- a/src/brayns/core/jsonrpc/Messages.h +++ b/src/brayns/core/jsonrpc/Messages.h @@ -50,10 +50,10 @@ inline std::string toString(const std::optional &id) struct JsonRpcRequest { - std::optional id; std::string method; JsonValue params; std::string binary = {}; + std::optional id = {}; }; template<> @@ -63,9 +63,9 @@ struct JsonObjectReflector { auto builder = JsonBuilder(); builder.constant("jsonrpc", "2.0"); - builder.field("id", [](auto &object) { return &object.id; }); builder.field("method", [](auto &object) { return &object.method; }); builder.field("params", [](auto &object) { return &object.params; }).required(false); + builder.field("id", [](auto &object) { return &object.id; }); return builder.build(); } }; @@ -112,8 +112,8 @@ struct JsonObjectReflector struct JsonRpcErrorResponse { - std::optional id; JsonRpcError error; + std::optional id = {}; }; template<> @@ -123,8 +123,8 @@ struct JsonObjectReflector { auto builder = JsonBuilder(); builder.constant("jsonrpc", "2.0"); - builder.field("id", [](auto &object) { return &object.id; }); builder.field("error", [](auto &object) { return &object.error; }); + builder.field("id", [](auto &object) { return &object.id; }); return builder.build(); } }; diff --git a/src/brayns/core/jsonrpc/Parser.cpp b/src/brayns/core/jsonrpc/Parser.cpp index 2f1a227d3..e388ece8f 100644 --- a/src/brayns/core/jsonrpc/Parser.cpp +++ b/src/brayns/core/jsonrpc/Parser.cpp @@ -85,6 +85,11 @@ std::string composeAsText(const JsonRpcSuccessResponse &response) return stringifyToJson(response); } +std::string composeAsText(const JsonRpcErrorResponse &response) +{ + return stringifyToJson(response); +} + std::string composeAsBinary(const JsonRpcSuccessResponse &response) { auto text = composeAsText(response); @@ -101,21 +106,17 @@ std::string composeAsBinary(const JsonRpcSuccessResponse &response) return header + text + response.binary; } -std::string composeError(const JsonRpcErrorResponse &response) +JsonRpcError composeError(const JsonRpcException &e) { - return stringifyToJson(response); + return { + .code = e.getCode(), + .message = e.what(), + .data = e.getData(), + }; } -std::string composeError(const std::optional &id, const JsonRpcException &e) +JsonRpcErrorResponse composeErrorResponse(const JsonRpcException &e, std::optional id) { - return composeError({ - .id = id, - .error = - { - .code = e.getCode(), - .message = e.what(), - .data = e.getData(), - }, - }); + return {composeError(e), std::move(id)}; } } diff --git a/src/brayns/core/jsonrpc/Parser.h b/src/brayns/core/jsonrpc/Parser.h index 9bb33cac7..f55f8c9b0 100644 --- a/src/brayns/core/jsonrpc/Parser.h +++ b/src/brayns/core/jsonrpc/Parser.h @@ -31,7 +31,8 @@ namespace brayns JsonRpcRequest parseTextJsonRpcRequest(const std::string &text); JsonRpcRequest parseBinaryJsonRpcRequest(const std::string &binary); std::string composeAsText(const JsonRpcSuccessResponse &response); +std::string composeAsText(const JsonRpcErrorResponse &response); std::string composeAsBinary(const JsonRpcSuccessResponse &response); -std::string composeError(const JsonRpcErrorResponse &response); -std::string composeError(const std::optional &id, const JsonRpcException &e); +JsonRpcError composeError(const JsonRpcException &e); +JsonRpcErrorResponse composeErrorResponse(const JsonRpcException &e, std::optional id = std::nullopt); } diff --git a/src/brayns/core/service/Service.cpp b/src/brayns/core/service/Service.cpp index 93664b60d..d25f5eb4a 100644 --- a/src/brayns/core/service/Service.cpp +++ b/src/brayns/core/service/Service.cpp @@ -29,133 +29,111 @@ namespace { using namespace brayns; -struct ResponseData +JsonRpcRequest parseRequest(const Message &message) { - std::string data; - bool binary = false; -}; - -JsonRpcRequest parseRequest(const RawRequest &request) -{ - if (request.binary) - { - return parseBinaryJsonRpcRequest(request.data); - } - - return parseTextJsonRpcRequest(request.data); -} - -ResponseData composeResponse(const JsonRpcId &id, RawResult result) -{ - if (result.binary.empty()) + if (message.binary) { - auto data = composeAsText({id, result.json}); - return {.data = std::move(data), .binary = false}; + return parseBinaryJsonRpcRequest(message.data); } - auto data = composeAsBinary({id, result.json, std::move(result.binary)}); - return {.data = std::move(data), .binary = true}; + return parseTextJsonRpcRequest(message.data); } -void sendExecutionError( - const std::optional &id, - const JsonRpcException &e, - const ResponseHandler &respond, - Logger &logger) +Message composeResponse(const JsonRpcSuccessResponse &response) { - if (!id) + if (response.binary.empty()) { - logger.info("No ID in request, skipping error response"); - return; + return {.data = composeAsText(response), .binary = false}; } - logger.info("Composing execution error"); - auto text = composeError(*id, e); - logger.info("Execution error composed"); - - logger.info("Sending execution error"); - respond({std::move(text)}); - logger.info("Execution error sent"); + return {.data = composeAsBinary(response), .binary = true}; } -void sendParsingError(const JsonRpcException &e, const ResponseHandler &respond, Logger &logger) +Message composeResponse(const JsonRpcErrorResponse &response) { - logger.info("Composing parsing error"); - auto text = composeError(std::nullopt, e); - logger.info("Parsing error composed"); - - logger.info("Sending parsing error"); - respond({std::move(text)}); - logger.info("Parsing error sent"); + return {.data = composeAsText(response), .binary = false}; } -void executeRequest(JsonRpcRequest request, const ResponseHandler &respond, Api &api, Logger &logger) +std::optional execute(JsonRpcRequest request, Api &api, Logger &logger) { try { - auto params = RawParams{std::move(request.params), std::move(request.binary)}; - logger.info("Calling endpoint for request {}", toString(request.id)); + auto params = RawParams{std::move(request.params), std::move(request.binary)}; auto result = api.execute(request.method, std::move(params)); logger.info("Successfully called endpoint"); if (!request.id) { logger.info("No ID in request, skipping response"); - return; + return {}; } - logger.info("Composing response"); - auto response = composeResponse(*request.id, std::move(result)); - logger.info("Successfully composed response"); - - logger.info("Sending response"); - respond({response.data, response.binary}); - logger.info("Response sent"); + return JsonRpcSuccessResponse{*request.id, std::move(result)}; } catch (const JsonRpcException &e) { logger.warn("Error during request execution: '{}'", e.what()); - sendExecutionError(request.id, e, respond, logger); + return composeErrorResponse(e, *request.id); } catch (const std::exception &e) { logger.error("Unexpected error during request execution: '{}'", e.what()); - sendExecutionError(request.id, InternalError(e.what()), respond, logger); + return composeErrorResponse(InternalError(e), *request.id); } catch (...) { logger.error("Unknown error during request execution"); - sendExecutionError(request.id, InternalError("Unknown execution error"), respond, logger); + return composeErrorResponse(InternalError("Unknown execution error"), *request.id); } } -void handleRequest(const RawRequest &request, Api &api, Logger &logger) +std::optional parseAndExecute(const Message &message, Api &api, Logger &logger) { try { - logger.info("Parsing JSON-RPC request from client {}", request.clientId); - auto jsonRpcRequest = parseRequest(request); + logger.info("Parsing request"); + auto request = parseRequest(message); logger.info("Successfully parsed request"); - executeRequest(std::move(jsonRpcRequest), request.respond, api, logger); + return execute(std::move(request), api, logger); } catch (const JsonRpcException &e) { logger.warn("Error while parsing request: '{}'", e.what()); - sendParsingError(e, request.respond, logger); + return composeErrorResponse(e); } catch (const std::exception &e) { logger.error("Unexpected error while parsing request: '{}'", e.what()); - sendParsingError(InternalError(e.what()), request.respond, logger); + return composeErrorResponse(InternalError(e)); } catch (...) { logger.error("Unknown error while parsing request"); - sendParsingError(InternalError("Unknown parsing error"), request.respond, logger); + return composeErrorResponse(InternalError("Unknown parsing error")); } } + +void handleRequest(const Request &request, Api &api, Logger &logger) +{ + logger.info("Handling JSON-RPC request from client {}", request.clientId); + + auto response = parseAndExecute(request.message, api, logger); + + if (!response) + { + return; + } + + logger.info("Composing response"); + auto message = std::visit([](const auto &value) { return composeResponse(value); }, *response); + logger.info("Successfully composed response"); + + logger.info("Sending response"); + request.respond(message); + logger.info("Response sent"); +} } namespace brayns @@ -170,17 +148,24 @@ void StopToken::stop() _stopped = true; } -void runService(WebSocketServer &server, Api &api, StopToken &token, Logger &logger) +void runService(RequestProvider waitForRequests, Api &api, StopToken &token, Logger &logger) { while (true) { logger.info("Waiting for incoming requests"); - auto requests = server.waitForRequests(); - + auto requests = waitForRequests(); logger.info("Received {} requests", requests.size()); + for (const auto &request : requests) { - handleRequest(request, api, logger); + try + { + handleRequest(request, api, logger); + } + catch (...) + { + logger.error("Unknown error while handling request"); + } if (token.isStopped()) { diff --git a/src/brayns/core/service/Service.h b/src/brayns/core/service/Service.h index fe1b78f97..1482c26a8 100644 --- a/src/brayns/core/service/Service.h +++ b/src/brayns/core/service/Service.h @@ -21,9 +21,11 @@ #pragma once +#include + #include #include -#include +#include namespace brayns { @@ -37,5 +39,7 @@ class StopToken bool _stopped = false; }; -void runService(WebSocketServer &server, Api &api, StopToken &token, Logger &logger); +using RequestProvider = std::function()>; + +void runService(RequestProvider waitForRequests, Api &api, StopToken &token, Logger &logger); } diff --git a/src/brayns/core/websocket/RequestQueue.cpp b/src/brayns/core/websocket/RequestQueue.cpp index 78ffaaf15..b1f5878f9 100644 --- a/src/brayns/core/websocket/RequestQueue.cpp +++ b/src/brayns/core/websocket/RequestQueue.cpp @@ -25,7 +25,7 @@ namespace brayns { -void RequestQueue::push(RawRequest request) +void RequestQueue::push(Request request) { auto lock = std::lock_guard(_mutex); @@ -33,7 +33,7 @@ void RequestQueue::push(RawRequest request) _condition.notify_all(); } -std::vector RequestQueue::wait() +std::vector RequestQueue::wait() { auto lock = std::unique_lock(_mutex); diff --git a/src/brayns/core/websocket/RequestQueue.h b/src/brayns/core/websocket/RequestQueue.h index 1fb52792a..53abf1be0 100644 --- a/src/brayns/core/websocket/RequestQueue.h +++ b/src/brayns/core/websocket/RequestQueue.h @@ -30,31 +30,30 @@ namespace brayns { using ClientId = std::uint32_t; -struct RawResponse +struct Message { - std::string_view data; + std::string data; bool binary = false; }; -using ResponseHandler = std::function; +using ResponseHandler = std::function; -struct RawRequest +struct Request { ClientId clientId; - std::string data; - bool binary = false; + Message message; ResponseHandler respond; }; class RequestQueue { public: - void push(RawRequest request); - std::vector wait(); + void push(Request request); + std::vector wait(); private: std::mutex _mutex; std::condition_variable _condition; - std::vector _requests; + std::vector _requests; }; } diff --git a/src/brayns/core/websocket/WebSocketHandler.cpp b/src/brayns/core/websocket/WebSocketHandler.cpp index be7ca34b1..a5f09b862 100644 --- a/src/brayns/core/websocket/WebSocketHandler.cpp +++ b/src/brayns/core/websocket/WebSocketHandler.cpp @@ -30,13 +30,7 @@ namespace { using namespace brayns; -struct WebSocketBuffer -{ - std::string data; - bool binary = false; -}; - -void onContinuation(const WebSocketFrame &frame, WebSocketBuffer &buffer, Logger &logger) +void onContinuation(const WebSocketFrame &frame, Message &buffer, Logger &logger) { logger.info("Continuation frame received (binary = {})", buffer.binary); @@ -48,7 +42,7 @@ void onContinuation(const WebSocketFrame &frame, WebSocketBuffer &buffer, Logger buffer.data.append(frame.data); } -void onText(const WebSocketFrame &frame, WebSocketBuffer &buffer, Logger &logger) +void onText(const WebSocketFrame &frame, Message &buffer, Logger &logger) { logger.info("Text frame received"); @@ -61,7 +55,7 @@ void onText(const WebSocketFrame &frame, WebSocketBuffer &buffer, Logger &logger buffer.binary = false; } -void onBinary(const WebSocketFrame &frame, WebSocketBuffer &buffer, Logger &logger) +void onBinary(const WebSocketFrame &frame, Message &buffer, Logger &logger) { logger.info("Binary frame received"); @@ -93,9 +87,9 @@ void onPong(Logger &logger) logger.info("Pong frame received, ignoring"); } -void respond(ClientId clientId, WebSocket &websocket, Logger &logger, const RawResponse &response) +void respond(ClientId clientId, WebSocket &websocket, Logger &logger, const Message &response) { - auto data = response.data; + auto data = std::string_view(response.data); logger.info("Sending response of {} bytes to client {}", data.size(), clientId); @@ -141,7 +135,7 @@ void respond(ClientId clientId, WebSocket &websocket, Logger &logger, const RawR void runClientLoop(ClientId clientId, WebSocket &websocket, RequestQueue &requests, Logger &logger) { - auto buffer = WebSocketBuffer(); + auto buffer = Message(); while (true) { @@ -177,23 +171,34 @@ void runClientLoop(ClientId clientId, WebSocket &websocket, RequestQueue &reques continue; } - auto request = RawRequest{ - .clientId = clientId, - .data = std::exchange(buffer.data, {}), - .binary = buffer.binary, - .respond = [=, &logger](const auto &response) mutable { respond(clientId, websocket, logger, response); }, - }; - - logger.info("Received request of {} bytes from client {}", request.data.size(), clientId); + logger.info("Received request of {} bytes from client {}", buffer.data.size(), clientId); - if (!request.binary) + if (!buffer.binary) { - logger.debug("Text request data: {}", request.data); + logger.debug("Text request data: {}", buffer.data); } + auto request = Request{ + .clientId = clientId, + .message = std::exchange(buffer, {}), + .respond = [=, &logger](const auto &response) mutable { respond(clientId, websocket, logger, response); }, + }; + requests.push(std::move(request)); } } + +ClientId generateClientId(std::mutex &mutex, IdGenerator &ids) +{ + auto lock = std::lock_guard(mutex); + return ids.next(); +} + +void recycleClientId(std::mutex &mutex, IdGenerator &ids, ClientId id) +{ + auto lock = std::lock_guard(mutex); + return ids.recycle(id); +} } namespace brayns @@ -206,7 +211,7 @@ WebSocketHandler::WebSocketHandler(RequestQueue &requests, Logger &logger): void WebSocketHandler::handle(WebSocket &websocket) { - auto clientId = _clientIds.next(); + auto clientId = generateClientId(_mutex, _ids); try { @@ -228,7 +233,7 @@ void WebSocketHandler::handle(WebSocket &websocket) websocket.close(WebSocketStatus::UnexpectedCondition, "Internal error"); } - _clientIds.recycle(clientId); + recycleClientId(_mutex, _ids, clientId); _logger->info("Client {} disconnected", clientId); } diff --git a/src/brayns/core/websocket/WebSocketHandler.h b/src/brayns/core/websocket/WebSocketHandler.h index d267e1d28..d99ecc69f 100644 --- a/src/brayns/core/websocket/WebSocketHandler.h +++ b/src/brayns/core/websocket/WebSocketHandler.h @@ -21,9 +21,7 @@ #pragma once -#include -#include -#include +#include #include #include @@ -43,6 +41,7 @@ class WebSocketHandler private: RequestQueue *_requests; Logger *_logger; - IdGenerator _clientIds; + std::mutex _mutex; + IdGenerator _ids; }; } diff --git a/src/brayns/core/websocket/WebSocketServer.cpp b/src/brayns/core/websocket/WebSocketServer.cpp index b69421c7f..7fc7ab547 100644 --- a/src/brayns/core/websocket/WebSocketServer.cpp +++ b/src/brayns/core/websocket/WebSocketServer.cpp @@ -66,8 +66,8 @@ class HealthcheckRequestHandler : public Poco::Net::HTTPRequestHandler class WebSocketRequestHandler : public Poco::Net::HTTPRequestHandler { public: - explicit WebSocketRequestHandler(WebSocketHandler handler, std::size_t maxFrameSize, Logger &logger): - _handler(std::move(handler)), + explicit WebSocketRequestHandler(WebSocketHandler &handler, std::size_t maxFrameSize, Logger &logger): + _handler(&handler), _maxFrameSize(maxFrameSize), _logger(&logger) { @@ -77,42 +77,13 @@ class WebSocketRequestHandler : public Poco::Net::HTTPRequestHandler { _logger->info("Upgrading to websocket"); - auto websocket = _tryUpgrade(request, response); - - if (!websocket) - { - return; - } - - _logger->info("Upgrade complete, host {} is now connected", request.getHost()); - try { - _handler.handle(*websocket); - } - catch (...) - { - _logger->error("Unexpected error in websocket request handler"); - websocket->close(WebSocketStatus::UnexpectedCondition, "Internal error"); - } - } + auto websocket = upgrade(request, response); -private: - WebSocketHandler _handler; - std::size_t _maxFrameSize; - Logger *_logger; - - std::optional _tryUpgrade(Poco::Net::HTTPServerRequest &request, Poco::Net::HTTPServerResponse &response) - { - try - { - auto websocket = Poco::Net::WebSocket(request, response); + _logger->info("Upgrade complete, host {} is now connected", request.getHost()); - auto maxFrameSize = static_cast(_maxFrameSize); - - websocket.setMaxPayloadSize(maxFrameSize); - - return WebSocket(websocket); + handle(websocket); } catch (const Poco::Net::WebSocketException &e) { @@ -130,15 +101,38 @@ class WebSocketRequestHandler : public Poco::Net::HTTPRequestHandler response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_INTERNAL_SERVER_ERROR); response.send() << "Internal error during websocket handshake"; } + } + +private: + WebSocketHandler *_handler; + std::size_t _maxFrameSize; + Logger *_logger; + + WebSocket upgrade(Poco::Net::HTTPServerRequest &request, Poco::Net::HTTPServerResponse &response) + { + auto webocket = Poco::Net::WebSocket(request, response); + webocket.setMaxPayloadSize(static_cast(_maxFrameSize)); + return WebSocket(webocket); + } - return std::nullopt; + void handle(WebSocket &websocket) + { + try + { + _handler->handle(websocket); + } + catch (...) + { + _logger->error("Unexpected error in websocket request handler"); + websocket.close(WebSocketStatus::UnexpectedCondition, "Internal error"); + } } }; class RequestHandlerFactory : public Poco::Net::HTTPRequestHandlerFactory { public: - explicit RequestHandlerFactory(WebSocketHandler handler, std::size_t maxFrameSize, Logger &logger): + explicit RequestHandlerFactory(std::unique_ptr handler, std::size_t maxFrameSize, Logger &logger): _handler(std::move(handler)), _maxFrameSize(maxFrameSize), _logger(&logger) @@ -159,7 +153,7 @@ class RequestHandlerFactory : public Poco::Net::HTTPRequestHandlerFactory if (uri == "/") { - return new WebSocketRequestHandler(_handler, _maxFrameSize, *_logger); + return new WebSocketRequestHandler(*_handler, _maxFrameSize, *_logger); } _logger->error("No request handlers for uri '{}'", uri); @@ -168,7 +162,7 @@ class RequestHandlerFactory : public Poco::Net::HTTPRequestHandlerFactory } private: - WebSocketHandler _handler; + std::unique_ptr _handler; std::size_t _maxFrameSize; Logger *_logger; }; @@ -235,7 +229,7 @@ WebSocketServer::~WebSocketServer() } } -std::vector WebSocketServer::waitForRequests() +std::vector WebSocketServer::waitForRequests() { return _requests->wait(); } @@ -250,7 +244,7 @@ WebSocketServer startServer(const WebSocketServerSettings &settings, Logger &log try { auto requests = std::make_unique(); - auto handler = WebSocketHandler(*requests, logger); + auto handler = std::make_unique(*requests, logger); auto factory = Poco::makeShared(std::move(handler), settings.maxFrameSize, logger); auto socket = createServerSocket(settings); auto params = extractServerParams(settings); diff --git a/src/brayns/core/websocket/WebSocketServer.h b/src/brayns/core/websocket/WebSocketServer.h index af53751a6..298403cc8 100644 --- a/src/brayns/core/websocket/WebSocketServer.h +++ b/src/brayns/core/websocket/WebSocketServer.h @@ -63,7 +63,7 @@ class WebSocketServer WebSocketServer &operator=(const WebSocketServer &) = delete; WebSocketServer &operator=(WebSocketServer &&) = default; - std::vector waitForRequests(); + std::vector waitForRequests(); private: std::unique_ptr _server; From f340c9a5cf56773e5e4348efbeada9dbf1eff0d0 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Wed, 31 Jul 2024 15:49:42 +0200 Subject: [PATCH 02/12] Fixed connection bug. --- python/README.md | 2 - python/brayns/__init__.py | 2 + python/brayns/network/connection.py | 22 ++++--- python/brayns/network/websocket.py | 2 +- python/brayns/utils/logger.py | 36 +++++++++++ python/tests/integration/conftest.py | 40 ++----------- src/brayns/core/endpoints/ObjectEndpoints.cpp | 51 +++------------- src/brayns/core/objects/ObjectManager.cpp | 60 +------------------ src/brayns/core/objects/ObjectManager.h | 22 +++---- src/brayns/core/service/Service.cpp | 3 +- src/brayns/core/websocket/WebSocketServer.cpp | 21 +++++-- tests/core/objects/TestObjectManager.cpp | 23 +++---- 12 files changed, 104 insertions(+), 180 deletions(-) create mode 100644 python/brayns/utils/logger.py diff --git a/python/README.md b/python/README.md index af1aeac35..3bb828414 100644 --- a/python/README.md +++ b/python/README.md @@ -81,8 +81,6 @@ pip install -r requirements-dev.txt ```bash BRAYNS_HOST=localhost BRAYNS_PORT=5000 -BRAYNS_EXECUTABLE=path/to/braynsService -LD_LIBRARY_PATH=path/to/additional/libs ``` Note: integration testing can be disable using the pytest --without-integration flag. diff --git a/python/brayns/__init__.py b/python/brayns/__init__.py index 5c7b9e971..54fc77b73 100644 --- a/python/brayns/__init__.py +++ b/python/brayns/__init__.py @@ -50,6 +50,7 @@ ) from .network.websocket import ServiceUnavailable, WebSocketError from .version import VERSION +from .utils.logger import create_logger __version__ = VERSION """Version tag of brayns Python package (major.minor.patch).""" @@ -58,6 +59,7 @@ "cancel_task", "connect", "Connection", + "create_logger", "Endpoint", "FutureResponse", "get_endpoint", diff --git a/python/brayns/network/connection.py b/python/brayns/network/connection.py index 42b468229..f78573b7e 100644 --- a/python/brayns/network/connection.py +++ b/python/brayns/network/connection.py @@ -19,10 +19,12 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import asyncio -from logging import Logger +from logging import WARNING, Logger from ssl import SSLContext from typing import Any, NamedTuple, Self +from brayns.utils.logger import create_logger + from .json_rpc import ( JsonRpcError, JsonRpcErrorResponse, @@ -110,6 +112,9 @@ def done(self) -> bool: return self._buffer.is_done(self._request_id) async def poll(self) -> None: + if self._request_id is None: + raise ValueError("Cannot poll requests without ID") + if self.done: return @@ -203,7 +208,7 @@ async def connect( logger: Logger | None = None, ) -> Connection: if logger is None: - logger = Logger("Brayns") + logger = create_logger(WARNING) protocol = "ws" if ssl is None else "wss" url = f"{protocol}://{host}:{port}" @@ -214,15 +219,16 @@ async def connect( try: logger.info("Connection attempt %d", attempt) websocket = await connect_websocket(url, ssl, max_frame_size, logger) - break + logger.info("Connection suceeded") + + return Connection(websocket) except ServiceUnavailable as e: - logger.warning("Connection attempt failed: %s", e) + logger.warning("Connection attempt %d failed: %s", attempt, e) + + attempt += 1 + if max_attempts is not None and attempt >= max_attempts: logger.warning("Max connection attempts reached, aborted") raise await asyncio.sleep(sleep_between_attempts) - - logger.info("Connection suceeded") - - return Connection(websocket) diff --git a/python/brayns/network/websocket.py b/python/brayns/network/websocket.py index bf80f37c0..ad1a99d5f 100644 --- a/python/brayns/network/websocket.py +++ b/python/brayns/network/websocket.py @@ -127,7 +127,7 @@ async def connect_websocket(url: str, ssl: SSLContext | None, max_frame_size: in logger.warning("Connection failed: %s", e) raise WebSocketError(str(e)) except OSError as e: - logger.warning("Service not found (probably not ready): %s", e) + logger.warning("Service not found (maybe not ready): %s", e) raise ServiceUnavailable(str(e)) wrapper = _WebSocket(websocket, max_frame_size, logger) diff --git a/python/brayns/utils/logger.py b/python/brayns/utils/logger.py new file mode 100644 index 000000000..c65c14964 --- /dev/null +++ b/python/brayns/utils/logger.py @@ -0,0 +1,36 @@ +# Copyright (c) 2015-2024 EPFL/Blue Brain Project +# All rights reserved. Do not distribute without permission. +# +# Responsible Author: adrien.fleury@epfl.ch +# +# This file is part of Brayns +# +# This library is free software; you can redistribute it and/or modify it under +# the terms of the GNU Lesser General Public License version 3.0 as published +# by the Free Software Foundation. +# +# This library is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more +# details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +from logging import INFO, Formatter, Logger, StreamHandler +import sys + + +def create_logger(level: int | str = INFO) -> Logger: + logger = Logger("Brayns", level) + + format = "[%(asctime)s][%(name)s][%(levelname)s] %(message)s" + formatter = Formatter(format) + + handler = StreamHandler(sys.stdout) + handler.setFormatter(formatter) + + logger.addHandler(handler) + + return logger diff --git a/python/tests/integration/conftest.py b/python/tests/integration/conftest.py index 574ab1a07..32dc744ca 100644 --- a/python/tests/integration/conftest.py +++ b/python/tests/integration/conftest.py @@ -18,52 +18,24 @@ # along with this library; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +from logging import DEBUG import os -from collections.abc import AsyncIterator, Iterator -from subprocess import PIPE, STDOUT, Popen +from collections.abc import AsyncIterator -import pytest import pytest_asyncio -from brayns import Connection, connect +from brayns import Connection, connect, create_logger HOST = os.getenv("BRAYNS_HOST", "localhost") PORT = int(os.getenv("BRAYNS_PORT", "5000")) -EXECUTABLE = os.getenv("BRAYNS_EXECUTABLE", "") - - -def start_service() -> Popen[str]: - return Popen( - args=[ - EXECUTABLE, - "--host", - HOST, - "--port", - str(PORT), - ], - stdin=PIPE, - stdout=PIPE, - stderr=STDOUT, - text=True, - ) async def connect_to_service() -> Connection: - return await connect(HOST, PORT, max_attempts=100) - - -@pytest.fixture(scope="session") -def service() -> Iterator[None]: - if not EXECUTABLE: - yield - return - - with start_service() as process: - yield - process.terminate() + logger = create_logger(DEBUG) + return await connect(HOST, PORT, max_attempts=10, logger=logger) @pytest_asyncio.fixture -async def connection(service) -> AsyncIterator[Connection]: +async def connection() -> AsyncIterator[Connection]: async with await connect_to_service() as connection: yield connection diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index 4586bb3a0..dbc076553 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -23,68 +23,38 @@ namespace brayns { -struct TagList -{ - std::vector tags; -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("tags", [](auto &object) { return &object.tags; }).description("List of object tag"); - return builder.build(); - } -}; - -struct MetadataList +struct ObjectList { std::vector objects; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); + auto builder = JsonBuilder(); builder.field("objects", [](auto &object) { return &object.objects; }) .description("List of object generic properties"); return builder.build(); } }; -std::vector getMetadata(ObjectManager &objects, const std::vector &ids) +std::vector getSelectedObjects(ObjectManager &objects, const std::vector &ids) { auto metadatas = std::vector(); metadatas.reserve(ids.size()); for (auto id : ids) { - const auto &metadata = objects.getMetadata(id); + const auto &metadata = objects.getObject(id); metadatas.push_back(metadata); } return metadatas; } -std::vector getIdsFromTags(ObjectManager &objects, const std::vector &tags) -{ - auto ids = std::vector(); - ids.reserve(tags.size()); - - for (const auto &tag : tags) - { - auto id = objects.getId(tag); - ids.push_back(id); - } - - return ids; -} - -void removeObjects(ObjectManager &objects, const std::vector &ids) +void removeSelectedObjects(ObjectManager &objects, const std::vector &ids) { for (auto id : ids) { @@ -94,16 +64,13 @@ void removeObjects(ObjectManager &objects, const std::vector &ids) void addObjectEndpoints(ApiBuilder &builder, ObjectManager &objects) { - builder.endpoint("get-all-objects", [&] { return MetadataList{objects.getAllMetadata()}; }) + builder.endpoint("get-all-objects", [&] { return ObjectList{objects.getAllObjects()}; }) .description("Return the generic properties of all objects, use get-{type} to get specific properties"); - builder.endpoint("get-objects", [&](IdList params) { return MetadataList{getMetadata(objects, params.ids)}; }) + builder.endpoint("get-objects", [&](IdList params) { return ObjectList{getSelectedObjects(objects, params.ids)}; }) .description("Get generic object properties from given object IDs"); - builder.endpoint("get-object-ids", [&](TagList params) { return IdList{getIdsFromTags(objects, params.tags)}; }) - .description("Map given list of tags to object IDs (result is an array in the same order as params)"); - - builder.endpoint("remove-objects", [&](IdList params) { removeObjects(objects, params.ids); }) + builder.endpoint("remove-objects", [&](IdList params) { removeSelectedObjects(objects, params.ids); }) .description( "Remove objects from the registry, the ID can be reused by future objects. Note that the object can stay " "in memory as long as it is used by other objects (using a ref-counted system)"); diff --git a/src/brayns/core/objects/ObjectManager.cpp b/src/brayns/core/objects/ObjectManager.cpp index 0a4ede43c..8b030006b 100644 --- a/src/brayns/core/objects/ObjectManager.cpp +++ b/src/brayns/core/objects/ObjectManager.cpp @@ -27,25 +27,11 @@ namespace { using namespace brayns; -using IdByTag = std::unordered_map; - void disableNullId(IdGenerator &ids) { ids.next(); } -void checkTagIsNotAlreadyUsed(const IdByTag &ids, const std::string &tag) -{ - auto i = ids.find(tag); - - if (i == ids.end()) - { - return; - } - - throw InvalidParams(fmt::format("Tag '{}' already used by object with ID {}", tag, i->first)); -} - auto getObjectIterator(auto &objects, ObjectId id) { auto i = objects.find(id); @@ -67,7 +53,7 @@ ObjectManager::ObjectManager() disableNullId(_ids); } -std::vector ObjectManager::getAllMetadata() const +std::vector ObjectManager::getAllObjects() const { auto objects = std::vector(); objects.reserve(_objects.size()); @@ -80,37 +66,18 @@ std::vector ObjectManager::getAllMetadata() const return objects; } -const Metadata &ObjectManager::getMetadata(ObjectId id) const +const Metadata &ObjectManager::getObject(ObjectId id) const { auto i = getObjectIterator(_objects, id); return *i->second.getMetadata(); } -ObjectId ObjectManager::getId(const std::string &tag) const -{ - auto i = _idsByTag.find(tag); - - if (i == _idsByTag.end()) - { - throw InvalidParams(fmt::format("No objects found with tag '{}'", tag)); - } - - return i->second; -} - void ObjectManager::remove(ObjectId id) { auto i = getObjectIterator(_objects, id); auto &metadata = *i->second.getMetadata(); - const auto &tag = metadata.tag; - - if (!tag.empty()) - { - _idsByTag.erase(tag); - } - metadata.id = nullId; _objects.erase(i); } @@ -140,29 +107,6 @@ void ObjectManager::checkType(const ObjectManagerEntry &entry, const std::type_i const ObjectManagerEntry &ObjectManager::getEntry(ObjectId id) const { auto i = getObjectIterator(_objects, id); - return i->second; } - -void ObjectManager::addEntry(ObjectId id, ObjectManagerEntry entry) -{ - const auto &metadata = *entry.getMetadata(); - const auto &tag = metadata.tag; - - if (!tag.empty()) - { - checkTagIsNotAlreadyUsed(_idsByTag, tag); - _idsByTag.emplace(tag, id); - } - - try - { - _objects.emplace(id, std::move(entry)); - } - catch (...) - { - _idsByTag.erase(tag); - throw; - } -} } diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index 9f489db89..4dc94b88d 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -43,7 +43,6 @@ struct Metadata { ObjectId id; std::string type; - std::string tag = {}; JsonValue userData = {}; }; @@ -57,8 +56,6 @@ struct JsonObjectReflector .description("Object ID, primary way to query this object"); builder.field("type", [](auto &object) { return &object.type; }) .description("Object type, use endpoint 'get-{type}' to query detailed information about the object"); - builder.field("tag", [](auto &object) { return &object.tag; }) - .description("Optional user defined tag, can also be used to query this object"); builder.field("user_data", [](auto &object) { return &object.userData; }) .description("Optional user data (only for user, not used by brayns)"); return builder.build(); @@ -107,10 +104,11 @@ struct ObjectManagerEntry std::function getMetadata; }; +template struct UserObjectSettings { std::string type; - std::string tag = {}; + Properties properties; JsonValue userData = {}; }; @@ -119,9 +117,8 @@ class ObjectManager public: explicit ObjectManager(); - std::vector getAllMetadata() const; - const Metadata &getMetadata(ObjectId id) const; - ObjectId getId(const std::string &tag) const; + std::vector getAllObjects() const; + const Metadata &getObject(ObjectId id) const; void remove(ObjectId id); void clear(); @@ -140,21 +137,19 @@ class ObjectManager } template - T &create(UserObjectSettings settings, GetUserObjectProperties properties) + T &create(UserObjectSettings> settings) { auto id = _ids.next(); try { - auto &[type, tag, userData] = settings; - - auto metadata = Metadata{id, std::move(type), std::move(tag), userData}; - auto object = T{std::move(metadata), std::move(properties)}; + auto metadata = Metadata{id, std::move(settings.type), settings.userData}; + auto object = T{std::move(metadata), std::move(settings.properties)}; auto ptr = std::make_shared(std::move(object)); auto entry = ObjectManagerEntry{ptr, [=] { return &ptr->metadata; }}; - addEntry(id, std::move(entry)); + _objects.emplace(id, std::move(entry)); return *ptr; } @@ -173,6 +168,5 @@ class ObjectManager static void checkType(const ObjectManagerEntry &entry, const std::type_info &expected); const ObjectManagerEntry &getEntry(ObjectId id) const; - void addEntry(ObjectId id, ObjectManagerEntry entry); }; } diff --git a/src/brayns/core/service/Service.cpp b/src/brayns/core/service/Service.cpp index d25f5eb4a..05f74e82d 100644 --- a/src/brayns/core/service/Service.cpp +++ b/src/brayns/core/service/Service.cpp @@ -61,6 +61,7 @@ std::optional execute(JsonRpcRequest request, Api &api, Logger logger.info("Calling endpoint for request {}", toString(request.id)); auto params = RawParams{std::move(request.params), std::move(request.binary)}; auto result = api.execute(request.method, std::move(params)); + logger.info("Successfully called endpoint"); if (!request.id) @@ -69,7 +70,7 @@ std::optional execute(JsonRpcRequest request, Api &api, Logger return {}; } - return JsonRpcSuccessResponse{*request.id, std::move(result)}; + return JsonRpcSuccessResponse{*request.id, result.json, std::move(result.binary)}; } catch (const JsonRpcException &e) { diff --git a/src/brayns/core/websocket/WebSocketServer.cpp b/src/brayns/core/websocket/WebSocketServer.cpp index 7fc7ab547..c26a59045 100644 --- a/src/brayns/core/websocket/WebSocketServer.cpp +++ b/src/brayns/core/websocket/WebSocketServer.cpp @@ -54,7 +54,7 @@ class HealthcheckRequestHandler : public Poco::Net::HTTPRequestHandler response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_OK); - response.send() << "OK"; + response.send() << "OK\n"; _logger->debug("Healthcheck ok"); } @@ -146,7 +146,7 @@ class RequestHandlerFactory : public Poco::Net::HTTPRequestHandlerFactory _logger->info("New HTTP request from {} to uri '{}'", host, uri); - if (uri == "/healthz") + if (uri == "/health") { return new HealthcheckRequestHandler(*_logger); } @@ -189,17 +189,26 @@ Poco::Net::Context::Ptr createSslContext(const SslSettings &settings) Poco::Net::ServerSocket createServerSocket(const WebSocketServerSettings &settings) { auto address = Poco::Net::SocketAddress(settings.host, settings.port); + auto reuseAddress = true; + auto reusePort = false; + auto backlog = 64; if (!settings.ssl) { - return Poco::Net::ServerSocket(address); - } + auto socket = Poco::Net::ServerSocket(); + socket.bind(address, reuseAddress, reusePort); + socket.listen(backlog); - auto backlog = 64; + return socket; + } auto context = createSslContext(*settings.ssl); - return Poco::Net::SecureServerSocket(address, backlog, context); + auto socket = Poco::Net::SecureServerSocket(context); + socket.bind(address, reuseAddress, reusePort); + socket.listen(backlog); + + return socket; } Poco::Net::HTTPServerParams::Ptr extractServerParams(const WebSocketServerSettings &settings) diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index 3a26407b0..300df4490 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -50,23 +50,21 @@ TEST_CASE("Create and remove objects") { auto objects = ObjectManager(); - auto &object = objects.create({"type", "tag", "someUserData"}, {123, "123"}); + auto &object = objects.create({"type", {123, "123"}, "someUserData"}); CHECK_EQ(object.metadata.id, 1); CHECK_EQ(object.metadata.type, "type"); - CHECK_EQ(object.metadata.tag, "tag"); CHECK_EQ(object.metadata.userData.extract(), "someUserData"); CHECK_EQ(object.properties.someInt, 123); CHECK_EQ(object.properties.someString, "123"); - auto &another = objects.create({"type"}, {}); + auto &another = objects.create({"type", {}}); CHECK_EQ(another.metadata.id, 2); - CHECK_EQ(objects.getAllMetadata().size(), 2); + CHECK_EQ(objects.getAllObjects().size(), 2); CHECK_EQ(&objects.get(1), &object); - CHECK_EQ(&objects.getMetadata(1), &object.metadata); - CHECK_EQ(objects.getId("tag"), 1); + CHECK_EQ(&objects.getObject(1), &object.metadata); auto shared = objects.getShared(1); @@ -74,24 +72,21 @@ TEST_CASE("Create and remove objects") CHECK_EQ(shared->metadata.id, nullId); - CHECK_THROWS_AS(objects.getMetadata(1), InvalidParams); + CHECK_THROWS_AS(objects.getObject(1), InvalidParams); objects.clear(); - CHECK(objects.getAllMetadata().empty()); + CHECK(objects.getAllObjects().empty()); } TEST_CASE("Errors") { auto objects = ObjectManager(); - objects.create({"type", "tag"}, {}); + objects.create({"type", {}}); - CHECK_THROWS_AS(objects.create({"type", "tag"}, {}), InvalidParams); - - CHECK_THROWS_AS(objects.getId("invalid tag"), InvalidParams); - CHECK_THROWS_AS(objects.getMetadata(0), InvalidParams); - CHECK_THROWS_AS(objects.getMetadata(2), InvalidParams); + CHECK_THROWS_AS(objects.getObject(0), InvalidParams); + CHECK_THROWS_AS(objects.getObject(2), InvalidParams); CHECK_THROWS_AS(objects.remove(2), InvalidParams); CHECK_THROWS_AS(objects.get>(1), InvalidParams); } From 5dd0870dfdc0d88c0d58e9c86a583d4a888dfefe Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Fri, 2 Aug 2024 10:10:03 +0200 Subject: [PATCH 03/12] Object creation endpoints + tests. --- python/brayns/__init__.py | 16 ++++ python/brayns/api/core/objects.py | 89 +++++++++++++++++++ python/tests/integration/conftest.py | 3 +- python/tests/integration/core/test_objects.py | 84 +++++++++++++++++ src/brayns/core/endpoints/ObjectEndpoints.cpp | 37 ++++---- src/brayns/core/endpoints/ObjectEndpoints.h | 60 ++++++++++++- src/brayns/core/objects/ObjectManager.cpp | 5 +- src/brayns/core/objects/ObjectManager.h | 5 +- tests/core/objects/TestObjectManager.cpp | 12 +-- 9 files changed, 275 insertions(+), 36 deletions(-) create mode 100644 python/brayns/api/core/objects.py create mode 100644 python/tests/integration/core/test_objects.py diff --git a/python/brayns/__init__.py b/python/brayns/__init__.py index 54fc77b73..d0d7b48b9 100644 --- a/python/brayns/__init__.py +++ b/python/brayns/__init__.py @@ -51,19 +51,32 @@ from .network.websocket import ServiceUnavailable, WebSocketError from .version import VERSION from .utils.logger import create_logger +from .api.core.objects import ( + Object, + Metadata, + get_object, + get_all_objects, + remove_objects, + clear_objects, + create_object, +) __version__ = VERSION """Version tag of brayns Python package (major.minor.patch).""" __all__ = [ "cancel_task", + "clear_objects", "connect", "Connection", "create_logger", + "create_object", "Endpoint", "FutureResponse", + "get_all_objects", "get_endpoint", "get_methods", + "get_object", "get_task_result", "get_task", "get_tasks", @@ -74,6 +87,9 @@ "JsonRpcRequest", "JsonRpcResponse", "JsonRpcSuccessResponse", + "Metadata", + "Object", + "remove_objects", "Request", "Response", "ServiceUnavailable", diff --git a/python/brayns/api/core/objects.py b/python/brayns/api/core/objects.py new file mode 100644 index 000000000..ee9ecca95 --- /dev/null +++ b/python/brayns/api/core/objects.py @@ -0,0 +1,89 @@ +# Copyright (c) 2015-2024 EPFL/Blue Brain Project +# All rights reserved. Do not distribute without permission. +# +# Responsible Author: adrien.fleury@epfl.ch +# +# This file is part of Brayns +# +# This library is free software; you can redistribute it and/or modify it under +# the terms of the GNU Lesser General Public License version 3.0 as published +# by the Free Software Foundation. +# +# This library is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more +# details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +from dataclasses import dataclass +from typing import Any, TypeVar +from brayns.network.connection import Connection +from brayns.utils.parsing import get, check_type + + +@dataclass +class Metadata: + id: int + type: str + user_data: Any + + +@dataclass +class Object: + metadata: Metadata + + @property + def id(self) -> int: + return self.metadata.id + + @property + def user_data(self) -> Any: + return self.metadata.user_data + + +def parse_metadata(message: dict[str, Any]) -> Metadata: + return Metadata( + id=get(message, "id", int), + type=get(message, "type", str), + user_data=get(message, "user_data", Any), + ) + + +def extract_metadata(result: dict[str, Any]) -> Metadata: + return parse_metadata(result["metadata"]) + + +T = TypeVar("T", bound=Object) + + +async def get_all_objects(connection: Connection) -> list[Object]: + result = await connection.get_result("get-all-objects") + check_type(result, dict[str, Any]) + objects = get(result, "objects", list[dict[str, Any]]) + return [Object(parse_metadata(item)) for item in objects] + + +async def get_object(connection: Connection, id: int) -> Object: + result = await connection.get_result("get-object", {"id": id}) + check_type(result, dict[str, Any]) + return Object(parse_metadata(result)) + + +async def remove_objects(connection: Connection, ids: list[int]) -> None: + await connection.get_result("remove-objects", {"ids": ids}) + + +async def clear_objects(connection: Connection) -> None: + await connection.get_result("clear-objects") + + +async def create_object(connection: Connection, user_data: Any = None) -> Object: + result = await connection.get_result("create-empty-object", {"user_data": user_data}) + check_type(result, dict[str, Any]) + return Object(extract_metadata(result)) + + +T = TypeVar("T", bound=Metadata) diff --git a/python/tests/integration/conftest.py b/python/tests/integration/conftest.py index 32dc744ca..85b42d712 100644 --- a/python/tests/integration/conftest.py +++ b/python/tests/integration/conftest.py @@ -24,7 +24,7 @@ import pytest_asyncio -from brayns import Connection, connect, create_logger +from brayns import Connection, connect, create_logger, clear_objects HOST = os.getenv("BRAYNS_HOST", "localhost") PORT = int(os.getenv("BRAYNS_PORT", "5000")) @@ -38,4 +38,5 @@ async def connect_to_service() -> Connection: @pytest_asyncio.fixture async def connection() -> AsyncIterator[Connection]: async with await connect_to_service() as connection: + await clear_objects(connection) yield connection diff --git a/python/tests/integration/core/test_objects.py b/python/tests/integration/core/test_objects.py new file mode 100644 index 000000000..e7dded765 --- /dev/null +++ b/python/tests/integration/core/test_objects.py @@ -0,0 +1,84 @@ +# Copyright (c) 2015-2024 EPFL/Blue Brain Project +# All rights reserved. Do not distribute without permission. +# +# Responsible Author: adrien.fleury@epfl.ch +# +# This file is part of Brayns +# +# This library is free software; you can redistribute it and/or modify it under +# the terms of the GNU Lesser General Public License version 3.0 as published +# by the Free Software Foundation. +# +# This library is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more +# details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +from brayns import ( + get_all_objects, + get_object, + clear_objects, + remove_objects, + Connection, + JsonRpcError, + create_object, +) + +import pytest + + +@pytest.mark.integration_test +@pytest.mark.asyncio +async def test_create_object(connection: Connection) -> None: + obj = await create_object(connection, "test") + assert obj.id == 1 + assert obj.metadata.type == "object" + assert obj.user_data == "test" + + +@pytest.mark.integration_test +@pytest.mark.asyncio +async def test_get_all_objects(connection: Connection) -> None: + objects = [await create_object(connection) for _ in range(10)] + tests = await get_all_objects(connection) + assert tests == objects + + +@pytest.mark.integration_test +@pytest.mark.asyncio +async def test_get_object(connection: Connection) -> None: + objects = [await create_object(connection) for _ in range(10)] + tests = [await get_object(connection, obj.id) for obj in objects] + assert tests == objects + + with pytest.raises(JsonRpcError): + await get_object(connection, 123) + + +@pytest.mark.integration_test +@pytest.mark.asyncio +async def test_remove_objects(connection: Connection) -> None: + objects = [await create_object(connection) for _ in range(10)] + await remove_objects(connection, [1, 2, 3]) + tests = await get_all_objects(connection) + assert [obj.id for obj in tests] == list(range(4, 11)) + + for obj in objects[:3]: + with pytest.raises(JsonRpcError): + await get_object(connection, obj.id) + + +@pytest.mark.integration_test +@pytest.mark.asyncio +async def test_clear_objects(connection: Connection) -> None: + objects = [await create_object(connection) for _ in range(10)] + await clear_objects(connection) + assert not await get_all_objects(connection) + + for obj in objects: + with pytest.raises(JsonRpcError): + await get_object(connection, obj.id) diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index dbc076553..3577766bd 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -40,41 +40,40 @@ struct JsonObjectReflector } }; -std::vector getSelectedObjects(ObjectManager &objects, const std::vector &ids) +void removeSelectedObjects(ObjectManager &objects, const std::vector &ids) { - auto metadatas = std::vector(); - metadatas.reserve(ids.size()); - for (auto id : ids) { - const auto &metadata = objects.getObject(id); - metadatas.push_back(metadata); + objects.remove(id); } - - return metadatas; } -void removeSelectedObjects(ObjectManager &objects, const std::vector &ids) +using EmptyObject = UserObject>; +using EmptyObjectParams = UserObjectParams>; + +EmptyObject &createEmptyObject(ObjectManager &objects, const EmptyObjectParams ¶ms) { - for (auto id : ids) - { - objects.remove(id); - } + return objects.create({"object", {}, params.userData}); } void addObjectEndpoints(ApiBuilder &builder, ObjectManager &objects) { - builder.endpoint("get-all-objects", [&] { return ObjectList{objects.getAllObjects()}; }) + builder.endpoint("get-all-objects", [&] { return ObjectList{objects.getAllMetadata()}; }) .description("Return the generic properties of all objects, use get-{type} to get specific properties"); - builder.endpoint("get-objects", [&](IdList params) { return ObjectList{getSelectedObjects(objects, params.ids)}; }) + builder.endpoint("get-object", [&](ObjectIdParams params) { return objects.getMetadata(params.id); }) .description("Get generic object properties from given object IDs"); - builder.endpoint("remove-objects", [&](IdList params) { removeSelectedObjects(objects, params.ids); }) - .description( - "Remove objects from the registry, the ID can be reused by future objects. Note that the object can stay " - "in memory as long as it is used by other objects (using a ref-counted system)"); + builder.endpoint("remove-objects", [&](ObjectIds params) { removeSelectedObjects(objects, params.ids); }) + .description("Remove selected objects from registry (but not from scene)"); builder.endpoint("clear-objects", [&] { objects.clear(); }).description("Remove all objects currently in registry"); + + builder + .endpoint("create-empty-object", [&](EmptyObjectParams params) { return createEmptyObject(objects, params); }) + .description("Create an empty object just to store user data"); + + builder.endpoint("get-empty-object", [&](ObjectIdParams params) { return objects.get(params.id); }) + .description("Create an empty object just to store user data"); } } diff --git a/src/brayns/core/endpoints/ObjectEndpoints.h b/src/brayns/core/endpoints/ObjectEndpoints.h index dfbd52643..d29d77565 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.h +++ b/src/brayns/core/endpoints/ObjectEndpoints.h @@ -26,18 +26,70 @@ namespace brayns { -struct IdList +struct ObjectIds { std::vector ids; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); - builder.field("ids", [](auto &object) { return &object.ids; }).description("List of objects ID"); + auto builder = JsonBuilder(); + builder.field("ids", [](auto &object) { return &object.ids; }).description("List of object IDs"); + return builder.build(); + } +}; + +struct ObjectIdParams +{ + ObjectId id; +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("id", [](auto &object) { return &object.id; }).description("Objects ID"); + return builder.build(); + } +}; + +struct EmptyJsonObject +{ +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.description("Placeholder empty object"); + return builder.build(); + } +}; + +template +struct UserObjectParams +{ + T settings; + JsonValue userData; +}; + +template +struct JsonObjectReflector> +{ + static auto reflect() + { + auto builder = JsonBuilder>(); + builder.field("settings", [](auto &object) { return &object.settings; }) + .description("Settings specific to object type"); + builder.field("user_data", [](auto &object) { return &object.userData; }) + .description("User data to attach to the object (not used by Brayns)"); return builder.build(); } }; diff --git a/src/brayns/core/objects/ObjectManager.cpp b/src/brayns/core/objects/ObjectManager.cpp index 8b030006b..f6111a31b 100644 --- a/src/brayns/core/objects/ObjectManager.cpp +++ b/src/brayns/core/objects/ObjectManager.cpp @@ -53,7 +53,7 @@ ObjectManager::ObjectManager() disableNullId(_ids); } -std::vector ObjectManager::getAllObjects() const +std::vector ObjectManager::getAllMetadata() const { auto objects = std::vector(); objects.reserve(_objects.size()); @@ -66,7 +66,7 @@ std::vector ObjectManager::getAllObjects() const return objects; } -const Metadata &ObjectManager::getObject(ObjectId id) const +const Metadata &ObjectManager::getMetadata(ObjectId id) const { auto i = getObjectIterator(_objects, id); return *i->second.getMetadata(); @@ -85,7 +85,6 @@ void ObjectManager::remove(ObjectId id) void ObjectManager::clear() { _objects.clear(); - _idsByTag.clear(); _ids = {}; disableNullId(_ids); } diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index 4dc94b88d..c1530a438 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -117,8 +117,8 @@ class ObjectManager public: explicit ObjectManager(); - std::vector getAllObjects() const; - const Metadata &getObject(ObjectId id) const; + std::vector getAllMetadata() const; + const Metadata &getMetadata(ObjectId id) const; void remove(ObjectId id); void clear(); @@ -162,7 +162,6 @@ class ObjectManager private: std::map _objects; - std::unordered_map _idsByTag; IdGenerator _ids; static void checkType(const ObjectManagerEntry &entry, const std::type_info &expected); diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index 300df4490..d9260b5d1 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -60,11 +60,11 @@ TEST_CASE("Create and remove objects") auto &another = objects.create({"type", {}}); CHECK_EQ(another.metadata.id, 2); - CHECK_EQ(objects.getAllObjects().size(), 2); + CHECK_EQ(objects.getAllMetadata().size(), 2); CHECK_EQ(&objects.get(1), &object); - CHECK_EQ(&objects.getObject(1), &object.metadata); + CHECK_EQ(&objects.getMetadata(1), &object.metadata); auto shared = objects.getShared(1); @@ -72,11 +72,11 @@ TEST_CASE("Create and remove objects") CHECK_EQ(shared->metadata.id, nullId); - CHECK_THROWS_AS(objects.getObject(1), InvalidParams); + CHECK_THROWS_AS(objects.getMetadata(1), InvalidParams); objects.clear(); - CHECK(objects.getAllObjects().empty()); + CHECK(objects.getAllMetadata().empty()); } TEST_CASE("Errors") @@ -85,8 +85,8 @@ TEST_CASE("Errors") objects.create({"type", {}}); - CHECK_THROWS_AS(objects.getObject(0), InvalidParams); - CHECK_THROWS_AS(objects.getObject(2), InvalidParams); + CHECK_THROWS_AS(objects.getMetadata(0), InvalidParams); + CHECK_THROWS_AS(objects.getMetadata(2), InvalidParams); CHECK_THROWS_AS(objects.remove(2), InvalidParams); CHECK_THROWS_AS(objects.get>(1), InvalidParams); } From e61db7667681c1971ca0ca8b7cc410e7e2af9220 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Fri, 2 Aug 2024 11:35:51 +0200 Subject: [PATCH 04/12] SSL test + bug in poco -> downgrade version. --- python/README.md | 1 + python/tests/data/certificate.pem | 33 ++++++++++++ python/tests/data/key.pem | 54 +++++++++++++++++++ python/tests/data/self_signed_certificate.sh | 1 + python/tests/integration/conftest.py | 13 ++++- scripts/superbuild/CMakeLists.txt | 2 +- .../core/websocket/WebSocketHandler.cpp | 2 +- 7 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 python/tests/data/certificate.pem create mode 100644 python/tests/data/key.pem create mode 100644 python/tests/data/self_signed_certificate.sh diff --git a/python/README.md b/python/README.md index 3bb828414..620b7b5c7 100644 --- a/python/README.md +++ b/python/README.md @@ -81,6 +81,7 @@ pip install -r requirements-dev.txt ```bash BRAYNS_HOST=localhost BRAYNS_PORT=5000 +BRAYNS_SSL=0 ``` Note: integration testing can be disable using the pytest --without-integration flag. diff --git a/python/tests/data/certificate.pem b/python/tests/data/certificate.pem new file mode 100644 index 000000000..021c41a94 --- /dev/null +++ b/python/tests/data/certificate.pem @@ -0,0 +1,33 @@ +-----BEGIN CERTIFICATE----- +MIIFoTCCA4mgAwIBAgIUEIpfZpNVx+nIciUxF3YET/9EzUAwDQYJKoZIhvcNAQEL +BQAwYDELMAkGA1UEBhMCU1cxDzANBgNVBAgMBkdlbmV2YTEPMA0GA1UEBwwGR2Vu +ZXZhMQ0wCwYDVQQKDARFUEZMMQwwCgYDVQQLDANCQlAxEjAQBgNVBAMMCWxvY2Fs +aG9zdDAeFw0yNDA4MDIwODI0MTJaFw0zNDA3MzEwODI0MTJaMGAxCzAJBgNVBAYT +AlNXMQ8wDQYDVQQIDAZHZW5ldmExDzANBgNVBAcMBkdlbmV2YTENMAsGA1UECgwE +RVBGTDEMMAoGA1UECwwDQkJQMRIwEAYDVQQDDAlsb2NhbGhvc3QwggIiMA0GCSqG +SIb3DQEBAQUAA4ICDwAwggIKAoICAQCOr1EI4RFp4Kmy32oKojKHnQ6uQCXA3kE+ +3jMYlRSKtXfDSkmxZ7bct5w9L9LX2UrFStA7RpJ9tk9QIANdHMQ+ydA/Le3+nhxP +QblYmvZfHegYAFiWtxLasHjObNslAOYBhDmEGEgmXNrr3dGiNciUYdU9J1rU5Nqi +3MiwAl5wke9akUAwYA/AYb+044eNE8QU8lQJCBa7Y/oqhr4dE4vsxWbwrH76saIY +c2otztXPCdpLt8OZT6Wo7R4F0GfYOotirT9a3W1xwWSKQpOjxpWzljSjvEQnJJ/e +yaaPnM2ST3TxV7AGQdDAjJWPa66yxO5xYMZU6fdSzJmH2+kKBpxX1p9POfHRHhGZ +ua52gWuz7ylLtSOCGmyLvbmZugjn1DWsvciaDSczNbSeFGf7bUgBcjC4tl2M6BXq +lc9YzcNDn/yKLBzNF4BeMx1Z631nNhJiRu9dJTdMIV/gng+ZdVxLLz0hllCSNT6O +QSmQINFI/kGTOctDW62Obf/NeNMoat6kQG/x7VijVwPRiL5ryb2eiFF+jx7x7MtK +U3+tGYCtpwhHHR+QGgxkP8w6t0Wn70JGihClz7LUwcdLy6lrqUGUF4ZO9NtuBq6m +Zx4M56ijGp6Gnwh0EMaVLIpj37+3V+7CoiZ9wX7LjvbBODYM5I9c2gmA8/gImVxc +3m8UO24wXQIDAQABo1MwUTAdBgNVHQ4EFgQUMSLGq+5A3l4EHtyqpImM4LrYIpkw +HwYDVR0jBBgwFoAUMSLGq+5A3l4EHtyqpImM4LrYIpkwDwYDVR0TAQH/BAUwAwEB +/zANBgkqhkiG9w0BAQsFAAOCAgEAcKm72f8HUmCN2cCsZnkbmP/qRWBsOW/P6x/P +udlwgcgmK5yg+GgkvBctTq221eiRQ4ASCYy6+2mbMDdBFQ8N4p7jF4yh682WNyTl +yayxfm4J9rphLlGwnHVvp8E+HuAfa876lYu7zrAeE6dYQoLInd1gBqY7683UBHqk +hJtGJymZYLolAbfgfIPTh8pZ97UXYSJv+K1R1AIItO1fEUl5WLCUx2GNdu/1eieq +9rDExehKhRtB8wCYKVyRkXJUshp3MCshBM1VRm0CGdD3ta49or6k7uJcHjo/K6yQ +nT5PfpbHNvF3A03IDLF1DQTTkuRMZIX+SZ4TJCoC5QH9bo4uyfKhEVmxmVYcaqMs +COvLo0xmoHw4fxJ4zXrJa65thl47ZXALVA7TKWeqsh73j7cwWgwtLNpZsfSEh4Z5 +3qQAmvAzOH65pFNDtNS7DhJDh9c6pKk9ueZGwL4yVcAarUdEjG54gRQvxPy6YhWo +CkV2hWtkDPBnDxf2dziOa78NeB0oEPBw+tBt2P/jejdWB774wp7qgjgOIL1NKvGk +7/bN8TG8o/msicYenPCkBRv2K4/Klfh8wzCq2t8ailvTYM8k3R+/Nkz4LF/pesg/ +QlkZHRkia7T4rJa8WPg+GzE87LsDhs8qbV8EK8X363TZyPwMfGAZWyDN+AiXud10 +osxjP8g= +-----END CERTIFICATE----- diff --git a/python/tests/data/key.pem b/python/tests/data/key.pem new file mode 100644 index 000000000..a07831130 --- /dev/null +++ b/python/tests/data/key.pem @@ -0,0 +1,54 @@ +-----BEGIN ENCRYPTED PRIVATE KEY----- +MIIJnDBOBgkqhkiG9w0BBQ0wQTApBgkqhkiG9w0BBQwwHAQINeH1ztyoIGsCAggA +MAwGCCqGSIb3DQIJBQAwFAYIKoZIhvcNAwcECMTdCXUZQX86BIIJSJzjqY+QMWPn +1KIFI2/kQkmwE6ur87lh23tyHSZNXNmn7qsFDlzbsEHoAsWbS48KWXH+/ACTUj3s +q4tR2T5wgszzI6zxo8+icT6wrRPZBtVu3k+EPpytmc+KatPkNUJTXEtqMFq6mxjI +mJq7njWvZQapKCU4FwcnDCoSk/4VZRk6GD6PVSF2U2kYjxlon2XkXXGZC5Qfy2jH +m+6wRYsJW5+waxksq4WFj+/JyF+hYzNAqOBua5rYpr1fmioCSdqE7zieabwmRFXK +PgN0EIS9TxQzT/YW751qm850ndW0JdyeCyT56gFKOfC9OfEi0CoD7jQfWoiRwHxg +xekaTZhSIjU6yHP5UqsE3fENOncN0d91ZRmyYtgaFBvhx+2WGtTnvHH7C8rKVJ8L +670EJbHmMf/dRzbJzVxYVX85jVkpXCPWq7WAEWZe4rlixmQaaWpi8+4U6Sho8Wob +w8A/Ki2d6KQOLNF/5LTnXdkKmxYKyFvQDr8rtjkHTGQq3py8HyXHJYqjanHveQRP +vwInyPvzclgjGMQdWGjzcX2ba1X0uf8igdn5HmP6f5+sby0PrSCPwXttHDSteywY +dLZFjZe5X274IN8zjvPk+UABrAyMNV/EzUa+K38MPdNidibmwqs/+90fcSh3ELF+ +sB1vRAN4EWgr4XOszlXXqjnlX/NIm5kQmHkZfw0cMKNuYlMDQmAQ5yufdMl6XV/l +9ijgIuNgi2O0ZTjh4barw87AKuMfwaAdijvBojJTFndArVXbf47DQFXDp2vgJUL1 +9S396fg0vhzfPMRQA//X30eGN+3+/NpjkwFmMOxaMrhx7HHxGsxdYI1CfjmF26rm +k3CY++dzb8JwsuQNdGzoriRh2TZ4hy1iDBeu43ObDOLDAVyfNAlf7ShMKEPwJnwN +nDpP8KH2aVwPLQGiKRZRgqRW6g44xQxCFG/aaJ2wD0+x7JsmNDyhsarFN54WvMlK +nDvThjcjJo3WzjCvMfSZh5C7mp9rZsQBJK9ad1tvx7sjECpfmoRTaOxcUWS4uP5B +jltnP88V1KvrUmAydNmOcoNURP6MrDgD6ly+88qqyheF3k7xwtPlUixxoC0Ibve8 +/K2lqWlge2u7Uvt5ggm/X81MmzmRKpJ9cC7kYwqbFJIt25DxrOXguVD2epxOJv1R +aijCl5OYNiQFLxkxPpmGl/2hf+K8FsjK79+JB7KTCj9K9MvrYijASO00KVeW0D/u +AKgTVWwAmhYngNh44CjUC+eLXxL3Rhbmt01OEoSt06gWIr5QV04d6nuqRRCVtLVJ +feSZNXcem1YG+Sf1CAoxUQbLy31hAL0RIz31kO371vm2D0RfNRKDwtqI+pMGJ9rm +dtoT+3E1flnm3QFClRVF757RhFsWS1W7+uA4aiN2e2WOPXs0ZQ+zuHNrqLwl0byA +qIaAuH7CxYFcfSJZWehY6/LRftGaS9JZTPc6TCLRLjkpw9LEHDg5HEIXMe6+1nWo +iU3kkHmkyXnsrJdJzSLlitlbYfsIvOFisEpq+gPvzUSmKLC3sLWW8MoXZzb4ena8 +STU2T+3uLRA9criUYNAIG8HqaNqsqP1CjsPC/HApeRR2chXPMZ7n2UY4zM1gw42o +PPn5G++T1VeNLL9EN847wdPtdrjom+yb0tDoEs3gDWx5CW/c0vjAU5EGZ0JBx7TA +asGZF/VbGiPhzWU/zVdcOyN/xA4U7KM4FiV6aeTLeDLmrs5Suxv6V0K2LJpHHpuk +sq34Z/jfjC8sjNXITekhxF+U0C4FDhg18i0zSa+hb/NOpH+86LO/xlItYFbiGzQa +q5UhBmvDu9gecVz0sAGIHASp4augRsogGaHKe84bdPPvJY6CfXXmkKjzeCFVv8rK +eeAClf2EdXtl6q/+dylRbWMjIBF82GIJzJaWTXc6jAVjIkSwoV2/TPLY1bR7LcSi +Bq5vXobK1C80vB4NYaG/QbCzVrMyQbWL2dUuQH3jrfSV3N0XsTTdUNqXi6yw4MwX +dSUMiC9r34W4JFiVHK49tQsWf0bsKjyq9Bm2wW4/c5FMjGhKt4VMzOHP6pAi+EH7 +EwFQSxaSclQYwY8ySLWaSZweBCLA56ogJiefTJL2LRKHOG4ZF+85xfmp9/e10q1G +g8dC4HPgqiQ0LLdh665vPn+i6wl5J0pkgSqPWDpFZSYfo+HOOaIHcZWYkF+QHF7x +t9FfKlN9xPs8KrSbAn69ROVXu1k+e5JxmXmaMjvO4CahdvsPRnE3yCBU1PPnAZa2 +dTdgXqaUH1iiyo/cDdMDkhmtKO4KvCkbSasqbKIo7ru6p/EuD4S6YfWnBv0HSOrX +g3dczdfpan/Sxj5BUrO94K/OgqwAdJSveTcxrF31115TvJZkg8kFaGMp9r+Px+KZ +hMNJekDdIR1IF+rpMyebbPIStxofMb4sFJ2fMDaTwNbOeAOtC+ebxSgnNd4nMoyp +w3bFcq0PSySIoq/NcdlT3DlehMRhgXkBZgbZWuKjUv8D37Bqxx8ZAvgT4ZP03Kst +5DVdzK59H71hU9u0K2xUqZM2WgkAAp7kt0eZxu6c7MkEY1gUPE7Cr7mQ0+Kl5KRv +pRXz3YQM5gIiwthzqsxZo+/aYr8J5f1YxTY2ApbDolhBBC00VuMogcI5KbjrnScL +cElMGnFKajoWNXtLm7nSJrkE3dOFIwYU+SEzo1EAGubR9aA468+u3CJHveFlhFxa +uFf9+6QLRNIJcS9P6zlF2hiwspdzcJtgj/pA9SISG9LUV+nVhBtSXvIoRWcTTvUB +KK+BA3zPJmIXFXNTw08kkO9W0gQp46dpxIrMul2LyZxpRERclWgLzvPyKgrpMU6N +ukdJUKV+uLcGcP1DE0Ov2crO9TqQv0UsEY8i+faZjsc7TtfFzYa1CrtLDg2Vqqhz +t16fFNAFxxkHlQRKAGNXPNi8HnywLNOD0kkS33JRku5OiJvycdhEIgtG4XiIem6u +U00nn6Jz4IOkamQWZkw7pXjPb/p5d8aVQshT3nQEenHM8w3DqBnUyt7TsXq7UaJN +IzkjZRlOypgnti64Bcm1nONwEmiqJCvrZJ2SrYYL0f4cuHCE42QZOTN4ek0qOsID +MdmAXTdYUYNJneQjntbn2OW5R32EVmbGr2U7d8dOuWwmSVVClrySWJO6wHH6bECW +rHIlFjoH8apKDTgMgbF1BA== +-----END ENCRYPTED PRIVATE KEY----- diff --git a/python/tests/data/self_signed_certificate.sh b/python/tests/data/self_signed_certificate.sh new file mode 100644 index 000000000..2ad436337 --- /dev/null +++ b/python/tests/data/self_signed_certificate.sh @@ -0,0 +1 @@ +openssl req -x509 -newkey rsa:4096 -passout pass:'test' -keyout key.pem -out certificate.pem -sha256 -days 3650 -subj "/C=SW/ST=Geneva/L=Geneva/O=EPFL/OU=BBP/CN=localhost" diff --git a/python/tests/integration/conftest.py b/python/tests/integration/conftest.py index 85b42d712..1b78bdcae 100644 --- a/python/tests/integration/conftest.py +++ b/python/tests/integration/conftest.py @@ -21,6 +21,8 @@ from logging import DEBUG import os from collections.abc import AsyncIterator +from pathlib import Path +from ssl import SSLContext, create_default_context, Purpose import pytest_asyncio @@ -28,11 +30,20 @@ HOST = os.getenv("BRAYNS_HOST", "localhost") PORT = int(os.getenv("BRAYNS_PORT", "5000")) +SSL = bool(int(os.getenv("BRAYNS_SSL", "0"))) + +DATA = Path(__file__).parent.parent / "data" +CA = DATA / "certificate.pem" + + +def create_ssl_context() -> SSLContext: + return create_default_context(Purpose.SERVER_AUTH, cafile=CA) async def connect_to_service() -> Connection: logger = create_logger(DEBUG) - return await connect(HOST, PORT, max_attempts=10, logger=logger) + ssl = create_ssl_context() if SSL else None + return await connect(HOST, PORT, ssl, max_attempts=10, logger=logger) @pytest_asyncio.fixture diff --git a/scripts/superbuild/CMakeLists.txt b/scripts/superbuild/CMakeLists.txt index 7250644f0..a00f56367 100644 --- a/scripts/superbuild/CMakeLists.txt +++ b/scripts/superbuild/CMakeLists.txt @@ -103,7 +103,7 @@ ExternalProject_Add( ExternalProject_Add( poco GIT_REPOSITORY https://github.com/pocoproject/poco.git - GIT_TAG poco-1.13.3-release + GIT_TAG poco-1.12.5-release GIT_SHALLOW ON CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} diff --git a/src/brayns/core/websocket/WebSocketHandler.cpp b/src/brayns/core/websocket/WebSocketHandler.cpp index a5f09b862..baaf7f949 100644 --- a/src/brayns/core/websocket/WebSocketHandler.cpp +++ b/src/brayns/core/websocket/WebSocketHandler.cpp @@ -109,7 +109,7 @@ void respond(ClientId clientId, WebSocket &websocket, Logger &logger, const Mess try { - logger.info("Sending websocket frame of {} bytes", data.size()); + logger.info("Sending websocket frame of {} bytes", chunk.size()); websocket.send({opcode, chunk, finalFrame}); } catch (const WebSocketException &e) From 5c8d43c2444c26f163ec0779123cfa8772e2a9db Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Mon, 5 Aug 2024 16:32:54 +0200 Subject: [PATCH 05/12] C++ implementation rewritten. --- README.md | 36 +--- python/README.md | 6 +- src/brayns/core/Launcher.cpp | 7 +- src/brayns/core/endpoints/ObjectEndpoints.cpp | 84 +++++--- src/brayns/core/endpoints/ObjectEndpoints.h | 87 ++++++--- src/brayns/core/objects/Messages.h | 119 ++++++++++++ src/brayns/core/objects/ObjectManager.cpp | 53 ++--- src/brayns/core/objects/ObjectManager.h | 183 +++++++++--------- src/brayns/core/objects/ObjectReflector.h | 103 ++++++++++ tests/core/objects/TestObjectManager.cpp | 109 +++++++++-- 10 files changed, 562 insertions(+), 225 deletions(-) create mode 100644 src/brayns/core/objects/Messages.h create mode 100644 src/brayns/core/objects/ObjectReflector.h diff --git a/README.md b/README.md index 4087ddfe2..efa661877 100644 --- a/README.md +++ b/README.md @@ -10,37 +10,11 @@ Brayns comes with a main application: ## Building -Brayns is developed, maintained and run on Linux-based operating systems, being tested mainly on RHEL and Ubuntu. The following platforms and build environments have been tested: - -* Linux: Ubuntu 16.04, Ubuntu 18.04, Ubuntu 20.04, Debian 9, RHEL 7 (Makefile, x64) +TODO ### System dependencies -The following components must be installed on the system where Brayns will be built: - -* GCC 12.1 or higher (Requires C++ 20 support) -* CMake 3.15 or higher -* Make or Ninja build systems -* Git -* Package config -* SSL Development files -* Python 3.9 or higher -* Custom OSPRay 2.10.5 (https://github.com/BlueBrain/ospray/tree/v2.10.5) -* zlib - -Optionally, to build the core plugins of Brayns, the following components are required. - -* HDF5 development files -* Bzip2 - -Brayns uses further dependencies, but if they are not present on the system, it will download them by itself during build. - -* Poco libraries 1.12.4 (https://github.com/pocoproject/poco/tree/poco-1.12.4-release) -* spdlog 1.9.2 (https://github.com/gabime/spdlog/tree/v1.9.2) -* stb (https://github.com/nothings/stb) -* tinyexr (https://github.com/syoyo/tinyexr/tree/v1.0.1) -* libsonata 0.1.22 (https://github.com/BlueBrain/libsonata/tree/v0.1.22) -* MorphIO 3.3.5 (https://github.com/BlueBrain/MorphIO/tree/v3.3.5) +TODO ### Build command @@ -70,7 +44,7 @@ The following cmake options (shown with their default value) can be used during To run the braynsService app, execute the following command (The command assumes braynsService executable is available on the system **PATH**): - $ braynsService --uri 0.0.0.0:5000 + $ braynsService --host 0.0.0.0 --port 5000 The ***--uri*** parameter allows to specify an address and a port to bind to. In the example, the service is binding to all available addresses and the port 5000. @@ -82,7 +56,7 @@ Use the following command to get more details about command line arguments. Brayns is available as a docker image at https://hub.docker.com/r/bluebrain/brayns. The image allows to launch the braynsService application. -It is built with every commit merged into the main repository branch (develop), and deployed into docker hub as brayns:latest. Furthermore, when a new release is made, and a new tag created, an additional image is built and deployed with the same tag. +It is built with every commit merged into the main repository branch (master), and deployed into docker hub as brayns:latest. Furthermore, when a new release is made, and a new tag created, an additional image is built and deployed with the same tag. To get Brayns docker image, you will need to have docker installed. Then execute the following command to download it: @@ -90,7 +64,7 @@ To get Brayns docker image, you will need to have docker installed. Then execute To run it, simply execute the following command: - $ docker run -ti --rm -p 5000:5000 bluebrain/brayns --uri 0.0.0.0:5000 + $ docker run -ti --rm -p 5000:5000 bluebrain/brayns --host 0.0.0.0 --port 5000 Additional parameters, can be specified in a similar fashion as in the **braynsService** application. diff --git a/python/README.md b/python/README.md index 620b7b5c7..e2d55befb 100644 --- a/python/README.md +++ b/python/README.md @@ -76,7 +76,7 @@ pip install -r requirements.txt pip install -r requirements-dev.txt ``` -3. For integration testing, create a `.env` file: +3. Optional (for integration testing), create a `.env` file: ```bash BRAYNS_HOST=localhost @@ -86,7 +86,7 @@ BRAYNS_SSL=0 Note: integration testing can be disable using the pytest --without-integration flag. -4. Create a .vscode folder and create a `launch.json` inside to use to debug tests: +4. Create a .vscode folder and create a `launch.json` file inside to be able to debug tests: ```json { @@ -107,7 +107,7 @@ Note: integration testing can be disable using the pytest --without-integration } ``` -5. In the same folder, create a `settings.json` to configure pytest: +5. In the same folder, create a `settings.json` file to configure pytest: ```json { diff --git a/src/brayns/core/Launcher.cpp b/src/brayns/core/Launcher.cpp index ba34b2f1e..199ddf90f 100644 --- a/src/brayns/core/Launcher.cpp +++ b/src/brayns/core/Launcher.cpp @@ -72,14 +72,17 @@ void startServerAndRunService(const ServiceSettings &settings, Logger &logger) logger.info("Building JSON-RPC API"); auto api = Api(); - auto builder = ApiBuilder(); addServiceEndpoints(builder, api, token); auto objects = ObjectManager(); - addObjectEndpoints(builder, objects); + addDefaultObjects(objects); + + auto locked = LockedObjects(std::move(objects), logger); + + addObjectEndpoints(builder, locked); api = builder.build(); diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index 3577766bd..7e7a5ab91 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -23,57 +23,95 @@ namespace brayns { -struct ObjectList +struct Metadatas { std::vector objects; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); + auto builder = JsonBuilder(); builder.field("objects", [](auto &object) { return &object.objects; }) .description("List of object generic properties"); return builder.build(); } }; -void removeSelectedObjects(ObjectManager &objects, const std::vector &ids) +Metadatas getAllObjects(LockedObjects &locked) { - for (auto id : ids) + return {locked.visit([](auto &objects) { return objects.getAllMetadata(); })}; +} + +Metadata getObject(LockedObjects &locked, ObjectId id) +{ + return locked.visit([&](auto &objects) { return objects.getMetadata(id); }); +} + +void removeSelectedObjects(LockedObjects &locked, const std::vector &ids) +{ + locked.visit( + [&](auto &objects) + { + for (auto id : ids) + { + objects.remove(id); + } + }); +} + +void clearObjects(LockedObjects &locked) +{ + locked.visit([](auto &objects) { objects.clear(); }); +} + +struct EmptyObject +{ +}; + +template<> +struct ObjectReflector +{ + using Settings = EmptyJson; + + static std::string getType() { - objects.remove(id); + return "empty-object"; } -} -using EmptyObject = UserObject>; -using EmptyObjectParams = UserObjectParams>; + static EmptyJson getProperties(const EmptyObject &) + { + return {}; + } +}; + +LockedObjects::LockedObjects(ObjectManager objects, Logger &logger): + _objects(std::move(objects)), + _logger(&logger) +{ +} -EmptyObject &createEmptyObject(ObjectManager &objects, const EmptyObjectParams ¶ms) +void addDefaultObjects(ObjectManager &objects) { - return objects.create({"object", {}, params.userData}); + objects.addFactory([](auto) { return EmptyObject(); }); } -void addObjectEndpoints(ApiBuilder &builder, ObjectManager &objects) +void addObjectEndpoints(ApiBuilder &builder, LockedObjects &locked) { - builder.endpoint("get-all-objects", [&] { return ObjectList{objects.getAllMetadata()}; }) - .description("Return the generic properties of all objects, use get-{type} to get specific properties"); + builder.endpoint("get-all-objects", [&] { return getAllObjects(locked); }) + .description("Get generic properties of all objects, use get-{type} to get details of an object"); - builder.endpoint("get-object", [&](ObjectIdParams params) { return objects.getMetadata(params.id); }) + builder.endpoint("get-object", [&](ObjectIdParams params) { return getObject(locked, params.id); }) .description("Get generic object properties from given object IDs"); - builder.endpoint("remove-objects", [&](ObjectIds params) { removeSelectedObjects(objects, params.ids); }) + builder.endpoint("remove-objects", [&](ObjectIds params) { removeSelectedObjects(locked, params.ids); }) .description("Remove selected objects from registry (but not from scene)"); - builder.endpoint("clear-objects", [&] { objects.clear(); }).description("Remove all objects currently in registry"); - - builder - .endpoint("create-empty-object", [&](EmptyObjectParams params) { return createEmptyObject(objects, params); }) - .description("Create an empty object just to store user data"); + builder.endpoint("clear-objects", [&] { clearObjects(locked); }) + .description("Remove all objects currently in registry"); - builder.endpoint("get-empty-object", [&](ObjectIdParams params) { return objects.get(params.id); }) - .description("Create an empty object just to store user data"); + addCreateAndGet(builder, locked); } } diff --git a/src/brayns/core/endpoints/ObjectEndpoints.h b/src/brayns/core/endpoints/ObjectEndpoints.h index d29d77565..b6b8041a0 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.h +++ b/src/brayns/core/endpoints/ObjectEndpoints.h @@ -21,25 +21,35 @@ #pragma once +#include +#include +#include +#include + #include #include +#include namespace brayns { -struct ObjectIds +class LockedObjects { - std::vector ids; -}; +public: + explicit LockedObjects(ObjectManager objects, Logger &logger); -template<> -struct JsonObjectReflector -{ - static auto reflect() + auto visit(std::invocable auto &&callable) -> decltype(callable(std::declval())) { - auto builder = JsonBuilder(); - builder.field("ids", [](auto &object) { return &object.ids; }).description("List of object IDs"); - return builder.build(); + _logger->info("Waiting for object manager lock"); + auto lock = std::lock_guard(_mutex); + _logger->info("Object manager lock acquired"); + + return callable(_objects); } + +private: + std::mutex _mutex; + ObjectManager _objects; + Logger *_logger; }; struct ObjectIdParams @@ -58,41 +68,54 @@ struct JsonObjectReflector } }; -struct EmptyJsonObject +struct ObjectIds { + std::vector ids; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); - builder.description("Placeholder empty object"); + auto builder = JsonBuilder(); + builder.field("ids", [](auto &object) { return &object.ids; }).description("List of object IDs"); return builder.build(); } }; -template -struct UserObjectParams +template +ObjectResult> createObject(LockedObjects &locked, ObjectParams> params) { - T settings; - JsonValue userData; -}; + return locked.visit( + [&](ObjectManager &objects) + { + auto id = objects.create(std::move(params.settings), params.userData); + return objects.getResult(id); + }); +} -template -struct JsonObjectReflector> +template +ObjectResult> getObject(LockedObjects &locked, ObjectId id) { - static auto reflect() - { - auto builder = JsonBuilder>(); - builder.field("settings", [](auto &object) { return &object.settings; }) - .description("Settings specific to object type"); - builder.field("user_data", [](auto &object) { return &object.userData; }) - .description("User data to attach to the object (not used by Brayns)"); - return builder.build(); - } -}; + return locked.visit([=](ObjectManager &objects) { return objects.getResult(id); }); +} + +template +void addCreateAndGet(ApiBuilder &builder, LockedObjects &locked) +{ + const auto &type = getObjectType(); + + builder.endpoint("get-" + type, [&](ObjectIdParams params) { return getObject(locked, params.id); }) + .description("Get all properties properties of given " + type); + + builder + .endpoint( + "create-" + type, + [&](ObjectParams> params) { return createObject(locked, std::move(params)); }) + .description("Create a new " + type + " and returns it"); +} -void addObjectEndpoints(ApiBuilder &builder, ObjectManager &objects); +void addDefaultObjects(ObjectManager &objects); +void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects); } diff --git a/src/brayns/core/objects/Messages.h b/src/brayns/core/objects/Messages.h new file mode 100644 index 000000000..a195b2b7a --- /dev/null +++ b/src/brayns/core/objects/Messages.h @@ -0,0 +1,119 @@ +/* Copyright (c) 2015-2024 EPFL/Blue Brain Project + * All rights reserved. Do not distribute without permission. + * + * Responsible Author: adrien.fleury@epfl.ch + * + * This file is part of Brayns + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License version 3.0 as published + * by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include + +#include + +namespace brayns +{ +using ObjectId = std::uint32_t; + +constexpr auto nullId = ObjectId(0); + +struct Metadata +{ + ObjectId id; + std::string type; + std::size_t size = 0; + JsonValue userData = {}; +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("id", [](auto &object) { return &object.id; }) + .description("Object ID, primary way to query this object (starts at 1)"); + builder.field("type", [](auto &object) { return &object.type; }) + .description("Object type key, use 'get-{type}' to query detailed information about the object"); + builder.field("size", [](auto &object) { return &object.size; }) + .description("Object size in memory (in bytes), 0 if negligible"); + builder.field("user_data", [](auto &object) { return &object.userData; }).description("Data set by user"); + return builder.build(); + } +}; + +template +struct ObjectParams +{ + T settings; + JsonValue userData = {}; +}; + +template +struct JsonObjectReflector> +{ + static auto reflect() + { + auto builder = JsonBuilder>(); + builder.field("settings", [](auto &object) { return &object.settings; }) + .description("Settings to create the object"); + builder.field("user_data", [](auto &object) { return &object.userData; }) + .description("Optional user data (only for user, not used by Brayns)") + .defaultValue(NullJson()); + return builder.build(); + } +}; + +template +struct ObjectResult +{ + Metadata metadata; + T properties; +}; + +template +struct JsonObjectReflector> +{ + static auto reflect() + { + auto builder = JsonBuilder>(); + builder.field("metadata", [](auto &object) { return &object.metadata; }) + .description("Generic object properties"); + builder.field("properties", [](auto &object) { return &object.properties; }) + .description("Properties that are specific to the object type"); + return builder.build(); + } +}; + +struct EmptyJsonObject +{ +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.description("Can also be an empty object"); + return builder.build(); + } +}; + +using EmptyJson = std::optional; +} diff --git a/src/brayns/core/objects/ObjectManager.cpp b/src/brayns/core/objects/ObjectManager.cpp index f6111a31b..f72c6f9d2 100644 --- a/src/brayns/core/objects/ObjectManager.cpp +++ b/src/brayns/core/objects/ObjectManager.cpp @@ -32,7 +32,7 @@ void disableNullId(IdGenerator &ids) ids.next(); } -auto getObjectIterator(auto &objects, ObjectId id) +auto getStorageIterator(auto &objects, ObjectId id) { auto i = objects.find(id); @@ -47,7 +47,6 @@ auto getObjectIterator(auto &objects, ObjectId id) namespace brayns { - ObjectManager::ObjectManager() { disableNullId(_ids); @@ -55,57 +54,61 @@ ObjectManager::ObjectManager() std::vector ObjectManager::getAllMetadata() const { - auto objects = std::vector(); - objects.reserve(_objects.size()); + auto metadatas = std::vector(); + metadatas.reserve(_objects.size()); for (const auto &[id, object] : _objects) { - objects.push_back(*object.getMetadata()); + auto metadata = createMetadata(id, object); + metadatas.push_back(std::move(metadata)); } - return objects; + return metadatas; } -const Metadata &ObjectManager::getMetadata(ObjectId id) const +Metadata ObjectManager::getMetadata(ObjectId id) const { - auto i = getObjectIterator(_objects, id); - return *i->second.getMetadata(); + const auto &storage = getStorage(id); + + return createMetadata(id, storage); } void ObjectManager::remove(ObjectId id) { - auto i = getObjectIterator(_objects, id); + auto i = getStorageIterator(_objects, id); - auto &metadata = *i->second.getMetadata(); + i->second.remove(); - metadata.id = nullId; _objects.erase(i); } void ObjectManager::clear() { + for (const auto &[id, object] : _objects) + { + object.remove(); + } + _objects.clear(); + _ids = {}; disableNullId(_ids); } -void ObjectManager::checkType(const ObjectManagerEntry &entry, const std::type_info &expected) +Metadata ObjectManager::createMetadata(ObjectId id, const ObjectStorage &storage) { - if (entry.object.type() == expected) - { - return; - } - - const auto &metadata = *entry.getMetadata(); - auto id = metadata.id; - const auto &type = metadata.type; - - throw InvalidParams(fmt::format("Invalid type '{}' for object with ID {}", id, type)); + return { + .id = id, + .type = storage.type, + .size = storage.getSize(), + .userData = storage.userData, + }; } -const ObjectManagerEntry &ObjectManager::getEntry(ObjectId id) const +const ObjectStorage &ObjectManager::getStorage(ObjectId id) const { - auto i = getObjectIterator(_objects, id); + auto i = getStorageIterator(_objects, id); + return i->second; } } diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index c1530a438..29ca2db0f 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -26,91 +26,31 @@ #include #include #include -#include +#include #include +#include + #include #include #include +#include "Messages.h" +#include "ObjectReflector.h" + namespace brayns { -using ObjectId = std::uint32_t; - -constexpr auto nullId = ObjectId(0); - -struct Metadata +struct ObjectStorage { - ObjectId id; + std::any object; std::string type; - JsonValue userData = {}; -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("id", [](auto &object) { return &object.id; }) - .description("Object ID, primary way to query this object"); - builder.field("type", [](auto &object) { return &object.type; }) - .description("Object type, use endpoint 'get-{type}' to query detailed information about the object"); - builder.field("user_data", [](auto &object) { return &object.userData; }) - .description("Optional user data (only for user, not used by brayns)"); - return builder.build(); - } -}; - -template -struct UserObject -{ - Metadata metadata; - Properties properties; -}; - -template -struct JsonObjectReflector> -{ - static auto reflect() - { - auto builder = JsonBuilder>(); - builder.field("metadata", [](auto &object) { return &object.metadata; }) - .description("Generic object properties (not specific to object type)"); - builder.field("properties", [](auto &object) { return &object.properties; }) - .description("Object properties (specific to object type)"); - return builder.build(); - } -}; - -template -struct UserObjectReflector; - -template -struct UserObjectReflector> -{ - using Type = Properties; + JsonValue userData; + std::function getSize; + std::function remove; }; template -concept ValidUserObject = requires { typename UserObjectReflector::Type; }; - -template -using GetUserObjectProperties = typename UserObjectReflector::Type; - -struct ObjectManagerEntry -{ - std::any object; - std::function getMetadata; -}; - -template -struct UserObjectSettings -{ - std::string type; - Properties properties; - JsonValue userData = {}; -}; +using ObjectFactory = std::function)>; class ObjectManager { @@ -118,40 +58,75 @@ class ObjectManager explicit ObjectManager(); std::vector getAllMetadata() const; - const Metadata &getMetadata(ObjectId id) const; + Metadata getMetadata(ObjectId id) const; void remove(ObjectId id); void clear(); - template + template + void addFactory(ObjectFactory factory) + { + const auto &type = typeid(T); + + if (_factories.contains(type)) + { + throw std::invalid_argument("A factory is already registered for given type"); + } + + _factories[type] = factory; + } + + template const std::shared_ptr &getShared(ObjectId id) const { - auto &entry = getEntry(id); - checkType(entry, typeid(std::shared_ptr)); - return std::any_cast &>(entry.object); + const auto &storage = getStorage(id); + + return castObject(id, storage); } - template + template T &get(ObjectId id) const { return *getShared(id); } - template - T &create(UserObjectSettings> settings) + template + GetProperties getProperties(ObjectId id) const + { + auto &object = get(id); + + return getObjectProperties(object); + } + + template + ObjectResult> getResult(ObjectId id) const + { + const auto &storage = getStorage(id); + const auto &object = castObject(id, storage); + + return {createMetadata(id, storage), getObjectProperties(*object)}; + } + + template + ObjectId create(GetSettings settings, const JsonValue &userData = {}) { auto id = _ids.next(); try { - auto metadata = Metadata{id, std::move(settings.type), settings.userData}; - auto object = T{std::move(metadata), std::move(settings.properties)}; + auto object = createObject({id, std::move(settings)}); auto ptr = std::make_shared(std::move(object)); - auto entry = ObjectManagerEntry{ptr, [=] { return &ptr->metadata; }}; + auto storage = ObjectStorage{ + .object = ptr, + .type = ObjectReflector::getType(), + .userData = std::move(userData), + .getSize = [=] { return getObjectSize(*ptr); }, + .remove = [=] { removeObject(*ptr); }, + }; - _objects.emplace(id, std::move(entry)); + _objects.emplace(id, std::move(storage)); - return *ptr; + return id; } catch (...) { @@ -161,11 +136,43 @@ class ObjectManager } private: - std::map _objects; + std::map _factories; + std::map _objects; IdGenerator _ids; - static void checkType(const ObjectManagerEntry &entry, const std::type_info &expected); + static Metadata createMetadata(ObjectId id, const ObjectStorage &storage); - const ObjectManagerEntry &getEntry(ObjectId id) const; + template + static const std::shared_ptr &castObject(ObjectId id, const ObjectStorage &storage) + { + auto ptr = std::any_cast>(&storage.object); + + if (ptr != nullptr) + { + return *ptr; + } + + const auto &expected = getObjectType(); + const auto &got = storage.type; + + throw InvalidParams(fmt::format("Invalid type for object with ID {}: expected {}, got {}", id, expected, got)); + } + + const ObjectStorage &getStorage(ObjectId id) const; + + template + T createObject(const ParamsOf ¶ms) + { + auto i = _factories.find(typeid(T)); + + if (i == _factories.end()) + { + throw std::invalid_argument("Unsupported object type"); + } + + const auto &factory = std::any_cast &>(i->second); + + return factory(std::move(params)); + } }; } diff --git a/src/brayns/core/objects/ObjectReflector.h b/src/brayns/core/objects/ObjectReflector.h new file mode 100644 index 000000000..98131ea46 --- /dev/null +++ b/src/brayns/core/objects/ObjectReflector.h @@ -0,0 +1,103 @@ +/* Copyright (c) 2015-2024 EPFL/Blue Brain Project + * All rights reserved. Do not distribute without permission. + * + * Responsible Author: adrien.fleury@epfl.ch + * + * This file is part of Brayns + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License version 3.0 as published + * by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include +#include + +#include + +#include "Messages.h" + +namespace brayns +{ +template +struct ObjectReflector; + +template +using GetSettings = typename ObjectReflector::Settings; + +template +concept WithSettings = ReflectedJson>; + +template +concept WithType = std::convertible_to::getType()), std::string>; + +template +using GetProperties = std::decay_t::getProperties(std::declval()))>; + +template +concept WithProperties = ReflectedJson>; + +template +concept ReflectedObject = WithSettings && WithType && WithProperties; + +template +struct ParamsOf +{ + ObjectId id; + GetSettings settings; +}; + +template +concept WithSize = std::convertible_to::getSize(std::declval())), std::size_t>; + +template +concept WithRemove = std::same_as::remove(std::declval())), void>; + +template +const std::string &getObjectType() +{ + static const std::string type = ObjectReflector::getType(); + return type; +} + +template +GetProperties getObjectProperties(const T &object) +{ + return ObjectReflector::getProperties(object); +} + +std::size_t getObjectSize(const auto &object) +{ + (void)object; + return 0; +} + +template +std::size_t getObjectSize(const T &object) +{ + return ObjectReflector::getSize(object); +} + +void removeObject(auto &object) +{ + (void)object; +} + +template +void removeObject(T &object) +{ + ObjectReflector::remove(object); +} +} diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index d9260b5d1..2b3b447b1 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -25,68 +25,135 @@ using namespace brayns; namespace brayns { -struct TestProperties +struct TestSettings { int someInt = 0; std::string someString = {}; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); + auto builder = JsonBuilder(); builder.field("some_int", [](auto &object) { return &object.someInt; }); builder.field("some_string", [](auto &object) { return &object.someString; }); return builder.build(); } }; -using TestObject = UserObject; +struct TestProperties +{ + float someFloat = 0.0F; +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("some_float", [](auto &object) { return &object.someFloat; }); + return builder.build(); + } +}; + +struct TestObject +{ + ObjectId id; + TestSettings settings; +}; + +template<> +struct ObjectReflector +{ + using Settings = TestSettings; + + static std::string getType() + { + return "test"; + } + + static TestProperties getProperties(const TestObject &object) + { + return {static_cast(object.settings.someInt)}; + } + + static std::size_t getSize(const TestObject &object) + { + return object.settings.someString.size(); + } + + static void remove(TestObject &object) + { + object.id = nullId; + } +}; + +void registerTestObject(ObjectManager &objects) +{ + objects.addFactory([](auto params) { return TestObject{params.id, std::move(params.settings)}; }); +} } TEST_CASE("Create and remove objects") { auto objects = ObjectManager(); + registerTestObject(objects); - auto &object = objects.create({"type", {123, "123"}, "someUserData"}); - CHECK_EQ(object.metadata.id, 1); - CHECK_EQ(object.metadata.type, "type"); - CHECK_EQ(object.metadata.userData.extract(), "someUserData"); - CHECK_EQ(object.properties.someInt, 123); - CHECK_EQ(object.properties.someString, "123"); + auto id = objects.create({2, "data"}, "someUserData"); - auto &another = objects.create({"type", {}}); - CHECK_EQ(another.metadata.id, 2); + CHECK_EQ(id, 1); - CHECK_EQ(objects.getAllMetadata().size(), 2); + auto metadata = objects.getMetadata(id); - CHECK_EQ(&objects.get(1), &object); + CHECK_EQ(metadata.id, id); + CHECK_EQ(metadata.type, "test"); + CHECK_EQ(metadata.size, 4); + CHECK_EQ(metadata.userData.extract(), "someUserData"); - CHECK_EQ(&objects.getMetadata(1), &object.metadata); + auto properties = objects.getProperties(id); - auto shared = objects.getShared(1); + CHECK_EQ(properties.someFloat, 2.0F); - objects.remove(1); + auto &object = objects.get(id); - CHECK_EQ(shared->metadata.id, nullId); + CHECK_EQ(object.id, id); + CHECK_EQ(object.settings.someInt, 2); + CHECK_EQ(object.settings.someString, "data"); - CHECK_THROWS_AS(objects.getMetadata(1), InvalidParams); + auto shared = objects.getShared(id); + + auto id2 = objects.create({3, "data2"}, "someUserData2"); + + CHECK_EQ(objects.getAllMetadata().size(), 2); + + objects.remove(id); + + CHECK_EQ(shared->id, nullId); + + CHECK_THROWS_AS(objects.getMetadata(id), InvalidParams); + CHECK_THROWS_AS(objects.getProperties(id), InvalidParams); + CHECK_THROWS_AS(objects.getResult(id), InvalidParams); + + auto shared2 = objects.getShared(id2); objects.clear(); + CHECK_EQ(shared2->id, nullId); + CHECK(objects.getAllMetadata().empty()); } TEST_CASE("Errors") { auto objects = ObjectManager(); + registerTestObject(objects); - objects.create({"type", {}}); + objects.create({}); CHECK_THROWS_AS(objects.getMetadata(0), InvalidParams); CHECK_THROWS_AS(objects.getMetadata(2), InvalidParams); CHECK_THROWS_AS(objects.remove(2), InvalidParams); - CHECK_THROWS_AS(objects.get>(1), InvalidParams); } From 55a3b8396daffbf3de5a814205f82b8bc6f78b51 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Tue, 6 Aug 2024 09:05:47 +0200 Subject: [PATCH 06/12] Refactor payloads. --- src/brayns/core/api/Api.cpp | 8 +- src/brayns/core/api/Api.h | 6 +- src/brayns/core/api/ApiBuilder.h | 90 ++++------------- src/brayns/core/api/Endpoint.h | 8 +- src/brayns/core/api/Progress.cpp | 5 +- src/brayns/core/api/Progress.h | 5 +- src/brayns/core/api/Task.h | 63 +++++++++--- src/brayns/core/jsonrpc/Messages.h | 16 ++-- src/brayns/core/jsonrpc/Parser.cpp | 4 +- .../PayloadReflector.h} | 96 ++++++------------- src/brayns/core/objects/Messages.h | 2 +- src/brayns/core/service/Service.cpp | 8 +- tests/core/api/TestApi.cpp | 16 ++-- tests/core/jsonrpc/TestJsonRpc.cpp | 8 +- 14 files changed, 139 insertions(+), 196 deletions(-) rename src/brayns/core/{api/ApiReflector.h => jsonrpc/PayloadReflector.h} (56%) diff --git a/src/brayns/core/api/Api.cpp b/src/brayns/core/api/Api.cpp index 5403ab229..d3515a8af 100644 --- a/src/brayns/core/api/Api.cpp +++ b/src/brayns/core/api/Api.cpp @@ -30,7 +30,7 @@ namespace { using namespace brayns; -const RawTask &getRawTask(const std::map &tasks, TaskId id) +const TaskInterface &getRawTask(const std::map &tasks, TaskId id) { auto i = tasks.find(id); @@ -42,7 +42,7 @@ const RawTask &getRawTask(const std::map &tasks, TaskId id) return i->second; } -TaskId addTask(RawTask task, std::map &tasks, IdGenerator ids) +TaskId addTask(TaskInterface task, std::map &tasks, IdGenerator ids) { auto id = ids.next(); assert(!tasks.contains(id)); @@ -101,7 +101,7 @@ const EndpointSchema &Api::getSchema(const std::string &method) const return i->second.schema; } -RawResult Api::execute(const std::string &method, RawParams params) +Payload Api::execute(const std::string &method, Payload params) { auto i = _endpoints.find(method); @@ -161,7 +161,7 @@ TaskInfo Api::getTask(TaskId id) const return {id, operationCount, std::move(currentOperation)}; } -RawResult Api::waitForTaskResult(TaskId id) +Payload Api::waitForTaskResult(TaskId id) { const auto &task = getRawTask(_tasks, id); diff --git a/src/brayns/core/api/Api.h b/src/brayns/core/api/Api.h index d4bb86f52..23d470f10 100644 --- a/src/brayns/core/api/Api.h +++ b/src/brayns/core/api/Api.h @@ -43,15 +43,15 @@ class Api std::vector getMethods() const; const EndpointSchema &getSchema(const std::string &method) const; - RawResult execute(const std::string &method, RawParams params); + Payload execute(const std::string &method, Payload params); std::vector getTasks() const; TaskInfo getTask(TaskId id) const; - RawResult waitForTaskResult(TaskId id); + Payload waitForTaskResult(TaskId id); void cancelTask(TaskId id); private: std::map _endpoints; - std::map _tasks; + std::map _tasks; IdGenerator _ids; }; } diff --git a/src/brayns/core/api/ApiBuilder.h b/src/brayns/core/api/ApiBuilder.h index 7719ab76f..57c8d3af0 100644 --- a/src/brayns/core/api/ApiBuilder.h +++ b/src/brayns/core/api/ApiBuilder.h @@ -22,53 +22,28 @@ #pragma once #include -#include #include #include #include #include +#include #include #include "Api.h" -#include "ApiReflector.h" #include "Endpoint.h" #include "Task.h" namespace brayns { template -using GetParamsType = std::decay_t>; +using GetParams = std::decay_t>; template -using GetResultType = std::decay_t>; - -template -struct Task -{ - std::size_t operationCount; - std::function getCurrentOperation; - std::function wait; - std::function cancel; -}; - -template -struct TaskReflector; - -template -struct TaskReflector> -{ - using Result = T; -}; - -template -concept ReflectedTask = requires { typename TaskReflector::Result; }; - -template -using GetTaskResult = typename TaskReflector::Result; +using GetResult = std::decay_t>; template -concept WithReflectedParams = getArgCount == 1 && ApiReflected>; +concept WithReflectedParams = getArgCount == 1 && ReflectedPayload>; template concept WithoutParams = getArgCount == 0; @@ -77,19 +52,19 @@ template concept WithParams = WithReflectedParams || WithoutParams; template -concept WithReflectedResult = ApiReflected>; +concept WithReflectedResult = ReflectedPayload>; template -concept WithoutResult = std::is_void_v>; +concept WithoutResult = std::is_void_v>; template concept WithResult = WithReflectedResult || WithoutResult; template -concept WithReflectedTaskResult = ApiReflected>>; +concept WithReflectedTaskResult = ReflectedPayload>>; template -concept WithoutTaskResult = std::is_void_v>>; +concept WithoutTaskResult = std::is_void_v>>; template concept WithTaskResult = WithReflectedTaskResult || WithoutTaskResult; @@ -123,7 +98,7 @@ auto ensureHasResult(WithReflectedResult auto handler) auto ensureHasResult(WithoutResult auto handler) { - using Params = GetParamsType; + using Params = GetParams; return [handler = std::move(handler)](Params params) { @@ -139,7 +114,7 @@ auto ensureHasTaskResult(WithReflectedTaskResult auto handler) auto ensureHasTaskResult(WithoutTaskResult auto handler) { - using Params = GetParamsType; + using Params = GetParams; return [handler = std::move(handler)](Params params) { @@ -160,10 +135,10 @@ auto ensureHasTaskResult(WithoutTaskResult auto handler) }; } -template -RawTask addParsingToTask(Task task) +template +TaskInterface addParsingToTask(Task task) { - using ResultReflector = ApiReflector; + using ResultReflector = PayloadReflector; return { .operationCount = task.operationCount, @@ -176,7 +151,7 @@ RawTask addParsingToTask(Task task) template AsyncEndpointHandler addParsingToAsyncHandler(T handler) { - using ParamsReflector = ApiReflector>; + using ParamsReflector = PayloadReflector>; return [handler = std::move(handler)](auto rawParams) { @@ -189,8 +164,8 @@ AsyncEndpointHandler addParsingToAsyncHandler(T handler) template EndpointSchema reflectAsyncEndpointSchema(std::string method) { - using ParamsReflector = ApiReflector>; - using ResultReflector = ApiReflector>>; + using ParamsReflector = PayloadReflector>; + using ResultReflector = PayloadReflector>>; return { .method = std::move(method), @@ -203,8 +178,8 @@ EndpointSchema reflectAsyncEndpointSchema(std::string method) template SyncEndpointHandler addParsingToSyncHandler(T handler) { - using ParamsReflector = ApiReflector>; - using ResultReflector = ApiReflector>; + using ParamsReflector = PayloadReflector>; + using ResultReflector = PayloadReflector>; return [handler = std::move(handler)](auto rawParams) { @@ -217,8 +192,8 @@ SyncEndpointHandler addParsingToSyncHandler(T handler) template EndpointSchema reflectSyncEndpointSchema(std::string method) { - using ParamsReflector = ApiReflector>; - using ResultReflector = ApiReflector>; + using ParamsReflector = PayloadReflector>; + using ResultReflector = PayloadReflector>; return { .method = std::move(method), @@ -227,31 +202,6 @@ EndpointSchema reflectSyncEndpointSchema(std::string method) }; } -struct TaskSettings -{ - std::size_t operationCount; - std::string initialOperation; -}; - -template Handler> -Task> startTask( - Handler handler, - ParamsType params, - TaskSettings settings) -{ - auto monitor = std::make_shared(settings.operationCount, std::move(settings.initialOperation)); - - auto future = std::async(std::launch::async, std::move(handler), Progress(monitor), std::move(params)); - auto shared = std::make_shared(std::move(future)); - - return { - .operationCount = settings.operationCount, - .getCurrentOperation = [=] { return monitor->getCurrentOperation(); }, - .wait = [=] { return shared->get(); }, - .cancel = [=] { monitor->cancel(); }, - }; -} - class EndpointBuilder { public: diff --git a/src/brayns/core/api/Endpoint.h b/src/brayns/core/api/Endpoint.h index 1c60776f0..d0ac30468 100644 --- a/src/brayns/core/api/Endpoint.h +++ b/src/brayns/core/api/Endpoint.h @@ -56,14 +56,14 @@ struct JsonObjectReflector .description("JSON schema of the method result"); builder.field("async", [](auto &object) { return &object.async; }) .description( - "If true, the endpoint does not return its result directly but instead an object {\"task_id\": ID}. " - "This ID can be used to get the method result, cancel it or get its progress"); + "If true, the endpoint does not return its result directly but instead an object {\"task_id\": }. " + "This ID can be used to get the task result, cancel it or get its progress"); return builder.build(); } }; -using SyncEndpointHandler = std::function; -using AsyncEndpointHandler = std::function; +using SyncEndpointHandler = std::function; +using AsyncEndpointHandler = std::function; using EndpointHandler = std::variant; struct Endpoint diff --git a/src/brayns/core/api/Progress.cpp b/src/brayns/core/api/Progress.cpp index b9dbc1fc1..923bd813b 100644 --- a/src/brayns/core/api/Progress.cpp +++ b/src/brayns/core/api/Progress.cpp @@ -30,9 +30,8 @@ TaskCancelledException::TaskCancelledException(): { } -TaskMonitor::TaskMonitor(std::size_t operationCount, std::string initialOperation): - _operationCount(operationCount), - _currentOperation{std::move(initialOperation)} +TaskMonitor::TaskMonitor(std::size_t operationCount): + _operationCount(operationCount) { } diff --git a/src/brayns/core/api/Progress.h b/src/brayns/core/api/Progress.h index f35820df8..560cb0942 100644 --- a/src/brayns/core/api/Progress.h +++ b/src/brayns/core/api/Progress.h @@ -23,7 +23,6 @@ #include #include -#include #include #include @@ -33,7 +32,7 @@ namespace brayns { struct TaskOperation { - std::string description; + std::string description = "Task startup"; float completion = 0.0F; std::size_t index = 0; }; @@ -63,7 +62,7 @@ class TaskCancelledException : public JsonRpcException class TaskMonitor { public: - explicit TaskMonitor(std::size_t operationCount, std::string initialOperation); + explicit TaskMonitor(std::size_t operationCount); std::size_t getOperationCount() const; TaskOperation getCurrentOperation(); diff --git a/src/brayns/core/api/Task.h b/src/brayns/core/api/Task.h index 936f2615d..31f866052 100644 --- a/src/brayns/core/api/Task.h +++ b/src/brayns/core/api/Task.h @@ -21,32 +21,24 @@ #pragma once +#include #include +#include #include +#include #include +#include #include "Progress.h" namespace brayns { -struct RawParams -{ - JsonValue json; - std::string binary = {}; -}; - -struct RawResult -{ - JsonValue json; - std::string binary = {}; -}; - -struct RawTask +struct TaskInterface { std::size_t operationCount; std::function getCurrentOperation; - std::function wait; + std::function wait; std::function cancel; }; @@ -90,4 +82,47 @@ struct JsonObjectReflector return builder.build(); } }; + +template +struct Task +{ + std::size_t operationCount; + std::function getCurrentOperation; + std::function wait; + std::function cancel; +}; + +template +struct TaskReflector; + +template +struct TaskReflector> +{ + using Result = T; +}; + +template +concept ReflectedTask = requires { typename TaskReflector::Result; }; + +template +using GetTaskResult = typename TaskReflector::Result; + +template Handler> +Task> startTask( + Handler handler, + ParamsType params, + std::size_t operationCount) +{ + auto monitor = std::make_shared(operationCount); + + auto future = std::async(std::launch::async, std::move(handler), Progress(monitor), std::move(params)); + auto shared = std::make_shared(std::move(future)); + + return { + .operationCount = operationCount, + .getCurrentOperation = [=] { return monitor->getCurrentOperation(); }, + .wait = [=] { return shared->get(); }, + .cancel = [=] { monitor->cancel(); }, + }; +} } diff --git a/src/brayns/core/jsonrpc/Messages.h b/src/brayns/core/jsonrpc/Messages.h index b675b9484..f3b3c45e2 100644 --- a/src/brayns/core/jsonrpc/Messages.h +++ b/src/brayns/core/jsonrpc/Messages.h @@ -48,11 +48,16 @@ inline std::string toString(const std::optional &id) return id ? toString(*id) : "null"; } +struct Payload +{ + JsonValue json; + std::string binary = {}; +}; + struct JsonRpcRequest { std::string method; - JsonValue params; - std::string binary = {}; + Payload params; std::optional id = {}; }; @@ -64,7 +69,7 @@ struct JsonObjectReflector auto builder = JsonBuilder(); builder.constant("jsonrpc", "2.0"); builder.field("method", [](auto &object) { return &object.method; }); - builder.field("params", [](auto &object) { return &object.params; }).required(false); + builder.field("params", [](auto &object) { return &object.params.json; }).required(false); builder.field("id", [](auto &object) { return &object.id; }); return builder.build(); } @@ -73,8 +78,7 @@ struct JsonObjectReflector struct JsonRpcSuccessResponse { JsonRpcId id; - JsonValue result; - std::string binary = {}; + Payload result; }; template<> @@ -85,7 +89,7 @@ struct JsonObjectReflector auto builder = JsonBuilder(); builder.constant("jsonrpc", "2.0"); builder.field("id", [](auto &object) { return &object.id; }); - builder.field("result", [](auto &object) { return &object.result; }); + builder.field("result", [](auto &object) { return &object.result.json; }); return builder.build(); } }; diff --git a/src/brayns/core/jsonrpc/Parser.cpp b/src/brayns/core/jsonrpc/Parser.cpp index e388ece8f..93857ff59 100644 --- a/src/brayns/core/jsonrpc/Parser.cpp +++ b/src/brayns/core/jsonrpc/Parser.cpp @@ -75,7 +75,7 @@ JsonRpcRequest parseBinaryJsonRpcRequest(const std::string &binary) auto request = parseTextJsonRpcRequest(std::string(text)); - request.binary = binary.substr(4 + textSize); + request.params.binary = binary.substr(4 + textSize); return request; } @@ -103,7 +103,7 @@ std::string composeAsBinary(const JsonRpcSuccessResponse &response) auto header = composeBytes(static_cast(textSize), std::endian::little); - return header + text + response.binary; + return header + text + response.result.binary; } JsonRpcError composeError(const JsonRpcException &e) diff --git a/src/brayns/core/api/ApiReflector.h b/src/brayns/core/jsonrpc/PayloadReflector.h similarity index 56% rename from src/brayns/core/api/ApiReflector.h rename to src/brayns/core/jsonrpc/PayloadReflector.h index df278da77..9a942443b 100644 --- a/src/brayns/core/api/ApiReflector.h +++ b/src/brayns/core/jsonrpc/PayloadReflector.h @@ -26,130 +26,88 @@ #include #include - -#include "Task.h" +#include +#include namespace brayns { template -struct ApiReflector; +struct PayloadReflector; template -concept ApiReflected = std::same_as::getSchema())> - && std::same_as::serialize(std::declval()))> - && std::same_as::deserialize(RawParams()))>; - -template -struct ApiReflector -{ - static JsonSchema getSchema() - { - return getJsonSchema(); - } - - static RawResult serialize(T value) - { - return {serializeToJson(value)}; - } - - static T deserialize(RawParams params) - { - if (!params.binary.empty()) - { - throw InvalidParams("This endpoint does not accept additional binary data"); - } - - return deserializeAs(params.json); - } -}; - -template -struct Params -{ - T value; - std::string binary = {}; -}; +concept ReflectedPayload = std::same_as::getSchema())> + && std::same_as::serialize(std::declval()))> + && std::same_as::deserialize(Payload()))>; template<> -struct ApiReflector +struct PayloadReflector { static JsonSchema getSchema() { return getJsonSchema(); } - static RawResult serialize(RawParams) + static Payload serialize(Payload result) { - throw std::invalid_argument("Cannot serialize params"); + return result; } - static RawParams deserialize(RawParams params) + static Payload deserialize(Payload params) { return params; } }; template -struct ApiReflector> +struct PayloadReflector { static JsonSchema getSchema() { return getJsonSchema(); } - static RawResult serialize(Params) + static Payload serialize(T result) { - throw std::invalid_argument("Cannot serialize params"); + return {serializeToJson(result)}; } - static Params deserialize(RawParams params) + static T deserialize(Payload params) { - return {deserializeAs(params.json), std::move(params.binary)}; + if (!params.binary.empty()) + { + throw InvalidParams("This endpoint does not accept additional binary data"); + } + + return deserializeAs(params.json); } }; template -struct Result +struct Params { T value; std::string binary = {}; }; -template<> -struct ApiReflector -{ - static JsonSchema getSchema() - { - return getJsonSchema(); - } - - static RawResult serialize(RawResult value) - { - return value; - } - - static RawResult deserialize(RawParams) - { - throw std::invalid_argument("Cannot deserialize result"); - } -}; +template +using Result = Params; template -struct ApiReflector> +struct PayloadReflector> { static JsonSchema getSchema() { return getJsonSchema(); } - static RawResult serialize(Result result) + static Payload serialize(Params result) { return {serializeToJson(result.value), std::move(result.binary)}; } - static Result deserialize(RawParams) + static Params deserialize(Payload params) { - throw std::invalid_argument("Cannot deserialize result"); + return {deserializeAs(params.json), std::move(params.binary)}; } }; } diff --git a/src/brayns/core/objects/Messages.h b/src/brayns/core/objects/Messages.h index a195b2b7a..ea2653907 100644 --- a/src/brayns/core/objects/Messages.h +++ b/src/brayns/core/objects/Messages.h @@ -110,7 +110,7 @@ struct JsonObjectReflector static auto reflect() { auto builder = JsonBuilder(); - builder.description("Can also be an empty object"); + builder.description("An empty object"); return builder.build(); } }; diff --git a/src/brayns/core/service/Service.cpp b/src/brayns/core/service/Service.cpp index 05f74e82d..9aef13df3 100644 --- a/src/brayns/core/service/Service.cpp +++ b/src/brayns/core/service/Service.cpp @@ -41,7 +41,7 @@ JsonRpcRequest parseRequest(const Message &message) Message composeResponse(const JsonRpcSuccessResponse &response) { - if (response.binary.empty()) + if (response.result.binary.empty()) { return {.data = composeAsText(response), .binary = false}; } @@ -59,9 +59,7 @@ std::optional execute(JsonRpcRequest request, Api &api, Logger try { logger.info("Calling endpoint for request {}", toString(request.id)); - auto params = RawParams{std::move(request.params), std::move(request.binary)}; - auto result = api.execute(request.method, std::move(params)); - + auto result = api.execute(request.method, std::move(request.params)); logger.info("Successfully called endpoint"); if (!request.id) @@ -70,7 +68,7 @@ std::optional execute(JsonRpcRequest request, Api &api, Logger return {}; } - return JsonRpcSuccessResponse{*request.id, result.json, std::move(result.binary)}; + return JsonRpcSuccessResponse{*request.id, std::move(result)}; } catch (const JsonRpcException &e) { diff --git a/tests/core/api/TestApi.cpp b/tests/core/api/TestApi.cpp index 0acbc2dbb..52797a12b 100644 --- a/tests/core/api/TestApi.cpp +++ b/tests/core/api/TestApi.cpp @@ -48,7 +48,7 @@ TEST_CASE("Basic") CHECK_EQ(schema.result, getJsonSchema()); CHECK_FALSE(schema.async); - auto params = RawParams{2}; + auto params = Payload{2}; auto result = api.execute("test", params); @@ -75,7 +75,7 @@ TEST_CASE("With binary") auto api = builder.build(); - auto params = RawParams{1, "123"}; + auto params = Payload{1, "123"}; CHECK_THROWS_AS(api.execute("test1", params), InvalidParams); @@ -105,9 +105,9 @@ TEST_CASE("No params or result") CHECK_EQ(api.getSchema("test3").params, getJsonSchema()); CHECK_EQ(api.getSchema("test3").result, getJsonSchema()); - CHECK_EQ(api.execute("test1", RawParams()).json, serializeToJson(0)); - CHECK_EQ(api.execute("test2", RawParams(0)).json, serializeToJson(NullJson())); - CHECK_EQ(api.execute("test3", RawParams()).json, serializeToJson(NullJson())); + CHECK_EQ(api.execute("test1", Payload()).json, serializeToJson(0)); + CHECK_EQ(api.execute("test2", Payload(0)).json, serializeToJson(NullJson())); + CHECK_EQ(api.execute("test3", Payload()).json, serializeToJson(NullJson())); } struct NonCopyable @@ -143,7 +143,7 @@ TEST_CASE("Copy") auto api = builder.build(); - auto params = RawParams{createJsonObject()}; + auto params = Payload{createJsonObject()}; auto result = api.execute("test", params); @@ -174,11 +174,11 @@ TEST_CASE("Task") return value + offset; }; - builder.task("test", [=](int value) { return startTask(worker, value, {2, "Test"}); }); + builder.task("test", [=](int value) { return startTask(worker, value, 2); }); auto api = builder.build(); - auto params = RawParams{2}; + auto params = Payload{2}; auto [json, binary] = api.execute("test", params); diff --git a/tests/core/jsonrpc/TestJsonRpc.cpp b/tests/core/jsonrpc/TestJsonRpc.cpp index 2a10e4228..bc58b8b3e 100644 --- a/tests/core/jsonrpc/TestJsonRpc.cpp +++ b/tests/core/jsonrpc/TestJsonRpc.cpp @@ -42,8 +42,8 @@ TEST_CASE("JsonRpcParser") CHECK(request.id); CHECK_EQ(std::get(*request.id), 1); CHECK_EQ(request.method, "test"); - CHECK_EQ(request.params, 123); - CHECK_EQ(request.binary, ""); + CHECK_EQ(request.params.json, 123); + CHECK_EQ(request.params.binary, ""); } SUBCASE("Binary") { @@ -56,8 +56,8 @@ TEST_CASE("JsonRpcParser") CHECK(request.id); CHECK_EQ(std::get(*request.id), 1); CHECK_EQ(request.method, "test"); - CHECK_EQ(request.params, 123); - CHECK_EQ(request.binary, "binary"); + CHECK_EQ(request.params.json, 123); + CHECK_EQ(request.params.binary, "binary"); } SUBCASE("Invalid JSON") { From ea104c886385eb660b0adef0e243542e4b7c45d2 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Tue, 6 Aug 2024 10:45:08 +0200 Subject: [PATCH 07/12] Object cleanup. --- src/brayns/core/endpoints/ObjectEndpoints.cpp | 39 ++++++++++-- src/brayns/core/endpoints/ObjectEndpoints.h | 6 +- src/brayns/core/objects/Messages.h | 6 +- src/brayns/core/objects/ObjectManager.cpp | 51 ++++++++------- src/brayns/core/objects/ObjectManager.h | 62 ++++++++++--------- .../{ObjectReflector.h => UserObject.h} | 60 +++++++++++++----- tests/core/objects/TestObjectManager.cpp | 48 +++++++------- 7 files changed, 166 insertions(+), 106 deletions(-) rename src/brayns/core/objects/{ObjectReflector.h => UserObject.h} (68%) diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index 7e7a5ab91..932065fec 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -40,7 +40,28 @@ struct JsonObjectReflector } }; +struct UpdateObjectParams +{ + ObjectId objectId; + JsonValue userData; +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("object_id", [](auto &object) { return &object.objectId; }) + .description("ID of the object to update"); + builder.field("user_data", [](auto &object) { return &object.userData; }) + .description("New user data to store in the object"); + return builder.build(); + } +}; + Metadatas getAllObjects(LockedObjects &locked) + { return {locked.visit([](auto &objects) { return objects.getAllMetadata(); })}; } @@ -50,6 +71,16 @@ Metadata getObject(LockedObjects &locked, ObjectId id) return locked.visit([&](auto &objects) { return objects.getMetadata(id); }); } +Metadata updateObject(LockedObjects &locked, const UpdateObjectParams ¶ms) +{ + return locked.visit( + [&](auto &objects) + { + objects.setUserData(params.objectId, params.userData); + return objects.getMetadata(params.objectId); + }); +} + void removeSelectedObjects(LockedObjects &locked, const std::vector &ids) { locked.visit( @@ -67,9 +98,7 @@ void clearObjects(LockedObjects &locked) locked.visit([](auto &objects) { objects.clear(); }); } -struct EmptyObject -{ -}; +using EmptyObject = UserObject; template<> struct ObjectReflector @@ -95,7 +124,7 @@ LockedObjects::LockedObjects(ObjectManager objects, Logger &logger): void addDefaultObjects(ObjectManager &objects) { - objects.addFactory([](auto) { return EmptyObject(); }); + objects.addFactory([](auto, auto) { return EmptyJson(); }); } void addObjectEndpoints(ApiBuilder &builder, LockedObjects &locked) @@ -106,6 +135,8 @@ void addObjectEndpoints(ApiBuilder &builder, LockedObjects &locked) builder.endpoint("get-object", [&](ObjectIdParams params) { return getObject(locked, params.id); }) .description("Get generic object properties from given object IDs"); + builder.endpoint("update-object", [&](UpdateObjectParams params) { return updateObject(locked, params); }); + builder.endpoint("remove-objects", [&](ObjectIds params) { removeSelectedObjects(locked, params.ids); }) .description("Remove selected objects from registry (but not from scene)"); diff --git a/src/brayns/core/endpoints/ObjectEndpoints.h b/src/brayns/core/endpoints/ObjectEndpoints.h index b6b8041a0..135167b6a 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.h +++ b/src/brayns/core/endpoints/ObjectEndpoints.h @@ -90,8 +90,8 @@ ObjectResult> createObject(LockedObjects &locked, ObjectParams< return locked.visit( [&](ObjectManager &objects) { - auto id = objects.create(std::move(params.settings), params.userData); - return objects.getResult(id); + auto &object = objects.create(std::move(params.settings), params.userData); + return objects.getResult(object.id); }); } @@ -107,7 +107,7 @@ void addCreateAndGet(ApiBuilder &builder, LockedObjects &locked) const auto &type = getObjectType(); builder.endpoint("get-" + type, [&](ObjectIdParams params) { return getObject(locked, params.id); }) - .description("Get all properties properties of given " + type); + .description("Get all properties of " + type + " with given ID"); builder .endpoint( diff --git a/src/brayns/core/objects/Messages.h b/src/brayns/core/objects/Messages.h index ea2653907..140c3b91d 100644 --- a/src/brayns/core/objects/Messages.h +++ b/src/brayns/core/objects/Messages.h @@ -28,13 +28,9 @@ namespace brayns { -using ObjectId = std::uint32_t; - -constexpr auto nullId = ObjectId(0); - struct Metadata { - ObjectId id; + std::uint32_t id; std::string type; std::size_t size = 0; JsonValue userData = {}; diff --git a/src/brayns/core/objects/ObjectManager.cpp b/src/brayns/core/objects/ObjectManager.cpp index f72c6f9d2..d5d6c8107 100644 --- a/src/brayns/core/objects/ObjectManager.cpp +++ b/src/brayns/core/objects/ObjectManager.cpp @@ -47,6 +47,16 @@ auto getStorageIterator(auto &objects, ObjectId id) namespace brayns { +Metadata createMetadata(const ObjectInterface &object) +{ + return { + .id = object.getId(), + .type = object.getType(), + .size = object.getSize(), + .userData = object.getUserData(), + }; +} + ObjectManager::ObjectManager() { disableNullId(_ids); @@ -59,7 +69,7 @@ std::vector ObjectManager::getAllMetadata() const for (const auto &[id, object] : _objects) { - auto metadata = createMetadata(id, object); + auto metadata = createMetadata(object); metadatas.push_back(std::move(metadata)); } @@ -68,16 +78,30 @@ std::vector ObjectManager::getAllMetadata() const Metadata ObjectManager::getMetadata(ObjectId id) const { - const auto &storage = getStorage(id); + const auto &interface = getInterface(id); + + return createMetadata(interface); +} + +const ObjectInterface &ObjectManager::getInterface(ObjectId id) const +{ + auto i = getStorageIterator(_objects, id); - return createMetadata(id, storage); + return i->second; +} + +void ObjectManager::setUserData(ObjectId id, const JsonValue &userData) +{ + auto i = getStorageIterator(_objects, id); + + i->second.setUserData(userData); } void ObjectManager::remove(ObjectId id) { auto i = getStorageIterator(_objects, id); - i->second.remove(); + i->second.removeId(); _objects.erase(i); } @@ -86,7 +110,7 @@ void ObjectManager::clear() { for (const auto &[id, object] : _objects) { - object.remove(); + object.removeId(); } _objects.clear(); @@ -94,21 +118,4 @@ void ObjectManager::clear() _ids = {}; disableNullId(_ids); } - -Metadata ObjectManager::createMetadata(ObjectId id, const ObjectStorage &storage) -{ - return { - .id = id, - .type = storage.type, - .size = storage.getSize(), - .userData = storage.userData, - }; -} - -const ObjectStorage &ObjectManager::getStorage(ObjectId id) const -{ - auto i = getStorageIterator(_objects, id); - - return i->second; -} } diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index 29ca2db0f..c579e38bf 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -36,21 +36,22 @@ #include #include "Messages.h" -#include "ObjectReflector.h" +#include "UserObject.h" namespace brayns { -struct ObjectStorage +struct ObjectInterface { std::any object; - std::string type; - JsonValue userData; + std::function getId; + std::function removeId; + std::function getType; std::function getSize; - std::function remove; + std::function getUserData; + std::function setUserData; }; -template -using ObjectFactory = std::function)>; +Metadata createMetadata(const ObjectInterface &object); class ObjectManager { @@ -59,6 +60,8 @@ class ObjectManager std::vector getAllMetadata() const; Metadata getMetadata(ObjectId id) const; + const ObjectInterface &getInterface(ObjectId id) const; + void setUserData(ObjectId id, const JsonValue &userData); void remove(ObjectId id); void clear(); @@ -78,9 +81,9 @@ class ObjectManager template const std::shared_ptr &getShared(ObjectId id) const { - const auto &storage = getStorage(id); + const auto &interface = getInterface(id); - return castObject(id, storage); + return castObject(id, interface); } template @@ -100,33 +103,34 @@ class ObjectManager template ObjectResult> getResult(ObjectId id) const { - const auto &storage = getStorage(id); - const auto &object = castObject(id, storage); + auto &object = get(id); - return {createMetadata(id, storage), getObjectProperties(*object)}; + return createResult(object); } template - ObjectId create(GetSettings settings, const JsonValue &userData = {}) + T &create(GetSettings settings, const JsonValue &userData = {}) { auto id = _ids.next(); try { - auto object = createObject({id, std::move(settings)}); + auto object = createObject(id, userData, std::move(settings)); auto ptr = std::make_shared(std::move(object)); - auto storage = ObjectStorage{ + auto interface = ObjectInterface{ .object = ptr, - .type = ObjectReflector::getType(), - .userData = std::move(userData), + .getId = [=] { return ptr->id; }, + .removeId = [=] { ptr->id = nullId; }, + .getType = [] { return getObjectType(); }, .getSize = [=] { return getObjectSize(*ptr); }, - .remove = [=] { removeObject(*ptr); }, + .getUserData = [=] { return ptr->userData; }, + .setUserData = [=](const auto &value) { ptr->userData = value; }, }; - _objects.emplace(id, std::move(storage)); + _objects.emplace(id, std::move(interface)); - return id; + return *ptr; } catch (...) { @@ -137,15 +141,13 @@ class ObjectManager private: std::map _factories; - std::map _objects; + std::map _objects; IdGenerator _ids; - static Metadata createMetadata(ObjectId id, const ObjectStorage &storage); - template - static const std::shared_ptr &castObject(ObjectId id, const ObjectStorage &storage) + static const std::shared_ptr &castObject(ObjectId id, const ObjectInterface &interface) { - auto ptr = std::any_cast>(&storage.object); + auto ptr = std::any_cast>(&interface.object); if (ptr != nullptr) { @@ -153,15 +155,13 @@ class ObjectManager } const auto &expected = getObjectType(); - const auto &got = storage.type; + auto got = interface.getType(); throw InvalidParams(fmt::format("Invalid type for object with ID {}: expected {}, got {}", id, expected, got)); } - const ObjectStorage &getStorage(ObjectId id) const; - template - T createObject(const ParamsOf ¶ms) + T createObject(ObjectId id, const JsonValue &userData, GetSettings settings) { auto i = _factories.find(typeid(T)); @@ -172,7 +172,9 @@ class ObjectManager const auto &factory = std::any_cast &>(i->second); - return factory(std::move(params)); + auto value = factory(id, std::move(settings)); + + return {id, std::move(value), userData}; } }; } diff --git a/src/brayns/core/objects/ObjectReflector.h b/src/brayns/core/objects/UserObject.h similarity index 68% rename from src/brayns/core/objects/ObjectReflector.h rename to src/brayns/core/objects/UserObject.h index 98131ea46..7d26ead72 100644 --- a/src/brayns/core/objects/ObjectReflector.h +++ b/src/brayns/core/objects/UserObject.h @@ -22,7 +22,7 @@ #pragma once #include -#include +#include #include #include @@ -31,6 +31,33 @@ namespace brayns { +using ObjectId = std::uint32_t; + +constexpr auto nullId = ObjectId(0); + +template +struct UserObject +{ + ObjectId id; + T value; + JsonValue userData = {}; +}; + +template +struct UserObjectReflector; + +template +struct UserObjectReflector> +{ + using Value = T; +}; + +template +concept ValidUserObject = requires { typename UserObjectReflector::Value; }; + +template +using GetUserObjectValue = typename UserObjectReflector::Value; + template struct ObjectReflector; @@ -50,21 +77,11 @@ template concept WithProperties = ReflectedJson>; template -concept ReflectedObject = WithSettings && WithType && WithProperties; - -template -struct ParamsOf -{ - ObjectId id; - GetSettings settings; -}; +concept ReflectedObject = ValidUserObject && WithSettings && WithType && WithProperties; template concept WithSize = std::convertible_to::getSize(std::declval())), std::size_t>; -template -concept WithRemove = std::same_as::remove(std::declval())), void>; - template const std::string &getObjectType() { @@ -90,14 +107,23 @@ std::size_t getObjectSize(const T &object) return ObjectReflector::getSize(object); } -void removeObject(auto &object) +template +using ObjectFactory = std::function(ObjectId, GetSettings)>; + +template +Metadata createMetadata(const T &object) { - (void)object; + return { + .id = object.id, + .type = getObjectType(), + .size = getObjectSize(object), + .userData = object.userData, + }; } -template -void removeObject(T &object) +template +ObjectResult> createResult(const T &object) { - ObjectReflector::remove(object); + return {createMetadata(object), getObjectProperties(object)}; } } diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index 2b3b447b1..2006c3aed 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -59,12 +59,14 @@ struct JsonObjectReflector } }; -struct TestObject +struct TestValue { ObjectId id; TestSettings settings; }; +using TestObject = UserObject; + template<> struct ObjectReflector { @@ -77,23 +79,18 @@ struct ObjectReflector static TestProperties getProperties(const TestObject &object) { - return {static_cast(object.settings.someInt)}; + return {static_cast(object.value.settings.someInt)}; } static std::size_t getSize(const TestObject &object) { - return object.settings.someString.size(); - } - - static void remove(TestObject &object) - { - object.id = nullId; + return object.value.settings.someString.size(); } }; void registerTestObject(ObjectManager &objects) { - objects.addFactory([](auto params) { return TestObject{params.id, std::move(params.settings)}; }); + objects.addFactory([](auto id, auto settings) { return TestValue{id, std::move(settings)}; }); } } @@ -102,42 +99,43 @@ TEST_CASE("Create and remove objects") auto objects = ObjectManager(); registerTestObject(objects); - auto id = objects.create({2, "data"}, "someUserData"); + auto &object = objects.create({2, "data"}, "someUserData"); - CHECK_EQ(id, 1); + CHECK_EQ(object.id, 1); - auto metadata = objects.getMetadata(id); + auto metadata = objects.getMetadata(object.id); - CHECK_EQ(metadata.id, id); + CHECK_EQ(metadata.id, object.id); CHECK_EQ(metadata.type, "test"); CHECK_EQ(metadata.size, 4); CHECK_EQ(metadata.userData.extract(), "someUserData"); - auto properties = objects.getProperties(id); + auto properties = objects.getProperties(object.id); CHECK_EQ(properties.someFloat, 2.0F); - auto &object = objects.get(id); + auto &retreived = objects.get(object.id); - CHECK_EQ(object.id, id); - CHECK_EQ(object.settings.someInt, 2); - CHECK_EQ(object.settings.someString, "data"); + CHECK_EQ(retreived.id, object.id); + CHECK_EQ(retreived.userData, object.userData); + CHECK_EQ(object.value.settings.someInt, 2); + CHECK_EQ(object.value.settings.someString, "data"); - auto shared = objects.getShared(id); + auto shared = objects.getShared(object.id); - auto id2 = objects.create({3, "data2"}, "someUserData2"); + auto &object2 = objects.create({3, "data2"}, "someUserData2"); CHECK_EQ(objects.getAllMetadata().size(), 2); - objects.remove(id); + objects.remove(object.id); CHECK_EQ(shared->id, nullId); - CHECK_THROWS_AS(objects.getMetadata(id), InvalidParams); - CHECK_THROWS_AS(objects.getProperties(id), InvalidParams); - CHECK_THROWS_AS(objects.getResult(id), InvalidParams); + CHECK_THROWS_AS(objects.getMetadata(object.id), InvalidParams); + CHECK_THROWS_AS(objects.getProperties(object.id), InvalidParams); + CHECK_THROWS_AS(objects.getResult(object.id), InvalidParams); - auto shared2 = objects.getShared(id2); + auto shared2 = objects.getShared(object2.id); objects.clear(); From 2dc523a485041e31845e9541b7bba370787fc54c Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Tue, 6 Aug 2024 13:16:12 +0200 Subject: [PATCH 08/12] Python object tests. --- python/brayns/__init__.py | 24 +++++----- python/brayns/api/core/objects.py | 47 ++++++++----------- python/tests/integration/core/test_objects.py | 47 ++++++++++++++----- src/brayns/core/endpoints/ObjectEndpoints.cpp | 9 ++-- 4 files changed, 69 insertions(+), 58 deletions(-) diff --git a/python/brayns/__init__.py b/python/brayns/__init__.py index d0d7b48b9..9992bc5d0 100644 --- a/python/brayns/__init__.py +++ b/python/brayns/__init__.py @@ -24,6 +24,15 @@ This package provides an API to interact with Brayns service. """ +from .api.core.objects import ( + Metadata, + clear_objects, + create_empty_object, + get_all_objects, + get_object, + remove_objects, + update_object, +) from .api.core.service import ( Endpoint, Task, @@ -49,17 +58,8 @@ JsonRpcSuccessResponse, ) from .network.websocket import ServiceUnavailable, WebSocketError -from .version import VERSION from .utils.logger import create_logger -from .api.core.objects import ( - Object, - Metadata, - get_object, - get_all_objects, - remove_objects, - clear_objects, - create_object, -) +from .version import VERSION __version__ = VERSION """Version tag of brayns Python package (major.minor.patch).""" @@ -70,7 +70,8 @@ "connect", "Connection", "create_logger", - "create_object", + "update_object", + "create_empty_object", "Endpoint", "FutureResponse", "get_all_objects", @@ -88,7 +89,6 @@ "JsonRpcResponse", "JsonRpcSuccessResponse", "Metadata", - "Object", "remove_objects", "Request", "Response", diff --git a/python/brayns/api/core/objects.py b/python/brayns/api/core/objects.py index ee9ecca95..055dbc298 100644 --- a/python/brayns/api/core/objects.py +++ b/python/brayns/api/core/objects.py @@ -19,57 +19,51 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. from dataclasses import dataclass -from typing import Any, TypeVar +from typing import Any + from brayns.network.connection import Connection -from brayns.utils.parsing import get, check_type +from brayns.utils.parsing import check_type, get @dataclass class Metadata: id: int type: str + size: int user_data: Any -@dataclass -class Object: - metadata: Metadata - - @property - def id(self) -> int: - return self.metadata.id - - @property - def user_data(self) -> Any: - return self.metadata.user_data - - def parse_metadata(message: dict[str, Any]) -> Metadata: return Metadata( id=get(message, "id", int), type=get(message, "type", str), + size=get(message, "size", int), user_data=get(message, "user_data", Any), ) def extract_metadata(result: dict[str, Any]) -> Metadata: - return parse_metadata(result["metadata"]) - + metadata = get(result, "metadata", dict[str, Any]) + return parse_metadata(metadata) -T = TypeVar("T", bound=Object) - -async def get_all_objects(connection: Connection) -> list[Object]: +async def get_all_objects(connection: Connection) -> list[Metadata]: result = await connection.get_result("get-all-objects") check_type(result, dict[str, Any]) objects = get(result, "objects", list[dict[str, Any]]) - return [Object(parse_metadata(item)) for item in objects] + return [parse_metadata(item) for item in objects] -async def get_object(connection: Connection, id: int) -> Object: +async def get_object(connection: Connection, id: int) -> Metadata: result = await connection.get_result("get-object", {"id": id}) check_type(result, dict[str, Any]) - return Object(parse_metadata(result)) + return parse_metadata(result) + + +async def update_object(connection: Connection, id: int, user_data: Any) -> Metadata: + result = await connection.get_result("update-object", {"id": id, "user_data": user_data}) + check_type(result, dict[str, Any]) + return parse_metadata(result) async def remove_objects(connection: Connection, ids: list[int]) -> None: @@ -80,10 +74,7 @@ async def clear_objects(connection: Connection) -> None: await connection.get_result("clear-objects") -async def create_object(connection: Connection, user_data: Any = None) -> Object: +async def create_empty_object(connection: Connection, user_data: Any = None) -> Metadata: result = await connection.get_result("create-empty-object", {"user_data": user_data}) check_type(result, dict[str, Any]) - return Object(extract_metadata(result)) - - -T = TypeVar("T", bound=Metadata) + return extract_metadata(result) diff --git a/python/tests/integration/core/test_objects.py b/python/tests/integration/core/test_objects.py index e7dded765..a8dbc66b1 100644 --- a/python/tests/integration/core/test_objects.py +++ b/python/tests/integration/core/test_objects.py @@ -18,53 +18,72 @@ # along with this library; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +import pytest + from brayns import ( + Connection, + JsonRpcError, + clear_objects, + create_empty_object, get_all_objects, get_object, - clear_objects, remove_objects, - Connection, - JsonRpcError, - create_object, + update_object, ) -import pytest - @pytest.mark.integration_test @pytest.mark.asyncio -async def test_create_object(connection: Connection) -> None: - obj = await create_object(connection, "test") +async def test_create_empty_object(connection: Connection) -> None: + obj = await create_empty_object(connection, "test") + assert obj.id == 1 - assert obj.metadata.type == "object" + assert obj.type == "empty-object" assert obj.user_data == "test" @pytest.mark.integration_test @pytest.mark.asyncio async def test_get_all_objects(connection: Connection) -> None: - objects = [await create_object(connection) for _ in range(10)] + objects = [await create_empty_object(connection) for _ in range(10)] tests = await get_all_objects(connection) + assert tests == objects @pytest.mark.integration_test @pytest.mark.asyncio async def test_get_object(connection: Connection) -> None: - objects = [await create_object(connection) for _ in range(10)] + objects = [await create_empty_object(connection) for _ in range(10)] tests = [await get_object(connection, obj.id) for obj in objects] + assert tests == objects with pytest.raises(JsonRpcError): await get_object(connection, 123) +@pytest.mark.integration_test +@pytest.mark.asyncio +async def test_update_object(connection: Connection) -> None: + objects = [await create_empty_object(connection) for _ in range(10)] + + test = await update_object(connection, objects[0].id, "test") + + assert test.id == objects[0].id + assert test.type == objects[0].type + assert test.size == objects[0].size + assert test.user_data == "test" + + @pytest.mark.integration_test @pytest.mark.asyncio async def test_remove_objects(connection: Connection) -> None: - objects = [await create_object(connection) for _ in range(10)] + objects = [await create_empty_object(connection) for _ in range(10)] + await remove_objects(connection, [1, 2, 3]) tests = await get_all_objects(connection) + assert [obj.id for obj in tests] == list(range(4, 11)) for obj in objects[:3]: @@ -75,8 +94,10 @@ async def test_remove_objects(connection: Connection) -> None: @pytest.mark.integration_test @pytest.mark.asyncio async def test_clear_objects(connection: Connection) -> None: - objects = [await create_object(connection) for _ in range(10)] + objects = [await create_empty_object(connection) for _ in range(10)] + await clear_objects(connection) + assert not await get_all_objects(connection) for obj in objects: diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index 932065fec..0b4df5286 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -42,7 +42,7 @@ struct JsonObjectReflector struct UpdateObjectParams { - ObjectId objectId; + ObjectId id; JsonValue userData; }; @@ -52,8 +52,7 @@ struct JsonObjectReflector static auto reflect() { auto builder = JsonBuilder(); - builder.field("object_id", [](auto &object) { return &object.objectId; }) - .description("ID of the object to update"); + builder.field("id", [](auto &object) { return &object.id; }).description("ID of the object to update"); builder.field("user_data", [](auto &object) { return &object.userData; }) .description("New user data to store in the object"); return builder.build(); @@ -76,8 +75,8 @@ Metadata updateObject(LockedObjects &locked, const UpdateObjectParams ¶ms) return locked.visit( [&](auto &objects) { - objects.setUserData(params.objectId, params.userData); - return objects.getMetadata(params.objectId); + objects.setUserData(params.id, params.userData); + return objects.getMetadata(params.id); }); } From 113731fa9880a55cacaf3e77b25a4f453d9e9d01 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Wed, 7 Aug 2024 10:31:32 +0200 Subject: [PATCH 09/12] Improved object management. --- src/brayns/core/endpoints/CameraEndpoints.cpp | 150 ++++++++++++++ src/brayns/core/endpoints/CameraEndpoints.h | 32 +++ src/brayns/core/endpoints/ObjectEndpoints.cpp | 60 ++++-- src/brayns/core/endpoints/ObjectEndpoints.h | 92 +-------- src/brayns/core/engine/Camera.cpp | 61 ++++-- src/brayns/core/engine/Camera.h | 23 ++- src/brayns/core/objects/LockedObjects.h | 73 +++++++ src/brayns/core/objects/Messages.h | 22 +- src/brayns/core/objects/ObjectManager.cpp | 34 ++-- src/brayns/core/objects/ObjectManager.h | 83 +++----- src/brayns/core/objects/UserObject.h | 190 ++++++++++++++---- tests/core/engine/TestRender.cpp | 7 +- tests/core/objects/TestObjectManager.cpp | 71 ++++--- 13 files changed, 613 insertions(+), 285 deletions(-) create mode 100644 src/brayns/core/endpoints/CameraEndpoints.cpp create mode 100644 src/brayns/core/endpoints/CameraEndpoints.h create mode 100644 src/brayns/core/objects/LockedObjects.h diff --git a/src/brayns/core/endpoints/CameraEndpoints.cpp b/src/brayns/core/endpoints/CameraEndpoints.cpp new file mode 100644 index 000000000..1c5619b3e --- /dev/null +++ b/src/brayns/core/endpoints/CameraEndpoints.cpp @@ -0,0 +1,150 @@ +/* Copyright (c) 2015-2024 EPFL/Blue Brain Project + * All rights reserved. Do not distribute without permission. + * + * Responsible Author: adrien.fleury@epfl.ch + * + * This file is part of Brayns + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License version 3.0 as published + * by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "CameraEndpoints.h" + +#include +#include + +namespace brayns +{ +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("position", [](auto &object) { return &object.position; }).description("Camera position XYZ"); + builder.field("direction", [](auto &object) { return &object.direction; }) + .description("Camera forward direction XYZ"); + builder.field("up", [](auto &object) { return &object.direction; }) + .description("Camera up direction XYZ") + .defaultValue(Vector3(0.0F, 1.0F, 0.0F)); + builder.field("near_clipping_distance", [](auto &object) { return &object.nearClippingDistance; }) + .description("Distance to clip objects that are too close to the camera") + .defaultValue(0.0F); + return builder.build(); + } +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("fovy", [](auto &object) { return &object.fovy; }) + .description("Camera vertical field of view (horizontal is deduced from framebuffer resolution)") + .defaultValue(45.0F); + return builder.build(); + } +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("height", [](auto &object) { return &object.height; }) + .description("Camera viewport height (horizontal is deduced from framebuffer resolution)"); + return builder.build(); + } +}; + +template +struct CameraParams +{ + CameraView view; + T projection; +}; + +template +struct JsonObjectReflector> +{ + static auto reflect() + { + auto builder = JsonBuilder>(); + builder.field("view", [](auto &object) { return &object.view; }) + .description("Camera view (common to all camera types)"); + builder.field("projection", [](auto &object) { return &object.projection; }) + .description("Camera projection (specific to each camera type)"); + return builder.build(); + } +}; + +/*struct UserCamera +{ + std::any params; + std::function setAspectRatio; +}; + +using PerspectiveParams = CameraParams; +using PerspectiveUserCamera = UserCamera; + +template +struct ObjectReflector> +{ + using Settings = PerspectiveParams; + + static std::string getType() + { + return "perspective-camera"; + } + + static PerspectiveParams getProperties(const PerspectiveUserCamera &camera) + { + return camera.value.params; + } +}; + +using OrthographicParams = CameraParams; +using OrthographicUserCamera = UserCamera; + +template<> +struct ObjectReflector +{ + using Settings = OrthographicParams; + + static std::string getType() + { + return "orthographic-camera"; + } + + static OrthographicParams getProperties(const OrthographicUserCamera &camera) + { + return camera.value.params; + } +};*/ + +void addCameraObjects(ObjectManager &objects, Device &device) +{ + (void)objects; + (void)device; +} + +void addCameraEndpoints(ApiBuilder &builder, LockedObjects &objects, Device &device) +{ + (void)builder; + (void)objects; + (void)device; +} +} diff --git a/src/brayns/core/endpoints/CameraEndpoints.h b/src/brayns/core/endpoints/CameraEndpoints.h new file mode 100644 index 000000000..3d3277b8e --- /dev/null +++ b/src/brayns/core/endpoints/CameraEndpoints.h @@ -0,0 +1,32 @@ +/* Copyright (c) 2015-2024 EPFL/Blue Brain Project + * All rights reserved. Do not distribute without permission. + * + * Responsible Author: adrien.fleury@epfl.ch + * + * This file is part of Brayns + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License version 3.0 as published + * by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include +#include + +namespace brayns +{ +void addCameraObjects(ObjectManager &objects, Device &device); +void addCameraEndpoints(ApiBuilder &builder, LockedObjects &objects, Device &device); +} diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index 0b4df5286..f5ded290c 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -23,19 +23,19 @@ namespace brayns { -struct Metadatas +struct AllObjectsResult { std::vector objects; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); + auto builder = JsonBuilder(); builder.field("objects", [](auto &object) { return &object.objects; }) - .description("List of object generic properties"); + .description("Metadata of all objects in registry"); return builder.build(); } }; @@ -59,8 +59,23 @@ struct JsonObjectReflector } }; -Metadatas getAllObjects(LockedObjects &locked) +struct RemoveParams +{ + std::vector ids; +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("ids", [](auto &object) { return &object.ids; }).description("IDs of the objects to remove"); + return builder.build(); + } +}; +AllObjectsResult getAllObjects(LockedObjects &locked) { return {locked.visit([](auto &objects) { return objects.getAllMetadata(); })}; } @@ -97,14 +112,16 @@ void clearObjects(LockedObjects &locked) locked.visit([](auto &objects) { objects.clear(); }); } -using EmptyObject = UserObject; +struct EmptyObject +{ +}; template<> struct ObjectReflector { using Settings = EmptyJson; - static std::string getType() + static std::string getType(const EmptyObject &) { return "empty-object"; } @@ -115,33 +132,34 @@ struct ObjectReflector } }; -LockedObjects::LockedObjects(ObjectManager objects, Logger &logger): - _objects(std::move(objects)), - _logger(&logger) -{ -} - void addDefaultObjects(ObjectManager &objects) { - objects.addFactory([](auto, auto) { return EmptyJson(); }); + objects.addFactory([](auto, auto) { return EmptyObject(); }); } -void addObjectEndpoints(ApiBuilder &builder, LockedObjects &locked) +void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects) { - builder.endpoint("get-all-objects", [&] { return getAllObjects(locked); }) + builder.endpoint("get-all-objects", [&] { return getAllObjects(objects); }) .description("Get generic properties of all objects, use get-{type} to get details of an object"); - builder.endpoint("get-object", [&](ObjectIdParams params) { return getObject(locked, params.id); }) + builder.endpoint("get-object", [&](GetObjectParams params) { return getObject(objects, params.id); }) .description("Get generic object properties from given object IDs"); - builder.endpoint("update-object", [&](UpdateObjectParams params) { return updateObject(locked, params); }); + builder.endpoint("update-object", [&](UpdateObjectParams params) { return updateObject(objects, params); }); - builder.endpoint("remove-objects", [&](ObjectIds params) { removeSelectedObjects(locked, params.ids); }) + builder.endpoint("remove-objects", [&](RemoveParams params) { removeSelectedObjects(objects, params.ids); }) .description("Remove selected objects from registry (but not from scene)"); - builder.endpoint("clear-objects", [&] { clearObjects(locked); }) + builder.endpoint("clear-objects", [&] { clearObjects(objects); }) .description("Remove all objects currently in registry"); - addCreateAndGet(builder, locked); + builder + .endpoint( + "create-empty-object", + [&](ParamsOf params) { return objects.create(params); }) + .description("Create an empty object with only metadata"); + + builder.endpoint("get-empty-object", [&](GetObjectParams params) { return objects.get(params); }) + .description("Get empty object with given ID"); } } diff --git a/src/brayns/core/endpoints/ObjectEndpoints.h b/src/brayns/core/endpoints/ObjectEndpoints.h index 135167b6a..d67058e3d 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.h +++ b/src/brayns/core/endpoints/ObjectEndpoints.h @@ -21,101 +21,11 @@ #pragma once -#include -#include -#include -#include - #include -#include -#include +#include namespace brayns { -class LockedObjects -{ -public: - explicit LockedObjects(ObjectManager objects, Logger &logger); - - auto visit(std::invocable auto &&callable) -> decltype(callable(std::declval())) - { - _logger->info("Waiting for object manager lock"); - auto lock = std::lock_guard(_mutex); - _logger->info("Object manager lock acquired"); - - return callable(_objects); - } - -private: - std::mutex _mutex; - ObjectManager _objects; - Logger *_logger; -}; - -struct ObjectIdParams -{ - ObjectId id; -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("id", [](auto &object) { return &object.id; }).description("Objects ID"); - return builder.build(); - } -}; - -struct ObjectIds -{ - std::vector ids; -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("ids", [](auto &object) { return &object.ids; }).description("List of object IDs"); - return builder.build(); - } -}; - -template -ObjectResult> createObject(LockedObjects &locked, ObjectParams> params) -{ - return locked.visit( - [&](ObjectManager &objects) - { - auto &object = objects.create(std::move(params.settings), params.userData); - return objects.getResult(object.id); - }); -} - -template -ObjectResult> getObject(LockedObjects &locked, ObjectId id) -{ - return locked.visit([=](ObjectManager &objects) { return objects.getResult(id); }); -} - -template -void addCreateAndGet(ApiBuilder &builder, LockedObjects &locked) -{ - const auto &type = getObjectType(); - - builder.endpoint("get-" + type, [&](ObjectIdParams params) { return getObject(locked, params.id); }) - .description("Get all properties of " + type + " with given ID"); - - builder - .endpoint( - "create-" + type, - [&](ObjectParams> params) { return createObject(locked, std::move(params)); }) - .description("Create a new " + type + " and returns it"); -} - void addDefaultObjects(ObjectManager &objects); void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects); } diff --git a/src/brayns/core/engine/Camera.cpp b/src/brayns/core/engine/Camera.cpp index 49be463d5..3f302a95c 100644 --- a/src/brayns/core/engine/Camera.cpp +++ b/src/brayns/core/engine/Camera.cpp @@ -25,41 +25,76 @@ namespace { using namespace brayns; -void setCameraParams(OSPCamera handle, const CameraSettings &settings) +void setCameraViewParams(OSPCamera handle, const CameraView &view) { - setObjectParam(handle, "position", settings.position); - setObjectParam(handle, "direction", settings.direction); - setObjectParam(handle, "up", settings.up); - setObjectParam(handle, "nearClip", settings.nearClippingDistance); + setObjectParam(handle, "position", view.position); + setObjectParam(handle, "direction", view.direction); + setObjectParam(handle, "up", view.up); + setObjectParam(handle, "nearClip", view.nearClippingDistance); } } namespace brayns { -PerspectiveCamera createPerspectiveCamera(Device &device, const PerspectiveCameraSettings &settings) +void Camera::setView(const CameraView &view) +{ + auto handle = getHandle(); + setCameraViewParams(handle, view); + commitObject(handle); +} + +void PerspectiveCamera::setFovy(float fovy) +{ + auto handle = getHandle(); + setObjectParam(handle, "fovy", fovy); + commitObject(handle); +} + +void PerspectiveCamera::setAspectRatio(float aspectRatio) +{ + auto handle = getHandle(); + setObjectParam(handle, "aspect", aspectRatio); + commitObject(handle); +} + +PerspectiveCamera createPerspectiveCamera(Device &device, const CameraView &view, const Perspective &perspective) { auto handle = ospNewCamera("perspective"); auto camera = wrapObjectHandleAs(device, handle); - setCameraParams(handle, settings.base); + setCameraViewParams(handle, view); - setObjectParam(handle, "fovy", settings.fovy); - setObjectParam(handle, "aspect", settings.aspectRatio); + setObjectParam(handle, "fovy", perspective.fovy); + setObjectParam(handle, "aspect", perspective.aspectRatio); commitObject(handle); return camera; } -OrthographicCamera createOrthographicCamera(Device &device, const OrthographicCameraSettings &settings) +void OrthographicCamera::setHeight(float height) +{ + auto handle = getHandle(); + setObjectParam(handle, "height", height); + commitObject(handle); +} + +void OrthographicCamera::setAspectRatio(float aspectRatio) +{ + auto handle = getHandle(); + setObjectParam(handle, "aspect", aspectRatio); + commitObject(handle); +} + +OrthographicCamera createOrthographicCamera(Device &device, const CameraView &view, const Viewport &viewport) { auto handle = ospNewCamera("orthographic"); auto camera = wrapObjectHandleAs(device, handle); - setCameraParams(handle, settings.base); + setCameraViewParams(handle, view); - setObjectParam(handle, "height", settings.height); - setObjectParam(handle, "aspect", settings.aspectRatio); + setObjectParam(handle, "height", viewport.height); + setObjectParam(handle, "aspect", viewport.aspectRatio); commitObject(handle); diff --git a/src/brayns/core/engine/Camera.h b/src/brayns/core/engine/Camera.h index 1e3569c47..2859023db 100644 --- a/src/brayns/core/engine/Camera.h +++ b/src/brayns/core/engine/Camera.h @@ -26,7 +26,7 @@ namespace brayns { -struct CameraSettings +struct CameraView { Vector3 position = {0.0F, 0.0F, 0.0F}; Vector3 direction = {0.0F, 0.0F, 1.0F}; @@ -38,11 +38,12 @@ class Camera : public Managed { public: using Managed::Managed; + + void setView(const CameraView &view); }; -struct PerspectiveCameraSettings +struct Perspective { - CameraSettings base = {}; float fovy = 60.0F; float aspectRatio = 1.0F; }; @@ -51,13 +52,18 @@ class PerspectiveCamera : public Camera { public: using Camera::Camera; + + void setFovy(float fovy); + void setAspectRatio(float aspectRatio); }; -PerspectiveCamera createPerspectiveCamera(Device &device, const PerspectiveCameraSettings &settings); +PerspectiveCamera createPerspectiveCamera( + Device &device, + const CameraView &view = {}, + const Perspective &perspective = {}); -struct OrthographicCameraSettings +struct Viewport { - CameraSettings base = {}; float height = 1.0F; float aspectRatio = 1.0F; }; @@ -66,7 +72,10 @@ class OrthographicCamera : public Camera { public: using Camera::Camera; + + void setHeight(float height); + void setAspectRatio(float aspectRatio); }; -OrthographicCamera createOrthographicCamera(Device &device, const OrthographicCameraSettings &settings); +OrthographicCamera createOrthographicCamera(Device &device, const CameraView &view = {}, const Viewport &viewport = {}); } diff --git a/src/brayns/core/objects/LockedObjects.h b/src/brayns/core/objects/LockedObjects.h new file mode 100644 index 000000000..69247c46e --- /dev/null +++ b/src/brayns/core/objects/LockedObjects.h @@ -0,0 +1,73 @@ +/* Copyright (c) 2015-2024 EPFL/Blue Brain Project + * All rights reserved. Do not distribute without permission. + * + * Responsible Author: adrien.fleury@epfl.ch + * + * This file is part of Brayns + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License version 3.0 as published + * by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include + +#include + +#include "ObjectManager.h" + +namespace brayns +{ +class LockedObjects +{ +public: + explicit LockedObjects(ObjectManager objects, Logger &logger): + _objects(std::move(objects)), + _logger(&logger) + { + } + + auto visit(std::invocable auto &&callable) -> decltype(callable(std::declval())) + { + _logger->info("Waiting for object manager lock"); + auto lock = std::lock_guard(_mutex); + _logger->info("Object manager lock acquired"); + + return callable(_objects); + } + + template + ResultOf create(ParamsOf params) + { + return visit( + [&](ObjectManager &objects) + { + auto object = objects.create(std::move(params.settings), params.userData); + return object.getResult(); + }); + } + + template + ResultOf get(GetObjectParams params) + { + return visit([&](ObjectManager &objects) { return objects.getResult(params.id); }); + } + +private: + std::mutex _mutex; + ObjectManager _objects; + Logger *_logger; +}; +} diff --git a/src/brayns/core/objects/Messages.h b/src/brayns/core/objects/Messages.h index 140c3b91d..2c51f05df 100644 --- a/src/brayns/core/objects/Messages.h +++ b/src/brayns/core/objects/Messages.h @@ -28,9 +28,13 @@ namespace brayns { +using ObjectId = std::uint32_t; + +constexpr auto nullId = ObjectId(0); + struct Metadata { - std::uint32_t id; + ObjectId id; std::string type; std::size_t size = 0; JsonValue userData = {}; @@ -112,4 +116,20 @@ struct JsonObjectReflector }; using EmptyJson = std::optional; + +struct GetObjectParams +{ + ObjectId id; +}; + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("id", [](auto &object) { return &object.id; }).description("ID of the object to retreive"); + return builder.build(); + } +}; } diff --git a/src/brayns/core/objects/ObjectManager.cpp b/src/brayns/core/objects/ObjectManager.cpp index d5d6c8107..a84b53614 100644 --- a/src/brayns/core/objects/ObjectManager.cpp +++ b/src/brayns/core/objects/ObjectManager.cpp @@ -47,16 +47,6 @@ auto getStorageIterator(auto &objects, ObjectId id) namespace brayns { -Metadata createMetadata(const ObjectInterface &object) -{ - return { - .id = object.getId(), - .type = object.getType(), - .size = object.getSize(), - .userData = object.getUserData(), - }; -} - ObjectManager::ObjectManager() { disableNullId(_ids); @@ -69,7 +59,7 @@ std::vector ObjectManager::getAllMetadata() const for (const auto &[id, object] : _objects) { - auto metadata = createMetadata(object); + auto metadata = createObjectMetadata(object); metadatas.push_back(std::move(metadata)); } @@ -80,14 +70,7 @@ Metadata ObjectManager::getMetadata(ObjectId id) const { const auto &interface = getInterface(id); - return createMetadata(interface); -} - -const ObjectInterface &ObjectManager::getInterface(ObjectId id) const -{ - auto i = getStorageIterator(_objects, id); - - return i->second; + return createObjectMetadata(interface); } void ObjectManager::setUserData(ObjectId id, const JsonValue &userData) @@ -101,16 +84,18 @@ void ObjectManager::remove(ObjectId id) { auto i = getStorageIterator(_objects, id); - i->second.removeId(); + i->second.remove(); _objects.erase(i); + + _ids.recycle(id); } void ObjectManager::clear() { for (const auto &[id, object] : _objects) { - object.removeId(); + object.remove(); } _objects.clear(); @@ -118,4 +103,11 @@ void ObjectManager::clear() _ids = {}; disableNullId(_ids); } + +const ObjectInterface &ObjectManager::getInterface(ObjectId id) const +{ + auto i = getStorageIterator(_objects, id); + + return i->second; +} } diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index c579e38bf..cd8a89222 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -22,9 +22,7 @@ #pragma once #include -#include #include -#include #include #include #include @@ -40,19 +38,6 @@ namespace brayns { -struct ObjectInterface -{ - std::any object; - std::function getId; - std::function removeId; - std::function getType; - std::function getSize; - std::function getUserData; - std::function setUserData; -}; - -Metadata createMetadata(const ObjectInterface &object); - class ObjectManager { public: @@ -60,7 +45,6 @@ class ObjectManager std::vector getAllMetadata() const; Metadata getMetadata(ObjectId id) const; - const ObjectInterface &getInterface(ObjectId id) const; void setUserData(ObjectId id, const JsonValue &userData); void remove(ObjectId id); void clear(); @@ -78,59 +62,38 @@ class ObjectManager _factories[type] = factory; } - template - const std::shared_ptr &getShared(ObjectId id) const - { - const auto &interface = getInterface(id); - - return castObject(id, interface); - } - template T &get(ObjectId id) const { - return *getShared(id); + return getShared(id)->value; } template - GetProperties getProperties(ObjectId id) const + Stored getStored(ObjectId id) const { - auto &object = get(id); - - return getObjectProperties(object); + return Stored(getShared(id)); } template - ObjectResult> getResult(ObjectId id) const + ResultOf getResult(ObjectId id) const { - auto &object = get(id); - - return createResult(object); + return createObjectResult(*getShared(id)); } template - T &create(GetSettings settings, const JsonValue &userData = {}) + Stored create(GetSettings settings, const JsonValue &userData = {}) { auto id = _ids.next(); try { - auto object = createObject(id, userData, std::move(settings)); - auto ptr = std::make_shared(std::move(object)); - - auto interface = ObjectInterface{ - .object = ptr, - .getId = [=] { return ptr->id; }, - .removeId = [=] { ptr->id = nullId; }, - .getType = [] { return getObjectType(); }, - .getSize = [=] { return getObjectSize(*ptr); }, - .getUserData = [=] { return ptr->userData; }, - .setUserData = [=](const auto &value) { ptr->userData = value; }, - }; + auto object = createUserObject(id, std::move(settings), userData); + auto ptr = std::make_shared>(std::move(object)); + auto interface = createObjectInterface(ptr); _objects.emplace(id, std::move(interface)); - return *ptr; + return Stored(std::move(ptr)); } catch (...) { @@ -144,24 +107,34 @@ class ObjectManager std::map _objects; IdGenerator _ids; + const ObjectInterface &getInterface(ObjectId id) const; + template - static const std::shared_ptr &castObject(ObjectId id, const ObjectInterface &interface) + static const std::shared_ptr> &castObject(const ObjectInterface &interface) { - auto ptr = std::any_cast>(&interface.object); + auto ptr = std::any_cast>>(&interface.value); if (ptr != nullptr) { return *ptr; } - const auto &expected = getObjectType(); - auto got = interface.getType(); + auto id = interface.getId(); + auto type = interface.getType(); + + throw InvalidParams(fmt::format("Invalid type for object with ID {}: {}", id, type)); + } + + template + const std::shared_ptr> &getShared(ObjectId id) const + { + const auto &interface = getInterface(id); - throw InvalidParams(fmt::format("Invalid type for object with ID {}: expected {}, got {}", id, expected, got)); + return castObject(interface); } template - T createObject(ObjectId id, const JsonValue &userData, GetSettings settings) + UserObject createUserObject(ObjectId id, GetSettings settings, const JsonValue &userData) { auto i = _factories.find(typeid(T)); @@ -172,9 +145,9 @@ class ObjectManager const auto &factory = std::any_cast &>(i->second); - auto value = factory(id, std::move(settings)); + auto object = factory(id, std::move(settings)); - return {id, std::move(value), userData}; + return {id, std::move(object), userData}; } }; } diff --git a/src/brayns/core/objects/UserObject.h b/src/brayns/core/objects/UserObject.h index 7d26ead72..84fda3f6b 100644 --- a/src/brayns/core/objects/UserObject.h +++ b/src/brayns/core/objects/UserObject.h @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -31,62 +32,43 @@ namespace brayns { -using ObjectId = std::uint32_t; - -constexpr auto nullId = ObjectId(0); - template -struct UserObject -{ - ObjectId id; - T value; - JsonValue userData = {}; -}; - -template -struct UserObjectReflector; - -template -struct UserObjectReflector> -{ - using Value = T; -}; - -template -concept ValidUserObject = requires { typename UserObjectReflector::Value; }; +struct ObjectReflector; template -using GetUserObjectValue = typename UserObjectReflector::Value; +using GetSettings = typename ObjectReflector::Settings; template -struct ObjectReflector; +concept WithSettings = ReflectedJson>; template -using GetSettings = typename ObjectReflector::Settings; +concept WithType = std::same_as::getType(std::declval()))>; template -concept WithSettings = ReflectedJson>; +concept WithSize = std::same_as::getSize(std::declval()))>; template -concept WithType = std::convertible_to::getType()), std::string>; +concept WithRemove = std::is_void_v::remove(std::declval()))>; template -using GetProperties = std::decay_t::getProperties(std::declval()))>; +using GetProperties = decltype(ObjectReflector::getProperties(std::declval())); template concept WithProperties = ReflectedJson>; template -concept ReflectedObject = ValidUserObject && WithSettings && WithType && WithProperties; +concept ReflectedObject = WithSettings && WithType && WithProperties; -template -concept WithSize = std::convertible_to::getSize(std::declval())), std::size_t>; +template +using ParamsOf = ObjectParams>; + +template +using ResultOf = ObjectResult>; template -const std::string &getObjectType() +std::string getObjectType(const T &object) { - static const std::string type = ObjectReflector::getType(); - return type; + return ObjectReflector::getType(object); } template @@ -95,7 +77,8 @@ GetProperties getObjectProperties(const T &object) return ObjectReflector::getProperties(object); } -std::size_t getObjectSize(const auto &object) +template +std::size_t getObjectSize(const T &object) { (void)object; return 0; @@ -107,23 +90,148 @@ std::size_t getObjectSize(const T &object) return ObjectReflector::getSize(object); } +template +void removeObject(T &object) +{ + (void)object; +} + +template +void removeObject(T &object) +{ + return ObjectReflector::remove(object); +} + template -using ObjectFactory = std::function(ObjectId, GetSettings)>; +struct UserObject +{ + ObjectId id; + T value; + JsonValue userData = {}; +}; template -Metadata createMetadata(const T &object) +void removeStoredObject(UserObject &object) +{ + object.id = nullId; + removeObject(object.value); +} + +template +Metadata createObjectMetadata(const UserObject &object) { return { .id = object.id, - .type = getObjectType(), - .size = getObjectSize(object), + .type = getObjectType(object.value), + .size = getObjectSize(object.value), .userData = object.userData, }; } template -ObjectResult> createResult(const T &object) +ResultOf createObjectResult(const UserObject &object) +{ + return {createObjectMetadata(object), getObjectProperties(object.value)}; +} + +template +class Stored { - return {createMetadata(object), getObjectProperties(object)}; +public: + explicit Stored(std::shared_ptr> object): + _object(std::move(object)) + { + } + + ObjectId getId() const + { + return _object->id; + } + + bool isRemoved() const + { + return _object->id == nullId; + } + + std::string getType() const + { + return getObjectType(_object->value); + } + + std::size_t getSize() const + { + return getObjectSize(_object->value); + } + + JsonValue getUserData() const + { + return _object->userData; + } + + void setUserData(const JsonValue &userData) + { + _object->userData = userData; + } + + Metadata getMetadata() const + { + return createObjectMetadata(*_object); + } + + GetProperties getProperties() const + { + return getObjectProperties(*_object); + } + + ResultOf getResult() const + { + return createObjectResult(*_object); + } + + T &get() const + { + return _object->value; + } + +private: + std::shared_ptr> _object; +}; + +template +using ObjectFactory = std::function)>; + +struct ObjectInterface +{ + std::any value; + std::function getId; + std::function remove; + std::function getType; + std::function getSize; + std::function getUserData; + std::function setUserData; +}; + +template +ObjectInterface createObjectInterface(const std::shared_ptr> &object) +{ + return { + .value = object, + .getId = [=] { return object->id; }, + .remove = [=] { removeStoredObject(*object); }, + .getType = [=] { return getObjectType(object->value); }, + .getSize = [=] { return getObjectSize(object->value); }, + .getUserData = [=] { return object->userData; }, + .setUserData = [=](const auto &userData) { object->userData = userData; }, + }; +} + +inline Metadata createObjectMetadata(const ObjectInterface &object) +{ + return { + .id = object.getId(), + .type = object.getType(), + .size = object.getSize(), + .userData = object.getUserData(), + }; } } diff --git a/tests/core/engine/TestRender.cpp b/tests/core/engine/TestRender.cpp index 437585dec..3b1b150a5 100644 --- a/tests/core/engine/TestRender.cpp +++ b/tests/core/engine/TestRender.cpp @@ -149,12 +149,7 @@ TEST_CASE("Render") auto renderer = createScivisRenderer(device, {{.materials = createData(device, {material})}}); - auto camera = createPerspectiveCamera( - device, - { - .fovy = 45.0F, - .aspectRatio = float(width) / float(height), - }); + auto camera = createPerspectiveCamera(device, {}, {.fovy = 45.0F, .aspectRatio = float(width) / float(height)}); auto sphereData = std::vector{{0, 0, 3, 0.25F}, {1, 0, 3, 0.25F}, {1, 1, 3, 0.25F}}; auto colors = std::vector{{1, 0, 0, 1}, {0, 0, 1, 1}, {0, 1, 0, 1}}; diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index 2006c3aed..5dc5096aa 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -59,38 +59,41 @@ struct JsonObjectReflector } }; -struct TestValue +struct TestObject { ObjectId id; TestSettings settings; }; -using TestObject = UserObject; - template<> struct ObjectReflector { using Settings = TestSettings; - static std::string getType() + static std::string getType(const TestObject &object) { - return "test"; + return object.settings.someString; } static TestProperties getProperties(const TestObject &object) { - return {static_cast(object.value.settings.someInt)}; + return {static_cast(object.settings.someInt)}; } static std::size_t getSize(const TestObject &object) { - return object.value.settings.someString.size(); + return object.settings.someString.size(); + } + + static void remove(TestObject &object) + { + object.id = nullId; } }; void registerTestObject(ObjectManager &objects) { - objects.addFactory([](auto id, auto settings) { return TestValue{id, std::move(settings)}; }); + objects.addFactory([](auto id, auto settings) { return TestObject{id, std::move(settings)}; }); } } @@ -99,47 +102,57 @@ TEST_CASE("Create and remove objects") auto objects = ObjectManager(); registerTestObject(objects); - auto &object = objects.create({2, "data"}, "someUserData"); + auto object = objects.create({2, "data"}, "someUserData"); - CHECK_EQ(object.id, 1); + auto id = object.getId(); - auto metadata = objects.getMetadata(object.id); + CHECK_EQ(id, 1); - CHECK_EQ(metadata.id, object.id); - CHECK_EQ(metadata.type, "test"); + auto metadata = objects.getMetadata(id); + + CHECK_EQ(metadata.id, id); + CHECK_EQ(metadata.type, "data"); CHECK_EQ(metadata.size, 4); CHECK_EQ(metadata.userData.extract(), "someUserData"); - auto properties = objects.getProperties(object.id); + auto result = objects.getResult(id); + + CHECK_EQ(result.metadata.id, id); + CHECK_EQ(result.properties.someFloat, 2.0F); - CHECK_EQ(properties.someFloat, 2.0F); + auto &retreived = objects.get(id); - auto &retreived = objects.get(object.id); + CHECK_EQ(retreived.id, id); + CHECK_EQ(retreived.settings.someInt, 2); + CHECK_EQ(retreived.settings.someString, "data"); - CHECK_EQ(retreived.id, object.id); - CHECK_EQ(retreived.userData, object.userData); - CHECK_EQ(object.value.settings.someInt, 2); - CHECK_EQ(object.value.settings.someString, "data"); + auto stored = objects.getStored(id); - auto shared = objects.getShared(object.id); + CHECK_EQ(stored.getId(), id); - auto &object2 = objects.create({3, "data2"}, "someUserData2"); + auto object2 = objects.create({}); + auto id2 = object2.getId(); CHECK_EQ(objects.getAllMetadata().size(), 2); - objects.remove(object.id); + objects.remove(id); + + CHECK(stored.isRemoved()); + CHECK_EQ(stored.get().id, nullId); - CHECK_EQ(shared->id, nullId); + CHECK_THROWS_AS(objects.getMetadata(id), InvalidParams); + CHECK_THROWS_AS(objects.get(id), InvalidParams); + CHECK_THROWS_AS(objects.getResult(id), InvalidParams); - CHECK_THROWS_AS(objects.getMetadata(object.id), InvalidParams); - CHECK_THROWS_AS(objects.getProperties(object.id), InvalidParams); - CHECK_THROWS_AS(objects.getResult(object.id), InvalidParams); + auto object3 = objects.create({}); + CHECK_EQ(object3.getId(), 1); - auto shared2 = objects.getShared(object2.id); + auto stored2 = objects.getStored(id2); objects.clear(); - CHECK_EQ(shared2->id, nullId); + CHECK(stored2.isRemoved()); + CHECK(object3.isRemoved()); CHECK(objects.getAllMetadata().empty()); } From b423d1cfa5d28c5edb706c3a8ed86cc5e6a39163 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Wed, 7 Aug 2024 16:07:54 +0200 Subject: [PATCH 10/12] Still improving objects management. --- src/brayns/core/Launcher.cpp | 9 ++- src/brayns/core/endpoints/CameraEndpoints.cpp | 6 -- src/brayns/core/endpoints/CameraEndpoints.h | 1 - src/brayns/core/endpoints/ObjectEndpoints.cpp | 35 ++++++---- src/brayns/core/endpoints/ObjectEndpoints.h | 1 - src/brayns/core/objects/LockedObjects.h | 17 ----- src/brayns/core/objects/ObjectManager.h | 45 +----------- src/brayns/core/objects/UserObject.h | 47 +------------ tests/core/objects/TestObjectManager.cpp | 68 +++++-------------- 9 files changed, 48 insertions(+), 181 deletions(-) diff --git a/src/brayns/core/Launcher.cpp b/src/brayns/core/Launcher.cpp index 199ddf90f..cb52274d8 100644 --- a/src/brayns/core/Launcher.cpp +++ b/src/brayns/core/Launcher.cpp @@ -23,8 +23,10 @@ #include +#include #include #include +#include #include #include #include @@ -77,13 +79,14 @@ void startServerAndRunService(const ServiceSettings &settings, Logger &logger) addServiceEndpoints(builder, api, token); auto objects = ObjectManager(); - - addDefaultObjects(objects); - auto locked = LockedObjects(std::move(objects), logger); addObjectEndpoints(builder, locked); + auto device = createDevice(logger); + + addCameraEndpoints(builder, locked, device); + api = builder.build(); logger.info("JSON-RCP API ready"); diff --git a/src/brayns/core/endpoints/CameraEndpoints.cpp b/src/brayns/core/endpoints/CameraEndpoints.cpp index 1c5619b3e..c0e4a87d5 100644 --- a/src/brayns/core/endpoints/CameraEndpoints.cpp +++ b/src/brayns/core/endpoints/CameraEndpoints.cpp @@ -135,12 +135,6 @@ struct ObjectReflector } };*/ -void addCameraObjects(ObjectManager &objects, Device &device) -{ - (void)objects; - (void)device; -} - void addCameraEndpoints(ApiBuilder &builder, LockedObjects &objects, Device &device) { (void)builder; diff --git a/src/brayns/core/endpoints/CameraEndpoints.h b/src/brayns/core/endpoints/CameraEndpoints.h index 3d3277b8e..dc53f2f2e 100644 --- a/src/brayns/core/endpoints/CameraEndpoints.h +++ b/src/brayns/core/endpoints/CameraEndpoints.h @@ -27,6 +27,5 @@ namespace brayns { -void addCameraObjects(ObjectManager &objects, Device &device); void addCameraEndpoints(ApiBuilder &builder, LockedObjects &objects, Device &device); } diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index f5ded290c..349d11a66 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -112,6 +112,9 @@ void clearObjects(LockedObjects &locked) locked.visit([](auto &objects) { objects.clear(); }); } +using EmptyParams = ObjectParams; +using EmptyResult = ObjectResult; + struct EmptyObject { }; @@ -119,22 +122,30 @@ struct EmptyObject template<> struct ObjectReflector { - using Settings = EmptyJson; - static std::string getType(const EmptyObject &) { return "empty-object"; } - - static EmptyJson getProperties(const EmptyObject &) - { - return {}; - } }; -void addDefaultObjects(ObjectManager &objects) +ObjectResult createEmptyObject(LockedObjects &locked, const JsonValue &userData) { - objects.addFactory([](auto, auto) { return EmptyObject(); }); + return locked.visit( + [&](auto &objects) + { + auto object = objects.add(EmptyObject(), userData); + return ObjectResult{object.getMetadata(), {}}; + }); +} + +ObjectResult getEmptyObject(LockedObjects &locked, ObjectId id) +{ + return locked.visit( + [&](ObjectManager &objects) + { + auto object = objects.getStored(id); + return ObjectResult(object.getMetadata(), {}); + }); } void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects) @@ -154,12 +165,10 @@ void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects) .description("Remove all objects currently in registry"); builder - .endpoint( - "create-empty-object", - [&](ParamsOf params) { return objects.create(params); }) + .endpoint("create-empty-object", [&](EmptyParams params) { return createEmptyObject(objects, params.userData); }) .description("Create an empty object with only metadata"); - builder.endpoint("get-empty-object", [&](GetObjectParams params) { return objects.get(params); }) + builder.endpoint("get-empty-object", [&](GetObjectParams params) { return getEmptyObject(objects, params.id); }) .description("Get empty object with given ID"); } } diff --git a/src/brayns/core/endpoints/ObjectEndpoints.h b/src/brayns/core/endpoints/ObjectEndpoints.h index d67058e3d..01a8b944b 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.h +++ b/src/brayns/core/endpoints/ObjectEndpoints.h @@ -26,6 +26,5 @@ namespace brayns { -void addDefaultObjects(ObjectManager &objects); void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects); } diff --git a/src/brayns/core/objects/LockedObjects.h b/src/brayns/core/objects/LockedObjects.h index 69247c46e..9efcc36bd 100644 --- a/src/brayns/core/objects/LockedObjects.h +++ b/src/brayns/core/objects/LockedObjects.h @@ -48,23 +48,6 @@ class LockedObjects return callable(_objects); } - template - ResultOf create(ParamsOf params) - { - return visit( - [&](ObjectManager &objects) - { - auto object = objects.create(std::move(params.settings), params.userData); - return object.getResult(); - }); - } - - template - ResultOf get(GetObjectParams params) - { - return visit([&](ObjectManager &objects) { return objects.getResult(params.id); }); - } - private: std::mutex _mutex; ObjectManager _objects; diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index cd8a89222..f5bc243b3 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -24,12 +24,10 @@ #include #include #include -#include #include #include -#include #include #include @@ -49,19 +47,6 @@ class ObjectManager void remove(ObjectId id); void clear(); - template - void addFactory(ObjectFactory factory) - { - const auto &type = typeid(T); - - if (_factories.contains(type)) - { - throw std::invalid_argument("A factory is already registered for given type"); - } - - _factories[type] = factory; - } - template T &get(ObjectId id) const { @@ -75,20 +60,14 @@ class ObjectManager } template - ResultOf getResult(ObjectId id) const - { - return createObjectResult(*getShared(id)); - } - - template - Stored create(GetSettings settings, const JsonValue &userData = {}) + Stored add(T value, const JsonValue &userData = {}) { auto id = _ids.next(); try { - auto object = createUserObject(id, std::move(settings), userData); - auto ptr = std::make_shared>(std::move(object)); + auto object = UserObject{id, std::move(value), userData}; + auto ptr = std::make_shared(std::move(object)); auto interface = createObjectInterface(ptr); _objects.emplace(id, std::move(interface)); @@ -103,7 +82,6 @@ class ObjectManager } private: - std::map _factories; std::map _objects; IdGenerator _ids; @@ -132,22 +110,5 @@ class ObjectManager return castObject(interface); } - - template - UserObject createUserObject(ObjectId id, GetSettings settings, const JsonValue &userData) - { - auto i = _factories.find(typeid(T)); - - if (i == _factories.end()) - { - throw std::invalid_argument("Unsupported object type"); - } - - const auto &factory = std::any_cast &>(i->second); - - auto object = factory(id, std::move(settings)); - - return {id, std::move(object), userData}; - } }; } diff --git a/src/brayns/core/objects/UserObject.h b/src/brayns/core/objects/UserObject.h index 84fda3f6b..b9057466e 100644 --- a/src/brayns/core/objects/UserObject.h +++ b/src/brayns/core/objects/UserObject.h @@ -26,8 +26,6 @@ #include #include -#include - #include "Messages.h" namespace brayns @@ -35,12 +33,6 @@ namespace brayns template struct ObjectReflector; -template -using GetSettings = typename ObjectReflector::Settings; - -template -concept WithSettings = ReflectedJson>; - template concept WithType = std::same_as::getType(std::declval()))>; @@ -51,19 +43,7 @@ template concept WithRemove = std::is_void_v::remove(std::declval()))>; template -using GetProperties = decltype(ObjectReflector::getProperties(std::declval())); - -template -concept WithProperties = ReflectedJson>; - -template -concept ReflectedObject = WithSettings && WithType && WithProperties; - -template -using ParamsOf = ObjectParams>; - -template -using ResultOf = ObjectResult>; +concept ReflectedObject = WithType && std::default_initializable; template std::string getObjectType(const T &object) @@ -71,12 +51,6 @@ std::string getObjectType(const T &object) return ObjectReflector::getType(object); } -template -GetProperties getObjectProperties(const T &object) -{ - return ObjectReflector::getProperties(object); -} - template std::size_t getObjectSize(const T &object) { @@ -128,12 +102,6 @@ Metadata createObjectMetadata(const UserObject &object) }; } -template -ResultOf createObjectResult(const UserObject &object) -{ - return {createObjectMetadata(object), getObjectProperties(object.value)}; -} - template class Stored { @@ -178,16 +146,6 @@ class Stored return createObjectMetadata(*_object); } - GetProperties getProperties() const - { - return getObjectProperties(*_object); - } - - ResultOf getResult() const - { - return createObjectResult(*_object); - } - T &get() const { return _object->value; @@ -197,9 +155,6 @@ class Stored std::shared_ptr> _object; }; -template -using ObjectFactory = std::function)>; - struct ObjectInterface { std::any value; diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index 5dc5096aa..e29cfcfd9 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -25,24 +25,6 @@ using namespace brayns; namespace brayns { -struct TestSettings -{ - int someInt = 0; - std::string someString = {}; -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("some_int", [](auto &object) { return &object.someInt; }); - builder.field("some_string", [](auto &object) { return &object.someString; }); - return builder.build(); - } -}; - struct TestProperties { float someFloat = 0.0F; @@ -61,48 +43,36 @@ struct JsonObjectReflector struct TestObject { - ObjectId id; - TestSettings settings; + std::string type; + std::size_t size; + bool removed = false; }; template<> struct ObjectReflector { - using Settings = TestSettings; - static std::string getType(const TestObject &object) { - return object.settings.someString; - } - - static TestProperties getProperties(const TestObject &object) - { - return {static_cast(object.settings.someInt)}; + return object.type; } static std::size_t getSize(const TestObject &object) { - return object.settings.someString.size(); + return object.size; } static void remove(TestObject &object) { - object.id = nullId; + object.removed = true; } }; - -void registerTestObject(ObjectManager &objects) -{ - objects.addFactory([](auto id, auto settings) { return TestObject{id, std::move(settings)}; }); -} } TEST_CASE("Create and remove objects") { auto objects = ObjectManager(); - registerTestObject(objects); - auto object = objects.create({2, "data"}, "someUserData"); + auto object = objects.add({"type", 4}, "someUserData"); auto id = object.getId(); @@ -111,26 +81,21 @@ TEST_CASE("Create and remove objects") auto metadata = objects.getMetadata(id); CHECK_EQ(metadata.id, id); - CHECK_EQ(metadata.type, "data"); + CHECK_EQ(metadata.type, "type"); CHECK_EQ(metadata.size, 4); CHECK_EQ(metadata.userData.extract(), "someUserData"); - auto result = objects.getResult(id); - - CHECK_EQ(result.metadata.id, id); - CHECK_EQ(result.properties.someFloat, 2.0F); - auto &retreived = objects.get(id); - CHECK_EQ(retreived.id, id); - CHECK_EQ(retreived.settings.someInt, 2); - CHECK_EQ(retreived.settings.someString, "data"); + CHECK_EQ(retreived.size, 4); + CHECK_EQ(retreived.type, "type"); + CHECK_FALSE(retreived.removed); auto stored = objects.getStored(id); CHECK_EQ(stored.getId(), id); - auto object2 = objects.create({}); + auto object2 = objects.add({}); auto id2 = object2.getId(); CHECK_EQ(objects.getAllMetadata().size(), 2); @@ -138,13 +103,13 @@ TEST_CASE("Create and remove objects") objects.remove(id); CHECK(stored.isRemoved()); - CHECK_EQ(stored.get().id, nullId); + CHECK(stored.get().removed); CHECK_THROWS_AS(objects.getMetadata(id), InvalidParams); CHECK_THROWS_AS(objects.get(id), InvalidParams); - CHECK_THROWS_AS(objects.getResult(id), InvalidParams); + CHECK_THROWS_AS(objects.getStored(id), InvalidParams); - auto object3 = objects.create({}); + auto object3 = objects.add({}); CHECK_EQ(object3.getId(), 1); auto stored2 = objects.getStored(id2); @@ -160,9 +125,8 @@ TEST_CASE("Create and remove objects") TEST_CASE("Errors") { auto objects = ObjectManager(); - registerTestObject(objects); - objects.create({}); + objects.add({}); CHECK_THROWS_AS(objects.getMetadata(0), InvalidParams); CHECK_THROWS_AS(objects.getMetadata(2), InvalidParams); From 0e3fae9d419b611a446c90d68db6fb9deb764cf5 Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Thu, 8 Aug 2024 14:02:26 +0200 Subject: [PATCH 11/12] Simplified API. --- src/brayns/core/endpoints/CameraEndpoints.cpp | 270 +++++++++++++++--- src/brayns/core/endpoints/ObjectEndpoints.cpp | 89 +++--- src/brayns/core/engine/Camera.cpp | 20 +- src/brayns/core/engine/Camera.h | 17 +- src/brayns/core/objects/Messages.h | 84 ++---- src/brayns/core/objects/ObjectManager.cpp | 16 +- src/brayns/core/objects/ObjectManager.h | 26 +- src/brayns/core/objects/UserObject.h | 41 +-- tests/core/engine/TestRender.cpp | 2 +- tests/core/objects/TestObjectManager.cpp | 55 ++-- 10 files changed, 349 insertions(+), 271 deletions(-) diff --git a/src/brayns/core/endpoints/CameraEndpoints.cpp b/src/brayns/core/endpoints/CameraEndpoints.cpp index c0e4a87d5..e63eb3cf2 100644 --- a/src/brayns/core/endpoints/CameraEndpoints.cpp +++ b/src/brayns/core/endpoints/CameraEndpoints.cpp @@ -45,31 +45,6 @@ struct JsonObjectReflector } }; -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("fovy", [](auto &object) { return &object.fovy; }) - .description("Camera vertical field of view (horizontal is deduced from framebuffer resolution)") - .defaultValue(45.0F); - return builder.build(); - } -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("height", [](auto &object) { return &object.height; }) - .description("Camera viewport height (horizontal is deduced from framebuffer resolution)"); - return builder.build(); - } -}; - template struct CameraParams { @@ -91,54 +66,259 @@ struct JsonObjectReflector> } }; -/*struct UserCamera +template +struct CameraReflector; + +template T> +using GetProjection = typename CameraReflector::Projection; + +template T> +using CameraParamsOf = CameraParams>; + +template T> +using ProjectionUpdate = UpdateParams>; + +using ViewUpdate = UpdateParams; + +template +concept ReflectedCamera = + ReflectedJson> && std::same_as::getType())> + && std::same_as::create(std::declval(), CameraParamsOf()))> + && std::is_void_v::setAspect(std::declval(), 0.0F))>; + +template +std::string getCameraType() +{ + return CameraReflector::getType(); +} + +template +T createCamera(Device &device, const CameraParamsOf ¶ms) +{ + return CameraReflector::create(device, params); +} + +template +void setCameraAspect(T &camera, float aspect) +{ + CameraReflector::setAspect(camera, aspect); +} + +template +struct UserCamera +{ + T deviceObject; + CameraParamsOf params; +}; + +struct CameraInterface { - std::any params; - std::function setAspectRatio; + std::any value; + std::function getType; + std::function getDeviceObject; + std::function getView; + std::function setView; + std::function setAspect; }; -using PerspectiveParams = CameraParams; -using PerspectiveUserCamera = UserCamera; +template<> +struct ObjectReflector +{ + static std::string getType(const CameraInterface &camera) + { + return camera.getType(); + } +}; -template -struct ObjectReflector> +template +CameraInterface createCameraInterface(const std::shared_ptr> &camera) +{ + return { + .value = camera, + .getType = [] { return getCameraType(); }, + .getDeviceObject = [=] { return camera->deviceObject; }, + .getView = [=] { return camera->params.view; }, + .setView = [=](const auto &view) { camera->params.view = view; }, + .setAspect = [=](auto aspect) { setCameraAspect(camera->deviceObject, aspect); }, + }; +} + +template +UserCamera &castCamera(CameraInterface &camera) +{ + auto ptr = std::any_cast>>(&camera.value); + + if (ptr != nullptr) + { + return **ptr; + } + + auto expected = getCameraType(); + auto got = camera.getType(); + + throw InvalidParams(fmt::format("Invalid camera type: expected {}, got {}", expected, got)); +} + +template +ObjectResult createUserCamera(LockedObjects &locked, Device &device, const CameraParamsOf ¶ms) { - using Settings = PerspectiveParams; + return locked.visit( + [&](ObjectManager &objects) + { + auto camera = createCamera(device, params); + auto object = UserCamera{camera, params}; + auto ptr = std::make_shared(std::move(object)); + + auto interface = createCameraInterface(ptr); + + auto stored = objects.add(std::move(interface)); + + return stored.getResult(); + }); +} + +CameraView getCameraView(LockedObjects &locked, const ObjectParams ¶ms) +{ + return locked.visit( + [&](ObjectManager &objects) + { + auto &camera = objects.get(params.id); + + return camera.getView(); + }); +} + +template +GetProjection getCameraProjection(LockedObjects &locked, const ObjectParams ¶ms) +{ + return locked.visit( + [&](ObjectManager &objects) + { + auto &interface = objects.get(params.id); + auto &camera = castCamera(interface); + + return camera.params.projection; + }); +} + +void updateCameraView(LockedObjects &locked, const ViewUpdate ¶ms) +{ + locked.visit( + [&](ObjectManager &objects) + { + auto &camera = objects.get(params.id); + + camera.setView(params.properties); + }); +} + +template +void updateCameraProjection(LockedObjects &locked, const ProjectionUpdate ¶ms) +{ + locked.visit( + [&](ObjectManager &objects) + { + auto &interface = objects.get(params.id); + auto &camera = castCamera(interface); + + camera.params.projection = params.properties; + }); +} + +template +void addCameraType(ApiBuilder &builder, LockedObjects &objects, Device &device) +{ + auto type = getCameraType(); + + builder + .endpoint( + "create-" + type, + [&](CameraParamsOf params) { return createUserCamera(objects, device, params); }) + .description("Create a camera of type " + type); + + builder.endpoint("get-" + type, [&](ObjectParams params) { return getCameraProjection(objects, params); }) + .description("Get projection part of a camera of type " + type); + + builder.endpoint("update-" + type, [&](ProjectionUpdate params) { updateCameraProjection(objects, params); }) + .description("Update projection part of a camera of type " + type); +} + +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("fovy", [](auto &object) { return &object.fovy; }) + .description("Camera vertical field of view (horizontal is deduced from framebuffer aspect)") + .defaultValue(45.0F); + return builder.build(); + } +}; + +template<> +struct CameraReflector +{ + using Projection = Perspective; static std::string getType() { return "perspective-camera"; } - static PerspectiveParams getProperties(const PerspectiveUserCamera &camera) + static PerspectiveCamera create(Device &device, const CameraParamsOf ¶ms) + { + return createPerspectiveCamera(device, params.view, params.projection); + } + + static void setAspect(PerspectiveCamera &camera, float aspect) { - return camera.value.params; + camera.setAspect(aspect); } }; -using OrthographicParams = CameraParams; -using OrthographicUserCamera = UserCamera; +template<> +struct JsonObjectReflector +{ + static auto reflect() + { + auto builder = JsonBuilder(); + builder.field("height", [](auto &object) { return &object.height; }) + .description("Camera viewport height in world coordinates (horizontal is deduced from framebuffer aspect)"); + return builder.build(); + } +}; template<> -struct ObjectReflector +struct CameraReflector { - using Settings = OrthographicParams; + using Projection = Orthographic; static std::string getType() { return "orthographic-camera"; } - static OrthographicParams getProperties(const OrthographicUserCamera &camera) + static OrthographicCamera create(Device &device, const CameraParamsOf ¶ms) { - return camera.value.params; + return createOrthographicCamera(device, params.view, params.projection); } -};*/ + + static void setAspect(OrthographicCamera &camera, float aspect) + { + camera.setAspect(aspect); + } +}; void addCameraEndpoints(ApiBuilder &builder, LockedObjects &objects, Device &device) { - (void)builder; - (void)objects; - (void)device; + addCameraType(builder, objects, device); + addCameraType(builder, objects, device); + + builder.endpoint("get-camera", [&](ObjectParams params) { return getCameraView(objects, params); }) + .description("Get the view of a camera of any type"); + + builder.endpoint("update-camera", [&](ViewUpdate params) { return updateCameraView(objects, params); }) + .description("Update the view of a camera of any type"); } } diff --git a/src/brayns/core/endpoints/ObjectEndpoints.cpp b/src/brayns/core/endpoints/ObjectEndpoints.cpp index 349d11a66..b5bcff86d 100644 --- a/src/brayns/core/endpoints/ObjectEndpoints.cpp +++ b/src/brayns/core/endpoints/ObjectEndpoints.cpp @@ -25,7 +25,7 @@ namespace brayns { struct AllObjectsResult { - std::vector objects; + std::vector objects; }; template<> @@ -35,30 +35,44 @@ struct JsonObjectReflector { auto builder = JsonBuilder(); builder.field("objects", [](auto &object) { return &object.objects; }) - .description("Metadata of all objects in registry"); + .description("Generic properties of all objects in registry"); return builder.build(); } }; -struct UpdateObjectParams +AllObjectsResult getAllObjects(LockedObjects &locked) +{ + return {locked.visit([](auto &objects) { return objects.getAllObjects(); })}; +} + +ObjectInfo getObject(LockedObjects &locked, const ObjectParams ¶ms) +{ + return locked.visit([&](auto &objects) { return objects.getObject(params.id); }); +} + +struct UserProperties { - ObjectId id; JsonValue userData; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); - builder.field("id", [](auto &object) { return &object.id; }).description("ID of the object to update"); - builder.field("user_data", [](auto &object) { return &object.userData; }) - .description("New user data to store in the object"); + auto builder = JsonBuilder(); + builder.field("user_data", [](auto &object) { return &object.userData; }).description("User data"); return builder.build(); } }; +using ObjectUpdate = UpdateParams; + +void updateObject(LockedObjects &locked, const ObjectUpdate ¶ms) +{ + locked.visit([&](auto &objects) { objects.setUserData(params.id, params.properties.userData); }); +} + struct RemoveParams { std::vector ids; @@ -75,32 +89,12 @@ struct JsonObjectReflector } }; -AllObjectsResult getAllObjects(LockedObjects &locked) -{ - return {locked.visit([](auto &objects) { return objects.getAllMetadata(); })}; -} - -Metadata getObject(LockedObjects &locked, ObjectId id) -{ - return locked.visit([&](auto &objects) { return objects.getMetadata(id); }); -} - -Metadata updateObject(LockedObjects &locked, const UpdateObjectParams ¶ms) -{ - return locked.visit( - [&](auto &objects) - { - objects.setUserData(params.id, params.userData); - return objects.getMetadata(params.id); - }); -} - -void removeSelectedObjects(LockedObjects &locked, const std::vector &ids) +void removeObjects(LockedObjects &locked, const RemoveParams ¶ms) { locked.visit( [&](auto &objects) { - for (auto id : ids) + for (auto id : params.ids) { objects.remove(id); } @@ -112,9 +106,6 @@ void clearObjects(LockedObjects &locked) locked.visit([](auto &objects) { objects.clear(); }); } -using EmptyParams = ObjectParams; -using EmptyResult = ObjectResult; - struct EmptyObject { }; @@ -128,23 +119,13 @@ struct ObjectReflector } }; -ObjectResult createEmptyObject(LockedObjects &locked, const JsonValue &userData) +ObjectResult createEmptyObject(LockedObjects &locked) { return locked.visit( [&](auto &objects) { - auto object = objects.add(EmptyObject(), userData); - return ObjectResult{object.getMetadata(), {}}; - }); -} - -ObjectResult getEmptyObject(LockedObjects &locked, ObjectId id) -{ - return locked.visit( - [&](ObjectManager &objects) - { - auto object = objects.getStored(id); - return ObjectResult(object.getMetadata(), {}); + auto object = objects.add(EmptyObject()); + return object.getResult(); }); } @@ -153,22 +134,18 @@ void addObjectEndpoints(ApiBuilder &builder, LockedObjects &objects) builder.endpoint("get-all-objects", [&] { return getAllObjects(objects); }) .description("Get generic properties of all objects, use get-{type} to get details of an object"); - builder.endpoint("get-object", [&](GetObjectParams params) { return getObject(objects, params.id); }) + builder.endpoint("get-object", [&](ObjectParams params) { return getObject(objects, params); }) .description("Get generic object properties from given object IDs"); - builder.endpoint("update-object", [&](UpdateObjectParams params) { return updateObject(objects, params); }); + builder.endpoint("update-object", [&](ObjectUpdate params) { return updateObject(objects, params); }); - builder.endpoint("remove-objects", [&](RemoveParams params) { removeSelectedObjects(objects, params.ids); }) + builder.endpoint("remove-objects", [&](RemoveParams params) { removeObjects(objects, params); }) .description("Remove selected objects from registry (but not from scene)"); builder.endpoint("clear-objects", [&] { clearObjects(objects); }) .description("Remove all objects currently in registry"); - builder - .endpoint("create-empty-object", [&](EmptyParams params) { return createEmptyObject(objects, params.userData); }) - .description("Create an empty object with only metadata"); - - builder.endpoint("get-empty-object", [&](GetObjectParams params) { return getEmptyObject(objects, params.id); }) - .description("Get empty object with given ID"); + builder.endpoint("create-empty-object", [&] { return createEmptyObject(objects); }) + .description("Create an empty object (for testing or to store user data)"); } } diff --git a/src/brayns/core/engine/Camera.cpp b/src/brayns/core/engine/Camera.cpp index 3f302a95c..d1496f6cf 100644 --- a/src/brayns/core/engine/Camera.cpp +++ b/src/brayns/core/engine/Camera.cpp @@ -50,22 +50,22 @@ void PerspectiveCamera::setFovy(float fovy) commitObject(handle); } -void PerspectiveCamera::setAspectRatio(float aspectRatio) +void PerspectiveCamera::setAspect(float aspect) { auto handle = getHandle(); - setObjectParam(handle, "aspect", aspectRatio); + setObjectParam(handle, "aspect", aspect); commitObject(handle); } -PerspectiveCamera createPerspectiveCamera(Device &device, const CameraView &view, const Perspective &perspective) +PerspectiveCamera createPerspectiveCamera(Device &device, const CameraView &view, const Perspective &projection) { auto handle = ospNewCamera("perspective"); auto camera = wrapObjectHandleAs(device, handle); setCameraViewParams(handle, view); - setObjectParam(handle, "fovy", perspective.fovy); - setObjectParam(handle, "aspect", perspective.aspectRatio); + setObjectParam(handle, "fovy", projection.fovy); + setObjectParam(handle, "aspect", projection.aspect); commitObject(handle); @@ -79,22 +79,22 @@ void OrthographicCamera::setHeight(float height) commitObject(handle); } -void OrthographicCamera::setAspectRatio(float aspectRatio) +void OrthographicCamera::setAspect(float aspect) { auto handle = getHandle(); - setObjectParam(handle, "aspect", aspectRatio); + setObjectParam(handle, "aspect", aspect); commitObject(handle); } -OrthographicCamera createOrthographicCamera(Device &device, const CameraView &view, const Viewport &viewport) +OrthographicCamera createOrthographicCamera(Device &device, const CameraView &view, const Orthographic &projection) { auto handle = ospNewCamera("orthographic"); auto camera = wrapObjectHandleAs(device, handle); setCameraViewParams(handle, view); - setObjectParam(handle, "height", viewport.height); - setObjectParam(handle, "aspect", viewport.aspectRatio); + setObjectParam(handle, "height", projection.height); + setObjectParam(handle, "aspect", projection.aspect); commitObject(handle); diff --git a/src/brayns/core/engine/Camera.h b/src/brayns/core/engine/Camera.h index 2859023db..a71b1901a 100644 --- a/src/brayns/core/engine/Camera.h +++ b/src/brayns/core/engine/Camera.h @@ -45,7 +45,7 @@ class Camera : public Managed struct Perspective { float fovy = 60.0F; - float aspectRatio = 1.0F; + float aspect = 1.0F; }; class PerspectiveCamera : public Camera @@ -54,18 +54,18 @@ class PerspectiveCamera : public Camera using Camera::Camera; void setFovy(float fovy); - void setAspectRatio(float aspectRatio); + void setAspect(float aspect); }; PerspectiveCamera createPerspectiveCamera( Device &device, const CameraView &view = {}, - const Perspective &perspective = {}); + const Perspective &projection = {}); -struct Viewport +struct Orthographic { float height = 1.0F; - float aspectRatio = 1.0F; + float aspect = 1.0F; }; class OrthographicCamera : public Camera @@ -74,8 +74,11 @@ class OrthographicCamera : public Camera using Camera::Camera; void setHeight(float height); - void setAspectRatio(float aspectRatio); + void setAspect(float aspect); }; -OrthographicCamera createOrthographicCamera(Device &device, const CameraView &view = {}, const Viewport &viewport = {}); +OrthographicCamera createOrthographicCamera( + Device &device, + const CameraView &view = {}, + const Orthographic &projection = {}); } diff --git a/src/brayns/core/objects/Messages.h b/src/brayns/core/objects/Messages.h index 2c51f05df..0335bea4f 100644 --- a/src/brayns/core/objects/Messages.h +++ b/src/brayns/core/objects/Messages.h @@ -21,7 +21,6 @@ #pragma once -#include #include #include @@ -32,103 +31,64 @@ using ObjectId = std::uint32_t; constexpr auto nullId = ObjectId(0); -struct Metadata +struct ObjectInfo { ObjectId id; std::string type; - std::size_t size = 0; JsonValue userData = {}; }; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); + auto builder = JsonBuilder(); builder.field("id", [](auto &object) { return &object.id; }) - .description("Object ID, primary way to query this object (starts at 1)"); + .description("Object ID (starts at 1, uses 0 for objects that are not in registry)"); builder.field("type", [](auto &object) { return &object.type; }) - .description("Object type key, use 'get-{type}' to query detailed information about the object"); - builder.field("size", [](auto &object) { return &object.size; }) - .description("Object size in memory (in bytes), 0 if negligible"); - builder.field("user_data", [](auto &object) { return &object.userData; }).description("Data set by user"); - return builder.build(); - } -}; - -template -struct ObjectParams -{ - T settings; - JsonValue userData = {}; -}; - -template -struct JsonObjectReflector> -{ - static auto reflect() - { - auto builder = JsonBuilder>(); - builder.field("settings", [](auto &object) { return &object.settings; }) - .description("Settings to create the object"); + .description("Object type, use 'get-{type}' to query detailed information about the object"); builder.field("user_data", [](auto &object) { return &object.userData; }) - .description("Optional user data (only for user, not used by Brayns)") - .defaultValue(NullJson()); + .description("Data set by user (not used by Brayns)") + .required(false); return builder.build(); } }; -template -struct ObjectResult -{ - Metadata metadata; - T properties; -}; - -template -struct JsonObjectReflector> +struct ObjectParams { - static auto reflect() - { - auto builder = JsonBuilder>(); - builder.field("metadata", [](auto &object) { return &object.metadata; }) - .description("Generic object properties"); - builder.field("properties", [](auto &object) { return &object.properties; }) - .description("Properties that are specific to the object type"); - return builder.build(); - } + ObjectId id; }; -struct EmptyJsonObject -{ -}; +using ObjectResult = ObjectParams; template<> -struct JsonObjectReflector +struct JsonObjectReflector { static auto reflect() { - auto builder = JsonBuilder(); - builder.description("An empty object"); + auto builder = JsonBuilder(); + builder.field("id", [](auto &object) { return &object.id; }).description("Object ID"); return builder.build(); } }; -using EmptyJson = std::optional; - -struct GetObjectParams +template +struct UpdateParams { ObjectId id; + T properties; }; -template<> -struct JsonObjectReflector +template +struct JsonObjectReflector> { static auto reflect() { - auto builder = JsonBuilder(); - builder.field("id", [](auto &object) { return &object.id; }).description("ID of the object to retreive"); + auto builder = JsonBuilder>(); + builder.field("id", [](auto &object) { return &object.id; }).description("ID of the object to update"); + builder.field("properties", [](auto &object) { return &object.properties; }) + .description("New object properties to replace the old ones"); return builder.build(); } }; diff --git a/src/brayns/core/objects/ObjectManager.cpp b/src/brayns/core/objects/ObjectManager.cpp index a84b53614..942770220 100644 --- a/src/brayns/core/objects/ObjectManager.cpp +++ b/src/brayns/core/objects/ObjectManager.cpp @@ -52,25 +52,25 @@ ObjectManager::ObjectManager() disableNullId(_ids); } -std::vector ObjectManager::getAllMetadata() const +std::vector ObjectManager::getAllObjects() const { - auto metadatas = std::vector(); - metadatas.reserve(_objects.size()); + auto objects = std::vector(); + objects.reserve(_objects.size()); for (const auto &[id, object] : _objects) { - auto metadata = createObjectMetadata(object); - metadatas.push_back(std::move(metadata)); + auto result = getObjectInfo(object); + objects.push_back(std::move(result)); } - return metadatas; + return objects; } -Metadata ObjectManager::getMetadata(ObjectId id) const +ObjectInfo ObjectManager::getObject(ObjectId id) const { const auto &interface = getInterface(id); - return createObjectMetadata(interface); + return getObjectInfo(interface); } void ObjectManager::setUserData(ObjectId id, const JsonValue &userData) diff --git a/src/brayns/core/objects/ObjectManager.h b/src/brayns/core/objects/ObjectManager.h index f5bc243b3..d80f0cd30 100644 --- a/src/brayns/core/objects/ObjectManager.h +++ b/src/brayns/core/objects/ObjectManager.h @@ -41,8 +41,8 @@ class ObjectManager public: explicit ObjectManager(); - std::vector getAllMetadata() const; - Metadata getMetadata(ObjectId id) const; + std::vector getAllObjects() const; + ObjectInfo getObject(ObjectId id) const; void setUserData(ObjectId id, const JsonValue &userData); void remove(ObjectId id); void clear(); @@ -60,14 +60,17 @@ class ObjectManager } template - Stored add(T value, const JsonValue &userData = {}) + Stored add(T object) { auto id = _ids.next(); try { - auto object = UserObject{id, std::move(value), userData}; - auto ptr = std::make_shared(std::move(object)); + auto user = UserObject{id, std::move(object)}; + auto ptr = std::make_shared(std::move(user)); + + addObject(ptr->value, id); + auto interface = createObjectInterface(ptr); _objects.emplace(id, std::move(interface)); @@ -88,8 +91,10 @@ class ObjectManager const ObjectInterface &getInterface(ObjectId id) const; template - static const std::shared_ptr> &castObject(const ObjectInterface &interface) + const std::shared_ptr> &getShared(ObjectId id) const { + const auto &interface = getInterface(id); + auto ptr = std::any_cast>>(&interface.value); if (ptr != nullptr) @@ -97,18 +102,9 @@ class ObjectManager return *ptr; } - auto id = interface.getId(); auto type = interface.getType(); throw InvalidParams(fmt::format("Invalid type for object with ID {}: {}", id, type)); } - - template - const std::shared_ptr> &getShared(ObjectId id) const - { - const auto &interface = getInterface(id); - - return castObject(interface); - } }; } diff --git a/src/brayns/core/objects/UserObject.h b/src/brayns/core/objects/UserObject.h index b9057466e..9301ddd89 100644 --- a/src/brayns/core/objects/UserObject.h +++ b/src/brayns/core/objects/UserObject.h @@ -37,7 +37,7 @@ template concept WithType = std::same_as::getType(std::declval()))>; template -concept WithSize = std::same_as::getSize(std::declval()))>; +concept WithAdd = std::is_void_v::add(std::declval(), ObjectId()))>; template concept WithRemove = std::is_void_v::remove(std::declval()))>; @@ -52,16 +52,16 @@ std::string getObjectType(const T &object) } template -std::size_t getObjectSize(const T &object) +void addObject(T &object, ObjectId id) { (void)object; - return 0; + (void)id; } -template -std::size_t getObjectSize(const T &object) +template +void addObject(T &object, ObjectId id) { - return ObjectReflector::getSize(object); + return ObjectReflector::add(object, id); } template @@ -85,23 +85,12 @@ struct UserObject }; template -void removeStoredObject(UserObject &object) +void removeUserObject(UserObject &object) { object.id = nullId; removeObject(object.value); } -template -Metadata createObjectMetadata(const UserObject &object) -{ - return { - .id = object.id, - .type = getObjectType(object.value), - .size = getObjectSize(object.value), - .userData = object.userData, - }; -} - template class Stored { @@ -126,11 +115,6 @@ class Stored return getObjectType(_object->value); } - std::size_t getSize() const - { - return getObjectSize(_object->value); - } - JsonValue getUserData() const { return _object->userData; @@ -141,9 +125,9 @@ class Stored _object->userData = userData; } - Metadata getMetadata() const + ObjectResult getResult() const { - return createObjectMetadata(*_object); + return {getId()}; } T &get() const @@ -161,7 +145,6 @@ struct ObjectInterface std::function getId; std::function remove; std::function getType; - std::function getSize; std::function getUserData; std::function setUserData; }; @@ -172,20 +155,18 @@ ObjectInterface createObjectInterface(const std::shared_ptr> &obje return { .value = object, .getId = [=] { return object->id; }, - .remove = [=] { removeStoredObject(*object); }, + .remove = [=] { removeUserObject(*object); }, .getType = [=] { return getObjectType(object->value); }, - .getSize = [=] { return getObjectSize(object->value); }, .getUserData = [=] { return object->userData; }, .setUserData = [=](const auto &userData) { object->userData = userData; }, }; } -inline Metadata createObjectMetadata(const ObjectInterface &object) +inline ObjectInfo getObjectInfo(const ObjectInterface &object) { return { .id = object.getId(), .type = object.getType(), - .size = object.getSize(), .userData = object.getUserData(), }; } diff --git a/tests/core/engine/TestRender.cpp b/tests/core/engine/TestRender.cpp index 3b1b150a5..c10b9a696 100644 --- a/tests/core/engine/TestRender.cpp +++ b/tests/core/engine/TestRender.cpp @@ -149,7 +149,7 @@ TEST_CASE("Render") auto renderer = createScivisRenderer(device, {{.materials = createData(device, {material})}}); - auto camera = createPerspectiveCamera(device, {}, {.fovy = 45.0F, .aspectRatio = float(width) / float(height)}); + auto camera = createPerspectiveCamera(device, {}, {.fovy = 45.0F, .aspect = float(width) / float(height)}); auto sphereData = std::vector{{0, 0, 3, 0.25F}, {1, 0, 3, 0.25F}, {1, 1, 3, 0.25F}}; auto colors = std::vector{{1, 0, 0, 1}, {0, 0, 1, 1}, {0, 1, 0, 1}}; diff --git a/tests/core/objects/TestObjectManager.cpp b/tests/core/objects/TestObjectManager.cpp index e29cfcfd9..e14361230 100644 --- a/tests/core/objects/TestObjectManager.cpp +++ b/tests/core/objects/TestObjectManager.cpp @@ -25,27 +25,10 @@ using namespace brayns; namespace brayns { -struct TestProperties -{ - float someFloat = 0.0F; -}; - -template<> -struct JsonObjectReflector -{ - static auto reflect() - { - auto builder = JsonBuilder(); - builder.field("some_float", [](auto &object) { return &object.someFloat; }); - return builder.build(); - } -}; - struct TestObject { std::string type; - std::size_t size; - bool removed = false; + ObjectId id = nullId; }; template<> @@ -56,14 +39,14 @@ struct ObjectReflector return object.type; } - static std::size_t getSize(const TestObject &object) + static void add(TestObject &object, ObjectId id) { - return object.size; + object.id = id; } static void remove(TestObject &object) { - object.removed = true; + object.id = nullId; } }; } @@ -72,44 +55,42 @@ TEST_CASE("Create and remove objects") { auto objects = ObjectManager(); - auto object = objects.add({"type", 4}, "someUserData"); + auto object = objects.add(TestObject{"type"}); auto id = object.getId(); CHECK_EQ(id, 1); - auto metadata = objects.getMetadata(id); + auto info = objects.getObject(id); - CHECK_EQ(metadata.id, id); - CHECK_EQ(metadata.type, "type"); - CHECK_EQ(metadata.size, 4); - CHECK_EQ(metadata.userData.extract(), "someUserData"); + CHECK_EQ(info.id, id); + CHECK_EQ(info.type, "type"); + CHECK(info.userData.isEmpty()); auto &retreived = objects.get(id); - CHECK_EQ(retreived.size, 4); CHECK_EQ(retreived.type, "type"); - CHECK_FALSE(retreived.removed); + CHECK_EQ(retreived.id, id); auto stored = objects.getStored(id); CHECK_EQ(stored.getId(), id); - auto object2 = objects.add({}); + auto object2 = objects.add(TestObject()); auto id2 = object2.getId(); - CHECK_EQ(objects.getAllMetadata().size(), 2); + CHECK_EQ(objects.getAllObjects().size(), 2); objects.remove(id); CHECK(stored.isRemoved()); - CHECK(stored.get().removed); + CHECK_EQ(stored.get().id, nullId); - CHECK_THROWS_AS(objects.getMetadata(id), InvalidParams); + CHECK_THROWS_AS(objects.getObject(id), InvalidParams); CHECK_THROWS_AS(objects.get(id), InvalidParams); CHECK_THROWS_AS(objects.getStored(id), InvalidParams); - auto object3 = objects.add({}); + auto object3 = objects.add(TestObject()); CHECK_EQ(object3.getId(), 1); auto stored2 = objects.getStored(id2); @@ -119,7 +100,7 @@ TEST_CASE("Create and remove objects") CHECK(stored2.isRemoved()); CHECK(object3.isRemoved()); - CHECK(objects.getAllMetadata().empty()); + CHECK(objects.getAllObjects().empty()); } TEST_CASE("Errors") @@ -128,7 +109,7 @@ TEST_CASE("Errors") objects.add({}); - CHECK_THROWS_AS(objects.getMetadata(0), InvalidParams); - CHECK_THROWS_AS(objects.getMetadata(2), InvalidParams); + CHECK_THROWS_AS(objects.getObject(0), InvalidParams); + CHECK_THROWS_AS(objects.getObject(2), InvalidParams); CHECK_THROWS_AS(objects.remove(2), InvalidParams); } From 7d335a930fb859e0c3d749fa7b2f206ece48868c Mon Sep 17 00:00:00 2001 From: Adrien4193 Date: Thu, 8 Aug 2024 14:19:42 +0200 Subject: [PATCH 12/12] Fix Python side. --- python/brayns/__init__.py | 8 ++-- python/brayns/api/core/objects.py | 38 +++++++---------- python/tests/integration/core/test_objects.py | 42 +++++++++---------- 3 files changed, 39 insertions(+), 49 deletions(-) diff --git a/python/brayns/__init__.py b/python/brayns/__init__.py index 9992bc5d0..7f29de6fd 100644 --- a/python/brayns/__init__.py +++ b/python/brayns/__init__.py @@ -25,7 +25,7 @@ """ from .api.core.objects import ( - Metadata, + Object, clear_objects, create_empty_object, get_all_objects, @@ -69,9 +69,8 @@ "clear_objects", "connect", "Connection", - "create_logger", - "update_object", "create_empty_object", + "create_logger", "Endpoint", "FutureResponse", "get_all_objects", @@ -88,7 +87,7 @@ "JsonRpcRequest", "JsonRpcResponse", "JsonRpcSuccessResponse", - "Metadata", + "Object", "remove_objects", "Request", "Response", @@ -97,6 +96,7 @@ "Task", "TaskInfo", "TaskOperation", + "update_object", "Version", "WebSocketError", ] diff --git a/python/brayns/api/core/objects.py b/python/brayns/api/core/objects.py index 055dbc298..7a8ad0b56 100644 --- a/python/brayns/api/core/objects.py +++ b/python/brayns/api/core/objects.py @@ -22,48 +22,40 @@ from typing import Any from brayns.network.connection import Connection -from brayns.utils.parsing import check_type, get +from brayns.utils.parsing import check_type, get, try_get @dataclass -class Metadata: +class Object: id: int type: str - size: int user_data: Any -def parse_metadata(message: dict[str, Any]) -> Metadata: - return Metadata( +def parse_object(message: dict[str, Any]) -> Object: + return Object( id=get(message, "id", int), type=get(message, "type", str), - size=get(message, "size", int), - user_data=get(message, "user_data", Any), + user_data=try_get(message, "user_data", Any, None), ) -def extract_metadata(result: dict[str, Any]) -> Metadata: - metadata = get(result, "metadata", dict[str, Any]) - return parse_metadata(metadata) - - -async def get_all_objects(connection: Connection) -> list[Metadata]: +async def get_all_objects(connection: Connection) -> list[Object]: result = await connection.get_result("get-all-objects") check_type(result, dict[str, Any]) objects = get(result, "objects", list[dict[str, Any]]) - return [parse_metadata(item) for item in objects] + return [parse_object(item) for item in objects] -async def get_object(connection: Connection, id: int) -> Metadata: +async def get_object(connection: Connection, id: int) -> Object: result = await connection.get_result("get-object", {"id": id}) check_type(result, dict[str, Any]) - return parse_metadata(result) + return parse_object(result) -async def update_object(connection: Connection, id: int, user_data: Any) -> Metadata: - result = await connection.get_result("update-object", {"id": id, "user_data": user_data}) - check_type(result, dict[str, Any]) - return parse_metadata(result) +async def update_object(connection: Connection, id: int, user_data: Any) -> None: + properties = {"user_data": user_data} + await connection.get_result("update-object", {"id": id, "properties": properties}) async def remove_objects(connection: Connection, ids: list[int]) -> None: @@ -74,7 +66,7 @@ async def clear_objects(connection: Connection) -> None: await connection.get_result("clear-objects") -async def create_empty_object(connection: Connection, user_data: Any = None) -> Metadata: - result = await connection.get_result("create-empty-object", {"user_data": user_data}) +async def create_empty_object(connection: Connection) -> int: + result = await connection.get_result("create-empty-object") check_type(result, dict[str, Any]) - return extract_metadata(result) + return get(result, "id", int) diff --git a/python/tests/integration/core/test_objects.py b/python/tests/integration/core/test_objects.py index a8dbc66b1..ac451bb6b 100644 --- a/python/tests/integration/core/test_objects.py +++ b/python/tests/integration/core/test_objects.py @@ -35,29 +35,26 @@ @pytest.mark.integration_test @pytest.mark.asyncio async def test_create_empty_object(connection: Connection) -> None: - obj = await create_empty_object(connection, "test") - - assert obj.id == 1 - assert obj.type == "empty-object" - assert obj.user_data == "test" + assert await create_empty_object(connection) == 1 @pytest.mark.integration_test @pytest.mark.asyncio async def test_get_all_objects(connection: Connection) -> None: - objects = [await create_empty_object(connection) for _ in range(10)] - tests = await get_all_objects(connection) + created = [await create_empty_object(connection) for _ in range(10)] + retreived = await get_all_objects(connection) - assert tests == objects + assert created == [object.id for object in retreived] @pytest.mark.integration_test @pytest.mark.asyncio async def test_get_object(connection: Connection) -> None: - objects = [await create_empty_object(connection) for _ in range(10)] - tests = [await get_object(connection, obj.id) for obj in objects] + created = [await create_empty_object(connection) for _ in range(10)] + retreived = await get_all_objects(connection) + tests = [await get_object(connection, id) for id in created] - assert tests == objects + assert tests == retreived with pytest.raises(JsonRpcError): await get_object(connection, 123) @@ -66,40 +63,41 @@ async def test_get_object(connection: Connection) -> None: @pytest.mark.integration_test @pytest.mark.asyncio async def test_update_object(connection: Connection) -> None: - objects = [await create_empty_object(connection) for _ in range(10)] + ids = [await create_empty_object(connection) for _ in range(10)] + + await update_object(connection, ids[0], "test") - test = await update_object(connection, objects[0].id, "test") + test = await get_object(connection, ids[0]) - assert test.id == objects[0].id - assert test.type == objects[0].type - assert test.size == objects[0].size + assert test.id == 1 + assert test.type == "empty-object" assert test.user_data == "test" @pytest.mark.integration_test @pytest.mark.asyncio async def test_remove_objects(connection: Connection) -> None: - objects = [await create_empty_object(connection) for _ in range(10)] + ids = [await create_empty_object(connection) for _ in range(10)] await remove_objects(connection, [1, 2, 3]) tests = await get_all_objects(connection) assert [obj.id for obj in tests] == list(range(4, 11)) - for obj in objects[:3]: + for id in ids[:3]: with pytest.raises(JsonRpcError): - await get_object(connection, obj.id) + await get_object(connection, id) @pytest.mark.integration_test @pytest.mark.asyncio async def test_clear_objects(connection: Connection) -> None: - objects = [await create_empty_object(connection) for _ in range(10)] + ids = [await create_empty_object(connection) for _ in range(10)] await clear_objects(connection) assert not await get_all_objects(connection) - for obj in objects: + for id in ids: with pytest.raises(JsonRpcError): - await get_object(connection, obj.id) + await get_object(connection, id)