From 1abe978e87e8bf0c9e028b00475fda1ecb62ac49 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Mon, 28 Jun 2021 18:55:31 +0800 Subject: [PATCH] refactor job log Move job service logger to new program model Signed-off-by: Wang Yan --- src/common/dao/joblog.go | 40 ---------- src/common/dao/joblog_test.go | 44 ----------- src/common/models/base.go | 1 - src/jobservice/logger/backend/db_logger.go | 7 +- src/jobservice/logger/getter/db_getter.go | 5 +- src/jobservice/logger/sweeper/db_sweeper.go | 5 +- src/pkg/joblog/dao/dao.go | 69 +++++++++++++++++ src/pkg/joblog/dao/dao_test.go | 53 +++++++++++++ src/pkg/joblog/manager.go | 47 ++++++++++++ src/pkg/joblog/managet_test.go | 76 +++++++++++++++++++ src/{common => pkg/joblog}/models/joblog.go | 5 ++ src/testing/pkg/joblog/dao/dao.go | 83 +++++++++++++++++++++ src/testing/pkg/joblog/manager.go | 83 +++++++++++++++++++++ src/testing/pkg/pkg.go | 2 + 14 files changed, 428 insertions(+), 92 deletions(-) delete mode 100644 src/common/dao/joblog.go delete mode 100644 src/common/dao/joblog_test.go create mode 100644 src/pkg/joblog/dao/dao.go create mode 100644 src/pkg/joblog/dao/dao_test.go create mode 100644 src/pkg/joblog/manager.go create mode 100644 src/pkg/joblog/managet_test.go rename src/{common => pkg/joblog}/models/joblog.go (89%) create mode 100644 src/testing/pkg/joblog/dao/dao.go create mode 100644 src/testing/pkg/joblog/manager.go diff --git a/src/common/dao/joblog.go b/src/common/dao/joblog.go deleted file mode 100644 index 7fb0882ea..000000000 --- a/src/common/dao/joblog.go +++ /dev/null @@ -1,40 +0,0 @@ -package dao - -import ( - "github.com/astaxie/beego/orm" - "github.com/goharbor/harbor/src/common/models" - "time" -) - -// CreateOrUpdateJobLog ... -func CreateOrUpdateJobLog(log *models.JobLog) (int64, error) { - o := GetOrmer() - count, err := o.InsertOrUpdate(log, "job_uuid") - if err != nil { - return 0, err - } - - return count, nil -} - -// GetJobLog ... -func GetJobLog(uuid string) (*models.JobLog, error) { - o := GetOrmer() - jl := models.JobLog{UUID: uuid} - err := o.Read(&jl, "UUID") - if err == orm.ErrNoRows { - return nil, err - } - return &jl, nil -} - -// DeleteJobLogsBefore ... -func DeleteJobLogsBefore(t time.Time) (int64, error) { - o := GetOrmer() - sql := `delete from job_log where creation_time < ?` - res, err := o.Raw(sql, t).Exec() - if err != nil { - return 0, err - } - return res.RowsAffected() -} diff --git a/src/common/dao/joblog_test.go b/src/common/dao/joblog_test.go deleted file mode 100644 index 42d9ff851..000000000 --- a/src/common/dao/joblog_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package dao - -import ( - "testing" - - "github.com/goharbor/harbor/src/common/models" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "time" -) - -func TestMethodsOfJobLog(t *testing.T) { - uuid := "uuid_for_unit_test" - now := time.Now() - content := "content for unit text" - jobLog := &models.JobLog{ - UUID: uuid, - CreationTime: now, - Content: content, - } - - // create - _, err := CreateOrUpdateJobLog(jobLog) - require.Nil(t, err) - - // update - updateContent := "content for unit text update" - jobLog.Content = updateContent - _, err = CreateOrUpdateJobLog(jobLog) - require.Nil(t, err) - - // get - log, err := GetJobLog(uuid) - require.Nil(t, err) - assert.Equal(t, now.Second(), log.CreationTime.Second()) - assert.Equal(t, updateContent, log.Content) - assert.Equal(t, jobLog.LogID, log.LogID) - - // delete - count, err := DeleteJobLogsBefore(time.Now().Add(time.Duration(time.Minute))) - require.Nil(t, err) - assert.Equal(t, int64(1), count) -} diff --git a/src/common/models/base.go b/src/common/models/base.go index c290c687d..31f619090 100644 --- a/src/common/models/base.go +++ b/src/common/models/base.go @@ -23,7 +23,6 @@ func init() { new(User), new(Role), new(ResourceLabel), - new(JobLog), new(OIDCUser), ) } diff --git a/src/jobservice/logger/backend/db_logger.go b/src/jobservice/logger/backend/db_logger.go index 1f4ac05d5..991ef3ad7 100644 --- a/src/jobservice/logger/backend/db_logger.go +++ b/src/jobservice/logger/backend/db_logger.go @@ -3,9 +3,10 @@ package backend import ( "bufio" "bytes" - "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/pkg/joblog" + "github.com/goharbor/harbor/src/pkg/joblog/models" ) // DBLogger is an implementation of logger.Interface. @@ -47,7 +48,7 @@ func (dbl *DBLogger) Close() error { Content: dbl.buffer.String(), } - _, err = dao.CreateOrUpdateJobLog(&jobLog) + _, err = joblog.Mgr.Create(orm.Context(), &jobLog) if err != nil { return err } diff --git a/src/jobservice/logger/getter/db_getter.go b/src/jobservice/logger/getter/db_getter.go index de5171307..f74a35d62 100644 --- a/src/jobservice/logger/getter/db_getter.go +++ b/src/jobservice/logger/getter/db_getter.go @@ -3,8 +3,9 @@ package getter import ( "errors" "fmt" + "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/pkg/joblog" - "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/jobservice/errs" ) @@ -23,7 +24,7 @@ func (dbg *DBGetter) Retrieve(logID string) ([]byte, error) { return nil, errors.New("empty log identify") } - jobLog, err := dao.GetJobLog(logID) + jobLog, err := joblog.Mgr.Get(orm.Context(), logID) if err != nil { // Other errors have been ignored by GetJobLog() return nil, errs.NoObjectFoundError(fmt.Sprintf("log entity: %s", logID)) diff --git a/src/jobservice/logger/sweeper/db_sweeper.go b/src/jobservice/logger/sweeper/db_sweeper.go index 521675392..f8e7f36b7 100644 --- a/src/jobservice/logger/sweeper/db_sweeper.go +++ b/src/jobservice/logger/sweeper/db_sweeper.go @@ -2,7 +2,8 @@ package sweeper import ( "fmt" - "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/pkg/joblog" "time" ) @@ -28,7 +29,7 @@ func (dbs *DBSweeper) Sweep() (int, error) { // Start to sweep logs before := time.Now().Add(time.Duration(dbs.duration) * oneDay * -1) - count, err := dao.DeleteJobLogsBefore(before) + count, err := joblog.Mgr.DeleteBefore(orm.Context(), before) if err != nil { return 0, fmt.Errorf("sweep logs in DB failed before %s with error: %s", before, err) diff --git a/src/pkg/joblog/dao/dao.go b/src/pkg/joblog/dao/dao.go new file mode 100644 index 000000000..2e6348443 --- /dev/null +++ b/src/pkg/joblog/dao/dao.go @@ -0,0 +1,69 @@ +package dao + +import ( + "context" + "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/pkg/joblog/models" + "time" +) + +// DAO is the data access object for job log +type DAO interface { + // Create the job log + Create(ctx context.Context, jobLog *models.JobLog) (id int64, err error) + // Get the job log specified by UUID + Get(ctx context.Context, uuid string) (jobLog *models.JobLog, err error) + // DeleteBefore the job log specified by time + DeleteBefore(ctx context.Context, t time.Time) (id int64, err error) +} + +// New returns an instance of the default DAO +func New() DAO { + return &dao{} +} + +type dao struct{} + +// Create ... +func (d *dao) Create(ctx context.Context, jobLog *models.JobLog) (int64, error) { + ormer, err := orm.FromContext(ctx) + if err != nil { + return 0, err + } + count, err := ormer.InsertOrUpdate(jobLog, "job_uuid") + if err != nil { + return 0, err + } + + return count, nil +} + +// Get ... +func (d *dao) Get(ctx context.Context, uuid string) (jobLog *models.JobLog, err error) { + ormer, err := orm.FromContext(ctx) + if err != nil { + return nil, err + } + jl := models.JobLog{UUID: uuid} + err = ormer.Read(&jl, "UUID") + if e := orm.AsNotFoundError(err, "no job log founded"); e != nil { + log.Warningf("no job log founded. Query condition, uuid: %s, err: %v", uuid, e) + return nil, err + } + return &jl, nil +} + +// DeleteBefore ... +func (d *dao) DeleteBefore(ctx context.Context, t time.Time) (id int64, err error) { + ormer, err := orm.FromContext(ctx) + if err != nil { + return 0, err + } + sql := `delete from job_log where creation_time < ?` + res, err := ormer.Raw(sql, t).Exec() + if err != nil { + return 0, err + } + return res.RowsAffected() +} diff --git a/src/pkg/joblog/dao/dao_test.go b/src/pkg/joblog/dao/dao_test.go new file mode 100644 index 000000000..4f01c1aab --- /dev/null +++ b/src/pkg/joblog/dao/dao_test.go @@ -0,0 +1,53 @@ +package dao + +import ( + "github.com/goharbor/harbor/src/pkg/joblog/models" + htesting "github.com/goharbor/harbor/src/testing" + "time" +) + +type DaoTestSuite struct { + htesting.Suite + dao DAO +} + +func (suite *DaoTestSuite) SetupSuite() { + suite.Suite.SetupSuite() + suite.Suite.ClearTables = []string{"job_log"} + suite.dao = New() +} + +func (suite *DaoTestSuite) TestMethodsOfJobLog() { + ctx := suite.Context() + + uuid := "uuid_for_unit_test" + now := time.Now() + content := "content for unit text" + jobLog := &models.JobLog{ + UUID: uuid, + CreationTime: now, + Content: content, + } + + // create + _, err := suite.dao.Create(ctx, jobLog) + suite.Nil(err) + + // update + updateContent := "content for unit text update" + jobLog.Content = updateContent + _, err = suite.dao.Create(ctx, jobLog) + suite.Nil(err) + + // get + log, err := suite.dao.Get(ctx, uuid) + suite.Nil(err) + suite.Equal(now.Second(), log.CreationTime.Second()) + suite.Equal(updateContent, log.Content) + suite.Equal(jobLog.LogID, log.LogID) + + // delete + count, err := suite.dao.DeleteBefore(ctx, time.Now().Add(time.Duration(time.Minute))) + suite.Nil(err) + suite.Equal(int64(1), count) +} diff --git a/src/pkg/joblog/manager.go b/src/pkg/joblog/manager.go new file mode 100644 index 000000000..748662f8d --- /dev/null +++ b/src/pkg/joblog/manager.go @@ -0,0 +1,47 @@ +package joblog + +import ( + "context" + "github.com/goharbor/harbor/src/pkg/joblog/dao" + "github.com/goharbor/harbor/src/pkg/joblog/models" + "time" +) + +// Mgr is the global job log manager instance +var Mgr = New() + +// Manager is used for job log management +type Manager interface { + // Get the job log specified by ID + Get(ctx context.Context, uuid string) (jobLog *models.JobLog, err error) + // Create the job log + Create(ctx context.Context, jobLog *models.JobLog) (id int64, err error) + // DeleteBefore the job log specified by time + DeleteBefore(ctx context.Context, t time.Time) (id int64, err error) +} + +// New returns a default implementation of Manager +func New() Manager { + return &manager{ + dao: dao.New(), + } +} + +type manager struct { + dao dao.DAO +} + +// Get ... +func (m *manager) Get(ctx context.Context, uuid string) (jobLog *models.JobLog, err error) { + return m.dao.Get(ctx, uuid) +} + +// Create ... +func (m *manager) Create(ctx context.Context, jobLog *models.JobLog) (id int64, err error) { + return m.dao.Create(ctx, jobLog) +} + +// DeleteJobLogsBefore ... +func (m *manager) DeleteBefore(ctx context.Context, t time.Time) (id int64, err error) { + return m.dao.DeleteBefore(ctx, t) +} diff --git a/src/pkg/joblog/managet_test.go b/src/pkg/joblog/managet_test.go new file mode 100644 index 000000000..1e1f57a4f --- /dev/null +++ b/src/pkg/joblog/managet_test.go @@ -0,0 +1,76 @@ +package joblog + +import ( + "context" + "github.com/goharbor/harbor/src/pkg/joblog/models" + "github.com/goharbor/harbor/src/testing/mock" + "github.com/goharbor/harbor/src/testing/pkg/joblog/dao" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "os" + "testing" +) + +type managerTestingSuite struct { + suite.Suite + t *testing.T + assert *assert.Assertions + require *require.Assertions + mockJobLogDao *dao.DAO +} + +func (m *managerTestingSuite) SetupSuite() { + m.t = m.T() + m.assert = assert.New(m.t) + m.require = require.New(m.t) + + err := os.Setenv("RUN_MODE", "TEST") + m.require.Nil(err) +} + +func (m *managerTestingSuite) TearDownSuite() { + err := os.Unsetenv("RUN_MODE") + m.require.Nil(err) +} + +func (m *managerTestingSuite) SetupTest() { + m.mockJobLogDao = &dao.DAO{} + Mgr = &manager{ + dao: m.mockJobLogDao, + } +} + +func TestManagerTestingSuite(t *testing.T) { + suite.Run(t, &managerTestingSuite{}) +} + +func (m *managerTestingSuite) TestCreate() { + m.mockJobLogDao.On("Create", mock.Anything, mock.Anything).Return(int64(1), nil) + id, err := Mgr.Create(context.Background(), &models.JobLog{}) + m.mockJobLogDao.AssertCalled(m.t, "Create", mock.Anything, mock.Anything) + m.require.Nil(err) + m.assert.Equal(int64(1), id) +} + +func (m *managerTestingSuite) TestGet() { + m.mockJobLogDao.On("Get", mock.Anything, mock.Anything).Return(&models.JobLog{ + LogID: 1, + UUID: "1234", + Content: "test get", + }, nil) + ir, err := Mgr.Get(context.Background(), "1234") + m.mockJobLogDao.AssertCalled(m.t, "Get", mock.Anything, mock.Anything) + m.require.Nil(err) + m.require.NotNil(ir) + m.assert.Equal(1, ir.LogID) +} + +func (m *managerTestingSuite) TestDeleteBefore() { + m.mockJobLogDao.On("DeleteBefore", mock.Anything, mock.Anything).Return(int64(1), nil) + _, err := Mgr.DeleteBefore(context.Background(), time.Now()) + m.mockJobLogDao.AssertCalled(m.t, "DeleteBefore", mock.Anything, mock.Anything) + m.require.Nil(err) +} diff --git a/src/common/models/joblog.go b/src/pkg/joblog/models/joblog.go similarity index 89% rename from src/common/models/joblog.go rename to src/pkg/joblog/models/joblog.go index b004621ee..b1acabea3 100644 --- a/src/common/models/joblog.go +++ b/src/pkg/joblog/models/joblog.go @@ -1,9 +1,14 @@ package models import ( + "github.com/astaxie/beego/orm" "time" ) +func init() { + orm.RegisterModel(&JobLog{}) +} + // JobLogTable is the name of the table that record the job execution result. const JobLogTable = "job_log" diff --git a/src/testing/pkg/joblog/dao/dao.go b/src/testing/pkg/joblog/dao/dao.go new file mode 100644 index 000000000..ceab3332a --- /dev/null +++ b/src/testing/pkg/joblog/dao/dao.go @@ -0,0 +1,83 @@ +// Code generated by mockery v2.1.0. DO NOT EDIT. + +package dao + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + + models "github.com/goharbor/harbor/src/pkg/joblog/models" + + time "time" +) + +// DAO is an autogenerated mock type for the DAO type +type DAO struct { + mock.Mock +} + +// Create provides a mock function with given fields: ctx, jobLog +func (_m *DAO) Create(ctx context.Context, jobLog *models.JobLog) (int64, error) { + ret := _m.Called(ctx, jobLog) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, *models.JobLog) int64); ok { + r0 = rf(ctx, jobLog) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *models.JobLog) error); ok { + r1 = rf(ctx, jobLog) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// DeleteBefore provides a mock function with given fields: ctx, t +func (_m *DAO) DeleteBefore(ctx context.Context, t time.Time) (int64, error) { + ret := _m.Called(ctx, t) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, time.Time) int64); ok { + r0 = rf(ctx, t) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, time.Time) error); ok { + r1 = rf(ctx, t) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Get provides a mock function with given fields: ctx, uuid +func (_m *DAO) Get(ctx context.Context, uuid string) (*models.JobLog, error) { + ret := _m.Called(ctx, uuid) + + var r0 *models.JobLog + if rf, ok := ret.Get(0).(func(context.Context, string) *models.JobLog); ok { + r0 = rf(ctx, uuid) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*models.JobLog) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, uuid) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/src/testing/pkg/joblog/manager.go b/src/testing/pkg/joblog/manager.go new file mode 100644 index 000000000..8f6e9b877 --- /dev/null +++ b/src/testing/pkg/joblog/manager.go @@ -0,0 +1,83 @@ +// Code generated by mockery v2.1.0. DO NOT EDIT. + +package joblog + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + + models "github.com/goharbor/harbor/src/pkg/joblog/models" + + time "time" +) + +// Manager is an autogenerated mock type for the Manager type +type Manager struct { + mock.Mock +} + +// Create provides a mock function with given fields: ctx, jobLog +func (_m *Manager) Create(ctx context.Context, jobLog *models.JobLog) (int64, error) { + ret := _m.Called(ctx, jobLog) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, *models.JobLog) int64); ok { + r0 = rf(ctx, jobLog) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *models.JobLog) error); ok { + r1 = rf(ctx, jobLog) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// DeleteBefore provides a mock function with given fields: ctx, t +func (_m *Manager) DeleteBefore(ctx context.Context, t time.Time) (int64, error) { + ret := _m.Called(ctx, t) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, time.Time) int64); ok { + r0 = rf(ctx, t) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, time.Time) error); ok { + r1 = rf(ctx, t) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Get provides a mock function with given fields: ctx, uuid +func (_m *Manager) Get(ctx context.Context, uuid string) (*models.JobLog, error) { + ret := _m.Called(ctx, uuid) + + var r0 *models.JobLog + if rf, ok := ret.Get(0).(func(context.Context, string) *models.JobLog); ok { + r0 = rf(ctx, uuid) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*models.JobLog) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, uuid) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/src/testing/pkg/pkg.go b/src/testing/pkg/pkg.go index 14bee9dae..0c0847913 100644 --- a/src/testing/pkg/pkg.go +++ b/src/testing/pkg/pkg.go @@ -51,3 +51,5 @@ package pkg //go:generate mockery --case snake --dir ../../pkg/replication --name Manager --output ./replication --outpkg manager //go:generate mockery --case snake --dir ../../pkg/label --name Manager --output ./label --outpkg label //go:generate mockery --case snake --dir ../../pkg/label/dao --name DAO --output ./label/dao --outpkg dao +//go:generate mockery --case snake --dir ../../pkg/joblog --name Manager --output ./joblog --outpkg joblog +//go:generate mockery --case snake --dir ../../pkg/joblog/dao --name DAO --output ./joblog/dao --outpkg dao