-
Notifications
You must be signed in to change notification settings - Fork 536
Change feed: Adds id and pk to ChangeFeedMetadata for delete operations #5191
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
Changes from all commits
f629e50
970b689
1823f65
1e66f3b
60223b4
aacd447
b2f0e2a
4129a95
c41d59c
5ddaec1
31bc8b8
d0d775d
59f53a5
2870d20
079e9aa
aa60a72
2658b33
8cd1360
8694811
c175e7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
| namespace Microsoft.Azure.Cosmos | ||
| { | ||
| using System; | ||
| using System.Text.Json; | ||
| using System.Collections.Generic; | ||
| using Microsoft.Azure.Cosmos.Resource.FullFidelity; | ||
| using Microsoft.Azure.Cosmos.Resource.FullFidelity.Converters; | ||
| using Microsoft.Azure.Documents; | ||
|
|
@@ -16,43 +16,49 @@ namespace Microsoft.Azure.Cosmos | |
| /// The metadata of a change feed resource with <see cref="ChangeFeedMode"/> is initialized to <see cref="ChangeFeedMode.AllVersionsAndDeletes"/>. | ||
| /// </summary> | ||
| [System.Text.Json.Serialization.JsonConverter(typeof(ChangeFeedMetadataConverter))] | ||
| [JsonConverter(typeof(ChangeFeedMetadataNewtonSoftConverter))] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is newtonsoftconverter needed?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because we added list of tuple<string, object> for partition key in the metadata which the default JsonProperty is not able to deserialize
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True all changeFeed is applied with user-serializer.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking loud here, for non-primitive types respective de-serializers are needed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One question I have is what is the purpose of moving all the annotations from individual properties to the class?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One drawback I see of this approach is that we loose the capability to do different null value handling behavior for properties
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran into it and what I observed was when you have a class-level JsonConverter attribute, the converter takes full control. Any property-level annotations (like [JsonPropertyName], [JsonProperty], etc.) are bypassed. |
||
| #if PREVIEW | ||
| public | ||
| #else | ||
| internal | ||
| #endif | ||
| class ChangeFeedMetadata | ||
| class ChangeFeedMetadata | ||
| { | ||
| /// <summary> | ||
| /// The change's conflict resolution timestamp. | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = ChangeFeedMetadataFields.ConflictResolutionTimestamp, NullValueHandling = NullValueHandling.Ignore)] | ||
| [JsonConverter(typeof(UnixDateTimeConverter))] | ||
| public DateTime ConflictResolutionTimestamp { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// The current change's logical sequence number. | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = ChangeFeedMetadataFields.Lsn, NullValueHandling = NullValueHandling.Ignore)] | ||
| public long Lsn { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// The change's feed operation type <see cref="ChangeFeedOperationType"/>. | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = ChangeFeedMetadataFields.OperationType, NullValueHandling = NullValueHandling.Ignore)] | ||
| [JsonConverter(typeof(StringEnumConverter))] | ||
| public ChangeFeedOperationType OperationType { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// The previous change's logical sequence number. | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = ChangeFeedMetadataFields.PreviousImageLSN, NullValueHandling = NullValueHandling.Ignore)] | ||
| public long PreviousLsn { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// Used to distinquish explicit deletes (e.g. via DeleteItem) from deletes caused by TTL expiration (a collection may define time-to-live policy for documents). | ||
| /// Used to distinguish explicit deletes (e.g. via DeleteItem) from deletes caused by TTL expiration (a collection may define time-to-live policy for documents). | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = ChangeFeedMetadataFields.TimeToLiveExpired, NullValueHandling = NullValueHandling.Ignore)] | ||
| public bool IsTimeToLiveExpired { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// Applicable for delete operations only, otherwise null. | ||
| /// The id of the previous item version. | ||
| /// </summary> | ||
| public string Id { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// Applicable for delete operations only, otherwise null. | ||
| /// The partition key of the previous item version. string is the partition key property name and object is the partition key property value. All levels of hierarchy will be represented in order if a HPK is used. | ||
| /// </summary> | ||
| public List<(string, object)> PartitionKey { get; internal set; } | ||
|
dibahlfi marked this conversation as resolved.
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,201 @@ | ||
| // ------------------------------------------------------------ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // ------------------------------------------------------------ | ||
|
|
||
| namespace Microsoft.Azure.Cosmos.Resource.FullFidelity.Converters | ||
| { | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using Microsoft.Azure.Cosmos.Spatial; | ||
| using Newtonsoft.Json; | ||
|
|
||
| internal class ChangeFeedMetadataNewtonSoftConverter : JsonConverter | ||
| { | ||
| private readonly static DateTime UnixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, 0, DateTimeKind.Utc); | ||
|
|
||
| /// <summary> | ||
| /// Writes the JSON representation of the object. | ||
| /// </summary> | ||
| /// <param name="writer">The <see cref="JsonWriter"/> to write to.</param> | ||
| /// <param name="value">The object value to write.</param> | ||
| /// <param name="serializer">The calling serializer.</param> | ||
| public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) | ||
| { | ||
| if (value is ChangeFeedMetadata metadata) | ||
| { | ||
| writer.WriteStartObject(); | ||
| writer.WritePropertyName(ChangeFeedMetadataFields.ConflictResolutionTimestamp); | ||
| serializer.Serialize(writer, ChangeFeedMetadataNewtonSoftConverter.ToUnixTimeInSecondsFromDateTime(metadata.ConflictResolutionTimestamp)); | ||
|
|
||
| writer.WritePropertyName(ChangeFeedMetadataFields.Lsn); | ||
| writer.WriteValue(metadata.Lsn); | ||
|
|
||
| writer.WritePropertyName(ChangeFeedMetadataFields.OperationType); | ||
| serializer.Serialize(writer, metadata.OperationType); | ||
|
|
||
| writer.WritePropertyName(ChangeFeedMetadataFields.PreviousImageLSN); | ||
| writer.WriteValue(metadata.PreviousLsn); | ||
|
|
||
| writer.WritePropertyName(ChangeFeedMetadataFields.TimeToLiveExpired); | ||
| writer.WriteValue(metadata.IsTimeToLiveExpired); | ||
|
|
||
| writer.WritePropertyName(ChangeFeedMetadataFields.Id); | ||
| writer.WriteValue(metadata.Id); | ||
| if (metadata.PartitionKey != null) | ||
| { | ||
| writer.WritePropertyName(ChangeFeedMetadataFields.PartitionKey); | ||
| writer.WriteStartObject(); | ||
|
|
||
| foreach ((string key, object objectValue) in metadata.PartitionKey) | ||
| { | ||
| writer.WritePropertyName(key); | ||
|
|
||
| if (objectValue == null) | ||
| { | ||
| writer.WriteNull(); | ||
| } | ||
| else | ||
| { | ||
| switch (objectValue) | ||
| { | ||
| case string stringValue: | ||
| writer.WriteValue(stringValue); | ||
| break; | ||
|
|
||
| case long longValue: | ||
| writer.WriteValue(longValue); | ||
| break; | ||
|
|
||
| case double doubleValue: | ||
| writer.WriteValue(doubleValue); | ||
| break; | ||
|
|
||
| case bool boolValue: | ||
| writer.WriteValue(boolValue); | ||
| break; | ||
|
|
||
| default: | ||
| throw new JsonSerializationException($"Unexpected value type '{objectValue.GetType()}' for PartitionKey property '{key}'."); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| writer.WriteEndObject(); // End PartitionKey object | ||
| } | ||
|
|
||
| writer.WriteEndObject(); | ||
| } | ||
| else | ||
| { | ||
| throw new JsonSerializationException($"Unexpected value '{value}' of type '{value?.GetType()}' when converting {nameof(ChangeFeedMetadata)}."); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Reads the JSON representation of the object. | ||
| /// </summary> | ||
| /// <param name="reader">The <see cref="JsonReader"/> to read from.</param> | ||
| /// <param name="objectType">Type of the object.</param> | ||
| /// <param name="existingValue">The existing value of object being read.</param> | ||
| /// <param name="serializer">The calling serializer.</param> | ||
| /// <returns>The deserialized object.</returns> | ||
| public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) | ||
| { | ||
| if (reader.TokenType == JsonToken.Null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| ChangeFeedMetadata metadata = new ChangeFeedMetadata(); | ||
| List<(string, object)> partitionKey = null; | ||
|
|
||
| reader.Read(); // StartObject | ||
|
|
||
| while (reader.TokenType == JsonToken.PropertyName) | ||
| { | ||
| string propertyName = reader.Value.ToString(); | ||
| reader.Read(); // Move to property value | ||
|
|
||
| switch (propertyName) | ||
| { | ||
| case ChangeFeedMetadataFields.ConflictResolutionTimestamp: | ||
| metadata.ConflictResolutionTimestamp = ChangeFeedMetadataNewtonSoftConverter.ToDateTimeFromUnixTimeInSeconds(Convert.ToInt64(reader.Value)); | ||
| break; | ||
|
|
||
| case ChangeFeedMetadataFields.Lsn: | ||
| metadata.Lsn = reader.Value != null ? Convert.ToInt64(reader.Value) : 0; | ||
| break; | ||
|
|
||
| case ChangeFeedMetadataFields.OperationType: | ||
| metadata.OperationType = serializer.Deserialize<ChangeFeedOperationType>(reader); | ||
| break; | ||
|
|
||
| case ChangeFeedMetadataFields.PreviousImageLSN: | ||
| metadata.PreviousLsn = reader.Value != null ? Convert.ToInt64(reader.Value) : 0; | ||
| break; | ||
|
|
||
| case ChangeFeedMetadataFields.TimeToLiveExpired: | ||
| metadata.IsTimeToLiveExpired = reader.Value != null && Convert.ToBoolean(reader.Value); | ||
| break; | ||
|
|
||
| case ChangeFeedMetadataFields.Id: | ||
| metadata.Id = reader.Value?.ToString(); | ||
| break; | ||
|
|
||
| case ChangeFeedMetadataFields.PartitionKey: | ||
| if (reader.TokenType == JsonToken.StartObject) | ||
|
dibahlfi marked this conversation as resolved.
|
||
| { | ||
| partitionKey ??= new List<(string, object)>(); | ||
| reader.Read(); // Move to the first property in the object | ||
| while (reader.TokenType == JsonToken.PropertyName) | ||
| { | ||
| string key = reader.Value.ToString(); | ||
| reader.Read(); // Move to the value of the property | ||
|
|
||
| object value = reader.TokenType switch | ||
| { | ||
| JsonToken.String => reader.Value.ToString(), | ||
| JsonToken.Integer => Convert.ToInt64(reader.Value), | ||
| JsonToken.Float => Convert.ToDouble(reader.Value), | ||
| JsonToken.Boolean => Convert.ToBoolean(reader.Value), | ||
| JsonToken.Null => null, | ||
| _ => throw new JsonSerializationException($"Unexpected token type: {reader.TokenType} for PartitionKey property.") | ||
| }; | ||
|
|
||
| partitionKey.Add((key, value)); | ||
| reader.Read(); // Move to the next property or EndObject | ||
| } | ||
| metadata.PartitionKey = partitionKey; | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| reader.Skip(); | ||
| break; | ||
| } | ||
|
|
||
| reader.Read(); // Move to next property or EndObject | ||
| } | ||
| return metadata; | ||
| } | ||
| /// <summary> | ||
| /// Determines whether this instance can convert the specified object type. | ||
| /// </summary> | ||
| /// <param name="objectType">Type of the object.</param> | ||
| /// <returns><c>true</c> if this instance can convert the specified object type; otherwise, <c>false</c>.</returns> | ||
| public override bool CanConvert(Type objectType) | ||
| { | ||
| return objectType == typeof(ChangeFeedMetadata); | ||
| } | ||
|
|
||
| private static long ToUnixTimeInSecondsFromDateTime(DateTime date) | ||
| { | ||
| return (long)(date - ChangeFeedMetadataNewtonSoftConverter.UnixEpoch).TotalSeconds; | ||
| } | ||
|
|
||
| private static DateTime ToDateTimeFromUnixTimeInSeconds(long unixTimeInSeconds) | ||
| { | ||
| return ChangeFeedMetadataNewtonSoftConverter.UnixEpoch.AddSeconds(unixTimeInSeconds); | ||
| } | ||
| } | ||
| } | ||
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.
On overall approach: new possible approach is to leverage https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonincludeattribute?view=net-9.0&viewFallbackFrom=netstandard-2.0 (assuming that we can use it with-out rebasing to latest .NET versions.
/cc: @yash2710 , @dibahlfi