refactor: templates pkg as per store abstraction (#96)

* refactor: change template repository interface to accept domain structs

* test: fix breaking tests after templates store abstraction refactoring

* refactor: remove redundant return from upsert methods

Author: whoAbhishekSah

api/handlers/v1beta1/grpc_template.go

@@ -6,6 +6,7 @@ import (
 	"github.com/odpf/siren/domain"
 	"github.com/odpf/siren/utils"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
@@ -45,7 +46,9 @@ func (s *GRPCServer) GetTemplateByName(_ context.Context, req *sirenv1beta1.GetT
 	if err != nil {
 		return nil, utils.GRPCLogError(s.logger, codes.Internal, err)
 	}
-
+	if template == nil {
+		return nil, status.Errorf(codes.NotFound, "template not found")
+	}
 	variables := make([]*sirenv1beta1.TemplateVariables, 0)
 	for _, variable := range template.Variables {
 		variables = append(variables, &sirenv1beta1.TemplateVariables{
@@ -79,14 +82,14 @@ func (s *GRPCServer) UpsertTemplate(_ context.Context, req *sirenv1beta1.UpsertT
 			Description: variable.Description,
 		})
 	}
-	payload := &domain.Template{
+	template := &domain.Template{
 		ID:        uint(req.GetId()),
 		Name:      req.GetName(),
 		Body:      req.GetBody(),
 		Tags:      req.GetTags(),
 		Variables: variables,
 	}
-	template, err := s.container.TemplatesService.Upsert(payload)
+	err := s.container.TemplatesService.Upsert(template)
 	if err != nil {
 		return nil, utils.GRPCLogError(s.logger, codes.Internal, err)
 	}

api/handlers/v1beta1/grpc_template_test.go

@@ -151,6 +151,25 @@ func TestGRPCServer_GetTemplateByName(t *testing.T) {
 		mockedTemplatesService.AssertCalled(t, "GetByName", dummyReq.Name)
 	})
 
+	t.Run("should return error code 5 if template does not exist", func(t *testing.T) {
+		mockedTemplatesService := &mocks.TemplatesService{}
+		dummyGRPCServer := GRPCServer{
+			container: &service.Container{
+				TemplatesService: mockedTemplatesService,
+			},
+			logger: log.NewNoop(),
+		}
+		dummyReq := &sirenv1beta1.GetTemplateByNameRequest{
+			Name: "foo",
+		}
+		mockedTemplatesService.
+			On("GetByName", "foo").
+			Return(nil, nil).Once()
+		res, err := dummyGRPCServer.GetTemplateByName(context.Background(), dummyReq)
+		assert.Nil(t, res)
+		assert.EqualError(t, err, "rpc error: code = NotFound desc = template not found")
+	})
+
 	t.Run("should return error code 13 if getting template by name failed", func(t *testing.T) {
 		mockedTemplatesService := &mocks.TemplatesService{}
 		dummyGRPCServer := GRPCServer{
@@ -210,9 +229,7 @@ func TestGRPCServer_UpsertTemplate(t *testing.T) {
 			logger: log.NewNoop(),
 		}
 
-		mockedTemplatesService.
-			On("Upsert", template).
-			Return(template, nil).Once()
+		mockedTemplatesService.On("Upsert", template).Return(nil).Once()
 		res, err := dummyGRPCServer.UpsertTemplate(context.Background(), dummyReq)
 		assert.Nil(t, err)
 		assert.Equal(t, uint64(1), res.GetTemplate().GetId())
@@ -230,9 +247,7 @@ func TestGRPCServer_UpsertTemplate(t *testing.T) {
 			},
 			logger: log.NewNoop(),
 		}
-		mockedTemplatesService.
-			On("Upsert", template).
-			Return(nil, errors.New("random error")).Once()
+		mockedTemplatesService.On("Upsert", template).Return(errors.New("random error")).Once()
 		res, err := dummyGRPCServer.UpsertTemplate(context.Background(), dummyReq)
 		assert.Nil(t, res)
 		assert.EqualError(t, err, "rpc error: code = Internal desc = random error")

domain/templates.go

@@ -23,7 +23,7 @@ type Template struct {
 
 // TemplatesService interface
 type TemplatesService interface {
-	Upsert(*Template) (*Template, error)
+	Upsert(*Template) error
 	Index(string) ([]Template, error)
 	GetByName(string) (*Template, error)
 	Delete(string) error

mocks/TemplatesService.go

@@ -3,7 +3,6 @@ package templates
 import (
 	"github.com/odpf/siren/domain"
 	"github.com/odpf/siren/store"
-	"github.com/odpf/siren/store/model"
 )
 
 // Service handles business logic
@@ -20,38 +19,16 @@ func (service Service) Migrate() error {
 	return service.repository.Migrate()
 }
 
-func (service Service) Upsert(template *domain.Template) (*domain.Template, error) {
-	t := &model.Template{}
-	t, err := t.FromDomain(template)
-	if err != nil {
-		return nil, err
-	}
-	upsertedTemplate, err := service.repository.Upsert(t)
-	if err != nil {
-		return nil, err
-	}
-	return upsertedTemplate.ToDomain()
+func (service Service) Upsert(template *domain.Template) error {
+	return service.repository.Upsert(template)
 }
 
 func (service Service) Index(tag string) ([]domain.Template, error) {
-	templates, err := service.repository.Index(tag)
-	if err != nil {
-		return nil, err
-	}
-	domainTemplates := make([]domain.Template, 0, len(templates))
-	for i := 0; i < len(templates); i++ {
-		t, _ := templates[i].ToDomain()
-		domainTemplates = append(domainTemplates, *t)
-	}
-	return domainTemplates, nil
+	return service.repository.Index(tag)
 }
 
 func (service Service) GetByName(name string) (*domain.Template, error) {
-	template, err := service.repository.GetByName(name)
-	if err != nil || template == nil {
-		return nil, err
-	}
-	return template.ToDomain()
+	return service.repository.GetByName(name)
 }
 
 func (service Service) Delete(name string) error {

pkg/templates/repository_mock.go

@@ -17,7 +17,7 @@ type Template struct {
 	Variables string         `gorm:"type:jsonb" sql:"type:jsonb" `
 }
 
-func (template *Template) FromDomain(t *domain.Template) (*Template, error) {
+func (template *Template) FromDomain(t *domain.Template) error {
 	template.ID = t.ID
 	template.CreatedAt = t.CreatedAt
 	template.UpdatedAt = t.UpdatedAt
@@ -26,10 +26,10 @@ func (template *Template) FromDomain(t *domain.Template) (*Template, error) {
 	template.Body = t.Body
 	jsonString, err := json.Marshal(t.Variables)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	template.Variables = string(jsonString)
-	return template, nil
+	return nil
 }
 
 func (template *Template) ToDomain() (*domain.Template, error) {