Skip to content

Commit 47257f8

Browse files
authored
[WebPubSub]Move StringEnumConverter to explicit type converter (Azure#33130)
Remove generic converter `StringEnumConverter` and `CamelCasePropertyNamesContractResolver` in global settings to avoid breaking others. Change to explicit type converter for the enum types that read by customer. Fix issue: Azure/azure-functions-durable-extension#2338
1 parent 3a52445 commit 47257f8

File tree

4 files changed

+110
-41
lines changed

4 files changed

+110
-41
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using Microsoft.Azure.WebPubSub.Common;
6+
using Newtonsoft.Json;
7+
using Newtonsoft.Json.Converters;
8+
9+
namespace Microsoft.Azure.WebJobs.Extensions.WebPubSub
10+
{
11+
internal class WebPubSubDataTypeJsonConverter : JsonConverter<WebPubSubDataType>
12+
{
13+
private static readonly JsonSerializer JsonSerializer = JsonSerializer.Create(new JsonSerializerSettings
14+
{
15+
Converters = new[]
16+
{
17+
new StringEnumConverter()
18+
}
19+
});
20+
21+
public override WebPubSubDataType ReadJson(JsonReader reader, Type objectType, WebPubSubDataType existingValue, bool hasExistingValue, JsonSerializer serializer)
22+
{
23+
return JsonSerializer.Deserialize<WebPubSubDataType>(reader);
24+
}
25+
26+
public override void WriteJson(JsonWriter writer, WebPubSubDataType value, JsonSerializer serializer)
27+
{
28+
JsonSerializer.Serialize(writer, value);
29+
}
30+
}
31+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using Microsoft.Azure.WebPubSub.Common;
6+
using Newtonsoft.Json;
7+
using Newtonsoft.Json.Converters;
8+
9+
namespace Microsoft.Azure.WebJobs.Extensions.WebPubSub
10+
{
11+
internal class WebPubSubEventTypeJsonConverter : JsonConverter<WebPubSubEventType>
12+
{
13+
private static readonly JsonSerializer JsonSerializer = JsonSerializer.Create(new JsonSerializerSettings
14+
{
15+
Converters = new[]
16+
{
17+
new StringEnumConverter()
18+
}
19+
});
20+
21+
public override WebPubSubEventType ReadJson(JsonReader reader, Type objectType, WebPubSubEventType existingValue, bool hasExistingValue, JsonSerializer serializer)
22+
{
23+
return JsonSerializer.Deserialize<WebPubSubEventType>(reader);
24+
}
25+
26+
public override void WriteJson(JsonWriter writer, WebPubSubEventType value, JsonSerializer serializer)
27+
{
28+
JsonSerializer.Serialize(writer, value);
29+
}
30+
}
31+
}

sdk/webpubsub/Microsoft.Azure.WebJobs.Extensions.WebPubSub/src/Config/WebPubSubConfigProvider.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@
1515
using Microsoft.Extensions.Logging;
1616
using Microsoft.Extensions.Options;
1717
using Newtonsoft.Json;
18-
using Newtonsoft.Json.Converters;
1918
using Newtonsoft.Json.Linq;
20-
using Newtonsoft.Json.Serialization;
2119

2220
namespace Microsoft.Azure.WebJobs.Extensions.WebPubSub
2321
{
@@ -158,11 +156,11 @@ internal static void RegisterJsonConverter()
158156
{
159157
Converters = new List<JsonConverter>
160158
{
161-
new StringEnumConverter(),
162159
new BinaryDataJsonConverter(),
163160
new ConnectionStatesNewtonsoftConverter(),
161+
new WebPubSubDataTypeJsonConverter(),
162+
new WebPubSubEventTypeJsonConverter(),
164163
},
165-
ContractResolver = new CamelCasePropertyNamesContractResolver()
166164
};
167165
}
168166

sdk/webpubsub/Microsoft.Azure.WebJobs.Extensions.WebPubSub/tests/JObjectTests.cs

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,16 @@ public async Task ParseConnectResponseWithValidStates()
183183
Assert.AreEqual("aaa", message.UserId);
184184
}
185185

186-
[TestCase]
187-
public async Task ParseMessageResponse()
186+
[TestCase(@"""binary""", Constants.ContentTypes.BinaryContentType)]
187+
[TestCase("0", Constants.ContentTypes.BinaryContentType)]
188+
[TestCase(@"""Json""", Constants.ContentTypes.JsonContentType)]
189+
[TestCase("1", Constants.ContentTypes.JsonContentType)]
190+
[TestCase(@"""text""", Constants.ContentTypes.PlainTextContentType)]
191+
[TestCase("2", Constants.ContentTypes.PlainTextContentType)]
192+
public async Task ParseMessageResponse(string dataType, string expectContentType)
188193
{
189-
var test = @"{""data"":""test"", ""dataType"":""text""}";
194+
var test = @"{""data"":""test"", ""dataType"":{0}}";
195+
test = test.Replace("{0}", dataType);
190196

191197
var result = BuildResponse(test, RequestType.User);
192198

@@ -195,7 +201,7 @@ public async Task ParseMessageResponse()
195201

196202
var message = await result.Content.ReadAsStringAsync();
197203
Assert.AreEqual("test", message);
198-
Assert.AreEqual(Constants.ContentTypes.PlainTextContentType, result.Content.Headers.ContentType.MediaType);
204+
Assert.AreEqual(expectContentType, result.Content.Headers.ContentType.MediaType);
199205
}
200206

201207
[TestCase]
@@ -222,14 +228,16 @@ public void ParseMessageResponse_InvalidEnumReturnServerError()
222228
Assert.AreEqual(HttpStatusCode.InternalServerError, result.StatusCode);
223229
}
224230

225-
[TestCase]
226-
public async Task ParseConnectResponse_ContentMatches()
231+
[TestCase(@"""Success""")]
232+
[TestCase("0")]
233+
public async Task ParseConnectResponse_ContentMatches(string code)
227234
{
228-
var test = @"{""test"":""test"",""errorMessage"":""not valid user.""}";
235+
var test = @"{""code"":{0},""test"":""test"",""errorMessage"":""not valid user.""}";
236+
test = test.Replace("{0}", code);
229237
var expected = JObject.FromObject(JsonConvert.DeserializeObject<ConnectEventResponse>(test));
230238

231-
// Will be formated to controlled class.
232-
Assert.AreEqual("Success", expected["code"].Value<string>());
239+
// Safe to be serialize to enum 0 which is not read by customer.
240+
Assert.AreEqual(0, expected["code"].Value<int>());
233241

234242
var result = BuildResponse(test, RequestType.Connect);
235243
var content = await result.Content.ReadAsStringAsync();
@@ -249,9 +257,10 @@ public void TestWebPubSubContext_ConnectedEvent()
249257

250258
Assert.NotNull(serialize["request"]);
251259
Assert.NotNull(serialize["response"]);
252-
Assert.AreEqual("", serialize["errorMessage"].ToString());
253-
Assert.AreEqual("False", serialize["hasError"].ToString());
254-
Assert.AreEqual("False", serialize["isPreflight"].ToString());
260+
Assert.AreEqual(null, serialize["errorMessage"].Value<string>());
261+
Assert.AreEqual(false, serialize["hasError"].Value<bool>());
262+
Assert.AreEqual(false, serialize["isPreflight"].Value<bool>());
263+
Assert.AreEqual("System", serialize["request"]["connectionContext"]["eventType"].Value<string>());
255264
}
256265

257266
[TestCase]
@@ -273,9 +282,9 @@ public void TestWebPubSubContext_ConnectEvent()
273282
Assert.NotNull(request);
274283
Assert.AreEqual(subprotocol, request["subprotocols"].ToObject<string[]>());
275284
Assert.NotNull(serialize["response"]);
276-
Assert.AreEqual("", serialize["errorMessage"].ToString());
277-
Assert.AreEqual("False", serialize["hasError"].ToString());
278-
Assert.AreEqual("False", serialize["isPreflight"].ToString());
285+
Assert.AreEqual(null, serialize["errorMessage"].Value<string>());
286+
Assert.AreEqual(false, serialize["hasError"].Value<bool>());
287+
Assert.AreEqual(false, serialize["isPreflight"].Value<bool>());
279288
}
280289

281290
[TestCase]
@@ -288,11 +297,11 @@ public void TestWebPubSubContext_UserEvent()
288297
var request = serialize["request"];
289298

290299
Assert.NotNull(request);
291-
Assert.AreEqual("test", request["data"].ToString());
300+
Assert.AreEqual("test", request["data"].Value<string>());
292301
Assert.NotNull(serialize["response"]);
293-
Assert.AreEqual("", serialize["errorMessage"].ToString());
294-
Assert.AreEqual("False", serialize["hasError"].ToString());
295-
Assert.AreEqual("False", serialize["isPreflight"].ToString());
302+
Assert.AreEqual(null, serialize["errorMessage"].Value<string>());
303+
Assert.AreEqual(false, serialize["hasError"].Value<bool>());
304+
Assert.AreEqual(false, serialize["isPreflight"].Value<bool>());
296305
}
297306

298307
[TestCase]
@@ -304,11 +313,11 @@ public void TestWebPubSubContext_DisconnectedEvent()
304313
var request = serialize["request"];
305314

306315
Assert.NotNull(request);
307-
Assert.AreEqual("dropped", request["reason"].ToString());
316+
Assert.AreEqual("dropped", request["reason"].Value<string>());
308317
Assert.NotNull(serialize["response"]);
309-
Assert.AreEqual("", serialize["errorMessage"].ToString());
310-
Assert.AreEqual("False", serialize["hasError"].ToString());
311-
Assert.AreEqual("False", serialize["isPreflight"].ToString());
318+
Assert.AreEqual(null, serialize["errorMessage"].Value<string>());
319+
Assert.AreEqual(false, serialize["hasError"].Value<bool>());
320+
Assert.AreEqual(false, serialize["isPreflight"].Value<bool>());
312321
}
313322

314323
[TestCase]
@@ -321,11 +330,11 @@ public void TestWebPubSubContext_ErrorResponse()
321330

322331
Assert.Null(serialize["request"]);
323332
Assert.NotNull(response);
324-
Assert.AreEqual("400", response["status"].ToString());
325-
Assert.AreEqual("Invalid Request", response["body"].ToString());
326-
Assert.AreEqual("Invalid Request", serialize["errorMessage"].ToString());
327-
Assert.AreEqual("True", serialize["hasError"].ToString());
328-
Assert.AreEqual("False", serialize["isPreflight"].ToString());
333+
Assert.AreEqual(400, response["status"].Value<int>());
334+
Assert.AreEqual("Invalid Request", response["body"].Value<string>());
335+
Assert.AreEqual("Invalid Request", serialize["errorMessage"].Value<string>());
336+
Assert.AreEqual(true, serialize["hasError"].Value<bool>());
337+
Assert.AreEqual(false, serialize["isPreflight"].Value<bool>());
329338
}
330339

331340
[TestCase]
@@ -338,9 +347,9 @@ public void TestWebPubSubConnectionJsonSerialize()
338347

339348
var json = JObject.FromObject(connection);
340349

341-
Assert.AreEqual(baseUrl, json["baseUrl"].ToString());
342-
Assert.AreEqual(accessToken, json["accessToken"].ToString());
343-
Assert.AreEqual(url, json["url"].ToString());
350+
Assert.AreEqual(baseUrl, json["baseUrl"].Value<Uri>().ToString());
351+
Assert.AreEqual(accessToken, json["accessToken"].Value<string>());
352+
Assert.AreEqual(url, json["url"].Value<Uri>().ToString());
344353
}
345354

346355
[TestCase]
@@ -371,11 +380,11 @@ public void TestWebPubSubContext_UserEventStates_Legacy()
371380
var request = jObj["request"];
372381

373382
Assert.NotNull(request);
374-
Assert.AreEqual("test", request["data"].ToString());
383+
Assert.AreEqual("test", request["data"].Value<string>());
375384
Assert.NotNull(jObj["response"]);
376-
Assert.AreEqual("", jObj["errorMessage"].ToString());
377-
Assert.AreEqual("False", jObj["hasError"].ToString());
378-
Assert.AreEqual("False", jObj["isPreflight"].ToString());
385+
Assert.AreEqual(null, jObj["errorMessage"].Value<string>());
386+
Assert.AreEqual(false, jObj["hasError"].Value<bool>());
387+
Assert.AreEqual(false, jObj["isPreflight"].Value<bool>());
379388

380389
var context1 = request["connectionContext"];
381390
Assert.NotNull(context1);
@@ -413,8 +422,8 @@ public void TestWebPubSubContext_UserEventStates()
413422
Assert.AreEqual("test", request["data"].Value<string>());
414423
Assert.NotNull(jObj["response"]);
415424
Assert.AreEqual(null, jObj["errorMessage"].Value<string>());
416-
Assert.AreEqual("False", jObj["hasError"].Value<string>());
417-
Assert.AreEqual("False", jObj["isPreflight"].Value<string>());
425+
Assert.AreEqual(false, jObj["hasError"].Value<bool>());
426+
Assert.AreEqual(false, jObj["isPreflight"].Value<bool>());
418427
var context1 = request["connectionContext"];
419428
Assert.NotNull(context1);
420429
var states1 = context1["states"];

0 commit comments

Comments
 (0)