Skip to content
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

refactor: rename IPush and IPull sources to better reflect their purposes #295

Merged
merged 3 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#pragma once

#include <launchdarkly/server_side/config/built/data_system/lazy_load_config.hpp>
#include <launchdarkly/server_side/data_interfaces/sources/iserialized_pull_source.hpp>
#include <launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp>

#include <launchdarkly/error.hpp>

Expand All @@ -25,8 +25,7 @@ namespace launchdarkly::server_side::config::builders {
* another SDK) is necessary.
*/
struct LazyLoadBuilder {
using SourcePtr =
std::shared_ptr<data_interfaces::ISerializedDataPullSource>;
using SourcePtr = std::shared_ptr<data_interfaces::ISerializedDataReader>;
using EvictionPolicy = built::LazyLoadConfig::EvictionPolicy;
/**
* \brief Constructs a new LazyLoadBuilder.
Expand All @@ -35,7 +34,7 @@ struct LazyLoadBuilder {

/**
* \brief Specify the source of the data.
* \param source Component implementing ISerializedDataPullSource.
* \param source Component implementing ISerializedDataReader.
* \return Reference to this.
*/
LazyLoadBuilder& Source(SourcePtr source);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#pragma once

#include <launchdarkly/server_side/data_interfaces/sources/iserialized_pull_source.hpp>
#include <launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp>

#include <chrono>
#include <memory>

namespace launchdarkly::server_side::config::built {

struct LazyLoadConfig {

Check warning on line 10 in libs/server-sdk/include/launchdarkly/server_side/config/built/data_system/lazy_load_config.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/include/launchdarkly/server_side/config/built/data_system/lazy_load_config.hpp:10:8 [cppcoreguidelines-pro-type-member-init]

constructor does not initialize these fields: eviction_policy
/**
* \brief Specifies the action taken when a data item's TTL expires.
*/
Expand All @@ -19,7 +19,6 @@

EvictionPolicy eviction_policy;
std::chrono::milliseconds eviction_ttl;
std::shared_ptr<server_side::data_interfaces::ISerializedDataPullSource>
source;
std::shared_ptr<data_interfaces::ISerializedDataReader> source;
};
} // namespace launchdarkly::server_side::config::built
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

Check notice on line 1 in libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

Run clang-format on libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

File libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp does not conform to Custom style guidelines. (lines 34)

#include <launchdarkly/server_side/integrations/serialized_descriptors.hpp>

Check failure on line 3 in libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp:3:10 [clang-diagnostic-error]

'launchdarkly/server_side/integrations/serialized_descriptors.hpp' file not found

#include <tl/expected.hpp>

Expand All @@ -10,7 +10,7 @@
namespace launchdarkly::server_side::data_interfaces {

/**
* Interface for a data source that provides feature flags and related data in a
* Interface for a data reader that provides feature flags and related data in a
* serialized form.
*
* This interface should be used for database integrations, or any other data
Expand All @@ -26,14 +26,14 @@
*
* Implementations must be thread-safe.
*/
class ISerializedDataPullSource {
class ISerializedDataReader {
public:
virtual ~ISerializedDataPullSource() = default;
ISerializedDataPullSource(ISerializedDataPullSource const& item) = delete;
ISerializedDataPullSource(ISerializedDataPullSource&& item) = delete;
ISerializedDataPullSource& operator=(ISerializedDataPullSource const&) =
virtual ~ISerializedDataReader() = default;
ISerializedDataReader(ISerializedDataReader const& item) = delete;
ISerializedDataReader(ISerializedDataReader&& item) = delete;
ISerializedDataReader& operator=(ISerializedDataReader const&) =
delete;
ISerializedDataPullSource& operator=(ISerializedDataPullSource&&) = delete;
ISerializedDataReader& operator=(ISerializedDataReader&&) = delete;

struct Error {
std::string message;
Expand All @@ -55,7 +55,7 @@
* if the item did not exist, or an error. For a deleted item the serialized
* item descriptor may contain a std::nullopt for the serializedItem.
*/
virtual GetResult Get(integrations::IPersistentKind const& kind,

Check warning on line 58 in libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp:58:5 [modernize-use-nodiscard]

function 'Get' should be marked [[nodiscard]]
std::string const& itemKey) const = 0;

/**
Expand All @@ -67,13 +67,13 @@
* @return Either all of the items of the type, or an error. If there are
* no items of the specified type, then return an empty collection.
*/
virtual AllResult All(integrations::IPersistentKind const& kind) const = 0;

Check warning on line 70 in libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp:70:5 [modernize-use-nodiscard]

function 'All' should be marked [[nodiscard]]

virtual std::string const& Identity() const = 0;

Check warning on line 72 in libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp:72:5 [modernize-use-nodiscard]

function 'Identity' should be marked [[nodiscard]]

virtual bool Initialized() const = 0;

Check warning on line 74 in libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/include/launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp:74:5 [modernize-use-nodiscard]

function 'Initialized' should be marked [[nodiscard]]

protected:
ISerializedDataPullSource() = default;
ISerializedDataReader() = default;
};
} // namespace launchdarkly::server_side::data_interfaces
2 changes: 2 additions & 0 deletions libs/server-sdk/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ target_sources(${LIBNAME}
data_components/expiration_tracker/expiration_tracker.cpp
data_components/memory_store/memory_store.hpp
data_components/memory_store/memory_store.cpp
data_components/serialization_adapters/json_data_reader.hpp
data_components/serialization_adapters/json_data_reader.cpp
data_systems/background_sync/sources/noop/null_data_source.hpp
data_systems/background_sync/sources/noop/null_data_source.cpp
data_systems/background_sync/sources/polling/polling_data_source.hpp
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#include "json_data_reader.hpp"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a file renaming, not new content (except for updating the ISerializedDataReader symbol.)


#include <launchdarkly/serialization/json_flag.hpp>
#include <launchdarkly/serialization/json_segment.hpp>

#include <launchdarkly/server_side/integrations/serialized_item_descriptor.hpp>

#include <boost/json.hpp>

namespace launchdarkly::server_side::data_components {

JsonDataReader::JsonDataReader(data_interfaces::ISerializedDataReader& reader)
: flag_kind_(), segment_kind_(), reader_(reader) {}

data_interfaces::IDataReader::Single<data_model::FlagDescriptor>
JsonDataReader::GetFlag(std::string const& key) const {
return Deserialize<data_model::Flag>(flag_kind_, key);
}

data_interfaces::IDataReader::Single<data_model::SegmentDescriptor>
JsonDataReader::GetSegment(std::string const& key) const {
return Deserialize<data_model::Segment>(segment_kind_, key);
}

data_interfaces::IDataReader::Collection<data_model::FlagDescriptor>
JsonDataReader::AllFlags() const {
// TODO: deserialize then return
}

data_interfaces::IDataReader::Collection<data_model::SegmentDescriptor>
JsonDataReader::AllSegments() const {
// TODO: deserialize then return
}

std::string const& JsonDataReader::Identity() const {
return reader_.Identity();
}

} // namespace launchdarkly::server_side::data_components
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
#pragma once
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since IPullSource is now an IDataReader, it makes sense that the JsonSource is now a JsonDataReader.


#include "../../data_interfaces/source/ipull_source.hpp"
#include "../../data_interfaces/source/idata_reader.hpp"
#include "../kinds/kinds.hpp"

#include <launchdarkly/server_side/data_interfaces/sources/iserialized_pull_source.hpp>
#include <launchdarkly/server_side/data_interfaces/sources/iserialized_data_reader.hpp>

namespace launchdarkly::server_side::data_components {

class JsonSource final : public data_interfaces::IPullSource {
class JsonDataReader final : public data_interfaces::IDataReader {
public:
explicit JsonSource(
data_interfaces::ISerializedDataPullSource& json_source);
explicit JsonDataReader(data_interfaces::ISerializedDataReader& reader);

[[nodiscard]] Single<data_model::FlagDescriptor> GetFlag(
std::string const& key) const override;
Expand All @@ -31,7 +30,7 @@ class JsonSource final : public data_interfaces::IPullSource {
Single<data_model::ItemDescriptor<DataModel>> Deserialize(
DataKind const& kind,
std::string const& key) const {
auto result = source_.Get(kind, key);
auto result = reader_.Get(kind, key);

if (!result) {
/* the actual fetch failed */
Expand Down Expand Up @@ -66,7 +65,7 @@ class JsonSource final : public data_interfaces::IPullSource {

FlagKind const flag_kind_;
FlagKind const segment_kind_;
data_interfaces::ISerializedDataPullSource& source_;
data_interfaces::ISerializedDataReader& reader_;
};

} // namespace launchdarkly::server_side::data_components

This file was deleted.

4 changes: 2 additions & 2 deletions libs/server-sdk/src/data_interfaces.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "data_interfaces/destination/idestination.hpp"
#include "data_interfaces/destination/iserialized_destination.hpp"
#include "data_interfaces/source/ipull_source.hpp"
#include "data_interfaces/source/ipush_source.hpp"
#include "data_interfaces/source/idata_reader.hpp"
#include "data_interfaces/source/idata_synchronizer.hpp"
#include "data_interfaces/store/istore.hpp"
#include "data_interfaces/system/idata_system.hpp"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
namespace launchdarkly::server_side::data_interfaces {

/**
* \brief IPullSource obtains data on-demand. Calls to obtain data may fail, so
* \brief IDataReader obtains data on-demand. Calls to obtain data may fail, so
* the getter methods use tl::expected in order to propagate error codes.
*
* The IPullSource does not perform caching, so parent components must be
* The IDataReader does not perform caching, so parent components must be
* careful to avoid repeatedly fetching data (i.e. use a cache.)
*
*/
class IPullSource {
class IDataReader {
public:
using Error = std::string;

Expand Down Expand Up @@ -67,14 +67,14 @@ class IPullSource {
*/
[[nodiscard]] virtual std::string const& Identity() const = 0;

virtual ~IPullSource() = default;
IPullSource(IPullSource const& item) = delete;
IPullSource(IPullSource&& item) = delete;
IPullSource& operator=(IPullSource const&) = delete;
IPullSource& operator=(IPullSource&&) = delete;
virtual ~IDataReader() = default;
IDataReader(IDataReader const& item) = delete;
IDataReader(IDataReader&& item) = delete;
IDataReader& operator=(IDataReader const&) = delete;
IDataReader& operator=(IDataReader&&) = delete;

protected:
IPullSource() = default;
IDataReader() = default;
};

} // namespace launchdarkly::server_side::data_interfaces
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
namespace launchdarkly::server_side::data_interfaces {

/**
* \brief IPushSource obtains data via a push synchronization mechanism,
* updating a local cache whenever changes are made upstream.
* \brief IDataSynchronizer obtains data via a background synchronization
* mechanism, updating a local cache whenever changes are made upstream.
*/
class IPushSource {
class IDataSynchronizer {
public:
/**
* \brief Initialize the source, optionally with an initial data set. Init
Expand Down Expand Up @@ -41,14 +41,14 @@ class IPushSource {
*/
[[nodiscard]] virtual std::string const& Identity() const = 0;

virtual ~IPushSource() = default;
IPushSource(IPushSource const& item) = delete;
IPushSource(IPushSource&& item) = delete;
IPushSource& operator=(IPushSource const&) = delete;
IPushSource& operator=(IPushSource&&) = delete;
virtual ~IDataSynchronizer() = default;
IDataSynchronizer(IDataSynchronizer const& item) = delete;
IDataSynchronizer(IDataSynchronizer&& item) = delete;
IDataSynchronizer& operator=(IDataSynchronizer const&) = delete;
IDataSynchronizer& operator=(IDataSynchronizer&&) = delete;

protected:
IPushSource() = default;
IDataSynchronizer() = default;
};

} // namespace launchdarkly::server_side::data_interfaces
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "../../data_components/change_notifier/change_notifier.hpp"
#include "../../data_components/memory_store/memory_store.hpp"
#include "../../data_components/status_notifications/data_source_status_manager.hpp"
#include "../../data_interfaces/source/ipush_source.hpp"
#include "../../data_interfaces/source/idata_synchronizer.hpp"
#include "../../data_interfaces/system/idata_system.hpp"

#include <launchdarkly/data_model/descriptors.hpp>
Expand All @@ -24,7 +24,7 @@
* too large to fit in memory, or a direct connection to LaunchDarkly isn't
* desired, necessitating use of the alternate LazyLoad system.
*/
class BackgroundSync final : public data_interfaces::IDataSystem {

Check warning on line 27 in libs/server-sdk/src/data_systems/background_sync/background_sync_system.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/src/data_systems/background_sync/background_sync_system.hpp:27:7 [cppcoreguidelines-special-member-functions]

class 'BackgroundSync' defines a copy constructor, a copy assignment operator, a move constructor and a move assignment operator but does not define a destructor
public:
BackgroundSync(
config::built::ServiceEndpoints const& endpoints,
Expand Down Expand Up @@ -64,6 +64,6 @@
data_components::ChangeNotifier change_notifier_;
// Needs to be shared to that the source can keep itself alive through
// async operations.
std::shared_ptr<data_interfaces::IPushSource> synchronizer_;
std::shared_ptr<data_interfaces::IDataSynchronizer> synchronizer_;
};
} // namespace launchdarkly::server_side::data_systems
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#pragma once

#include "../../../../data_components/status_notifications/data_source_status_manager.hpp"
#include "../../../../data_interfaces/source/ipush_source.hpp"
#include "../../../../data_interfaces/source/idata_synchronizer.hpp"

#include <boost/asio/any_io_executor.hpp>

namespace launchdarkly::server_side::data_systems {

class NullDataSource : public data_interfaces::IPushSource {
class NullDataSource : public data_interfaces::IDataSynchronizer {
public:
explicit NullDataSource(
boost::asio::any_io_executor exec,
Expand All @@ -15,7 +15,7 @@

void Init(std::optional<data_model::SDKDataSet> initial_data) override;
void StartAsync() override;
void ShutdownAsync(std::function<void()>) override;

Check warning on line 18 in libs/server-sdk/src/data_systems/background_sync/sources/noop/null_data_source.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/src/data_systems/background_sync/sources/noop/null_data_source.hpp:18:45 [readability-named-parameter]

all parameters should be named in a function

[[nodiscard]] std::string const& Identity() const override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include "../../../../data_components/status_notifications/data_source_status_manager.hpp"
#include "../../../../data_interfaces/destination/idestination.hpp"
#include "../../../../data_interfaces/source/ipush_source.hpp"
#include "../../../../data_interfaces/source/idata_synchronizer.hpp"

#include <launchdarkly/server_side/config/built/all_built.hpp>

Expand All @@ -16,7 +16,7 @@
namespace launchdarkly::server_side::data_systems {

class PollingDataSource
: public data_interfaces::IPushSource,
: public data_interfaces::IDataSynchronizer,
public std::enable_shared_from_this<PollingDataSource> {
public:
PollingDataSource(config::built::ServiceEndpoints const& endpoints,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include "../../../../data_components/status_notifications/data_source_status_manager.hpp"
#include "../../../../data_interfaces/destination/idestination.hpp"
#include "../../../../data_interfaces/source/ipush_source.hpp"
#include "../../../../data_interfaces/source/idata_synchronizer.hpp"

#include <launchdarkly/config/shared/built/data_source_config.hpp>
#include <launchdarkly/config/shared/built/http_properties.hpp>
Expand All @@ -24,7 +24,7 @@ using namespace std::chrono_literals;
namespace launchdarkly::server_side::data_systems {

class StreamingDataSource final
: public data_interfaces::IPushSource,
: public data_interfaces::IDataSynchronizer,
public std::enable_shared_from_this<StreamingDataSource> {
public:
StreamingDataSource(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* LazyLoad is able to remain efficient because it caches responses from the
* store. Over time, data becomes stale causing the system to refresh data.
*/
class LazyLoad : public data_interfaces::IDataSystem {

Check warning on line 30 in libs/server-sdk/src/data_systems/lazy_load/lazy_load_system.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sdk/src/data_systems/lazy_load/lazy_load_system.hpp:30:7 [cppcoreguidelines-special-member-functions]

class 'LazyLoad' defines a copy constructor, a copy assignment operator, a move constructor and a move assignment operator but does not define a destructor
public:
LazyLoad();

Expand Down Expand Up @@ -80,8 +80,8 @@
}

mutable data_components::MemoryStore cache_;
std::shared_ptr<data_interfaces::ISerializedDataPullSource> raw_source_;
data_components::JsonSource source_;
std::shared_ptr<data_interfaces::ISerializedDataReader> raw_source_;
data_components::JsonDataReader source_;

mutable data_components::ExpirationTracker tracker_;
std::function<std::chrono::time_point<std::chrono::steady_clock>()> time_;
Expand Down
Loading