feat: segment data model #153
Merged
Conversation
This file contains hidden or 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
…legates to std::optional<T> overloads
|
This pull request has been linked to Shortcut Story #207649: Segment data model. |
Merged
kinyoklion
reviewed
Jun 30, 2023
kinyoklion
reviewed
Jun 30, 2023
| status_manager); | ||
|
|
||
| auto res = stream_handler.HandleMessage("put", "{\"potato\": {}}"); | ||
| auto res = stream_handler.HandleMessage("put", "{\"potato\": 3}"); |
Member
There was a problem hiding this comment.
Can I get an explainer on this one?
Contributor
Author
There was a problem hiding this comment.
Reverted.
This was due to some confusion on my part - I intended to make {} be a valid message (viewing it as an implicit value of 0 for version, and null for value.)
But I implemented that logic incorrectly and made it a valid deserialization because I was treating it as empty/omitted.
That's a backwards way of thinking about it: "it's an empty object, therefore treat it as empty/omitted" vs "it's omitted/omitted, therefore treat it as an empty object" - the former is wrong, the latter is what I'm doing in the later Flag Model PR.
kinyoklion
reviewed
Jun 30, 2023
kinyoklion
reviewed
Jun 30, 2023
| struct SDKDataSet { | ||
| using FlagKey = std::string; | ||
| using SegmentKey = std::string; | ||
| // std::unordered_map<FlagKey, ItemDescriptor<Flag>> flags; |
Contributor
Author
There was a problem hiding this comment.
(This one is added in the next (flag model) PR)
kinyoklion
reviewed
Jun 30, 2023
kinyoklion
reviewed
Jun 30, 2023
kinyoklion
approved these changes
Jun 30, 2023
kinyoklion
approved these changes
Jun 30, 2023
kinyoklion
approved these changes
Jun 30, 2023
cwaldren-ld
added a commit
that referenced
this pull request
Jun 30, 2023
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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_invokefunctions present in this PR are what allows theboost::jsonlibrary to recursively deserialize complex structures.Here, I've added a couple new
tag_invokesfor bool, string, vector, etc. The special part is that they deserialize intoexpected<type, error>rather than the primitive directly; this allows us to return an error if the deserialization failed.