refactor: decouple business logic from db logic #90
Conversation
| @@ -73,13 +72,14 @@ func RunServer(c *domain.Config) error { | |||
| return err | |||
| } | |||
|
|
|||
| store, err := store.New(&c.DB) | |||
| gormDB, err := store.New(&c.DB) | |||
There was a problem hiding this comment.
@whoAbhishekSah this can simply be called db.
| ) | ||
|
|
||
| type NamespaceRepository interface { | ||
| Migrate() error |
There was a problem hiding this comment.
@whoAbhishekSah Let's remove migrate from repositories and we can have a migrate method that takes care of overall database migration.
There was a problem hiding this comment.
I feel repository being the single interface for all DB-related actions, it should have the Migrate method, so that caller can use it. Otherwise, the caller will have to know the DB details.
There was a problem hiding this comment.
Migrate is a bootstrap process which is something services will not use. Check out Guardian ref. Only database implementation should know about migration.
There was a problem hiding this comment.
I'll fix this one while refactoring each package one by one.
cmd/alert.go
Outdated
| @@ -94,9 +94,9 @@ func listAlertsCmd(c *configuration) *cobra.Command { | |||
| } | |||
|
|
|||
| cmd.Flags().StringVar(&providerName, "provider-name", "", "provider name") | |||
| cmd.MarkFlagRequired("provider-name") | |||
| _ = cmd.MarkFlagRequired("provider-name") | |||
There was a problem hiding this comment.
what happens when we weren't using the _ = assignment?
There was a problem hiding this comment.
didn't check what happened, it was added as part of linting issues.
There was a problem hiding this comment.
We can disable this linting rule if needed. But lets avoid adding it.
There was a problem hiding this comment.
@whoAbhishekSah lets avoid this change.
| List() ([]*model.Namespace, error) | ||
| Create(*model.Namespace) (*model.Namespace, error) | ||
| Get(uint64) (*model.Namespace, error) | ||
| Update(*model.Namespace) (*model.Namespace, error) | ||
| Delete(uint64) error |
There was a problem hiding this comment.
repository should accept and return *domain.Namespace instead of *model.Namespace so we don't have to use namespaceModel.ToDomain(), namespaceModel.FromDomain() or any other model operations inside service. It should be inside the repository only.
plus, when it's only used inside repository, ToDomain() and FromDomain() could be private methods as well
| @@ -1,14 +1,13 @@ | |||
| package alerts | |||
| package model | |||
There was a problem hiding this comment.
can get rid this model package and have the model definitions inside store/postgres package all together, since the model definitions are specific to postgres and/or gorm
There was a problem hiding this comment.
fyi we're going to do the same for guardian
whoAbhishekSah
changed the title
whoAbhishekSah
force-pushed
the
refactor/decouple_business_logic_from_db_logic
branch
from
8c3cf5d
to
a61957d
Compare
* refactor: rename metric to telemetry * refactor: rename handler v1 to v1beta1 * refactor: rename helper to utils * build: add golangci-lint in CI pipeline and fix linting issues * refactor: remove redundant codeexchange package * refactor: cleanup go.mod file * refactor: move provider repository to postgres store * refactor: move namespace repository to postgres store * refactor: move templates repository to postgres store * refactor: move receiver repository to postgres store * refactor: move all repository interface definitions in one file * refactor: remove dependency from gorm in services by injecting repositories * refactor: move alerts repository to postgres store * refactor: move slack notifier client to provider package * refactor: update provider repository to operate on domain definition * chore: remove errcheck for CLI flags and disable linting rule
* refactor: rename metric to telemetry * refactor: rename handler v1 to v1beta1 * refactor: rename helper to utils * build: add golangci-lint in CI pipeline and fix linting issues * refactor: remove redundant codeexchange package * refactor: cleanup go.mod file * refactor: move provider repository to postgres store * refactor: move namespace repository to postgres store * refactor: move templates repository to postgres store * refactor: move receiver repository to postgres store * refactor: move all repository interface definitions in one file * refactor: remove dependency from gorm in services by injecting repositories * refactor: move alerts repository to postgres store * refactor: move slack notifier client to provider package * refactor: update provider repository to operate on domain definition * chore: remove errcheck for CLI flags and disable linting rule
Refactor packages to abstract out repositories into Store for