Skip to content

Commit

Permalink
[NOREF] cedar system dataloader (#2577)
Browse files Browse the repository at this point in the history
* feat: WIP cedar systems data loaders, need to update data loaders signature

* add stubbed functions to dataloader instantiation in tests

* fix loader fn

* fix tests to use testconfig context with dataloaders

* update system intake systems to also use dataloaders

---------

Co-authored-by: Steven Wade <steven.wade@oddball.io>
  • Loading branch information
mynar7 and StevenWadeOddball committed May 13, 2024
1 parent d0a3702 commit 917f763
Show file tree
Hide file tree
Showing 14 changed files with 170 additions and 156 deletions.
6 changes: 5 additions & 1 deletion cmd/test_cedar_intake/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,11 @@ func execute() {
panic(fmt.Errorf("problem intializing store in test_cedar_intake: %w", err))
}

ctx = dataloaders.CTXWithLoaders(ctx, dataloaders.NewDataLoaders(store, func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil }))
ctx = dataloaders.CTXWithLoaders(ctx, dataloaders.NewDataLoaders(
store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
func(ctx context.Context) ([]*models.CedarSystem, error) { return nil, nil },
))

rootCmd.AddCommand(
submitCmd(ctx),
Expand Down
6 changes: 5 additions & 1 deletion pkg/cedar/intake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ func TestClientTestSuite(t *testing.T) {

ctx := context.Background()

ctx = dataloaders.CTXWithLoaders(ctx, dataloaders.NewDataLoaders(store, func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil }))
ctx = dataloaders.CTXWithLoaders(ctx, dataloaders.NewDataLoaders(
store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
func(ctx context.Context) ([]*models.CedarSystem, error) { return nil, nil },
))

tests := &ClientTestSuite{
Suite: suite.Suite{},
Expand Down
75 changes: 75 additions & 0 deletions pkg/dataloaders/cedar_system_loader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package dataloaders

import (
"context"
"fmt"

"github.com/graph-gophers/dataloader"
"go.uber.org/zap"

"github.com/cmsgov/easi-app/pkg/appcontext"
"github.com/cmsgov/easi-app/pkg/models"
)

const cedar_system_loader_key = "id"

func (loaders *DataLoaders) getCedarSystemBatchFunction(ctx context.Context, keys dataloader.Keys) []*dataloader.Result {
logger := appcontext.ZLogger(ctx)
cedarSystems, err := loaders.GetCedarSystems(ctx)
if err != nil {
logger.Error("error getting cedar systems in data loader", zap.Error(err))
loaderErrors := make([]*dataloader.Result, len(keys))
for index := range keys {
loaderErrors[index] = &dataloader.Result{Data: nil, Error: err}
}
return loaderErrors
}

systemSummaryMap := make(map[string]*models.CedarSystem)
for _, sys := range cedarSystems {
if sys != nil {
systemSummaryMap[sys.ID.String] = sys
}
}
output := make([]*dataloader.Result, len(keys))

for index, key := range keys {
ck, ok := key.Raw().(KeyArgs)
if ok {
resKey := fmt.Sprint(ck.Args[cedar_system_loader_key])
cedarSystem, ok := systemSummaryMap[resKey]
if ok {
output[index] = &dataloader.Result{Data: cedarSystem, Error: nil}
} else {
err := fmt.Errorf("cedar system not found for id %s", resKey)
output[index] = &dataloader.Result{Data: nil, Error: err}
}
} else {
err := fmt.Errorf("could not retrive key from %s", key.String())
output[index] = &dataloader.Result{Data: nil, Error: err}
}
}

return output
}

func GetCedarSystemByID(ctx context.Context, id string) (*models.CedarSystem, error) {
allLoaders := Loaders(ctx)
cedarSystemLoader := allLoaders.cedarSystemByIDLoader

key := NewKeyArgs()
key.Args[cedar_system_loader_key] = id

thunk := cedarSystemLoader.Loader.Load(ctx, key)
result, err := thunk()
if err != nil {
return nil, err
}

system, isCedarSystem := result.(*models.CedarSystem)
if !isCedarSystem {
return nil, fmt.Errorf("result is not a cedar system it is Type %T", system)
}

return system, nil
}
9 changes: 6 additions & 3 deletions pkg/dataloaders/data_loaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ type DataLoaders struct {
systemIntakeSystemsLoader *WrappedDataLoader
trbRequestContractNumbersLoader *WrappedDataLoader
trbRequestSystemsLoader *WrappedDataLoader
cedarSystemByIDLoader *WrappedDataLoader
cedarSystemIsBookmarked *WrappedDataLoader
FetchUserInfos func(context.Context, []string) ([]*models.UserInfo, error)
GetCedarSystems func(ctx context.Context) ([]*models.CedarSystem, error)
}

// NewDataLoaders instantiates data loaders for the middleware
func NewDataLoaders(store *storage.Store, fetchUserInfos func(context.Context, []string) ([]*models.UserInfo, error)) *DataLoaders {
func NewDataLoaders(store *storage.Store, fetchUserInfos func(context.Context, []string) ([]*models.UserInfo, error), getCedarSystems func(ctx context.Context) ([]*models.CedarSystem, error)) *DataLoaders {
loaders := &DataLoaders{
DataReader: &DataReader{
Store: store,
},
FetchUserInfos: fetchUserInfos,
FetchUserInfos: fetchUserInfos,
GetCedarSystems: getCedarSystems,
}

loaders.UserAccountLoader = newWrappedDataLoader(loaders.GetUserAccountsByIDLoader)
Expand All @@ -39,7 +42,7 @@ func NewDataLoaders(store *storage.Store, fetchUserInfos func(context.Context, [

loaders.trbRequestContractNumbersLoader = newWrappedDataLoader(loaders.getTRBRequestContractNumbersByTRBRequestID)
loaders.trbRequestSystemsLoader = newWrappedDataLoader(loaders.getTRBRequestSystemsByTRBRequestID)

loaders.cedarSystemByIDLoader = newWrappedDataLoader(loaders.getCedarSystemBatchFunction)
loaders.cedarSystemIsBookmarked = newWrappedDataLoader(loaders.getBookmarkedCEDARSystems)
return loaders
}
Expand Down
18 changes: 10 additions & 8 deletions pkg/graph/resolvers/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"testing"

"github.com/guregu/null/zero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
Expand All @@ -13,6 +12,7 @@ import (
"github.com/cmsgov/easi-app/pkg/appconfig"
"github.com/cmsgov/easi-app/pkg/appcontext"
"github.com/cmsgov/easi-app/pkg/authentication"
cedarcore "github.com/cmsgov/easi-app/pkg/cedar/core"
"github.com/cmsgov/easi-app/pkg/dataloaders"
"github.com/cmsgov/easi-app/pkg/email"
"github.com/cmsgov/easi-app/pkg/local"
Expand Down Expand Up @@ -102,8 +102,16 @@ func (tc *TestConfigs) GetDefaults() {
// principal is fetched between each test in SetupTest()
ctx := appcontext.WithLogger(context.Background(), tc.Logger)
ctx = appcontext.WithPrincipal(ctx, getTestPrincipal(tc.Store, tc.UserInfo.Username))
coreClient := cedarcore.NewClient(ctx, "", "", "", true, true)
getCedarSystems := func(ctx context.Context) ([]*models.CedarSystem, error) {
return coreClient.GetSystemSummary(ctx)
}
// Set up mocked dataloaders for the test context
ctx = dataloaders.CTXWithLoaders(ctx, dataloaders.NewDataLoaders(tc.Store, func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil }))
ctx = dataloaders.CTXWithLoaders(ctx, dataloaders.NewDataLoaders(
tc.Store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
getCedarSystems,
))

tc.Context = ctx

Expand Down Expand Up @@ -178,9 +186,3 @@ func (suite *ResolverSuite) createNewIntake() *models.SystemIntake {

return newIntake
}

func mockGetCedarSystem(_ context.Context, systemID string) (*models.CedarSystem, error) {
return &models.CedarSystem{
ID: zero.StringFrom(systemID),
}, nil
}
67 changes: 19 additions & 48 deletions pkg/graph/resolvers/system_intake_relation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/guregu/null/zero"
"github.com/jmoiron/sqlx"

"github.com/cmsgov/easi-app/pkg/dataloaders"
"github.com/cmsgov/easi-app/pkg/graph/model"
"github.com/cmsgov/easi-app/pkg/models"
"github.com/cmsgov/easi-app/pkg/sqlutils"
Expand All @@ -23,13 +22,6 @@ type systemIntakeRelationTestCase struct {
func (suite *ResolverSuite) TestSetSystemIntakeRelationNewSystem() {
ctx := suite.testConfigs.Context
store := suite.testConfigs.Store
ctx = dataloaders.CTXWithLoaders(
ctx,
dataloaders.NewDataLoaders(
store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
),
)

submittedAt := time.Now()

Expand Down Expand Up @@ -57,7 +49,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationNewSystem() {
"should remove existing system IDs": {
InitialContractNumbers: []string{"1", "2"},
NewContractNumbers: []string{"1"},
InitialSystemIDs: []string{"a", "b"},
InitialSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
NewSystemIDs: []string{},
},
"should not add system IDs": {
Expand Down Expand Up @@ -96,7 +88,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationNewSystem() {
})
suite.NoError(err)

updatedIntakeSystemIDs, err := SystemIntakeSystems(ctx, mockGetCedarSystem, openIntake.ID)
updatedIntakeSystemIDs, err := SystemIntakeSystems(ctx, openIntake.ID)
suite.NoError(err)
suite.Equal(len(caseValues.InitialSystemIDs), len(updatedIntakeSystemIDs))

Expand All @@ -117,7 +109,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationNewSystem() {
updatedIntakeContractNumbers, err = SystemIntakeContractNumbers(ctx, updatedIntake.ID)
suite.NoError(err)

updatedIntakeSystemIDs, err = SystemIntakeSystems(ctx, mockGetCedarSystem, openIntake.ID)
updatedIntakeSystemIDs, err = SystemIntakeSystems(ctx, openIntake.ID)
suite.NoError(err)

// Ensure the system IDs were modified properly
Expand Down Expand Up @@ -146,13 +138,6 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationNewSystem() {
func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingSystem() {
ctx := suite.testConfigs.Context
store := suite.testConfigs.Store
ctx = dataloaders.CTXWithLoaders(
ctx,
dataloaders.NewDataLoaders(
store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
),
)

submittedAt := time.Now()

Expand All @@ -161,31 +146,31 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingSystem() {
InitialContractNumbers: []string{},
InitialSystemIDs: []string{},
NewContractNumbers: []string{"1", "2"},
NewSystemIDs: []string{"a", "b"},
NewSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
},
"removes existing contract numbers and system IDs when none are given": {
InitialContractNumbers: []string{"1", "2"},
InitialSystemIDs: []string{"a", "b"},
InitialSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
NewContractNumbers: []string{},
NewSystemIDs: []string{},
},
"changes existing contract numbers and system IDs to different ones": {
InitialContractNumbers: []string{"1", "2"},
InitialSystemIDs: []string{"a", "b"},
InitialSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
NewContractNumbers: []string{"3", "4"},
NewSystemIDs: []string{"c", "d"},
NewSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC3D}", "{11AB1A00-1234-5678-ABC1-1A001B00CC4E}"},
},
"changes existing contract numbers and system IDs to add new ones": {
InitialContractNumbers: []string{"1", "2"},
InitialSystemIDs: []string{"a", "b"},
InitialSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
NewContractNumbers: []string{"1", "2", "3"},
NewSystemIDs: []string{"a", "b", "c"},
NewSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC3D}", "{11AB1A00-1234-5678-ABC1-1A001B00CC4E}", "{11AB1A00-1234-5678-ABC1-1A001B00CC0A}"},
},
"changes existing contract numbers and system IDs to remove old ones": {
InitialContractNumbers: []string{"1", "2"},
InitialSystemIDs: []string{"a", "b"},
InitialSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
NewContractNumbers: []string{"1"},
NewSystemIDs: []string{"a"},
NewSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}"},
},
}

Expand Down Expand Up @@ -216,7 +201,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingSystem() {
})
suite.NoError(err)

updatedIntakeSystemIDs, err := SystemIntakeSystems(ctx, mockGetCedarSystem, openIntake.ID)
updatedIntakeSystemIDs, err := SystemIntakeSystems(ctx, openIntake.ID)
suite.NoError(err)
suite.Equal(len(caseValues.InitialSystemIDs), len(updatedIntakeSystemIDs))

Expand Down Expand Up @@ -244,7 +229,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingSystem() {
updatedIntakeContractNumbers, err = SystemIntakeContractNumbers(ctx, updatedIntake.ID)
suite.NoError(err)

updatedIntakeSystemIDs, err = SystemIntakeSystems(ctx, mockGetCedarSystem, openIntake.ID)
updatedIntakeSystemIDs, err = SystemIntakeSystems(ctx, openIntake.ID)
suite.NoError(err)

// Ensure the system IDs were modified properly
Expand Down Expand Up @@ -272,13 +257,6 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingSystem() {
func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingService() {
ctx := suite.testConfigs.Context
store := suite.testConfigs.Store
ctx = dataloaders.CTXWithLoaders(
ctx,
dataloaders.NewDataLoaders(
store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
),
)

submittedAt := time.Now()

Expand Down Expand Up @@ -306,7 +284,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingService() {
"should remove existing system IDs": {
InitialContractNumbers: []string{"1", "2"},
NewContractNumbers: []string{"1"},
InitialSystemIDs: []string{"a", "b"},
InitialSystemIDs: []string{"{11AB1A00-1234-5678-ABC1-1A001B00CC2C}", "{11AB1A00-1234-5678-ABC1-1A001B00CC1B}"},
NewSystemIDs: []string{},
},
"should not add system IDs": {
Expand Down Expand Up @@ -346,7 +324,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingService() {
})
suite.NoError(err)

updatedIntakeSystemIDs, err := SystemIntakeSystems(ctx, mockGetCedarSystem, openIntake.ID)
updatedIntakeSystemIDs, err := SystemIntakeSystems(ctx, openIntake.ID)
suite.NoError(err)
suite.Equal(len(caseValues.InitialSystemIDs), len(updatedIntakeSystemIDs))

Expand All @@ -367,7 +345,7 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingService() {
updatedIntakeContractNumbers, err = SystemIntakeContractNumbers(ctx, updatedIntake.ID)
suite.NoError(err)

updatedIntakeSystemIDs, err = SystemIntakeSystems(ctx, mockGetCedarSystem, openIntake.ID)
updatedIntakeSystemIDs, err = SystemIntakeSystems(ctx, openIntake.ID)
suite.NoError(err)

// Ensure the system IDs were modified properly
Expand Down Expand Up @@ -396,13 +374,6 @@ func (suite *ResolverSuite) TestSetSystemIntakeRelationExistingService() {
func (suite *ResolverSuite) TestUnlinkSystemIntakeRelation() {
ctx := suite.testConfigs.Context
store := suite.testConfigs.Store
ctx = dataloaders.CTXWithLoaders(
ctx,
dataloaders.NewDataLoaders(
store,
func(ctx context.Context, s []string) ([]*models.UserInfo, error) { return nil, nil },
),
)

submittedAt := time.Now()

Expand Down Expand Up @@ -440,7 +411,7 @@ func (suite *ResolverSuite) TestUnlinkSystemIntakeRelation() {
// suite.Empty(nums)

// Check system IDs are cleared
systemIDs, err := SystemIntakeSystems(ctx, mockGetCedarSystem, unlinkedIntake.ID)
systemIDs, err := SystemIntakeSystems(ctx, unlinkedIntake.ID)
suite.NoError(err)
suite.Empty(systemIDs)
})
Expand Down Expand Up @@ -487,7 +458,7 @@ func (suite *ResolverSuite) TestUnlinkSystemIntakeRelation() {
// suite.Empty(nums)

// Check system IDs are cleared
systemIDs, err := SystemIntakeSystems(ctx, mockGetCedarSystem, unlinkedIntake.ID)
systemIDs, err := SystemIntakeSystems(ctx, unlinkedIntake.ID)
suite.NoError(err)
suite.Empty(systemIDs)
})
Expand Down Expand Up @@ -526,7 +497,7 @@ func (suite *ResolverSuite) TestUnlinkSystemIntakeRelation() {
suite.Empty(nums)

// Check system IDs are cleared
systemIDs, err := SystemIntakeSystems(ctx, mockGetCedarSystem, unlinkedIntake.ID)
systemIDs, err := SystemIntakeSystems(ctx, unlinkedIntake.ID)
suite.NoError(err)
suite.Empty(systemIDs)
})
Expand Down

0 comments on commit 917f763

Please sign in to comment.