diff --git a/cmd/main/main.go b/cmd/main/main.go index 189919046..fe81d8361 100644 --- a/cmd/main/main.go +++ b/cmd/main/main.go @@ -285,30 +285,28 @@ func main() { workerUID, _ := uuid.NewV4() service := service.NewService( - service.ServiceConfig{ - Repository: repo, - RedisClient: redisClient, - TemporalClient: temporalClient, - ACLClient: &aclClient, - Converter: service.NewConverter(service.ConverterConfig{ - MgmtClient: mgmtPrivateServiceClient, - RedisClient: redisClient, - ACLClient: &aclClient, - Repository: repo, - InstillCoreHost: config.Config.Server.InstillCoreHost, - ComponentStore: compStore, - }), - MgmtPublicServiceClient: mgmtPublicServiceClient, - MgmtPrivateServiceClient: mgmtPrivateServiceClient, - MinioClient: minioClient, - ComponentStore: compStore, - Memory: ms, - WorkerUID: workerUID, - RetentionHandler: nil, - BinaryFetcher: binaryFetcher, - ArtifactPublicServiceClient: artifactPublicServiceClient, - ArtifactPrivateServiceClient: artifactPrivateServiceClient, - }, + repo, + redisClient, + temporalClient, + &aclClient, + service.NewConverter(service.ConverterConfig{ + MgmtClient: mgmtPrivateServiceClient, + RedisClient: redisClient, + ACLClient: &aclClient, + Repository: repo, + InstillCoreHost: config.Config.Server.InstillCoreHost, + ComponentStore: compStore, + }), + mgmtPublicServiceClient, + mgmtPrivateServiceClient, + minioClient, + compStore, + ms, + workerUID, + nil, + binaryFetcher, + artifactPublicServiceClient, + artifactPrivateServiceClient, ) privateGrpcS := grpc.NewServer(grpcServerOpts...) diff --git a/pkg/service/main.go b/pkg/service/main.go index aa1540930..8d0aa0da7 100644 --- a/pkg/service/main.go +++ b/pkg/service/main.go @@ -117,50 +117,45 @@ type service struct { artifactPrivateServiceClient artifactpb.ArtifactPrivateServiceClient } -// ServiceConfig is the configuration for the service -type ServiceConfig struct { - Repository repository.Repository - RedisClient *redis.Client - TemporalClient client.Client - ACLClient acl.ACLClientInterface - Converter Converter - MgmtPublicServiceClient mgmtpb.MgmtPublicServiceClient - MgmtPrivateServiceClient mgmtpb.MgmtPrivateServiceClient - MinioClient miniox.MinioI - ComponentStore *componentstore.Store - Memory memory.MemoryStore - WorkerUID uuid.UUID - RetentionHandler MetadataRetentionHandler - BinaryFetcher external.BinaryFetcher - ArtifactPublicServiceClient artifactpb.ArtifactPublicServiceClient - ArtifactPrivateServiceClient artifactpb.ArtifactPrivateServiceClient -} - // NewService initiates a service instance func NewService( - cfg ServiceConfig, + repository repository.Repository, + redisClient *redis.Client, + temporalClient client.Client, + aclClient acl.ACLClientInterface, + converter Converter, + mgmtPublicServiceClient mgmtpb.MgmtPublicServiceClient, + mgmtPrivateServiceClient mgmtpb.MgmtPrivateServiceClient, + minioClient miniox.MinioI, + componentStore *componentstore.Store, + memory memory.MemoryStore, + workerUID uuid.UUID, + retentionHandler MetadataRetentionHandler, + binaryFetcher external.BinaryFetcher, + artifactPublicServiceClient artifactpb.ArtifactPublicServiceClient, + artifactPrivateServiceClient artifactpb.ArtifactPrivateServiceClient, ) Service { zapLogger, _ := logger.GetZapLogger(context.Background()) - if cfg.RetentionHandler == nil { - cfg.RetentionHandler = NewRetentionHandler() + if retentionHandler == nil { + retentionHandler = NewRetentionHandler() } return &service{ - repository: cfg.Repository, - redisClient: cfg.RedisClient, - temporalClient: cfg.TemporalClient, - mgmtPublicServiceClient: cfg.MgmtPublicServiceClient, - mgmtPrivateServiceClient: cfg.MgmtPrivateServiceClient, - component: cfg.ComponentStore, - aclClient: cfg.ACLClient, - converter: cfg.Converter, - minioClient: cfg.MinioClient, - memory: cfg.Memory, + repository: repository, + redisClient: redisClient, + temporalClient: temporalClient, + mgmtPublicServiceClient: mgmtPublicServiceClient, + mgmtPrivateServiceClient: mgmtPrivateServiceClient, + component: componentStore, + aclClient: aclClient, + converter: converter, + minioClient: minioClient, + memory: memory, log: zapLogger, - workerUID: cfg.WorkerUID, - retentionHandler: cfg.RetentionHandler, - binaryFetcher: cfg.BinaryFetcher, - artifactPublicServiceClient: cfg.ArtifactPublicServiceClient, - artifactPrivateServiceClient: cfg.ArtifactPrivateServiceClient, + workerUID: workerUID, + retentionHandler: retentionHandler, + binaryFetcher: binaryFetcher, + artifactPublicServiceClient: artifactPublicServiceClient, + artifactPrivateServiceClient: artifactPrivateServiceClient, } } diff --git a/pkg/service/pipeline_test.go b/pkg/service/pipeline_test.go index c937deccf..778cf9d06 100644 --- a/pkg/service/pipeline_test.go +++ b/pkg/service/pipeline_test.go @@ -8,17 +8,22 @@ import ( "github.com/go-redis/redismock/v9" "github.com/gofrs/uuid" "github.com/gojuno/minimock/v3" + "github.com/redis/go-redis/v9" "go.temporal.io/sdk/client" "github.com/instill-ai/pipeline-backend/config" + "github.com/instill-ai/pipeline-backend/pkg/acl" "github.com/instill-ai/pipeline-backend/pkg/datamodel" "github.com/instill-ai/pipeline-backend/pkg/external" "github.com/instill-ai/pipeline-backend/pkg/memory" "github.com/instill-ai/pipeline-backend/pkg/mock" "github.com/instill-ai/pipeline-backend/pkg/repository" "github.com/instill-ai/pipeline-backend/pkg/resource" + miniox "github.com/instill-ai/x/minio" componentstore "github.com/instill-ai/pipeline-backend/pkg/component/store" + artifactpb "github.com/instill-ai/protogen-go/artifact/artifact/v1alpha" + mgmtpb "github.com/instill-ai/protogen-go/core/mgmt/v1beta" pb "github.com/instill-ai/protogen-go/vdp/pipeline/v1beta" ) @@ -71,19 +76,17 @@ func TestService_UpdateNamespacePipelineByID(t *testing.T) { }) workerUID, _ := uuid.NewV4() - service := NewService( - ServiceConfig{ - Repository: repo, - RedisClient: redisClient, - TemporalClient: temporalClient, - ACLClient: aclClient, - Converter: converter, - MgmtPrivateServiceClient: mgmtPrivateClient, - MinioClient: nil, - ComponentStore: compStore, - Memory: memory.NewMemoryStore(), - WorkerUID: workerUID, - RetentionHandler: nil, + service := newService( + serviceConfig{ + repository: repo, + redisClient: redisClient, + temporalClient: temporalClient, + aCLClient: aclClient, + converter: converter, + mgmtPrivateServiceClient: mgmtPrivateClient, + componentStore: compStore, + memory: memory.NewMemoryStore(), + workerUID: workerUID, }, ) @@ -119,3 +122,46 @@ func TestService_UpdateNamespacePipelineByID(t *testing.T) { c.Assert(err, quicktest.IsNil) c.Assert(updatedPbPipeline, quicktest.IsNotNil) } + +type serviceConfig struct { + repository repository.Repository + redisClient *redis.Client + temporalClient client.Client + aCLClient acl.ACLClientInterface + converter Converter + mgmtPublicServiceClient mgmtpb.MgmtPublicServiceClient + mgmtPrivateServiceClient mgmtpb.MgmtPrivateServiceClient + minioClient miniox.MinioI + componentStore *componentstore.Store + memory memory.MemoryStore + workerUID uuid.UUID + retentionHandler MetadataRetentionHandler + binaryFetcher external.BinaryFetcher + artifactPublicServiceClient artifactpb.ArtifactPublicServiceClient + artifactPrivateServiceClient artifactpb.ArtifactPrivateServiceClient +} + +// newService is a compact helper to instantiate a new service, which allows us +// to only define the dependencies we'll use in a test. This approach shouldn't +// be used in production code, where we want every dependency to be injected +// every time, so we rely on the compiler to guard us against missing +// dependency injections. +func newService(cfg serviceConfig) Service { + return NewService( + cfg.repository, + cfg.redisClient, + cfg.temporalClient, + cfg.aCLClient, + cfg.converter, + cfg.mgmtPublicServiceClient, + cfg.mgmtPrivateServiceClient, + cfg.minioClient, + cfg.componentStore, + cfg.memory, + cfg.workerUID, + cfg.retentionHandler, + cfg.binaryFetcher, + cfg.artifactPublicServiceClient, + cfg.artifactPrivateServiceClient, + ) +} diff --git a/pkg/service/pipelinerun_test.go b/pkg/service/pipelinerun_test.go index 9e6a1705a..29f35f01d 100644 --- a/pkg/service/pipelinerun_test.go +++ b/pkg/service/pipelinerun_test.go @@ -177,10 +177,10 @@ func TestService_ListPipelineRuns(t *testing.T) { repo := repository.NewRepository(tx, redisClient) - svc := NewService(ServiceConfig{ - Repository: repo, - MgmtPrivateServiceClient: mgmtPrivateClient, - MinioClient: mockMinio, + svc := newService(serviceConfig{ + repository: repo, + mgmtPrivateServiceClient: mgmtPrivateClient, + minioClient: mockMinio, }) ctx := context.Background() @@ -389,11 +389,11 @@ func TestService_ListPipelineRuns_OrgResource(t *testing.T) { repo := repository.NewRepository(tx, redisClient) - svc := NewService(ServiceConfig{ - Repository: repo, - RedisClient: redisClient, - MgmtPrivateServiceClient: mgmtPrivateClient, - MinioClient: mockMinio, + svc := newService(serviceConfig{ + repository: repo, + redisClient: redisClient, + mgmtPrivateServiceClient: mgmtPrivateClient, + minioClient: mockMinio, }) ctx := context.Background() @@ -489,10 +489,10 @@ func TestService_ListPipelineRunsByRequester(t *testing.T) { repo := repository.NewRepository(tx, redisClient) - svc := NewService(ServiceConfig{ - Repository: repo, - MgmtPrivateServiceClient: mgmtPrivateClient, - MinioClient: mockMinio, + svc := newService(serviceConfig{ + repository: repo, + mgmtPrivateServiceClient: mgmtPrivateClient, + minioClient: mockMinio, }) ctx := context.Background()