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.

Sign up for GitHub

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

Jump to bottom

Fixed public properties to have same JSON attributes #1112

Merged
merged 11 commits into from
Jan 21, 2020
Merged

Fixed public properties to have same JSON attributes #1112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b41d79b
Fixed public properties to all have the same JSON attributes for the …
Dec 19, 2019
04c757f
Updated changelog
Dec 19, 2019
1f1a23c
reverted type change to avoid breaking change
Dec 19, 2019
c43f9f7
Merge branch 'master' into users/jawilley/bug/lastmodifiedtime
j82w Jan 21, 2020
7d42953
Merged to latest
Jan 21, 2020
a688edc
Updated unit tests
Jan 21, 2020
eed56de
Merge branch 'users/jawilley/bug/lastmodifiedtime' of https://github.…
Jan 21, 2020
2a007f4
Updated contract change
Jan 21, 2020
2f9484b
Merge remote-tracking branch 'origin/master' into users/jawilley/bug/…
Jan 21, 2020
502dc3e
Set formatting in test
Jan 21, 2020
b6563b8
Set formatting on another serializer
Jan 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Microsoft.Azure.Cosmos/src/Resource/Settings/AccountProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ internal AccountProperties()
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
public string ETag { get; internal set; }

/// <summary>
Expand All @@ -89,7 +89,7 @@ internal AccountProperties()
/// resource whether that is a database, a collection or a document.
/// These resource ids are used when building up SelfLinks, a static addressable Uri for each resource within a database account.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.RId)]
[JsonProperty(PropertyName = Constants.Properties.RId, NullValueHandling = NullValueHandling.Ignore)]
internal string ResourceId { get; set; }

[JsonProperty(PropertyName = Constants.Properties.WritableLocations)]
Expand Down
6 changes: 3 additions & 3 deletions Microsoft.Azure.Cosmos/src/Resource/Settings/DatabaseProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ public string Id
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
public string ETag { get; private set; }

/// <summary>
/// Gets the last modified time stamp associated with <see cref="DatabaseProperties" /> from the Azure Cosmos DB service.
/// </summary>
/// <value>The last modified time stamp associated with the resource.</value>
[JsonConverter(typeof(UnixDateTimeConverter))]
[JsonProperty(PropertyName = Constants.Properties.LastModified)]
[JsonProperty(PropertyName = Constants.Properties.LastModified, NullValueHandling = NullValueHandling.Ignore)]
public DateTime? LastModified { get; private set; }

/// <summary>
Expand All @@ -145,7 +145,7 @@ public string Id
/// resource whether that is a database, a collection or a document.
/// These resource ids are used when building up SelfLinks, a static addressable Uri for each resource within a database account.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.RId)]
[JsonProperty(PropertyName = Constants.Properties.RId, NullValueHandling = NullValueHandling.Ignore)]
internal string ResourceId { get; private set; }
}
}
4 changes: 2 additions & 2 deletions Microsoft.Azure.Cosmos/src/Resource/Settings/PermissionProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Microsoft.Azure.Cosmos
public class PermissionProperties
{
/// <summary>
/// Initialize a new instance of the <see cref="PermissionProperties"/> with permssion to <see cref="Container"/>.
/// Initialize a new instance of the <see cref="PermissionProperties"/> with permission to <see cref="Container"/>.
/// </summary>
/// <param name="id">The permission id.</param>
/// <param name="permissionMode">The <see cref="PermissionMode"/>.</param>
Expand All @@ -40,7 +40,7 @@ public PermissionProperties(string id,
}

/// <summary>
/// Initialize a new instance of the <see cref="PermissionProperties"/> with permssion to cosnmos item.
/// Initialize a new instance of the <see cref="PermissionProperties"/> with permission to Cosmos item.
/// </summary>
/// <param name="id">The permission id.</param>
/// <param name="permissionMode">The <see cref="PermissionMode"/>.</param>
Expand Down
6 changes: 3 additions & 3 deletions Microsoft.Azure.Cosmos/src/Resource/Settings/StoredProcedureProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ public string Id
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
public string ETag { get; private set; }

/// <summary>
/// Gets the last modified timestamp associated with <see cref="StoredProcedureProperties" /> from the Azure Cosmos DB service.
/// </summary>
/// <value>The last modified timestamp associated with the resource.</value>
[JsonConverter(typeof(UnixDateTimeConverter))]
[JsonProperty(PropertyName = Constants.Properties.LastModified)]
[JsonProperty(PropertyName = Constants.Properties.LastModified, NullValueHandling = NullValueHandling.Ignore)]
public DateTime? LastModified { get; private set; }

/// <summary>
Expand All @@ -114,7 +114,7 @@ public string Id
/// resource whether that is a database, a collection or a document.
/// These resource ids are used when building up SelfLinks, a static addressable Uri for each resource within a database account.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.RId)]
[JsonProperty(PropertyName = Constants.Properties.RId, NullValueHandling = NullValueHandling.Ignore)]
internal string ResourceId { get; private set; }
}
}
10 changes: 5 additions & 5 deletions Microsoft.Azure.Cosmos/src/Resource/Settings/ThroughputProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ public class ThroughputProperties
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
Copy link
Member

Choose a reason for hiding this comment

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

Is this breaking today? or what's the reason to add the NullValueHandling?

Copy link
Contributor Author

@j82w j82w Dec 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Half of the properties have the ignore the other don't. It seems like making it consistent between all the types was missed. Serializing a null _etag or other system properties and sending it to the backend only increase payload size and has no benefit.

public string ETag { get; private set; }

/// <summary>
/// Gets the last modified time stamp associated with <see cref="DatabaseProperties" /> from the Azure Cosmos DB service.
/// </summary>
/// <value>The last modified time stamp associated with the resource.</value>
[JsonConverter(typeof(UnixDateTimeConverter))]
[JsonProperty(PropertyName = Constants.Properties.LastModified)]
public DateTime LastModified { get; private set; }
[JsonProperty(PropertyName = Constants.Properties.LastModified, NullValueHandling = NullValueHandling.Ignore)]
public DateTime? LastModified { get; private set; }

/// <summary>
/// Gets the provisioned throughput for a resource in measurement of request units per second in the Azure Cosmos service.
Expand All @@ -69,13 +69,13 @@ public int? Throughput
/// <summary>
/// Gets the offer rid.
/// </summary>
[JsonProperty(PropertyName = Constants.Properties.RId)]
[JsonProperty(PropertyName = Constants.Properties.RId, NullValueHandling = NullValueHandling.Ignore)]
internal string OfferRID { get; private set; }

/// <summary>
/// Gets the resource rid.
/// </summary>
[JsonProperty(PropertyName = Constants.Properties.OfferResourceId)]
[JsonProperty(PropertyName = Constants.Properties.OfferResourceId, NullValueHandling = NullValueHandling.Ignore)]
internal string ResourceRID { get; private set; }

[JsonProperty(PropertyName = "content", DefaultValueHandling = DefaultValueHandling.Ignore)]
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/Resource/Settings/TriggerProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class TriggerProperties
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
public string ETag { get; private set; }

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/Resource/Settings/UserDefinedFunctionProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public class UserDefinedFunctionProperties
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
public string ETag { get; private set; }

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions Microsoft.Azure.Cosmos/src/Resource/Settings/UserProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ public string Id
/// <remarks>
/// ETags are used for concurrency checking when updating resources.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.ETag)]
[JsonProperty(PropertyName = Constants.Properties.ETag, NullValueHandling = NullValueHandling.Ignore)]
public string ETag { get; private set; }

/// <summary>
/// Gets the last modified time stamp associated with <see cref="DatabaseProperties" /> from the Azure Cosmos DB service.
/// </summary>
/// <value>The last modified time stamp associated with the resource.</value>
[JsonConverter(typeof(UnixDateTimeConverter))]
[JsonProperty(PropertyName = Constants.Properties.LastModified)]
[JsonProperty(PropertyName = Constants.Properties.LastModified, NullValueHandling = NullValueHandling.Ignore)]
public DateTime? LastModified { get; private set; }

/// <summary>
Expand All @@ -98,7 +98,7 @@ public string Id
/// resource whether that is a database, a collection or a document.
/// These resource ids are used when building up SelfLinks, a static addressable Uri for each resource within a database account.
/// </remarks>
[JsonProperty(PropertyName = Constants.Properties.RId)]
[JsonProperty(PropertyName = Constants.Properties.RId, NullValueHandling = NullValueHandling.Ignore)]
internal string ResourceId { get; set; }

/// <summary>
Expand Down
67 changes: 67 additions & 0 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosJsonSeriliazerUnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,73 @@ public void ValidateSerializer()
}
}

[TestMethod]
public void ValidatePropertySerialization()
{
string id = "testId";
this.TestProperty<AccountProperties>(
id,
$@"{{""id"":""{id}"",""writableLocations"":[],""readableLocations"":[],""userConsistencyPolicy"":null,""addresses"":null,""userReplicationPolicy"":null,""systemReplicationPolicy"":null,""readPolicy"":null,""queryEngineConfiguration"":null,""enableMultipleWriteLocations"":false}}");

this.TestProperty<DatabaseProperties>(
id,
$@"{{""id"":""{id}""}}");

this.TestProperty<ContainerProperties>(
id,
$@"{{""id"":""{id}"",""partitionKey"":{{""paths"":[],""kind"":""Hash""}}}}");

this.TestProperty<StoredProcedureProperties>(
id,
$@"{{""body"":""bodyCantBeNull"",""id"":""testId""}}");

this.TestProperty<TriggerProperties>(
id,
$@"{{""body"":null,""triggerType"":""Pre"",""triggerOperation"":""All"",""id"":""{id}""}}");

this.TestProperty<UserDefinedFunctionProperties>(
id,
$@"{{""body"":null,""id"":""{id}""}}");

this.TestProperty<UserProperties>
(id,
$@"{{""id"":""{id}"",""_permissions"":null}}");

this.TestProperty<PermissionProperties>(
id,
$@"{{""id"":""{id}"",""resource"":null,""permissionMode"":0}}");

this.TestProperty<ConflictProperties>(
id,
$@"{{""id"":""{id}"",""operationType"":""Invalid"",""resourceType"":null,""resourceId"":null,""content"":null,""conflict_lsn"":0}}");

// Throughput doesn't have an id.
string defaultThroughputJson = @"{""Throughput"":null}";
ThroughputProperties property = JsonConvert.DeserializeObject<ThroughputProperties>(defaultThroughputJson);
Assert.IsNull(property.Throughput);
string propertyJson = JsonConvert.SerializeObject(property, new JsonSerializerSettings()
{
Formatting = Formatting.None
});
Assert.AreEqual(defaultThroughputJson, propertyJson);
}

private void TestProperty<T>(string id, string defaultJson)
{
dynamic property = JsonConvert.DeserializeObject<T>(defaultJson);
Assert.AreEqual(id, property.Id);
string propertyJson = JsonConvert.SerializeObject(property, new JsonSerializerSettings() {
Formatting = Formatting.None
});

Assert.AreEqual(defaultJson, propertyJson);
// System properties should be ignored if null
Assert.IsFalse(propertyJson.Contains("_etag"));
Assert.IsFalse(propertyJson.Contains("_rid"));
Assert.IsFalse(propertyJson.Contains("_ts"));
Assert.IsFalse(propertyJson.Contains("_self"));
}

[TestMethod]
public void ValidateJson()
{
Expand Down
14 changes: 7 additions & 7 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosSerializerCoreTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ public void ValidateCustomSerializerNotUsedForInternalTypes()
this.TestProperty<AccountProperties>(
serializerCore,
id,
$@"{{""id"":""{id}"",""_etag"":null,""_rid"":null,""writableLocations"":[],""readableLocations"":[],""userConsistencyPolicy"":null,""addresses"":null,""userReplicationPolicy"":null,""systemReplicationPolicy"":null,""readPolicy"":null,""queryEngineConfiguration"":null,""enableMultipleWriteLocations"":false}}");
$@"{{""id"":""{id}"",""writableLocations"":[],""readableLocations"":[],""userConsistencyPolicy"":null,""addresses"":null,""userReplicationPolicy"":null,""systemReplicationPolicy"":null,""readPolicy"":null,""queryEngineConfiguration"":null,""enableMultipleWriteLocations"":false}}");

this.TestProperty<DatabaseProperties>(
serializerCore,
id,
$@"{{""id"":""{id}"",""_etag"":null,""_ts"":1576767176,""_rid"":null}}");
$@"{{""id"":""{id}""}}");

this.TestProperty<ContainerProperties>(
serializerCore,
Expand All @@ -118,22 +118,22 @@ public void ValidateCustomSerializerNotUsedForInternalTypes()
this.TestProperty<StoredProcedureProperties>(
serializerCore,
id,
$@"{{""body"":""bodyCantBeNull"",""id"":""{id}"",""_etag"":null,""_ts"":1576767176,""_rid"":null}}");
$@"{{""body"":""bodyCantBeNull"",""id"":""{id}""}}");

this.TestProperty<TriggerProperties>(
serializerCore,
id,
$@"{{""body"":null,""triggerType"":""Pre"",""triggerOperation"":""All"",""id"":""{id}"",""_etag"":null}}");
$@"{{""body"":null,""triggerType"":""Pre"",""triggerOperation"":""All"",""id"":""{id}""}}");

this.TestProperty<UserDefinedFunctionProperties>(
serializerCore,
id,
$@"{{""body"":null,""id"":""{id}"",""_etag"":null}}");
$@"{{""body"":null,""id"":""{id}""}}");

this.TestProperty<UserProperties>(
serializerCore,
id,
$@"{{""id"":""{id}"",""_etag"":null,""_ts"":1576767176,""_rid"":null,""_permissions"":null}}");
$@"{{""id"":""{id}"",""_permissions"":null}}");

this.TestProperty<PermissionProperties>(
serializerCore,
Expand All @@ -146,7 +146,7 @@ public void ValidateCustomSerializerNotUsedForInternalTypes()
$@"{{""id"":""{id}"",""operationType"":""Invalid"",""resourceType"":null,""resourceId"":null,""content"":null,""conflict_lsn"":0}}");

// Throughput doesn't have an id.
string defaultThroughputJson = @"{""_etag"":null,""_ts"":1576767176,""Throughput"":null,""_rid"":null,""offerResourceId"":null}";
string defaultThroughputJson = @"{""Throughput"":null}";
ThroughputProperties property = JsonConvert.DeserializeObject<ThroughputProperties>(defaultThroughputJson);
Assert.IsNull(property.Throughput);
string propertyJson = JsonConvert.SerializeObject(property);
Expand Down
Loading