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

feat: Add common persistence. #193

Closed
wants to merge 78 commits into from
Closed

Conversation

kinyoklion
Copy link
Member

No description provided.

cwaldren-ld and others added 30 commits June 12, 2023 11:02
Moves the client's `ItemDescriptor` to internal where it can be shared,
and gives it a generic parameter so it can hold flag rules/segments.
Augments event processor with context-key deduplication abilities using
an LRU cache.
This PR extends our existing deserialization/serialization technique
from the client-side SDK to the server-side SDK data model.

Specifically, it adds deserialization for the segment structure. I've
also introduced some helpers to make the logic less repetitive, and
centralize some decisions (like what to do if a JSON field isn't
present, or is null.)

As background, the `tag_invoke` functions present in this PR are what
allows the `boost::json` library to recursively deserialize complex
structures.

Here, I've added a couple new `tag_invokes` for bool, string, vector,
etc. The special part is that they deserialize into `expected<type,
error>` rather than the primitive directly; this allows us to return an
error if the deserialization failed.
This PR builds on #153, extending the deserialization to flag data. 

I've added to the `PARSE_FIELD` macro to handle the following cases:
1. Parse a field where the zero-value is a valid part of the domain:
`PARSE_FIELD`. In other words, if the field is omitted or null, it will
be filled in with a default value. Most common scenario.
2. Like (1), but supply a default value in case the field is null or
omitted: `PARSE_FIELD_DEFAULT`.
3. Parse a field that doesn't have a valid default value, and so is
represented as an `optional`: `PARSE_CONDITIONAL_FIELD`. For example,
the value `""` wouldn't be a valid default for a unique key.
4. Parse a field that must be present in the JSON data:
`PARSE_REQUIRED_FIELD`. Usage of this should be rare; the test would be
if the SDK is totally unable to function without this.

To achieve this, I've made the `tag_invoke`s operate on
`expected<optional<T>, JsonError`'s, instead of `expected<T, JsonError`.

We could potentially revert that (and instead add a new enum case to
`JsonError`), but I figure `optional` is a good representation of
null/omitted. I think I need to actually try removing it to determine if
it results in a better API.

One thing lacking is better error messages - we use kSchemaFailure
everywhere but we now have the ability to report exactly what was
missing/wrong type, etc.
Adds in missing `Flag` model to the top-level `SDKDataSet`.

Additionally, both the flag/segment fields now use use `PARSE_FIELD`, so
we can get an empty `unordered_map` if they are missing.
Previously, context kind was an `std::string`. 

The requirement that it not be an empty string was enforced by parent
data model deserializers.

Now, we have a dedicated type-safe wrapper via`BOOST_STRONG_TYPEDEF`.
The custom `tag_invoke` enforces the invariant so it's not possible to
deserialize it inconsistently.
This commit contains the server-side evaluation engine and associated tests.
cwaldren-ld and others added 23 commits August 25, 2023 11:01
This significantly refactors the `Variation` internal calls to be much
easier to reason about. In the process, it fixes some broken behavior
which is now reflected in the suppressions list.

It also refactors event generation to use the
`EventScope`/`EventFactory` pattern found in other SDKs, such as Rust
and Go.
This removes a bunch of duplicated work from the top-level typed
Variation methods, and pushes it into the internal `Variation` method.
We had an infinite loop where the `Value` tag invoke deserializer was
calling the `tl::expected<Value, JsonError` version, which called back
to itself.

This was manifesting as a runtime infinite loop. 

It's not caught by contract tests since I believe the trigger is having
a list of variations that are themselves arrays, like:
```
"variations": [["a","b","c",123,true,"false"], [ .. other stuff ..], ... ]
```
The fix is to break the cycle and remove the incorrect implementation.
This moves all the `_ErrorInfo_` C bindings out of the client-side SDK
and into the common library, allowing them to be shared with the server.

This should be backwards compatible because the `sdk.h` includes the new
`error_info.h`, providing the same symbols as before.
Adds a new README for the server. 

Also adds a link from the client-side README to this one.

---------

Co-authored-by: Molly <[email protected]>
I hadn't wired up the `DataStoreUpdater` component, so we were just
inserting flag updates directly in the memory store.

With this in place, we can remove the streaming update suppression.
Implements a set of C bindings for the server-side SDK. 

Missing are flag notifier bindings, but this isn't implemented in the
C++ side yet either.
Hooks up the initial reconnect delay parameter to the server-side
streaming data source. Allows us to remove another batch of contract
test failures.
The argument to `ValidChar` is a `char`, so its max value is `255`
rendering the check redundant. This causes a warning on our Mac
compilations as well as a linter warning.

---------

Co-authored-by: Ryan Lamb <[email protected]>
Base automatically changed from server-side to main October 23, 2023 17:49
@cwaldren-ld cwaldren-ld force-pushed the rlamb/common-persistent-store branch from bfcb09c to 8345ebb Compare October 24, 2023 18:11
@cwaldren-ld cwaldren-ld closed this Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants