Skip to content

Commit cfd8bce

Browse files
erlendoksvollclaude
andcommitted
Fix duplicate key crash in bulk reference data lookups
Properties with multiple addresses on the same veg or in the same krets passed duplicate ids to GetVeger/GetKretser. The store client preserves caller duplicates, so the ToDictionary fan-in in FetchAddressTuples threw "An item with the same key has already been added", and the swallowed exception left affected properties without addresses. Deduplicate the id lists at every bulk-fetch call site that feeds a ToDictionary, and add a regression test whose fakes mirror the store client's duplicate-preserving semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8e6719d commit cfd8bce

2 files changed

Lines changed: 81 additions & 7 deletions

File tree

src/Dan.Plugin.Kartverket/Clients/KartverketGrunnbok.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ private static (int gnr, int bnr, int fnr, int snr, long knr) ParseMatrikkelKey(
205205
return ("", "");
206206

207207
// One (cached) bulk lookup for the whole krets list instead of one call per id
208-
var kretser = (await _matrikkelStoreClient.GetKretser(kretsList.Select(k => k.value)))
208+
var kretser = (await _matrikkelStoreClient.GetKretser(kretsList.Select(k => k.value).Distinct()))
209209
.ToDictionary(k => k.id.value);
210210

211211
return GetPostalInformation(kretsList, kretser);
@@ -552,7 +552,7 @@ public async Task<List<Address>> GetAdresseByMatrikkelNumber(string matrikkelNum
552552
}
553553

554554
var bruksenhetIder = await _matrikkelBruksenhetService.GetBruksenheter(matrikkelenhetid.value);
555-
var bruksenheter = (await _matrikkelStoreClient.GetBruksenheter(bruksenhetIder.Select(id => id.value)))
555+
var bruksenheter = (await _matrikkelStoreClient.GetBruksenheter(bruksenhetIder.Select(id => id.value).Distinct()))
556556
.ToDictionary(b => b.id.value);
557557

558558
foreach (var bruksenhetId in bruksenhetIder)
@@ -587,7 +587,7 @@ private async Task<Address> GetAddressByMatrikkelenhetId(long matrikkelenhetId)
587587
var theAddress = new Address();
588588

589589
var matrikkelEnhetAddresseListe = await _matrikkelAdresseClientService.GetAdresserForMatrikkelenhet(matrikkelenhetId);
590-
var adresser = (await _matrikkelStoreClient.GetAdresser(matrikkelEnhetAddresseListe.Select(a => a.value)))
590+
var adresser = (await _matrikkelStoreClient.GetAdresser(matrikkelEnhetAddresseListe.Select(a => a.value).Distinct()))
591591
.ToDictionary(a => a.id.value);
592592

593593
foreach(var adresse in matrikkelEnhetAddresseListe)
@@ -654,7 +654,7 @@ private async Task<Address> GetAddresseByBruksenhet(Bruksenhet bruksenhet, long
654654
theAddress.Street = addresse;
655655

656656
var matrikkelEnhetAddresseListe = await _matrikkelAdresseClientService.GetAdresserForMatrikkelenhet(matrikkelenhetId);
657-
var matrikkelAdresser = (await _matrikkelStoreClient.GetAdresser(matrikkelEnhetAddresseListe.Select(a => a.value)))
657+
var matrikkelAdresser = (await _matrikkelStoreClient.GetAdresser(matrikkelEnhetAddresseListe.Select(a => a.value).Distinct()))
658658
.ToDictionary(a => a.id.value);
659659
foreach (var add in matrikkelEnhetAddresseListe)
660660
{
@@ -682,7 +682,7 @@ private async Task<Address> GetAddresseByBruksenhet(Bruksenhet bruksenhet, long
682682
private async Task<List<(string street, string postal, string city)?>> FetchAddressTuples(
683683
BruksenhetServiceBruksenhetId[] bruksenhetIds, AdresseServiceAdresseId[] matrikkelAddrIds)
684684
{
685-
var bruksenheter = (await _matrikkelStoreClient.GetBruksenheter(bruksenhetIds.Select(id => id.value)).ConfigureAwait(false))
685+
var bruksenheter = (await _matrikkelStoreClient.GetBruksenheter(bruksenhetIds.Select(id => id.value).Distinct()).ConfigureAwait(false))
686686
.ToDictionary(b => b.id.value);
687687

688688
var adresseIds = bruksenheter.Values
@@ -696,8 +696,8 @@ private async Task<Address> GetAddresseByBruksenhet(Bruksenhet bruksenhet, long
696696

697697
// Veger og kretser er cachede referansedata — hent alle i ett kall per type
698698
var vegadresser = adresser.Values.OfType<Vegadresse>().ToList();
699-
var vegerTask = _matrikkelStoreClient.GetVeger(vegadresser.Where(v => v.vegId != null).Select(v => v.vegId.value));
700-
var kretserTask = _matrikkelStoreClient.GetKretser(vegadresser.SelectMany(v => v.kretsIds ?? Array.Empty<KretsId>()).Select(k => k.value));
699+
var vegerTask = _matrikkelStoreClient.GetVeger(vegadresser.Where(v => v.vegId != null).Select(v => v.vegId.value).Distinct());
700+
var kretserTask = _matrikkelStoreClient.GetKretser(vegadresser.SelectMany(v => v.kretsIds ?? Array.Empty<KretsId>()).Select(k => k.value).Distinct());
701701
await Task.WhenAll(vegerTask, kretserTask).ConfigureAwait(false);
702702
var veger = vegerTask.Result.ToDictionary(v => v.id.value);
703703
var kretser = kretserTask.Result.ToDictionary(k => k.id.value);

test/Altinn.Dan.Plugin.Kartverket.Test/Clients/KartverketGrunnbokMatrikkelServiceTest.cs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,80 @@ public async Task GetAddresses_UsesBulkFetchesAndAssemblesAddresses()
301301
A.CallTo(() => _matrikkelStoreService.GetKrets(A<long>._)).MustNotHaveHappened();
302302
}
303303

304+
[Fact]
305+
public async Task GetAddresses_WithAddressesSharingVegAndKrets_DeduplicatesReferenceIdLookups()
306+
{
307+
var property = new Property
308+
{
309+
MunicipalityNumber = "1860",
310+
HoldingNumber = "134",
311+
SubholdingNumber = "14",
312+
LeaseNumber = "0",
313+
SectionNumber = "0"
314+
};
315+
var input = new KartverketResponse
316+
{
317+
PropertyRights = new PropertyRights
318+
{
319+
Properties = new List<Property> { property },
320+
PropertiesWithRights = new List<PropertyWithRights>()
321+
}
322+
};
323+
324+
A.CallTo(() => _matrikkelenhetService.GetMatrikkelenhet(134, 14, 0, 0, "1860"))
325+
.Returns(new MatrikkelenhetServiceMatrikkelenhetId { value = 10 });
326+
A.CallTo(() => _matrikkelStoreService.GetKommune(1860))
327+
.Returns(new MatrikkelStoreKommune { kommunenavn = "Testkommune" });
328+
329+
A.CallTo(() => _bruksenhetService.GetBruksenheter(10)).Returns(new[]
330+
{
331+
new BruksenhetServiceBruksenhetId { value = 1 },
332+
new BruksenhetServiceBruksenhetId { value = 2 }
333+
});
334+
A.CallTo(() => _adresseService.GetAdresserForMatrikkelenhet(10))
335+
.Returns(Array.Empty<AdresseServiceAdresseId>());
336+
337+
// Both bruksenheter have their own address on the same veg and in the same krets —
338+
// the veg/krets bulk lookups must be deduplicated before hitting the store client
339+
A.CallTo(() => _matrikkelStoreService.GetBruksenheter(A<IEnumerable<long>>._)).Returns(new List<Bruksenhet>
340+
{
341+
new() { id = new BruksenhetId { value = 1 }, adresseId = new AdresseId { value = 20 } },
342+
new() { id = new BruksenhetId { value = 2 }, adresseId = new AdresseId { value = 21 } }
343+
});
344+
A.CallTo(() => _matrikkelStoreService.GetAdresser(A<IEnumerable<long>>._)).Returns(new List<Adresse>
345+
{
346+
new Vegadresse { id = new AdresseId { value = 20 }, vegId = new VegId { value = 30 }, nummer = 1, bokstav = "A", kretsIds = new[] { new KretsId { value = 40 } } },
347+
new Vegadresse { id = new AdresseId { value = 21 }, vegId = new VegId { value = 30 }, nummer = 2, bokstav = "B", kretsIds = new[] { new KretsId { value = 40 } } }
348+
});
349+
350+
// Like the real store client, the fakes return one object per requested id and
351+
// preserve duplicates — duplicate ids would make the ToDictionary fan-in throw
352+
IEnumerable<long> requestedVegIds = null;
353+
A.CallTo(() => _matrikkelStoreService.GetVeger(A<IEnumerable<long>>._))
354+
.ReturnsLazily((IEnumerable<long> ids) =>
355+
{
356+
requestedVegIds = ids.ToList();
357+
return Task.FromResult(ids.Select(id => new Veg { id = new VegId { value = id }, adressenavn = "Testveien" }).ToList());
358+
});
359+
IEnumerable<long> requestedKretsIds = null;
360+
A.CallTo(() => _matrikkelStoreService.GetKretser(A<IEnumerable<long>>._))
361+
.ReturnsLazily((IEnumerable<long> ids) =>
362+
{
363+
requestedKretsIds = ids.ToList();
364+
return Task.FromResult(ids.Select(id => (Krets)new Postnummeromrade { id = new KretsId { value = id }, kretsnummer = 1234, kretsnavn = "Testbyen" }).ToList());
365+
});
366+
367+
var result = await CreateService().GetAddresses(input);
368+
369+
requestedVegIds.Should().Equal(30);
370+
requestedKretsIds.Should().Equal(40);
371+
372+
var updated = result.PropertyRights.Properties.Single();
373+
updated.AddressList.Should().Equal("Testveien 1A", "Testveien 2B");
374+
updated.PostalCode.Should().Be("1234");
375+
updated.City.Should().Be("Testbyen");
376+
}
377+
304378
#endregion
305379

306380
#region PropertyHasFritidsbolig

0 commit comments

Comments
 (0)