Skip to content

Commit 635f162

Browse files
russcamrusscam
authored
Use correct IJsonFormatter<T> in attributes (#4383)
This commit fixes a bug where VerifyRepositoryResponse was attributed with the wrong formatter type in JsonFormatterAttribute applied at the type level. Utf8Json's formatters are strongly typed for speed at the cost of less flexibility. Fix other cases where the incorrect formatter type is used, and introduce a convention test to assert that types and their properties attributed with JsonFormatterAttribute use the correct formatter type. Fixes #4382
1 parent 38ec44d commit 635f162

File tree

8 files changed

+195
-97
lines changed

8 files changed

+195
-97
lines changed

‎src/Nest/Ingest/Processors/ScriptProcessor.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public interface IScriptProcessor : IProcessor
2727
/// Parameters for the script
2828
/// </summary>
2929
[DataMember(Name ="params")]
30-
[JsonFormatter(typeof(VerbatimDictionaryInterfaceKeysFormatter<string, object>))]
30+
[JsonFormatter(typeof(VerbatimDictionaryKeysFormatter<string, object>))]
3131
Dictionary<string, object> Params { get; set; }
3232

3333
/// <summary>

‎src/Nest/Modules/SnapshotAndRestore/Repositories/VerifyRepository/VerifyRepositoryResponse.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public class VerifyRepositoryResponse : ResponseBase
1212
/// A dictionary of nodeId => nodeinfo of nodes that verified the repository
1313
/// </summary>
1414
[DataMember(Name = "nodes")]
15-
[JsonFormatter(typeof(VerbatimDictionaryInterfaceKeysFormatter<string, CompactNodeInfo>))]
15+
[JsonFormatter(typeof(VerbatimInterfaceReadOnlyDictionaryKeysFormatter<string, CompactNodeInfo>))]
1616
public IReadOnlyDictionary<string, CompactNodeInfo> Nodes { get; internal set; } = EmptyReadOnly<string, CompactNodeInfo>.Dictionary;
1717
}
1818
}

‎src/Nest/XPack/MachineLearning/UpdateJob/UpdateJobRequest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public partial interface IUpdateJobRequest
3030
/// Contains custom meta data about the job.
3131
/// </summary>
3232
[DataMember(Name ="custom_settings")]
33-
[JsonFormatter(typeof(VerbatimDictionaryInterfaceKeysFormatter<string, object>))]
33+
[JsonFormatter(typeof(VerbatimDictionaryKeysFormatter<string, object>))]
3434
Dictionary<string, object> CustomSettings { get; set; }
3535

3636
/// <summary>

‎src/Nest/XPack/Watcher/Action/ActionBase.cs‎

Lines changed: 104 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.Collections.Generic;
1+
using System;
2+
using System.Collections.Generic;
23
using System.Linq;
34
using System.Runtime.Serialization;
45
using Elasticsearch.Net;
@@ -120,8 +121,108 @@ private void AddAction(ActionBase agg)
120121
}
121122
}
122123

124+
internal class ActionsInterfaceFormatter : IJsonFormatter<IActions>
125+
{
126+
private static readonly ActionsFormatter ActionsFormatter = new ActionsFormatter();
127+
128+
public void Serialize(ref JsonWriter writer, IActions value, IJsonFormatterResolver formatterResolver)
129+
{
130+
writer.WriteBeginObject();
131+
if (value != null)
132+
{
133+
var count = 0;
134+
foreach (var kvp in value.Where(kv => kv.Value != null))
135+
{
136+
if (count > 0)
137+
writer.WriteValueSeparator();
138+
139+
var action = kvp.Value;
140+
writer.WritePropertyName(kvp.Key);
141+
writer.WriteBeginObject();
142+
if (action.ThrottlePeriod != null)
143+
{
144+
writer.WritePropertyName("throttle_period");
145+
var timeFormatter = formatterResolver.GetFormatter<Time>();
146+
timeFormatter.Serialize(ref writer, action.ThrottlePeriod, formatterResolver);
147+
writer.WriteValueSeparator();
148+
}
149+
150+
if (!string.IsNullOrEmpty(action.Foreach))
151+
{
152+
writer.WritePropertyName("foreach");
153+
writer.WriteString(action.Foreach);
154+
writer.WriteValueSeparator();
155+
}
156+
if (action.MaxIterations.HasValue)
157+
{
158+
writer.WritePropertyName("max_iterations");
159+
writer.WriteInt32(action.MaxIterations.Value);
160+
writer.WriteValueSeparator();
161+
}
162+
163+
if (action.Transform != null)
164+
{
165+
writer.WritePropertyName("transform");
166+
formatterResolver.GetFormatter<TransformContainer>().Serialize(ref writer, action.Transform, formatterResolver);
167+
writer.WriteValueSeparator();
168+
}
169+
170+
if (action.Condition != null)
171+
{
172+
writer.WritePropertyName("condition");
173+
formatterResolver.GetFormatter<ConditionContainer>().Serialize(ref writer, action.Condition, formatterResolver);
174+
writer.WriteValueSeparator();
175+
}
176+
177+
writer.WritePropertyName(kvp.Value.ActionType.GetStringValue());
178+
179+
switch (action.ActionType)
180+
{
181+
case ActionType.Email:
182+
Serialize<IEmailAction>(ref writer, action, formatterResolver);
183+
break;
184+
case ActionType.Webhook:
185+
Serialize<IWebhookAction>(ref writer, action, formatterResolver);
186+
break;
187+
case ActionType.Index:
188+
Serialize<IIndexAction>(ref writer, action, formatterResolver);
189+
break;
190+
case ActionType.Logging:
191+
Serialize<ILoggingAction>(ref writer, action, formatterResolver);
192+
break;
193+
case ActionType.Slack:
194+
Serialize<ISlackAction>(ref writer, action, formatterResolver);
195+
break;
196+
case ActionType.PagerDuty:
197+
Serialize<IPagerDutyAction>(ref writer, action, formatterResolver);
198+
break;
199+
default:
200+
var actionFormatter = formatterResolver.GetFormatter<IAction>();
201+
actionFormatter.Serialize(ref writer, action, formatterResolver);
202+
break;
203+
}
204+
205+
writer.WriteEndObject();
206+
count++;
207+
}
208+
}
209+
writer.WriteEndObject();
210+
}
211+
212+
private static void Serialize<TAction>(ref JsonWriter writer, IAction value, IJsonFormatterResolver formatterResolver)
213+
where TAction : class, IAction
214+
{
215+
var formatter = formatterResolver.GetFormatter<TAction>();
216+
formatter.Serialize(ref writer, value as TAction, formatterResolver);
217+
}
218+
public IActions Deserialize(ref JsonReader reader, IJsonFormatterResolver formatterResolver) =>
219+
ActionsFormatter.Deserialize(ref reader, formatterResolver);
220+
}
221+
123222
internal class ActionsFormatter : IJsonFormatter<Actions>
124223
{
224+
private static readonly ActionsInterfaceFormatter ActionsInterfaceFormatter = new ActionsInterfaceFormatter();
225+
125226
private static readonly AutomataDictionary Fields = new AutomataDictionary
126227
{
127228
{ "throttle_period", 0 },
@@ -225,95 +326,7 @@ public Actions Deserialize(ref JsonReader reader, IJsonFormatterResolver formatt
225326
return new Actions(dictionary);
226327
}
227328

228-
public void Serialize(ref JsonWriter writer, Actions value, IJsonFormatterResolver formatterResolver)
229-
{
230-
writer.WriteBeginObject();
231-
if (value != null)
232-
{
233-
var count = 0;
234-
foreach (var kvp in value.Where(kv => kv.Value != null))
235-
{
236-
if (count > 0)
237-
writer.WriteValueSeparator();
238-
239-
var action = kvp.Value;
240-
writer.WritePropertyName(kvp.Key);
241-
writer.WriteBeginObject();
242-
if (action.ThrottlePeriod != null)
243-
{
244-
writer.WritePropertyName("throttle_period");
245-
var timeFormatter = formatterResolver.GetFormatter<Time>();
246-
timeFormatter.Serialize(ref writer, action.ThrottlePeriod, formatterResolver);
247-
writer.WriteValueSeparator();
248-
}
249-
250-
if (!string.IsNullOrEmpty(action.Foreach))
251-
{
252-
writer.WritePropertyName("foreach");
253-
writer.WriteString(action.Foreach);
254-
writer.WriteValueSeparator();
255-
}
256-
if (action.MaxIterations.HasValue)
257-
{
258-
writer.WritePropertyName("max_iterations");
259-
writer.WriteInt32(action.MaxIterations.Value);
260-
writer.WriteValueSeparator();
261-
}
262-
263-
if (action.Transform != null)
264-
{
265-
writer.WritePropertyName("transform");
266-
formatterResolver.GetFormatter<TransformContainer>().Serialize(ref writer, action.Transform, formatterResolver);
267-
writer.WriteValueSeparator();
268-
}
269-
270-
if (action.Condition != null)
271-
{
272-
writer.WritePropertyName("condition");
273-
formatterResolver.GetFormatter<ConditionContainer>().Serialize(ref writer, action.Condition, formatterResolver);
274-
writer.WriteValueSeparator();
275-
}
276-
277-
writer.WritePropertyName(kvp.Value.ActionType.GetStringValue());
278-
279-
switch (action.ActionType)
280-
{
281-
case ActionType.Email:
282-
Serialize<IEmailAction>(ref writer, action, formatterResolver);
283-
break;
284-
case ActionType.Webhook:
285-
Serialize<IWebhookAction>(ref writer, action, formatterResolver);
286-
break;
287-
case ActionType.Index:
288-
Serialize<IIndexAction>(ref writer, action, formatterResolver);
289-
break;
290-
case ActionType.Logging:
291-
Serialize<ILoggingAction>(ref writer, action, formatterResolver);
292-
break;
293-
case ActionType.Slack:
294-
Serialize<ISlackAction>(ref writer, action, formatterResolver);
295-
break;
296-
case ActionType.PagerDuty:
297-
Serialize<IPagerDutyAction>(ref writer, action, formatterResolver);
298-
break;
299-
default:
300-
var actionFormatter = formatterResolver.GetFormatter<IAction>();
301-
actionFormatter.Serialize(ref writer, action, formatterResolver);
302-
break;
303-
}
304-
305-
writer.WriteEndObject();
306-
count++;
307-
}
308-
}
309-
writer.WriteEndObject();
310-
}
311-
312-
private static void Serialize<TAction>(ref JsonWriter writer, IAction value, IJsonFormatterResolver formatterResolver)
313-
where TAction : class, IAction
314-
{
315-
var formatter = formatterResolver.GetFormatter<TAction>();
316-
formatter.Serialize(ref writer, value as TAction, formatterResolver);
317-
}
329+
public void Serialize(ref JsonWriter writer, Actions value, IJsonFormatterResolver formatterResolver) =>
330+
ActionsInterfaceFormatter.Serialize(ref writer, value, formatterResolver);
318331
}
319332
}

‎src/Nest/XPack/Watcher/Action/Actions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace Nest
77
{
8-
[JsonFormatter(typeof(ActionsFormatter))]
8+
[JsonFormatter(typeof(ActionsInterfaceFormatter))]
99
public interface IActions : IIsADictionary<string, IAction> { }
1010

1111
[JsonFormatter(typeof(ActionsFormatter))]

‎src/Nest/XPack/Watcher/Transform/ScriptTransformBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public interface IScriptTransform : ITransform
1515
string Lang { get; set; }
1616

1717
[DataMember(Name = "params")]
18-
[JsonFormatter(typeof(VerbatimDictionaryInterfaceKeysFormatter<string, object>))]
18+
[JsonFormatter(typeof(VerbatimDictionaryKeysFormatter<string, object>))]
1919
Dictionary<string, object> Params { get; set; }
2020
}
2121

‎tests/Tests.Reproduce/GitHubIssue4382.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using System;
2+
using System.Text;
3+
using Elastic.Xunit.XunitPlumbing;
4+
using Elasticsearch.Net;
5+
using FluentAssertions;
6+
using Nest;
7+
8+
namespace Tests.Reproduce
9+
{
10+
public class GitHubIssue4382
11+
{
12+
[U]
13+
public void CanDeserializeVerifyRepositoryResponse()
14+
{
15+
var json = @"{""nodes"":{""1cWG9trDRi--6I-46lOlBw"":{""name"":""DESKTOP-5L01F6I""}}}";
16+
17+
var bytes = Encoding.UTF8.GetBytes(json);
18+
var pool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
19+
var connectionSettings = new ConnectionSettings(pool, new InMemoryConnection(bytes)).DefaultIndex("default_index");
20+
var client = new ElasticClient(connectionSettings);
21+
22+
var response = client.Snapshot.VerifyRepository("repository");
23+
response.Nodes.Should().NotBeNullOrEmpty();
24+
response.Nodes["1cWG9trDRi--6I-46lOlBw"].Name.Should().Be("DESKTOP-5L01F6I");
25+
}
26+
}
27+
}

‎tests/Tests/CodeStandards/Serialization/Formatters.doc.cs‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Nest;
66
using Tests.Framework;
77
using System.Collections.Generic;
8+
using System.Reflection;
89
using Elastic.Xunit.XunitPlumbing;
910
using Elasticsearch.Net.Utf8Json;
1011

@@ -15,7 +16,6 @@ public class Formatters
1516
[U]
1617
public void CustomFormattersShouldBeInternal()
1718
{
18-
// TODO: Make internals visible to IL generated modules
1919
var formatters = typeof(IElasticClient).Assembly.GetTypes()
2020
.Concat(typeof(IElasticLowLevelClient).Assembly.GetTypes())
2121
.Where(t => t.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IJsonFormatter<>)))
@@ -28,5 +28,63 @@ public void CustomFormattersShouldBeInternal()
2828
}
2929
visible.Should().BeEmpty();
3030
}
31+
32+
[U]
33+
public void TypesAndPropertiesShouldBeAttributedWithCorrectlyTypedJsonFormatter()
34+
{
35+
Type GetFormatterTargetType(Type t)
36+
{
37+
var attribute = t.GetCustomAttribute<JsonFormatterAttribute>();
38+
39+
if (attribute == null)
40+
return null;
41+
42+
var formatterType = attribute.FormatterType;
43+
44+
if (formatterType.IsGenericType && !formatterType.IsConstructedGenericType)
45+
return null;
46+
47+
return formatterType.GetInterfaces()
48+
.Where(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IJsonFormatter<>))
49+
.Select(i => i.GetGenericArguments()[0])
50+
.Single();
51+
}
52+
53+
var typesAndProperties =
54+
from t in typeof(IElasticClient).Assembly.GetTypes().Concat(typeof(IElasticLowLevelClient).Assembly.GetTypes())
55+
let p = t.GetProperties()
56+
let typeHasFormatter = t.GetCustomAttribute<JsonFormatterAttribute>(false) != null
57+
let propertiesHaveFormatter = p.Any(pp => pp.GetCustomAttribute<JsonFormatterAttribute>(false) != null)
58+
where typeHasFormatter || propertiesHaveFormatter
59+
select new { Type = t, TypeHasFormatter = typeHasFormatter, Properties = p, PropertiesHaveFormatter = propertiesHaveFormatter };
60+
61+
var invalid = new List<string>();
62+
63+
foreach (var typeAndProperties in typesAndProperties)
64+
{
65+
if (typeAndProperties.TypeHasFormatter)
66+
{
67+
var t = typeAndProperties.Type;
68+
var f = GetFormatterTargetType(t);
69+
70+
if (f != null && t != f)
71+
invalid.Add($"{t.FullName} has IJsonFormatter<{f.FullName}>");
72+
}
73+
74+
if (typeAndProperties.PropertiesHaveFormatter)
75+
{
76+
foreach (var property in typeAndProperties.Properties)
77+
{
78+
var t = property.PropertyType;
79+
var f = GetFormatterTargetType(t);
80+
81+
if (f != null && t != f)
82+
invalid.Add($"property {property.Name} on {typeAndProperties.Type.FullName} has IJsonFormatter<{f.FullName}>");
83+
}
84+
}
85+
}
86+
87+
invalid.Should().BeEmpty();
88+
}
3189
}
3290
}

0 commit comments

Comments
 (0)