feat(project): introduce cache layer for project_metadata (#16891)

Implement cache layer for project_metadata and migrate metadata.Mgr to
pkg.ProjectMetaMgr.

Signed-off-by: chlins <chenyuzh@vmware.com>

src/controller/project/controller.go

@@ -70,7 +70,7 @@ type Controller interface {
 func NewController() Controller {
 	return &controller{
 		projectMgr:   pkg.ProjectMgr,
-		metaMgr:      metadata.Mgr,
+		metaMgr:      pkg.ProjectMetaMgr,
 		allowlistMgr: allowlist.NewDefaultManager(),
 		userMgr:      user.Mgr,
 	}

src/controller/project/metadata/controller.go

@@ -16,6 +16,8 @@ package metadata
 
 import (
 	"context"
+
+	"github.com/goharbor/harbor/src/pkg"
 	"github.com/goharbor/harbor/src/pkg/project/metadata"
 )
 
@@ -40,7 +42,7 @@ type Controller interface {
 // NewController creates an instance of the default controller
 func NewController() Controller {
 	return &controller{
-		mgr: metadata.Mgr,
+		mgr: pkg.ProjectMetaMgr,
 	}
 }
 

src/controller/scanner/base_controller.go

@@ -26,6 +26,7 @@ import (
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/q"
+	"github.com/goharbor/harbor/src/pkg"
 	"github.com/goharbor/harbor/src/pkg/project/metadata"
 	"github.com/goharbor/harbor/src/pkg/scan/dao/scanner"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
@@ -45,7 +46,7 @@ var DefaultController = New()
 func New() Controller {
 	return &basicController{
 		manager:    rscanner.New(),
-		proMetaMgr: metadata.Mgr,
+		proMetaMgr: pkg.ProjectMetaMgr,
 		clientPool: v1.DefaultClientPool,
 	}
 }

src/pkg/cached/manager.go

@@ -27,6 +27,8 @@ const (
 	ResourceTypeArtifact = "artifact"
 	// ResourceTypeProject defines project type.
 	ResourceTypeProject = "project"
+	// ResourceTypeProject defines project metadata type.
+	ResourceTypeProjectMeta = "project_metadata"
 	// ResourceTypeRepository defines repository type.
 	ResourceTypeRepository = "repository"
 )

src/pkg/cached/project/redis/manager_test.go

@@ -61,14 +61,14 @@ func (m *managerTestSuite) TestCount() {
 	m.Equal(int64(100), c)
 }
 
-func (m *managerTestSuite) List() {
+func (m *managerTestSuite) TestList() {
 	m.projectMgr.On("List", mock.Anything, mock.Anything).Return([]*models.Project{}, nil)
 	ps, err := m.cachedManager.List(m.ctx, q.New(q.KeyWords{}))
 	m.NoError(err)
 	m.ElementsMatch([]*models.Project{}, ps)
 }
 
-func (m *managerTestSuite) ListRoles() {
+func (m *managerTestSuite) TestListRoles() {
 	m.projectMgr.On("ListRoles", mock.Anything, mock.Anything, mock.Anything).Return([]int{1}, nil)
 	rs, err := m.cachedManager.ListRoles(m.ctx, 1, 1)
 	m.NoError(err)

src/pkg/cached/project_metadata/redis/manager.go

@@ -0,0 +1,182 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package redis
+
+import (
+	"context"
+	"strings"
+	"time"
+
+	libcache "github.com/goharbor/harbor/src/lib/cache"
+	"github.com/goharbor/harbor/src/lib/config"
+	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/lib/retry"
+	"github.com/goharbor/harbor/src/pkg/cached"
+	"github.com/goharbor/harbor/src/pkg/project/metadata"
+	"github.com/goharbor/harbor/src/pkg/project/metadata/models"
+)
+
+var _ CachedManager = &manager{}
+
+// CachedManager is the interface combines raw resource manager and cached manager for better extension.
+type CachedManager interface {
+	// Manager is the raw resource manager.
+	metadata.Manager
+	// Manager is the common interface for resource cache.
+	cached.Manager
+}
+
+// manager is the cached manager implemented by redis.
+type manager struct {
+	// delegator delegates the raw crud to DAO.
+	delegator metadata.Manager
+	// client returns the redis cache client.
+	client func() libcache.Cache
+	// keyBuilder builds cache object key.
+	keyBuilder *cached.ObjectKey
+	// lifetime is the cache life time.
+	lifetime time.Duration
+}
+
+// NewManager returns the redis cache manager.
+func NewManager(m metadata.Manager) *manager {
+	return &manager{
+		delegator:  m,
+		client:     func() libcache.Cache { return libcache.Default() },
+		keyBuilder: cached.NewObjectKey(cached.ResourceTypeProjectMeta),
+		lifetime:   time.Duration(config.CacheExpireHours()) * time.Hour,
+	}
+}
+
+func (m *manager) Add(ctx context.Context, projectID int64, meta map[string]string) error {
+	return m.delegator.Add(ctx, projectID, meta)
+}
+
+func (m *manager) List(ctx context.Context, name string, value string) ([]*models.ProjectMetadata, error) {
+	return m.delegator.List(ctx, name, value)
+}
+
+func (m *manager) Get(ctx context.Context, projectID int64, meta ...string) (map[string]string, error) {
+	key, err := m.keyBuilder.Format("projectID", projectID, "meta", strings.Join(meta, ","))
+	if err != nil {
+		return nil, err
+	}
+
+	result := make(map[string]string)
+	if err = m.client().Fetch(ctx, key, &result); err == nil {
+		return result, nil
+	}
+
+	log.Debugf("get project %d metadata from cache error: %v, will query from database.", projectID, err)
+
+	result, err = m.delegator.Get(ctx, projectID, meta...)
+	if err != nil {
+		return nil, err
+	}
+
+	if err = m.client().Save(ctx, key, &result, m.lifetime); err != nil {
+		// log error if save to cache failed
+		log.Debugf("save project metadata %v to cache error: %v", result, err)
+	}
+
+	return result, nil
+}
+
+func (m *manager) Delete(ctx context.Context, projectID int64, meta ...string) error {
+	// pass on delete operation
+	if err := m.delegator.Delete(ctx, projectID, meta...); err != nil {
+		return err
+	}
+	// clean cache
+	m.cleanUp(ctx, projectID, meta...)
+	return nil
+}
+
+func (m *manager) Update(ctx context.Context, projectID int64, meta map[string]string) error {
+	if err := m.delegator.Update(ctx, projectID, meta); err != nil {
+		return err
+	}
+	// clean cache
+	prefix, err := m.keyBuilder.Format("projectID", projectID)
+	if err != nil {
+		return err
+	}
+	// lookup all keys with projectID prefix
+	keys, err := m.client().Keys(ctx, prefix)
+	if err != nil {
+		return err
+	}
+
+	for _, key := range keys {
+		if err = retry.Retry(func() error { return m.client().Delete(ctx, key) }); err != nil {
+			log.Errorf("delete project metadata cache key %s error: %v", key, err)
+		}
+	}
+
+	return nil
+}
+
+// cleanUp cleans up data in cache.
+func (m *manager) cleanUp(ctx context.Context, projectID int64, meta ...string) {
+	key, err := m.keyBuilder.Format("projectID", projectID, "meta", strings.Join(meta, ","))
+	if err != nil {
+		log.Errorf("format project metadata key error: %v", err)
+	} else {
+		// retry to avoid dirty data
+		if err = retry.Retry(func() error { return m.client().Delete(ctx, key) }); err != nil {
+			log.Errorf("delete project metadata cache key %s error: %v", key, err)
+		}
+	}
+}
+
+func (m *manager) ResourceType(ctx context.Context) string {
+	return cached.ResourceTypeProjectMeta
+}
+
+func (m *manager) CountCache(ctx context.Context) (int64, error) {
+	// prefix is resource type
+	keys, err := m.client().Keys(ctx, m.ResourceType(ctx))
+	if err != nil {
+		return 0, err
+	}
+
+	return int64(len(keys)), nil
+}
+
+func (m *manager) DeleteCache(ctx context.Context, key string) error {
+	return m.client().Delete(ctx, key)
+}
+
+func (m *manager) FlushAll(ctx context.Context) error {
+	// prefix is resource type
+	keys, err := m.client().Keys(ctx, m.ResourceType(ctx))
+	if err != nil {
+		return err
+	}
+
+	var errs errors.Errors
+	for _, key := range keys {
+		if err = m.client().Delete(ctx, key); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if errs.Len() > 0 {
+		return errs
+	}
+
+	return nil
+}

src/pkg/cached/project_metadata/redis/manager_test.go

@@ -0,0 +1,123 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package redis
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/goharbor/harbor/src/lib/cache"
+	"github.com/goharbor/harbor/src/pkg/project/metadata/models"
+	testcache "github.com/goharbor/harbor/src/testing/lib/cache"
+	"github.com/goharbor/harbor/src/testing/mock"
+	testProjectMeta "github.com/goharbor/harbor/src/testing/pkg/project/metadata"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type managerTestSuite struct {
+	suite.Suite
+	cachedManager  CachedManager
+	projectMetaMgr *testProjectMeta.Manager
+	cache          *testcache.Cache
+	ctx            context.Context
+}
+
+func (m *managerTestSuite) SetupTest() {
+	m.projectMetaMgr = &testProjectMeta.Manager{}
+	m.cache = &testcache.Cache{}
+	m.cachedManager = NewManager(
+		m.projectMetaMgr,
+	)
+	m.cachedManager.(*manager).client = func() cache.Cache { return m.cache }
+	m.ctx = context.TODO()
+}
+
+func (m *managerTestSuite) TestAdd() {
+	m.projectMetaMgr.On("Add", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	err := m.cachedManager.Add(m.ctx, 1, map[string]string{})
+	m.NoError(err)
+}
+
+func (m *managerTestSuite) TestList() {
+	m.projectMetaMgr.On("List", mock.Anything, mock.Anything, mock.Anything).Return([]*models.ProjectMetadata{}, nil)
+	ps, err := m.cachedManager.List(m.ctx, "", "")
+	m.NoError(err)
+	m.ElementsMatch([]*models.ProjectMetadata{}, ps)
+}
+
+func (m *managerTestSuite) TestGet() {
+	// get from cache directly
+	m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
+	_, err := m.cachedManager.Get(m.ctx, 100)
+	m.NoError(err, "should get from cache")
+	m.projectMetaMgr.AssertNotCalled(m.T(), "Get", mock.Anything, mock.Anything)
+
+	// not found in cache, read from dao
+	m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(cache.ErrNotFound).Once()
+	m.cache.On("Save", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
+	m.projectMetaMgr.On("Get", mock.Anything, mock.Anything).Return(map[string]string{}, nil).Once()
+	_, err = m.cachedManager.Get(m.ctx, 100)
+	m.NoError(err, "should get from projectMetaMgr")
+	m.projectMetaMgr.AssertCalled(m.T(), "Get", mock.Anything, mock.Anything)
+}
+
+func (m *managerTestSuite) TestDelete() {
+	// delete from projectMgr error
+	errDelete := errors.New("delete failed")
+	m.projectMetaMgr.On("Delete", mock.Anything, mock.Anything).Return(errDelete).Once()
+	m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
+	err := m.cachedManager.Delete(m.ctx, 100)
+	m.ErrorIs(err, errDelete, "delete should error")
+	m.cache.AssertNotCalled(m.T(), "Delete", mock.Anything, mock.Anything)
+
+	// delete from projectMgr success
+	m.projectMetaMgr.On("Delete", mock.Anything, mock.Anything).Return(nil).Once()
+	m.cache.On("Fetch", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
+	m.cache.On("Delete", mock.Anything, mock.Anything).Return(nil).Twice()
+	err = m.cachedManager.Delete(m.ctx, 100)
+	m.NoError(err, "delete should success")
+	m.cache.AssertCalled(m.T(), "Delete", mock.Anything, mock.Anything)
+}
+
+func (m *managerTestSuite) TestResourceType() {
+	t := m.cachedManager.ResourceType(m.ctx)
+	m.Equal("project_metadata", t)
+}
+
+func (m *managerTestSuite) TestCountCache() {
+	m.cache.On("Keys", mock.Anything, mock.Anything).Return([]string{"1"}, nil).Once()
+	c, err := m.cachedManager.CountCache(m.ctx)
+	m.NoError(err)
+	m.Equal(int64(1), c)
+}
+
+func (m *managerTestSuite) TestDeleteCache() {
+	m.cache.On("Delete", mock.Anything, mock.Anything).Return(nil).Once()
+	err := m.cachedManager.DeleteCache(m.ctx, "key")
+	m.NoError(err)
+}
+
+func (m *managerTestSuite) TestFlushAll() {
+	m.cache.On("Keys", mock.Anything, mock.Anything).Return([]string{"1"}, nil).Once()
+	m.cache.On("Delete", mock.Anything, mock.Anything).Return(nil).Once()
+	err := m.cachedManager.FlushAll(m.ctx)
+	m.NoError(err)
+}
+
+func TestManager(t *testing.T) {
+	suite.Run(t, &managerTestSuite{})
+}

src/pkg/factory.go

@@ -19,8 +19,10 @@ import (
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	cachedArtifact "github.com/goharbor/harbor/src/pkg/cached/artifact/redis"
 	cachedProject "github.com/goharbor/harbor/src/pkg/cached/project/redis"
+	cachedProjectMeta "github.com/goharbor/harbor/src/pkg/cached/project_metadata/redis"
 	cachedRepo "github.com/goharbor/harbor/src/pkg/cached/repository/redis"
 	"github.com/goharbor/harbor/src/pkg/project"
+	"github.com/goharbor/harbor/src/pkg/project/metadata"
 	"github.com/goharbor/harbor/src/pkg/repository"
 )
 
@@ -30,6 +32,8 @@ var (
 	ArtifactMgr artifact.Manager
 	// ProjectMgr is the manager for project.
 	ProjectMgr project.Manager
+	// ProjectMetaMgr is the manager for project metadata.
+	ProjectMetaMgr metadata.Manager
 	// RepositoryMgr is the manager for repository.
 	RepositoryMgr repository.Manager
 )
@@ -39,6 +43,7 @@ func init() {
 	cacheEnabled := config.CacheEnabled()
 	initArtifactMgr(cacheEnabled)
 	initProjectMgr(cacheEnabled)
+	initProjectMetaMgr(cacheEnabled)
 	initRepositoryMgr(cacheEnabled)
 }
 
@@ -62,6 +67,15 @@ func initProjectMgr(cacheEnabled bool) {
 	}
 }
 
+func initProjectMetaMgr(cacheEnabled bool) {
+	projectMetaMgr := metadata.New()
+	if cacheEnabled {
+		ProjectMetaMgr = cachedProjectMeta.NewManager(projectMetaMgr)
+	} else {
+		ProjectMetaMgr = projectMetaMgr
+	}
+}
+
 func initRepositoryMgr(cacheEnabled bool) {
 	repoMgr := repository.New()
 	if cacheEnabled {

src/pkg/factory_test.go

@@ -20,8 +20,10 @@ import (
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	cachedArtifact "github.com/goharbor/harbor/src/pkg/cached/artifact/redis"
 	cachedProject "github.com/goharbor/harbor/src/pkg/cached/project/redis"
+	cachedProjectMeta "github.com/goharbor/harbor/src/pkg/cached/project_metadata/redis"
 	cachedRepo "github.com/goharbor/harbor/src/pkg/cached/repository/redis"
 	"github.com/goharbor/harbor/src/pkg/project"
+	"github.com/goharbor/harbor/src/pkg/project/metadata"
 	"github.com/goharbor/harbor/src/pkg/repository"
 	"github.com/stretchr/testify/assert"
 )
@@ -48,6 +50,17 @@ func TestInitProjectMgr(t *testing.T) {
 	assert.IsType(t, cachedProject.NewManager(project.New()), ProjectMgr)
 }
 
+func TestInitProjectMetaMgr(t *testing.T) {
+	// cache not enable
+	assert.NotNil(t, ProjectMetaMgr)
+	assert.IsType(t, metadata.New(), ProjectMetaMgr)
+
+	// cache enable
+	initProjectMetaMgr(true)
+	assert.NotNil(t, ProjectMetaMgr)
+	assert.IsType(t, cachedProjectMeta.NewManager(metadata.New()), ProjectMetaMgr)
+}
+
 func TestInitRepositoryMgr(t *testing.T) {
 	// cache not enable
 	assert.NotNil(t, RepositoryMgr)

src/pkg/project/metadata/manager.go

@@ -23,11 +23,6 @@ import (
 	"github.com/goharbor/harbor/src/pkg/project/metadata/models"
 )
 
-var (
-	// Mgr is the global project metadata manager
-	Mgr = New()
-)
-
 // Manager defines the operations that a project metadata manager should implement
 type Manager interface {
 	// Add metadatas for project specified by projectID

src/server/v2.0/handler/project.go

@@ -43,6 +43,7 @@ import (
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
+	"github.com/goharbor/harbor/src/pkg"
 	"github.com/goharbor/harbor/src/pkg/audit"
 	"github.com/goharbor/harbor/src/pkg/member"
 	"github.com/goharbor/harbor/src/pkg/project/metadata"
@@ -62,7 +63,7 @@ const defaultDaysToRetentionForProxyCacheProject = 7
 func newProjectAPI() *projectAPI {
 	return &projectAPI{
 		auditMgr:      audit.Mgr,
-		metadataMgr:   metadata.Mgr,
+		metadataMgr:   pkg.ProjectMetaMgr,
 		userCtl:       user.Ctl,
 		repositoryCtl: repository.Ctl,
 		projectCtl:    project.Ctl,

src/server/v2.0/handler/retention.go

@@ -12,6 +12,7 @@ import (
 	projectCtl "github.com/goharbor/harbor/src/controller/project"
 	retentionCtl "github.com/goharbor/harbor/src/controller/retention"
 	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/pkg"
 	"github.com/goharbor/harbor/src/pkg/project/metadata"
 	"github.com/goharbor/harbor/src/pkg/retention/policy"
 	"github.com/goharbor/harbor/src/pkg/task"
@@ -24,7 +25,7 @@ func newRetentionAPI() *retentionAPI {
 	return &retentionAPI{
 		projectCtl:   projectCtl.Ctl,
 		retentionCtl: retentionCtl.Ctl,
-		proMetaMgr:   metadata.Mgr,
+		proMetaMgr:   pkg.ProjectMetaMgr,
 	}
 }