fix(acl): read each objecttype version instead of the versions collection (refs #149)
The version resolve assumed `GET {objecttype}/versions` returns a bare list.
Every other collection in the Objecttypen API returns a paginated envelope, and
nothing in the repo exercises that endpoint, so the shape was a guess. Follow
the path infra/registerrecord-check.py already proves against the real API
instead: read the `versions` URLs off the objecttype and fetch each for its
status. Costs a request per version, once per gateway instance.
ACL mutation score 92.23% (baseline 91.37%).
This commit is contained in:
@@ -52,16 +52,23 @@ public sealed class ObjectenGateway(HttpClient http, ObjectenOptions options, IC
|
|||||||
?? throw new InvalidOperationException(
|
?? throw new InvalidOperationException(
|
||||||
$"No objecttype '{options.ObjecttypeName}' registered in Objecttypen — is the RegisterRecord seed applied?");
|
$"No objecttype '{options.ObjecttypeName}' registered in Objecttypen — is the RegisterRecord seed applied?");
|
||||||
|
|
||||||
var url = new Uri(match.Url);
|
|
||||||
// Write against the highest *published* version: a draft version's schema is still being
|
// Write against the highest *published* version: a draft version's schema is still being
|
||||||
// shaped, and objects written against it would be validated by a moving target.
|
// shaped, and objects written against it would be validated by a moving target. The objecttype
|
||||||
var versions = await GetAsync<IReadOnlyList<ObjecttypeVersionDto>>(
|
// carries its versions as URLs, so each is fetched for its status (the collection response
|
||||||
new Uri(url + "/versions"), options.ObjecttypenToken, crs: false, "objecttype versions", ct);
|
// gives no status) — once per gateway instance, alongside the lookup above.
|
||||||
var latest = versions.Where(v => v.Status == "published").Select(v => v.Version).DefaultIfEmpty(0).Max();
|
var latest = 0;
|
||||||
|
foreach (var versionUrl in match.Versions ?? [])
|
||||||
|
{
|
||||||
|
var version = await GetAsync<ObjecttypeVersionDto>(
|
||||||
|
new Uri(versionUrl), options.ObjecttypenToken, crs: false, "objecttype version", ct);
|
||||||
|
if (version.Status == "published" && version.Version > latest)
|
||||||
|
latest = version.Version;
|
||||||
|
}
|
||||||
|
|
||||||
if (latest == 0)
|
if (latest == 0)
|
||||||
throw new InvalidOperationException($"Objecttype '{options.ObjecttypeName}' has no published version");
|
throw new InvalidOperationException($"Objecttype '{options.ObjecttypeName}' has no published version");
|
||||||
|
|
||||||
return new Objecttype(url, latest);
|
return new Objecttype(new Uri(match.Url), latest);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>The URL of the object already holding this registration's record, or null if there is none.</summary>
|
/// <summary>The URL of the object already holding this registration's record, or null if there is none.</summary>
|
||||||
@@ -121,7 +128,8 @@ public sealed class ObjectenGateway(HttpClient http, ObjectenOptions options, IC
|
|||||||
|
|
||||||
private sealed record ObjecttypeDto(
|
private sealed record ObjecttypeDto(
|
||||||
[property: JsonPropertyName("url")] string Url,
|
[property: JsonPropertyName("url")] string Url,
|
||||||
[property: JsonPropertyName("name")] string? Name);
|
[property: JsonPropertyName("name")] string? Name,
|
||||||
|
[property: JsonPropertyName("versions")] IReadOnlyList<string>? Versions);
|
||||||
|
|
||||||
private sealed record ObjecttypeVersionDto(
|
private sealed record ObjecttypeVersionDto(
|
||||||
[property: JsonPropertyName("version")] int Version,
|
[property: JsonPropertyName("version")] int Version,
|
||||||
|
|||||||
@@ -51,24 +51,27 @@ public class ObjectenGatewayTests
|
|||||||
},
|
},
|
||||||
new FixedClock(new DateOnly(2026, 6, 4)));
|
new FixedClock(new DateOnly(2026, 6, 4)));
|
||||||
|
|
||||||
// A stack that answers the three reads every write is preceded by: the objecttype list (matched by
|
// A published v1 and v2, plus a draft v3 that must never be written against even though it is the
|
||||||
// name), that objecttype's versions, and the Objecten search for an existing record.
|
// highest version.
|
||||||
|
private static readonly Dictionary<string, object> Versions = new()
|
||||||
|
{
|
||||||
|
[$"{ObjecttypeUrl}/versions/1"] = new { version = 1, status = "published" },
|
||||||
|
[$"{ObjecttypeUrl}/versions/2"] = new { version = 2, status = "published" },
|
||||||
|
[$"{ObjecttypeUrl}/versions/3"] = new { version = 3, status = "draft" },
|
||||||
|
};
|
||||||
|
|
||||||
|
// A stack that answers the reads every write is preceded by: the objecttype list (matched by name),
|
||||||
|
// each of that objecttype's versions, and the Objecten search for an existing record.
|
||||||
private static HttpResponseMessage Route(HttpRequestMessage req, object[] existingObjects) =>
|
private static HttpResponseMessage Route(HttpRequestMessage req, object[] existingObjects) =>
|
||||||
req.RequestUri!.AbsolutePath.EndsWith("/versions", StringComparison.Ordinal)
|
Versions.TryGetValue(req.RequestUri!.ToString(), out var version)
|
||||||
? Json(new object[]
|
? Json(version)
|
||||||
{
|
|
||||||
new { version = 1, status = "published" },
|
|
||||||
new { version = 2, status = "published" },
|
|
||||||
// A draft must never be written against, even though it is the highest version.
|
|
||||||
new { version = 3, status = "draft" },
|
|
||||||
})
|
|
||||||
: req.RequestUri.AbsolutePath.StartsWith("/api/v2/objecttypes", StringComparison.Ordinal)
|
: req.RequestUri.AbsolutePath.StartsWith("/api/v2/objecttypes", StringComparison.Ordinal)
|
||||||
? Json(new
|
? Json(new
|
||||||
{
|
{
|
||||||
results = new[]
|
results = new[]
|
||||||
{
|
{
|
||||||
new { url = "http://objecttypen:8000/api/v2/objecttypes/other", name = "SomethingElse" },
|
new { url = "http://objecttypen:8000/api/v2/objecttypes/other", name = "SomethingElse", versions = Array.Empty<string>() },
|
||||||
new { url = ObjecttypeUrl, name = "RegisterRecord" },
|
new { url = ObjecttypeUrl, name = "RegisterRecord", versions = Versions.Keys.ToArray() },
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
: req.Method == HttpMethod.Get
|
: req.Method == HttpMethod.Get
|
||||||
@@ -166,8 +169,8 @@ public class ObjectenGatewayTests
|
|||||||
public async Task Fails_loudly_when_the_objecttype_has_no_published_version()
|
public async Task Fails_loudly_when_the_objecttype_has_no_published_version()
|
||||||
{
|
{
|
||||||
var sent = new List<Sent>();
|
var sent = new List<Sent>();
|
||||||
var gateway = Gateway(sent, req => req.RequestUri!.AbsolutePath.EndsWith("/versions", StringComparison.Ordinal)
|
var gateway = Gateway(sent, req => req.RequestUri!.AbsolutePath.Contains("/versions/", StringComparison.Ordinal)
|
||||||
? Json(new object[] { new { version = 1, status = "draft" } })
|
? Json(new { version = 1, status = "draft" })
|
||||||
: Route(req, []));
|
: Route(req, []));
|
||||||
|
|
||||||
var error = await Assert.ThrowsAsync<InvalidOperationException>(() => gateway.UpsertAsync(Record()));
|
var error = await Assert.ThrowsAsync<InvalidOperationException>(() => gateway.UpsertAsync(Record()));
|
||||||
@@ -230,6 +233,30 @@ public class ObjectenGatewayTests
|
|||||||
Assert.DoesNotContain(sent, s => s.Method == HttpMethod.Post || s.Method == HttpMethod.Patch);
|
Assert.DoesNotContain(sent, s => s.Method == HttpMethod.Post || s.Method == HttpMethod.Patch);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Fails_loudly_when_the_objecttype_carries_no_versions_at_all()
|
||||||
|
{
|
||||||
|
var sent = new List<Sent>();
|
||||||
|
var gateway = Gateway(sent, req => req.RequestUri!.AbsolutePath == "/api/v2/objecttypes"
|
||||||
|
? Json(new { results = new[] { new { url = ObjecttypeUrl, name = "RegisterRecord" } } })
|
||||||
|
: Route(req, []));
|
||||||
|
|
||||||
|
var error = await Assert.ThrowsAsync<InvalidOperationException>(() => gateway.UpsertAsync(Record()));
|
||||||
|
Assert.Contains("published version", error.Message);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Says_which_read_failed_when_the_objecten_search_errors()
|
||||||
|
{
|
||||||
|
var sent = new List<Sent>();
|
||||||
|
var gateway = Gateway(sent, req => req.Method == HttpMethod.Get && req.RequestUri!.AbsolutePath == "/api/v2/objects"
|
||||||
|
? new HttpResponseMessage(HttpStatusCode.InternalServerError) { Content = new StringContent("boom") }
|
||||||
|
: Route(req, []));
|
||||||
|
|
||||||
|
var error = await Assert.ThrowsAsync<HttpRequestException>(() => gateway.UpsertAsync(Record()));
|
||||||
|
Assert.Contains("Querying objects", error.Message);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task Surfaces_an_empty_read_body_rather_than_dereferencing_it()
|
public async Task Surfaces_an_empty_read_body_rather_than_dereferencing_it()
|
||||||
{
|
{
|
||||||
@@ -247,13 +274,12 @@ public class ObjectenGatewayTests
|
|||||||
public async Task Treats_a_result_less_response_as_no_match_rather_than_crashing()
|
public async Task Treats_a_result_less_response_as_no_match_rather_than_crashing()
|
||||||
{
|
{
|
||||||
var sent = new List<Sent>();
|
var sent = new List<Sent>();
|
||||||
// Neither collection carries a `results` key — the objecttype is absent, which must surface as
|
// The objecttypes collection carries no `results` key — the objecttype is absent, which must
|
||||||
// the "not registered" error rather than a NullReferenceException.
|
// surface as the "not registered" error rather than an ArgumentNullException from LINQ.
|
||||||
var gateway = Gateway(sent, req => req.RequestUri!.AbsolutePath.EndsWith("/versions", StringComparison.Ordinal)
|
var gateway = Gateway(sent, _ => Json(new { }));
|
||||||
? Json(Array.Empty<object>())
|
|
||||||
: Json(new { }));
|
|
||||||
|
|
||||||
await Assert.ThrowsAsync<InvalidOperationException>(() => gateway.UpsertAsync(Record()));
|
var error = await Assert.ThrowsAsync<InvalidOperationException>(() => gateway.UpsertAsync(Record()));
|
||||||
|
Assert.Contains("RegisterRecord", error.Message);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user