-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add meta proto service for service discovery #543
Draft
oguzcanoguz
wants to merge
47
commits into
main
Choose a base branch
from
feature/proto_service_discovery
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 12 commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
4c3eb9f
Add meta proto service for service discovery
oguzcanoguz cdc8fb3
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz adb0092
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz f15b249
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz d5d9dc2
Update protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
oguzcanoguz cf77db0
Update protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
oguzcanoguz 9622fe2
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz b6cc192
Update protobuf/meta_services/ServiceDiscoveryEcho.cpp
oguzcanoguz bbc7075
Update protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
oguzcanoguz b9239e9
Apply suggestions from code review
oguzcanoguz 2850b82
Add service changed notification to ServviceDiscovery
oguzcanoguz 151e119
Merge remote-tracking branch 'origin/main' into feature/proto_service…
oguzcanoguz e8ca6d6
ServiceDiscoveryEcho refactor header
oguzcanoguz 2dadfb7
Implement StartMethod and NotifyServiceChanges in ServiceDiscoveryEcho
oguzcanoguz 2e575b3
Refactor ServiceDiscoveryTest
oguzcanoguz 0db7aa5
Fix function static storage in ServiceDiscoveryEcho
oguzcanoguz ca8bdaa
Solve functio storage problem in ServiceDiscoveryTest
oguzcanoguz 914754e
Make EchoConsole accept recursive directories for protobuf files
oguzcanoguz a2dd642
Start with PeerServiceDiscoverer
oguzcanoguz f0fc274
PeerServiceDiscovererTest: Add iterative service discovery
oguzcanoguz a8a24b8
PeerServiceDiscovererTest contd
oguzcanoguz df27790
Merge commit 'e9afc63890f512f66b28fe2b409de5b9fe471934'
oguzcanoguz 0068cc6
Start with providing id range in NotifyServicesChanged
oguzcanoguz 3685020
Add notifying service changes with a range
oguzcanoguz 5db6745
Make service changes amnesiac
oguzcanoguz d5519bd
remove ServiceDiscoveryStarted from PeerServiceDiscovery
oguzcanoguz 067ec70
Refactor PeerServiceDiscovererObserver
oguzcanoguz 9118a61
PeerServiceDiscoverer: Start service discovery on construction
oguzcanoguz abfa695
PeerServiceDiscoverer: Handle service updated notification
oguzcanoguz e2c95a7
PeerServiceDiscovererTest: WIP Streamlining services changed tests
oguzcanoguz 5a0a2be
Refactor PeerServiceDiscovery tests
oguzcanoguz a89a492
PeerServiceDiscovery: Discovery can end with a service or no service
oguzcanoguz 6543e14
Start with a test echo server that can be run in devcontainer
oguzcanoguz a30b880
fix cmakelists
oguzcanoguz a471b1d
Try to instantiate PeerServiceDiscovery in echo console
oguzcanoguz 62a871f
Fix: Use echo only when the connection observer is attached to a conn…
oguzcanoguz 50d9170
ServiceDiscoveryEcho: Optimize FirstServiceSupported
oguzcanoguz 138fc45
Merge branch 'temp' into feature/proto_service_discovery
oguzcanoguz 161fcb6
trace discovered services
oguzcanoguz f65605a
Start working on attaching EchoConsole to Echo as a Service
oguzcanoguz 6ad3bb7
TestConsoleService preparation
oguzcanoguz ba3f652
Merge commit '6c84af5324c79219059d1066cc5ce3aad8f2b927' into feature/…
oguzcanoguz d3a528f
Fix build error
oguzcanoguz 52afcbd
Remove incorrect include (breaks embedded build)
oguzcanoguz 922f501
Make echo_console only list available services
oguzcanoguz 5410260
Fix lots of stuff to make EchoConsole to discover services of RefProd
oguzcanoguz 8052a28
Remove temporary tracing
oguzcanoguz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
add_library(protobuf.meta_services ${EMIL_EXCLUDE_FROM_ALL} STATIC) | ||
|
||
protocol_buffer_csharp(protobuf.meta_services ServiceDiscovery.proto) | ||
protocol_buffer_java(protobuf.meta_services ServiceDiscovery.proto) | ||
protocol_buffer_echo_all(protobuf.meta_services ServiceDiscovery.proto) | ||
|
||
target_sources(protobuf.meta_services PRIVATE | ||
ServiceDiscoveryEcho.cpp | ||
ServiceDiscoveryEcho.hpp | ||
) | ||
|
||
add_subdirectory(test) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
syntax = "proto3"; | ||
|
||
import "EchoAttributes.proto"; | ||
|
||
package service_discovery; | ||
option java_package = "com.philips.emil.protobufEcho"; | ||
option java_outer_classname = "ServiceDiscoveryProto"; | ||
|
||
message Uint32Value { | ||
uint32 value = 1; | ||
} | ||
|
||
message BoolValue | ||
{ | ||
bool value = 1; | ||
} | ||
|
||
message ServiceRange | ||
{ | ||
uint32 startServiceId = 1; | ||
uint32 endServiceId = 2; | ||
} | ||
|
||
service ServiceDiscovery | ||
{ | ||
option (service_id) = 1000; | ||
|
||
rpc FindFirstServiceInRange(ServiceRange) returns (Nothing) { option (method_id) = 1; } | ||
rpc NotifyServiceChanges(BoolValue) returns (Nothing) { option (method_id) = 2; } | ||
} | ||
|
||
service ServiceDiscoveryResponse | ||
{ | ||
option (service_id) = 1001; | ||
|
||
rpc FirstServiceSupported(Uint32Value) returns (Nothing) { option (method_id) = 1; } | ||
rpc NoServiceSupported(Nothing) returns (Nothing) { option (method_id) = 2; } | ||
rpc ServicesChanged(Nothing) returns (Nothing) { option (method_id) = 3; } | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
#include "protobuf/meta_services/ServiceDiscoveryEcho.hpp" | ||
#include "echo/ServiceDiscovery.pb.hpp" | ||
|
||
namespace application | ||
{ | ||
void ServiceDiscoveryEcho::FindFirstServiceInRange(uint32_t startServiceId, uint32_t endServiceId) | ||
{ | ||
infra::Optional<uint32_t> service; | ||
|
||
for (auto id = startServiceId; id <= endServiceId; ++id) | ||
{ | ||
if (AcceptsService(id)) | ||
{ | ||
service = id; | ||
break; | ||
} | ||
} | ||
|
||
service_discovery::ServiceDiscoveryResponseProxy::RequestSend([this, service] | ||
{ | ||
if (service) | ||
FirstServiceSupported(*service); | ||
else | ||
NoServiceSupported(); | ||
}); | ||
|
||
MethodDone(); | ||
} | ||
|
||
bool ServiceDiscoveryEcho::AcceptsService(uint32_t serviceId) const | ||
{ | ||
return service_discovery::ServiceDiscovery::AcceptsService(serviceId) | ||
|| IsProxyServiceSupported(serviceId); | ||
} | ||
|
||
bool ServiceDiscoveryEcho::IsProxyServiceSupported(uint32_t& serviceId) const | ||
{ | ||
auto serviceFound = false; | ||
services::Echo::NotifyObservers([serviceId, &serviceFound](auto& observer) | ||
{ | ||
if (observer.AcceptsService(serviceId)) | ||
serviceFound = true; | ||
}); | ||
|
||
return serviceFound; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,50 @@ | ||||||
#ifndef PROTOBUF_ECHO_SERVICE_DISCOVERY_HPP | ||||||
#define PROTOBUF_ECHO_SERVICE_DISCOVERY_HPP | ||||||
|
||||||
#include "generated/echo/ServiceDiscovery.pb.hpp" | ||||||
#include "protobuf/echo/Echo.hpp" | ||||||
#include <cstdint> | ||||||
|
||||||
namespace application | ||||||
{ | ||||||
class ServiceDiscoveryEcho | ||||||
: public service_discovery::ServiceDiscovery | ||||||
, public service_discovery::ServiceDiscoveryResponseProxy | ||||||
, public services::Echo | ||||||
{ | ||||||
public: | ||||||
explicit ServiceDiscoveryEcho(services::Echo& echo) | ||||||
: service_discovery::ServiceDiscovery(echo) | ||||||
, service_discovery::ServiceDiscoveryResponseProxy(echo) | ||||||
{} | ||||||
|
||||||
virtual ~ServiceDiscoveryEcho() = default; | ||||||
|
||||||
void FindFirstServiceInRange(uint32_t startServiceId, uint32_t endServiceId) override; | ||||||
void NotifyServiceChanges(bool value) override | ||||||
{} | ||||||
|
||||||
bool AcceptsService(uint32_t id) const override; | ||||||
//infra::SharedPtr<services::MethodDeserializer> StartMethod(uint32_t serviceId, uint32_t methodId, uint32_t size, const services::EchoErrorPolicy& errorPolicy) override; | ||||||
|
||||||
void RequestSend(ServiceProxy& serviceProxy) override | ||||||
{ | ||||||
service_discovery::ServiceDiscovery::Rpc().RequestSend(serviceProxy); | ||||||
} | ||||||
|
||||||
void ServiceDone() override | ||||||
{ | ||||||
service_discovery::ServiceDiscovery::Rpc().ServiceDone(); | ||||||
} | ||||||
|
||||||
services::MethodSerializerFactory& SerializerFactory() override | ||||||
{ | ||||||
return service_discovery::ServiceDiscovery::Rpc().SerializerFactory(); | ||||||
} | ||||||
|
||||||
protected: | ||||||
bool IsProxyServiceSupported(uint32_t& serviceId) const; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MegaLinter] reported by reviewdog 🐶
Suggested change
|
||||||
}; | ||||||
} | ||||||
|
||||||
#endif |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
add_executable(protobuf.meta_services_test) | ||
emil_build_for(protobuf.meta_services_test BOOL EMIL_BUILD_TESTS) | ||
emil_add_test(protobuf.meta_services_test) | ||
|
||
target_sources(protobuf.meta_services_test PRIVATE | ||
TestServiceDiscoveryEcho.cpp | ||
) | ||
|
||
target_link_libraries(protobuf.meta_services_test PUBLIC | ||
gmock_main | ||
protobuf.test_doubles | ||
protobuf.meta_services | ||
) |
134 changes: 134 additions & 0 deletions
134
protobuf/meta_services/test/TestServiceDiscoveryEcho.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
#include "echo/ServiceDiscovery.pb.hpp" | ||
#include "infra/util/Optional.hpp" | ||
#include "protobuf/echo/Echo.hpp" | ||
#include "protobuf/echo/test_doubles/EchoSingleLoopback.hpp" | ||
#include "protobuf/echo/test_doubles/ServiceStub.hpp" | ||
#include "protobuf/meta_services/ServiceDiscoveryEcho.hpp" | ||
#include "gmock/gmock.h" | ||
#include "gtest/gtest.h" | ||
#include <cstdint> | ||
#include <sys/types.h> | ||
|
||
oguzcanoguz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
namespace | ||
{ | ||
class ServiceDiscoveryResponseMock | ||
: public service_discovery::ServiceDiscoveryResponse | ||
{ | ||
public: | ||
using service_discovery::ServiceDiscoveryResponse::ServiceDiscoveryResponse; | ||
virtual ~ServiceDiscoveryResponseMock() = default; | ||
|
||
void FirstServiceSupported(uint32_t value) override | ||
{ | ||
FirstServiceSupportedMock(value); | ||
MethodDone(); | ||
} | ||
|
||
void NoServiceSupported() override | ||
{ | ||
NoServiceSupportedMock(); | ||
MethodDone(); | ||
} | ||
|
||
void ServicesChanged() override | ||
{ | ||
ServicesChangedMock(); | ||
MethodDone(); | ||
} | ||
|
||
MOCK_METHOD(void, FirstServiceSupportedMock, (uint32_t)); | ||
MOCK_METHOD(void, NoServiceSupportedMock, ()); | ||
MOCK_METHOD(void, ServicesChangedMock, ()); | ||
}; | ||
|
||
class ServiceStubWithCustomServiceId | ||
: public services::ServiceStub | ||
{ | ||
public: | ||
ServiceStubWithCustomServiceId(services::Echo& echo, uint32_t serviceId) | ||
: services::ServiceStub(echo) | ||
, serviceId(serviceId) | ||
{} | ||
|
||
virtual ~ServiceStubWithCustomServiceId() = default; | ||
|
||
bool AcceptsService(uint32_t serviceId) const override | ||
{ | ||
return this->serviceId == serviceId; | ||
} | ||
|
||
private: | ||
const uint32_t serviceId; | ||
}; | ||
}; | ||
|
||
class ServiceDiscoveryTest | ||
: public testing::Test | ||
{ | ||
public: | ||
services::MethodSerializerFactory::ForServices<service_discovery::ServiceDiscovery, service_discovery::ServiceDiscoveryResponse>::AndProxies<service_discovery::ServiceDiscoveryProxy> serializerFactory; | ||
application::EchoSingleLoopback echo{ serializerFactory }; | ||
service_discovery::ServiceDiscoveryProxy proxy{ echo }; | ||
testing::StrictMock<ServiceDiscoveryResponseMock> serviceDiscoveryResponse{ echo }; | ||
|
||
application::ServiceDiscoveryEcho serviceDiscoveryEcho{ echo }; | ||
}; | ||
|
||
TEST_F(ServiceDiscoveryTest, return_no_service) | ||
{ | ||
ServiceStubWithCustomServiceId serviceMock{ serviceDiscoveryEcho, 5 }; | ||
|
||
EXPECT_CALL(serviceDiscoveryResponse, NoServiceSupportedMock); | ||
|
||
proxy.RequestSend([this] | ||
{ | ||
proxy.FindFirstServiceInRange(0, 4); | ||
}); | ||
} | ||
|
||
TEST_F(ServiceDiscoveryTest, return_service) | ||
{ | ||
ServiceStubWithCustomServiceId serviceMock{ serviceDiscoveryEcho, 5 }; | ||
|
||
EXPECT_CALL(serviceDiscoveryResponse, FirstServiceSupportedMock(5)); | ||
|
||
proxy.RequestSend([this] | ||
{ | ||
proxy.FindFirstServiceInRange(0, 15); | ||
}); | ||
} | ||
|
||
TEST_F(ServiceDiscoveryTest, return_service_with_lowest_id) | ||
{ | ||
ServiceStubWithCustomServiceId serviceMock{ serviceDiscoveryEcho, 5 }; | ||
ServiceStubWithCustomServiceId serviceMock2{ serviceDiscoveryEcho, 6 }; | ||
|
||
EXPECT_CALL(serviceDiscoveryResponse, FirstServiceSupportedMock(5)); | ||
|
||
proxy.RequestSend([this] | ||
{ | ||
proxy.FindFirstServiceInRange(0, 15); | ||
}); | ||
} | ||
|
||
oguzcanoguz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
TEST_F(ServiceDiscoveryTest, DISABLED_notify_service_change) | ||
{ | ||
proxy.RequestSend([this] | ||
{ | ||
proxy.NotifyServiceChanges(true); | ||
}); | ||
|
||
EXPECT_CALL(serviceDiscoveryResponse, ServicesChangedMock()); | ||
|
||
ServiceStubWithCustomServiceId serviceMock{ serviceDiscoveryEcho, 5 }; | ||
} | ||
|
||
TEST_F(ServiceDiscoveryTest, find_own_service_id) | ||
{ | ||
EXPECT_CALL(serviceDiscoveryResponse, FirstServiceSupportedMock(service_discovery::ServiceDiscovery::serviceId)); | ||
|
||
proxy.RequestSend([this] | ||
{ | ||
proxy.FindFirstServiceInRange(service_discovery::ServiceDiscovery::serviceId, service_discovery::ServiceDiscovery::serviceId); | ||
}); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MegaLinter] reported by reviewdog 🐶