-
Notifications
You must be signed in to change notification settings - Fork 323
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
Protocol v2 changes #672
Protocol v2 changes #672
Changes from all commits
064340a
0507a7b
45b237a
b6e8238
e2da06d
dda2b84
3876bfa
9a0002c
5a0e4e9
ec2d332
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities | |
using System.IO; | ||
|
||
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces; | ||
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel; | ||
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Serialization; | ||
|
||
using Newtonsoft.Json; | ||
|
@@ -20,26 +19,33 @@ public class JsonDataSerializer : IDataSerializer | |
{ | ||
private static JsonDataSerializer instance; | ||
|
||
private static JsonSerializer serializer; | ||
private static JsonSerializer payloadSerializer; | ||
private static JsonSerializer payloadSerializer2; | ||
|
||
/// <summary> | ||
/// Prevents a default instance of the <see cref="JsonDataSerializer"/> class from being created. | ||
/// </summary> | ||
private JsonDataSerializer() | ||
{ | ||
serializer = JsonSerializer.Create( | ||
new JsonSerializerSettings | ||
{ | ||
ContractResolver = new TestPlatformContractResolver(), | ||
DateFormatHandling = DateFormatHandling.IsoDateFormat, | ||
DateParseHandling = DateParseHandling.DateTimeOffset, | ||
DateTimeZoneHandling = DateTimeZoneHandling.Utc, | ||
TypeNameHandling = TypeNameHandling.None | ||
}); | ||
var jsonSettings = new JsonSerializerSettings | ||
{ | ||
DateFormatHandling = DateFormatHandling.IsoDateFormat, | ||
DateParseHandling = DateParseHandling.DateTimeOffset, | ||
DateTimeZoneHandling = DateTimeZoneHandling.Utc, | ||
TypeNameHandling = TypeNameHandling.None | ||
}; | ||
|
||
payloadSerializer = JsonSerializer.Create(jsonSettings); | ||
payloadSerializer2 = JsonSerializer.Create(jsonSettings); | ||
|
||
payloadSerializer.ContractResolver = new TestPlatformContractResolver1(); | ||
payloadSerializer2.ContractResolver = new DefaultTestPlatformContractResolver(); | ||
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. When v3 comes in, we need to change this code? Why not consider versioning the ContractResolvers suffixed with version number. The class name need not say what's default, let it be determined only in this file. E.g. |
||
|
||
#if DEBUG | ||
// MemoryTraceWriter can help diagnose serialization issues. Enable it for | ||
// debug builds only. | ||
serializer.TraceWriter = new MemoryTraceWriter(); | ||
payloadSerializer.TraceWriter = new MemoryTraceWriter(); | ||
payloadSerializer2.TraceWriter = new MemoryTraceWriter(); | ||
#endif | ||
} | ||
|
||
|
@@ -61,7 +67,9 @@ public static JsonDataSerializer Instance | |
/// <returns>A <see cref="Message"/> instance.</returns> | ||
public Message DeserializeMessage(string rawMessage) | ||
{ | ||
return JsonConvert.DeserializeObject<Message>(rawMessage); | ||
// Convert to VersionedMessage | ||
// Message can be deserialized to VersionedMessage where version will be 0 | ||
return JsonConvert.DeserializeObject<VersionedMessage>(rawMessage); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -74,28 +82,24 @@ public T DeserializePayload<T>(Message message) | |
{ | ||
T retValue = default(T); | ||
|
||
// TODO: Currently we use json serializer auto only for non-testmessage types | ||
// CHECK: Can't we just use auto for everything | ||
if (MessageType.TestMessage.Equals(message.MessageType)) | ||
{ | ||
retValue = message.Payload.ToObject<T>(); | ||
} | ||
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. Assuming |
||
else | ||
{ | ||
retValue = message.Payload.ToObject<T>(serializer); | ||
} | ||
var versionedMessage = message as VersionedMessage; | ||
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. Please log the default serializer determined here. |
||
var serializer = this.GetPayloadSerializer(versionedMessage?.Version); | ||
|
||
retValue = message.Payload.ToObject<T>(serializer); | ||
return retValue; | ||
} | ||
|
||
/// <summary> | ||
/// Deserialize raw JSON to an object using the default serializer. | ||
/// </summary> | ||
/// <param name="json">JSON string.</param> | ||
/// <param name="version">Version of serializer to be used.</param> | ||
/// <typeparam name="T">Target type to deserialize.</typeparam> | ||
/// <returns>An instance of <see cref="T"/>.</returns> | ||
public T Deserialize<T>(string json) | ||
public T Deserialize<T>(string json, int version = 1) | ||
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. Should the caller be aware of 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. This is just the payload deserialization, caller reads the version from the message. |
||
{ | ||
var serializer = this.GetPayloadSerializer(version); | ||
|
||
using (var stringReader = new StringReader(json)) | ||
using (var jsonReader = new JsonTextReader(stringReader)) | ||
{ | ||
|
@@ -121,30 +125,41 @@ public string SerializeMessage(string messageType) | |
/// <returns>Serialized message.</returns> | ||
public string SerializePayload(string messageType, object payload) | ||
{ | ||
JToken serializedPayload = null; | ||
|
||
// TODO: Currently we use json serializer auto only for non-testmessage types | ||
// CHECK: Can't we just use auto for everything | ||
if (MessageType.TestMessage.Equals(messageType)) | ||
{ | ||
serializedPayload = JToken.FromObject(payload); | ||
} | ||
else | ||
{ | ||
serializedPayload = JToken.FromObject(payload, serializer); | ||
} | ||
var serializedPayload = JToken.FromObject(payload, payloadSerializer); | ||
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 are these APIs not using versioned messages? How would the caller decide which API to use (and when)? 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. We might want to change the APIs. |
||
|
||
return JsonConvert.SerializeObject(new Message { MessageType = messageType, Payload = serializedPayload }); | ||
} | ||
|
||
/// <summary> | ||
/// Serialize a message with payload. | ||
/// </summary> | ||
/// <param name="messageType">Type of the message.</param> | ||
/// <param name="payload">Payload for the message.</param> | ||
/// <param name="version">Version for the message.</param> | ||
/// <returns>Serialized message.</returns> | ||
public string SerializePayload(string messageType, object payload, int version) | ||
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. How is this tested? |
||
{ | ||
var serializer = this.GetPayloadSerializer(version); | ||
var serializedPayload = JToken.FromObject(payload, serializer); | ||
|
||
var message = version == 1 ? | ||
new Message { MessageType = messageType, Payload = serializedPayload } : | ||
new VersionedMessage { MessageType = messageType, Version = version, Payload = serializedPayload }; | ||
|
||
return JsonConvert.SerializeObject(message); | ||
} | ||
|
||
/// <summary> | ||
/// Serialize an object to JSON using default serialization settings. | ||
/// </summary> | ||
/// <typeparam name="T">Type of object to serialize.</typeparam> | ||
/// <param name="data">Instance of the object to serialize.</param> | ||
/// <param name="version">Version to be stamped.</param> | ||
/// <returns>JSON string.</returns> | ||
public string Serialize<T>(T data) | ||
public string Serialize<T>(T data, int version = 1) | ||
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. shouldn't this by default be the highest? |
||
{ | ||
var serializer = this.GetPayloadSerializer(version); | ||
|
||
using (var stringWriter = new StringWriter()) | ||
using (var jsonWriter = new JsonTextWriter(stringWriter)) | ||
{ | ||
|
@@ -153,5 +168,10 @@ public string Serialize<T>(T data) | |
return stringWriter.ToString(); | ||
} | ||
} | ||
|
||
private JsonSerializer GetPayloadSerializer(int? version) | ||
{ | ||
return version == 2 ? payloadSerializer2 : payloadSerializer; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities | ||
{ | ||
/// <summary> | ||
/// Construct with version used for communication | ||
/// Introduced in 15.1.0 version and default message protocol v2 onwards. | ||
/// </summary> | ||
public class VersionedMessage : Message | ||
{ | ||
/// <summary> | ||
/// Gets or sets the version of the message | ||
/// </summary> | ||
public int Version { get; set; } | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Serialization | ||
{ | ||
using System; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel; | ||
using Newtonsoft.Json; | ||
|
||
/// <summary> | ||
/// Converter used by v1 protocol serializer to serialize TestCase object to and from v1 json | ||
/// </summary> | ||
public class TestCaseConverter : JsonConverter | ||
{ | ||
/// <inheritdoc/> | ||
public override bool CanRead => false; | ||
|
||
/// <inheritdoc/> | ||
public override bool CanConvert(Type objectType) | ||
{ | ||
return typeof(TestCase) == objectType; | ||
} | ||
|
||
/// <inheritdoc/> | ||
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) | ||
{ | ||
// We do not need this as SetPropetyValue inside StoreKvpList will | ||
// set the properties as expected. | ||
throw new NotImplementedException(); | ||
} | ||
|
||
/// <inheritdoc/> | ||
public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) | ||
{ | ||
// P2 to P1 | ||
var testCase = value as TestCase; | ||
var properties = testCase.GetProperties(); | ||
|
||
writer.WriteStartObject(); | ||
writer.WritePropertyName("Properties"); | ||
writer.WriteStartArray(); | ||
foreach (var property in properties) | ||
{ | ||
serializer.Serialize(writer, property); | ||
} | ||
|
||
writer.WriteEndArray(); | ||
writer.WriteEndObject(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Serialization | ||
{ | ||
using System; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel; | ||
using Newtonsoft.Json.Serialization; | ||
|
||
/// <summary> | ||
/// JSON contract resolver for mapping test platform types for v1 serialization. | ||
/// </summary> | ||
public class TestPlatformContractResolver1 : DefaultTestPlatformContractResolver | ||
{ | ||
/// <inheritdoc/> | ||
protected override JsonContract CreateContract(Type objectType) | ||
{ | ||
var contract = base.CreateContract(objectType); | ||
if (typeof(TestCase) == objectType) | ||
{ | ||
contract.Converter = new TestCaseConverter(); | ||
} | ||
else if (typeof(TestResult) == objectType) | ||
{ | ||
contract.Converter = new TestResultConverter(); | ||
} | ||
|
||
return contract; | ||
} | ||
} | ||
} |
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.
an array maybe?