feat(#7): XSD validation from game directory#50
Conversation
- Create IXsdSchemaProvider interface - Implement XsdSchemaProvider for loading XSD schemas - Support finding XSD files in game directory - Validate XML files against XSD schemas - Add comprehensive unit tests Closes #7
There was a problem hiding this comment.
Pull request overview
Adds an XSD schema provider to locate/load game-provided XSDs and validate XML files against them, plus unit tests for basic behaviors.
Changes:
- Introduce
IXsdSchemaProviderandXsdSchemaProviderwith XSD discovery, loading, and XML validation APIs - Implement directory scanning for XSD files under expected game folders
- Add xUnit tests covering path discovery and missing-file cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| BannerlordModEditor.Common/Services/XsdSchemaProvider.cs | Adds schema provider interface + implementation for locating/loading XSDs and validating XML |
| BannerlordModEditor.Common.Tests/Services/XsdSchemaProviderTests.cs | Adds unit tests for XSD discovery/loading and validation error cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| private const string XsdDirectoryRelativePath = "XmlSchemas"; | ||
| private const string ModuleDataRelativePath = "Modules"; | ||
| private const string NativeModuleRelativePath = "Modules\\Native\\ModuleData"; |
There was a problem hiding this comment.
NativeModuleRelativePath uses Windows backslashes. When combined on non-Windows platforms, the embedded \ becomes part of the directory name, causing schema discovery/scanning to fail. Define the path using Path.Combine("Modules", "Native", "ModuleData") (or store segments and combine) so it’s platform-correct.
| private const string NativeModuleRelativePath = "Modules\\Native\\ModuleData"; | |
| private static readonly string NativeModuleRelativePath = Path.Combine("Modules", "Native", "ModuleData"); |
| { | ||
| Path.Combine(gameDirectory, XsdDirectoryRelativePath, $"{xmlFileName}.xsd"), | ||
| Path.Combine(gameDirectory, XsdDirectoryRelativePath, "Native", $"{xmlFileName}.xsd"), | ||
| Path.Combine(gameDirectory, NativeModuleRelativePath, $"{xmlFileName}.xsd") |
There was a problem hiding this comment.
NativeModuleRelativePath uses Windows backslashes. When combined on non-Windows platforms, the embedded \ becomes part of the directory name, causing schema discovery/scanning to fail. Define the path using Path.Combine("Modules", "Native", "ModuleData") (or store segments and combine) so it’s platform-correct.
| var searchPaths = new[] | ||
| { | ||
| Path.Combine(gameDirectory, XsdDirectoryRelativePath), | ||
| Path.Combine(gameDirectory, NativeModuleRelativePath) |
There was a problem hiding this comment.
NativeModuleRelativePath uses Windows backslashes. When combined on non-Windows platforms, the embedded \ becomes part of the directory name, causing schema discovery/scanning to fail. Define the path using Path.Combine("Modules", "Native", "ModuleData") (or store segments and combine) so it’s platform-correct.
| public XmlSchemaSet LoadSchemaSet(string xsdPath) | ||
| { | ||
| var schemaSet = new XmlSchemaSet(); | ||
|
|
||
| if (!File.Exists(xsdPath)) | ||
| return schemaSet; | ||
|
|
||
| try | ||
| { | ||
| using var stream = new FileStream(xsdPath, FileMode.Open, FileAccess.Read); | ||
| var schema = XmlSchema.Read(stream, null); | ||
| if (schema != null) | ||
| { | ||
| schemaSet.Add(schema); | ||
| } | ||
| } |
There was a problem hiding this comment.
XmlSchemaSet can resolve <xs:include>/<xs:import> via an XmlResolver (default may allow external fetches / local file access depending on runtime), which is risky if schemas are not fully trusted. Consider setting schemaSet.XmlResolver = null (or a constrained resolver) and also ensure the XmlReaderSettings.XmlResolver is null during validation to prevent external entity/resource resolution.
| try | ||
| { | ||
| xsdFiles.AddRange(Directory.GetFiles(searchPath, "*.xsd", SearchOption.AllDirectories)); | ||
| } | ||
| catch | ||
| { | ||
| } |
There was a problem hiding this comment.
Swallowing all exceptions here can silently hide permission issues, path-too-long errors, or IO failures and make schema discovery non-deterministic in production. Catch specific exceptions (e.g., UnauthorizedAccessException, IOException) and either (a) surface them via an error list/logging, or (b) at least document why ignoring is safe and expected.
| public class XsdSchemaProvider : IXsdSchemaProvider | ||
| { | ||
| private const string XsdDirectoryRelativePath = "XmlSchemas"; | ||
| private const string ModuleDataRelativePath = "Modules"; |
There was a problem hiding this comment.
ModuleDataRelativePath is declared but not used anywhere in this file. Removing it avoids dead code and reduces confusion about intended search paths.
| private const string ModuleDataRelativePath = "Modules"; |
| public bool ValidateXmlAgainstXsd(string xmlFilePath, string xsdPath, out List<string> errors) | ||
| { | ||
| errors = new List<string>(); | ||
|
|
||
| if (!File.Exists(xmlFilePath)) | ||
| { | ||
| errors.Add($"XML file not found: {xmlFilePath}"); | ||
| return false; | ||
| } | ||
|
|
||
| if (!File.Exists(xsdPath)) | ||
| { | ||
| errors.Add($"XSD schema not found: {xsdPath}"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Tests currently cover missing XML/XSD paths, but don’t verify the core behavior of validating a real XML against a real XSD (both a passing case returning true and a failing case that returns false with populated validation errors). Adding those tests would cover the main feature this method introduces.
Summary
IXsdSchemaProviderinterfaceXsdSchemaProviderfor loading XSD schemasChanges
BannerlordModEditor.Common/Services/XsdSchemaProvider.cs- ImplementationBannerlordModEditor.Common.Tests/Services/XsdSchemaProviderTests.cs- Unit testsCloses #7