bimapper: fix a bug and add some more test coverage (#18387)

internal/resource/mappers/bimapper/bimapper.go

@@ -145,14 +145,14 @@ func (m *Mapper) LinksForItem(item *pbresource.ID) []*pbresource.Reference {
 	m.lock.Lock()
 	defer m.lock.Unlock()
 
-	items, ok := m.linkToItem[resource.NewReferenceKey(item)]
+	links, ok := m.itemToLink[resource.NewReferenceKey(item)]
 	if !ok {
 		return nil
 	}
 
-	out := make([]*pbresource.Reference, 0, len(items))
-	for item := range items {
-		out = append(out, item.ToReference())
+	out := make([]*pbresource.Reference, 0, len(links))
+	for link := range links {
+		out = append(out, link.ToReference())
 	}
 	return out
 }
@@ -195,11 +195,11 @@ func (m *Mapper) MapLink(_ context.Context, _ controller.Runtime, res *pbresourc
 	return out, nil
 }
 
-func (m *Mapper) itemsByLink(ref resource.ReferenceKey) []*pbresource.ID {
+func (m *Mapper) itemsByLink(link resource.ReferenceKey) []*pbresource.ID {
 	m.lock.Lock()
 	defer m.lock.Unlock()
 
-	items, ok := m.linkToItem[ref]
+	items, ok := m.linkToItem[link]
 	if !ok {
 		return nil
 	}

internal/resource/mappers/bimapper/bimapper_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/internal/controller"
+	"github.com/hashicorp/consul/internal/resource"
 	rtest "github.com/hashicorp/consul/internal/resource/resourcetest"
 	"github.com/hashicorp/consul/proto-public/pbresource"
 	"github.com/hashicorp/consul/proto/private/prototest"
@@ -42,28 +43,40 @@ func TestMapper(t *testing.T) {
 	barSvc := rtest.Resource(fakeBarType, "bar").Build()
 	wwwSvc := rtest.Resource(fakeBarType, "www").Build()
 
+	apiRef := newRef(fakeBarType, "api")
+	fooRef := newRef(fakeBarType, "foo")
+	barRef := newRef(fakeBarType, "bar")
+	wwwRef := newRef(fakeBarType, "www")
+
 	fail1 := rtest.Resource(fakeFooType, "api").Build()
 	fail1_refs := []*pbresource.Reference{
-		newRef(fakeBarType, "api"),
-		newRef(fakeBarType, "foo"),
-		newRef(fakeBarType, "bar"),
+		apiRef,
+		fooRef,
+		barRef,
 	}
 
 	fail2 := rtest.Resource(fakeFooType, "www").Build()
 	fail2_refs := []*pbresource.Reference{
-		newRef(fakeBarType, "www"),
-		newRef(fakeBarType, "foo"),
+		wwwRef,
+		fooRef,
 	}
 
 	fail1_updated := rtest.Resource(fakeFooType, "api").Build()
 	fail1_updated_refs := []*pbresource.Reference{
-		newRef(fakeBarType, "api"),
-		newRef(fakeBarType, "bar"),
+		apiRef,
+		barRef,
 	}
 
 	m := New(fakeFooType, fakeBarType)
 
 	// Nothing tracked yet so we assume nothing.
+	requireLinksForItem(t, m, fail1.Id)
+	requireLinksForItem(t, m, fail2.Id)
+	requireItemsForLink(t, m, apiRef)
+	requireItemsForLink(t, m, fooRef)
+	requireItemsForLink(t, m, barRef)
+	requireItemsForLink(t, m, wwwRef)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc)
 	requireServicesTracked(t, m, fooSvc)
@@ -74,6 +87,13 @@ func TestMapper(t *testing.T) {
 	m.UntrackItem(fail1.Id)
 
 	// still nothing
+	requireLinksForItem(t, m, fail1.Id)
+	requireLinksForItem(t, m, fail2.Id)
+	requireItemsForLink(t, m, apiRef)
+	requireItemsForLink(t, m, fooRef)
+	requireItemsForLink(t, m, barRef)
+	requireItemsForLink(t, m, wwwRef)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc)
 	requireServicesTracked(t, m, fooSvc)
@@ -83,6 +103,12 @@ func TestMapper(t *testing.T) {
 	// Actually insert some data.
 	m.TrackItem(fail1.Id, fail1_refs)
 
+	requireLinksForItem(t, m, fail1.Id, fail1_refs...)
+	requireItemsForLink(t, m, apiRef, fail1.Id)
+	requireItemsForLink(t, m, fooRef, fail1.Id)
+	requireItemsForLink(t, m, barRef, fail1.Id)
+	requireItemsForLink(t, m, wwwRef)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc, fail1.Id)
 	requireServicesTracked(t, m, fooSvc, fail1.Id)
@@ -92,6 +118,12 @@ func TestMapper(t *testing.T) {
 	// track it again, no change
 	m.TrackItem(fail1.Id, fail1_refs)
 
+	requireLinksForItem(t, m, fail1.Id, fail1_refs...)
+	requireItemsForLink(t, m, apiRef, fail1.Id)
+	requireItemsForLink(t, m, fooRef, fail1.Id)
+	requireItemsForLink(t, m, barRef, fail1.Id)
+	requireItemsForLink(t, m, wwwRef)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc, fail1.Id)
 	requireServicesTracked(t, m, fooSvc, fail1.Id)
@@ -101,6 +133,13 @@ func TestMapper(t *testing.T) {
 	// track new one that overlaps slightly
 	m.TrackItem(fail2.Id, fail2_refs)
 
+	requireLinksForItem(t, m, fail1.Id, fail1_refs...)
+	requireLinksForItem(t, m, fail2.Id, fail2_refs...)
+	requireItemsForLink(t, m, apiRef, fail1.Id)
+	requireItemsForLink(t, m, fooRef, fail1.Id, fail2.Id)
+	requireItemsForLink(t, m, barRef, fail1.Id)
+	requireItemsForLink(t, m, wwwRef, fail2.Id)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc, fail1.Id)
 	requireServicesTracked(t, m, fooSvc, fail1.Id, fail2.Id)
@@ -110,6 +149,13 @@ func TestMapper(t *testing.T) {
 	// update the original to change it
 	m.TrackItem(fail1_updated.Id, fail1_updated_refs)
 
+	requireLinksForItem(t, m, fail1.Id, fail1_updated_refs...)
+	requireLinksForItem(t, m, fail2.Id, fail2_refs...)
+	requireItemsForLink(t, m, apiRef, fail1.Id)
+	requireItemsForLink(t, m, fooRef, fail2.Id)
+	requireItemsForLink(t, m, barRef, fail1.Id)
+	requireItemsForLink(t, m, wwwRef, fail2.Id)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc, fail1.Id)
 	requireServicesTracked(t, m, fooSvc, fail2.Id)
@@ -119,6 +165,13 @@ func TestMapper(t *testing.T) {
 	// delete the original
 	m.UntrackItem(fail1.Id)
 
+	requireLinksForItem(t, m, fail1.Id)
+	requireLinksForItem(t, m, fail2.Id, fail2_refs...)
+	requireItemsForLink(t, m, apiRef)
+	requireItemsForLink(t, m, fooRef, fail2.Id)
+	requireItemsForLink(t, m, barRef)
+	requireItemsForLink(t, m, wwwRef, fail2.Id)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc)
 	requireServicesTracked(t, m, fooSvc, fail2.Id)
@@ -128,6 +181,13 @@ func TestMapper(t *testing.T) {
 	// delete the other one
 	m.UntrackItem(fail2.Id)
 
+	requireLinksForItem(t, m, fail1.Id)
+	requireLinksForItem(t, m, fail2.Id)
+	requireItemsForLink(t, m, apiRef)
+	requireItemsForLink(t, m, fooRef)
+	requireItemsForLink(t, m, barRef)
+	requireItemsForLink(t, m, wwwRef)
+
 	requireServicesTracked(t, m, randoSvc)
 	requireServicesTracked(t, m, apiSvc)
 	requireServicesTracked(t, m, fooSvc)
@@ -152,6 +212,22 @@ func requireServicesTracked(t *testing.T, mapper *Mapper, link *pbresource.Resou
 	}
 }
 
+func requireLinksForItem(t *testing.T, mapper *Mapper, item *pbresource.ID, links ...*pbresource.Reference) {
+	t.Helper()
+
+	got := mapper.LinksForItem(item)
+
+	prototest.AssertElementsMatch(t, links, got)
+}
+
+func requireItemsForLink(t *testing.T, mapper *Mapper, link *pbresource.Reference, items ...*pbresource.ID) {
+	t.Helper()
+
+	got := mapper.ItemsForLink(resource.IDFromReference(link))
+
+	prototest.AssertElementsMatch(t, items, got)
+}
+
 func newRef(typ *pbresource.Type, name string) *pbresource.Reference {
 	return rtest.Resource(typ, name).Reference("")
 }