From d2076941f4eef0e2fe5335031461e5c1cea32b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Franke?= Date: Thu, 19 Jan 2023 14:07:28 +0100 Subject: [PATCH] Add tests. Also update mocks with mockery. --- .../pkg/identity/ldap_education_class.go | 2 +- .../pkg/identity/ldap_education_class_test.go | 289 ++++++++++++++++++ .../pkg/identity/mocks/education_backend.go | 23 ++ .../pkg/service/v0/educationclasses_test.go | 12 +- .../search/pkg/content/mocks/extractor.go | 2 +- .../search/pkg/content/mocks/retriever.go | 2 +- services/search/pkg/engine/mocks/engine.go | 2 +- services/search/pkg/search/mocks/searcher.go | 2 +- 8 files changed, 325 insertions(+), 9 deletions(-) diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index 2fe3b1849..046fbccbd 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -218,7 +218,7 @@ func (i *LDAP) UpdateEducationClass(ctx context.Context, id string, class libreg func (i *LDAP) updateClassExternalID(ctx context.Context, dn, externalID string) (string, error) { logger := i.logger.SubloggerWithRequestID(ctx) - newDN := fmt.Sprintf("ocEducationExternalID=%s", externalID) + newDN := fmt.Sprintf("ocEducationExternalId=%s", externalID) mrdn := ldap.NewModifyDNRequest(dn, newDN, true, "") i.logger.Debug().Str("Backend", "ldap"). diff --git a/services/graph/pkg/identity/ldap_education_class_test.go b/services/graph/pkg/identity/ldap_education_class_test.go index f72713021..8f1b995ef 100644 --- a/services/graph/pkg/identity/ldap_education_class_test.go +++ b/services/graph/pkg/identity/ldap_education_class_test.go @@ -20,6 +20,7 @@ var classEntry = ldap.NewEntry("ocEducationExternalId=Math0123", "ocEducationClassType": {"course"}, "entryUUID": {"abcd-defg"}, }) + var classEntryWithMember = ldap.NewEntry("ocEducationExternalId=Math0123", map[string][]string{ "cn": {"Math"}, @@ -300,3 +301,291 @@ func TestGetEducationClassMembers(t *testing.T) { } } } + +func TestLDAP_UpdateEducationClass(t *testing.T) { + externalIDs := []string{"Math3210"} + changeString := "xxxx-xxxx" + type args struct { + id string + class libregraph.EducationClass + } + type modifyData struct { + arg *ldap.ModifyRequest + ret error + } + type modifyDNData struct { + arg *ldap.ModifyDNRequest + ret error + } + type searchData struct { + res *ldap.SearchResult + err error + } + tests := []struct { + name string + args args + modifyDNData modifyDNData + modifyData modifyData + searchData searchData + assertion func(assert.TestingT, error, ...interface{}) bool + }{ + { + name: "Change name", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + DisplayName: "Math-2", + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Nil(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{ + DN: "ocEducationExternalId=Math0123", + Changes: []ldap.Change{ + { + Operation: ldap.ReplaceAttribute, + Modification: ldap.PartialAttribute{ + Type: "cn", + Vals: []string{"Math-2"}, + }, + }, + }, + }, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Change external ID", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + ExternalId: &externalIDs[0], + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Nil(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{ + DN: "ocEducationExternalId=Math0123", + NewRDN: "ocEducationExternalId=Math3210", + DeleteOldRDN: true, + NewSuperior: "", + }, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Change both name and external ID", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + DisplayName: "Math-2", + ExternalId: &externalIDs[0], + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Nil(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{ + DN: "ocEducationExternalId=Math3210,ou=groups,dc=test", + Changes: []ldap.Change{ + { + Operation: ldap.ReplaceAttribute, + Modification: ldap.PartialAttribute{ + Type: "cn", + Vals: []string{"Math-2"}, + }, + }, + }, + }, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{ + DN: "ocEducationExternalId=Math0123", + NewRDN: "ocEducationExternalId=Math3210", + DeleteOldRDN: true, + NewSuperior: "", + }, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Check error: attempt at changing ID", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + Id: &changeString, + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Error(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Check error: attempt at changing SAM account name", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + OnPremisesSamAccountName: &changeString, + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Error(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Check error: attempt at changing Domain Name", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + OnPremisesDomainName: &changeString, + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Error(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Check error: attempt at changing description", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + Description: &changeString, + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Error(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Check error: attempt at changing classification", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + Classification: changeString, + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Error(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + { + name: "Check error: attempt at changing members", + args: args{ + id: "abcd-defg", + class: libregraph.EducationClass{ + Members: []libregraph.User{*libregraph.NewUser()}, + }, + }, + assertion: func(tt assert.TestingT, err error, i ...interface{}) bool { return assert.Error(tt, err) }, + modifyData: modifyData{ + arg: &ldap.ModifyRequest{}, + }, + modifyDNData: modifyDNData{ + arg: &ldap.ModifyDNRequest{}, + ret: nil, + }, + searchData: searchData{ + res: &ldap.SearchResult{ + Entries: []*ldap.Entry{classEntry}, + }, + err: nil, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lm := &mocks.Client{} + b, err := getMockedBackend(lm, eduConfig, &logger) + if err != nil { + panic(err) + } + + lm.On("Modify", tt.modifyData.arg).Return(tt.modifyData.ret) + lm.On("ModifyDN", tt.modifyDNData.arg).Return(tt.modifyDNData.ret) + lm.On("Search", mock.Anything).Return(tt.searchData.res, tt.searchData.err) + + ctx := context.Background() + + _, err = b.UpdateEducationClass(ctx, tt.args.id, tt.args.class) + tt.assertion(t, err) + }) + } +} diff --git a/services/graph/pkg/identity/mocks/education_backend.go b/services/graph/pkg/identity/mocks/education_backend.go index 2c2b4868b..cb8278116 100644 --- a/services/graph/pkg/identity/mocks/education_backend.go +++ b/services/graph/pkg/identity/mocks/education_backend.go @@ -340,6 +340,29 @@ func (_m *EducationBackend) RemoveUserFromEducationSchool(ctx context.Context, s return r0 } +// UpdateEducationClass provides a mock function with given fields: ctx, id, class +func (_m *EducationBackend) UpdateEducationClass(ctx context.Context, id string, class libregraph.EducationClass) (*libregraph.EducationClass, error) { + ret := _m.Called(ctx, id, class) + + var r0 *libregraph.EducationClass + if rf, ok := ret.Get(0).(func(context.Context, string, libregraph.EducationClass) *libregraph.EducationClass); ok { + r0 = rf(ctx, id, class) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*libregraph.EducationClass) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string, libregraph.EducationClass) error); ok { + r1 = rf(ctx, id, class) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // UpdateEducationSchool provides a mock function with given fields: ctx, numberOrID, school func (_m *EducationBackend) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { ret := _m.Called(ctx, numberOrID, school) diff --git a/services/graph/pkg/service/v0/educationclasses_test.go b/services/graph/pkg/service/v0/educationclasses_test.go index d1f2a9d0b..bfec31cea 100644 --- a/services/graph/pkg/service/v0/educationclasses_test.go +++ b/services/graph/pkg/service/v0/educationclasses_test.go @@ -284,13 +284,16 @@ var _ = Describe("EducationClass", func() { Context("with an existing group", func() { BeforeEach(func() { identityEducationBackend.On("GetEducationClass", mock.Anything, mock.Anything, mock.Anything).Return(newClass, nil) + identityEducationBackend.On("UpdateEducationClass", mock.Anything, mock.Anything, mock.Anything).Return(newClass, nil) }) It("fails when the number of users is exceeded - spec says 20 max", func() { updatedClass := libregraph.NewEducationClassWithDefaults() updatedClass.SetDisplayName("class updated") - updatedClass.SetMembersodataBind([]string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", - "19", "20", "21"}) + updatedClass.SetMembersodataBind([]string{ + "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", + "19", "20", "21", + }) updatedClassJson, err := json.Marshal(updatedClass) Expect(err).ToNot(HaveOccurred()) @@ -323,6 +326,7 @@ var _ = Describe("EducationClass", func() { service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), + service.WithIdentityEducationBackend(identityEducationBackend), ) r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/education/classes", bytes.NewBuffer(updatedClassJson)) @@ -385,7 +389,7 @@ var _ = Describe("EducationClass", func() { r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) svc.PatchEducationClass(rr, r) - Expect(rr.Code).To(Equal(http.StatusNoContent)) + Expect(rr.Code).To(Equal(http.StatusOK)) identityBackend.AssertNumberOfCalls(GinkgoT(), "AddMembersToGroup", 1) }) }) @@ -408,7 +412,7 @@ var _ = Describe("EducationClass", func() { Expect(rr.Code).To(Equal(http.StatusNoContent)) identityEducationBackend.AssertNumberOfCalls(GinkgoT(), "DeleteEducationClass", 1) - //eventsPublisher.AssertNumberOfCalls(GinkgoT(), "Publish", 1) + // eventsPublisher.AssertNumberOfCalls(GinkgoT(), "Publish", 1) }) }) diff --git a/services/search/pkg/content/mocks/extractor.go b/services/search/pkg/content/mocks/extractor.go index d1ab8d081..d3f705014 100644 --- a/services/search/pkg/content/mocks/extractor.go +++ b/services/search/pkg/content/mocks/extractor.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.15.0. DO NOT EDIT. +// Code generated by mockery v2.14.1. DO NOT EDIT. package mocks diff --git a/services/search/pkg/content/mocks/retriever.go b/services/search/pkg/content/mocks/retriever.go index 9b50186c6..bc380c7e6 100644 --- a/services/search/pkg/content/mocks/retriever.go +++ b/services/search/pkg/content/mocks/retriever.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.15.0. DO NOT EDIT. +// Code generated by mockery v2.14.1. DO NOT EDIT. package mocks diff --git a/services/search/pkg/engine/mocks/engine.go b/services/search/pkg/engine/mocks/engine.go index 61037adc5..273fd2e93 100644 --- a/services/search/pkg/engine/mocks/engine.go +++ b/services/search/pkg/engine/mocks/engine.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.15.0. DO NOT EDIT. +// Code generated by mockery v2.14.1. DO NOT EDIT. package mocks diff --git a/services/search/pkg/search/mocks/searcher.go b/services/search/pkg/search/mocks/searcher.go index 2f94ad0ad..557e9a0e5 100644 --- a/services/search/pkg/search/mocks/searcher.go +++ b/services/search/pkg/search/mocks/searcher.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.15.0. DO NOT EDIT. +// Code generated by mockery v2.14.1. DO NOT EDIT. package mocks