diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05a0d8f81..e54857bab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,20 +12,20 @@ permissions: jobs: build: name: Build up-cpp and dependencies - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: + - name: Fetch up-cpp + uses: actions/checkout@v4 + with: + path: up-cpp + - name: Install Conan id: conan uses: turtlebrowser/get-conan@main with: version: 2.3.2 - - name: Fetch up-cpp - uses: actions/checkout@v4 - with: - path: up-cpp - - name: Install conan CI profile shell: bash run: | @@ -42,7 +42,7 @@ jobs: - name: Build up-core-api conan package shell: bash run: | - conan create --version 1.6.0-alpha3 up-conan-recipes/up-core-api/release + conan create --version 1.6.0-alpha4 up-conan-recipes/up-core-api/release - name: Build up-cpp with tests shell: bash @@ -78,7 +78,7 @@ jobs: test: name: Run up-cpp tests - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: build steps: @@ -105,7 +105,7 @@ jobs: # NOTE: Run dynamic analysis in unit tests memcheck: name: Run Valgrind Memcheck - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: build steps: @@ -173,7 +173,7 @@ jobs: threadcheck: name: Run Valgrind ThreadCheck - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: build steps: @@ -242,7 +242,7 @@ jobs: helgrind: name: Run Valgrind Helgrind - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: build steps: @@ -311,7 +311,7 @@ jobs: dhat: name: Run Valgrind DHAT - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: build steps: @@ -380,17 +380,17 @@ jobs: lint: name: Lint C++ sources - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: build permissions: contents: write pull-requests: read steps: - - name: Get build commands - uses: actions/download-artifact@v4 + - name: Fetch up-cpp + uses: actions/checkout@v4 with: - name: compile-commands + path: up-cpp - name: Install Conan id: conan @@ -398,29 +398,30 @@ jobs: with: version: 2.3.2 - - name: Create default Conan profile - run: conan profile detect - - - name: Get conan cache - uses: actions/download-artifact@v4 - with: - name: conan-cache - - - name: Restore conan cache from archive + - name: Install conan CI profile shell: bash run: | - conan cache restore conan-cache.tgz + conan profile detect + cp up-cpp/.github/workflows/ci_conan_profile "$(conan profile path default)" + conan profile show - - name: Fetch up-cpp + - name: Fetch up-core-api conan recipe uses: actions/checkout@v4 with: - path: up-cpp + path: up-conan-recipes + repository: eclipse-uprotocol/up-conan-recipes - - name: Get build artifacts - uses: actions/download-artifact@v4 - with: - name: build-artifacts - path: up-cpp/build/Release + - name: Build up-core-api conan package + shell: bash + run: | + conan create --version 1.6.0-alpha4 up-conan-recipes/up-core-api/release + + - name: Build up-cpp with tests + shell: bash + run: | + cd up-cpp + conan install --build=missing . + cmake --preset conan-release -DCMAKE_EXPORT_COMPILE_COMMANDS=yes - name: Run linters on source continue-on-error: true @@ -430,11 +431,14 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: repo-root: up-cpp + version: 13 ignore: 'test' style: 'file' # read .clang-format for configuration tidy-checks: '' # Read .clang-tidy for configuration database: build/Release/compile_commands.json + version: 12 + - name: Run linters on tests continue-on-error: true id: test-linter @@ -447,6 +451,8 @@ jobs: style: 'file' # read .clang-format for configuration tidy-checks: '' # Read .clang-tidy for configuration database: build/Release/compile_commands.json + version: 12 + - name: Report lint failure if: steps.source-linter.outputs.checks-failed > 0 || steps.test-linter.outputs.checks-failed > 0 @@ -460,9 +466,10 @@ jobs: # job to signal whether all CI checks have passed. ci: name: CI status checks - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: [build, test, memcheck, threadcheck, helgrind, dhat] if: always() steps: - name: Check whether all jobs pass run: echo '${{ toJson(needs) }}' | jq -e 'all(.result == "success")' + diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index b6bf557d5..b62f60f0d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -69,7 +69,7 @@ jobs: name: Build up-core-api conan package shell: bash run: | - conan create --version 1.6.0-alpha3 up-conan-recipes/up-core-api/release + conan create --version 1.6.0-alpha4 up-conan-recipes/up-core-api/release - if: matrix.build-mode == 'manual' name: Build up-cpp with tests diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index d83aeec6e..39776455a 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -42,7 +42,7 @@ jobs: - name: Build up-core-api conan package shell: bash run: | - conan create --version 1.6.0-alpha3 up-conan-recipes/up-core-api/release + conan create --version 1.6.0-alpha4 up-conan-recipes/up-core-api/release - name: Build up-cpp with tests shell: bash @@ -65,7 +65,7 @@ jobs: run: | cd up-cpp/build/Release mkdir -p ../Coverage - gcovr -r ../../ --html --html-details -o ../Coverage/index.html -e '.*test.*' + gcovr -r ../../ --html --html-details -o ../Coverage/index.html -e '.*test.*' --gcov-ignore-parse-errors negative_hits.warn_once_per_file cd .. echo "Coverage report can be found here: ../Coverage/index.html" diff --git a/README.md b/README.md index 6b42e598e..52439280b 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ implementation, such as [up-transport-zenoh-cpp][zenoh-transport-repo]. Using the recipes found in [up-conan-recipes][conan-recipe-repo], build these Conan packages: -1. [up-core-api][spec-repo]: `conan create --version 1.6.0-alpha3 --build=missing up-core-api/release` +1. [up-core-api][spec-repo]: `conan create --version 1.6.0-alpha4 --build=missing up-core-api/release` **NOTE:** all `conan` commands in this document use Conan 2.x syntax. Please adjust accordingly when using Conan 1.x. diff --git a/conanfile.txt b/conanfile.txt index e45041ebf..2270c2519 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -1,5 +1,5 @@ [requires] -up-core-api/[~1.6, include_prerelease] +up-core-api/1.6.0-alpha4 spdlog/[~1.13] protobuf/[~3.21] diff --git a/include/up-cpp/client/usubscription/v3/Consumer.h b/include/up-cpp/client/usubscription/v3/Consumer.h new file mode 100644 index 000000000..2fed6ab4b --- /dev/null +++ b/include/up-cpp/client/usubscription/v3/Consumer.h @@ -0,0 +1,222 @@ +// SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache License Version 2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: Apache-2.0 + +#ifndef UP_CPP_CLIENT_USUBSCRIPTION_V3_CONSUMER_H +#define UP_CPP_CLIENT_USUBSCRIPTION_V3_CONSUMER_H + +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace uprotocol::client::usubscription::v3 { +using uprotocol::core::usubscription::v3::SubscriptionRequest; +using uprotocol::core::usubscription::v3::UnsubscribeRequest; +using uprotocol::core::usubscription::v3::Update; +using uprotocol::core::usubscription::v3::uSubscription; + +/** + * @struct ConsumerOptions + * @brief Additional details for uSubscription service. + * + * Each member represents an optional parameter for the uSubscription service. + */ +struct ConsumerOptions { + /// Permission level of the subscription request + std::optional permission_level; + /// TAP token for access. + std::optional token; + /// Expiration time of the subscription. + std::optional when_expire; + /// Sample period for the subscription messages in milliseconds. + std::optional sample_period_ms; + /// Details of the subscriber. + std::optional subscriber_details; + /// Details of the subscription. + std::optional subscription_details; +}; + +/// @struct uSubscriptionUUriBuilder +/// @brief Structure to build uSubscription request URIs. +/// +/// This structure is used to build URIs for uSubscription service. It uses the +/// service options from uSubscription proto to set the authority name, ue_id, +/// ue_version_major, and the notification topic resource ID in the URI. +struct USubscriptionUUriBuilder { +private: + /// URI for the uSubscription service + v1::UUri uri_; + /// Resource ID of the notification topic + uint32_t sink_resource_id_; + +public: + /// @brief Constructor for uSubscriptionUUriBuilder. + USubscriptionUUriBuilder() { + // Get the service descriptor + const google::protobuf::ServiceDescriptor* service = + uSubscription::descriptor(); + const auto& service_options = service->options(); + + // Get the service options + const auto& service_name = + service_options.GetExtension(uprotocol::service_name); + const auto& service_version_major = + service_options.GetExtension(uprotocol::service_version_major); + const auto& service_id = + service_options.GetExtension(uprotocol::service_id); + const auto& notification_topic = + service_options.GetExtension(uprotocol::notification_topic, 0); + + // Set the values in the URI + uri_.set_authority_name(service_name); + uri_.set_ue_id(service_id); + uri_.set_ue_version_major(service_version_major); + sink_resource_id_ = notification_topic.id(); + } + + /// @brief Get the URI with a specific resource ID. + /// + /// @param resource_id The resource ID to set in the URI. + /// + /// @return The URI with the specified resource ID. + v1::UUri getServiceUriWithResourceId(uint32_t resource_id) const { + v1::UUri uri = uri_; // Copy the base URI + uri.set_resource_id(resource_id); + return uri; + } + + /// @brief Get the notification URI. + /// + /// @return The notification URI. + v1::UUri getNotificationUri() const { + v1::UUri uri = uri_; // Copy the base URI + uri.set_resource_id(sink_resource_id_); + return uri; + } +}; + +/// @brief Interface for uEntities to create subscriptions. +/// +/// Like all L3 client APIs, the Consumer is a wrapper on top of the +/// L2 Communication APIs and USubscription service. +struct Consumer { + using ConsumerOrStatus = + utils::Expected, v1::UStatus>; + using ListenCallback = transport::UTransport::ListenCallback; + using ListenHandle = transport::UTransport::ListenHandle; + using SubscriptionResponse = core::usubscription::v3::SubscriptionResponse; + + /// @brief Create a subscription + /// + /// @param transport Transport to register with. + /// @param subscription_topic Topic to subscribe to. + /// @param callback Function that is called when publish message is + /// received. + /// @param priority Priority of the subscription request. + /// @param subscribe_request_ttl Time to live for the subscription request. + /// @param consumer_options Additional details for uSubscription service. + [[nodiscard]] static ConsumerOrStatus create( + std::shared_ptr transport, + const v1::UUri& subscription_topic, ListenCallback&& callback, + v1::UPriority priority, + std::chrono::milliseconds subscription_request_ttl, + ConsumerOptions consumer_options); + + /// @brief Unsubscribe from the topic and call uSubscription service to + /// close the subscription. + /// + /// @param priority Priority of the unsubscribe request. + /// @param request_ttl Time to live for the unsubscribe request. + void unsubscribe(v1::UPriority priority, + std::chrono::milliseconds request_ttl); + + /// @brief getter for subscription update + /// + /// @return subscription update + Update getSubscriptionUpdate() const { return subscription_update_; } + + /// @brief Destructor + ~Consumer() = default; + + /// This section for test code only delete later + +protected: + /// @brief Constructor + /// + /// @param transport Transport to register with. + /// @param subscriber_details Additional details about the subscriber. + Consumer(std::shared_ptr transport, + v1::UUri subscription_topic, + ConsumerOptions consumer_options = {}); + +private: + // Transport + std::shared_ptr transport_; + + // Topic to subscribe to + const v1::UUri subscription_topic_; + // Additional details about uSubscription service + ConsumerOptions consumer_options_; + + // URI info about the uSubscription service + USubscriptionUUriBuilder uSubscriptionUUriBuilder_; + + // Subscription updates + std::unique_ptr noficationSinkHandle_; + Update subscription_update_; + + // RPC request + std::unique_ptr rpc_client_; + communication::RpcClient::InvokeHandle rpc_handle_; + SubscriptionResponse subscription_response_; + + // L2 Subscriber details + std::unique_ptr subscriber_; + + // Allow the protected constructor for this class to be used in make_unique + // inside of create() + friend std::unique_ptr + std::make_unique, + const uprotocol::v1::UUri, + uprotocol::client::usubscription::v3::ConsumerOptions>( + std::shared_ptr&&, + const uprotocol::v1::UUri&&, + uprotocol::client::usubscription::v3::ConsumerOptions&&); + + /// @brief Build SubscriptionRequest for subscription request + SubscriptionRequest buildSubscriptionRequest(); + + /// @brief Build UnsubscriptionRequest for unsubscription request + UnsubscribeRequest buildUnsubscriptionRequest(); + + /// @brief Create a notification sink to receive subscription updates + v1::UStatus createNotificationSink(); + + /// @brief Subscribe to the topic + /// + /// @param topic Topic to subscribe to. + /// @param subscription_request_ttl Time to live for the subscription + /// request. + /// @param callback Function that is called when a published message is + /// received. + v1::UStatus subscribe(v1::UPriority priority, + std::chrono::milliseconds subscription_request_ttl, + ListenCallback&& callback); +}; + +} // namespace uprotocol::client::usubscription::v3 + +#endif // UP_CPP_CLIENT_USUBSCRIPTION_V3_CONSUMER_H \ No newline at end of file diff --git a/include/up-cpp/communication/NotificationSink.h b/include/up-cpp/communication/NotificationSink.h index 2e93b7ea5..007c4c3b8 100644 --- a/include/up-cpp/communication/NotificationSink.h +++ b/include/up-cpp/communication/NotificationSink.h @@ -58,7 +58,7 @@ struct NotificationSink { /// successfully. /// * UStatus containing an error state otherwise. [[nodiscard]] static SinkOrStatus create( - std::shared_ptr transport, + const std::shared_ptr& transport, ListenCallback&& callback, const v1::UUri& source_filter); /// @note DEPRECATED @@ -76,13 +76,12 @@ struct NotificationSink { [[deprecated( "See alternate overload of " "create()")]] [[nodiscard]] static SinkOrStatus - create(std::shared_ptr transport, + create(const std::shared_ptr& transport, const v1::UUri& sink, ListenCallback&& callback, std::optional&& source_filter); ~NotificationSink() = default; -protected: /// @brief Constructs a notification listener connected to a given /// transport. /// diff --git a/include/up-cpp/communication/RpcClient.h b/include/up-cpp/communication/RpcClient.h index 8ba7e8b0e..c78ab5f94 100644 --- a/include/up-cpp/communication/RpcClient.h +++ b/include/up-cpp/communication/RpcClient.h @@ -85,7 +85,7 @@ struct RpcClient { /// @name Passthroughs for std::future /// @{ auto get() { return future_.get(); } - auto valid() const noexcept { return future_.valid(); } + [[nodiscard]] auto valid() const noexcept { return future_.valid(); } void wait() const { future_.wait(); } template auto wait_for(Args&&... args) const { @@ -163,7 +163,7 @@ struct RpcClient { [[nodiscard]] InvokeFuture invokeMethod(); /// @brief Default move constructor (defined in RpcClient.cpp) - RpcClient(RpcClient&&); + RpcClient(RpcClient&&) noexcept; /// @brief Default destructor (defined in RpcClient.cpp) ~RpcClient(); diff --git a/include/up-cpp/communication/RpcServer.h b/include/up-cpp/communication/RpcServer.h index a5ff77103..daeebebec 100644 --- a/include/up-cpp/communication/RpcServer.h +++ b/include/up-cpp/communication/RpcServer.h @@ -90,9 +90,9 @@ struct RpcServer { /// @param ttl (Optional) Time response will be valid from the moment /// respond() is called. Note that the original request's TTL /// may also still apply. - RpcServer(std::shared_ptr transport, - std::optional format = {}, - std::optional ttl = {}); + explicit RpcServer(std::shared_ptr transport, + std::optional format = {}, + std::optional ttl = {}); /// @brief Allows std::make_unique to directly access RpcServer's private /// constructor. diff --git a/include/up-cpp/datamodel/builder/Payload.h b/include/up-cpp/datamodel/builder/Payload.h index 1caa021c1..303b373a5 100644 --- a/include/up-cpp/datamodel/builder/Payload.h +++ b/include/up-cpp/datamodel/builder/Payload.h @@ -12,6 +12,7 @@ #ifndef UP_CPP_DATAMODEL_BUILDER_PAYLOAD_H #define UP_CPP_DATAMODEL_BUILDER_PAYLOAD_H +#include #include #include @@ -79,13 +80,13 @@ struct Payload { /// will compile out. /// @param data Data to be serialized and stored. template - Payload(Serializer s, const ValueT& data) { - auto serializedData = Serializer::serialize(data); + Payload(Serializer s [[maybe_unused]], const ValueT& data) { + auto serialized_data = Serializer::serialize(data); if (!UPayloadFormat_IsValid( - std::get(serializedData))) { + std::get(serialized_data))) { throw std::out_of_range("Invalid Serializer payload format"); } - payload_ = std::move(serializedData); + payload_ = std::move(serialized_data); } /// @brief Creates a Payload builder with a provided pre-serialized data. @@ -94,8 +95,7 @@ struct Payload { /// @param format The data format of the payload in value_bytes. /// /// @throws std::out_of_range If format is not valid for v1::UPayloadFormat - Payload(const std::vector& value_bytes, - const v1::UPayloadFormat format); + Payload(const std::vector& value_bytes, v1::UPayloadFormat format); /// @brief Creates a Payload builder with a provided pre-serialized data. /// @@ -106,7 +106,7 @@ struct Payload { /// /// @note This would typically be used for UPAYLOAD_FORMAT_TEXT or /// UPAYLOAD_FORMAT_JSON, but can be used for other payload formats. - Payload(const std::string& value, const v1::UPayloadFormat format); + Payload(const std::string& value, v1::UPayloadFormat format); /// @brief Creates a Payload builder with a provided pre-serialized data. /// @@ -119,7 +119,7 @@ struct Payload { /// /// @note This would typically be used for UPAYLOAD_FORMAT_TEXT or /// UPAYLOAD_FORMAT_JSON, but can be used for other payload formats. - Payload(std::string&& value, const v1::UPayloadFormat format); + Payload(std::string&& value, v1::UPayloadFormat format); /// @brief Creates a Payload builder with a provided pre-serialized data. /// @@ -131,6 +131,13 @@ struct Payload { /// for v1::UPayloadFormat explicit Payload(Serialized&&); + /// @brief Creates a Payload builder with a provided protobuf::Any. + /// + /// The contents of value will be moved into the Payload object. + /// + /// @param An initialized google::protobuf::Any object.. + explicit Payload(const google::protobuf::Any&); + /// @brief Move constructor. Payload(Payload&&) noexcept; diff --git a/include/up-cpp/datamodel/builder/UMessage.h b/include/up-cpp/datamodel/builder/UMessage.h index e9eaf2592..ae9e40d82 100644 --- a/include/up-cpp/datamodel/builder/UMessage.h +++ b/include/up-cpp/datamodel/builder/UMessage.h @@ -244,11 +244,11 @@ struct UMessageBuilder { private: /// @brief Constructs a UMessageBuilder with the provided attributes. /// - /// @param msgType + /// @param msg_type /// @param source /// @param sink /// @param request_id - UMessageBuilder(v1::UMessageType msgType, v1::UUri&& source, + UMessageBuilder(v1::UMessageType msg_type, v1::UUri&& source, std::optional&& sink = {}, std::optional&& request_id = {}); diff --git a/include/up-cpp/datamodel/builder/Uuid.h b/include/up-cpp/datamodel/builder/Uuid.h index 5a9421b25..da674a463 100644 --- a/include/up-cpp/datamodel/builder/Uuid.h +++ b/include/up-cpp/datamodel/builder/Uuid.h @@ -85,7 +85,7 @@ struct UuidBuilder { v1::UUID build(); private: - UuidBuilder(bool testing); + explicit UuidBuilder(bool testing); const bool testing_{false}; std::function time_source_; diff --git a/include/up-cpp/datamodel/constants/UuidConstants.h b/include/up-cpp/datamodel/constants/UuidConstants.h index fcd3e98b6..05eb7d663 100644 --- a/include/up-cpp/datamodel/constants/UuidConstants.h +++ b/include/up-cpp/datamodel/constants/UuidConstants.h @@ -38,6 +38,34 @@ constexpr uint64_t MASK_32_BITS = 0xFFFFFFFF; constexpr uint64_t MASK_16_BITS = 0xFFFF; constexpr uint64_t MASK_14_BITS = 0x3FFF; +// number of digits needed to represent a given number of bits in base 16 +constexpr uint64_t LEN_16_BITS_IN_HEX = 4; +constexpr uint64_t LEN_32_BITS_IN_HEX = 8; +constexpr uint64_t LEN_48_BITS_IN_HEX = 12; + +// number of characters a valid uuid +constexpr uint64_t TOTAL_UUID_LENGTH = 36; +constexpr uint64_t LEN_MSB_IN_HEX = 8; +constexpr uint64_t LEN_LSB_IN_HEX = 4; +constexpr uint64_t LEN_VCANT_IN_HEX = 4; +constexpr uint64_t LEN_VARR_IN_HEX = 4; +constexpr uint64_t LEN_RAND_IN_HEX = 8; + +// number of bits represented by a single hex character +constexpr uint64_t LEN_HEX_TO_BIT = 4; + +// number of bits to represent uint64 +constexpr uint64_t LEN_UINT64_IN_BIT = sizeof(uint64_t) * 8; + +// expected positions of the '-' separators in a valid uuid +constexpr uint64_t POS_FIRST_SEPARATOR = LEN_MSB_IN_HEX; +constexpr uint64_t POS_SECOND_SEPARATOR = + POS_FIRST_SEPARATOR + LEN_LSB_IN_HEX + 1; +constexpr uint64_t POS_THIRD_SEPARATOR = + POS_SECOND_SEPARATOR + LEN_VCANT_IN_HEX + 1; +constexpr uint64_t POS_FOURTH_SEPARATOR = + POS_THIRD_SEPARATOR + LEN_VARR_IN_HEX + 1; + } // namespace uprotocol::datamodel #endif // UP_CPP_DATAMODEL_CONSTANTS_UUIDCONSTANTS_H diff --git a/include/up-cpp/datamodel/serializer/Uuid.h b/include/up-cpp/datamodel/serializer/Uuid.h index b5f91dd1f..9641aafa1 100644 --- a/include/up-cpp/datamodel/serializer/Uuid.h +++ b/include/up-cpp/datamodel/serializer/Uuid.h @@ -27,13 +27,13 @@ namespace uprotocol::datamodel::serializer::uuid { /// @brief Converts to and from a human-readable string representation of UUID struct AsString { - [[nodiscard]] static std::string serialize(v1::UUID); + [[nodiscard]] static std::string serialize(const v1::UUID&); [[nodiscard]] static v1::UUID deserialize(const std::string&); }; /// @brief Converts to and from byte vector representation of UUID struct AsBytes { - [[nodiscard]] static std::vector serialize(v1::UUID); + [[nodiscard]] static std::vector serialize(const v1::UUID&); [[nodiscard]] static v1::UUID deserialize(const std::vector&); }; diff --git a/include/up-cpp/datamodel/validator/UUri.h b/include/up-cpp/datamodel/validator/UUri.h index 840b543fd..8989a5cf3 100644 --- a/include/up-cpp/datamodel/validator/UUri.h +++ b/include/up-cpp/datamodel/validator/UUri.h @@ -163,7 +163,7 @@ isValidDefaultSource(const v1::UUri&); struct InvalidUUri : public std::invalid_argument { // Forward constructors template - InvalidUUri(Args&&... args) + explicit InvalidUUri(Args&&... args) : std::invalid_argument(std::forward(args)...) {} InvalidUUri(InvalidUUri&&) noexcept; diff --git a/include/up-cpp/datamodel/validator/Uuid.h b/include/up-cpp/datamodel/validator/Uuid.h index ab5b0430a..b415af760 100644 --- a/include/up-cpp/datamodel/validator/Uuid.h +++ b/include/up-cpp/datamodel/validator/Uuid.h @@ -54,13 +54,13 @@ using ValidationResult = std::tuple>; /// @{ /// @brief Checks if the provided UUID contains valid uP v8 UUID data. /// @returns True if the UUID has valid UUID data, false otherwise. -ValidationResult isUuid(v1::UUID); +ValidationResult isUuid(const v1::UUID&); /// @brief Checks if the provided UUID has expired based on the given TTL. /// @throws InvalidUuid if the UUID does not contain valid UUID data /// @returns True if the difference between the current system time and /// the the timestamp in the UUID is greater than the TTL. -ValidationResult isExpired(v1::UUID uuid, std::chrono::milliseconds ttl); +ValidationResult isExpired(const v1::UUID& uuid, std::chrono::milliseconds ttl); /// @} /// @name Inspection utilities @@ -68,30 +68,30 @@ ValidationResult isExpired(v1::UUID uuid, std::chrono::milliseconds ttl); /// @brief Gets the version field from a UUID object /// @throws InvalidUuid if the UUID does not contain valid UUID data /// @returns The UUID's version -uint8_t getVersion(v1::UUID); +uint8_t getVersion(const v1::UUID&); /// @brief Gets the variant field from a UUID object /// @throws InvalidUuid if the UUID does not contain valid UUID data /// @returns The UUID's variant -uint8_t getVariant(v1::UUID); +uint8_t getVariant(const v1::UUID&); /// @brief Gets the timestamp field from a UUID object /// @throws InvalidUuid if the UUID does not contain valid UUID data /// @returns The UUID's timestamp as a chrono::time_point for the system /// clock. -std::chrono::system_clock::time_point getTime(v1::UUID uuid); +std::chrono::system_clock::time_point getTime(const v1::UUID& uuid); /// @brief Gets the difference between a UUID's timestamp and the current /// time according to the system clock/ /// @throws InvalidUuid if the UUID does not contain valid UUID data /// @returns The age of the UUID in milliseconds -std::chrono::milliseconds getElapsedTime(v1::UUID); +std::chrono::milliseconds getElapsedTime(const v1::UUID& uuid); /// @brief Gets the time remaining before the UUID expires, based on the /// given TTL. /// @throws InvalidUuid if the UUID does not contain valid UUID data /// @returns Remaining time (ttl - getElapsedTime(uuid)) in milliseconds -std::chrono::milliseconds getRemainingTime(v1::UUID uuid, +std::chrono::milliseconds getRemainingTime(const v1::UUID& uuid, std::chrono::milliseconds ttl); /// @} diff --git a/include/up-cpp/transport/UTransport.h b/include/up-cpp/transport/UTransport.h index 299f92c9e..cb54f96ea 100644 --- a/include/up-cpp/transport/UTransport.h +++ b/include/up-cpp/transport/UTransport.h @@ -57,7 +57,7 @@ class UTransport { /// /// @see uprotocol::datamodel::validator::uri::isValidEntityUri() /// @see uprotocol::datamodel::validator::uri::InvalidUUri - explicit UTransport(const v1::UUri&); + explicit UTransport(v1::UUri); /// @brief Send a message. /// @@ -264,7 +264,7 @@ class UTransport { /// connection they represent. /// /// @param listener CallerHandle for the connection that has been broken. - virtual void cleanupListener(CallableConn listener); + virtual void cleanupListener(const CallableConn& listener); private: /// @brief URI for the entity owning this transport. @@ -274,14 +274,15 @@ class UTransport { const v1::UUri entity_uri_; }; -struct NullTransport : public std::invalid_argument { +struct NullTransport : std::invalid_argument { template - NullTransport(Args&&... args) + explicit NullTransport(Args&&... args) : std::invalid_argument(std::forward(args)...) {} template - NullTransport operator=(Args&&... args) { - return std::invalid_argument::operator=(std::forward(args)...); + NullTransport& operator=(Args&&... args) { + std::invalid_argument::operator=(std::forward(args)...); + return *this; } }; diff --git a/include/up-cpp/utils/CallbackConnection.h b/include/up-cpp/utils/CallbackConnection.h index 5d2455241..b0dbda58d 100644 --- a/include/up-cpp/utils/CallbackConnection.h +++ b/include/up-cpp/utils/CallbackConnection.h @@ -173,7 +173,7 @@ struct [[nodiscard]] Connection { } if constexpr (!std::is_void_v) { - return result; + return static_cast>(std::move(result)); } } @@ -193,7 +193,8 @@ struct [[nodiscard]] Connection { }; /// @brief Semi-private constructor. Use the static establish() instead. - Connection(std::shared_ptr cb, PrivateConstructToken) + Connection(std::shared_ptr cb, + PrivateConstructToken token [[maybe_unused]]) : callback_(cb) {} // Connection is only ever available wrapped in a std::shared_ptr. @@ -230,10 +231,22 @@ struct [[nodiscard]] Connection { /// reason. struct BadConnection : public std::runtime_error { template - BadConnection(Args&&... args) + explicit BadConnection(Args&&... args) : std::runtime_error(std::forward(args)...) {} }; +/// @brief Thrown if an empty std::function parameter was received +/// +/// A std::function can be empty. When an empty function is invoked, it will +/// throw std::bad_function_call. We can check earlier by casting the function +/// to a boolean. If the check fails, EmptyFunctionObject is thrown. This makes +/// the error appear earlier without waiting for invocation to occur. +struct EmptyFunctionObject : public std::invalid_argument { + template + explicit EmptyFunctionObject(Args&&... args) + : std::invalid_argument(std::forward(args)...) {} +}; + template struct [[nodiscard]] CalleeHandle { using Conn = Connection; @@ -245,7 +258,7 @@ struct [[nodiscard]] CalleeHandle { CalleeHandle(std::shared_ptr connection, std::shared_ptr callback, std::optional&& cleanup, - typename Conn::PrivateConstructToken) + typename Conn::PrivateConstructToken token [[maybe_unused]]) : connection_(connection), callback_(callback), cleanup_(std::move(cleanup)) { @@ -254,11 +267,21 @@ struct [[nodiscard]] CalleeHandle { "Attempted to create a connected CalleeHandle with bad " "connection pointer"); } + if (!callback_) { throw BadConnection( "Attempted to create a connected CalleeHandle with bad " "callback pointer"); } + + const auto& callback_obj = *callback_; + if (!callback_obj) { + throw EmptyFunctionObject("Callback function is empty"); + } + + if (cleanup_ && !cleanup_.value()) { + throw EmptyFunctionObject("Cleanup function is empty"); + } } /// @brief CalleeHandles can be move constructed @@ -311,7 +334,7 @@ struct [[nodiscard]] CalleeHandle { /// * False if the connection has been broken (i.e. This handle has /// been reset/moved, or all other references to the connection /// have been discarded) - bool isConnected() const { + [[nodiscard]] bool isConnected() const { auto locked_connection = connection_.lock(); return locked_connection && (*locked_connection); } @@ -329,7 +352,7 @@ struct [[nodiscard]] CalleeHandle { /// CallerHandle that needs to be corrected. struct BadCallerAccess : public std::logic_error { template - BadCallerAccess(Args&&... args) + explicit BadCallerAccess(Args&&... args) : std::logic_error(std::forward(args)...) {} }; @@ -342,7 +365,7 @@ struct [[nodiscard]] CallerHandle { /// @brief Creates a connected handle. Only usable by Connection CallerHandle(std::shared_ptr connection, - typename Conn::PrivateConstructToken) + typename Conn::PrivateConstructToken token [[maybe_unused]]) : connection_(connection) { if (!connection_) { throw BadConnection( @@ -368,7 +391,9 @@ struct [[nodiscard]] CallerHandle { /// * False if the connection has been broken (i.e. This handle has /// been reset/moved, or all other references to the connection /// have been discarded) - bool isConnected() const { return connection_ && (*connection_); } + [[nodiscard]] bool isConnected() const { + return connection_ && (*connection_); + } /// @throws BadCallerAccess if this handle has been default constructed OR /// reset() has left it without a valid conneciton pointer. @@ -418,7 +443,7 @@ struct InvokeResult { value_ = std::move(v); return *this; } - operator std::optional&&() && { return std::move(value_); } + explicit operator std::optional&&() && { return std::move(value_); } private: std::optional value_; diff --git a/include/up-cpp/utils/CyclicQueue.h b/include/up-cpp/utils/CyclicQueue.h index 2fc8ee13f..29739e94e 100644 --- a/include/up-cpp/utils/CyclicQueue.h +++ b/include/up-cpp/utils/CyclicQueue.h @@ -23,7 +23,7 @@ namespace uprotocol::utils { template class CyclicQueue final { public: - explicit CyclicQueue(const size_t max_size); + explicit CyclicQueue(size_t max_size); CyclicQueue(const CyclicQueue&) = delete; CyclicQueue& operator=(const CyclicQueue&) = delete; diff --git a/include/up-cpp/utils/Expected.h b/include/up-cpp/utils/Expected.h index 56e2e8ba8..a9a0482ae 100644 --- a/include/up-cpp/utils/Expected.h +++ b/include/up-cpp/utils/Expected.h @@ -31,7 +31,7 @@ static_assert(!__has_cpp_attribute(__cpp_lib_expected), /// @{ struct BadExpectedAccess : public std::runtime_error { template - BadExpectedAccess(Args&&... args) + explicit BadExpectedAccess(Args&&... args) : std::runtime_error(std::forward(args)...) {} }; @@ -41,7 +41,7 @@ template class Unexpected { public: constexpr Unexpected(const Unexpected&) = default; - constexpr Unexpected(Unexpected&&) = default; + constexpr Unexpected(Unexpected&&) noexcept = default; constexpr explicit Unexpected(const E& rhs) : storage_(rhs) {} constexpr explicit Unexpected(E&& rhs) : storage_(std::move(rhs)) {} @@ -57,14 +57,18 @@ class Unexpected { template class Expected { public: - template - constexpr Expected(Args&&... args) - : storage_(std::forward(args)...) {} + constexpr explicit Expected(T arg) : storage_(std::forward(arg)) {} + // It E and T are the same type, this can cause problems. Previously, this + // was in use by implicid conversion + // constexpr explicit Expected(E arg) : + // storage_(std::forward>(Unexpected(arg))) {} + constexpr explicit Expected(Unexpected arg) + : storage_(std::forward>(arg)) {} constexpr Expected(const Expected&) = default; - constexpr Expected(Expected&&) = default; + constexpr Expected(Expected&&) noexcept = default; - constexpr bool has_value() const noexcept { + [[nodiscard]] constexpr bool has_value() const noexcept { return std::holds_alternative(storage_); } @@ -79,44 +83,50 @@ class Expected { } constexpr const T& value() const& { - if (!has_value()) + if (!has_value()) { throw BadExpectedAccess( "Attempt to access value() when unexpected."); + } return std::get(storage_); } constexpr T value() && { - if (!has_value()) + if (!has_value()) { throw BadExpectedAccess( "Attempt to access value() when unexpected."); + } return std::move(std::get(storage_)); } constexpr const E& error() const& { - if (has_value()) + if (has_value()) { throw BadExpectedAccess( "Attempt to access error() when not unexpected."); + } return std::get>(storage_).error(); } constexpr E error() && { - if (has_value()) + if (has_value()) { throw BadExpectedAccess( "Attempt to access error() when not unexpected."); + } return std::move(std::get>(storage_)).error(); } constexpr const T& operator*() const { - if (!has_value()) + if (!has_value()) { throw BadExpectedAccess( "Attempt to dereference expected value when unexpected."); + } return std::get(storage_); } constexpr const T* operator->() const { - if (!has_value()) + if (!has_value()) { throw BadExpectedAccess( "Attempt to dereference expected pointer when unexpected."); + } return &std::get(storage_); } diff --git a/include/up-cpp/utils/IpAddress.h b/include/up-cpp/utils/IpAddress.h index b8ed8c735..420810474 100644 --- a/include/up-cpp/utils/IpAddress.h +++ b/include/up-cpp/utils/IpAddress.h @@ -59,7 +59,7 @@ struct IpAddress { /// @brief Constructs an IP address from a string representation of /// an address. - explicit IpAddress(std::string_view const ip_string); + explicit IpAddress(std::string_view ip_string); /// @brief Constructs an IP address from a binary representation of /// an address. @@ -83,10 +83,10 @@ struct IpAddress { [[nodiscard]] std::string getBytesString() const; /// @brief Number of bytes in IPv4 address. - static constexpr uint8_t ip_v4_address_bytes = 4; + static constexpr uint8_t IP_V4_ADDRESS_BYTES = 4; /// @brief Number of bytes in IPv6 address. - static constexpr uint8_t ip_v6_address_bytes = 16; + static constexpr uint8_t IP_V6_ADDRESS_BYTES = 16; private: /// @brief Updates the state of this instance from the value of the diff --git a/include/up-cpp/utils/ProtoConverter.h b/include/up-cpp/utils/ProtoConverter.h new file mode 100644 index 000000000..ee4123936 --- /dev/null +++ b/include/up-cpp/utils/ProtoConverter.h @@ -0,0 +1,58 @@ +#ifndef UP_CPP_UTILS_PROTOCONVERTER_H +#define UP_CPP_UTILS_PROTOCONVERTER_H + +#include +#include + +#include +#include + +namespace uprotocol::utils { +using uprotocol::core::usubscription::v3::SubscribeAttributes; +using uprotocol::core::usubscription::v3::SubscriberInfo; +using uprotocol::core::usubscription::v3::SubscriptionRequest; +using uprotocol::core::usubscription::v3::UnsubscribeRequest; + +struct ProtoConverter { + /// @brief Converts std::chrono::time_point to google::protobuf::Timestamp + /// + /// @param tp the time point to convert + /// @return the converted google::protobuf::Timestamp + static google::protobuf::Timestamp ConvertToProtoTimestamp( + const std::chrono::system_clock::time_point& tp); + + /// @brief Builds a SubscriberInfo from the given parameters + /// + /// @param entity_uri the UUri of the entity subscribing + /// @return the built SubscriberInfo + static SubscriberInfo BuildSubscriberInfo(const v1::UUri& entity_uri); + + /// @brief Builds a SubscribeAttributes from the given parameters + /// + /// @param when_expire the optional time point when the subscription expires + /// @param subscription_details the details of the subscription + /// @param sample_period_ms the optional sample period in milliseconds + /// @return the built SubscribeAttributes + static SubscribeAttributes BuildSubscribeAttributes( + std::optional when_expire, + std::optional subscription_details, + std::optional sample_period_ms); + + /// @brief Builds a SubscriptionRequest from the given parameters + /// + /// @param subscription_topic the UUri of the topic to subscribe to + /// @param attributes the SubscribeAttributes for the subscription + /// @return the built SubscriptionRequest + static SubscriptionRequest BuildSubscriptionRequest( + const v1::UUri& subscription_topic, + std::optional attributes = {}); + + /// @brief Builds a UnsubscribeRequest from the given parameters + /// + /// @param subscription_topic the UUri of the topic to unsubscribe from + /// @return the built UnsubscribeRequest + static UnsubscribeRequest BuildUnSubscribeRequest( + const v1::UUri& subscription_topic); +}; +}; // namespace uprotocol::utils +#endif // UP_CPP_UTILS_PROTOCONVERTER_H diff --git a/include/up-cpp/utils/ThreadPool.h b/include/up-cpp/utils/ThreadPool.h index d1d6cde46..f888509cf 100644 --- a/include/up-cpp/utils/ThreadPool.h +++ b/include/up-cpp/utils/ThreadPool.h @@ -32,7 +32,7 @@ class ThreadPool { ThreadPool& operator=(const ThreadPool&) = delete; ThreadPool& operator=(ThreadPool&&) = delete; - ThreadPool(const size_t max_queue_size, const size_t max_num_of_threads, + ThreadPool(size_t max_queue_size, size_t max_num_of_threads, std::chrono::milliseconds task_timeout); ~ThreadPool(); diff --git a/include/up-cpp/utils/base64.h b/include/up-cpp/utils/base64.h index 88453460e..77b2cf7bf 100644 --- a/include/up-cpp/utils/base64.h +++ b/include/up-cpp/utils/base64.h @@ -18,7 +18,7 @@ #include /// @brief Utilities for encoding / decoding data in Base 64 format. -namespace uprotocol::utils::Base64 { +namespace uprotocol::utils::base64 { /// @brief Encode a string of bytes to base64. std::string encode(std::string_view); @@ -44,6 +44,6 @@ size_t encodedLen(std::vector); /// decoded data. size_t decodedLen(std::string_view); -} // namespace uprotocol::utils::Base64 +} // namespace uprotocol::utils::base64 #endif // UP_CPP_UTILS_BASE64_H diff --git a/lint/clang-format.sh b/lint/clang-format.sh index afaf1c838..f66c92d55 100755 --- a/lint/clang-format.sh +++ b/lint/clang-format.sh @@ -2,11 +2,11 @@ PROJECT_ROOT="$(realpath "$(dirname "$0")/../")" -if [ -n "$(which clang-format-12)" ]; then +if [ -n "$(which clang-format-13)" ]; then # NOTE: Using clang-format-12 in CI system, too - FORMATTER=clang-format-12 + FORMATTER=clang-format-13 elif [ -n "$(which clang-format)" ]; then - echo "Did not find clang-format-12. Trying clang-format. Results may not" + echo "Did not find clang-format-13. Trying clang-format. Results may not" echo "match formatting in GitHub CI process." FORMATTER=clang-format else diff --git a/lint/clang-tidy.sh b/lint/clang-tidy.sh index 67e9242e8..e2b490758 100755 --- a/lint/clang-tidy.sh +++ b/lint/clang-tidy.sh @@ -2,11 +2,11 @@ PROJECT_ROOT="$(realpath "$(dirname "$0")/../")" -if [ -n "$(which clang-tidy-12)" ]; then - # NOTE: Using clang-tidy-12 in CI system, too - LINTER=clang-tidy-12 +if [ -n "$(which clang-tidy-13)" ]; then + # NOTE: Using clang-tidy-13 in CI system, too + LINTER=clang-tidy-13 elif [ -n "$(which clang-tidy)" ]; then - echo "Did not find clang-tidy-12. Trying clang-tidy. Results may not" + echo "Did not find clang-tidy-13. Trying clang-tidy. Results may not" echo "match formatting in GitHub CI process." LINTER=clang-tidy else @@ -58,7 +58,7 @@ if [ -z "$target_source" ]; then shopt -s globstar pushd "$PROJECT_ROOT" > /dev/null - for f in **/*.h **/*.cpp; do + for f in include/**/*.h src/**/*.cpp; do # test/coverage/**/*.cpp test/extra/**/*.cpp test/include/**/*.h; do if [[ ! ("$f" =~ "build/") ]]; then echo echo "Checking file '$f'" diff --git a/src/client/usubscription/v3/Consumer.cpp b/src/client/usubscription/v3/Consumer.cpp new file mode 100644 index 000000000..0217b63ea --- /dev/null +++ b/src/client/usubscription/v3/Consumer.cpp @@ -0,0 +1,156 @@ +// SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache License Version 2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include + +namespace uprotocol::client::usubscription::v3 { + +Consumer::Consumer(std::shared_ptr transport, + v1::UUri subscription_topic, + ConsumerOptions consumer_options) + : transport_(std::move(transport)), + subscription_topic_(std::move(subscription_topic)), + consumer_options_(std::move(consumer_options)), + rpc_client_(nullptr) { + // Initialize uSubscriptionUUriBuilder_ + uSubscriptionUUriBuilder_ = USubscriptionUUriBuilder(); +} + +[[nodiscard]] Consumer::ConsumerOrStatus Consumer::create( + std::shared_ptr transport, + const v1::UUri& subscription_topic, ListenCallback&& callback, + v1::UPriority priority, std::chrono::milliseconds subscription_request_ttl, + ConsumerOptions consumer_options) { + auto consumer = std::make_unique( + std::forward>(transport), + std::forward(subscription_topic), + std::forward(consumer_options)); + + // Attempt to connect create notification sink for updates. + auto status = consumer->createNotificationSink(); + if (status.code() == v1::UCode::OK) { + status = consumer->subscribe(priority, subscription_request_ttl, + std::move(callback)); + if (status.code() == v1::UCode::OK) { + return ConsumerOrStatus(std::move(consumer)); + } + return ConsumerOrStatus(utils::Unexpected(status)); + } + // If connection fails, return the error status. + return ConsumerOrStatus(utils::Unexpected(status)); +} + +v1::UStatus Consumer::createNotificationSink() { + auto notification_sink_callback = [this](const v1::UMessage& update) { + if (update.has_payload()) { + Update data; + if (data.ParseFromString(update.payload())) { + if (data.topic().SerializeAsString() == + subscription_topic_.SerializeAsString()) { + subscription_update_ = std::move(data); + } + } + } + }; + + auto notification_topic = uSubscriptionUUriBuilder_.getNotificationUri(); + + auto result = communication::NotificationSink::create( + transport_, std::move(notification_sink_callback), notification_topic); + + if (result.has_value()) { + noficationSinkHandle_ = std::move(result).value(); + v1::UStatus status; + status.set_code(v1::UCode::OK); + return status; + } + return result.error(); +} + +SubscriptionRequest Consumer::buildSubscriptionRequest() { + auto attributes = utils::ProtoConverter::BuildSubscribeAttributes( + consumer_options_.when_expire, consumer_options_.subscription_details, + consumer_options_.sample_period_ms); + + auto subscription_request = utils::ProtoConverter::BuildSubscriptionRequest( + subscription_topic_, attributes); + return subscription_request; +} + +v1::UStatus Consumer::subscribe( + v1::UPriority priority, std::chrono::milliseconds subscription_request_ttl, + ListenCallback&& callback) { + rpc_client_ = std::make_unique( + transport_, uSubscriptionUUriBuilder_.getServiceUriWithResourceId(1), + priority, subscription_request_ttl); + + auto on_response = [this](const auto& maybe_response) { + if (maybe_response.has_value() && + maybe_response.value().has_payload()) { + SubscriptionResponse response; + if (response.ParseFromString(maybe_response.value().payload())) { + if (response.topic().SerializeAsString() == + subscription_topic_.SerializeAsString()) { + subscription_response_ = response; + } + } + } + }; + + SubscriptionRequest const subscription_request = buildSubscriptionRequest(); + auto payload = datamodel::builder::Payload(subscription_request); + + rpc_handle_ = + rpc_client_->invokeMethod(std::move(payload), std::move(on_response)); + + // Create a L2 subscription + auto result = communication::Subscriber::subscribe( + transport_, subscription_topic_, std::move(callback)); + + if (result.has_value()) { + subscriber_ = std::move(result).value(); + v1::UStatus status; + status.set_code(v1::UCode::OK); + return status; + } + return result.error(); +} + +UnsubscribeRequest Consumer::buildUnsubscriptionRequest() { + auto unsubscribe_request = + utils::ProtoConverter::BuildUnSubscribeRequest(subscription_topic_); + return unsubscribe_request; +} + +void Consumer::unsubscribe(v1::UPriority priority, + std::chrono::milliseconds request_ttl) { + rpc_client_ = std::make_unique( + transport_, uSubscriptionUUriBuilder_.getServiceUriWithResourceId(2), + priority, request_ttl); + + auto on_response = [](const auto& maybe_response) { + if (!maybe_response.has_value()) { + // Do something as this means sucessfully unsubscribed. + } + }; + + UnsubscribeRequest const unsubscribe_request = buildUnsubscriptionRequest(); + auto payload = datamodel::builder::Payload(unsubscribe_request); + + rpc_handle_ = + rpc_client_->invokeMethod(std::move(payload), std::move(on_response)); + + subscriber_.reset(); +} + +} // namespace uprotocol::client::usubscription::v3 \ No newline at end of file diff --git a/src/communication/NotificationSink.cpp b/src/communication/NotificationSink.cpp index 27d7bafa4..08043f3d6 100644 --- a/src/communication/NotificationSink.cpp +++ b/src/communication/NotificationSink.cpp @@ -19,8 +19,8 @@ namespace uprotocol::communication { namespace UriValidator = datamodel::validator::uri; NotificationSink::SinkOrStatus NotificationSink::create( - std::shared_ptr transport, ListenCallback&& callback, - const uprotocol::v1::UUri& source_filter) { + const std::shared_ptr& transport, + ListenCallback&& callback, const uprotocol::v1::UUri& source_filter) { // Standard check - transport pointer cannot be null if (!transport) { throw transport::NullTransport("transport cannot be null"); @@ -40,17 +40,16 @@ NotificationSink::SinkOrStatus NotificationSink::create( std::move(callback), source_filter, transport->getEntityUri()); if (!listener) { - return uprotocol::utils::Unexpected(listener.error()); + return SinkOrStatus(utils::Unexpected(listener.error())); } - return std::make_unique( - std::forward>(transport), - std::forward(std::move(listener).value())); + return SinkOrStatus(std::make_unique( + transport, std::forward(std::move(listener).value()))); } // NOTE: deprecated NotificationSink::SinkOrStatus NotificationSink::create( - std::shared_ptr transport, + const std::shared_ptr& transport, const uprotocol::v1::UUri& sink, ListenCallback&& callback, std::optional&& source_filter) { // Standard check - transport pointer cannot be null diff --git a/src/communication/NotificationSource.cpp b/src/communication/NotificationSource.cpp index 262320312..c5cbb863b 100644 --- a/src/communication/NotificationSource.cpp +++ b/src/communication/NotificationSource.cpp @@ -12,7 +12,8 @@ #include "up-cpp/communication/NotificationSource.h" namespace uprotocol::communication { -using namespace uprotocol::datamodel::builder; + +using uprotocol::datamodel::builder::UMessageBuilder; NotificationSource::NotificationSource( std::shared_ptr transport, v1::UUri&& source, @@ -40,7 +41,7 @@ v1::UStatus NotificationSource::notify( datamodel::builder::Payload&& payload) const { auto message = notify_builder_.build(std::move(payload)); - return transport_->send(std::move(message)); + return transport_->send(message); } v1::UStatus NotificationSource::notify() const { @@ -49,7 +50,7 @@ v1::UStatus NotificationSource::notify() const { throw transport::NullTransport("transport cannot be null"); } - return transport_->send(std::move(message)); + return transport_->send(message); } } // namespace uprotocol::communication \ No newline at end of file diff --git a/src/communication/Publisher.cpp b/src/communication/Publisher.cpp index 5bf30f7e7..55a055caa 100644 --- a/src/communication/Publisher.cpp +++ b/src/communication/Publisher.cpp @@ -14,7 +14,7 @@ #include namespace uprotocol::communication { -using namespace uprotocol::datamodel::builder; +using uprotocol::datamodel::builder::UMessageBuilder; Publisher::Publisher(std::shared_ptr transport, v1::UUri&& topic, v1::UPayloadFormat format, @@ -40,7 +40,7 @@ v1::UStatus Publisher::publish(datamodel::builder::Payload&& payload) const { throw transport::NullTransport("transport cannot be null"); } - return transport_->send(std::move(message)); + return transport_->send(message); } } // namespace uprotocol::communication \ No newline at end of file diff --git a/src/communication/RpcClient.cpp b/src/communication/RpcClient.cpp index 4a0b061bf..87239a71f 100644 --- a/src/communication/RpcClient.cpp +++ b/src/communication/RpcClient.cpp @@ -15,24 +15,39 @@ #include #include +#include namespace { namespace detail { -using namespace uprotocol; +using uprotocol::v1::UStatus; +using ListenHandle = uprotocol::transport::UTransport::ListenHandle; struct PendingRequest { - std::chrono::steady_clock::time_point when_expire; - transport::UTransport::ListenHandle response_listener; - std::function expire; - size_t instance_id; + friend struct ScrubablePendingQueue; + friend struct ExpireWorker; + + PendingRequest(const std::chrono::steady_clock::time_point& when_expire, + ListenHandle response_listener, + std::function expire, + const size_t& instance_id) + : when_expire_(when_expire), + response_listener_(std::move(response_listener)), + expire_(std::move(expire)), + instance_id_(instance_id) {} auto operator>(const PendingRequest& other) const; + +private: + std::chrono::steady_clock::time_point when_expire_; + ListenHandle response_listener_; + std::function expire_; + size_t instance_id_{}; }; struct ScrubablePendingQueue : public std::priority_queue, - std::greater> { + std::greater<>> { ~ScrubablePendingQueue(); auto scrub(size_t instance_id); PendingRequest& top(); @@ -41,7 +56,7 @@ struct ScrubablePendingQueue struct ExpireWorker { ExpireWorker(); ~ExpireWorker(); - void enqueue(PendingRequest&& request); + void enqueue(PendingRequest&& pending); void scrub(size_t instance_id); void doWork(); @@ -60,25 +75,25 @@ namespace uprotocol::communication { //////////////////////////////////////////////////////////////////////////////// struct RpcClient::ExpireService { - ExpireService() : instance_id_(next_instance_id++) {} + ExpireService() : instance_id_(next_instance_id_++) {} - ~ExpireService() { worker.scrub(instance_id_); } + ~ExpireService() { worker_.scrub(instance_id_); } void enqueue(std::chrono::steady_clock::time_point when_expire, transport::UTransport::ListenHandle&& response_listener, - std::function expire) { - detail::PendingRequest pending; - pending.when_expire = when_expire; - pending.response_listener = std::move(response_listener); - pending.expire = std::move(expire); - pending.instance_id = instance_id_; - - worker.enqueue(std::move(pending)); + std::function expire) const { + auto pending = + detail::PendingRequest(when_expire, std::move(response_listener), + std::move(expire), instance_id_); + + worker_.enqueue(std::move(pending)); } private: - static inline std::atomic next_instance_id{0}; - static inline detail::ExpireWorker worker; + static inline std::atomic next_instance_id_{0}; + // constructor for ExpireWorker can throw an exception when trying to create + // new thread this can be problematic when used in a static constructor + static inline detail::ExpireWorker worker_; // NOLINT size_t instance_id_; }; @@ -89,7 +104,7 @@ RpcClient::RpcClient(std::shared_ptr transport, std::optional payload_format, std::optional permission_level, std::optional token) - : transport_(transport), + : transport_(std::move(transport)), ttl_(ttl), builder_(datamodel::builder::UMessageBuilder::request( std::move(method), v1::UUri(transport_->getEntityUri()), priority, @@ -124,8 +139,9 @@ RpcClient::InvokeHandle RpcClient::invokeMethod(v1::UMessage&& request, // attempt to call the callback succeeds. auto callback_once = std::make_shared(); - auto [callback_handle, callable] = - Connection::establish(std::move(callback)); + auto connected_pair = Connection::establish(std::move(callback)); + auto callback_handle = std::move(std::get<0>(connected_pair)); + auto callable = std::get<1>(connected_pair); /////////////////////////////////////////////////////////////////////////// // Wraps the callback to handle receive filtering and commstatus checking. @@ -145,10 +161,11 @@ RpcClient::InvokeHandle RpcClient::invokeMethod(v1::UMessage&& request, v1::UStatus status; status.set_code(m.attributes().commstatus()); status.set_message("Received response with !OK commstatus"); - std::call_once(*callback_once, [&callable, - status = std::move(status)]() { - callable(utils::Unexpected(std::move(status))); - }); + std::call_once( + *callback_once, [&callable, status = std::move(status)]() { + callable(utils::Expected( + utils::Unexpected(status))); + }); } } }; @@ -158,10 +175,11 @@ RpcClient::InvokeHandle RpcClient::invokeMethod(v1::UMessage&& request, // Called when the request has expired or failed. Will be handed off to the // expiration monitoring service once the request has been sent. auto expire = [callable, callback_once](v1::UStatus&& reason) mutable { - std::call_once( - *callback_once, [&callable, reason = std::move(reason)]() { - callable(utils::Unexpected(std::move(reason))); - }); + std::call_once(*callback_once, + [&callable, reason = std::move(reason)]() { + callable(utils::Expected( + utils::Unexpected(reason))); + }); }; /////////////////////////////////////////////////////////////////////////// @@ -182,7 +200,7 @@ RpcClient::InvokeHandle RpcClient::invokeMethod(v1::UMessage&& request, } } - return std::move(callback_handle); + return callback_handle; } RpcClient::InvokeHandle RpcClient::invokeMethod( @@ -203,10 +221,11 @@ RpcClient::InvokeFuture RpcClient::invokeMethod( // allows exactly one call to the callback via std::call_once. auto promise = std::make_shared>(); auto future = promise->get_future(); - auto handle = invokeMethod( - std::move(payload), [promise](MessageOrStatus maybe_message) mutable { - promise->set_value(maybe_message); - }); + auto handle = + invokeMethod(std::move(payload), + [promise](const MessageOrStatus& maybe_message) mutable { + promise->set_value(maybe_message); + }); return {std::move(future), std::move(handle)}; } @@ -219,14 +238,14 @@ RpcClient::InvokeFuture RpcClient::invokeMethod() { auto promise = std::make_shared>(); auto future = promise->get_future(); auto handle = - invokeMethod([promise](MessageOrStatus maybe_message) mutable { + invokeMethod([promise](const MessageOrStatus& maybe_message) mutable { promise->set_value(maybe_message); }); return {std::move(future), std::move(handle)}; } -RpcClient::RpcClient(RpcClient&&) = default; +RpcClient::RpcClient(RpcClient&&) noexcept = default; RpcClient::~RpcClient() = default; RpcClient::InvokeFuture::InvokeFuture() = default; @@ -244,17 +263,19 @@ RpcClient::InvokeFuture::InvokeFuture(std::future&& future, namespace { namespace detail { -using namespace uprotocol; -using namespace std::chrono_literals; +using uprotocol::v1::UCode; +using uprotocol::v1::UStatus; +// using namespace std::chrono_literals; +using ListenHandle = uprotocol::transport::UTransport::ListenHandle; auto PendingRequest::operator>(const PendingRequest& other) const { - return when_expire > other.when_expire; + return when_expire_ > other.when_expire_; } ScrubablePendingQueue::~ScrubablePendingQueue() { - const v1::UStatus cancel_reason = []() { - v1::UStatus reason; - reason.set_code(v1::UCode::INTERNAL); + const UStatus cancel_reason = []() { + UStatus reason; + reason.set_code(UCode::INTERNAL); reason.set_message( "ERROR: ExpireWorker has shut down while requests are still " "pending. This should never occur and likely indicates that an " @@ -263,30 +284,30 @@ ScrubablePendingQueue::~ScrubablePendingQueue() { }(); for (auto& pending : c) { - pending.expire(cancel_reason); + pending.expire_(cancel_reason); } } auto ScrubablePendingQueue::scrub(size_t instance_id) { // Collect all the expire lambdas so they can be called without the // lock held. - std::vector> all_expired; + std::vector> all_expired; c.erase( std::remove_if(c.begin(), c.end(), [instance_id, &all_expired](const PendingRequest& p) { - if (instance_id == p.instance_id) { - all_expired.push_back(p.expire); + if (instance_id == p.instance_id_) { + all_expired.push_back(p.expire_); return true; } return false; }), c.end()); - // TODO - is there a better way to shrink the internal container? - // Maybe instead we should enforce a capacity limit - constexpr size_t capacity_shrink_threshold = 16; - if ((c.capacity() > capacity_shrink_threshold) && + // TODO(missing_author) - is there a better way to shrink the internal + // container? Maybe instead we should enforce a capacity limit + constexpr size_t CAPACITY_SHRINK_THRESHOLD = 16; + if ((c.capacity() > CAPACITY_SHRINK_THRESHOLD) && (c.size() < c.capacity() / 2)) { c.shrink_to_fit(); } @@ -304,29 +325,29 @@ ExpireWorker::ExpireWorker() { ExpireWorker::~ExpireWorker() { stop_ = true; { - std::lock_guard lock(pending_mtx_); + std::lock_guard const lock(pending_mtx_); wake_worker_.notify_one(); } worker_.join(); } void ExpireWorker::enqueue(PendingRequest&& pending) { - std::lock_guard lock(pending_mtx_); + std::lock_guard const lock(pending_mtx_); pending_.emplace(std::move(pending)); wake_worker_.notify_one(); } void ExpireWorker::scrub(size_t instance_id) { - std::vector> all_expired; + std::vector> all_expired; { - std::lock_guard lock(pending_mtx_); + std::lock_guard const lock(pending_mtx_); all_expired = pending_.scrub(instance_id); wake_worker_.notify_one(); } - static const v1::UStatus cancel_reason = []() { - v1::UStatus reason; - reason.set_code(v1::UCode::CANCELLED); + static const UStatus cancel_reason = []() { + UStatus reason; + reason.set_code(UCode::CANCELLED); reason.set_message("RpcClient for this request was discarded"); return reason; }(); @@ -339,17 +360,17 @@ void ExpireWorker::scrub(size_t instance_id) { void ExpireWorker::doWork() { while (!stop_) { const auto now = std::chrono::steady_clock::now(); - std::optional maybe_expire; + std::optional maybe_expire; { - transport::UTransport::ListenHandle expired_handle; - std::lock_guard lock(pending_mtx_); + ListenHandle expired_handle; + std::lock_guard const lock(pending_mtx_); if (!pending_.empty()) { - const auto when_expire = pending_.top().when_expire; + const auto when_expire = pending_.top().when_expire_; if (when_expire <= now) { - maybe_expire = std::move(pending_.top().expire); + maybe_expire = std::move(pending_.top().expire_); expired_handle = - std::move(pending_.top().response_listener); + std::move(pending_.top().response_listener_); pending_.pop(); } } @@ -358,9 +379,9 @@ void ExpireWorker::doWork() { if (maybe_expire) { auto& expire = *maybe_expire; - static const v1::UStatus expire_reason = []() { - v1::UStatus reason; - reason.set_code(v1::UCode::DEADLINE_EXCEEDED); + static const UStatus expire_reason = []() { + UStatus reason; + reason.set_code(UCode::DEADLINE_EXCEEDED); reason.set_message("Request expired before response received"); return reason; }(); @@ -379,11 +400,11 @@ void ExpireWorker::doWork() { // priority queue (either by insertion or deletion) // * The queue has been emptied (loop back to indefinite sleep) // * A stop has been requested - auto wake_when = pending_.top().when_expire; + auto wake_when = pending_.top().when_expire_; wake_worker_.wait_until(lock, wake_when, [this, &wake_when]() { auto when_next_wake = wake_when; if (!pending_.empty()) { - when_next_wake = pending_.top().when_expire; + when_next_wake = pending_.top().when_expire_; } return stop_ || when_next_wake != wake_when || pending_.empty() || diff --git a/src/communication/RpcServer.cpp b/src/communication/RpcServer.cpp index a56b7489f..54c540417 100644 --- a/src/communication/RpcServer.cpp +++ b/src/communication/RpcServer.cpp @@ -20,7 +20,7 @@ RpcServer::RpcServer(std::shared_ptr transport, std::optional ttl) : transport_(std::move(transport)), ttl_(ttl), - expected_payload_format_(std::move(format)) { + expected_payload_format_(format) { if (!transport_) { throw transport::NullTransport("transport cannot be null"); } @@ -43,7 +43,7 @@ RpcServer::ServerOrStatus RpcServer::create( v1::UStatus status; status.set_code(v1::UCode::INVALID_ARGUMENT); status.set_message("Invalid rpc URI"); - return uprotocol::utils::Unexpected(status); + return ServerOrStatus(utils::Unexpected(status)); } // Validate the payload format, if provided. @@ -53,7 +53,7 @@ RpcServer::ServerOrStatus RpcServer::create( v1::UStatus status; status.set_code(v1::UCode::OUT_OF_RANGE); status.set_message("Invalid payload format"); - return uprotocol::utils::Unexpected(status); + return ServerOrStatus(utils::Unexpected(status)); } } @@ -67,11 +67,10 @@ RpcServer::ServerOrStatus RpcServer::create( auto status = server->connect(method_name, std::move(callback)); if (status.code() == v1::UCode::OK) { // If connection is successful, return the server instance. - return server; - } else { - // If connection fails, return the error status. - return uprotocol::utils::Unexpected(std::move(status)); + return ServerOrStatus(std::move(server)); } + // If connection fails, return the error status. + return ServerOrStatus(utils::Unexpected(status)); } v1::UStatus RpcServer::connect(const v1::UUri& method, RpcCallback&& callback) { @@ -96,7 +95,7 @@ v1::UStatus RpcServer::connect(const v1::UUri& method, RpcCallback&& callback) { datamodel::builder::UMessageBuilder::response(request); // Call the RPC callback method with the request message. - auto payloadData = callback_(request); + auto payload_data = callback_(request); if (ttl_.has_value()) { builder.withTtl(ttl_.value()); @@ -108,7 +107,7 @@ v1::UStatus RpcServer::connect(const v1::UUri& method, RpcCallback&& callback) { // Check for payload data requirement based on expected format // presence - if (!payloadData.has_value()) { + if (!payload_data.has_value()) { // builder.build() verifies if payload format is required auto response = builder.build(); // Ignoring status code for transport send @@ -116,7 +115,7 @@ v1::UStatus RpcServer::connect(const v1::UUri& method, RpcCallback&& callback) { } else { // builder.build(payloadData) verifies if payload format // matches the expected - auto response = builder.build(std::move(payloadData).value()); + auto response = builder.build(std::move(payload_data).value()); // Ignoring status code for transport send std::ignore = transport_->send(response); } @@ -126,9 +125,13 @@ v1::UStatus RpcServer::connect(const v1::UUri& method, RpcCallback&& callback) { v1::UUri any_uri; any_uri.set_authority_name("*"); // Instance ID 0 and UE ID FFFF for wildcard - any_uri.set_ue_id(0x0000FFFF); - any_uri.set_ue_version_major(0xFF); - any_uri.set_resource_id(0xFFFF); + constexpr auto DEFAULT_INSTANCE_ID_WITH_WILDCARD_SERVICE_ID = + 0x0000FFFF; + constexpr auto VERSION_MAJOR_WILDCARD = 0xFF; + constexpr auto RESOURCE_ID_WILDCARD = 0xFFFF; + any_uri.set_ue_id(DEFAULT_INSTANCE_ID_WITH_WILDCARD_SERVICE_ID); + any_uri.set_ue_version_major(VERSION_MAJOR_WILDCARD); + any_uri.set_resource_id(RESOURCE_ID_WILDCARD); return any_uri; }(), // sink_filter= @@ -141,9 +144,8 @@ v1::UStatus RpcServer::connect(const v1::UUri& method, RpcCallback&& callback) { v1::UStatus status; status.set_code(v1::UCode::OK); return status; - } else { - return result.error(); } + return result.error(); } } // namespace uprotocol::communication diff --git a/src/communication/Subscriber.cpp b/src/communication/Subscriber.cpp index e2275eac4..14ca07a3b 100644 --- a/src/communication/Subscriber.cpp +++ b/src/communication/Subscriber.cpp @@ -11,10 +11,12 @@ #include "up-cpp/communication/Subscriber.h" +#include + #include "up-cpp/datamodel/validator/UUri.h" namespace uprotocol::communication { -namespace UriValidator = uprotocol::datamodel::validator::uri; +namespace uri_validator = uprotocol::datamodel::validator::uri; [[nodiscard]] Subscriber::SubscriberOrStatus Subscriber::subscribe( std::shared_ptr transport, const v1::UUri& topic, @@ -25,27 +27,28 @@ namespace UriValidator = uprotocol::datamodel::validator::uri; } auto [source_ok, bad_source_reason] = - UriValidator::isValidSubscription(topic); + uri_validator::isValidSubscription(topic); if (!source_ok) { - throw UriValidator::InvalidUUri( + throw uri_validator::InvalidUUri( "Topic URI is not a valid topic subscription pattern | " + - std::string(UriValidator::message(*bad_source_reason))); + std::string(uri_validator::message(*bad_source_reason))); } auto handle = transport->registerListener(std::move(callback), topic); if (!handle) { - return uprotocol::utils::Unexpected(handle.error()); + return SubscriberOrStatus( + utils::Unexpected(handle.error())); } - return std::make_unique( + return SubscriberOrStatus(std::make_unique( std::forward>(transport), - std::forward(std::move(handle).value())); + std::forward(std::move(handle).value()))); } Subscriber::Subscriber(std::shared_ptr transport, ListenHandle&& subscription) - : transport_(transport), subscription_(std::move(subscription)) { + : transport_(std::move(transport)), subscription_(std::move(subscription)) { // Constructor body. Any additional setup can go here. if (!transport_) { throw transport::NullTransport("transport cannot be null"); diff --git a/src/datamodel/builder/Payload.cpp b/src/datamodel/builder/Payload.cpp index 42f43ad19..865ea9ff5 100644 --- a/src/datamodel/builder/Payload.cpp +++ b/src/datamodel/builder/Payload.cpp @@ -27,7 +27,7 @@ Payload::Payload(const std::string& value, const v1::UPayloadFormat format) { if (!UPayloadFormat_IsValid(format)) { throw std::out_of_range("Invalid String payload format"); } - payload_ = std::make_tuple(std::move(value), format); + payload_ = std::make_tuple(value, format); } // Move string constructor @@ -46,27 +46,29 @@ Payload::Payload(Serialized&& serialized) { payload_ = std::move(serialized); } +// google::protobuf::Any constructor +Payload::Payload(const google::protobuf::Any& any) { + payload_ = std::make_tuple( + any.SerializeAsString(), + uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY); +} + // Move constructor Payload::Payload(Payload&& other) noexcept - : payload_(std::move(other.payload_)), moved_(std::move(other.moved_)) {} + : payload_(std::move(other.payload_)), moved_(other.moved_) {} // Copy constructor -Payload::Payload(const Payload& other) - : payload_(other.payload_), moved_(other.moved_) {} +Payload::Payload(const Payload& other) = default; // Move assignment operator Payload& Payload::operator=(Payload&& other) noexcept { payload_ = std::move(other.payload_); - moved_ = std::move(other.moved_); + moved_ = other.moved_; return *this; } // Copy assignment operator -Payload& Payload::operator=(const Payload& other) { - payload_ = other.payload_; - moved_ = other.moved_; - return *this; -} +Payload& Payload::operator=(const Payload& other) = default; Payload::PayloadMoved::PayloadMoved(PayloadMoved&& other) noexcept : std::runtime_error(std::move(other)) {} @@ -79,15 +81,11 @@ Payload::PayloadMoved& Payload::PayloadMoved::operator=( } // PayloadMoved copy constructor -Payload::PayloadMoved::PayloadMoved(const PayloadMoved& other) - : std::runtime_error(other) {} +Payload::PayloadMoved::PayloadMoved(const PayloadMoved& other) = default; // PayloadMoved copy assignment operator Payload::PayloadMoved& Payload::PayloadMoved::operator=( - const PayloadMoved& other) { - std::runtime_error::operator=(other); - return *this; -} + const PayloadMoved& other) = default; // buildCopy method [[nodiscard]] const Payload::Serialized& Payload::buildCopy() const { diff --git a/src/datamodel/builder/UMessage.cpp b/src/datamodel/builder/UMessage.cpp index c2d433f25..7653a02c5 100644 --- a/src/datamodel/builder/UMessage.cpp +++ b/src/datamodel/builder/UMessage.cpp @@ -111,11 +111,11 @@ UMessageBuilder UMessageBuilder::response(v1::UUri&& sink, UMessageBuilder UMessageBuilder::response(const v1::UMessage& request) { v1::UUri sink = request.attributes().source(); - v1::UUID reqId = request.attributes().id(); + v1::UUID req_id = request.attributes().id(); v1::UPriority priority = request.attributes().priority(); v1::UUri method = request.attributes().sink(); - return UMessageBuilder::response(std::move(sink), std::move(reqId), + return UMessageBuilder::response(std::move(sink), std::move(req_id), priority, std::move(method)); } @@ -224,11 +224,11 @@ v1::UMessage UMessageBuilder::build(builder::Payload&& payload) const { return message; } -UMessageBuilder::UMessageBuilder(v1::UMessageType msgType, v1::UUri&& source, +UMessageBuilder::UMessageBuilder(v1::UMessageType msg_type, v1::UUri&& source, std::optional&& sink, std::optional&& request_id) : uuidBuilder_(UuidBuilder::getBuilder()) { - attributes_.set_type(msgType); + attributes_.set_type(msg_type); *attributes_.mutable_source() = std::move(source); diff --git a/src/datamodel/serializer/UUri.cpp b/src/datamodel/serializer/UUri.cpp index 78232fd7a..4b0351fe7 100644 --- a/src/datamodel/serializer/UUri.cpp +++ b/src/datamodel/serializer/UUri.cpp @@ -18,7 +18,10 @@ namespace uprotocol::datamodel::serializer::uri { std::string AsString::serialize(const v1::UUri& uri) { - using namespace uprotocol::datamodel::validator::uri; + using uprotocol::datamodel::validator::uri::InvalidUUri; + using uprotocol::datamodel::validator::uri::isLocal; + using uprotocol::datamodel::validator::uri::isValidFilter; + // isValidFilter is the most permissive of the validators auto [valid, reason] = isValidFilter(uri); if (!valid) { @@ -37,23 +40,24 @@ std::string AsString::serialize(const v1::UUri& uri) { return std::move(ss).str(); } -std::string_view extractSegment(std::string_view& uriView) { - constexpr std::string_view segment_separator = "/"; - const auto end = uriView.find(segment_separator); - if (end == uriView.npos) { +std::string_view extractSegment(std::string_view& uri_view) { + constexpr std::string_view SEGMENT_SEPARATOR = "/"; + const auto end = uri_view.find(SEGMENT_SEPARATOR); + if (end == std::string_view::npos) { throw std::invalid_argument("Could not extract segment from '" + - std::string(uriView) + + std::string(uri_view) + "' with separator '" + "/" + "'"); } - auto segment = uriView.substr(0, end); - uriView = uriView.substr(end + 1); + auto segment = uri_view.substr(0, end); + uri_view = uri_view.substr(end + 1); return segment; } uint32_t segmentToUint32(const std::string_view& segment) { uint32_t value = 0; + constexpr auto HEX_BASE = 16; auto [end, ec] = std::from_chars( - segment.data(), segment.data() + segment.size(), value, 16); + segment.data(), segment.data() + segment.size(), value, HEX_BASE); const bool convert_ok = (ec == std::errc{}) && (end == segment.data() + segment.size()); if (!convert_ok) { @@ -63,51 +67,52 @@ uint32_t segmentToUint32(const std::string_view& segment) { return value; } -uprotocol::v1::UUri AsString::deserialize(const std::string& uriAsString) { - if (uriAsString.empty()) { +uprotocol::v1::UUri AsString::deserialize(const std::string& uri_as_string) { + if (uri_as_string.empty()) { throw std::invalid_argument("Cannot deserialize empty string"); } - constexpr std::string_view schema_prefix = "up://"; - constexpr std::string_view remote_prefix = "//"; - constexpr std::string_view segment_separator = "/"; + constexpr std::string_view SCHEMA_PREFIX = "up://"; + constexpr std::string_view REMOTE_PREFIX = "//"; + constexpr std::string_view SEGMENT_SEPARATOR = "/"; // Operating on a string view to avoid copies and reallocations - std::string_view uriView(uriAsString); + std::string_view uri_view(uri_as_string); // Extract and convert the rest of the URI string v1::UUri uri; // Verify start and extract Authority, if present // With up:// schema - if (uriView.substr(0, schema_prefix.size()) == schema_prefix) { + if (uri_view.substr(0, SCHEMA_PREFIX.size()) == SCHEMA_PREFIX) { // Advance past the prefix - uriView = uriView.substr(schema_prefix.size()); - uri.set_authority_name(std::string(extractSegment(uriView))); + uri_view = uri_view.substr(SCHEMA_PREFIX.size()); + uri.set_authority_name(std::string(extractSegment(uri_view))); // with // remote prefix - } else if (uriView.substr(0, remote_prefix.size()) == remote_prefix) { + } else if (uri_view.substr(0, REMOTE_PREFIX.size()) == REMOTE_PREFIX) { // Advance past the prefix - uriView = uriView.substr(remote_prefix.size()); - uri.set_authority_name(std::string(extractSegment(uriView))); + uri_view = uri_view.substr(REMOTE_PREFIX.size()); + uri.set_authority_name(std::string(extractSegment(uri_view))); // with / local prefix - } else if (uriView.substr(0, segment_separator.size()) == - segment_separator) { + } else if (uri_view.substr(0, SEGMENT_SEPARATOR.size()) == + SEGMENT_SEPARATOR) { // Advance past the prefix - uriView = uriView.substr(segment_separator.size()); + uri_view = uri_view.substr(SEGMENT_SEPARATOR.size()); // Missing required prefix } else { throw std::invalid_argument( "Did not find expected URI start in string: '" + - std::string(uriView) + "'"); + std::string(uri_view) + "'"); } - uri.set_ue_id(segmentToUint32(extractSegment(uriView))); - uri.set_ue_version_major(segmentToUint32(extractSegment(uriView))); - uri.set_resource_id(segmentToUint32(uriView)); + uri.set_ue_id(segmentToUint32(extractSegment(uri_view))); + uri.set_ue_version_major(segmentToUint32(extractSegment(uri_view))); + uri.set_resource_id(segmentToUint32(uri_view)); { - using namespace uprotocol::datamodel::validator::uri; + using uprotocol::datamodel::validator::uri::InvalidUUri; + using uprotocol::datamodel::validator::uri::isValidFilter; // isValidFilter is the most permissive of the validators auto [valid, reason] = isValidFilter(uri); if (!valid) { diff --git a/src/datamodel/serializer/Uuid.cpp b/src/datamodel/serializer/Uuid.cpp index 781a781ee..26c0f0dbf 100644 --- a/src/datamodel/serializer/Uuid.cpp +++ b/src/datamodel/serializer/Uuid.cpp @@ -21,15 +21,15 @@ namespace { constexpr uint64_t RANDOM_B_SHIFT = 48; -constexpr size_t MSB_HIGH_ = 0; -constexpr size_t MSB_LOW__ = 4; -constexpr size_t LSB_HIGH_ = 8; -constexpr size_t LSB_LOW__ = 12; +constexpr size_t MSB_HIGH = 0; +constexpr size_t MSB_LOW = 4; +constexpr size_t LSB_HIGH = 8; +constexpr size_t LSB_LOW = 12; } // namespace namespace uprotocol::datamodel::serializer::uuid { -std::string AsString::serialize(const uprotocol::v1::UUID uuid) { +std::string AsString::serialize(const uprotocol::v1::UUID& uuid) { // Extracting the parts of the UUIDv7 uint64_t unix_ts_ms = (uuid.msb() >> UUID_TIMESTAMP_SHIFT) & UUID_TIMESTAMP_MASK; @@ -40,19 +40,20 @@ std::string AsString::serialize(const uprotocol::v1::UUID uuid) { // Formatting the UUIDv8 in the traditional format std::stringstream ss; - ss << std::hex << std::setfill('0') << std::setw(8) - << ((unix_ts_ms >> 16) & MASK_32_BITS) // First 32 bits of timestamp - << "-" << std::setw(4) + ss << std::hex << std::setfill('0') << std::setw(LEN_32_BITS_IN_HEX) + << ((unix_ts_ms >> LEN_LSB_IN_HEX * LEN_HEX_TO_BIT) & + MASK_32_BITS) // First 32 bits of timestamp + << "-" << std::setw(LEN_16_BITS_IN_HEX) << ((unix_ts_ms)&MASK_16_BITS) // Next 16 bits of timestamp i.e. last 16 // bits of ts - << "-" << std::setw(4) + << "-" << std::setw(LEN_16_BITS_IN_HEX) << (((ver & UUID_VERSION_MASK) << UUID_VERSION_SHIFT) | (rand_a & UUID_RANDOM_A_MASK)) // Last 16 bits of timestamp and version - << "-" << std::setw(4) - << (((var & UUID_VARIANT_MASK) << 14) | + << "-" << std::setw(LEN_16_BITS_IN_HEX) + << (((var & UUID_VARIANT_MASK) << (LEN_HEX_TO_BIT * 3 + 2)) | ((rand_b >> RANDOM_B_SHIFT) & MASK_14_BITS)) // Variant and randb - << "-" << std::setw(12) + << "-" << std::setw(LEN_48_BITS_IN_HEX) << (rand_b & UUID_TIMESTAMP_MASK); // Random number return std::move(ss).str(); @@ -72,8 +73,9 @@ uprotocol::v1::UUID AsString::deserialize(const std::string& str) { // Please check UP-spec for UUID formatting: // https://github.com/eclipse-uprotocol/up-spec/blob/main/basics/uuid.adoc - if (str.length() != 36 || str[8] != '-' || str[13] != '-' || - str[18] != '-' || str[23] != '-') { + if (str.length() != TOTAL_UUID_LENGTH || str[POS_FIRST_SEPARATOR] != '-' || + str[POS_SECOND_SEPARATOR] != '-' || str[POS_THIRD_SEPARATOR] != '-' || + str[POS_FOURTH_SEPARATOR] != '-') { throw std::invalid_argument("Invalid UUID string format"); } @@ -86,45 +88,54 @@ uprotocol::v1::UUID AsString::deserialize(const std::string& str) { try { // Extract the parts from the UUID string - unix_ts_ms = std::stoull(str.substr(0, 8), nullptr, HEX_BASE) << 16; - unix_ts_ms |= std::stoull(str.substr(9, 4), nullptr, HEX_BASE); + unix_ts_ms = + std::stoull(str.substr(0, LEN_MSB_IN_HEX), nullptr, HEX_BASE) + << LEN_LSB_IN_HEX * LEN_HEX_TO_BIT; + unix_ts_ms |= + std::stoull(str.substr(POS_FIRST_SEPARATOR + 1, LEN_LSB_IN_HEX), + nullptr, HEX_BASE); uint16_t msb_low = static_cast( - std::stoul(str.substr(14, 4), nullptr, HEX_BASE)); - ver = (msb_low >> 12) & UUID_VERSION_MASK; + std::stoul(str.substr(POS_SECOND_SEPARATOR + 1, LEN_VCANT_IN_HEX), + nullptr, HEX_BASE)); + ver = (msb_low >> LEN_HEX_TO_BIT * 3) & UUID_VERSION_MASK; rand_a = msb_low & UUID_RANDOM_A_MASK; uint16_t var_randb = static_cast( - std::stoul(str.substr(19, 4), nullptr, HEX_BASE)); - var = (var_randb >> 14) & UUID_VARIANT_MASK; + std::stoul(str.substr(POS_THIRD_SEPARATOR + 1, LEN_VARR_IN_HEX), + nullptr, HEX_BASE)); + var = (var_randb >> (LEN_HEX_TO_BIT * 3 + 2)) & UUID_VARIANT_MASK; rand_b = static_cast(var_randb & MASK_14_BITS) << RANDOM_B_SHIFT; - rand_b |= std::stoull(str.substr(24), nullptr, HEX_BASE); + rand_b |= std::stoull(str.substr(POS_FOURTH_SEPARATOR + 1), nullptr, + HEX_BASE); } catch (const std::exception& e) { throw std::invalid_argument("Invalid UUID string format"); } // Reconstruct the UUID uuid.set_msb((unix_ts_ms << UUID_TIMESTAMP_SHIFT) | - (ver << UUID_VERSION_SHIFT) | rand_a); + ((static_cast(ver) << UUID_VERSION_SHIFT)) | rand_a); uuid.set_lsb((static_cast(var) << UUID_VARIANT_SHIFT) | rand_b); return uuid; } // Serialization function -std::vector AsBytes::serialize(const v1::UUID uuid) { +std::vector AsBytes::serialize(const v1::UUID& uuid) { std::vector bytes(UUID_BYTE_SIZE); - uint32_t msb_high = htonl(static_cast(uuid.msb() >> 32)); + uint32_t msb_high = + htonl(static_cast(uuid.msb() >> LEN_UINT64_IN_BIT / 2)); uint32_t msb_low = htonl(static_cast(uuid.msb() & MASK_32_BITS)); - uint32_t lsb_high = htonl(static_cast(uuid.lsb() >> 32)); + uint32_t lsb_high = + htonl(static_cast(uuid.lsb() >> LEN_UINT64_IN_BIT / 2)); uint32_t lsb_low = htonl(static_cast(uuid.lsb() & MASK_32_BITS)); - std::memcpy(&bytes[MSB_HIGH_], &msb_high, UUID_PART_SIZE); - std::memcpy(&bytes[MSB_LOW__], &msb_low, UUID_PART_SIZE); - std::memcpy(&bytes[LSB_HIGH_], &lsb_high, UUID_PART_SIZE); - std::memcpy(&bytes[LSB_LOW__], &lsb_low, UUID_PART_SIZE); + std::memcpy(&bytes[MSB_HIGH], &msb_high, UUID_PART_SIZE); + std::memcpy(&bytes[MSB_LOW], &msb_low, UUID_PART_SIZE); + std::memcpy(&bytes[LSB_HIGH], &lsb_high, UUID_PART_SIZE); + std::memcpy(&bytes[LSB_LOW], &lsb_low, UUID_PART_SIZE); return bytes; } @@ -134,17 +145,22 @@ v1::UUID AsBytes::deserialize(const std::vector& bytes) { throw std::invalid_argument("Invalid UUID byte array size"); } - uint32_t msb_high, msb_low, lsb_high, lsb_low; + uint32_t msb_high = 0; + uint32_t msb_low = 0; + uint32_t lsb_high = 0; + uint32_t lsb_low = 0; - std::memcpy(&msb_high, &bytes[MSB_HIGH_], UUID_PART_SIZE); - std::memcpy(&msb_low, &bytes[MSB_LOW__], UUID_PART_SIZE); - std::memcpy(&lsb_high, &bytes[LSB_HIGH_], UUID_PART_SIZE); - std::memcpy(&lsb_low, &bytes[LSB_LOW__], UUID_PART_SIZE); + std::memcpy(&msb_high, &bytes[MSB_HIGH], UUID_PART_SIZE); + std::memcpy(&msb_low, &bytes[MSB_LOW], UUID_PART_SIZE); + std::memcpy(&lsb_high, &bytes[LSB_HIGH], UUID_PART_SIZE); + std::memcpy(&lsb_low, &bytes[LSB_LOW], UUID_PART_SIZE); uint64_t msb = - (static_cast(ntohl(msb_high)) << 32) | ntohl(msb_low); + (static_cast(ntohl(msb_high)) << LEN_UINT64_IN_BIT / 2) | + ntohl(msb_low); uint64_t lsb = - (static_cast(ntohl(lsb_high)) << 32) | ntohl(lsb_low); + (static_cast(ntohl(lsb_high)) << LEN_UINT64_IN_BIT / 2) | + ntohl(lsb_low); v1::UUID uuid; uuid.set_msb(msb); diff --git a/src/datamodel/validator/UMessage.cpp b/src/datamodel/validator/UMessage.cpp index d8eab414b..6a9db80c7 100644 --- a/src/datamodel/validator/UMessage.cpp +++ b/src/datamodel/validator/UMessage.cpp @@ -18,8 +18,8 @@ namespace uprotocol::datamodel::validator::message { -using namespace uprotocol::v1; -using namespace uprotocol::datamodel::validator; +using uprotocol::v1::UPRIORITY_CS4; +// using uprotocol::datamodel::validator; std::string_view message(Reason reason) { switch (reason) { diff --git a/src/datamodel/validator/UUri.cpp b/src/datamodel/validator/UUri.cpp index cdd7d1dfb..f623b0f66 100644 --- a/src/datamodel/validator/UUri.cpp +++ b/src/datamodel/validator/UUri.cpp @@ -14,8 +14,13 @@ namespace { constexpr size_t AUTHORITY_SPEC_MAX_LENGTH = 128; +// TODO(max) try to find a better name +constexpr auto START_OF_TOPICS = 0x8000; +constexpr auto MAX_RESOURCE_ID = 0xFFFF; + +using uprotocol::datamodel::validator::uri::Reason; +using uprotocol::datamodel::validator::uri::ValidationResult; -using namespace uprotocol::datamodel::validator::uri; ValidationResult uriCommonValidChecks(const uprotocol::v1::UUri& uuri) { if (uuri.ue_version_major() == 0) { return {false, Reason::RESERVED_VERSION}; @@ -68,19 +73,25 @@ std::string_view message(Reason reason) { } bool uses_wildcards(const v1::UUri& uuri) { - if (uuri.authority_name().find("*") != std::string::npos) { + constexpr auto LOWER_8_BIT_MASK = 0xFF; + constexpr auto LOWER_16_BIT_MASK = 0xFFFF; + constexpr auto UPPER_16_BIT_MASK = 0xFFFF0000; + + if (uuri.authority_name().find_first_of('*') != std::string::npos) { return true; } - if ((uuri.ue_id() & 0xFFFF) == 0xFFFF) { // service ID + if ((uuri.ue_id() & LOWER_16_BIT_MASK) == + LOWER_16_BIT_MASK) { // service ID return true; } - if ((uuri.ue_id() & 0xFFFF0000) == 0xFFFF0000) { // service instance ID + if ((uuri.ue_id() & UPPER_16_BIT_MASK) == + UPPER_16_BIT_MASK) { // service instance ID return true; } - if (uuri.ue_version_major() == 0xFF) { + if (uuri.ue_version_major() == LOWER_8_BIT_MASK) { return true; } - if (uuri.resource_id() == 0xFFFF) { + if (uuri.resource_id() == LOWER_16_BIT_MASK) { return true; } return false; @@ -139,7 +150,7 @@ ValidationResult isValidRpcMethod(const v1::UUri& uuri) { } // check resource ID [0x0001, 0x7FFF] - if (uuri.resource_id() == 0 || uuri.resource_id() > 0x7FFF) { + if (uuri.resource_id() == 0 || uuri.resource_id() >= START_OF_TOPICS) { return {false, Reason::BAD_RESOURCE_ID}; } @@ -176,7 +187,8 @@ ValidationResult isValidPublishTopic(const v1::UUri& uuri) { return {false, Reason::DISALLOWED_WILDCARD}; } - if ((uuri.resource_id() < 0x8000) || (uuri.resource_id() > 0xFFFF)) { + if ((uuri.resource_id() < START_OF_TOPICS) || + (uuri.resource_id() > MAX_RESOURCE_ID)) { return {false, Reason::BAD_RESOURCE_ID}; } @@ -189,7 +201,8 @@ ValidationResult isValidNotificationSource(const v1::UUri& uuri) { return {false, Reason::DISALLOWED_WILDCARD}; } - if ((uuri.resource_id() < 0x8000) || (uuri.resource_id() > 0xFFFF)) { + if ((uuri.resource_id() < START_OF_TOPICS) || + (uuri.resource_id() > MAX_RESOURCE_ID)) { return {false, Reason::BAD_RESOURCE_ID}; } @@ -210,7 +223,8 @@ ValidationResult isValidNotificationSink(const v1::UUri& uuri) { } ValidationResult isValidSubscription(const v1::UUri& uuri) { - if (uuri.resource_id() < 0x8000 || uuri.resource_id() > 0xFFFF) { + if (uuri.resource_id() < START_OF_TOPICS || + uuri.resource_id() > MAX_RESOURCE_ID) { return {false, Reason::BAD_RESOURCE_ID}; } diff --git a/src/datamodel/validator/Uuid.cpp b/src/datamodel/validator/Uuid.cpp index 26e4f7b15..2530045cb 100644 --- a/src/datamodel/validator/Uuid.cpp +++ b/src/datamodel/validator/Uuid.cpp @@ -15,17 +15,16 @@ using namespace std::chrono_literals; -namespace { +namespace uprotocol::datamodel { -using namespace uprotocol::datamodel; -using milliseconds = std::chrono::milliseconds; +using Milliseconds = std::chrono::milliseconds; std::chrono::system_clock::time_point getUuidTimestamp( const uprotocol::v1::UUID& uuid) { uint64_t msb = uuid.msb(); uint64_t timestamp = (msb >> UUID_TIMESTAMP_SHIFT) & UUID_TIMESTAMP_MASK; - return std::chrono::system_clock::time_point(milliseconds(timestamp)); + return std::chrono::system_clock::time_point(Milliseconds(timestamp)); } uint8_t internalGetVersion(const uprotocol::v1::UUID& uuid) { @@ -36,7 +35,7 @@ uint8_t internalGetVariant(const uprotocol::v1::UUID& uuid) { return (uuid.lsb() >> UUID_VARIANT_SHIFT) & UUID_VARIANT_MASK; } -} // namespace +} // namespace uprotocol::datamodel namespace uprotocol::datamodel::validator::uuid { @@ -55,7 +54,7 @@ std::string_view message(Reason reason) { } } -ValidationResult isUuid(const uprotocol::v1::UUID uuid) { +ValidationResult isUuid(const uprotocol::v1::UUID& uuid) { uint8_t version = internalGetVersion(uuid); if (version != UUID_VERSION_7) { return {false, Reason::WRONG_VERSION}; @@ -76,7 +75,7 @@ ValidationResult isUuid(const uprotocol::v1::UUID uuid) { return {true, std::nullopt}; } -ValidationResult isExpired(const uprotocol::v1::UUID uuid, +ValidationResult isExpired(const uprotocol::v1::UUID& uuid, std::chrono::milliseconds ttl) { auto [valid, reason] = isUuid(uuid); if (!valid) { @@ -93,7 +92,7 @@ ValidationResult isExpired(const uprotocol::v1::UUID uuid, return {false, std::nullopt}; } -uint8_t getVersion(const uprotocol::v1::UUID uuid) { +uint8_t getVersion(const uprotocol::v1::UUID& uuid) { auto [valid, reason] = isUuid(uuid); if (!valid) { throw InvalidUuid(message(reason.value())); @@ -101,7 +100,7 @@ uint8_t getVersion(const uprotocol::v1::UUID uuid) { return internalGetVersion(uuid); } -uint8_t getVariant(const uprotocol::v1::UUID uuid) { +uint8_t getVariant(const uprotocol::v1::UUID& uuid) { auto [valid, reason] = isUuid(uuid); if (!valid) { throw InvalidUuid(message(reason.value())); @@ -109,7 +108,7 @@ uint8_t getVariant(const uprotocol::v1::UUID uuid) { return internalGetVariant(uuid); } -std::chrono::system_clock::time_point getTime(const uprotocol::v1::UUID uuid) { +std::chrono::system_clock::time_point getTime(const uprotocol::v1::UUID& uuid) { auto [valid, reason] = isUuid(uuid); if (!valid) { throw InvalidUuid(message(reason.value())); @@ -117,7 +116,7 @@ std::chrono::system_clock::time_point getTime(const uprotocol::v1::UUID uuid) { return getUuidTimestamp(uuid); } -std::chrono::milliseconds getElapsedTime(const uprotocol::v1::UUID uuid) { +std::chrono::milliseconds getElapsedTime(const uprotocol::v1::UUID& uuid) { auto [valid, reason] = isUuid(uuid); if (!valid) { throw InvalidUuid(message(reason.value())); @@ -126,10 +125,10 @@ std::chrono::milliseconds getElapsedTime(const uprotocol::v1::UUID uuid) { auto current_time = std::chrono::system_clock::now(); auto uuid_time = getTime(uuid); - return std::chrono::duration_cast(current_time - uuid_time); + return std::chrono::duration_cast(current_time - uuid_time); } -std::chrono::milliseconds getRemainingTime(const uprotocol::v1::UUID uuid, +std::chrono::milliseconds getRemainingTime(const uprotocol::v1::UUID& uuid, std::chrono::milliseconds ttl) { auto elapsed_time = getElapsedTime(uuid); return std::max(ttl - elapsed_time, 0ms); diff --git a/src/transport/UTransport.cpp b/src/transport/UTransport.cpp index 0904504ae..2f51eb823 100644 --- a/src/transport/UTransport.cpp +++ b/src/transport/UTransport.cpp @@ -11,30 +11,33 @@ #include "up-cpp/transport/UTransport.h" +#include + #include "up-cpp/datamodel/validator/UMessage.h" #include "up-cpp/datamodel/validator/UUri.h" #include "up-cpp/utils/Expected.h" namespace uprotocol::transport { -namespace UriValidator = uprotocol::datamodel::validator::uri; -namespace MessageValidator = uprotocol::datamodel::validator::message; +namespace uri_validator = uprotocol::datamodel::validator::uri; +namespace message_validator = uprotocol::datamodel::validator::message; -UTransport::UTransport(const v1::UUri& entity_uri) : entity_uri_(entity_uri) { - auto [uri_ok, reason] = UriValidator::isValidDefaultEntity(entity_uri_); +UTransport::UTransport(v1::UUri entity_uri) + : entity_uri_(std::move(entity_uri)) { + auto [uri_ok, reason] = uri_validator::isValidDefaultEntity(entity_uri_); if (!uri_ok) { - throw UriValidator::InvalidUUri( + throw uri_validator::InvalidUUri( "Transport's entity URI is not a valid URI | " + - std::string(UriValidator::message(*reason))); + std::string(uri_validator::message(*reason))); } } v1::UStatus UTransport::send(const v1::UMessage& message) { - auto [msgOk, reason] = MessageValidator::isValid(message); + auto [msgOk, reason] = message_validator::isValid(message); if (!msgOk) { - throw MessageValidator::InvalidUMessage( + throw message_validator::InvalidUMessage( "Invalid UMessage | " + - std::string(MessageValidator::message(*reason))); + std::string(message_validator::message(*reason))); } return sendImpl(message); @@ -56,27 +59,27 @@ UTransport::HandleOrStatus UTransport::registerListener( // Handle the special case of publish-like messages where only a source // address is provided. auto [source_ok, bad_source_reason] = - UriValidator::isValidSubscription(source_filter); + uri_validator::isValidSubscription(source_filter); if (!source_ok) { - throw UriValidator::InvalidUUri( + throw uri_validator::InvalidUUri( "source_filter is not a valid URI | " + - std::string(UriValidator::message(*bad_source_reason))); + std::string(uri_validator::message(*bad_source_reason))); } } else { auto [source_ok, bad_source_reason] = - UriValidator::isValidFilter(source_filter); + uri_validator::isValidFilter(source_filter); if (!source_ok) { - throw UriValidator::InvalidUUri( + throw uri_validator::InvalidUUri( "source_filter is not a valid URI | " + - std::string(UriValidator::message(*bad_source_reason))); + std::string(uri_validator::message(*bad_source_reason))); } auto [sink_ok, bad_sink_reason] = - UriValidator::isValidFilter(*sink_filter); + uri_validator::isValidFilter(*sink_filter); if (!sink_ok) { - throw UriValidator::InvalidUUri( + throw uri_validator::InvalidUUri( "sink_filter is not a valid URI | " + - std::string(UriValidator::message(*bad_sink_reason))); + std::string(uri_validator::message(*bad_sink_reason))); } } @@ -87,15 +90,14 @@ UTransport::HandleOrStatus UTransport::registerListener( std::move(callable), source_filter, std::move(sink_filter)); if (status.code() == v1::UCode::OK) { - return std::move(handle); - } else { - return utils::Unexpected(std::move(status)); + return HandleOrStatus(std::move(handle)); } + return HandleOrStatus(utils::Unexpected(status)); } // NOTE: deprecated utils::Expected -UTransport::registerListener(const v1::UUri& sink_filter, +UTransport::registerListener(const v1::UUri& sink_or_topic_filter, ListenCallback&& listener, std::optional&& source_filter) { if (!source_filter) { @@ -104,20 +106,19 @@ UTransport::registerListener(const v1::UUri& sink_filter, // as meaning that the sink_filter is a topic to subscribe to. This // will then pass the sink_filter as a source filter to the updated // registerListener(), which is then called with sink_filter unset. - return registerListener(std::move(listener), sink_filter, + return registerListener(std::move(listener), sink_or_topic_filter, std::move(source_filter)); - } else { - v1::UUri sink_filter_copy = sink_filter; - return registerListener(std::move(listener), *source_filter, - std::move(sink_filter_copy)); } + v1::UUri sink_filter_copy = sink_or_topic_filter; + return registerListener(std::move(listener), *source_filter, + std::move(sink_filter_copy)); } const v1::UUri& UTransport::getEntityUri() const { return entity_uri_; } const v1::UUri& UTransport::getDefaultSource() const { return getEntityUri(); } -void UTransport::cleanupListener(CallableConn listener) { +void UTransport::cleanupListener(const CallableConn& listener) { static_cast(listener); } diff --git a/src/utils/ProtoConverter.cpp b/src/utils/ProtoConverter.cpp new file mode 100644 index 000000000..c4159ee9f --- /dev/null +++ b/src/utils/ProtoConverter.cpp @@ -0,0 +1,79 @@ +#include "up-cpp/utils/ProtoConverter.h" + +namespace uprotocol::utils { +google::protobuf::Timestamp ProtoConverter::ConvertToProtoTimestamp( + const std::chrono::system_clock::time_point& tp) { + constexpr auto NANOSECONDS_PER_SECOND = 1000000000LL; + google::protobuf::Timestamp timestamp; + auto duration = tp.time_since_epoch(); + auto seconds = + std::chrono::duration_cast(duration).count(); + auto nanoseconds = + std::chrono::duration_cast(duration).count() % + NANOSECONDS_PER_SECOND; + + timestamp.set_seconds(seconds); + timestamp.set_nanos(static_cast(nanoseconds)); + + return timestamp; +} + +// SubscriberInfo builder +SubscriberInfo ProtoConverter::BuildSubscriberInfo(const v1::UUri& entity_uri) { + SubscriberInfo subscriber_info; + + // Create a new instance of UUri and copy the contents from entity_uri + *subscriber_info.mutable_uri() = entity_uri; + + return subscriber_info; +} + +// SubscribeAttributes builder +SubscribeAttributes ProtoConverter::BuildSubscribeAttributes( + std::optional when_expire, + std::optional subscription_details, + std::optional sample_period_ms) { + SubscribeAttributes attributes; + + if (when_expire.has_value()) { + *attributes.mutable_expire() = + ConvertToProtoTimestamp(when_expire.value()); + } + + if (subscription_details.has_value()) { + attributes.add_details()->CopyFrom(subscription_details.value()); + } + + if (sample_period_ms.has_value()) { + attributes.set_sample_period_ms( + static_cast(sample_period_ms.value().count())); + } + + return attributes; +} + +// SubscriptionRequest builder +SubscriptionRequest ProtoConverter::BuildSubscriptionRequest( + const v1::UUri& subscription_topic, + std::optional attributes) { + SubscriptionRequest subscription_request; + *subscription_request.mutable_topic() = subscription_topic; + + // Use mutable attributes if provided + if (attributes.has_value()) { + *subscription_request.mutable_attributes() = + std::move(attributes.value()); + } + + return subscription_request; +} + +UnsubscribeRequest ProtoConverter::BuildUnSubscribeRequest( + const v1::UUri& subscription_topic) { + UnsubscribeRequest unsubscribe_request; + *unsubscribe_request.mutable_topic() = subscription_topic; + + return unsubscribe_request; +} + +} // namespace uprotocol::utils diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0b04d4786..bd7e6d67f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -81,6 +81,9 @@ add_coverage_test("SubscriberTest" coverage/communication/SubscriberTest.cpp) add_coverage_test("NotificationSinkTest" coverage/communication/NotificationSinkTest.cpp) add_coverage_test("NotificationSourceTest" coverage/communication/NotificationSourceTest.cpp) +# client +add_coverage_test("ConsumerTest" coverage/client/usubscription/v3/ConsumerTest.cpp) + ########################## EXTRAS ############################################# add_extra_test("PublisherSubscriberTest" extra/PublisherSubscriberTest.cpp) add_extra_test("NotificationTest" extra/NotificationTest.cpp) diff --git a/test/coverage/client/usubscription/v3/ConsumerTest.cpp b/test/coverage/client/usubscription/v3/ConsumerTest.cpp new file mode 100644 index 000000000..1fd964cad --- /dev/null +++ b/test/coverage/client/usubscription/v3/ConsumerTest.cpp @@ -0,0 +1,199 @@ +// SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache License Version 2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include + +#include +#include + +#include "UTransportMock.h" + +namespace { +using MsgDiff = google::protobuf::util::MessageDifferencer; + +void someCallBack(const uprotocol::v1::UMessage& message) { + // Print the message + std::cout << message.DebugString() << std::endl; +} + +class ConsumerTest : public testing::Test { +protected: + // Run once per TEST_F. + // Used to set up clean environments per test. + std::shared_ptr mockTransportClient_; + std::shared_ptr mockTransportServer_; + uprotocol::v1::UUri client_uuri; + uprotocol::v1::UUri server_uuri; + uprotocol::v1::UUri subcription_uuri; + + void SetUp() override { + // Create a generic transport uri + client_uuri.set_authority_name("random_string"); + client_uuri.set_ue_id(0x18000); + client_uuri.set_ue_version_major(3); + client_uuri.set_resource_id(0); + + // Set up a transport + mockTransportClient_ = + std::make_shared(client_uuri); + + // Craete server default uri and set up a transport + server_uuri.set_authority_name("core.usubscription"); + server_uuri.set_ue_id(0); + server_uuri.set_ue_version_major(3); + server_uuri.set_resource_id(0); + + mockTransportServer_ = + std::make_shared(server_uuri); + + // Create a generic subscription uri + subcription_uuri.set_authority_name("10.0.0.2"); + subcription_uuri.set_ue_id(0x18000); + subcription_uuri.set_ue_version_major(3); + subcription_uuri.set_resource_id(0x8000); + }; + void TearDown() override {} + + // Run once per execution of the test application. + // Used for setup of all tests. Has access to this instance. + ConsumerTest() = default; + ~ConsumerTest() = default; + + void buildDefaultSourceURI(); + void buildValidNotificationURI(); + void buildInValidNotificationURI(); + + // Run once per execution of the test application. + // Used only for global setup outside of tests. + static void SetUpTestSuite() {} + static void TearDownTestSuite() {} +}; + +// Negative test case with no source filter +TEST_F(ConsumerTest, ConstructorTestSuccess) { + auto subcription_callback = someCallBack; + auto subscribe_request_ttl = std::chrono::milliseconds(1000); + auto priority = uprotocol::v1::UPriority::UPRIORITY_CS4; + + auto options = uprotocol::client::usubscription::v3::ConsumerOptions(); + + auto consumer_or_status = + uprotocol::client::usubscription::v3::Consumer::create( + mockTransportClient_, subcription_uuri, subcription_callback, + priority, subscribe_request_ttl, options); + + // Ensure that the consumer creation was successful + ASSERT_TRUE(consumer_or_status.has_value()); + + // Obtain a pointer to the created consumer instance + const auto& consumerPtr = consumer_or_status.value(); + + // Verify that the consumer pointer is not null, indicating successful + // creation + ASSERT_NE(consumerPtr, nullptr); +} + +TEST_F(ConsumerTest, SubscribeTestSuccess) { + auto subcriptionCallback = someCallBack; + auto subscribe_request_ttl = std::chrono::milliseconds(1000); + auto priority = uprotocol::v1::UPriority::UPRIORITY_CS4; + + auto options = uprotocol::client::usubscription::v3::ConsumerOptions(); + + auto consumerOrSatus = + uprotocol::client::usubscription::v3::Consumer::create( + mockTransportClient_, subcription_uuri, subcriptionCallback, + priority, subscribe_request_ttl, options); + + // Ensure that the consumer creation was successful + ASSERT_TRUE(consumerOrSatus.has_value()); + + // Obtain a pointer to the created consumer instance + auto& consumerPtr = consumerOrSatus.value(); + + // Verify that the consumer pointer is not null, indicating successful + // creation + ASSERT_NE(consumerPtr, nullptr); + + // Create notification source sink uri to match resource id of sink + auto notification_uuri = server_uuri; + notification_uuri.set_resource_id(0x8000); + + // set format UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY + auto format = + uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY; + + auto norificationSource = uprotocol::communication::NotificationSource( + mockTransportServer_, std::move(notification_uuri), + std::move(client_uuri), format); + // Build payload + const std::string data = "test"; + auto payload = uprotocol::datamodel::builder::Payload(data, format); + + norificationSource.notify(std::move(payload)); + + // Check send count + EXPECT_TRUE(mockTransportServer_->send_count_ == 1); + EXPECT_TRUE(mockTransportClient_->send_count_ == 1); +} + +TEST_F(ConsumerTest, UnsubscribeTestSuccess) { + auto subcriptionCallback = someCallBack; + auto subscribe_request_ttl = std::chrono::milliseconds(1000); + auto priority = uprotocol::v1::UPriority::UPRIORITY_CS4; + + auto options = uprotocol::client::usubscription::v3::ConsumerOptions(); + + auto consumerOrSatus = + uprotocol::client::usubscription::v3::Consumer::create( + mockTransportClient_, subcription_uuri, subcriptionCallback, + priority, subscribe_request_ttl, options); + + // Ensure that the consumer creation was successful + ASSERT_TRUE(consumerOrSatus.has_value()); + + // Obtain a pointer to the created consumer instance + const auto& consumerPtr = consumerOrSatus.value(); + + // Verify that the consumer pointer is not null, indicating successful + // creation + ASSERT_NE(consumerPtr, nullptr); + + // Create notification source sink uri to match resource id of sink + auto notification_uuri = server_uuri; + notification_uuri.set_resource_id(0x8000); + + // set format UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY + auto format = + uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY; + + auto norificationSource = uprotocol::communication::NotificationSource( + mockTransportServer_, std::move(notification_uuri), + std::move(client_uuri), format); + // Build payload + const std::string data = "test"; + auto payload = uprotocol::datamodel::builder::Payload(data, format); + + norificationSource.notify(std::move(payload)); + + // Check send count + EXPECT_TRUE(mockTransportServer_->send_count_ == 1); + EXPECT_TRUE(mockTransportClient_->send_count_ == 1); + + consumerPtr->unsubscribe(priority, subscribe_request_ttl); + + EXPECT_TRUE(mockTransportClient_->send_count_ == 2); +} + +} // namespace diff --git a/test/coverage/communication/NotificationSinkTest.cpp b/test/coverage/communication/NotificationSinkTest.cpp index a17da8722..0ab196d48 100644 --- a/test/coverage/communication/NotificationSinkTest.cpp +++ b/test/coverage/communication/NotificationSinkTest.cpp @@ -34,7 +34,7 @@ class NotificationSinkTest : public testing::Test { // Run once per execution of the test application. // Used for setup of all tests. Has access to this instance. NotificationSinkTest() = default; - ~NotificationSinkTest() = default; + ~NotificationSinkTest() override = default; void buildDefaultSourceURI(); void buildValidNotificationURI(); @@ -216,14 +216,23 @@ TEST_F(NotificationSinkTest, NullCallback) { testDefaultSourceUUri_); // bind to null callback - auto result = NotificationSink::create(transport, transport->getEntityUri(), - std::move(nullptr), testTopicUUri_); + auto test_create_nullptr = [transport, this]() { + std::ignore = + NotificationSink::create(transport, transport->getEntityUri(), + std::move(nullptr), testTopicUUri_); + }; + + using namespace uprotocol::utils; + + EXPECT_THROW(test_create_nullptr(), callbacks::EmptyFunctionObject); + + // Default construct a function object + auto test_create_empty = [transport, this]() { + std::ignore = NotificationSink::create( + transport, transport->getEntityUri(), {}, testTopicUUri_); + }; - uprotocol::v1::UMessage msg; - auto attr = std::make_shared(); - *msg.mutable_attributes() = *attr; - msg.set_payload(get_random_string(1400)); - EXPECT_THROW(transport->mockMessage(msg), std::bad_function_call); + EXPECT_THROW(test_create_empty(), callbacks::EmptyFunctionObject); } } // namespace diff --git a/test/coverage/communication/NotificationSourceTest.cpp b/test/coverage/communication/NotificationSourceTest.cpp index 1463876ef..97f13eecb 100644 --- a/test/coverage/communication/NotificationSourceTest.cpp +++ b/test/coverage/communication/NotificationSourceTest.cpp @@ -73,17 +73,17 @@ class TestNotificationSource : public testing::Test { }; TEST_F(TestNotificationSource, NotifyWithPayloadSuccess) { - std::string testPayloadStr = "test_payload"; - NotificationSource notificationSource(transportMock_, std::move(source_), - std::move(sink_), format_, priority_, - ttl_); - Payload testPayload(testPayloadStr, format_); + std::string test_payload_str = "test_payload"; + NotificationSource notification_source(transportMock_, std::move(source_), + std::move(sink_), format_, priority_, + ttl_); + Payload test_payload(test_payload_str, format_); uprotocol::v1::UStatus retval; retval.set_code(uprotocol::v1::UCode::OK); transportMock_->send_status_ = retval; - auto status = notificationSource.notify(std::move(testPayload)); + auto status = notification_source.notify(std::move(test_payload)); EXPECT_EQ(status.code(), retval.code()); @@ -94,16 +94,17 @@ TEST_F(TestNotificationSource, NotifyWithPayloadSuccess) { } TEST_F(TestNotificationSource, NotifyWithPayloadSuccessWithoutTTL) { - std::string testPayloadStr = "test_payload"; - NotificationSource notificationSource(transportMock_, std::move(source_), - std::move(sink_), format_, priority_); - Payload testPayload(testPayloadStr, format_); + std::string test_payload_str = "test_payload"; + NotificationSource notification_source(transportMock_, std::move(source_), + std::move(sink_), format_, + priority_); + Payload test_payload(test_payload_str, format_); uprotocol::v1::UStatus retval; retval.set_code(uprotocol::v1::UCode::OK); transportMock_->send_status_ = retval; - auto status = notificationSource.notify(std::move(testPayload)); + auto status = notification_source.notify(std::move(test_payload)); EXPECT_EQ(status.code(), retval.code()); @@ -116,17 +117,18 @@ TEST_F(TestNotificationSource, NotifyWithPayloadSuccessWithoutTTL) { } TEST_F(TestNotificationSource, NotifyWithPayloadSuccessWithoutPriority) { - std::string testPayloadStr = "test_payload"; + std::string test_payload_str = "test_payload"; priority_.reset(); - NotificationSource notificationSource(transportMock_, std::move(source_), - std::move(sink_), format_, priority_); - Payload testPayload(testPayloadStr, format_); + NotificationSource notification_source(transportMock_, std::move(source_), + std::move(sink_), format_, + priority_); + Payload test_payload(test_payload_str, format_); uprotocol::v1::UStatus retval; retval.set_code(uprotocol::v1::UCode::OK); transportMock_->send_status_ = retval; - auto status = notificationSource.notify(std::move(testPayload)); + auto status = notification_source.notify(std::move(test_payload)); EXPECT_EQ(status.code(), retval.code()); @@ -140,30 +142,30 @@ TEST_F(TestNotificationSource, NotifyWithPayloadSuccessWithoutPriority) { } TEST_F(TestNotificationSource, NotifyWithPayloadFailure) { - std::string testPayloadStr = "test_payload"; - NotificationSource notificationSource(transportMock_, std::move(source_), - std::move(sink_), format_, priority_, - ttl_); - Payload testPayload(testPayloadStr, format_); + std::string test_payload_str = "test_payload"; + NotificationSource notification_source(transportMock_, std::move(source_), + std::move(sink_), format_, priority_, + ttl_); + Payload test_payload(test_payload_str, format_); uprotocol::v1::UStatus retval; retval.set_code(uprotocol::v1::UCode::DATA_LOSS); transportMock_->send_status_ = retval; - auto status = notificationSource.notify(std::move(testPayload)); + auto status = notification_source.notify(std::move(test_payload)); EXPECT_EQ(status.code(), retval.code()); } TEST_F(TestNotificationSource, NotifyWithoutPayloadSuccess) { - NotificationSource notificationSource(transportMock_, std::move(source_), - std::move(sink_)); + NotificationSource notification_source(transportMock_, std::move(source_), + std::move(sink_)); uprotocol::v1::UStatus retval; retval.set_code(uprotocol::v1::UCode::OK); transportMock_->send_status_ = retval; - auto status = notificationSource.notify(); + auto status = notification_source.notify(); EXPECT_EQ(status.code(), retval.code()); @@ -173,14 +175,14 @@ TEST_F(TestNotificationSource, NotifyWithoutPayloadSuccess) { } TEST_F(TestNotificationSource, NotifyWithoutPayloadFailure) { - NotificationSource notificationSource(transportMock_, std::move(source_), - std::move(sink_)); + NotificationSource notification_source(transportMock_, std::move(source_), + std::move(sink_)); uprotocol::v1::UStatus retval; retval.set_code(uprotocol::v1::UCode::DATA_LOSS); transportMock_->send_status_ = retval; - auto status = notificationSource.notify(); + auto status = notification_source.notify(); EXPECT_EQ(status.code(), retval.code()); } diff --git a/test/coverage/communication/SubscriberTest.cpp b/test/coverage/communication/SubscriberTest.cpp index bb210d236..c5f8414cc 100644 --- a/test/coverage/communication/SubscriberTest.cpp +++ b/test/coverage/communication/SubscriberTest.cpp @@ -186,14 +186,21 @@ TEST_F(SubscriberTest, SubscribeNullCallback) { testDefaultSourceUUri_); // bind to null callback - auto result = - Subscriber::subscribe(transport, testTopicUUri_, std::move(nullptr)); + auto test_subscribe_nullptr = [transport, this]() { + std::ignore = Subscriber::subscribe(transport, testTopicUUri_, + std::move(nullptr)); + }; + + using namespace uprotocol::utils; + + EXPECT_THROW(test_subscribe_nullptr(), callbacks::EmptyFunctionObject); + + // Default construct a function object + auto test_subscribe_empty = [transport, this]() { + std::ignore = Subscriber::subscribe(transport, testTopicUUri_, {}); + }; - uprotocol::v1::UMessage msg; - auto attr = std::make_shared(); - *msg.mutable_attributes() = *attr; - msg.set_payload(get_random_string(1400)); - EXPECT_THROW(transport->mockMessage(msg), std::bad_function_call); + EXPECT_THROW(test_subscribe_empty(), callbacks::EmptyFunctionObject); } } // namespace diff --git a/test/coverage/datamodel/PayloadBuilderTest.cpp b/test/coverage/datamodel/PayloadBuilderTest.cpp index f624f817b..ac8d46da6 100644 --- a/test/coverage/datamodel/PayloadBuilderTest.cpp +++ b/test/coverage/datamodel/PayloadBuilderTest.cpp @@ -101,7 +101,7 @@ TEST_F(PayloadTest, CreateSerializedProtobufPayloadAndMoveTest) { // Act Payload payload(uriObject); - auto& [payload_reference, _] = payload.buildCopy(); + auto& [payload_reference, payload_format] = payload.buildCopy(); const void* original_address = payload_reference.data(); // Assert @@ -110,7 +110,7 @@ TEST_F(PayloadTest, CreateSerializedProtobufPayloadAndMoveTest) { uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF); EXPECT_EQ(payloadData, expectedPayloadData); - EXPECT_THROW(auto _ = payload.buildCopy(), Payload::PayloadMoved); + EXPECT_THROW(auto result = payload.buildCopy(), Payload::PayloadMoved); EXPECT_EQ(original_address, payloadData.data()); } @@ -130,7 +130,8 @@ TEST_F(PayloadTest, CreateSerializedProtobufPayloadAndMoveTwiceExceptionTest) { uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF); EXPECT_EQ(payloadData, uriObject.SerializeAsString()); - EXPECT_THROW(std::move(payload).buildMove(), Payload::PayloadMoved); + EXPECT_THROW({ auto _ = std::move(payload).buildMove(); }, + Payload::PayloadMoved); } // Create serialized protobuf payload. Call build after move. @@ -340,6 +341,31 @@ TEST_F(PayloadTest, StringMovePayloadTest) { EXPECT_THROW(auto _ = payload.buildCopy(), Payload::PayloadMoved); } +// Create Any and move payload object test +TEST_F(PayloadTest, AnyMovePayloadTest) { // NOLINT + // Arrange + uprotocol::v1::UUri uri_object; // NOLINT + uri_object.set_authority_name(testStringPayload_); + google::protobuf::Any any; + any.PackFrom(uri_object, "hello_world"); + + // Act + Payload payload(any); + auto [serialized_data, payload_format] = std::move(payload).buildMove(); + + // Assert + EXPECT_EQ( + payload_format, + uprotocol::v1::UPayloadFormat::UPAYLOAD_FORMAT_PROTOBUF_WRAPPED_IN_ANY); + google::protobuf::Any parsed_any; + EXPECT_TRUE(parsed_any.ParseFromString(serialized_data)); + EXPECT_EQ(parsed_any.type_url(), "hello_world/uprotocol.v1.UUri"); + + uprotocol::v1::UUri parsed_uri_object; + EXPECT_TRUE(parsed_uri_object.ParseFromString(parsed_any.value())); + EXPECT_EQ(parsed_uri_object.authority_name(), testStringPayload_); +} + /////////////////////RValue String Payload Tests///////////////////// // Create RValue String Payload diff --git a/test/coverage/datamodel/UUriValidatorTest.cpp b/test/coverage/datamodel/UUriValidatorTest.cpp index b8acca4e8..d41abc9b7 100644 --- a/test/coverage/datamodel/UUriValidatorTest.cpp +++ b/test/coverage/datamodel/UUriValidatorTest.cpp @@ -489,7 +489,7 @@ TEST_F(TestUUriValidator, ValidDefaultSource) { } TEST_F(TestUUriValidator, Empty) { - auto getUuri = []() { + auto get_uuri = []() { uprotocol::v1::UUri uuri; uuri.set_authority_name(""); uuri.set_ue_id(0); @@ -499,14 +499,14 @@ TEST_F(TestUUriValidator, Empty) { }; { - auto uuri = getUuri(); + auto uuri = get_uuri(); auto [valid, reason] = isEmpty(uuri); EXPECT_TRUE(valid); EXPECT_FALSE(reason.has_value()); } { - auto uuri = getUuri(); + auto uuri = get_uuri(); uuri.set_authority_name(" bad "); auto [valid, reason] = isEmpty(uuri); EXPECT_FALSE(valid); @@ -514,7 +514,7 @@ TEST_F(TestUUriValidator, Empty) { } { - auto uuri = getUuri(); + auto uuri = get_uuri(); uuri.set_authority_name(AUTHORITY_NAME); auto [valid, reason] = isEmpty(uuri); EXPECT_FALSE(valid); @@ -522,7 +522,7 @@ TEST_F(TestUUriValidator, Empty) { } { - auto uuri = getUuri(); + auto uuri = get_uuri(); uuri.set_ue_id(1); auto [valid, reason] = isEmpty(uuri); EXPECT_FALSE(valid); @@ -530,7 +530,7 @@ TEST_F(TestUUriValidator, Empty) { } { - auto uuri = getUuri(); + auto uuri = get_uuri(); uuri.set_ue_version_major(1); auto [valid, reason] = isEmpty(uuri); EXPECT_FALSE(valid); @@ -538,7 +538,7 @@ TEST_F(TestUUriValidator, Empty) { } { - auto uuri = getUuri(); + auto uuri = get_uuri(); uuri.set_resource_id(1); auto [valid, reason] = isEmpty(uuri); EXPECT_FALSE(valid); diff --git a/test/coverage/datamodel/UuidBuilderTest.cpp b/test/coverage/datamodel/UuidBuilderTest.cpp index f6f7d1cea..76162ff63 100644 --- a/test/coverage/datamodel/UuidBuilderTest.cpp +++ b/test/coverage/datamodel/UuidBuilderTest.cpp @@ -64,7 +64,7 @@ TEST(UuidBuilderTest, WithTimeSource) { // Test RandomSource TEST(UuidBuilderTest, WithRandomSource) { - auto fixed_random = 0x1234567890ABCDEF; + uint64_t fixed_random = 0x1234567890ABCDEF; auto builder = UuidBuilder::getTestBuilder().withRandomSource( [fixed_random]() { return fixed_random; }); auto uuid = builder.build(); diff --git a/test/coverage/utils/CallbackConnectionTest.cpp b/test/coverage/utils/CallbackConnectionTest.cpp index a0ae5fb09..581b23e2a 100644 --- a/test/coverage/utils/CallbackConnectionTest.cpp +++ b/test/coverage/utils/CallbackConnectionTest.cpp @@ -43,7 +43,8 @@ class CallbackTest : public testing::Test { TEST_F(CallbackTest, EstablishDoesNotThrow) { using namespace uprotocol::utils; - EXPECT_NO_THROW(callbacks::Connection::establish([]() {})); + EXPECT_NO_THROW(auto result = + callbacks::Connection::establish([]() {})); } /// It should be possible to establish a connection and call the callback @@ -698,4 +699,73 @@ TEST_F(CallbackTest, CalleeHandleCanDefaultConstruct) { }); } +/////////////////////////////////////////////////////////////////////////////// +// It is possible to create std::function objects with no target function. When +// they are invoked, they throw std::bad_function_call. This is not desireable, +// so the callback connections modules are required to check the validity of +// function objects they receive + +// Tests invalid callback function objects +TEST_F(CallbackTest, EstablishWithNonCallableCallback) { + using namespace uprotocol::utils; + + callbacks::Connection::ConnectedPair conn; + + EXPECT_THROW(conn = callbacks::Connection::establish({}), + callbacks::EmptyFunctionObject); + + auto& [handle, callable] = conn; + + // Ordering is important here. If handle.reset() tries blindly to call the + // cleanup callback, the exception could be thrown before the connection + // is broken. When that happens, the destructor will try to reset again. + // By resetting the callable second, there is no need to try the cleanup + // funciton again, so the destructor won't throw. + EXPECT_NO_THROW(handle.reset()); + EXPECT_NO_THROW(callable.reset()); +} + +// Tests invalid cleanup function objects +TEST_F(CallbackTest, EstablishWithNonCallableCleanup) { + using namespace uprotocol::utils; + + auto cb = []() -> bool { return true; }; + callbacks::Connection::Cleanup empty; + callbacks::Connection::ConnectedPair conn; + + EXPECT_THROW(conn = callbacks::Connection::establish(cb, empty), + callbacks::EmptyFunctionObject); + + auto& [handle, callable] = conn; + + // Ordering is important here. If handle.reset() tries blindly to call the + // cleanup callback, the exception could be thrown before the connection + // is broken. When that happens, the destructor will try to reset again. + // By resetting the callable second, there is no need to try the cleanup + // funciton again, so the destructor won't throw. + EXPECT_NO_THROW(handle.reset()); + EXPECT_NO_THROW(callable.reset()); +} + +// Tests both invalid cleanup and invalid callback function objects +TEST_F(CallbackTest, EstablishWithNonCallableCallbackAndCleanup) { + using namespace uprotocol::utils; + + callbacks::Connection::Cleanup empty; + callbacks::Connection::ConnectedPair conn; + + EXPECT_THROW(conn = callbacks::Connection::establish({}, empty), + callbacks::EmptyFunctionObject); + + auto& [handle, callable] = conn; + + // Ordering is important here. If handle.reset() tries blindly to call the + // cleanup callback, the exception could be thrown before the connection + // is broken. When that happens, the destructor will try to reset again. + // By resetting the callable second, there is no need to try the cleanup + // funciton again, so the destructor won't throw. + EXPECT_NO_THROW(handle.reset()); + EXPECT_NO_THROW(callable.reset()); +} + } // namespace diff --git a/test/coverage/utils/ExpectedTest.cpp b/test/coverage/utils/ExpectedTest.cpp index 76f712854..5b2b432e6 100644 --- a/test/coverage/utils/ExpectedTest.cpp +++ b/test/coverage/utils/ExpectedTest.cpp @@ -56,7 +56,7 @@ TEST_F(ExpectedTest, ExpectScalarScalar) { TEST_F(ExpectedTest, UnexpectScalarScalar) { int sample = get_rand(); - auto expected = Expected(Unexpected(sample)); + auto expected = Expected(Unexpected(sample)); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); EXPECT_EQ(sample, expected.error()); @@ -73,7 +73,7 @@ TEST_F(ExpectedTest, ExpectScalar) { TEST_F(ExpectedTest, UnexpectScalar) { int sample = get_rand(); - auto expected = Expected(Unexpected(sample)); + auto expected = Expected(Unexpected(sample)); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); EXPECT_EQ(sample, expected.error()); @@ -81,8 +81,8 @@ TEST_F(ExpectedTest, UnexpectScalar) { TEST_F(ExpectedTest, UnexpectValueOr) { int sample = get_rand(); - auto expected = - Expected(Unexpected(std::string("hello"))); + auto expected = Expected( + Unexpected(std::string("hello"))); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); EXPECT_EQ(sample, expected.value_or(sample)); @@ -111,7 +111,7 @@ TEST_F(ExpectedTest, UnexpectUnique) { auto x = get_rand(); auto y = get_rand(); auto expected = Expected>( - Unexpected(std::make_unique(x, y))); + Unexpected>(std::make_unique(x, y))); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); auto p = std::move(expected).error(); @@ -136,7 +136,7 @@ TEST_F(ExpectedTest, UnexpectShared) { auto x = get_rand(); auto y = get_rand(); auto expected = Expected>( - Unexpected(std::make_shared(x, y))); + Unexpected>(std::make_shared(x, y))); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); EXPECT_EQ(x, expected.error()->x); @@ -158,7 +158,7 @@ TEST_F(ExpectedTest, ExpectStruct) { TEST_F(ExpectedTest, UnexpectStruct) { auto x = get_rand(); auto y = get_rand(); - auto expected = Expected(Unexpected(Pair(x, y))); + auto expected = Expected(Unexpected(Pair(x, y))); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); EXPECT_EQ(x, expected.error().x); @@ -201,8 +201,8 @@ TEST_F(ExpectedTest, UnexpectStructDestruct) { { auto x = get_rand(); auto y = get_rand(); - auto expected = - Expected(Unexpected(PairDestruct(x, y))); + auto expected = Expected( + Unexpected(PairDestruct(x, y))); EXPECT_EQ(1, PairDestruct::cd_count); EXPECT_FALSE(bool(expected)); EXPECT_FALSE(expected.has_value()); @@ -213,8 +213,8 @@ TEST_F(ExpectedTest, UnexpectStructDestruct) { } TEST_F(ExpectedTest, ExceptionValueCheckedWhenIsError) { - auto expected = - Expected(Unexpected(std::string("hello"))); + auto expected = Expected( + Unexpected(std::string("hello"))); EXPECT_THROW( { try { @@ -248,8 +248,8 @@ TEST_F(ExpectedTest, ExceptionErrorCheckedWhenNotError) { } TEST_F(ExpectedTest, ExceptionDerefValueWhenUnexpected) { - auto expected = - Expected(Unexpected(std::string("hello"))); + auto expected = Expected( + Unexpected(std::string("hello"))); EXPECT_THROW( { try { @@ -267,8 +267,8 @@ TEST_F(ExpectedTest, ExceptionDerefValueWhenUnexpected) { } TEST_F(ExpectedTest, ExceptionDerefPtrWhenUnexpected) { - auto expected = - Expected(Unexpected(std::string("hello"))); + auto expected = Expected( + Unexpected(std::string("hello"))); EXPECT_THROW( { try { diff --git a/test/include/UTransportMock.h b/test/include/UTransportMock.h index 29129e6c3..5f93fc14d 100644 --- a/test/include/UTransportMock.h +++ b/test/include/UTransportMock.h @@ -9,8 +9,8 @@ // // SPDX-License-Identifier: Apache-2.0 -#ifndef UP_CPP_TEST_UTRANSPORTMOCK_H -#define UP_CPP_TEST_UTRANSPORTMOCK_H +#ifndef UTRANSPORTMOCK_H +#define UTRANSPORTMOCK_H #include #include @@ -33,6 +33,8 @@ class UTransportMock : public uprotocol::transport::UTransport { (*listener_)(msg); } + // TODO(max) set private again and fix access in RpcServerTest +public: std::atomic send_count_; uprotocol::v1::UStatus send_status_; @@ -51,9 +53,8 @@ class UTransportMock : public uprotocol::transport::UTransport { v1::UMessage message_; std::mutex message_mtx_; - virtual ~UTransportMock() = default; + ~UTransportMock() override = default; -private: [[nodiscard]] v1::UStatus sendImpl(const v1::UMessage& message) override { { std::lock_guard lock(message_mtx_); @@ -73,11 +74,11 @@ class UTransportMock : public uprotocol::transport::UTransport { return registerListener_status_; } - void cleanupListener(CallableConn listener) override { + void cleanupListener(const CallableConn& listener) override { cleanup_listener_ = listener; } }; }; // namespace uprotocol::test -#endif // UP_CPP_TEST_UTRANSPORTMOCK_H +#endif // UTRANSPORTMOCK_H