Skip to content

Commit

Permalink
refactor: replace gorm with jmoiron/sqlx (#65)
Browse files Browse the repository at this point in the history 
* feat: add db sql migrations

* feat: update migration to golang-migrate

* feat: update template repository with sqlx

* feat: update tag repository with sqlx

* fix: tests and repository mock

* refactor(postgres): create a new postgres client abstraction

* refactor(postgres): restructure repository

* refactor: clean up comment, fixing lint and format

* feat(postgres): add test to model's methods

* refactor(postgres): rename migrations file name to plural

* fix(postgres): fixing unstable test

* feat(postgres): update migrations with go embed

* refactor(postgres): move model to tag_model

* docs: explain a brief behaviour of tag template in postgres

* refactor(postgres): simplify repository test

* refactor(postgres): update errors to fmt.Errorf

* feat(postgres): bump up pgx to v4

* refactor(postgres): convert domain to model with constructor

* refactor(postgres): update naming, singular default to domain, singular+suffix model default to model

* refactor(tag): update error naming to Err prefix

* refactor(tag): simplify method params

* fix(tag): fix api behaviour

* refactor(tag): revert error to go convention

* refactor(postgres): rename templates to tag_templates and fields to tag_template_fields
  • Loading branch information
@mabdh
mabdh authored Jan 24, 2022
1 parent 7b1d69a commit 068d750
Show file tree
Hide file tree
Showing 35 changed files with 3,573 additions and 1,624 deletions.
10 changes: 5 additions & 5 deletions api/handlers/tag_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (h *TagHandler) Create(w http.ResponseWriter, r *http.Request) {
return
}

err := h.service.Create(&requestBody)
err := h.service.Create(r.Context(), &requestBody)
if errors.As(err, new(tag.DuplicateError)) {
writeJSONError(w, http.StatusConflict, err.Error())
return
Expand Down Expand Up @@ -89,7 +89,7 @@ func (h *TagHandler) GetByRecord(w http.ResponseWriter, r *http.Request) {
writeJSONError(w, http.StatusBadRequest, errEmptyRecordURN.Error())
return
}
tags, err := h.service.GetByRecord(recordType, recordURN)
tags, err := h.service.GetByRecord(r.Context(), recordType, recordURN)
if err != nil {
internalServerError(w, h.logger, fmt.Sprintf("error getting record tags: %s", err.Error()))
return
Expand Down Expand Up @@ -121,7 +121,7 @@ func (h *TagHandler) FindByRecordAndTemplate(w http.ResponseWriter, r *http.Requ
writeJSONError(w, http.StatusBadRequest, errEmptyTemplateURN.Error())
return
}
tags, err := h.service.FindByRecordAndTemplate(recordType, recordURN, templateURN)
tags, err := h.service.FindByRecordAndTemplate(r.Context(), recordType, recordURN, templateURN)
if errors.As(err, new(tag.NotFoundError)) || errors.As(err, new(tag.TemplateNotFoundError)) {
writeJSONError(w, http.StatusNotFound, err.Error())
return
Expand Down Expand Up @@ -167,7 +167,7 @@ func (h *TagHandler) Update(w http.ResponseWriter, r *http.Request) {
requestBody.RecordURN = recordURN
requestBody.RecordType = recordType
requestBody.TemplateURN = templateURN
err := h.service.Update(&requestBody)
err := h.service.Update(r.Context(), &requestBody)
if errors.As(err, new(tag.NotFoundError)) {
writeJSONError(w, http.StatusNotFound, err.Error())
return
Expand Down Expand Up @@ -203,7 +203,7 @@ func (h *TagHandler) Delete(w http.ResponseWriter, r *http.Request) {
return
}

err := h.service.Delete(recordType, recordURN, templateURN)
err := h.service.Delete(r.Context(), recordType, recordURN, templateURN)
if errors.As(err, new(tag.TemplateNotFoundError)) {
writeJSONError(w, http.StatusNotFound, err.Error())
return
Expand Down
89 changes: 38 additions & 51 deletions api/handlers/tag_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import (
"testing"

"github.com/odpf/columbus/api/handlers"
"github.com/odpf/columbus/lib/mock"
libmock "github.com/odpf/columbus/lib/mock"
"github.com/odpf/columbus/tag"
"github.com/odpf/columbus/tag/mocks"

"github.com/gorilla/mux"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
)

Expand All @@ -27,7 +28,7 @@ type TagHandlerTestSuite struct {

func (s *TagHandlerTestSuite) TestNewHandler() {
s.Run("should return handler and nil if service is not nil", func() {
actualHandler := handlers.NewTagHandler(new(mock.Logger), &tag.Service{})
actualHandler := handlers.NewTagHandler(new(libmock.Logger), &tag.Service{})

s.NotNil(actualHandler)
})
Expand All @@ -39,7 +40,7 @@ func (s *TagHandlerTestSuite) Setup() {
templateService := tag.NewTemplateService(s.templateRepository)
service := tag.NewService(s.tagRepository, templateService)

s.handler = handlers.NewTagHandler(new(mock.Logger), service)
s.handler = handlers.NewTagHandler(new(libmock.Logger), service)
s.recorder = httptest.NewRecorder()
}

Expand All @@ -64,8 +65,8 @@ func (s *TagHandlerTestSuite) TestCreate() {
s.Setup()
t := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(template.URN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", &t).Return(tag.TemplateNotFoundError{URN: t.TemplateURN})
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", mock.Anything, &t).Return(tag.TemplateNotFoundError{URN: t.TemplateURN})

body, err := json.Marshal(t)
s.Require().NoError(err)
Expand All @@ -79,8 +80,8 @@ func (s *TagHandlerTestSuite) TestCreate() {
s.Setup()
t := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(template.URN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", &t).Return(tag.ValidationError{Err: errors.New("validation error")})
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", mock.Anything, &t).Return(tag.ValidationError{Err: errors.New("validation error")})

body, err := json.Marshal(t)
s.Require().NoError(err)
Expand All @@ -94,8 +95,8 @@ func (s *TagHandlerTestSuite) TestCreate() {
s.Setup()
t := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(template.URN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", &t).Return(errors.New("unexpected error during insert"))
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", mock.Anything, &t).Return(errors.New("unexpected error during insert"))

body, err := json.Marshal(t)
s.Require().NoError(err)
Expand All @@ -109,8 +110,8 @@ func (s *TagHandlerTestSuite) TestCreate() {
s.Setup()
t := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(template.URN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", &t).Return(tag.DuplicateError{})
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", mock.Anything, &t).Return(tag.DuplicateError{})

body, err := json.Marshal(t)
s.Require().NoError(err)
Expand All @@ -125,8 +126,8 @@ func (s *TagHandlerTestSuite) TestCreate() {
originalDomainTag := s.buildTag()

template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(template.URN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", &originalDomainTag).Return(nil)
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Create", mock.Anything, &originalDomainTag).Return(nil)

body, err := json.Marshal(originalDomainTag)
s.Require().NoError(err)
Expand Down Expand Up @@ -196,7 +197,7 @@ func (s *TagHandlerTestSuite) TestGetByRecord() {
"record_urn": recordURN,
})

s.tagRepository.On("Read", tag.Tag{RecordType: recordType, RecordURN: recordURN}).Return(nil, errors.New("unexpected error"))
s.tagRepository.On("Read", mock.Anything, tag.Tag{RecordType: recordType, RecordURN: recordURN}).Return(nil, errors.New("unexpected error"))

s.handler.GetByRecord(s.recorder, request)
s.Equal(http.StatusInternalServerError, s.recorder.Result().StatusCode)
Expand All @@ -213,7 +214,7 @@ func (s *TagHandlerTestSuite) TestGetByRecord() {
"type": recordType,
"record_urn": recordURN,
})
s.tagRepository.On("Read", tag.Tag{RecordType: recordType, RecordURN: recordURN}).Return([]tag.Tag{t}, nil)
s.tagRepository.On("Read", mock.Anything, tag.Tag{RecordType: recordType, RecordURN: recordURN}).Return([]tag.Tag{t}, nil)

expectedStatusCode := http.StatusOK
rsp, err := json.Marshal([]tag.Tag{t})
Expand Down Expand Up @@ -306,9 +307,7 @@ func (s *TagHandlerTestSuite) TestFindByRecordAndTemplate() {
"template_urn": template.URN,
})

s.templateRepository.On("Read", tag.Template{
URN: template.URN,
}).Return([]tag.Template{}, tag.TemplateNotFoundError{URN: template.URN})
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{}, tag.TemplateNotFoundError{URN: template.URN})

s.handler.FindByRecordAndTemplate(s.recorder, request)
s.Equal(http.StatusNotFound, s.recorder.Result().StatusCode)
Expand All @@ -327,11 +326,9 @@ func (s *TagHandlerTestSuite) TestFindByRecordAndTemplate() {
"template_urn": template.URN,
})

s.templateRepository.On("Read", tag.Template{
URN: template.URN,
}).Return([]tag.Template{template}, nil)
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)

s.tagRepository.On("Read", tag.Tag{
s.tagRepository.On("Read", mock.Anything, tag.Tag{
RecordType: recordType,
RecordURN: recordURN,
TemplateURN: template.URN,
Expand Down Expand Up @@ -359,11 +356,9 @@ func (s *TagHandlerTestSuite) TestFindByRecordAndTemplate() {
"template_urn": templateURN,
})

s.templateRepository.On("Read", tag.Template{
URN: templateURN,
}).Return([]tag.Template{template}, nil)
s.templateRepository.On("Read", mock.Anything, templateURN).Return([]tag.Template{template}, nil)

s.tagRepository.On("Read", tag.Tag{
s.tagRepository.On("Read", mock.Anything, tag.Tag{
RecordType: recordType,
RecordURN: recordURN,
TemplateURN: templateURN,
Expand All @@ -385,11 +380,9 @@ func (s *TagHandlerTestSuite) TestFindByRecordAndTemplate() {
"template_urn": t.TemplateURN,
})

s.templateRepository.On("Read", tag.Template{
URN: t.TemplateURN,
}).Return([]tag.Template{template}, nil)
s.templateRepository.On("Read", mock.Anything, t.TemplateURN).Return([]tag.Template{template}, nil)

s.tagRepository.On("Read", tag.Tag{
s.tagRepository.On("Read", mock.Anything, tag.Tag{
RecordType: t.RecordType,
RecordURN: t.RecordURN,
TemplateURN: t.TemplateURN,
Expand Down Expand Up @@ -519,13 +512,13 @@ func (s *TagHandlerTestSuite) TestUpdate() {
s.Setup()
t := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(t.TemplateURN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Read", tag.Tag{
s.templateRepository.On("Read", mock.Anything, t.TemplateURN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Read", mock.Anything, tag.Tag{
RecordType: t.RecordType,
RecordURN: t.RecordURN,
TemplateURN: t.TemplateURN,
}).Return([]tag.Tag{}, nil)
s.tagRepository.On("Update", &t).Return(errors.New("unexpected error during update"))
s.tagRepository.On("Update", mock.Anything, &t).Return(errors.New("unexpected error during update"))

body, err := json.Marshal(t)
s.Require().NoError(err)
Expand All @@ -545,13 +538,13 @@ func (s *TagHandlerTestSuite) TestUpdate() {
s.Setup()
t := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(t.TemplateURN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Read", tag.Tag{
s.templateRepository.On("Read", mock.Anything, t.TemplateURN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Read", mock.Anything, tag.Tag{
RecordType: t.RecordType,
RecordURN: t.RecordURN,
TemplateURN: t.TemplateURN,
}).Return([]tag.Tag{t}, nil)
s.tagRepository.On("Update", &t).Return(errors.New("unexpected error during update"))
s.tagRepository.On("Update", mock.Anything, &t).Return(errors.New("unexpected error during update"))

body, err := json.Marshal(t)
s.Require().NoError(err)
Expand All @@ -571,13 +564,13 @@ func (s *TagHandlerTestSuite) TestUpdate() {
s.Setup()
originalDomainTag := s.buildTag()
template := s.buildTemplate()
s.templateRepository.On("Read", s.templateQuery(template.URN)).Return([]tag.Template{template}, nil)
s.tagRepository.On("Read", tag.Tag{
s.templateRepository.On("Read", mock.Anything, template.URN).Return([]tag.Template{template}, nil)
s.tagRepository.On("Read", mock.Anything, tag.Tag{
RecordType: originalDomainTag.RecordType,
RecordURN: originalDomainTag.RecordURN,
TemplateURN: originalDomainTag.TemplateURN,
}).Return([]tag.Tag{originalDomainTag}, nil)
s.tagRepository.On("Update", &originalDomainTag).Return(nil)
s.tagRepository.On("Update", mock.Anything, &originalDomainTag).Return(nil)

body, err := json.Marshal(originalDomainTag)
s.Require().NoError(err)
Expand Down Expand Up @@ -703,8 +696,8 @@ func (s *TagHandlerTestSuite) TestDelete() {
"record_urn": recordURN,
"template_urn": templateURN,
})
s.templateRepository.On("Read", s.templateQuery(templateURN)).Return([]tag.Template{{}}, nil)
s.tagRepository.On("Delete", tag.Tag{
s.templateRepository.On("Read", mock.Anything, templateURN).Return([]tag.Template{{}}, nil)
s.tagRepository.On("Delete", mock.Anything, tag.Tag{
RecordType: recordType,
RecordURN: recordURN,
TemplateURN: templateURN,
Expand All @@ -726,8 +719,8 @@ func (s *TagHandlerTestSuite) TestDelete() {
"record_urn": recordURN,
"template_urn": templateURN,
})
s.templateRepository.On("Read", s.templateQuery(templateURN)).Return([]tag.Template{{}}, nil)
s.tagRepository.On("Delete", tag.Tag{
s.templateRepository.On("Read", mock.Anything, templateURN).Return([]tag.Template{{}}, nil)
s.tagRepository.On("Delete", mock.Anything, tag.Tag{
RecordType: recordType,
RecordURN: recordURN,
TemplateURN: templateURN,
Expand All @@ -748,8 +741,8 @@ func (s *TagHandlerTestSuite) TestDelete() {
"record_urn": recordURN,
"template_urn": templateURN,
})
s.templateRepository.On("Read", s.templateQuery(templateURN)).Return([]tag.Template{{}}, nil)
s.tagRepository.On("Delete", tag.Tag{
s.templateRepository.On("Read", mock.Anything, templateURN).Return([]tag.Template{{}}, nil)
s.tagRepository.On("Delete", mock.Anything, tag.Tag{
RecordType: recordType,
RecordURN: recordURN,
TemplateURN: templateURN,
Expand All @@ -760,12 +753,6 @@ func (s *TagHandlerTestSuite) TestDelete() {
})
}

func (s *TagHandlerTestSuite) templateQuery(template_urn string) tag.Template {
return tag.Template{
URN: template_urn,
}
}

func (s *TagHandlerTestSuite) buildTemplate() tag.Template {
return tag.Template{
URN: "governance_policy",
Expand Down
20 changes: 9 additions & 11 deletions api/handlers/tag_template_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (h *TagTemplateHandler) Create(w http.ResponseWriter, r *http.Request) {
writeJSONError(w, http.StatusBadRequest, err.Error())
return
}
err := h.service.Create(&requestBody)
err := h.service.Create(r.Context(), &requestBody)
if errors.As(err, new(tag.DuplicateTemplateError)) {
writeJSONError(w, http.StatusConflict, err.Error())
return
Expand All @@ -52,10 +52,7 @@ func (h *TagTemplateHandler) Create(w http.ResponseWriter, r *http.Request) {
// Index handles template read requests
func (h *TagTemplateHandler) Index(w http.ResponseWriter, r *http.Request) {
urn := r.URL.Query().Get("urn")
queryDomainTemplate := tag.Template{
URN: urn,
}
listOfDomainTemplate, err := h.service.Index(queryDomainTemplate)
listOfDomainTemplate, err := h.service.Index(r.Context(), urn)
if err != nil {
internalServerError(w, h.logger, fmt.Sprintf("error finding templates: %s", err.Error()))
return
Expand All @@ -66,8 +63,8 @@ func (h *TagTemplateHandler) Index(w http.ResponseWriter, r *http.Request) {
// Update handles template update requests
func (h *TagTemplateHandler) Update(w http.ResponseWriter, r *http.Request) {
params := mux.Vars(r)
urn, ok := params["template_urn"]
if !ok || urn == "" {
targetTemplateURN, ok := params["template_urn"]
if !ok || targetTemplateURN == "" {
writeJSONError(w, http.StatusBadRequest, errEmptyTemplateURN.Error())
return
}
Expand All @@ -77,8 +74,9 @@ func (h *TagTemplateHandler) Update(w http.ResponseWriter, r *http.Request) {
writeJSONError(w, http.StatusBadRequest, err.Error())
return
}
requestBody.URN = urn
err := h.service.Update(&requestBody)

requestBody.URN = targetTemplateURN
err := h.service.Update(r.Context(), targetTemplateURN, &requestBody)
if errors.As(err, new(tag.TemplateNotFoundError)) {
writeJSONError(w, http.StatusNotFound, err.Error())
return
Expand All @@ -104,7 +102,7 @@ func (h *TagTemplateHandler) Find(w http.ResponseWriter, r *http.Request) {
return
}

domainTemplate, err := h.service.Find(urn)
domainTemplate, err := h.service.Find(r.Context(), urn)
if errors.As(err, new(tag.TemplateNotFoundError)) {
writeJSONError(w, http.StatusNotFound, err.Error())
return
Expand All @@ -126,7 +124,7 @@ func (h *TagTemplateHandler) Delete(w http.ResponseWriter, r *http.Request) {
return
}

err := h.service.Delete(urn)
err := h.service.Delete(r.Context(), urn)
if errors.As(err, new(tag.TemplateNotFoundError)) {
writeJSONError(w, http.StatusNotFound, err.Error())
return
Expand Down
Loading

0 comments on commit 068d750

Please sign in to comment.