Skip to content

Commit 46ed4a9

Browse files
EmyrkEmyrk
committed
test expected group equality
1 parent 34a971a commit 46ed4a9

File tree

2 files changed

+172
-12
lines changed

2 files changed

+172
-12
lines changed

‎coderd/idpsync/group.go

+26-12
Original file line numberDiff line numberDiff line change
@@ -146,18 +146,7 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
146146
})
147147

148148
add, remove := slice.SymmetricDifferenceFunc(existingGroupsTyped, expectedGroups, func(a, b ExpectedGroup) bool {
149-
// Must match
150-
if a.OrganizationID != b.OrganizationID {
151-
return false
152-
}
153-
// Only the name or the name needs to be checked, priority is given to the ID.
154-
if a.GroupID != nil && b.GroupID != nil {
155-
return *a.GroupID == *b.GroupID
156-
}
157-
if a.GroupName != nil && b.GroupName != nil {
158-
return *a.GroupName == *b.GroupName
159-
}
160-
return false
149+
return a.Equal(b)
161150
})
162151

163152
for _, r := range remove {
@@ -283,6 +272,31 @@ type ExpectedGroup struct {
283272
GroupName *string
284273
}
285274

275+
// Equal compares two ExpectedGroups. The org id must be the same.
276+
// If the group ID is set, it will be compared and take priorty, ignoring the
277+
// name value. So 2 groups with the same ID but different names will be
278+
// considered equal.
279+
func (a ExpectedGroup) Equal(b ExpectedGroup) bool {
280+
// Must match
281+
if a.OrganizationID != b.OrganizationID {
282+
return false
283+
}
284+
// Only the name or the name needs to be checked, priority is given to the ID.
285+
if a.GroupID != nil && b.GroupID != nil {
286+
return *a.GroupID == *b.GroupID
287+
}
288+
if a.GroupName != nil && b.GroupName != nil {
289+
return *a.GroupName == *b.GroupName
290+
}
291+
292+
// If everything is nil, it is equal. Although a bit pointless
293+
if a.GroupID == nil && b.GroupID == nil &&
294+
a.GroupName == nil && b.GroupName == nil {
295+
return true
296+
}
297+
return false
298+
}
299+
286300
// ParseClaims will take the merged claims from the IDP and return the groups
287301
// the user is expected to be a member of. The expected group can either be a
288302
// name or an ID.

‎coderd/idpsync/group_test.go

+146
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/coder/coder/v2/coderd/database/dbtestutil"
2222
"github.com/coder/coder/v2/coderd/idpsync"
2323
"github.com/coder/coder/v2/coderd/runtimeconfig"
24+
"github.com/coder/coder/v2/coderd/util/ptr"
2425
"github.com/coder/coder/v2/testutil"
2526
)
2627

@@ -553,6 +554,151 @@ func TestApplyGroupDifference(t *testing.T) {
553554
}
554555
}
555556

557+
func TestExpectedGroupEqual(t *testing.T) {
558+
t.Parallel()
559+
560+
ids := coderdtest.NewDeterministicUUIDGenerator()
561+
testCases := []struct {
562+
Name string
563+
A idpsync.ExpectedGroup
564+
B idpsync.ExpectedGroup
565+
Equal bool
566+
}{
567+
{
568+
Name: "Empty",
569+
A: idpsync.ExpectedGroup{},
570+
B: idpsync.ExpectedGroup{},
571+
Equal: true,
572+
},
573+
{
574+
Name: "DifferentOrgs",
575+
A: idpsync.ExpectedGroup{
576+
OrganizationID: uuid.New(),
577+
GroupID: ptr.Ref(ids.ID("g1")),
578+
GroupName: nil,
579+
},
580+
B: idpsync.ExpectedGroup{
581+
OrganizationID: uuid.New(),
582+
GroupID: ptr.Ref(ids.ID("g1")),
583+
GroupName: nil,
584+
},
585+
Equal: false,
586+
},
587+
{
588+
Name: "SameID",
589+
A: idpsync.ExpectedGroup{
590+
OrganizationID: ids.ID("org"),
591+
GroupID: ptr.Ref(ids.ID("g1")),
592+
GroupName: nil,
593+
},
594+
B: idpsync.ExpectedGroup{
595+
OrganizationID: ids.ID("org"),
596+
GroupID: ptr.Ref(ids.ID("g1")),
597+
GroupName: nil,
598+
},
599+
Equal: true,
600+
},
601+
{
602+
Name: "DifferentIDs",
603+
A: idpsync.ExpectedGroup{
604+
OrganizationID: ids.ID("org"),
605+
GroupID: ptr.Ref(uuid.New()),
606+
GroupName: nil,
607+
},
608+
B: idpsync.ExpectedGroup{
609+
OrganizationID: ids.ID("org"),
610+
GroupID: ptr.Ref(uuid.New()),
611+
GroupName: nil,
612+
},
613+
Equal: false,
614+
},
615+
{
616+
Name: "SameName",
617+
A: idpsync.ExpectedGroup{
618+
OrganizationID: ids.ID("org"),
619+
GroupID: nil,
620+
GroupName: ptr.Ref("foo"),
621+
},
622+
B: idpsync.ExpectedGroup{
623+
OrganizationID: ids.ID("org"),
624+
GroupID: nil,
625+
GroupName: ptr.Ref("foo"),
626+
},
627+
Equal: true,
628+
},
629+
{
630+
Name: "DifferentName",
631+
A: idpsync.ExpectedGroup{
632+
OrganizationID: ids.ID("org"),
633+
GroupID: nil,
634+
GroupName: ptr.Ref("foo"),
635+
},
636+
B: idpsync.ExpectedGroup{
637+
OrganizationID: ids.ID("org"),
638+
GroupID: nil,
639+
GroupName: ptr.Ref("bar"),
640+
},
641+
Equal: false,
642+
},
643+
// Edge cases
644+
{
645+
// A bit strange, but valid as ID takes priority.
646+
// We assume 2 groups with the same ID are equal, even if
647+
// their names are different. Names are mutable, IDs are not,
648+
// so there is 0% chance they are different groups.
649+
Name: "DifferentIDSameName",
650+
A: idpsync.ExpectedGroup{
651+
OrganizationID: ids.ID("org"),
652+
GroupID: ptr.Ref(ids.ID("g1")),
653+
GroupName: ptr.Ref("foo"),
654+
},
655+
B: idpsync.ExpectedGroup{
656+
OrganizationID: ids.ID("org"),
657+
GroupID: ptr.Ref(ids.ID("g1")),
658+
GroupName: ptr.Ref("bar"),
659+
},
660+
Equal: true,
661+
},
662+
{
663+
Name: "MixedNils",
664+
A: idpsync.ExpectedGroup{
665+
OrganizationID: ids.ID("org"),
666+
GroupID: ptr.Ref(ids.ID("g1")),
667+
GroupName: nil,
668+
},
669+
B: idpsync.ExpectedGroup{
670+
OrganizationID: ids.ID("org"),
671+
GroupID: nil,
672+
GroupName: ptr.Ref("bar"),
673+
},
674+
Equal: false,
675+
},
676+
{
677+
Name: "NoComparable",
678+
A: idpsync.ExpectedGroup{
679+
OrganizationID: ids.ID("org"),
680+
GroupID: ptr.Ref(ids.ID("g1")),
681+
GroupName: nil,
682+
},
683+
B: idpsync.ExpectedGroup{
684+
OrganizationID: ids.ID("org"),
685+
GroupID: nil,
686+
GroupName: nil,
687+
},
688+
Equal: false,
689+
},
690+
}
691+
692+
for _, tc := range testCases {
693+
tc := tc
694+
t.Run(tc.Name, func(t *testing.T) {
695+
t.Parallel()
696+
697+
require.Equal(t, tc.Equal, tc.A.Equal(tc.B))
698+
})
699+
}
700+
}
701+
556702
func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store, user database.User, orgID uuid.UUID, def orgSetupDefinition) {
557703
t.Helper()
558704

0 commit comments

Comments
 (0)