From 740f02114cccbdd104993790f460366fa648d864 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 20 Aug 2024 15:15:23 +0200 Subject: [PATCH] Add a property service Adds a service that either returns cached properties for a very short amount of time or contacts the provider to fetch updated service. The service also marshalls the properties to the database properly. Fixes: #4171 --- .../properties/service/mock/service.go | 102 +++ .../entities/properties/service/service.go | 288 +++++++ .../properties/service/service_test.go | 716 ++++++++++++++++++ 3 files changed, 1106 insertions(+) create mode 100644 internal/entities/properties/service/mock/service.go create mode 100644 internal/entities/properties/service/service.go create mode 100644 internal/entities/properties/service/service_test.go diff --git a/internal/entities/properties/service/mock/service.go b/internal/entities/properties/service/mock/service.go new file mode 100644 index 0000000000..a6546379ff --- /dev/null +++ b/internal/entities/properties/service/mock/service.go @@ -0,0 +1,102 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: ./service.go +// +// Generated by this command: +// +// mockgen -package mock_service -destination=./mock/service.go -source=./service.go +// + +// Package mock_service is a generated GoMock package. +package mock_service + +import ( + context "context" + reflect "reflect" + + uuid "github.com/google/uuid" + properties "github.com/stacklok/minder/internal/entities/properties" + v1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" + v10 "github.com/stacklok/minder/pkg/providers/v1" + gomock "go.uber.org/mock/gomock" +) + +// MockPropertiesService is a mock of PropertiesService interface. +type MockPropertiesService struct { + ctrl *gomock.Controller + recorder *MockPropertiesServiceMockRecorder +} + +// MockPropertiesServiceMockRecorder is the mock recorder for MockPropertiesService. +type MockPropertiesServiceMockRecorder struct { + mock *MockPropertiesService +} + +// NewMockPropertiesService creates a new mock instance. +func NewMockPropertiesService(ctrl *gomock.Controller) *MockPropertiesService { + mock := &MockPropertiesService{ctrl: ctrl} + mock.recorder = &MockPropertiesServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPropertiesService) EXPECT() *MockPropertiesServiceMockRecorder { + return m.recorder +} + +// RetrieveAllProperties mocks base method. +func (m *MockPropertiesService) RetrieveAllProperties(ctx context.Context, provider v10.Provider, projectId uuid.UUID, lookupProperties *properties.Properties, entType v1.Entity) (*properties.Properties, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RetrieveAllProperties", ctx, provider, projectId, lookupProperties, entType) + ret0, _ := ret[0].(*properties.Properties) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RetrieveAllProperties indicates an expected call of RetrieveAllProperties. +func (mr *MockPropertiesServiceMockRecorder) RetrieveAllProperties(ctx, provider, projectId, lookupProperties, entType any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetrieveAllProperties", reflect.TypeOf((*MockPropertiesService)(nil).RetrieveAllProperties), ctx, provider, projectId, lookupProperties, entType) +} + +// RetrieveProperty mocks base method. +func (m *MockPropertiesService) RetrieveProperty(ctx context.Context, provider v10.Provider, projectId uuid.UUID, lookupProperties *properties.Properties, entType v1.Entity, key string) (*properties.Property, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RetrieveProperty", ctx, provider, projectId, lookupProperties, entType, key) + ret0, _ := ret[0].(*properties.Property) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RetrieveProperty indicates an expected call of RetrieveProperty. +func (mr *MockPropertiesServiceMockRecorder) RetrieveProperty(ctx, provider, projectId, lookupProperties, entType, key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetrieveProperty", reflect.TypeOf((*MockPropertiesService)(nil).RetrieveProperty), ctx, provider, projectId, lookupProperties, entType, key) +} + +// SaveAllProperties mocks base method. +func (m *MockPropertiesService) SaveAllProperties(ctx context.Context, entityID uuid.UUID, props *properties.Properties) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SaveAllProperties", ctx, entityID, props) + ret0, _ := ret[0].(error) + return ret0 +} + +// SaveAllProperties indicates an expected call of SaveAllProperties. +func (mr *MockPropertiesServiceMockRecorder) SaveAllProperties(ctx, entityID, props any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveAllProperties", reflect.TypeOf((*MockPropertiesService)(nil).SaveAllProperties), ctx, entityID, props) +} + +// SaveProperty mocks base method. +func (m *MockPropertiesService) SaveProperty(ctx context.Context, entityID uuid.UUID, key string, prop *properties.Property) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SaveProperty", ctx, entityID, key, prop) + ret0, _ := ret[0].(error) + return ret0 +} + +// SaveProperty indicates an expected call of SaveProperty. +func (mr *MockPropertiesServiceMockRecorder) SaveProperty(ctx, entityID, key, prop any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveProperty", reflect.TypeOf((*MockPropertiesService)(nil).SaveProperty), ctx, entityID, key, prop) +} diff --git a/internal/entities/properties/service/service.go b/internal/entities/properties/service/service.go new file mode 100644 index 0000000000..959d1b06df --- /dev/null +++ b/internal/entities/properties/service/service.go @@ -0,0 +1,288 @@ +// +// Copyright 2024 Stacklok, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package service provides a service to interact with properties of an entity +package service + +import ( + "context" + "database/sql" + "errors" + "time" + + "github.com/google/uuid" + + "github.com/stacklok/minder/internal/db" + "github.com/stacklok/minder/internal/engine/entities" + "github.com/stacklok/minder/internal/entities/properties" + minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" + v1 "github.com/stacklok/minder/pkg/providers/v1" +) + +//go:generate go run go.uber.org/mock/mockgen -package mock_$GOPACKAGE -destination=./mock/$GOFILE -source=./$GOFILE + +const ( + // propertiesCacheTimeout is the default timeout for the cache of properties + propertiesCacheTimeout = time.Duration(60) * time.Second + // bypassCacheTimeout is a special value to bypass the cache timeout + // it is not exported from the package and should only be used for testing + bypassCacheTimeout = time.Duration(-1) +) + +// PropertiesService is the interface for the properties service +type PropertiesService interface { + // RetrieveAllProperties fetches all properties for the given entity + RetrieveAllProperties( + ctx context.Context, provider v1.Provider, projectId uuid.UUID, + lookupProperties *properties.Properties, entType minderv1.Entity, + ) (*properties.Properties, error) + // RetrieveProperty fetches a single property for the given entity + RetrieveProperty( + ctx context.Context, provider v1.Provider, projectId uuid.UUID, + lookupProperties *properties.Properties, entType minderv1.Entity, key string, + ) (*properties.Property, error) + // SaveAllProperties saves all properties for the given entity + SaveAllProperties(ctx context.Context, entityID uuid.UUID, props *properties.Properties) error + // SaveProperty saves a single property for the given entity + SaveProperty(ctx context.Context, entityID uuid.UUID, key string, prop *properties.Property) error +} + +type propertiesServiceOption func(*propertiesService) + +type propertiesService struct { + store db.Store + entityTimeout time.Duration +} + +// WithEntityTimeout sets the timeout for the cache of properties +func WithEntityTimeout(timeout time.Duration) propertiesServiceOption { + return func(ps *propertiesService) { + ps.entityTimeout = timeout + } +} + +// NewPropertiesService creates a new properties service +func NewPropertiesService( + store db.Store, + opts ...propertiesServiceOption, +) PropertiesService { + ps := &propertiesService{ + store: store, + entityTimeout: propertiesCacheTimeout, + } + + for _, opt := range opts { + opt(ps) + } + + return ps +} + +// RetrieveAllProperties fetches a single property for the given entity +func (ps *propertiesService) RetrieveAllProperties( + ctx context.Context, provider v1.Provider, projectId uuid.UUID, + lookupProperties *properties.Properties, entType minderv1.Entity, +) (*properties.Properties, error) { + // fetch the entity first. If there's no entity, there's no properties, go straight to provider + entID, err := ps.getEntityIdByProperties(ctx, projectId, lookupProperties, entType) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + + var dbProps []db.Property + if entID != uuid.Nil { + // fetch properties from db + dbProps, err = ps.store.GetAllPropertiesForEntity(ctx, entID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + } + + // if exists and not expired, turn into our model + if len(dbProps) > 0 && ps.areDatabasePropertiesValid(dbProps) { + // TODO: instead of a hard error, should we just re-fetch from provider? + return dbPropsToModel(dbProps) + } + + // if not, fetch from provider + props, err := provider.FetchAllProperties(ctx, lookupProperties, entType) + if err != nil { + return nil, err + } + + return props, nil +} + +// RetrieveProperty fetches a single property for the given entity +func (ps *propertiesService) RetrieveProperty( + ctx context.Context, provider v1.Provider, projectId uuid.UUID, + lookupProperties *properties.Properties, entType minderv1.Entity, key string, +) (*properties.Property, error) { + // fetch the entity first. If there's no entity, there's no properties, go straight to provider + entID, err := ps.getEntityIdByProperties(ctx, projectId, lookupProperties, entType) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + + // fetch properties from db + var dbProp db.Property + if entID != uuid.Nil { + dbProp, err = ps.store.GetProperty(ctx, db.GetPropertyParams{ + EntityID: entID, + Key: key, + }) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, err + } + } + + // if exists, turn into our model + if ps.isDatabasePropertyValid(dbProp) { + return dbPropToModel(dbProp) + } + + // if not, fetch from provider + prop, err := provider.FetchProperty(ctx, lookupProperties, entType, key) + if err != nil { + return nil, err + } + + return prop, nil +} + +func (ps *propertiesService) getEntityIdByProperties( + ctx context.Context, projectId uuid.UUID, + props *properties.Properties, entType minderv1.Entity, +) (uuid.UUID, error) { + // TODO: Add more ways to look up a property, e.g. by the upstream ID + name := props.GetProperty(properties.PropertyName) + if name != nil { + return ps.getEntityIdByName(ctx, projectId, name.GetString(), entType) + } + + // returning nil ID and nil error would make us just go to the provider. Slow, but we'd continue. + return uuid.Nil, nil +} + +func (ps *propertiesService) getEntityIdByName( + ctx context.Context, projectId uuid.UUID, + name string, entType minderv1.Entity, +) (uuid.UUID, error) { + ent, err := ps.store.GetEntityByName(ctx, db.GetEntityByNameParams{ + ProjectID: projectId, + Name: name, + EntityType: entities.EntityTypeToDB(entType), + }) + if err != nil { + return uuid.Nil, err + } + + return ent.ID, nil +} + +func (ps *propertiesService) SaveAllProperties( + ctx context.Context, entityID uuid.UUID, props *properties.Properties, +) error { + return ps.store.WithTransactionErr(func(qtx db.ExtendQuerier) error { + return replaceProperties(ctx, qtx, entityID, props) + }) +} + +func replaceProperties(ctx context.Context, txq db.ExtendQuerier, entityID uuid.UUID, props *properties.Properties) error { + err := txq.DeleteAllPropertiesForEntity(ctx, entityID) + if err != nil { + return err + } + + for key, prop := range props.Iterate() { + _, err := txq.UpsertPropertyValueV1(ctx, db.UpsertPropertyValueV1Params{ + EntityID: entityID, + Key: key, + Value: prop.RawValue(), + }) + if err != nil { + return err + } + } + + return nil +} + +func (ps *propertiesService) SaveProperty(ctx context.Context, entityID uuid.UUID, key string, prop *properties.Property) error { + return ps.store.WithTransactionErr(func(qtx db.ExtendQuerier) error { + return upsertProperty(ctx, qtx, entityID, key, prop) + }) +} + +func upsertProperty(ctx context.Context, txq db.ExtendQuerier, entityID uuid.UUID, key string, prop *properties.Property) error { + if prop == nil { + return txq.DeleteProperty(ctx, db.DeletePropertyParams{ + EntityID: entityID, + Key: key, + }) + } + + _, err := txq.UpsertPropertyValueV1(ctx, db.UpsertPropertyValueV1Params{ + EntityID: entityID, + Key: key, + Value: prop.RawValue(), + }) + return err +} + +func (ps *propertiesService) areDatabasePropertiesValid(dbProps []db.Property) bool { + // if the all the properties are to be valid, neither must be older than + // the cache timeout + for _, prop := range dbProps { + if !ps.isDatabasePropertyValid(prop) { + return false + } + } + return true +} + +func (ps *propertiesService) isDatabasePropertyValid(dbProp db.Property) bool { + if ps.entityTimeout == bypassCacheTimeout { + // this is mostly for testing + return false + } + return time.Since(dbProp.UpdatedAt) < ps.entityTimeout +} + +func dbPropsToModel(dbProps []db.Property) (*properties.Properties, error) { + propMap := make(map[string]any) + + // TODO: should we change the property API to include a Set + // and rather move the construction from a map to a separate method? + // this double iteration is not ideal + for _, prop := range dbProps { + anyVal, err := db.PropValueFromDbV1(prop.Value) + if err != nil { + return nil, err + } + propMap[prop.Key] = anyVal + } + + return properties.NewProperties(propMap) +} + +func dbPropToModel(dbProp db.Property) (*properties.Property, error) { + anyVal, err := db.PropValueFromDbV1(dbProp.Value) + if err != nil { + return nil, err + } + + return properties.NewProperty(anyVal) +} diff --git a/internal/entities/properties/service/service_test.go b/internal/entities/properties/service/service_test.go new file mode 100644 index 0000000000..587d704d64 --- /dev/null +++ b/internal/entities/properties/service/service_test.go @@ -0,0 +1,716 @@ +// +// Copyright 2024 Stacklok, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "context" + "database/sql" + "encoding/json" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/minder/internal/db" + "github.com/stacklok/minder/internal/db/embedded" + "github.com/stacklok/minder/internal/engine/entities" + "github.com/stacklok/minder/internal/entities/properties" + "github.com/stacklok/minder/internal/providers/github" + mock_github "github.com/stacklok/minder/internal/providers/github/mock" + "github.com/stacklok/minder/internal/util/rand" + minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" +) + +type githubMockBuilder func(*gomock.Controller) *mock_github.MockGitHub + +func newGithubMock(opts ...func(mock *mock_github.MockGitHub)) githubMockBuilder { + return func(ctrl *gomock.Controller) *mock_github.MockGitHub { + mock := mock_github.NewMockGitHub(ctrl) + for _, opt := range opts { + opt(mock) + } + return mock + } +} + +func withUpstreamRepoProperties(repoProperties map[string]any, entType minderv1.Entity) func(mock *mock_github.MockGitHub) { + return func(mock *mock_github.MockGitHub) { + props, err := properties.NewProperties(repoProperties) + if err != nil { + panic(err) + } + mock.EXPECT(). + FetchAllProperties(gomock.Any(), gomock.Any(), entType). + Return(props, nil) + } +} + +func withUpstreamRepoProperty(key string, val any, entType minderv1.Entity) func(mock *mock_github.MockGitHub) { + return func(mock *mock_github.MockGitHub) { + prop, err := properties.NewProperty(val) + if err != nil { + panic(err) + } + mock.EXPECT(). + FetchProperty(gomock.Any(), gomock.Any(), entType, key). + Return(prop, nil) + } +} + +func insertProperties(ctx context.Context, t *testing.T, store db.Store, entID uuid.UUID, props *properties.Properties) { + t.Helper() + + for key, prop := range props.Iterate() { + _, err := store.UpsertPropertyValueV1(ctx, db.UpsertPropertyValueV1Params{ + EntityID: entID, + Key: key, + Value: prop.RawValue(), + }) + require.NoError(t, err) + } +} + +func insertPropertiesFromMap(ctx context.Context, t *testing.T, store db.Store, entID uuid.UUID, propMap map[string]any) { + t.Helper() + + for key, val := range propMap { + _, err := store.UpsertPropertyValueV1(ctx, db.UpsertPropertyValueV1Params{ + EntityID: entID, + Key: key, + Value: val, + }) + require.NoError(t, err) + } +} + +type fetchParams struct { + entType minderv1.Entity + entName string + + providerID uuid.UUID + projectID uuid.UUID +} + +type testCtx struct { + testQueries db.Store + dbProj db.Project + ghAppProvider db.Provider +} + +func createTestCtx(ctx context.Context, t *testing.T) testCtx { + t.Helper() + + testQueries, td, err := embedded.GetFakeStore() + require.NoError(t, err, "expected no error when creating embedded store") + t.Cleanup(td) + + seed := time.Now().UnixNano() + dbProj, err := testQueries.CreateProject(ctx, db.CreateProjectParams{ + Name: rand.RandomName(seed), + Metadata: []byte(`{}`), + }) + require.NoError(t, err) + + ghAppProvider, err := testQueries.CreateProvider(context.Background(), + db.CreateProviderParams{ + Name: rand.RandomName(seed), + ProjectID: dbProj.ID, + Class: db.ProviderClassGithubApp, + Implements: []db.ProviderType{db.ProviderTypeGithub, db.ProviderTypeGit}, + AuthFlows: []db.AuthorizationFlow{db.AuthorizationFlowUserInput}, + Definition: json.RawMessage("{}"), + }) + require.NoError(t, err) + + return testCtx{ + testQueries: testQueries, + dbProj: dbProj, + ghAppProvider: ghAppProvider, + } +} + +func TestPropertiesService_SaveProperty(t *testing.T) { + t.Parallel() + + seed := time.Now().UnixNano() + + scenarios := []struct { + name string + key string + val any + dbSetup func(t *testing.T, entityID uuid.UUID, store db.Store) + checkFn func(t *testing.T, props *properties.Property) + }{ + { + name: "Save a new property", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: 123, + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + key: properties.RepoPropertyIsFork, + val: true, + checkFn: func(t *testing.T, props *properties.Property) { + t.Helper() + + require.Equal(t, props.GetBool(), true) + }, + }, + { + name: "Update an existing property", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + key: github.RepoPropertyId, + val: int64(456), + checkFn: func(t *testing.T, props *properties.Property) { + t.Helper() + + require.Equal(t, props.GetInt64(), int64(456)) + }, + }, + { + name: "The property no longer exists", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: 123, + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + key: properties.RepoPropertyIsPrivate, + val: nil, + checkFn: func(t *testing.T, props *properties.Property) { + t.Helper() + + require.Nil(t, props) + }, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + tctx := createTestCtx(ctx, t) + + for _, tt := range scenarios { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + ent, err := tctx.testQueries.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: rand.RandomName(seed), + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + }) + + require.NoError(t, err) + propSvc := NewPropertiesService(tctx.testQueries) + + var prop *properties.Property + if tt.val != nil { + prop, err = properties.NewProperty(tt.val) + require.NoError(t, err) + } + + err = propSvc.SaveProperty(ctx, ent.ID, tt.key, prop) + require.NoError(t, err) + + dbProp, err := tctx.testQueries.GetProperty(ctx, db.GetPropertyParams{ + EntityID: ent.ID, + Key: tt.key, + }) + if tt.val == nil { + require.ErrorIs(t, err, sql.ErrNoRows) + return + } + + require.NoError(t, err) + updatedProp, err := dbPropToModel(dbProp) + require.NoError(t, err) + tt.checkFn(t, updatedProp) + }) + } +} + +func TestPropertiesService_SaveAllProperties(t *testing.T) { + t.Parallel() + + seed := time.Now().UnixNano() + + scenarios := []struct { + name string + dbSetup func(t *testing.T, entityID uuid.UUID, store db.Store) + props map[string]any + checkFn func(t *testing.T, props *properties.Properties) + }{ + { + name: "Replace all properties", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + props: map[string]any{ + properties.RepoPropertyIsPrivate: false, + github.RepoPropertyId: int64(456), + }, + checkFn: func(t *testing.T, props *properties.Properties) { + t.Helper() + + require.Equal(t, props.GetProperty(properties.RepoPropertyIsPrivate).GetBool(), false) + require.Equal(t, props.GetProperty(github.RepoPropertyId).GetInt64(), int64(456)) + }, + }, + { + name: "One less property upstream", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + props: map[string]any{ + github.RepoPropertyId: int64(456), + }, + checkFn: func(t *testing.T, props *properties.Properties) { + t.Helper() + + require.Nil(t, props.GetProperty(properties.RepoPropertyIsPrivate)) + require.Equal(t, props.GetProperty(github.RepoPropertyId).GetInt64(), int64(456)) + }, + }, + { + name: "One more property upstream", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + props: map[string]any{ + properties.RepoPropertyIsPrivate: false, + properties.RepoPropertyIsFork: true, + github.RepoPropertyId: int64(456), + }, + checkFn: func(t *testing.T, props *properties.Properties) { + t.Helper() + + require.Equal(t, props.GetProperty(properties.RepoPropertyIsPrivate).GetBool(), false) + require.Equal(t, props.GetProperty(properties.RepoPropertyIsFork).GetBool(), true) + require.Equal(t, props.GetProperty(github.RepoPropertyId).GetInt64(), int64(456)) + }, + }, + { + name: "No properties upstream", + dbSetup: func(t *testing.T, entityID uuid.UUID, store db.Store) { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: 123, + } + insertPropertiesFromMap(context.TODO(), t, store, entityID, propMap) + }, + props: map[string]any{}, + checkFn: func(t *testing.T, props *properties.Properties) { + t.Helper() + + count := 0 + for range props.Iterate() { + count++ + } + require.Equal(t, count, 0) + }, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + tctx := createTestCtx(ctx, t) + + for _, tt := range scenarios { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + ent, err := tctx.testQueries.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: rand.RandomName(seed), + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + }) + + require.NoError(t, err) + propSvc := NewPropertiesService(tctx.testQueries) + + props, err := properties.NewProperties(tt.props) + require.NoError(t, err) + + err = propSvc.SaveAllProperties(ctx, ent.ID, props) + require.NoError(t, err) + + dbProps, err := tctx.testQueries.GetAllPropertiesForEntity(ctx, ent.ID) + require.NoError(t, err) + + updatedProps, err := dbPropsToModel(dbProps) + require.NoError(t, err) + tt.checkFn(t, updatedProps) + }) + } +} + +func TestPropertiesService_RetrieveProperty(t *testing.T) { + t.Parallel() + + seed := time.Now().UnixNano() + + scenarios := []struct { + name string + propName string + dbSetup func(t *testing.T, store db.Store, params fetchParams) + githubSetup func(params fetchParams) githubMockBuilder + params fetchParams + expectErr string + checkResult func(t *testing.T, props *properties.Property) + opts []propertiesServiceOption + }{ + { + name: "No cache, fetch from provider", + propName: properties.RepoPropertyIsPrivate, + dbSetup: func(_ *testing.T, _ db.Store, _ fetchParams) { + }, + githubSetup: func(params fetchParams) githubMockBuilder { + return newGithubMock( + withUpstreamRepoProperty(properties.RepoPropertyIsPrivate, true, params.entType), + ) + }, + params: fetchParams{ + entType: minderv1.Entity_ENTITY_REPOSITORIES, + entName: rand.RandomName(seed), + }, + checkResult: func(t *testing.T, prop *properties.Property) { + t.Helper() + require.Equal(t, prop.GetBool(), true) + }, + }, + { + name: "Cache miss, fetch from provider", + propName: github.RepoPropertyId, + dbSetup: func(t *testing.T, store db.Store, params fetchParams) { + t.Helper() + ent, err := store.CreateEntity(context.TODO(), db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(params.entType), + Name: params.entName, + ProjectID: params.projectID, + ProviderID: params.providerID, + }) + require.NoError(t, err) + + // these are different than tt.params.properties + oldPropMap := map[string]any{ + github.RepoPropertyId: int64(1234), + } + insertPropertiesFromMap(context.TODO(), t, store, ent.ID, oldPropMap) + }, + githubSetup: func(params fetchParams) githubMockBuilder { + t.Helper() + return newGithubMock( + withUpstreamRepoProperty(github.RepoPropertyId, int64(123), params.entType), + ) + }, + params: fetchParams{ + entType: minderv1.Entity_ENTITY_REPOSITORIES, + entName: rand.RandomName(seed), + }, + checkResult: func(t *testing.T, prop *properties.Property) { + t.Helper() + require.Equal(t, prop.GetInt64(), int64(123)) + }, + opts: []propertiesServiceOption{ + WithEntityTimeout(bypassCacheTimeout), + }, + }, + { + name: "Cache hit, fetch from cache", + propName: github.RepoPropertyId, + dbSetup: func(t *testing.T, store db.Store, params fetchParams) { + t.Helper() + + ent, err := store.CreateEntity(context.TODO(), db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(params.entType), + Name: params.entName, + ProjectID: params.projectID, + ProviderID: params.providerID, + }) + require.NoError(t, err) + + propMap := map[string]any{ + github.RepoPropertyId: int64(123), + } + props, err := properties.NewProperties(propMap) + require.NoError(t, err) + insertProperties(context.TODO(), t, store, ent.ID, props) + }, + githubSetup: func(_ fetchParams) githubMockBuilder { + return newGithubMock() + }, + params: fetchParams{ + entType: minderv1.Entity_ENTITY_REPOSITORIES, + entName: rand.RandomName(seed), + }, + checkResult: func(t *testing.T, prop *properties.Property) { + t.Helper() + + require.Equal(t, prop.GetInt64(), int64(123)) + }, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + tctx := createTestCtx(ctx, t) + + for _, tt := range scenarios { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + tt.params.providerID = tctx.ghAppProvider.ID + tt.params.projectID = tctx.dbProj.ID + + githubSetup := tt.githubSetup(tt.params) + githubMock := githubSetup(ctrl) + + tt.dbSetup(t, tctx.testQueries, tt.params) + + propSvc := NewPropertiesService(tctx.testQueries, tt.opts...) + + getByProps, err := properties.NewProperties(map[string]any{ + properties.PropertyName: tt.params.entName, + }) + require.NoError(t, err) + + gotProps, err := propSvc.RetrieveProperty(ctx, githubMock, tctx.dbProj.ID, getByProps, tt.params.entType, tt.propName) + + if tt.expectErr != "" { + require.Contains(t, err.Error(), tt.expectErr) + return + } + + require.NoError(t, err) + tt.checkResult(t, gotProps) + }) + } +} + +func TestPropertiesService_RetrieveAllProperties(t *testing.T) { + t.Parallel() + + seed := time.Now().UnixNano() + + scenarios := []struct { + name string + dbSetup func(t *testing.T, store db.Store, params fetchParams) + githubSetup func(t *testing.T, params fetchParams) githubMockBuilder + params fetchParams + expectErr string + checkResult func(t *testing.T, props *properties.Properties) + opts []propertiesServiceOption + }{ + { + name: "No cache, fetch from provider", + dbSetup: func(_ *testing.T, _ db.Store, _ fetchParams) { + }, + githubSetup: func(t *testing.T, params fetchParams) githubMockBuilder { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + return newGithubMock( + withUpstreamRepoProperties(propMap, params.entType), + ) + }, + params: fetchParams{ + entType: minderv1.Entity_ENTITY_REPOSITORIES, + entName: rand.RandomName(seed), + }, + checkResult: func(t *testing.T, props *properties.Properties) { + t.Helper() + + require.Equal(t, props.GetProperty(properties.RepoPropertyIsPrivate).GetBool(), true) + require.Equal(t, props.GetProperty(github.RepoPropertyId).GetInt64(), int64(123)) + }, + }, + { + name: "Cache miss, fetch from provider", + dbSetup: func(t *testing.T, store db.Store, params fetchParams) { + t.Helper() + + ent, err := store.CreateEntity(context.TODO(), db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(params.entType), + Name: params.entName, + ProjectID: params.projectID, + ProviderID: params.providerID, + }) + require.NoError(t, err) + + // these are different than the returned properties in github setup which we also + // check for + oldPropMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(1234), + } + insertPropertiesFromMap(context.TODO(), t, store, ent.ID, oldPropMap) + }, + githubSetup: func(t *testing.T, params fetchParams) githubMockBuilder { + t.Helper() + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + return newGithubMock( + withUpstreamRepoProperties(propMap, params.entType), + ) + }, + params: fetchParams{ + entType: minderv1.Entity_ENTITY_REPOSITORIES, + entName: rand.RandomName(seed), + }, + checkResult: func(t *testing.T, props *properties.Properties) { + t.Helper() + + require.Equal(t, props.GetProperty(properties.RepoPropertyIsPrivate).GetBool(), true) + require.Equal(t, props.GetProperty(github.RepoPropertyId).GetInt64(), int64(123)) + }, + opts: []propertiesServiceOption{ + WithEntityTimeout(bypassCacheTimeout), + }, + }, + { + name: "Cache hit, fetch from cache", + dbSetup: func(t *testing.T, store db.Store, params fetchParams) { + t.Helper() + + ent, err := store.CreateEntity(context.TODO(), db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(params.entType), + Name: params.entName, + ProjectID: params.projectID, + ProviderID: params.providerID, + }) + require.NoError(t, err) + + propMap := map[string]any{ + properties.RepoPropertyIsPrivate: true, + github.RepoPropertyId: int64(123), + } + props, err := properties.NewProperties(propMap) + require.NoError(t, err) + insertProperties(context.TODO(), t, store, ent.ID, props) + }, + githubSetup: func(t *testing.T, _ fetchParams) githubMockBuilder { + t.Helper() + + return newGithubMock() + }, + params: fetchParams{ + entType: minderv1.Entity_ENTITY_REPOSITORIES, + entName: "testorg/testrepo", + }, + checkResult: func(t *testing.T, props *properties.Properties) { + t.Helper() + + require.Equal(t, props.GetProperty(properties.RepoPropertyIsPrivate).GetBool(), true) + require.Equal(t, props.GetProperty(github.RepoPropertyId).GetInt64(), int64(123)) + }, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + tctx := createTestCtx(ctx, t) + + for _, tt := range scenarios { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + tt.params.providerID = tctx.ghAppProvider.ID + tt.params.projectID = tctx.dbProj.ID + + githubSetup := tt.githubSetup(t, tt.params) + githubMock := githubSetup(ctrl) + + tt.dbSetup(t, tctx.testQueries, tt.params) + + propSvc := NewPropertiesService(tctx.testQueries, tt.opts...) + + getByProps, err := properties.NewProperties(map[string]any{ + properties.PropertyName: tt.params.entName, + }) + require.NoError(t, err) + + gotProps, err := propSvc.RetrieveAllProperties(ctx, githubMock, tctx.dbProj.ID, getByProps, tt.params.entType) + + if tt.expectErr != "" { + require.Contains(t, err.Error(), tt.expectErr) + return + } + + require.NoError(t, err) + tt.checkResult(t, gotProps) + }) + } +}