Skip to content

Commit

Permalink
bimapper: fix a bug and add some more test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@rboyer
rboyer committed Aug 4, 2023
1 parent 1f28ac2 commit 4b23e40
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 13 deletions.
12 changes: 6 additions & 6 deletions internal/resource/mappers/bimapper/bimapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
90 changes: 83 additions & 7 deletions internal/resource/mappers/bimapper/bimapper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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("")
}
Expand Down

0 comments on commit 4b23e40

Please sign in to comment.