Skip to content

Refactor EntityMetadata to improve state consumption#195

Merged
jviau merged 4 commits intomicrosoft:feature/entitiesfrom
jviau:client-typing
Sep 25, 2023
Merged

Refactor EntityMetadata to improve state consumption#195
jviau merged 4 commits intomicrosoft:feature/entitiesfrom
jviau:client-typing

Conversation

@jviau
Copy link
Member

@jviau jviau commented Sep 22, 2023

This PR changes to following:

  1. includeState is true by default for all get entity methods (including get all)
  2. Added EntityMetadata<TState>, which has a strongly typed deserialized TState State property.
  3. Extracted un-typed state into SerializedData, which has a ReadAs<T>() method.
  4. EntityMetadata now implements EntityMetadata<SerializedData>
  5. EntityMetadata<TState> now has a bool IncludesState which indicates if state was retrieved or not.
  6. EntityMetadata<TState>.State now throws if IncludeState==false
  7. Added GetEntityAsync<T> and GetAllEntitiesAsync<T> overloads to get EntityMetadata<T>

Copy link
Member

@sebastianburckhardt sebastianburckhardt left a comment

Choose a reason for hiding this comment

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

some suggested changes to the comments, and one question about a possible simplification.
Otherwise looks good to me.

public string? SerializedState { get; init; }
[MemberNotNullWhen(true, "State")]
[MemberNotNullWhen(true, "state")]
public bool IncludesState { get; }

Choose a reason for hiding this comment

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

we could perhaps implement this more simply as

public bool IncludesState => this.state is not null;

Copy link
Member Author

@jviau jviau Sep 25, 2023

Choose a reason for hiding this comment

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

I don't have that to handle default value types. Basically, this has to be a pseudo Optional. Need to differentiate between state was fetched and it was a default value type, or state was not fetched at all (in which case the value is still default).

@jviau jviau merged commit 10bee17 into microsoft:feature/entities Sep 25, 2023
@jviau jviau deleted the client-typing branch September 25, 2023 17:30
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