Skip to content

Commit 05c5848

Browse files
dmealingclaude
andcommitted
fix(csharp): address loader-unification review findings (sync HttpClient, abs-URI guard, FromDirectory state-on-failure, registry overloads)
- UriSource.Read(): replace sync-over-async (GetAwaiter().GetResult()) with synchronous HttpClient.Send + ReadAsStream to eliminate deadlock risk. - UriSource ctor: reject relative URIs at construction with ArgumentException. - MetaDataLoader.FromDirectory: on directory-read failure, synthesize the failure LoadResult directly with state == "error" instead of calling Load([]) (which left _state == "loaded" because no errors were collected before the synthetic load completed). LoadResult gains a State field; loader exposes internal SetState for static-factory pre-Load shortcuts. - Add registry-aware overloads FromUris(uris, registry) and FromString(content, format, registry) for symmetry with FromDirectory. - Tests: assert UriSource rejects relative URIs; assert FromDirectory_read_failure surfaces state == "error". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent efaba20 commit 05c5848

4 files changed

Lines changed: 52 additions & 12 deletions

File tree

server/csharp/MetaObjects.Conformance.Tests/LoaderTests.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,6 @@ public void FromDirectory_read_failure_collects_error_on_synthetic_root()
4848
var result = MetaDataLoader.FromDirectory("/nonexistent/path/that/does/not/exist");
4949
Assert.NotEmpty(result.Errors);
5050
Assert.NotNull(result.Root);
51+
Assert.Equal("error", result.State);
5152
}
5253
}

server/csharp/MetaObjects.Conformance.Tests/UriSourceTests.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ public void FileScheme_ReadsLocalFile()
2424
}
2525
}
2626

27+
[Fact]
28+
public void RelativeUri_ThrowsArgumentException()
29+
{
30+
Assert.Throws<ArgumentException>(() => new UriSource(new Uri("foo/bar.json", UriKind.Relative)));
31+
}
32+
2733
[Fact]
2834
public void ExplicitFormat_OverridesExtensionInference()
2935
{

server/csharp/MetaObjects/Loader/MetaDataLoader.cs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ namespace MetaObjects.Loader;
2323
public sealed record LoadResult(
2424
MetaRoot Root,
2525
IReadOnlyList<string> Warnings,
26-
IReadOnlyList<MetaError> Errors);
26+
IReadOnlyList<MetaError> Errors,
27+
string State = "loaded");
2728

2829
/// <summary>
2930
/// Core metadata loader pipeline. Consumes a list of
@@ -79,32 +80,45 @@ public static LoadResult FromDirectory(string directory, DirectorySource.Options
7980
public static LoadResult FromDirectory(string directory, TypeRegistry registry, DirectorySource.Options? opts = null)
8081
{
8182
var src = new DirectorySource(directory, opts);
82-
var loader = new MetaDataLoader(registry);
8383
List<IMetaDataSource> sources;
8484
try
8585
{
8686
sources = src.Expand().Cast<IMetaDataSource>().ToList();
8787
}
8888
catch (Exception ex)
8989
{
90-
// Directory-read failure: surface as a collected error on an empty load.
91-
var empty = loader.Load(Array.Empty<IMetaDataSource>());
90+
// Directory-read failure: synthesize an error result directly so
91+
// state reflects "error" (calling Load([]) would set state to
92+
// "loaded" because no errors had been collected yet at that point).
93+
var loader = new MetaDataLoader(registry);
94+
loader.SetState("error");
95+
var root = MakeSyntheticRoot();
96+
if (loader.Freeze)
97+
{
98+
root.Freeze();
99+
}
92100
var errors = new List<MetaError>
93101
{
94102
new($"Failed to read directory \"{directory}\": {ex.Message}", ErrorCode.ERR_UNKNOWN),
95103
};
96-
errors.AddRange(empty.Errors);
97-
return new LoadResult(empty.Root, empty.Warnings, errors.AsReadOnly());
104+
return new LoadResult(root, Array.Empty<string>(), errors.AsReadOnly(), "error");
98105
}
99-
return loader.Load(sources);
106+
return new MetaDataLoader(registry).Load(sources);
100107
}
101108

102109
/// <summary>
103110
/// Convenience: wrap each URI in a <see cref="UriSource"/> and load in order.
104111
/// </summary>
105112
public static LoadResult FromUris(IReadOnlyList<Uri> uris)
113+
=> FromUris(uris, DefaultRegistry());
114+
115+
/// <summary>
116+
/// Registry-aware overload: wrap each URI in a <see cref="UriSource"/> and
117+
/// load using the supplied <paramref name="registry"/>.
118+
/// </summary>
119+
public static LoadResult FromUris(IReadOnlyList<Uri> uris, TypeRegistry registry)
106120
{
107-
var loader = new MetaDataLoader();
121+
var loader = new MetaDataLoader(registry);
108122
var sources = uris.Select(u => (IMetaDataSource)new UriSource(u)).ToList();
109123
return loader.Load(sources);
110124
}
@@ -113,8 +127,15 @@ public static LoadResult FromUris(IReadOnlyList<Uri> uris)
113127
/// Convenience: load a single in-memory string of the given format.
114128
/// </summary>
115129
public static LoadResult FromString(string content, MetaDataFormat format)
130+
=> FromString(content, format, DefaultRegistry());
131+
132+
/// <summary>
133+
/// Registry-aware overload: load a single in-memory string of the given
134+
/// format using the supplied <paramref name="registry"/>.
135+
/// </summary>
136+
public static LoadResult FromString(string content, MetaDataFormat format, TypeRegistry registry)
116137
{
117-
var loader = new MetaDataLoader();
138+
var loader = new MetaDataLoader(registry);
118139
return loader.Load(new IMetaDataSource[] { new InMemoryStringSource(content, format: format) });
119140
}
120141

@@ -312,9 +333,16 @@ public LoadResult Load(IReadOnlyList<IMetaDataSource> sources)
312333
}
313334

314335
_root = root;
315-
return new LoadResult(root, warnings.AsReadOnly(), errors.AsReadOnly());
336+
return new LoadResult(root, warnings.AsReadOnly(), errors.AsReadOnly(), _state);
316337
}
317338

339+
/// <summary>
340+
/// Internal state setter — used by static factories (e.g. FromDirectory)
341+
/// that need to short-circuit the pipeline on pre-Load failures and still
342+
/// report state == "error".
343+
/// </summary>
344+
internal void SetState(string state) => _state = state;
345+
318346
// -------------------------------------------------------------------------
319347
// Helpers
320348
// -------------------------------------------------------------------------

server/csharp/MetaObjects/Loader/UriSource.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public sealed class UriSource : IMetaDataSource
3030
public UriSource(Uri uri, MetaDataFormat? format = null)
3131
{
3232
Uri = uri ?? throw new ArgumentNullException(nameof(uri));
33+
if (!uri.IsAbsoluteUri)
34+
throw new ArgumentException($"UriSource requires an absolute URI; got relative '{uri}'.", nameof(uri));
3335
Id = uri.ToString();
3436
Format = format ?? MetaDataFormats.InferFromExtension(uri.AbsolutePath);
3537
}
@@ -43,9 +45,12 @@ public string Read()
4345

4446
if (Uri.Scheme == "http" || Uri.Scheme == "https")
4547
{
46-
using HttpResponseMessage resp = _http.GetAsync(Uri).GetAwaiter().GetResult();
48+
using var req = new HttpRequestMessage(HttpMethod.Get, Uri);
49+
using var resp = _http.Send(req);
4750
resp.EnsureSuccessStatusCode();
48-
return resp.Content.ReadAsStringAsync().GetAwaiter().GetResult();
51+
using var stream = resp.Content.ReadAsStream();
52+
using var reader = new System.IO.StreamReader(stream);
53+
return reader.ReadToEnd();
4954
}
5055

5156
throw new NotSupportedException($"Unsupported URI scheme '{Uri.Scheme}' on {Uri}");

0 commit comments

Comments
 (0)