Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(database): move service logic into separate package #816

Merged
merged 28 commits into from
May 15, 2023

Conversation

jbrockopp
Copy link
Contributor

@jbrockopp jbrockopp commented Apr 16, 2023

Based off of #574, #663, #687, #692, #721, #722, #782 and #810

This change continues the refactor efforts initially introduced in the above PRs.

This adds a new service package to the github.com/go-vela/server/database package.

This contains a ServiceInterface interface declaring all functions necessary for service based interactions with the database:

// ServiceInterface represents the Vela interface for service
// functions with the supported Database backends.
//
//nolint:revive // ignore name stutter
type ServiceInterface interface {
// Service Data Definition Language Functions
//
// https://en.wikipedia.org/wiki/Data_definition_language
// CreateServiceTable defines a function that creates the services table.
CreateServiceTable(string) error
// Service Data Manipulation Language Functions
//
// https://en.wikipedia.org/wiki/Data_manipulation_language
// CountServices defines a function that gets the count of all services.
CountServices() (int64, error)
// CountServicesForBuild defines a function that gets the count of services by build ID.
CountServicesForBuild(*library.Build, map[string]interface{}) (int64, error)
// CreateService defines a function that creates a new service.
CreateService(*library.Service) error
// DeleteService defines a function that deletes an existing service.
DeleteService(*library.Service) error
// GetService defines a function that gets a service by ID.
GetService(int64) (*library.Service, error)
// GetServiceForBuild defines a function that gets a service by number and build ID.
GetServiceForBuild(*library.Build, int) (*library.Service, error)
// ListServices defines a function that gets a list of all services.
ListServices() ([]*library.Service, error)
// ListServicesForBuild defines a function that gets a list of services by build ID.
ListServicesForBuild(*library.Build, map[string]interface{}, int, int) ([]*library.Service, int64, error)
// ListServiceImageCount defines a function that gets a list of all service images and the count of their occurrence.
ListServiceImageCount() (map[string]float64, error)
// ListServiceStatusCount defines a function that gets a list of all service statuses and the count of their occurrence.
ListServiceStatusCount() (map[string]float64, error)
// UpdateService defines a function that updates an existing service.
UpdateService(*library.Service) error
}

NOTE: The other refactor PRs above leveraged using the keyword "service" when referring to interfaces.

However, when I started that for this package, it left me with an interface{} type named ServiceService.

I thought this name wasn't great so I elected to refactor all of them from *Service -> *Interface.

This package also contains the engine which implements the above service interface:

// engine represents the service functionality that implements the ServiceInterface interface.
engine struct {
// engine configuration settings used in service functions
config *config
// gorm.io/gorm database client used in service functions
//
// https://pkg.go.dev/gorm.io/gorm#DB
client *gorm.DB
// sirupsen/logrus logger used in service functions
//
// https://pkg.go.dev/github.com/sirupsen/logrus#Entry
logger *logrus.Entry
}
)

This engine contains no raw SQL queries for integrating with the services table.

Instead, we leverage our DB library's (https://gorm.io/) agnostic abstraction for integrating with that table.

@jbrockopp jbrockopp added the enhancement Indicates an improvement to a feature label Apr 16, 2023
@jbrockopp jbrockopp self-assigned this Apr 16, 2023
@codecov
Copy link

codecov bot commented Apr 16, 2023

Codecov Report

Merging #816 (2cd9863) into main (cba432f) will decrease coverage by 0.22%.
The diff coverage is 82.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #816      +/-   ##
==========================================
- Coverage   57.70%   57.49%   -0.22%     
==========================================
  Files         263      271       +8     
  Lines       15922    15892      -30     
==========================================
- Hits         9188     9137      -51     
- Misses       6319     6331      +12     
- Partials      415      424       +9     
Impacted Files Coverage Δ
api/build.go 1.48% <0.00%> (ø)
api/metrics.go 0.00% <0.00%> (ø)
api/service.go 0.00% <0.00%> (ø)
api/step.go 0.00% <0.00%> (ø)
api/webhook.go 0.00% <0.00%> (ø)
database/hook/hook.go 51.72% <ø> (ø)
database/log/log.go 51.72% <ø> (ø)
database/pipeline/pipeline.go 51.72% <ø> (ø)
database/repo/repo.go 51.72% <ø> (ø)
database/secret/secret.go 51.72% <ø> (ø)
... and 27 more

@jbrockopp jbrockopp marked this pull request as ready for review April 22, 2023 00:35
@jbrockopp jbrockopp requested a review from a team as a code owner April 22, 2023 00:35
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎇

@KellyMerrick
Copy link
Contributor

@jbrockopp We plan to review this within the next couple weeks.

@wass3rw3rk
Copy link
Member

wass3rw3rk commented May 10, 2023

per githubstatus.com, github is having some issues currently. try rerunning workflows later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates an improvement to a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants