diff --git a/internal/controlplane/handlers_evalstatus.go b/internal/controlplane/handlers_evalstatus.go index fab119025e..9c4259191d 100644 --- a/internal/controlplane/handlers_evalstatus.go +++ b/internal/controlplane/handlers_evalstatus.go @@ -393,7 +393,7 @@ func (s *Server) sortEntitiesEvaluationStatus( continue } - efp, err := s.props.EntityWithProperties(ctx, e.EntityID, nil) + efp, err := s.props.EntityWithPropertiesByID(ctx, e.EntityID, nil) if err != nil { if errors.Is(err, propSvc.ErrEntityNotFound) { // If the entity is not found, log and skip diff --git a/internal/controlplane/handlers_profile.go b/internal/controlplane/handlers_profile.go index 5354633252..046de40aeb 100644 --- a/internal/controlplane/handlers_profile.go +++ b/internal/controlplane/handlers_profile.go @@ -456,7 +456,7 @@ func (s *Server) getRuleEvalStatus( // the caller just ignores allt the errors anyway, so we don't start a transaction as the integrity issues // would not be discovered anyway - efp, err := s.props.EntityWithProperties(ctx, dbRuleEvalStat.EntityID, nil) + efp, err := s.props.EntityWithPropertiesByID(ctx, dbRuleEvalStat.EntityID, nil) if err != nil { return nil, fmt.Errorf("error fetching entity for properties: %w", err) } diff --git a/internal/controlplane/handlers_providers_test.go b/internal/controlplane/handlers_providers_test.go index d66ee903e3..4e387b9e57 100644 --- a/internal/controlplane/handlers_providers_test.go +++ b/internal/controlplane/handlers_providers_test.go @@ -528,7 +528,7 @@ func TestDeleteProvider(t *testing.T) { mockprops := propSvc.NewMockPropertiesService(ctrl) mockprops.EXPECT(). - EntityWithProperties(gomock.Any(), gomock.Any(), nil). + EntityWithPropertiesByID(gomock.Any(), gomock.Any(), nil). Return(models.NewEntityWithPropertiesFromInstance( models.EntityInstance{}, nil), nil) @@ -649,7 +649,7 @@ func TestDeleteProviderByID(t *testing.T) { mockprops := propSvc.NewMockPropertiesService(ctrl) mockprops.EXPECT(). - EntityWithProperties(gomock.Any(), gomock.Any(), nil). + EntityWithPropertiesByID(gomock.Any(), gomock.Any(), nil). Return(models.NewEntityWithPropertiesFromInstance( models.EntityInstance{}, nil), nil) diff --git a/internal/engine/executor.go b/internal/engine/executor.go index 3838f011de..0f0b6d4056 100644 --- a/internal/engine/executor.go +++ b/internal/engine/executor.go @@ -235,7 +235,7 @@ func (e *executor) profileEvalStatus( } // get the entity with properties by the entity UUID - ewp, err := e.propService.EntityWithProperties(ctx, entityID, + ewp, err := e.propService.EntityWithPropertiesByID(ctx, entityID, service.CallBuilder().WithStoreOrTransaction(e.querier)) if err != nil { return fmt.Errorf("error getting entity with properties: %w", err) diff --git a/internal/engine/executor_test.go b/internal/engine/executor_test.go index 05a2b71d42..46a7297417 100644 --- a/internal/engine/executor_test.go +++ b/internal/engine/executor_test.go @@ -325,7 +325,7 @@ default allow = true`, mockPropSvc := mockprops.NewMockPropertiesService(ctrl) mockPropSvc.EXPECT(). - EntityWithProperties(gomock.Any(), repositoryID, gomock.Any()). + EntityWithPropertiesByID(gomock.Any(), repositoryID, gomock.Any()). Return(&models.EntityWithProperties{ Entity: models.EntityInstance{ ID: repositoryID, diff --git a/internal/entities/properties/service/helpers.go b/internal/entities/properties/service/helpers.go index 71201ab771..658f341e61 100644 --- a/internal/entities/properties/service/helpers.go +++ b/internal/entities/properties/service/helpers.go @@ -20,6 +20,7 @@ import ( "database/sql" "errors" "fmt" + "slices" "time" "github.com/google/uuid" @@ -143,35 +144,165 @@ func getEntityIdByName( return ent.ID, nil } -func getEntityIdByUpstreamID( - ctx context.Context, projectId uuid.UUID, +func getAllByProperty( + ctx context.Context, + propName string, + propVal any, + entType minderv1.Entity, + projectID uuid.UUID, providerID uuid.UUID, - upstreamID string, entType minderv1.Entity, qtx db.ExtendQuerier, -) (uuid.UUID, error) { +) ([]db.EntityInstance, error) { ents, err := qtx.GetTypedEntitiesByPropertyV1( ctx, entities.EntityTypeToDB(entType), - properties.PropertyUpstreamID, - upstreamID, + propName, + propVal, db.GetTypedEntitiesOptions{ - ProjectID: projectId, + ProjectID: projectID, ProviderID: providerID, }) if errors.Is(err, sql.ErrNoRows) { - return uuid.Nil, ErrEntityNotFound + return nil, ErrEntityNotFound } else if err != nil { - return uuid.Nil, fmt.Errorf("error fetching entities by property: %w", err) + return nil, fmt.Errorf("error fetching entities by property: %w", err) + } + + return ents, nil +} + +func getEntityIdByUpstreamID( + ctx context.Context, + projectID uuid.UUID, providerID uuid.UUID, + upstreamID string, entType minderv1.Entity, + qtx db.ExtendQuerier, +) (uuid.UUID, error) { + ents, err := getAllByProperty(ctx, properties.PropertyUpstreamID, upstreamID, entType, projectID, providerID, qtx) + if err != nil { + return uuid.Nil, err } if len(ents) > 1 { return uuid.Nil, ErrMultipleEntities - } else if len(ents) == 1 { - return ents[0].ID, nil + } else if len(ents) == 0 { + return uuid.Nil, ErrEntityNotFound + } + + return ents[0].ID, nil +} + +func matchEntityWithHint( + ctx context.Context, + props *properties.Properties, + entType minderv1.Entity, + hint *ByUpstreamHint, + l zerolog.Logger, + qtx db.ExtendQuerier, +) (*db.EntityInstance, error) { + if !hint.isSet() { + return nil, fmt.Errorf("at least one of projectID, providerID or providerImplements must be set in hint") + } + + var ents []db.EntityInstance + var err error + + lookupOrder := []string{properties.PropertyUpstreamID, properties.PropertyName} + for _, loopupProp := range lookupOrder { + prop := props.GetProperty(loopupProp) + if prop == nil { + continue + } + + l.Debug().Str("lookupProp", loopupProp).Msg("fetching by property") + ents, err = getAllByProperty(ctx, + loopupProp, prop.RawValue(), entType, + // we search across all projects and providers. This is expected because the lookup properties only + // contain upstream properties and the get-with-hint methods are only to be used by callers who don't + // know the project or provider ID and only have an upstream webhook payload. + uuid.Nil, uuid.Nil, + qtx) + if err != nil { + return nil, fmt.Errorf("failed to get entities by upstream ID: %w", err) + } + + match, err := findMatchByUpstreamHint(ctx, ents, hint, qtx) + if err != nil { + if errors.Is(err, ErrEntityNotFound) { + l.Error().Msg("no entity matched") + continue + } else if errors.Is(err, ErrMultipleEntities) { + l.Error().Msg("multiple entities matched") + return nil, ErrMultipleEntities + } + return nil, fmt.Errorf("failed to match entity by hint: %w", err) + } + return match, nil + } + + return nil, ErrEntityNotFound +} + +func findMatchByUpstreamHint( + ctx context.Context, ents []db.EntityInstance, hint *ByUpstreamHint, qtx db.ExtendQuerier, +) (*db.EntityInstance, error) { + var match *db.EntityInstance + for _, ent := range ents { + var thisMatch *db.EntityInstance + zerolog.Ctx(ctx).Debug().Msgf("matching entity %s", ent.ID.String()) + if dbEntMatchesUpstreamHint(ctx, ent, hint, qtx) { + zerolog.Ctx(ctx).Debug().Msgf("entity %s matched by hint", ent.ID.String()) + thisMatch = &ent + } + + if thisMatch != nil { + if match != nil { + zerolog.Ctx(ctx).Error().Msg("multiple entities matched") + return nil, ErrMultipleEntities + } + match = thisMatch + } } - // no entity found - return uuid.Nil, ErrEntityNotFound + if match == nil { + zerolog.Ctx(ctx).Debug().Msg("no entity matched") + return nil, ErrEntityNotFound + } + + return match, nil +} + +func dbEntMatchesUpstreamHint(ctx context.Context, ent db.EntityInstance, hint *ByUpstreamHint, qtx db.ExtendQuerier) bool { + logger := zerolog.Ctx(ctx) + + if hint.ProviderImplements.Valid || hint.ProviderClass.Valid { + dbProv, err := qtx.GetProviderByID(ctx, ent.ProviderID) + if err != nil { + logger.Error(). + Str("providerID", ent.ProviderID.String()). + Err(err). + Msg("error getting provider by ID") + return false + } + + if hint.ProviderClass.Valid && dbProv.Class != hint.ProviderClass.ProviderClass { + logger.Debug(). + Str("ProviderID", ent.ProviderID.String()). + Str("providerClass", string(dbProv.Class)). + Str("hintProviderClass", string(hint.ProviderClass.ProviderClass)). + Msg("provider class does not match hint") + return false + } + + if hint.ProviderImplements.Valid && !slices.Contains(dbProv.Implements, hint.ProviderImplements.ProviderType) { + logger.Debug(). + Str("ProviderID", ent.ProviderID.String()). + Str("providerType", string(hint.ProviderImplements.ProviderType)). + Msg("provider does not implement hint") + return false + } + } + + return true } func (ps *propertiesService) areDatabasePropertiesValid( diff --git a/internal/entities/properties/service/mock/service.go b/internal/entities/properties/service/mock/service.go index 948fcb66b7..2b4caa4981 100644 --- a/internal/entities/properties/service/mock/service.go +++ b/internal/entities/properties/service/mock/service.go @@ -47,34 +47,49 @@ func (m *MockPropertiesService) EXPECT() *MockPropertiesServiceMockRecorder { return m.recorder } -// EntityWithProperties mocks base method. -func (m *MockPropertiesService) EntityWithProperties(ctx context.Context, entityID uuid.UUID, opts *service.CallOptions) (*models.EntityWithProperties, error) { +// EntityWithPropertiesAsProto mocks base method. +func (m *MockPropertiesService) EntityWithPropertiesAsProto(ctx context.Context, ewp *models.EntityWithProperties, provMgr manager.ProviderManager) (protoreflect.ProtoMessage, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "EntityWithProperties", ctx, entityID, opts) + ret := m.ctrl.Call(m, "EntityWithPropertiesAsProto", ctx, ewp, provMgr) + ret0, _ := ret[0].(protoreflect.ProtoMessage) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// EntityWithPropertiesAsProto indicates an expected call of EntityWithPropertiesAsProto. +func (mr *MockPropertiesServiceMockRecorder) EntityWithPropertiesAsProto(ctx, ewp, provMgr any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityWithPropertiesAsProto", reflect.TypeOf((*MockPropertiesService)(nil).EntityWithPropertiesAsProto), ctx, ewp, provMgr) +} + +// EntityWithPropertiesByID mocks base method. +func (m *MockPropertiesService) EntityWithPropertiesByID(ctx context.Context, entityID uuid.UUID, opts *service.CallOptions) (*models.EntityWithProperties, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "EntityWithPropertiesByID", ctx, entityID, opts) ret0, _ := ret[0].(*models.EntityWithProperties) ret1, _ := ret[1].(error) return ret0, ret1 } -// EntityWithProperties indicates an expected call of EntityWithProperties. -func (mr *MockPropertiesServiceMockRecorder) EntityWithProperties(ctx, entityID, opts any) *gomock.Call { +// EntityWithPropertiesByID indicates an expected call of EntityWithPropertiesByID. +func (mr *MockPropertiesServiceMockRecorder) EntityWithPropertiesByID(ctx, entityID, opts any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityWithProperties", reflect.TypeOf((*MockPropertiesService)(nil).EntityWithProperties), ctx, entityID, opts) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityWithPropertiesByID", reflect.TypeOf((*MockPropertiesService)(nil).EntityWithPropertiesByID), ctx, entityID, opts) } -// EntityWithPropertiesAsProto mocks base method. -func (m *MockPropertiesService) EntityWithPropertiesAsProto(ctx context.Context, ewp *models.EntityWithProperties, provMgr manager.ProviderManager) (protoreflect.ProtoMessage, error) { +// EntityWithPropertiesByUpstreamHint mocks base method. +func (m *MockPropertiesService) EntityWithPropertiesByUpstreamHint(ctx context.Context, entType v1.Entity, getByProps *properties.Properties, hint service.ByUpstreamHint, opts *service.CallOptions) (*models.EntityWithProperties, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "EntityWithPropertiesAsProto", ctx, ewp, provMgr) - ret0, _ := ret[0].(protoreflect.ProtoMessage) + ret := m.ctrl.Call(m, "EntityWithPropertiesByUpstreamHint", ctx, entType, getByProps, hint, opts) + ret0, _ := ret[0].(*models.EntityWithProperties) ret1, _ := ret[1].(error) return ret0, ret1 } -// EntityWithPropertiesAsProto indicates an expected call of EntityWithPropertiesAsProto. -func (mr *MockPropertiesServiceMockRecorder) EntityWithPropertiesAsProto(ctx, ewp, provMgr any) *gomock.Call { +// EntityWithPropertiesByUpstreamHint indicates an expected call of EntityWithPropertiesByUpstreamHint. +func (mr *MockPropertiesServiceMockRecorder) EntityWithPropertiesByUpstreamHint(ctx, entType, getByProps, hint, opts any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityWithPropertiesAsProto", reflect.TypeOf((*MockPropertiesService)(nil).EntityWithPropertiesAsProto), ctx, ewp, provMgr) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EntityWithPropertiesByUpstreamHint", reflect.TypeOf((*MockPropertiesService)(nil).EntityWithPropertiesByUpstreamHint), ctx, entType, getByProps, hint, opts) } // ReplaceAllProperties mocks base method. diff --git a/internal/entities/properties/service/service.go b/internal/entities/properties/service/service.go index 0d7e43f9c1..acbedcce40 100644 --- a/internal/entities/properties/service/service.go +++ b/internal/entities/properties/service/service.go @@ -60,10 +60,20 @@ type PropertiesService interface { EntityWithPropertiesAsProto( ctx context.Context, ewp *models.EntityWithProperties, provMgr manager.ProviderManager, ) (protoreflect.ProtoMessage, error) - // EntityWithProperties Fetches an Entity by ID and Project in order to refresh the properties - EntityWithProperties( + // EntityWithPropertiesByID Fetches an Entity by ID and Project in order to refresh the properties + EntityWithPropertiesByID( ctx context.Context, entityID uuid.UUID, opts *CallOptions, ) (*models.EntityWithProperties, error) + // EntityWithPropertiesByUpstreamHint fetches an entity by upstream properties + // and returns the entity with its properties. It is expected that the caller + // does NOT know the project or provider ID. Whatever hints it may have + // are to be passed to the hint parameter which will be used in case multiple + // entries with the same ID are found. + EntityWithPropertiesByUpstreamHint( + ctx context.Context, + entType minderv1.Entity, getByProps *properties.Properties, + hint ByUpstreamHint, opts *CallOptions, + ) (*models.EntityWithProperties, error) // RetrieveAllProperties fetches all properties for an entity // given a project, provider, and identifying properties. // If the entity has properties in the database, it will return those @@ -290,28 +300,14 @@ func (ps *propertiesService) ReplaceProperty( return err } -func (ps *propertiesService) EntityWithProperties( - ctx context.Context, entityID uuid.UUID, +func (ps *propertiesService) getEntityWithProperties( + ctx context.Context, + ent db.EntityInstance, opts *CallOptions, ) (*models.EntityWithProperties, error) { q := ps.getStoreOrTransaction(opts) - zerolog.Ctx(ctx).Debug().Str("entityID", entityID.String()).Msg("fetching entity with properties") - ent, err := q.GetEntityByID(ctx, entityID) - if errors.Is(err, sql.ErrNoRows) { - return nil, ErrEntityNotFound - } else if err != nil { - return nil, fmt.Errorf("error getting entity: %w", err) - } - zerolog.Ctx(ctx).Debug(). - Str("projectID", ent.ProjectID.String()). - Str("providerID", ent.ProviderID.String()). - Str("entityType", string(ent.EntityType)). - Str("entityName", ent.Name). - Str("entityID", ent.ID.String()). - Msg("entity found") - - dbProps, err := q.GetAllPropertiesForEntity(ctx, entityID) + dbProps, err := q.GetAllPropertiesForEntity(ctx, ent.ID) if errors.Is(err, sql.ErrNoRows) { return nil, fmt.Errorf("failed to get properties for entity: %w", ErrEntityNotFound) } else if err != nil { @@ -337,6 +333,75 @@ func (ps *propertiesService) EntityWithProperties( return models.NewEntityWithProperties(ent, props), nil } +func (ps *propertiesService) EntityWithPropertiesByID( + ctx context.Context, entityID uuid.UUID, + opts *CallOptions, +) (*models.EntityWithProperties, error) { + q := ps.getStoreOrTransaction(opts) + + zerolog.Ctx(ctx).Debug().Str("entityID", entityID.String()).Msg("fetching entity with properties") + ent, err := q.GetEntityByID(ctx, entityID) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrEntityNotFound + } else if err != nil { + return nil, fmt.Errorf("error getting entity: %w", err) + } + + return ps.getEntityWithProperties(ctx, ent, opts) +} + +// ByUpstreamHint is a hint to help find an entity by upstream ID +// API-wise it should only be exposed in those methods of the service layer +// that do not know the project or provider ID and are searching for an entity +// based on upstream properties only. +// The hint +type ByUpstreamHint struct { + // ProviderImplements is the provider type to search by + ProviderImplements db.NullProviderType + ProviderClass db.NullProviderClass +} + +// ToLogDict converts the hint to a log dictionary for use by zerolog +func (hint *ByUpstreamHint) ToLogDict() *zerolog.Event { + dict := zerolog.Dict(). + Interface("ProviderImplements", hint.ProviderImplements) + + return dict +} + +func (hint *ByUpstreamHint) isSet() bool { + return hint.ProviderImplements.Valid || hint.ProviderClass.Valid +} + +func (ps *propertiesService) EntityWithPropertiesByUpstreamHint( + ctx context.Context, + entType minderv1.Entity, + getByProps *properties.Properties, + hint ByUpstreamHint, + opts *CallOptions, +) (*models.EntityWithProperties, error) { + q := ps.getStoreOrTransaction(opts) + + l := zerolog.Ctx(ctx).With(). + Dict("getByProps", getByProps.ToLogDict()). + Dict("hint", hint.ToLogDict()). + Logger() + + l.Debug().Msg("fetching entity with properties by upstream hint") + + ent, err := matchEntityWithHint(ctx, getByProps, entType, &hint, l, q) + if err != nil { + return nil, fmt.Errorf("failed to get entity ID: %w", err) + } + + ewp, err := ps.getEntityWithProperties(ctx, *ent, opts) + if err != nil { + return nil, err + } + + return ewp, nil +} + // EntityWithPropertiesAsProto converts the entity with properties to a protobuf message func (_ *propertiesService) EntityWithPropertiesAsProto( ctx context.Context, ewp *models.EntityWithProperties, provMgr manager.ProviderManager, diff --git a/internal/entities/properties/service/service_test.go b/internal/entities/properties/service/service_test.go index 3d4c7281a1..e095db189b 100644 --- a/internal/entities/properties/service/service_test.go +++ b/internal/entities/properties/service/service_test.go @@ -19,6 +19,7 @@ import ( "context" "database/sql" "encoding/json" + "fmt" "testing" "time" @@ -111,9 +112,10 @@ type fetchParams struct { } type testCtx struct { - testQueries db.Store - dbProj db.Project - ghAppProvider db.Provider + testQueries db.Store + dbProj db.Project + ghAppProvider db.Provider + dockerHubProvider db.Provider } func createTestCtx(ctx context.Context, t *testing.T) testCtx { @@ -141,10 +143,22 @@ func createTestCtx(ctx context.Context, t *testing.T) testCtx { }) require.NoError(t, err) + dockerHubProvider, err := testQueries.CreateProvider(context.Background(), + db.CreateProviderParams{ + Name: rand.RandomName(seed), + ProjectID: dbProj.ID, + Class: db.ProviderClassDockerhub, + Implements: []db.ProviderType{db.ProviderTypeOci}, + AuthFlows: []db.AuthorizationFlow{db.AuthorizationFlowOauth2AuthorizationCodeFlow}, + Definition: json.RawMessage("{}"), + }) + require.NoError(t, err) + return testCtx{ - testQueries: testQueries, - dbProj: dbProj, - ghAppProvider: ghAppProvider, + testQueries: testQueries, + dbProj: dbProj, + ghAppProvider: ghAppProvider, + dockerHubProvider: dockerHubProvider, } } @@ -603,6 +617,163 @@ func TestPropertiesService_RetrieveProperty(t *testing.T) { } } +func TestPropertiesService_EntityWithPropertiesByUpstreamHint(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + tctx := createTestCtx(ctx, t) + + scenarios := []struct { + name string + entType minderv1.Entity + byPropMap map[string]any + upstreamID string + hint ByUpstreamHint + dbSetup func(t *testing.T, store db.Store) + expectedError error + checkResult func(t *testing.T, result *models.EntityWithProperties) + }{ + { + name: "Entity not found", + entType: minderv1.Entity_ENTITY_REPOSITORIES, + byPropMap: map[string]any{properties.PropertyUpstreamID: "456"}, + hint: ByUpstreamHint{ + ProviderImplements: db.NullProviderType{ + ProviderType: db.ProviderTypeGithub, + Valid: true, + }, + }, + expectedError: ErrEntityNotFound, + }, + { + name: "Multiple entities returned by ID with provider hint", + entType: minderv1.Entity_ENTITY_REPOSITORIES, + byPropMap: map[string]any{properties.PropertyUpstreamID: "789"}, + hint: ByUpstreamHint{ + ProviderImplements: db.NullProviderType{ + ProviderType: db.ProviderTypeGithub, + Valid: true, + }, + }, + dbSetup: func(t *testing.T, store db.Store) { + t.Helper() + for i := range 2 { + ent, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: fmt.Sprintf("test-repo-%d", i), + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + }) + require.NoError(t, err) + + propMap := map[string]any{ + properties.PropertyUpstreamID: "789", + properties.PropertyName: fmt.Sprintf("test-repo-%d", i), + } + insertPropertiesFromMap(ctx, t, store, ent.ID, propMap) + } + }, + expectedError: ErrMultipleEntities, + }, + { + name: "One of multiple entities matched by provider hint", + entType: minderv1.Entity_ENTITY_REPOSITORIES, + byPropMap: map[string]any{properties.PropertyUpstreamID: "890"}, + hint: ByUpstreamHint{ + ProviderImplements: db.NullProviderType{ + ProviderType: db.ProviderTypeGithub, + Valid: true, + }, + }, + dbSetup: func(t *testing.T, store db.Store) { + t.Helper() + providerIDs := []uuid.UUID{tctx.ghAppProvider.ID, tctx.dockerHubProvider.ID} + for i, providerID := range providerIDs { + ent, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: fmt.Sprintf("test-repo-implements-hint-%d", i), + ProjectID: tctx.dbProj.ID, + ProviderID: providerID, + }) + require.NoError(t, err) + + propMap := map[string]any{ + properties.PropertyUpstreamID: "890", + properties.PropertyName: fmt.Sprintf("test-repo-implements-hint-%d", i), + } + insertPropertiesFromMap(ctx, t, store, ent.ID, propMap) + } + }, + checkResult: func(t *testing.T, result *models.EntityWithProperties) { + t.Helper() + require.NotNil(t, result) + require.Equal(t, "test-repo-implements-hint-0", result.Entity.Name) + require.Equal(t, "890", result.Properties.GetProperty(properties.PropertyUpstreamID).GetString()) + require.Equal(t, tctx.ghAppProvider.ID, result.Entity.ProviderID) + }, + }, + { + name: "Searching entity with provider hint for a nonexistent provider", + entType: minderv1.Entity_ENTITY_REPOSITORIES, + byPropMap: map[string]any{properties.PropertyUpstreamID: "891"}, + hint: ByUpstreamHint{ + ProviderImplements: db.NullProviderType{ + ProviderType: db.ProviderTypeRest, + Valid: true, + }, + }, + dbSetup: func(t *testing.T, store db.Store) { + t.Helper() + providerIDs := []uuid.UUID{tctx.ghAppProvider.ID, tctx.dockerHubProvider.ID} + for i, providerID := range providerIDs { + ent, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: fmt.Sprintf("test-repo-implements-hint-nomatch-%d", i), + ProjectID: tctx.dbProj.ID, + ProviderID: providerID, + }) + require.NoError(t, err) + + propMap := map[string]any{ + properties.PropertyUpstreamID: "891", + properties.PropertyName: fmt.Sprintf("test-repo-implements-hint-nomatch-%d", i), + } + insertPropertiesFromMap(ctx, t, store, ent.ID, propMap) + } + }, + expectedError: ErrEntityNotFound, + }, + } + + for _, tt := range scenarios { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if tt.dbSetup != nil { + tt.dbSetup(t, tctx.testQueries) + } + + propSvc := NewPropertiesService(tctx.testQueries) + + byProps, err := properties.NewProperties(tt.byPropMap) + require.NoError(t, err) + + result, err := propSvc.EntityWithPropertiesByUpstreamHint(ctx, tt.entType, byProps, tt.hint, nil) + + if tt.expectedError != nil { + require.Error(t, err) + require.ErrorIs(t, err, tt.expectedError) + } else { + require.NoError(t, err) + tt.checkResult(t, result) + } + }) + } +} + func TestPropertiesService_RetrieveAllProperties(t *testing.T) { t.Parallel() @@ -698,7 +869,7 @@ func TestPropertiesService_RetrieveAllProperties(t *testing.T) { }, }, { - name: "Cache hit, fetch from cache", + name: "Cache hit by name, fetch from cache", dbSetup: func(t *testing.T, store db.Store, params fetchParams) { t.Helper() @@ -904,7 +1075,7 @@ func TestPropertiesService_EntityWithProperties(t *testing.T) { Return(tt.dbPropsBuilder(tt.entityID), nil) ps := NewPropertiesService(mockDB) - result, err := ps.EntityWithProperties(ctx, tt.entityID, nil) + result, err := ps.EntityWithPropertiesByID(ctx, tt.entityID, nil) require.NoError(t, err) require.NotNil(t, result) require.Equal(t, result.Entity.ID, tt.entityID) diff --git a/internal/history/service.go b/internal/history/service.go index 5a7fc8d0cf..9b1b57c418 100644 --- a/internal/history/service.go +++ b/internal/history/service.go @@ -218,7 +218,7 @@ func (ehs *evaluationHistoryService) ListEvaluationHistory( data := make([]*OneEvalHistoryAndEntity, 0, len(rows)) for _, row := range rows { - efp, err := propsvc.EntityWithProperties(ctx, row.EntityID, + efp, err := propsvc.EntityWithPropertiesByID(ctx, row.EntityID, propertiessvc.CallBuilder().WithStoreOrTransaction(qtx)) if err != nil { return nil, fmt.Errorf("error fetching entity for properties: %w", err) diff --git a/internal/history/service_test.go b/internal/history/service_test.go index 792949ff59..5e8b971782 100644 --- a/internal/history/service_test.go +++ b/internal/history/service_test.go @@ -836,12 +836,12 @@ func TestListEvaluationHistory(t *testing.T) { pm := pmMock.NewMockProviderManager(ctrl) propsSvc := propsSvcMock.NewMockPropertiesService(ctrl) for _, efp := range tt.efp { - propsSvc.EXPECT().EntityWithProperties(ctx, gomock.Any(), gomock.Any()). + propsSvc.EXPECT().EntityWithPropertiesByID(ctx, gomock.Any(), gomock.Any()). Return(efp, tt.entityForPropertiesError) } if tt.entityForPropertiesError != nil && len(tt.efp) == 0 { - propsSvc.EXPECT().EntityWithProperties(ctx, gomock.Any(), gomock.Any()). + propsSvc.EXPECT().EntityWithPropertiesByID(ctx, gomock.Any(), gomock.Any()). Return(nil, tt.entityForPropertiesError).AnyTimes() } propsSvc.EXPECT().RetrieveAllPropertiesForEntity(ctx, gomock.Any(), gomock.Any(), gomock.Any()). diff --git a/internal/providers/github/manager/manager.go b/internal/providers/github/manager/manager.go index 770577784c..1124249395 100644 --- a/internal/providers/github/manager/manager.go +++ b/internal/providers/github/manager/manager.go @@ -177,7 +177,7 @@ func (g *githubProviderManager) Delete(ctx context.Context, config *db.Provider) } for _, ent := range entities { - ewp, err := g.propsSvc.EntityWithProperties(ctx, ent.ID, nil) + ewp, err := g.propsSvc.EntityWithPropertiesByID(ctx, ent.ID, nil) if err != nil { zerolog.Ctx(ctx).Error().Err(err). Str("provider_id", config.ID.String()). diff --git a/internal/repositories/service.go b/internal/repositories/service.go index 739f0c0ca3..87012e38da 100644 --- a/internal/repositories/service.go +++ b/internal/repositories/service.go @@ -257,7 +257,7 @@ func (r *repositoryService) ListRepositories( ents = make([]*models.EntityWithProperties, 0, len(repoEnts)) for _, ent := range repoEnts { - ewp, err := r.propSvc.EntityWithProperties(ctx, ent.ID, + ewp, err := r.propSvc.EntityWithPropertiesByID(ctx, ent.ID, service.CallBuilder().WithStoreOrTransaction(qtx)) if err != nil { return nil, fmt.Errorf("error fetching properties for repository: %w", err) @@ -315,7 +315,7 @@ func (r *repositoryService) DeleteByID(ctx context.Context, repositoryID uuid.UU logger.BusinessRecord(ctx).Project = projectID logger.BusinessRecord(ctx).Repository = repositoryID - ent, err := r.propSvc.EntityWithProperties(ctx, repositoryID, nil) + ent, err := r.propSvc.EntityWithPropertiesByID(ctx, repositoryID, nil) if err != nil { return fmt.Errorf("error fetching repository: %w", err) } @@ -355,7 +355,7 @@ func (r *repositoryService) DeleteByName( logger.BusinessRecord(ctx).Repository = repo.ID - ent, err := r.propSvc.EntityWithProperties(ctx, repo.ID, nil) + ent, err := r.propSvc.EntityWithPropertiesByID(ctx, repo.ID, nil) if err != nil { return fmt.Errorf("error fetching repository: %w", err) } diff --git a/internal/repositories/service_test.go b/internal/repositories/service_test.go index 84ec057666..b03b69946d 100644 --- a/internal/repositories/service_test.go +++ b/internal/repositories/service_test.go @@ -712,7 +712,7 @@ func withSuccessfulPropFetch(prop *properties.Properties) func(svcMock propSvcMo func withSuccessfulEntityWithProps(mock propSvcMock) { mock.EXPECT(). - EntityWithProperties(gomock.Any(), gomock.Any(), gomock.Any()). + EntityWithPropertiesByID(gomock.Any(), gomock.Any(), gomock.Any()). Return(models.NewEntityWithPropertiesFromInstance(models.EntityInstance{ ID: dbRepo.ID, ProjectID: projectID, @@ -722,7 +722,7 @@ func withSuccessfulEntityWithProps(mock propSvcMock) { func withFailedEntityWithProps(mock propSvcMock) { mock.EXPECT(). - EntityWithProperties(gomock.Any(), gomock.Any(), gomock.Any()). + EntityWithPropertiesByID(gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errDefault) }