Add cache usage to sapcontrol gatherer (#328)

internal/factsengine/gatherers/sapcontrol.go

@@ -11,11 +11,13 @@ import (
 	"github.com/spf13/afero"
 	"github.com/trento-project/agent/internal/core/sapsystem"
 	"github.com/trento-project/agent/internal/core/sapsystem/sapcontrolapi"
+	"github.com/trento-project/agent/internal/factsengine/factscache"
 	"github.com/trento-project/agent/pkg/factsengine/entities"
 )
 
 const (
-	SapControlGathererName = "sapcontrol"
+	SapControlGathererName  = "sapcontrol"
+	SapControlGathererCache = "sapcontrol"
 )
 
 // nolint:gochecknoglobals
@@ -89,24 +91,58 @@ type SapControlInstance struct {
 type SapControlGatherer struct {
 	webService sapcontrolapi.WebServiceConnector
 	fs         afero.Fs
+	cache      *factscache.FactsCache
 }
 
 func NewDefaultSapControlGatherer() *SapControlGatherer {
 	webService := sapcontrolapi.WebServiceUnix{}
 	fs := afero.NewOsFs()
-	return NewSapControlGatherer(webService, fs)
+	return NewSapControlGatherer(webService, fs, nil)
 }
 
-func NewSapControlGatherer(webService sapcontrolapi.WebServiceConnector, fs afero.Fs) *SapControlGatherer {
+func NewSapControlGatherer(
+	webService sapcontrolapi.WebServiceConnector,
+	fs afero.Fs,
+	cache *factscache.FactsCache) *SapControlGatherer {
+
 	return &SapControlGatherer{
 		webService: webService,
 		fs:         fs,
+		cache:      cache,
+	}
+}
+
+func (s *SapControlGatherer) SetCache(cache *factscache.FactsCache) {
+	s.cache = cache
+}
+
+func memoizeSapcontrol(args ...interface{}) (interface{}, error) {
+	ctx, ok := args[0].(context.Context)
+	if !ok {
+		return nil, ImplementationError.Wrap("error using memoizeSapcontrol. Context must be 1st argument")
+	}
+
+	webService, ok := args[1].(sapcontrolapi.WebServiceConnector)
+	if !ok {
+		return nil, ImplementationError.Wrap("error using memoizeSapcontrol. WebServiceConnector must be 2nd argument")
+	}
+
+	instanceNumber, ok := args[2].(string)
+	if !ok {
+		return nil, ImplementationError.Wrap("error using memoizeSapcontrol. string must be 3rd argument")
+	}
+
+	webmethod, ok := args[3].(func(ctx context.Context, conn sapcontrolapi.WebService) (interface{}, error))
+	if !ok {
+		return nil, ImplementationError.Wrap("error using memoizeSapcontrol. webmethod func must be 4th argument")
 	}
+
+	conn := webService.New(instanceNumber)
+	return webmethod(ctx, conn)
 }
 
 func (s *SapControlGatherer) Gather(factsRequests []entities.FactRequest) ([]entities.Fact, error) {
 	ctx := context.Background()
-	cachedFacts := make(map[string]entities.Fact)
 
 	log.Infof("Starting %s facts gathering process", SapControlGathererName)
 	facts := []entities.Fact{}
@@ -132,25 +168,22 @@ func (s *SapControlGatherer) Gather(factsRequests []entities.FactRequest) ([]ent
 			continue
 		}
 
-		cachedFact, cacheHit := cachedFacts[factReq.Argument]
-
-		if cacheHit {
-			facts = append(facts, entities.Fact{
-				Name:    factReq.Name,
-				CheckID: factReq.CheckID,
-				Value:   cachedFact.Value,
-				Error:   cachedFact.Error,
-			})
-			continue
-		}
-
 		sapControlMap := make(SapControlMap)
 		for sid, instances := range foundSystems {
 			sapControlInstance := []SapControlInstance{}
 			for _, instanceData := range instances {
 				instanceName, instanceNumber := instanceData[0], instanceData[1]
-				conn := s.webService.New(instanceNumber)
-				output, err := webmethod(ctx, conn)
+				cacheEntry := fmt.Sprintf("%s:%s:%s:%s", SapControlGathererCache, factReq.Argument, sid, instanceNumber)
+				output, err := factscache.GetOrUpdate(
+					s.cache,
+					cacheEntry,
+					memoizeSapcontrol,
+					ctx,
+					s.webService,
+					instanceNumber,
+					webmethod,
+				)
+
 				if err != nil {
 					log.Error(SapcontrolWebmethodError.
 						Wrap(fmt.Sprintf("argument %s for %s/%s", factReq.Argument, sid, instanceName)).
@@ -177,7 +210,6 @@ func (s *SapControlGatherer) Gather(factsRequests []entities.FactRequest) ([]ent
 		} else {
 			fact = entities.NewFactGatheredWithRequest(factReq, factValue)
 		}
-		cachedFacts[factReq.Argument] = fact
 
 		facts = append(facts, fact)
 	}

internal/factsengine/gatherers/sapcontrol_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/suite"
+	"github.com/trento-project/agent/internal/factsengine/factscache"
 	"github.com/trento-project/agent/internal/factsengine/gatherers"
 	"github.com/trento-project/agent/pkg/factsengine/entities"
 
@@ -17,6 +18,7 @@ import (
 type SapControlGathererSuite struct {
 	suite.Suite
 	testFS     afero.Fs
+	cache      *factscache.FactsCache
 	webService *sapControlMocks.WebServiceConnector
 }
 
@@ -33,11 +35,12 @@ func (suite *SapControlGathererSuite) SetupSuite() {
 }
 
 func (suite *SapControlGathererSuite) SetupTest() {
+	suite.cache = factscache.NewFactsCache()
 	suite.webService = new(sapControlMocks.WebServiceConnector)
 }
 
 func (suite *SapControlGathererSuite) TestSapControlGathererArgumentErrors() {
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS, nil)
 
 	fr := []entities.FactRequest{
 		{
@@ -81,7 +84,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererArgumentErrors() {
 }
 
 func (suite *SapControlGathererSuite) TestSapControlGathererEmptyFileSystem() {
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, afero.NewMemMapFs())
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, afero.NewMemMapFs(), nil)
 
 	fr := []entities.FactRequest{{
 		Name:     "sapcontrol",
@@ -121,7 +124,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererCacheHit() {
 
 	suite.webService.On("New", "00").Return(mockWebService).Once()
 
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS, suite.cache)
 
 	fr := []entities.FactRequest{
 		{
@@ -212,6 +215,9 @@ func (suite *SapControlGathererSuite) TestSapControlGathererCacheHit() {
 	suite.EqualValues(expectedFacts, results)
 	suite.webService.AssertNumberOfCalls(suite.T(), "New", 1)
 	mockWebService.AssertNumberOfCalls(suite.T(), "GetProcessList", 1)
+
+	entries := suite.cache.Entries()
+	suite.ElementsMatch([]string{"sapcontrol:GetProcessList:PRD:00"}, entries)
 }
 
 func (suite *SapControlGathererSuite) TestSapControlGathererMultipleInstaces() {
@@ -247,7 +253,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererMultipleInstaces() {
 		On("New", "01").Return(mockWebService).
 		On("New", "02").Return(mockWebServiceError)
 
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, testFS, suite.cache)
 
 	fr := []entities.FactRequest{
 		{
@@ -341,6 +347,15 @@ func (suite *SapControlGathererSuite) TestSapControlGathererMultipleInstaces() {
 	results, err := gatherer.Gather(fr)
 	suite.NoError(err)
 	suite.EqualValues(expectedFacts, results)
+
+	entries := suite.cache.Entries()
+	expectedEntries := []string{
+		"sapcontrol:GetProcessList:PRD:00",
+		"sapcontrol:GetProcessList:PRD:10",
+		"sapcontrol:GetProcessList:QAS:01",
+		"sapcontrol:GetProcessList:QAS:02",
+	}
+	suite.ElementsMatch(expectedEntries, entries)
 }
 
 func (suite *SapControlGathererSuite) TestSapControlGathererGetSystemInstanceList() {
@@ -359,7 +374,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererGetSystemInstanceLis
 
 	suite.webService.On("New", "00").Return(mockWebService)
 
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS, suite.cache)
 
 	fr := []entities.FactRequest{
 		{
@@ -411,6 +426,9 @@ func (suite *SapControlGathererSuite) TestSapControlGathererGetSystemInstanceLis
 	results, err := gatherer.Gather(fr)
 	suite.NoError(err)
 	suite.EqualValues(expectedFacts, results)
+
+	entries := suite.cache.Entries()
+	suite.ElementsMatch([]string{"sapcontrol:GetSystemInstanceList:PRD:00"}, entries)
 }
 
 func (suite *SapControlGathererSuite) TestSapControlGathererGetVersionInfo() {
@@ -431,7 +449,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererGetVersionInfo() {
 
 	suite.webService.On("New", "00").Return(mockWebService)
 
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS, nil)
 
 	fr := []entities.FactRequest{
 		{
@@ -511,7 +529,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererHACheckConfig() {
 
 	suite.webService.On("New", "00").Return(mockWebService)
 
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS, nil)
 
 	fr := []entities.FactRequest{
 		{
@@ -573,7 +591,7 @@ func (suite *SapControlGathererSuite) TestSapControlGathererHAGetFailoverConfig(
 
 	suite.webService.On("New", "00").Return(mockWebService)
 
-	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS)
+	gatherer := gatherers.NewSapControlGatherer(suite.webService, suite.testFS, nil)
 
 	fr := []entities.FactRequest{
 		{