Skip to content

Add CEcalRegistrationDatabase core, direct mutation API, and initial GTests#3

Draft
KerstinKeller wants to merge 3 commits into
masterfrom
codex/analyze-and-plan-registration-database-implementation
Draft

Add CEcalRegistrationDatabase core, direct mutation API, and initial GTests#3
KerstinKeller wants to merge 3 commits into
masterfrom
codex/analyze-and-plan-registration-database-implementation

Conversation

@KerstinKeller

Copy link
Copy Markdown
Owner

Motivation

  • Avoid forwarding full Registration::Sample objects everywhere by providing a deserializer-friendly direct mutation API for registration updates to reduce deserialization cost and duplicated state reconstruction.
  • Establish a canonical, normalized store that separates hot monitoring fields from cold registration payloads and provides deterministic revision semantics for incremental migration.
  • Ensure safe lifecycle semantics by codifying cascade-delete behavior for process removal so member entities (publishers/subscribers/servers/clients) are removed atomically with their monitoring state.

Description

  • Implemented a first CEcalRegistrationDatabase in ecal/core/src/registration/ with split registration vs monitoring stores, current + previous revision tracking, snapshot read semantics, and process-member tracking (ecal_registration_database.h/.cpp).
  • Added direct mutation APIs (AddOrUpdate* / Update*Monitoring / Remove*) and sample compatibility adapters (ApplySample / ApplySamples) which map incoming Registration::Sample commands to the direct APIs.
  • Implemented process cascade-delete that removes all member entities and their monitoring entries when a process is removed.
  • Registered the new core sources in ecal/core/CMakeLists.txt and added initial GTests in ecal/tests/cpp/registration_test/src/registration_database_test.cpp, and wired the test into ecal/tests/cpp/registration_test/CMakeLists.txt.

Testing

  • Ran a file-level C++ syntax check of the new core source with g++ -std=c++14 -fsyntax-only which succeeded for ecal_registration_database.cpp.
  • Attempted full project configuration with cmake --preset core but configuration failed due to missing third-party submodule trees in the environment (so project-level build and test execution were not possible).
  • Attempted to compile the new test source but compilation failed in this environment because gtest/gtest.h is not available, so the added GTests were not executed here.
  • The new tests assert ApplySample vs direct-mutation equivalence for publisher ingestion, process cascade deletion behavior, and current/previous revision tracking and are included for CI to run when third-party/test dependencies are available.

Codex Task

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 25. Check the log or trigger a new build to see more.

#include <utility>

namespace eCAL
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]

Suggested change
{

ecal/core/src/registration/ecal_registration_database.cpp:25:

-   namespace Registration
+   namespace eCAL::Registration

ecal/core/src/registration/ecal_registration_database.cpp:450:

- }

return ApplyMutation([key_, &delta_](State& state_, std::vector<EntityEvent>&, bool& changed_)
{
auto it = state_.process_monitoring.find(key_);
if (it == state_.process_monitoring.end() || !(it->second.state == delta_.state && it->second.time_sync_state == delta_.time_sync_state))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

Suggested change
if (it == state_.process_monitoring.end() || !(it->second.state == delta_.state && it->second.time_sync_state == delta_.time_sync_state))
if (it == state_.process_monitoring.end() || !it->second.state == delta_.state || it->second.time_sync_state != delta_.time_sync_state)

auto it = state_.REG_MAP.find(key_); if (it != state_.REG_MAP.end()) { RemoveMembership(state_.members_by_process, it->second.process_id, EntityType::TYPE_ENUM, key_); state_.REG_MAP.erase(it); state_.MON_MAP.erase(key_); events_.push_back({ EventType::deleted_entity, EntityType::TYPE_ENUM, key_ }); changed_ = true; } \
}); }

ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Publisher, publisher, publishers, publisher_monitoring)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

    ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Publisher, publisher, publishers, publisher_monitoring)
    ^
Additional context

ecal/core/src/registration/ecal_registration_database.cpp:237: expanded from macro 'ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC'

      else if (!(it->second.topic == delta_.topic && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name)) { \
               ^

}); }

ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Publisher, publisher, publishers, publisher_monitoring)
ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Subscriber, subscriber, subscribers, subscriber_monitoring)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

    ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Subscriber, subscriber, subscribers, subscriber_monitoring)
    ^
Additional context

ecal/core/src/registration/ecal_registration_database.cpp:237: expanded from macro 'ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC'

      else if (!(it->second.topic == delta_.topic && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name)) { \
               ^

events_.push_back({ EventType::new_entity, EntityType::server, key_ });
changed_ = true;
}
else if (!(it->second.service == delta_.service && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

Suggested change
else if (!(it->second.service == delta_.service && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name))
else if (!it->second.service == delta_.service || it->second.process_id != delta_.process_id || !it->second.host_name == delta_.host_name)

bool HasSubscriber(EntityKey key) const;
bool HasServer(EntityKey key) const;
bool HasClient(EntityKey key) const;
size_t ProcessCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'ProcessCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t ProcessCount() const;
[[nodiscard]] size_t ProcessCount() const;

bool HasServer(EntityKey key) const;
bool HasClient(EntityKey key) const;
size_t ProcessCount() const;
size_t PublisherCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'PublisherCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t PublisherCount() const;
[[nodiscard]] size_t PublisherCount() const;

bool HasClient(EntityKey key) const;
size_t ProcessCount() const;
size_t PublisherCount() const;
size_t SubscriberCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'SubscriberCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t SubscriberCount() const;
[[nodiscard]] size_t SubscriberCount() const;

size_t ProcessCount() const;
size_t PublisherCount() const;
size_t SubscriberCount() const;
size_t ServerCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'ServerCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t ServerCount() const;
[[nodiscard]] size_t ServerCount() const;

size_t PublisherCount() const;
size_t SubscriberCount() const;
size_t ServerCount() const;
size_t ClientCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'ClientCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t ClientCount() const;
[[nodiscard]] size_t ClientCount() const;

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 25. Check the log or trigger a new build to see more.

#include <utility>

namespace eCAL
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]

Suggested change
{

ecal/core/src/registration/ecal_registration_database.cpp:25:

-   namespace Registration
+   namespace eCAL::Registration

ecal/core/src/registration/ecal_registration_database.cpp:450:

- }

return ApplyMutation([key_, &delta_](State& state_, std::vector<EntityEvent>&, bool& changed_)
{
auto it = state_.process_monitoring.find(key_);
if (it == state_.process_monitoring.end() || !(it->second.state == delta_.state && it->second.time_sync_state == delta_.time_sync_state))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

Suggested change
if (it == state_.process_monitoring.end() || !(it->second.state == delta_.state && it->second.time_sync_state == delta_.time_sync_state))
if (it == state_.process_monitoring.end() || !it->second.state == delta_.state || it->second.time_sync_state != delta_.time_sync_state)

auto it = state_.REG_MAP.find(key_); if (it != state_.REG_MAP.end()) { RemoveMembership(state_.members_by_process, it->second.process_id, EntityType::TYPE_ENUM, key_); state_.REG_MAP.erase(it); state_.MON_MAP.erase(key_); events_.push_back({ EventType::deleted_entity, EntityType::TYPE_ENUM, key_ }); changed_ = true; } \
}); }

ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Publisher, publisher, publishers, publisher_monitoring)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

    ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Publisher, publisher, publishers, publisher_monitoring)
    ^
Additional context

ecal/core/src/registration/ecal_registration_database.cpp:237: expanded from macro 'ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC'

      else if (!(it->second.topic == delta_.topic && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name)) { \
               ^

}); }

ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Publisher, publisher, publishers, publisher_monitoring)
ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Subscriber, subscriber, subscribers, subscriber_monitoring)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

    ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC(Subscriber, subscriber, subscribers, subscriber_monitoring)
    ^
Additional context

ecal/core/src/registration/ecal_registration_database.cpp:237: expanded from macro 'ECAL_REGDB_ADD_UPDATE_REMOVE_TOPIC'

      else if (!(it->second.topic == delta_.topic && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name)) { \
               ^

events_.push_back({ EventType::new_entity, EntityType::server, key_ });
changed_ = true;
}
else if (!(it->second.service == delta_.service && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

Suggested change
else if (!(it->second.service == delta_.service && it->second.process_id == delta_.process_id && it->second.host_name == delta_.host_name))
else if (!it->second.service == delta_.service || it->second.process_id != delta_.process_id || !it->second.host_name == delta_.host_name)

bool HasSubscriber(EntityKey key) const;
bool HasServer(EntityKey key) const;
bool HasClient(EntityKey key) const;
size_t ProcessCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'ProcessCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t ProcessCount() const;
[[nodiscard]] size_t ProcessCount() const;

bool HasServer(EntityKey key) const;
bool HasClient(EntityKey key) const;
size_t ProcessCount() const;
size_t PublisherCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'PublisherCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t PublisherCount() const;
[[nodiscard]] size_t PublisherCount() const;

bool HasClient(EntityKey key) const;
size_t ProcessCount() const;
size_t PublisherCount() const;
size_t SubscriberCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'SubscriberCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t SubscriberCount() const;
[[nodiscard]] size_t SubscriberCount() const;

size_t ProcessCount() const;
size_t PublisherCount() const;
size_t SubscriberCount() const;
size_t ServerCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'ServerCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t ServerCount() const;
[[nodiscard]] size_t ServerCount() const;

size_t PublisherCount() const;
size_t SubscriberCount() const;
size_t ServerCount() const;
size_t ClientCount() const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'ClientCount' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t ClientCount() const;
[[nodiscard]] size_t ClientCount() const;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant