Skip to content

Commit fb91341

Browse files
Revert Copilot autofix and make DeserializeFromXml non-generic
Reverts c29ce27 (Copilot autofix broke the build with CS0030) and applies the proper fix: drop the false generality entirely. The method only ever deserialises OoklaServerList, so the generic parameter, the typeof(T) runtime check, and the (T)(object) cast were all unnecessary ceremony. Returning OoklaServerList? directly eliminates the CodeQL 'useless upcast' finding cleanly. Co-authored-by: Frank Ray <FrankRay78@users.noreply.github.com>
1 parent c29ce27 commit fb91341

3 files changed

Lines changed: 11 additions & 17 deletions

File tree

src/NetPace.Core.Tests/Clients/Ookla/Extensions/XmlExtensionsTests.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void Deserialize_RepresentativeOoklaResponse_PopulatesAllRequiredAttribut
2626
</settings>
2727
""";
2828

29-
var result = Xml.DeserializeFromXml<OoklaServerList>();
29+
var result = Xml.DeserializeFromXml();
3030

3131
result.ShouldNotBeNull();
3232
result.Servers.ShouldNotBeNull();
@@ -55,7 +55,7 @@ public void Deserialize_OptionalAttributesPresent_PopulatesCountryAndHost()
5555
</settings>
5656
""";
5757

58-
var result = Xml.DeserializeFromXml<OoklaServerList>();
58+
var result = Xml.DeserializeFromXml();
5959

6060
var server = result!.Servers!.Single();
6161
server.Country.ShouldBe("United Kingdom");
@@ -76,7 +76,7 @@ public void Deserialize_OptionalAttributesAbsent_LeavesCountryAndHostNull()
7676
</settings>
7777
""";
7878

79-
var result = Xml.DeserializeFromXml<OoklaServerList>();
79+
var result = Xml.DeserializeFromXml();
8080

8181
var server = result!.Servers!.Single();
8282
server.Country.ShouldBeNull();
@@ -102,7 +102,7 @@ public void Deserialize_NumericAttributes_UsesInvariantCultureUnderCommaDecimalL
102102
{
103103
CultureInfo.CurrentCulture = new CultureInfo("de-DE");
104104

105-
var result = Xml.DeserializeFromXml<OoklaServerList>();
105+
var result = Xml.DeserializeFromXml();
106106

107107
var server = result!.Servers!.Single();
108108
server.Latitude.ShouldBe(51.5074);
@@ -126,7 +126,7 @@ public void Deserialize_EmptyServersElement_ReturnsEmptyOrNullCollection()
126126
</settings>
127127
""";
128128

129-
var result = Xml.DeserializeFromXml<OoklaServerList>();
129+
var result = Xml.DeserializeFromXml();
130130

131131
result.ShouldNotBeNull();
132132
(result.Servers is null || result.Servers.Length == 0).ShouldBeTrue();
@@ -139,7 +139,7 @@ public void Deserialize_MalformedXml_ThrowsXmlException()
139139

140140
const string Xml = "<settings><servers><server id=\"1\" /></servers"; // unclosed
141141

142-
var thrown = Should.Throw<Exception>(() => Xml.DeserializeFromXml<OoklaServerList>());
142+
var thrown = Should.Throw<Exception>(() => Xml.DeserializeFromXml());
143143

144144
// Accept either an XmlException directly or an InvalidOperationException wrapping one
145145
var isXmlOrWrapped = thrown is XmlException

src/NetPace.Core/Clients/Ookla/Extensions/XmlExtensions.cs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,8 @@ namespace NetPace.Core.Clients.Ookla.Extensions;
66

77
internal static class XmlExtensions
88
{
9-
internal static T? DeserializeFromXml<T>(this string data)
9+
internal static OoklaServerList? DeserializeFromXml(this string data)
1010
{
11-
if (typeof(T) != typeof(OoklaServerList))
12-
{
13-
throw new NotSupportedException(
14-
$"DeserializeFromXml only supports {nameof(OoklaServerList)}; requested {typeof(T).FullName}.");
15-
}
16-
1711
var doc = XDocument.Parse(data);
1812
var settings = doc.Element("settings")
1913
?? throw new XmlException("Expected <settings> root element.");
@@ -23,14 +17,14 @@ internal static class XmlExtensions
2317

2418
if (serversElement is null)
2519
{
26-
return (T)serverList;
20+
return serverList;
2721
}
2822

2923
var serverElements = serversElement.Elements("server").ToArray();
3024
if (serverElements.Length == 0)
3125
{
3226
serverList.Servers = Array.Empty<OoklaServer>();
33-
return (T)serverList;
27+
return serverList;
3428
}
3529

3630
var servers = new OoklaServer[serverElements.Length];
@@ -40,7 +34,7 @@ internal static class XmlExtensions
4034
}
4135

4236
serverList.Servers = servers;
43-
return (T)serverList;
37+
return serverList;
4438
}
4539

4640
private static OoklaServer ParseServer(XElement element)

src/NetPace.Core/Clients/Ookla/OoklaSpeedtest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public OoklaSpeedtest(OoklaSpeedtestSettings? speedtestSettings = null, HttpClie
3535
public async Task<IServer[]> GetServersAsync(CancellationToken cancellationToken = default)
3636
{
3737
var serversXml = await httpClient.GetStringAsync(settings.ServerDiscovery.ServersUrl, cancellationToken).ConfigureAwait(false);
38-
var servers = serversXml.DeserializeFromXml<OoklaServerList>()?.Servers ?? Array.Empty<OoklaServer>();
38+
var servers = serversXml.DeserializeFromXml()?.Servers ?? Array.Empty<OoklaServer>();
3939
return servers.Where(s =>
4040
!string.IsNullOrWhiteSpace(s.Location) &&
4141
!string.IsNullOrWhiteSpace(s.Sponsor) &&

0 commit comments

Comments
 (0)