Skip to content

Commit 8f086ab

Browse files
Copilotjongalloway
andcommitted
Add validation to LoadFromXml method for consistency with LoadFromStream
Co-authored-by: jongalloway <[email protected]>
1 parent f7bf82e commit 8f086ab

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

src/NLWebNet/Services/ToolDefinitionLoader.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,20 @@ public ToolDefinitions LoadFromXml(string xmlContent)
101101
});
102102

103103
var result = (ToolDefinitions?)_serializer.Deserialize(xmlReader);
104-
return result ?? new ToolDefinitions();
104+
var toolDefinitions = result ?? new ToolDefinitions();
105+
106+
// Validate the loaded definitions
107+
var validationErrors = ValidateToolDefinitions(toolDefinitions).ToList();
108+
if (validationErrors.Any())
109+
{
110+
var errorMessage = $"Tool definitions validation failed: {string.Join("; ", validationErrors)}";
111+
_logger.LogError(errorMessage);
112+
throw new InvalidOperationException(errorMessage);
113+
}
114+
115+
return toolDefinitions;
105116
}
106-
catch (Exception ex)
117+
catch (Exception ex) when (!(ex is InvalidOperationException))
107118
{
108119
_logger.LogError(ex, "Failed to deserialize tool definitions from XML content");
109120
throw new InvalidOperationException("Failed to parse tool definitions XML", ex);

tests/NLWebNet.Tests/Extensions/ConfigurationFormatSupportTests.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,30 @@ public void XmlToolDefinitions_BasicLoading_ShouldWork()
112112
Assert.IsTrue(result.Tools[0].Enabled);
113113
}
114114

115+
[TestMethod]
116+
public void XmlToolDefinitions_ValidationErrors_ShouldThrowException()
117+
{
118+
// Arrange - XML with validation errors (empty tool ID)
119+
var xml = @"<?xml version=""1.0"" encoding=""utf-8""?>
120+
<ToolDefinitions>
121+
<Tool id="""" name=""Invalid Tool"" type=""search"" enabled=""true"" priority=""1"">
122+
<Description>Tool with empty ID</Description>
123+
<Parameters>
124+
<MaxResults>10</MaxResults>
125+
<TimeoutSeconds>30</TimeoutSeconds>
126+
</Parameters>
127+
</Tool>
128+
</ToolDefinitions>";
129+
130+
var logger = Substitute.For<Microsoft.Extensions.Logging.ILogger<ToolDefinitionLoader>>();
131+
var loader = new ToolDefinitionLoader(logger);
132+
133+
// Act & Assert
134+
var exception = Assert.ThrowsException<InvalidOperationException>(() => loader.LoadFromXml(xml));
135+
Assert.IsTrue(exception.Message.Contains("Tool definitions validation failed"));
136+
Assert.IsTrue(exception.Message.Contains("Tool ID cannot be empty"));
137+
}
138+
115139
[TestMethod]
116140
public void ConfigurationExtensions_ServiceRegistration_ShouldWork()
117141
{

0 commit comments

Comments
 (0)