-
Notifications
You must be signed in to change notification settings - Fork 2
feat:Properties: New Arrays and GeoCoordinate support #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
Conversation
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.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | View in Orca |
src/Weaviate.Client/DataClient.cs
Outdated
| [ | ||
| .. guids.Select(uuid => new Dictionary<string, string> | ||
| { | ||
| { "beacon", $"weaviate://localhost/{uuid}" }, |
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.
note: beacons may optionally specify the name of the collection to which that UUID belongs, in which case they have this format weaviate://localhost/{collection}/{uuid}.
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.
Yes, for multi-target references. Those are scheduled for a later beta.
| return char.ToLower(str[0]) + str[1..]; | ||
| } | ||
|
|
||
| public static bool IsNativeType(this Type type) |
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.
question: what is the difference between how native and non-native types are handled?
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.
If a type is native (primitive) I can just assign it. If it's a well-known object type (guid, geo, some datetime related types) I'm assuming the calling code can handle it directly. If it's an array, I then look into the element type and if it's a native type I can make the same assumption.
Basically, this is to rule out that a type is not a nested object of a custom type, which then needs to be handled as an object or list of objects, and that's scoped for later.
| { | ||
| object? nestedValue = UnmarshallProperties<ExpandoObject>(subDict); | ||
|
|
||
| target[kvp.Key.Capitalize()] = nestedValue ?? subDict; |
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.
question: why do we "capitalize" the key here?
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.
Convention, mostly. It's common for class properties to start with a capital letter. There's no technical requirement for keys to be capitalized.
Usually when the instance of type T is of IDictionary<string, object?> type, we're dealing with a dynamic/ExpandoObject type.
|
|
||
| var type = typeof(T); | ||
| var properties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance) | ||
| .Where(p => p.CanWrite) |
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.
question: if a property has CanWrite==false, does this mean it can only be set in constructor?
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.
Almost, it means it's read only OR init only. Some properties can be used for deriving values from other properties, sort of like parameter-less functions.
In a few lines below, we try to assign values to properties via reflection with a case-insensitive best-match, therefore we need to know which properties are writable, since attempting to write to a read only property will throw an exception.
Extras: - Simplified object retrieval in FetchObjectByID method to return a single object. - Updated tests to assert against the new object structure. - Enhanced batch property handling to support array types in DataClient. - Improved extension methods for better stream handling of native types.
Enhance serialization for array properties and introduce GeoCoordinate and PhoneNumber models. Update tests to validate new functionality and ensure compatibility with existing property types.
resolves #30